Skip to content

Commit

Permalink
Better dumping via private IP when bastion is not set
Browse files Browse the repository at this point in the history
Previously this would always fail in a confusing way,
regardless of whether we had connectivity,
because we tried to connect to an empty-string host.

Now we are more explicit about the error,
and will at least try to connect directly.
  • Loading branch information
justinsb committed Feb 19, 2025
1 parent 12931dd commit 3ea73f4
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 5 deletions.
3 changes: 3 additions & 0 deletions cmd/kops/toolbox_dump.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,9 @@ func RunToolboxDump(ctx context.Context, f commandutils.Factory, out io.Writer,
HostKeyCallback: ssh.InsecureIgnoreHostKey(),
}

klog.Infof("will SSH using username %q", sshConfig.User)
klog.Infof("ssh auth methods %v", sshConfig.Auth)

keyRing := agent.NewKeyring()
defer func(keyRing agent.Agent) {
_ = keyRing.RemoveAll()
Expand Down
30 changes: 25 additions & 5 deletions pkg/dump/dumper.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,12 @@ func (d *logDumper) dumpRegistered(ctx context.Context, node *corev1.Node) error
if publicIP != "" {
return d.dumpNode(ctx, node.Name, publicIP, false)
} else {
return d.dumpNode(ctx, node.Name, privateIP, true)
useBastion := true
if !d.sshClientFactory.HasBastion() {
klog.Warningf("no bastion address set, will attempt to connect to node %s directly via private IP %v", node.Name, privateIP)
useBastion = false
}
return d.dumpNode(ctx, node.Name, privateIP, useBastion)
}
}

Expand Down Expand Up @@ -274,7 +279,12 @@ type sshClient interface {

// sshClientFactory is an interface abstracting to a node over SSH
type sshClientFactory interface {
// Dial creates a new sshClient
Dial(ctx context.Context, host string, useBastion bool) (sshClient, error)

// HasBastion returns true if the sshClientFactory has a bastion configured.
// Calling Dial with useBastion=true will return an error if there is no bastion.
HasBastion() bool
}

// logDumperNode holds state for a particular node we are dumping
Expand Down Expand Up @@ -540,21 +550,31 @@ type sshClientFactoryImplementation struct {

var _ sshClientFactory = &sshClientFactoryImplementation{}

// HasBastion implements sshClientFactory::HasBastion
func (f *sshClientFactoryImplementation) HasBastion() bool {
return f.bastion != ""
}

// Dial implements sshClientFactory::Dial
func (f *sshClientFactoryImplementation) Dial(ctx context.Context, host string, useBastion bool) (sshClient, error) {
var addr string
addr := host
if useBastion {
if f.bastion == "" {
return nil, fmt.Errorf("bastion is not set, but useBastion is true")
}
addr = f.bastion
} else {
addr = host
}

if addr == "" {
return nil, fmt.Errorf("host is empty")
}
addr = net.JoinHostPort(addr, "22")
d := net.Dialer{
Timeout: 5 * time.Second,
}
conn, err := d.DialContext(ctx, "tcp", addr)
if err != nil {
return nil, err
return nil, fmt.Errorf("error dialing tcp %s: %w", addr, err)
}

// We have a TCP connection; we will force-close it to support context cancellation
Expand Down

0 comments on commit 3ea73f4

Please sign in to comment.