Juju 1.23-beta4 introduces ssh key bug when used w/ DHX

[Charles Butler]

There's an issue with the latest Juju 1.23-beta4 and it appears to be the SSH Key import

I'm coupling this with the juju dhx plugin, which runs successfully on first run, however on subsequent connection attempts to the same machine it spits out duplicate key warnings then panic's and stacktraces

- edit -

Interestingly enough, I seem to have confirmed its the ssh import process causing the panic. Commenting out the import id's in my dhx rc has corrected the issue.

I've also verified this behavior does not exist in beta3, nor in current stable 1.22

[Charles Butler (lazypower)]

Update for perrito666:

This has been confirmed on AWS and HPCloud substrates (but i imagine it affects more)

Steps to reproduce:

Clone the juju-plugins repository according to the readme: https://github.com/juju/plugins/blob/master/README.md

Once you have confirmed the DHX plugin is present, create a debug-hooks-rc.yaml in your $JUJU_HOME directory, similar to the following:

import_ids: ['mbruzek','whitmo', 'lazypower']

Bootstrap and deploy services

juju bootstrap
juju deploy trusty/mariadb

Attempt to connect to a host once a service is registered:

juju dhx mariadb/0

The unit will attempt to register SSH Keys and panic - aborting the connection.

[Martin Packman (gz)]

So, this is basically an API breakage caused by making import sanely when a key id corresponds to multiple keys.

cmd/juju/authorizedkeys_import.go

        results, err := client.ImportKeys(c.user, c.sshKeyIds...)
        if err != nil {
                return block.ProcessBlockedError(err, block.BlockChange)
        }
        for i, result := range results {
                if result.Error != nil {
                        fmt.Fprintf(context.Stderr, "cannot import key id %q: %v\n", c.sshKeyIds[i], result.Error)
                }
        }

This assumes that the returned results map 1:1 with the passed in sshKeyIds.

apiserver/keymanager/keymanager.go

func runSSHKeyImport(keyIds []string) []importedSSHKey {
        ...
        for _, keyId := range keyIds {
                output, err := RunSSHImportId(keyId)
                ...
                lines := strings.Split(output, "\n")
                hasKey := false
                for _, line := range lines {
                        ...
                        keyInfo = append(keyInfo, importedSSHKey{
                                key: line,
                                fingerprint: fingerprint,
                                err: errors.Annotatef(err, "invalid ssh key for %s", keyId),
                        })
                }
                ...
        }
        return keyInfo
}

This returns a longer keyInfo than given keyIds when ssh-import-id returns multiple lines of output.

The correct way to fix this depends on the branch. On trunk, we want a new api that returns a richer result struct.

On 1.23, we need to fix the current api by making the number of string-errors we return map to the key ids passed in.

For both, I think the runSSHKeyImport return should be changed to return []struct{keyId string, keyInfo []importedSSHKey}, and for the current api version we then mash all the errors into one.

[Horacio Durán (hduran-8)]

I proposed http://reviews.vapour.ws/r/1580/ for 1.23 and will propose a more complete solution for master.