pressly / sup

Super simple deployment tool - think of it like 'make' for a network of servers
https://pressly.github.io/sup
MIT License
2.48k stars 176 forks source link

connect failed if no "hostname" in sshconfig #142

Open kadefor opened 6 years ago

kadefor commented 6 years ago
cat ~/.ssh/config
Host 192.168.16.10
    User root
    Port 22

connecting to clients failed: connecting to remote host failed: Connect("root@:22"): ssh: handshake failed: ssh: unable to authenticate, attempted methods [none publickey], no supported methods remain

VojtechVitek commented 6 years ago

Anyone able to fix this?

mfridman commented 6 years ago

@kadefor You're correct that ssh falls back on a valid Host if HostName is omitted when reading from an .ssh/config file. IMO this logic should exist in mikkeloscar/sshconfig. But this is an easy enough fix I'll submit a PR.

kadefor commented 6 years ago

I'm thinking about it too :-)

@mfridman Currently, sup can't set ssh config for per host, this case:

Supfile:

networks:
    prod:
        - 192.168.16.10
        - 192.168.16.11

and ~/ssh/config:

Host 192.168.16.10
    User tom

Host 192.168.16.11
    User roy

now, we get two username "tom" and "roy"

cmd/sup/main.go:

342         // check network.Hosts for match
343         for _, host := range network.Hosts {
344             conf, found := confMap[host]
345             if found {
346                 network.User = conf.User
347                 network.IdentityFile = resolvePath(conf.IdentityFile)
348                 network.Hosts = []string{fmt.Sprintf("%s:%d", conf.HostName, conf.Port)}
349             }
350         }

network.User = ?

can we set tom@<host>:<port> in network.Hosts?

@VojtechVitek this line network.Hosts = []string{fmt.Sprintf("%s:%d", conf.HostName, conf.Port)} network.Hosts only one host? is it a bug? and here network.Hosts set a new value in for network.Hosts, is it ok in golang?

mfridman commented 6 years ago

@kadefor Yep, looks like it's a bug, the network.Hosts are being overwritten with []string{....

So for now it'd make sense to do the following:

Once Sup supports per host configs, can open it up.

for i, host := range network.Hosts {
    if conf, ok := confMap[host]; ok {
        // attempt fallback on Host definition when HostName is omitted
        if conf.HostName == "" {
            conf.HostName = host
        }
        // no point setting the below until Sup supports per host configuration
        // network.User = conf.User
        // network.IdentityFile = resolvePath(conf.IdentityFile)
        network.Hosts[i] = fmt.Sprintf("%s:%d", conf.HostName, conf.Port)
    }
}
kadefor commented 6 years ago

I also think we can add "TODO" here. When Sup supports per host config, then refactor :-)

mfridman commented 6 years ago

@kadefor Looking a bit more at the code you could do the following to fix your use case of username/hostname/port like so..

Because the parseHost() method takes into account whether a username was passed in or not.

for i, host := range network.Hosts {
    if conf, ok := confMap[host]; ok {
        // attempt fallback on Host definition when HostName is omitted
        if conf.HostName == "" {
            conf.HostName = host
        }
        if conf.User == "" {
            conf.User = network.User
        }
        // no point setting the below until Sup supports per host configuration
        // network.User = conf.User
        // network.IdentityFile = resolvePath(conf.IdentityFile)
        network.Hosts[i] = fmt.Sprintf("%s@%s:%d", conf.User, conf.HostName, conf.Port)
    }
}

This will return [tom@192.168.16.10:22 roy@192.168.16.11:22]

kadefor commented 6 years ago

@mfridman I'll try to send a PR to support per host config, if I can handle it :-D

mfridman commented 6 years ago

No problem, I'll close my PR.

Ideally your PR will address the immediate issue with setting the correct user, hostname and port when reading from a .ssh/config file.

untoreh commented 5 years ago

It would be nice if SUP_HOST reflected Host and maybe SUP_HOSTNAME the actual endpoint from the ssh config HostName, but this would break compatibility with existing scripts I think