From 0d78f9bb34edabd39bfbf433066096564ef48706 Mon Sep 17 00:00:00 2001 From: Ilker Manap Date: Wed, 11 Nov 2026 21:31:22 +0100 Subject: [PATCH] A machine given by address is reached through its ssh config Host node add (and server add, mirror peer, patroni init) is usually given an address, while the ssh config names the machine: `Host gpu01` with `HostName 10.0.0.21` and the User, IdentityFile or ProxyJump it needs. That block did not match the address, so the address had to be added to its Host line by hand. Now the concrete Host names in the config and its Includes are read, ssh -G says each one's HostName, and the first that is the address (or resolves to it) is connected through. Under sudo, the caller's name is now a fallback: a User in the config's block for the machine comes first. Co-Authored-By: Claude Opus 5.5 --- internal/cli/node.go | 7 +- internal/onboard/onboard.go | 8 +- internal/onboard/ssh.go | 37 ++++- internal/onboard/sshconfig.go | 223 +++++++++++++++++++++++++++++ internal/onboard/sshconfig_test.go | 87 +++++++++++ 5 files changed, 354 insertions(+), 8 deletions(-) create mode 100644 internal/onboard/sshconfig.go create mode 100644 internal/onboard/sshconfig_test.go diff --git a/internal/cli/node.go b/internal/cli/node.go index 681107b..c29002e 100644 --- a/internal/cli/node.go +++ b/internal/cli/node.go @@ -114,7 +114,7 @@ engines are kept.`, } f := cmd.Flags() f.StringVar(&na.opts.Node, "node", "", "node id (default: the machine's hostname)") - f.StringVar(&na.opts.User, "ssh-user", "", "user to log in as (default: the one before @ in the address, else whoever ran sudo, else you)") + f.StringVar(&na.opts.User, "ssh-user", "", "user to log in as (default: the one before @ in the address, else the ssh config's for the machine, else whoever ran sudo, else you)") f.StringArrayVar(&na.servers, "server", nil, "control plane address the agent dials, https://host:7443; repeatable for failover (default: this machine)") f.StringArrayVar(&na.fabrics, "fabric", nil, "InfiniBand or RoCE fabric and its adapters, name:mlx5_0,mlx5_1; repeatable") f.StringVar(&na.opts.MgmtAddr, "mgmt-addr", "", "address engines are reached at (default: found by the agent)") @@ -228,8 +228,9 @@ func (na *nodeAdd) target(addr string) onboard.Options { if o.User == "" { // Under sudo, the person who ran it — their account is usually the // one that exists on the nodes too. root's own keys still do the - // logging in, since that is who is running ssh. - o.User = os.Getenv("SUDO_USER") + // logging in, since that is who is running ssh. Only a fallback: a + // User in the ssh config's block for the machine comes first. + o.FallbackUser = os.Getenv("SUDO_USER") } return o } diff --git a/internal/onboard/onboard.go b/internal/onboard/onboard.go index 9faa07e..a67af9a 100644 --- a/internal/onboard/onboard.go +++ b/internal/onboard/onboard.go @@ -79,6 +79,9 @@ type Options struct { // Addr is how to reach the machine over SSH, and User who to log in as. Addr, User string + // FallbackUser logs in when User is empty and the ssh config names no + // user for the machine. + FallbackUser string // Node is its id in the cluster. Empty means its hostname, which is what // it should be: the node id, the hostname and the certificate's name // being one string saves an afternoon each time they would differ. @@ -193,7 +196,7 @@ type Runner struct { // New makes a Runner that reaches the machine with the system's ssh. func New(o Options, emit func(Event)) *Runner { - return &Runner{Opts: o, Remote: &SSH{Addr: o.Addr, User: o.User}, Emit: emit} + return &Runner{Opts: o, Remote: &SSH{Addr: o.Addr, User: o.User, FallbackUser: o.FallbackUser}, Emit: emit} } func (r *Runner) emit(step, level, detail, fix string) { @@ -302,6 +305,9 @@ func (r *Runner) connect(ctx context.Context, res *Result) error { } func (r *Runner) target() string { + if s, ok := r.Remote.(*SSH); ok { + return s.Target(context.Background()) + } if r.Opts.User == "" { return r.Opts.Addr } diff --git a/internal/onboard/ssh.go b/internal/onboard/ssh.go index 6234981..b526cb1 100644 --- a/internal/onboard/ssh.go +++ b/internal/onboard/ssh.go @@ -5,6 +5,7 @@ import ( "context" "os/exec" "runtime" + "sync" ) // Remote runs a shell command on the machine being onboarded, with input. @@ -14,8 +15,39 @@ type Remote interface { // SSH is Remote through the system's ssh, so the operator's keys, agent and // ~/.ssh/config apply exactly as they do when they log in by hand. +// +// Addr is looked up in the ssh config first: a Host block whose HostName is +// Addr is connected through by its name, so its User, IdentityFile and +// ProxyJump apply to a machine given by address, without the address +// having to be added to its Host line. type SSH struct { Addr, User string + // FallbackUser logs in when User is empty and the ssh config names no + // user for the machine: under sudo, the person who ran it. + FallbackUser string + + once sync.Once + target string +} + +// Target is what ssh is given: [user@]host, host being the ssh config's +// name for Addr when it has one. +func (s *SSH) Target(ctx context.Context) string { + s.once.Do(func() { + host := s.Addr + if a := configAlias(ctx, s.Addr); a != "" { + host = a + } + u := s.User + if u == "" && configUser(ctx, host) == "" { + u = s.FallbackUser + } + s.target = host + if u != "" { + s.target = u + "@" + host + } + }) + return s.target } // sshOptions never prompt: a question nobody is there to answer would hang @@ -37,10 +69,7 @@ var sshOptions = []string{ // Run runs script with sh on the machine. Files travel on standard input, // not scp, which a hardened sshd may not offer. func (s *SSH) Run(ctx context.Context, script string, stdin []byte) ([]byte, error) { - target := s.Addr - if s.User != "" { - target = s.User + "@" + s.Addr - } + target := s.Target(ctx) // "--" before the target: an address beginning with "-" is not read as // an option. args := append(append([]string{}, sshOptions...), "--", target, "sh", "-c", shellQuote(script)) diff --git a/internal/onboard/sshconfig.go b/internal/onboard/sshconfig.go new file mode 100644 index 0000000..bac8091 --- /dev/null +++ b/internal/onboard/sshconfig.go @@ -0,0 +1,223 @@ +package onboard + +import ( + "bufio" + "bytes" + "context" + "net" + "os" + "os/exec" + "os/user" + "path/filepath" + "strings" + "time" +) + +// Where ssh reads its configuration, and arguments every `ssh -G` here is +// given; variables so a test can point them at a file of its own. +var ( + userSSHConfig = func() string { return filepath.Join(sshHome(), ".ssh", "config") } + systemSSHConfig = "/etc/ssh/ssh_config" + sshExtraArgs []string +) + +// sshHome is the home ssh itself reads ~/.ssh from: the account's, from the +// password database, not $HOME, which sudo may leave as the caller's. +func sshHome() string { + if u, err := user.Current(); err == nil && u.HomeDir != "" { + return u.HomeDir + } + h, _ := os.UserHomeDir() + return h +} + +// configAlias is the ssh config's name for addr: the first concrete Host +// whose HostName is addr, or resolves to the same address. A machine is +// usually written in the config under its name, `Host gpu01` with +// `HostName 10.0.0.21` and the User, IdentityFile or ProxyJump it needs, +// and given to node add by its address, which that block does not match. +// Empty when addr is itself named on a Host line, or no block points at it. +func configAlias(ctx context.Context, addr string) string { + aliases := map[string]bool{} + var order []string + seen := map[string]bool{} + collect := func(path, base string) { + for _, h := range hostNames(path, base, seen, 0) { + if !aliases[h] { + aliases[h] = true + order = append(order, h) + } + } + } + collect(userSSHConfig(), filepath.Join(sshHome(), ".ssh")) + if systemSSHConfig != "" { + collect(systemSSHConfig, filepath.Dir(systemSSHConfig)) + } + for h := range aliases { + if strings.EqualFold(h, addr) { + return "" + } + } + if len(order) > 256 { + order = order[:256] + } + // Names are looked up only when no HostName is the address as written, + // and within one budget: a config of many hosts must not hold up a + // connection by seconds each. + var pending []string + hostOf := map[string]string{} + for _, a := range order { + h := sshGet(ctx, a)["hostname"] + if strings.EqualFold(h, addr) { + return a + } + if h != "" { + hostOf[a] = h + pending = append(pending, a) + } + } + if len(pending) == 0 { + return "" + } + lctx, cancel := context.WithTimeout(ctx, 3*time.Second) + defer cancel() + want := lookup(lctx, addr) + if len(want) == 0 { + return "" + } + for _, a := range pending { + for ip := range lookup(lctx, hostOf[a]) { + if want[ip] { + return a + } + } + } + return "" +} + +// configUser is the user the ssh config logs in to host as, empty when it +// names none. ssh -G reports the local account when nothing set one, so +// that is taken as none: the config saying `User root` to a root caller is +// the one case read wrongly, and it gets the fallback user instead. +func configUser(ctx context.Context, host string) string { + u := sshGet(ctx, host)["user"] + if cur, err := user.Current(); err == nil && u == cur.Username { + return "" + } + return u +} + +// sshGet is ssh's own reading of its configuration for host, keys +// lowercased; empty when ssh cannot say. +func sshGet(ctx context.Context, host string) map[string]string { + args := append(append([]string{"-G"}, sshExtraArgs...), "--", host) + out, err := exec.CommandContext(ctx, "ssh", args...).Output() + if err != nil { + return nil + } + m := map[string]string{} + sc := bufio.NewScanner(bytes.NewReader(out)) + for sc.Scan() { + k, v, ok := strings.Cut(sc.Text(), " ") + if k = strings.ToLower(k); ok { + if _, dup := m[k]; !dup { + m[k] = v + } + } + } + return m +} + +func lookup(ctx context.Context, host string) map[string]bool { + if ip := net.ParseIP(host); ip != nil { + return map[string]bool{ip.String(): true} + } + addrs, err := net.DefaultResolver.LookupHost(ctx, host) + if err != nil { + return nil + } + m := map[string]bool{} + for _, a := range addrs { + if ip := net.ParseIP(a); ip != nil { + m[ip.String()] = true + } + } + return m +} + +// hostNames are the concrete names on the Host lines of an ssh config file +// and the files it includes — no patterns, no negations. base is where a +// relative Include is looked for, as ssh does: ~/.ssh for the user's file, +// /etc/ssh for the system's. +func hostNames(path, base string, seen map[string]bool, depth int) []string { + if depth > 16 || seen[path] { + return nil + } + seen[path] = true + data, err := os.ReadFile(path) + if err != nil { + return nil + } + var names []string + for _, line := range strings.Split(string(data), "\n") { + key, args := configLine(line) + switch key { + case "host": + for _, a := range args { + if a != "" && !strings.ContainsAny(a, "*?!") { + names = append(names, a) + } + } + case "include": + for _, a := range args { + if strings.HasPrefix(a, "~/") { + a = filepath.Join(sshHome(), a[2:]) + } else if !filepath.IsAbs(a) { + a = filepath.Join(base, a) + } + matches, _ := filepath.Glob(a) + for _, m := range matches { + names = append(names, hostNames(m, base, seen, depth+1)...) + } + } + } + } + return names +} + +// configLine splits one line of ssh config into its keyword, lowercased, +// and arguments: "Host a b", "Host=a b" and quoted arguments alike. +func configLine(line string) (string, []string) { + line = strings.TrimSpace(line) + if line == "" || strings.HasPrefix(line, "#") { + return "", nil + } + i := strings.IndexAny(line, " \t=") + if i < 0 { + return strings.ToLower(line), nil + } + key := strings.ToLower(line[:i]) + rest := strings.TrimLeft(line[i:], " \t") + rest = strings.TrimPrefix(rest, "=") + var args []string + for rest = strings.TrimSpace(rest); rest != ""; rest = strings.TrimSpace(rest) { + if rest[0] == '"' { + end := strings.IndexByte(rest[1:], '"') + if end < 0 { + args = append(args, rest[1:]) + break + } + args = append(args, rest[1:1+end]) + rest = rest[2+end:] + continue + } + j := strings.IndexAny(rest, " \t") + if j < 0 { + args = append(args, rest) + break + } + args = append(args, rest[:j]) + rest = rest[j:] + } + return key, args +} diff --git a/internal/onboard/sshconfig_test.go b/internal/onboard/sshconfig_test.go new file mode 100644 index 0000000..b518e7e --- /dev/null +++ b/internal/onboard/sshconfig_test.go @@ -0,0 +1,87 @@ +package onboard + +import ( + "context" + "os" + "os/exec" + "os/user" + "path/filepath" + "testing" +) + +// withSSHConfig points configAlias and ssh -G at one file, and nothing else. +func withSSHConfig(t *testing.T, config string, extra map[string]string) { + t.Helper() + if _, err := exec.LookPath("ssh"); err != nil { + t.Skip("no ssh") + } + dir := t.TempDir() + path := filepath.Join(dir, "config") + if err := os.WriteFile(path, []byte(config), 0o600); err != nil { + t.Fatal(err) + } + for name, body := range extra { + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o600); err != nil { + t.Fatal(err) + } + } + oldUser, oldSys, oldArgs := userSSHConfig, systemSSHConfig, sshExtraArgs + userSSHConfig = func() string { return path } + systemSSHConfig = "" + sshExtraArgs = []string{"-F", path} + t.Cleanup(func() { userSSHConfig, systemSSHConfig, sshExtraArgs = oldUser, oldSys, oldArgs }) +} + +// A machine given by address is reached through the Host block whose +// HostName it is, with that block's user — not the sudo caller's. +func TestSSHTargetUsesConfigAlias(t *testing.T) { + withSSHConfig(t, ` +Host *.example + User nobody-wild +Host gpu01 gpu01-alt + HostName 10.9.8.7 + User ops +Host gpu02 + HostName=10.9.8.8 +`, nil) + ctx := context.Background() + cases := []struct { + s *SSH + want string + }{ + // The config's User applies by ssh itself: no fallback is added. + {&SSH{Addr: "10.9.8.7", FallbackUser: "ilker"}, "gpu01"}, + {&SSH{Addr: "10.9.8.7", User: "root", FallbackUser: "ilker"}, "root@gpu01"}, + {&SSH{Addr: "10.9.8.8", FallbackUser: "ilker"}, "ilker@gpu02"}, + {&SSH{Addr: "gpu01", FallbackUser: "ilker"}, "gpu01"}, + {&SSH{Addr: "10.9.8.9", FallbackUser: "ilker"}, "ilker@10.9.8.9"}, + {&SSH{Addr: "10.9.8.9"}, "10.9.8.9"}, + } + for _, c := range cases { + if got := c.s.Target(ctx); got != c.want { + t.Errorf("%+v: target %q, want %q", c.s.Addr, got, c.want) + } + } +} + +// A block in an included file counts, as it does for ssh. +func TestSSHTargetFollowsInclude(t *testing.T) { + withSSHConfig(t, "Include nodes.conf\n", map[string]string{ + "nodes.conf": "Host \"gpu03\"\n HostName 10.9.8.10\n", + }) + // ssh resolves a relative Include against ~/.ssh, not -F's directory; + // the scan does the same, so point base at the test's directory. + names := hostNames(userSSHConfig(), filepath.Dir(userSSHConfig()), map[string]bool{}, 0) + if len(names) != 1 || names[0] != "gpu03" { + t.Fatalf("names %q", names) + } +} + +func TestConfigUserIgnoresLocalDefault(t *testing.T) { + withSSHConfig(t, "Host x\n HostName 10.9.8.11\n", nil) + if cur, err := user.Current(); err == nil { + if u := configUser(context.Background(), "x"); u != "" { + t.Errorf("config user %q for a block naming none (local %q)", u, cur.Username) + } + } +}