Skip to content

Commit

Permalink
Merge pull request #115 from kikisdeliveryservice/mco-ssh-keys
Browse files Browse the repository at this point in the history 
Update SSH keys via MCD
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot authored Jan 11, 2019
2 parents 2abccd8 + caea699 commit 0cd475c
Show file tree
Hide file tree
Showing 2 changed files with 167 additions and 40 deletions.
60 changes: 47 additions & 13 deletions pkg/daemon/update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,27 +175,38 @@ func (dn *Daemon) reconcilable(oldConfig, newConfig *mcfgv1.MachineConfig) *stri

// Passwd section

// we don't currently configure groups or users in place. we can't fix it if
// something changed here.
// we don't currently configure Groups in place. we don't configure Users except
// for setting/updating SSHAuthorizedKeys for the only allowed user "core".
// otherwise we can't fix it if something changed here.
if !reflect.DeepEqual(oldIgn.Passwd, newIgn.Passwd) {
if !reflect.DeepEqual(oldIgn.Passwd.Groups, newIgn.Passwd.Groups) {
msg := "Ignition Passwd Groups section contains changes"
return &msg
}
// check if the prior config is empty and that this is the first time running.
// if so, the SSHKey from the cluster config and user "core" must be added to machine config,.
if !reflect.DeepEqual(oldIgn.Passwd.Users, newIgn.Passwd.Users) {
// check if the prior config is empty and that this is the first time running.
// if so, the SSHKey from the cluster config and user "core" must be added to machine config.
if len(oldIgn.Passwd.Users) == 0 && len(newIgn.Passwd.Users) == 1 {
if newIgn.Passwd.Users[0].Name == "core" && len(newIgn.Passwd.Users[0].SSHAuthorizedKeys) > 0 {
glog.Info("SSH Keys reconcilable")
} else {
msg := "Ignition passwd user section contains unsupported changes"
glog.Infof("user data sent to verify before ssh init: %v", newIgn.Passwd.Users[0])
msg := verifyUserFields(newIgn.Passwd.Users[0])
if msg != "" {
return &msg
}
} else if len(oldIgn.Passwd.Users) > 0 && len(newIgn.Passwd.Users) >= 1 {
// there is an update to Users, we must verify that it is ONLY making an acceptable
// change to the SSHAuthorizedKeys for the user "core"
for _, user := range newIgn.Passwd.Users {
if user.Name != "core" {
msg := "Ignition passwd user section contains unsupported changes: non-core user"
return &msg
}
}
glog.Infof("user data to be verified before ssh update: %v", newIgn.Passwd.Users[len(newIgn.Passwd.Users)-1])
msg := verifyUserFields(newIgn.Passwd.Users[len(newIgn.Passwd.Users)-1])
if msg != "" {
return &msg
}
}
} else {
msg := "Ignition passwd section contains unsupported changes"
return &msg
}
}

Expand Down Expand Up @@ -235,6 +246,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
// this user and if all other fields in User are empty.
// Otherwise, an error msg will be returned and the proposed config will not be reconcilable.
// At this time we do not support non-"core" users or any changes to the "core" user
// outside of SSHAuthorizedKeys.
func verifyUserFields(pwdUser ignv2_2types.PasswdUser) string {
var msg string
emptyUser := ignv2_2types.PasswdUser{}
tempUser := pwdUser
if tempUser.Name == "core" && len(tempUser.SSHAuthorizedKeys) >= 1 {
tempUser.Name = ""
tempUser.SSHAuthorizedKeys = nil
if !reflect.DeepEqual(emptyUser, tempUser) {
msg = "Ignition passwd user section contains unsupported changes: non-sshKey changes"
} else {
glog.Info("SSH Keys reconcilable")
}
} else {
msg = "Ignition passwd user section contains unsupported changes: user must be core and have 1 or more sshKeys"
}
return msg
}

// updateFiles writes files specified by the nodeconfig to disk. it also writes
// systemd units. there is no support for multiple filesystems at this point.
//
Expand Down Expand Up @@ -563,15 +597,15 @@ func (dn *Daemon) updateSSHKeys(newUsers []ignv2_2types.PasswdUser) error {
}
sshDirPath := filepath.Join("/home", newUsers[0].Name, ".ssh")
// we are only dealing with the "core" User at this time, so only dealing with the first entry in Users[]
glog.Infof("Writing SSHKeys at %q:", sshDirPath)
glog.Infof("Writing SSHKeys at %q", sshDirPath)
if err := dn.fileSystemClient.MkdirAll(filepath.Dir(sshDirPath), os.FileMode(0600)); err != nil {
return fmt.Errorf("Failed to create directory %q: %v", filepath.Dir(sshDirPath), err)
}
glog.V(2).Infof("Created directory: %s", sshDirPath)

authkeypath := filepath.Join(sshDirPath, "authorized_keys")
var concatSSHKeys string
for _, k := range newUsers[0].SSHAuthorizedKeys {
for _, k := range newUsers[len(newUsers)-1].SSHAuthorizedKeys {
concatSSHKeys = concatSSHKeys + string(k) + "\n"
}

Expand Down
147 changes: 120 additions & 27 deletions pkg/daemon/update_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,22 +65,6 @@ func TestUpdateOS(t *testing.T) {
// TestReconcilable attempts to verify the conditions in which configs would and would not be
// reconcilable. Welcome to the longest unittest you've ever read.
func TestReconcilable(t *testing.T) {
// checkReconcilableResults is a shortcut for verifying results that should be reconcilable
checkReconcilableResults := func(key string, reconcilableError *string) {

if reconcilableError != nil {
t.Errorf("Expected the same %s values would be reconcilable. Received error: %v", key, *reconcilableError)
}
}

// checkIreconcilableResults is a shortcut for verifing results that should be ireconcilable
checkIreconcilableResults := func(key string, reconcilableError *string) {

if reconcilableError == nil {
t.Errorf("Expected different %s values would not be reconcilable.", key)
}
}

d := Daemon{
name: "nodeName",
OperatingSystem: MachineConfigDaemonOSRHCOS,
Expand Down Expand Up @@ -116,12 +100,12 @@ func TestReconcilable(t *testing.T) {

// Verify Ignition version changes react as expected
isReconcilable := d.reconcilable(oldConfig, newConfig)
checkIreconcilableResults("ignition", isReconcilable)
checkIrreconcilableResults(t, "ignition", isReconcilable)

// Match ignition versions
oldConfig.Spec.Config.Ignition.Version = "1.0"
isReconcilable = d.reconcilable(oldConfig, newConfig)
checkReconcilableResults("ignition", isReconcilable)
checkReconcilableResults(t, "ignition", isReconcilable)

// Verify Networkd unit changes react as expected
oldConfig.Spec.Config.Networkd = ignv2_2types.Networkd{}
Expand All @@ -133,13 +117,13 @@ func TestReconcilable(t *testing.T) {
},
}
isReconcilable = d.reconcilable(oldConfig, newConfig)
checkIreconcilableResults("networkd", isReconcilable)
checkIrreconcilableResults(t, "networkd", isReconcilable)

// Match Networkd
oldConfig.Spec.Config.Networkd = newConfig.Spec.Config.Networkd

isReconcilable = d.reconcilable(oldConfig, newConfig)
checkReconcilableResults("networkd", isReconcilable)
checkReconcilableResults(t, "networkd", isReconcilable)

// Verify Disk changes react as expected
oldConfig.Spec.Config.Storage.Disks = []ignv2_2types.Disk{
Expand All @@ -149,12 +133,12 @@ func TestReconcilable(t *testing.T) {
}

isReconcilable = d.reconcilable(oldConfig, newConfig)
checkIreconcilableResults("disk", isReconcilable)
checkIrreconcilableResults(t, "disk", isReconcilable)

// Match storage disks
newConfig.Spec.Config.Storage.Disks = oldConfig.Spec.Config.Storage.Disks
isReconcilable = d.reconcilable(oldConfig, newConfig)
checkReconcilableResults("disk", isReconcilable)
checkReconcilableResults(t, "disk", isReconcilable)

// Verify Filesystems changes react as expected
oldConfig.Spec.Config.Storage.Filesystems = []ignv2_2types.Filesystem{
Expand All @@ -164,12 +148,12 @@ func TestReconcilable(t *testing.T) {
}

isReconcilable = d.reconcilable(oldConfig, newConfig)
checkIreconcilableResults("filesystem", isReconcilable)
checkIrreconcilableResults(t, "filesystem", isReconcilable)

// Match Storage filesystems
newConfig.Spec.Config.Storage.Filesystems = oldConfig.Spec.Config.Storage.Filesystems
isReconcilable = d.reconcilable(oldConfig, newConfig)
checkReconcilableResults("filesystem", isReconcilable)
checkReconcilableResults(t, "filesystem", isReconcilable)

// Verify Raid changes react as expected
oldConfig.Spec.Config.Storage.Raid = []ignv2_2types.Raid{
Expand All @@ -179,12 +163,105 @@ func TestReconcilable(t *testing.T) {
}

isReconcilable = d.reconcilable(oldConfig, newConfig)
checkIreconcilableResults("raid", isReconcilable)
checkIrreconcilableResults(t, "raid", isReconcilable)

// Match storage raid
newConfig.Spec.Config.Storage.Raid = oldConfig.Spec.Config.Storage.Raid
isReconcilable = d.reconcilable(oldConfig, newConfig)
checkReconcilableResults("raid", isReconcilable)
checkReconcilableResults(t, "raid", isReconcilable)

// Verify Passwd Groups changes unsupported
oldConfig = &mcfgv1.MachineConfig{}
tempGroup := ignv2_2types.PasswdGroup{Name: "testGroup"}
newMcfg := &mcfgv1.MachineConfig{
Spec: mcfgv1.MachineConfigSpec{
Config: ignv2_2types.Config{
Passwd: ignv2_2types.Passwd{
Groups: []ignv2_2types.PasswdGroup{tempGroup},
},
},
},
}
isReconcilable = d.reconcilable(oldConfig, newMcfg)
checkIrreconcilableResults(t, "passwdGroups", isReconcilable)

}

func TestReconcilableSSH(t *testing.T) {
// expectedError is the error we will use when expecting an error to return
expectedError := fmt.Errorf("broken")

// testClient is the NodeUpdaterClient mock instance that will front
// calls to update the host.
testClient := RpmOstreeClientMock{
GetBootedOSImageURLReturns: []GetBootedOSImageURLReturn{},
RunPivotReturns: []error{
// First run will return no error
nil,
// Second rrun will return our expected error
expectedError},
}

// Create a Daemon instance with mocked clients
d := Daemon{
name: "nodeName",
OperatingSystem: MachineConfigDaemonOSRHCOS,
NodeUpdaterClient: testClient,
loginClient: nil, // set to nil as it will not be used within tests
client: fake.NewSimpleClientset(),
kubeClient: k8sfake.NewSimpleClientset(),
rootMount: "/",
bootedOSImageURL: "test",
}

// Check that updating SSH Key of user core supported
//tempUser1 := ignv2_2types.PasswdUser{Name: "core", SSHAuthorizedKeys: []ignv2_2types.SSHAuthorizedKey{"1234"}}
oldMcfg := &mcfgv1.MachineConfig{}
tempUser1 := ignv2_2types.PasswdUser{Name: "core", SSHAuthorizedKeys: []ignv2_2types.SSHAuthorizedKey{"5678", "abc"}}
newMcfg := &mcfgv1.MachineConfig{
Spec: mcfgv1.MachineConfigSpec{
Config: ignv2_2types.Config{
Passwd: ignv2_2types.Passwd{
Users: []ignv2_2types.PasswdUser{tempUser1},
},
},
},
}

errMsg := d.reconcilable(oldMcfg, newMcfg)
checkReconcilableResults(t, "ssh", errMsg)

// Check that updating User with User that is not core is not supported
tempUser2 := ignv2_2types.PasswdUser{Name: "core", SSHAuthorizedKeys: []ignv2_2types.SSHAuthorizedKey{"1234"}}
oldMcfg.Spec.Config.Passwd.Users = append(oldMcfg.Spec.Config.Passwd.Users, tempUser2)
tempUser3 := ignv2_2types.PasswdUser{Name: "another user", SSHAuthorizedKeys: []ignv2_2types.SSHAuthorizedKey{"5678"}}
newMcfg.Spec.Config.Passwd.Users[0] = tempUser3

errMsg = d.reconcilable(oldMcfg, newMcfg)
checkIrreconcilableResults(t, "ssh", errMsg)

// check that we cannot make updates if any other Passwd.User field is changed.
tempUser4 := ignv2_2types.PasswdUser{Name: "core", SSHAuthorizedKeys: []ignv2_2types.SSHAuthorizedKey{"5678"}, HomeDir: "somedir"}
newMcfg.Spec.Config.Passwd.Users[0] = tempUser4

errMsg = d.reconcilable(oldMcfg, newMcfg)
checkIrreconcilableResults(t, "ssh", errMsg)

// check that we cannot add a user or have len(Passwd.Users)> 1
tempUser5 := ignv2_2types.PasswdUser{Name: "some user", SSHAuthorizedKeys: []ignv2_2types.SSHAuthorizedKey{"5678"}}
newMcfg.Spec.Config.Passwd.Users = append(newMcfg.Spec.Config.Passwd.Users, tempUser5)

errMsg = d.reconcilable(oldMcfg, newMcfg)
checkIrreconcilableResults(t, "ssh", errMsg)

// check that user is not attempting to remove the only sshkey from core user
tempUser6 := ignv2_2types.PasswdUser{Name: "core", SSHAuthorizedKeys: []ignv2_2types.SSHAuthorizedKey{}}
newMcfg.Spec.Config.Passwd.Users[0] = tempUser6
newMcfg.Spec.Config.Passwd.Users = newMcfg.Spec.Config.Passwd.Users[:len(newMcfg.Spec.Config.Passwd.Users)-1]

errMsg = d.reconcilable(oldMcfg, newMcfg)
checkIrreconcilableResults(t, "ssh", errMsg)

}

func TestUpdateSSHKeys(t *testing.T) {
Expand Down Expand Up @@ -214,7 +291,7 @@ func TestUpdateSSHKeys(t *testing.T) {
fileSystemClient: mockFS,
}
// Set up machineconfigs that are identical except for SSH keys
tempUser := ignv2_2types.PasswdUser{Name: "core", SSHAuthorizedKeys: []ignv2_2types.SSHAuthorizedKey{"1234"}}
tempUser := ignv2_2types.PasswdUser{Name: "core", SSHAuthorizedKeys: []ignv2_2types.SSHAuthorizedKey{"1234", "4567"}}

newMcfg := &mcfgv1.MachineConfig{
Spec: mcfgv1.MachineConfigSpec{
Expand All @@ -237,3 +314,19 @@ func TestUpdateSSHKeys(t *testing.T) {
t.Errorf("Expected error, user is not core")
}
}

// checkReconcilableResults is a shortcut for verifying results that should be reconcilable
func checkReconcilableResults(t *testing.T, key string, reconcilableError *string) {

if reconcilableError != nil {
t.Errorf("Expected the same %s values would be reconcilable. Received error: %v", key, *reconcilableError)
}
}

// checkIrreconcilableResults is a shortcut for verifing results that should be irreconcilable
func checkIrreconcilableResults(t *testing.T, key string, reconcilableError *string) {

if reconcilableError == nil {
t.Errorf("Expected different %s values would not be reconcilable.", key)
}
}

0 comments on commit 0cd475c

Please sign in to comment.