Update SSH keys via MCD #115

kikisdeliveryservice · 2018-10-05T23:02:52Z

Allow MCD to update SSH Keys.

We are only allowing one user named "core" and only updating the field SSHAuthorizedKeys for this user.

kikisdeliveryservice · 2018-10-05T23:06:07Z

/assign @ashcrow

ashcrow

The logic looks sound! Some nits and recommendations but nothing that is blocking. Good work!

ashcrow · 2018-10-08T14:33:04Z

pkg/daemon/update.go

+					break
+				}
+			}
+			// length of Users is different, we don't do those kinds of changes, so nope.


I'm stuck between an attempted addition of a new user should noop (as it's doing here) or if we should raise the irreconcilable flag as the user was attempting to do something we are not going to do. @sdemos thoughts?

pkg/daemon/update.go

ashcrow · 2018-10-08T14:38:17Z

pkg/daemon/update.go

+			// ssh keys are in PasswdUser.HomeDir/.ssh
+			path = filepath.Join(oldUsers[idx].HomeDir, ".ssh")
+			glog.V(2).Infof("Writing SSHKeys at %q:", path)
+			// write the SSHKeys


ashcrow · 2018-10-08T14:39:15Z

pkg/daemon/update.go

+			glog.V(2).Infof("Writing SSHKeys at %q:", path)
+			// write the SSHKeys
+		} else {
+			return fmt.Errorf("Failed to write SSHKeys")


Once the ssh key writing is added I recommend returning the error that is bubbled up from that work OR adding a log statement with details before this return

kikisdeliveryservice · 2018-10-08T16:14:37Z

Thanks @ashcrow for your thoughts! I'll incorporate them into the PR. 😺

smarterclayton · 2018-10-09T04:17:04Z

/test e2e-aws

ericavonb · 2018-10-10T22:40:47Z

/cc @openshift/sig-security

ashcrow · 2019-01-10T20:46:59Z

/retest

kikisdeliveryservice · 2019-01-10T20:47:26Z

Passed manual testing, only changes made since last review:

removed some of my logging that was added for testing
added additional comments for clarity
squashed commits

pkg/daemon/update.go

cgwalters · 2019-01-10T21:00:46Z

/lgtm

ashcrow

LGTM. I ask one more person do a review before we merge (for two sets of eyes).

ashcrow · 2019-01-10T20:48:22Z

pkg/daemon/update.go

-						msg := "Ignition passwd user section contains unsupported changes"
+			} else if len(oldIgn.Passwd.Users) > 0 && len(newIgn.Passwd.Users) >= 1 {
+				for _, user := range newIgn.Passwd.Users {
+					if user.Name != "core" {


Short explanation: the way Ignition works the newUsers array is actually the compilation of all the Users from all of the machineconfigs in oc get machineconfigs.

I spoke with @kikisdeliveryservice and this is a well known issue with the current ignition. She is going to write up some doc text around this as well.

ashcrow · 2019-01-10T20:58:02Z

pkg/daemon/update.go

-						msg := "Ignition passwd user section contains unsupported changes"
+			} else if len(oldIgn.Passwd.Users) > 0 && len(newIgn.Passwd.Users) >= 1 {
+				for _, user := range newIgn.Passwd.Users {
+					if user.Name != "core" {


/cc @dustymabe, @ajeddeloh, and @darkmuggle just for visibility

ashcrow · 2019-01-10T21:00:04Z

pkg/daemon/update.go

@@ -233,6 +244,29 @@ func (dn *Daemon) reconcilable(oldConfig, newConfig *mcfgv1.MachineConfig) *stri
 	return nil
 }

+// verifyUserFields returns nil if the user Name = "core", if 1 or more SSHKeys exist for


ashcrow · 2019-01-10T21:04:12Z

/retest

ashcrow · 2019-01-10T21:05:34Z

Looks like @cgwalters reviewed at the same time.

/lgtm

openshift-ci-robot · 2019-01-10T21:05:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ashcrow, cgwalters

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/daemon/OWNERS~~ [ashcrow,cgwalters]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ashcrow · 2019-01-10T21:06:31Z

Infra failure flake

could not wait for build: the build machine-config-controller failed with reason PullBuilderImageFailed: Failed pulling builder image

ashcrow · 2019-01-10T21:14:50Z

/test images

ashcrow · 2019-01-10T21:26:21Z

/test images

ashcrow · 2019-01-10T21:31:57Z

/retest

sjenning · 2019-01-10T21:46:03Z

We are hitting the same image build failures in #249

Something is up with the registry in api.ci

ashcrow · 2019-01-10T21:53:06Z

/retest

ashcrow · 2019-01-10T22:04:50Z

/retest

kikisdeliveryservice · 2019-01-10T22:05:14Z

/retest

kikisdeliveryservice · 2019-01-11T00:02:10Z

/retest

kikisdeliveryservice · 2019-01-11T01:47:08Z

/retest

kikisdeliveryservice · 2019-01-11T02:42:20Z

/retest

kikisdeliveryservice · 2019-01-11T04:11:33Z

/retest

kikisdeliveryservice · 2019-01-11T05:19:04Z

/test e2e-aws

kikisdeliveryservice · 2019-01-11T06:32:38Z

My tests finally all passed and now they are rerunning. ☹️

ashcrow · 2019-01-11T14:19:09Z

CI MERGED! 🎉

Cvo tweaks

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 5, 2018

openshift-ci-robot requested review from jlebon and sdemos October 5, 2018 23:03

kikisdeliveryservice force-pushed the mco-ssh-keys branch from e4d3b04 to 325ad23 Compare October 5, 2018 23:11

openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 5, 2018

kikisdeliveryservice force-pushed the mco-ssh-keys branch 2 times, most recently from b4e1633 to fc55bb4 Compare October 6, 2018 07:05

ashcrow reviewed Oct 8, 2018

View reviewed changes

kikisdeliveryservice force-pushed the mco-ssh-keys branch from fc55bb4 to 80b6983 Compare October 8, 2018 17:10

openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 8, 2018

kikisdeliveryservice force-pushed the mco-ssh-keys branch from 80b6983 to 24acfda Compare October 8, 2018 17:12

kikisdeliveryservice force-pushed the mco-ssh-keys branch 3 times, most recently from 0f2b84a to 2d52841 Compare October 10, 2018 23:31

openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 10, 2018

kikisdeliveryservice force-pushed the mco-ssh-keys branch 7 times, most recently from a55c95d to bfbc6d0 Compare October 15, 2018 22:27

cgwalters reviewed Jan 10, 2019

View reviewed changes

pkg/daemon/update.go Show resolved Hide resolved

openshift-ci-robot assigned cgwalters Jan 10, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 10, 2019

ashcrow approved these changes Jan 10, 2019

View reviewed changes

openshift-ci-robot assigned ashcrow Jan 10, 2019

kikisdeliveryservice mentioned this pull request Jan 10, 2019

docs: how to update sshkeys for user "core" via mcd #286

Merged

openshift-merge-robot merged commit 0cd475c into openshift:master Jan 11, 2019

cgwalters mentioned this pull request Jan 14, 2019

support removing SSH keys entirely #296

Closed

osherdp pushed a commit to osherdp/machine-config-operator that referenced this pull request Apr 13, 2021

Merge pull request openshift#115 from gabemontero/cvo-tweaks

b8c013d

Cvo tweaks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update SSH keys via MCD #115

Update SSH keys via MCD #115

kikisdeliveryservice commented Oct 5, 2018 •

edited

Loading

kikisdeliveryservice commented Oct 5, 2018

ashcrow left a comment

ashcrow Oct 8, 2018

ashcrow Oct 8, 2018

ashcrow Oct 8, 2018

kikisdeliveryservice commented Oct 8, 2018

smarterclayton commented Oct 9, 2018

ericavonb commented Oct 10, 2018

ashcrow commented Jan 10, 2019

kikisdeliveryservice commented Jan 10, 2019

cgwalters commented Jan 10, 2019

ashcrow left a comment

ashcrow Jan 10, 2019

ashcrow Jan 10, 2019

ashcrow Jan 10, 2019

ashcrow commented Jan 10, 2019

ashcrow commented Jan 10, 2019

openshift-ci-robot commented Jan 10, 2019

ashcrow commented Jan 10, 2019 •

edited

Loading

ashcrow commented Jan 10, 2019

ashcrow commented Jan 10, 2019

ashcrow commented Jan 10, 2019

sjenning commented Jan 10, 2019 •

edited

Loading

ashcrow commented Jan 10, 2019

ashcrow commented Jan 10, 2019

kikisdeliveryservice commented Jan 10, 2019

kikisdeliveryservice commented Jan 11, 2019

kikisdeliveryservice commented Jan 11, 2019

kikisdeliveryservice commented Jan 11, 2019

kikisdeliveryservice commented Jan 11, 2019

kikisdeliveryservice commented Jan 11, 2019

kikisdeliveryservice commented Jan 11, 2019

ashcrow commented Jan 11, 2019

Update SSH keys via MCD #115

Update SSH keys via MCD #115

Conversation

kikisdeliveryservice commented Oct 5, 2018 • edited Loading

kikisdeliveryservice commented Oct 5, 2018

ashcrow left a comment

Choose a reason for hiding this comment

ashcrow Oct 8, 2018

Choose a reason for hiding this comment

ashcrow Oct 8, 2018

Choose a reason for hiding this comment

ashcrow Oct 8, 2018

Choose a reason for hiding this comment

kikisdeliveryservice commented Oct 8, 2018

smarterclayton commented Oct 9, 2018

ericavonb commented Oct 10, 2018

ashcrow commented Jan 10, 2019

kikisdeliveryservice commented Jan 10, 2019

cgwalters commented Jan 10, 2019

ashcrow left a comment

Choose a reason for hiding this comment

ashcrow Jan 10, 2019

Choose a reason for hiding this comment

ashcrow Jan 10, 2019

Choose a reason for hiding this comment

ashcrow Jan 10, 2019

Choose a reason for hiding this comment

ashcrow commented Jan 10, 2019

ashcrow commented Jan 10, 2019

openshift-ci-robot commented Jan 10, 2019

ashcrow commented Jan 10, 2019 • edited Loading

ashcrow commented Jan 10, 2019

ashcrow commented Jan 10, 2019

ashcrow commented Jan 10, 2019

sjenning commented Jan 10, 2019 • edited Loading

ashcrow commented Jan 10, 2019

ashcrow commented Jan 10, 2019

kikisdeliveryservice commented Jan 10, 2019

kikisdeliveryservice commented Jan 11, 2019

kikisdeliveryservice commented Jan 11, 2019

kikisdeliveryservice commented Jan 11, 2019

kikisdeliveryservice commented Jan 11, 2019

kikisdeliveryservice commented Jan 11, 2019

kikisdeliveryservice commented Jan 11, 2019

ashcrow commented Jan 11, 2019

kikisdeliveryservice commented Oct 5, 2018 •

edited

Loading

ashcrow commented Jan 10, 2019 •

edited

Loading

sjenning commented Jan 10, 2019 •

edited

Loading