Disable password authentication when using keys

[frantisekz]

Fixes #2687

Pull Request Checklist

• implement the feature
• write the documentation
• extend the test coverage
• update the specification
• adjust plugin docstring
• modify the json schema
• mention the version
• include a release note

[frantisekz]

My hypothesis for this is here: #2687 (comment)

[psss]

/packit test

[psss]

Thanks for digging into the issue! The change looks safe. Just a minor proposal to make things a bit more clear for the future.

[lukaszachy]

So if I use 'password' option (which we have in the spec - https://tmt.readthedocs.io/en/stable/plugins/provision.html#id9) the problem will appear again?

Isn't there a way for testcloud to have the info that cloud-init finished its job? (and will allow tmt to try to connect only after that?)

[frantisekz]

So if I use 'password' option (which we have in the spec - https://tmt.readthedocs.io/en/stable/plugins/provision.html#id9) the problem will appear again?

Yeah, I am thinking here if tmt shouldn't internally always use a ssh key for the connections to the testcloud guests and take the user:password from the spec as something that users can use to connect to guests manually outside of tmt.

From testcloud's perspective, if the image contains a working cloud-init (which are currently the only ones the testcloud supports, I have long rotting code for serial console fun for images without cloud-init, but that's another story), that should be all that's needed, and if some tests require to be ran under a different user, su user after establishing a root connection via the ssh key is the way.

That said, I may be missing use-cases and/or reasons from the wider tmt perspective why user:password has to work too apart from just the ssh keys even for the master connection.

Isn't there a way for testcloud to have the info that cloud-init finished its job? (and will allow tmt to try to connect only after that?)

I've been thinking about this too while working on the issue. There may be ways to do this (but if we get to this route, I'd still prefer to have this PR merged as it's a painkiller for the default setup):

• Change the default ssh port from the guest perspective in the cloud-init's runcmd, and setup mapping for the different port, so unchanged ssh port is invisible for the tmt < this seems like the perfect candidate
• Let cloud-init asynchronously spawn something on some arbitrarily set port, which the tmt would wait for to be open before attempting to do anything
• Send a http request from the guest from the cloud-init to the host (I don't like this one, it seems like we'll get a different set of bugs due to user networking from guest to host fun)
• Leverage virtiofs - tmt part isn't ready yet, user sessions aren't supported by libvirt at all for this
• Leverage the direct kernel boot, append sshd mask to the /proc/cmdline, which results in ssh being stopped until cloud-init spawns it (described in the issue, has its own problems too)
• There may be more options, but I didn't come by any so far

Probably apart from the first in the list, these would still leave a (albeit fractionally smaller, but still) surface of race, as both would be the last step of the cloud-init's runcmd, while the ssh restart happens in a non-atomic way after this phase.

[psss]

Hm, so it seems that the /tests/prepare/multihost test failed despite the change.

[psss]

And now it's green ;-) The full log should be there next time.

[psss]

So we ran ten instances of the test and all are green :-P

[psss]

/packit test -i provision

[psss]

@frantisekz, so... it seems, that even if the fix is removed, everything is green 😁

[frantisekz]

@frantisekz, so... it seems, that even if the fix is removed, everything is green 😁

😂 hehe, and that's how it is with races... I'd still say let's merge this change, it'll decrease the surface for the race for the default (key) auth method.

And to kill it completely, I'll come with the proposed "Change the default ssh port from the guest perspective in the cloud-init's runcmd, and setup mapping for the different port, so unchanged ssh port is invisible for the tmt " mid-term (it'll require some api design work testcloud side if it were to be implemented properly).

[psss]

Thanks for looking into this!

[psss]

I'd still say let's merge this change, it'll decrease the surface for the race for the default (key) auth method.

Yeah, makes sense. And we've also found why everything was green (wrong provision method variable).

[psss]

Here's a failure from a recent job.