openshift/machine-config-operator

Question: Is this ssh key check necessary: https://github.com/openshift/machine-config-operator/blob/d4dc34299e409010592d6c5d3d9bbbcdd572e2ac/pkg/daemon/update.go#L851

relyt0925 opened this issue · 10 comments

I was just curious why the sshkey check specifically is there:

if tempUser.Name == coreUserName && len(tempUser.SSHAuthorizedKeys) >= 1 {

I believe it is supported in openshift to boot nodes without an ssh key. Just curious if someone has some background on that specific line

Note this was noticed in the machine-config-daemon firstboot-complete-machineconfig

Going to a payload that contains
{"ignition":{"version":"3.1.0"},"passwd":{"users":[{"name":"core"}]}..... (with no ssh key)

Since the initial payload initialized here has users nil:

oldConfig := canonicalizeEmptyMC(nil)

and the new payload has a user with no ssh key this code path is triggered:

if !reflect.DeepEqual(oldIgn.Passwd.Users, newIgn.Passwd.Users) {

if err := verifyUserFields(newIgn.Passwd.Users[len(newIgn.Passwd.Users)-1]); err != nil {

And then it fails out. I believe if the len(tempUser.SSHAuthorizedKeys) >= 1 check is removed it would work

This could be bug in the code but so far we didn't hit this issue in MCO. Can you please provide exact steps to reproduce the issue?

I can!

So this is in regards to the newer hypershift repo/offering:
https://github.com/openshift/hypershift

It is on the machine-config-daemon firstboot-complete-machineconfig command.

Steps to reproduce:

  1. Place any ignition config that contains a section defining the core user without any ssh keys at
    /etc/ignition-machine-config-encapsulated.json

  2. run machine-config-daemon firstboot-complete-machineconfig and it will error on the ssh check. However: I believe it should pass since no ssh keys is a valid configuration.

This can be done on any RHCOS box (doesn't need to be a part of an active openshift cluster: can be done by just booting the RHCOS box up with a basic ignition config.

This can be done on any RHCOS box (doesn't need to be a part of an active openshift cluster: can be done by just booting the RHCOS box up with a basic ignition config.

I don't think that's true, because the MCD and the machine-config-daemon-firstboot.service etc. are part of the Ignition rendered by the MCO. So presumably you are at least manually including that in the Ignition?

For all default OCP installs, the installer generates a MC that has sshkeys in it. I think this was likely originally wrote with that in mind. I am not sure if not providing a sshkey affects functionality at all. If not, maybe we can relax that check a bit.

You can "reproduce" this by deleting the installer provided ssh key on any OCP cluster I believe.

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.