Skip to content

Commit

Permalink
Check machine configs have an SSH key for upgrade (#6542)
Browse files Browse the repository at this point in the history
This was already an implicit need, due how the capi spec generation is
written, but the error produced wasn't clear and it also happen too late
in the upgrade workflow. Now we throw an error during preflight
validations.
  • Loading branch information
g-gaston authored Aug 23, 2023
1 parent be8a9d3 commit 094fb01
Show file tree
Hide file tree
Showing 8 changed files with 276 additions and 1 deletion.
5 changes: 5 additions & 0 deletions pkg/api/v1alpha1/cloudstackmachineconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,11 @@ func (c *CloudStackMachineConfig) OSFamily() OSFamily {
return ""
}

// Users returns a list of configuration for OS users.
func (c *CloudStackMachineConfig) Users() []UserConfiguration {
return c.Spec.Users
}

func (c *CloudStackMachineConfigSpec) Equal(o *CloudStackMachineConfigSpec) bool {
if c == o {
return true
Expand Down
5 changes: 5 additions & 0 deletions pkg/api/v1alpha1/nutanixmachineconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ func (in *NutanixMachineConfig) OSFamily() OSFamily {
return in.Spec.OSFamily
}

// Users returns a list of configuration for OS users.
func (in *NutanixMachineConfig) Users() []UserConfiguration {
return in.Spec.Users
}

// GetNamespace returns the namespace of the NutanixMachineConfig.
func (in *NutanixMachineConfig) GetNamespace() string {
return in.Namespace
Expand Down
5 changes: 5 additions & 0 deletions pkg/api/v1alpha1/tinkerbellmachineconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ func (c *TinkerbellMachineConfig) OSFamily() OSFamily {
return c.Spec.OSFamily
}

// Users returns a list of configuration for OS users.
func (c *TinkerbellMachineConfig) Users() []UserConfiguration {
return c.Spec.Users
}

func (c *TinkerbellMachineConfig) GetNamespace() string {
return c.Namespace
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/api/v1alpha1/vspheremachineconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ func (c *VSphereMachineConfig) OSFamily() OSFamily {
return c.Spec.OSFamily
}

// Users returns a list of configuration for OS users.
func (c *VSphereMachineConfig) Users() []UserConfiguration {
return c.Spec.Users
}

func (c *VSphereMachineConfig) GetNamespace() string {
return c.Namespace
}
Expand Down
72 changes: 72 additions & 0 deletions pkg/providers/ssh.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package providers

import (
"context"

"github.com/pkg/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"

"github.com/aws/eks-anywhere/pkg/api/v1alpha1"
"github.com/aws/eks-anywhere/pkg/cluster"
eksaerrors "github.com/aws/eks-anywhere/pkg/errors"
"github.com/aws/eks-anywhere/pkg/validation"
)

// ValidateSSHKeyPresentForUpgrade checks that all machine configs in the cluster spec
// contain at least one SSH key.
func ValidateSSHKeyPresentForUpgrade(_ context.Context, spec *cluster.Spec) error {
machines := make([]machineWithUsers, 0)

// We don't add snow since SnowMachineConfig's don't have []User

machines = appendMachinesWithUsers(machines, spec.VSphereMachineConfigs)
machines = appendMachinesWithUsers(machines, spec.CloudStackMachineConfigs)
machines = appendMachinesWithUsers(machines, spec.NutanixMachineConfigs)
machines = appendMachinesWithUsers(machines, spec.TinkerbellMachineConfigs)

if err := validateAtLeastOneSSHKey(machines); err != nil {
return validation.WithRemediation(err, "Please include at least one SSH key per machine config. If your keys were autogenerated during the create cluster operation, make sure you include them in your cluster config for all lifecycle operations")
}

return nil
}

type machineWithUsers interface {
metav1.Object
runtime.Object
Users() []v1alpha1.UserConfiguration
}

func validateAtLeastOneSSHKey(machines []machineWithUsers) error {
var errs []error

Machines:
for _, m := range machines {
for _, user := range m.Users() {
for _, key := range user.SshAuthorizedKeys {
if key != "" {
continue Machines
}
}
}

errs = append(errs,
errors.Errorf(
"%s %s is invalid: it should contain at least one SSH key",
m.GetName(),
m.GetObjectKind().GroupVersionKind().Kind,
),
)
}

return eksaerrors.NewAggregate(errs)
}

func appendMachinesWithUsers[O machineWithUsers](m []machineWithUsers, addMap map[string]O) []machineWithUsers {
for _, machine := range addMap {
m = append(m, machine)
}

return m
}
158 changes: 158 additions & 0 deletions pkg/providers/ssh_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
package providers_test

import (
"context"
"fmt"
"strings"
"testing"

. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

anywherev1 "github.com/aws/eks-anywhere/pkg/api/v1alpha1"
"github.com/aws/eks-anywhere/pkg/cluster"
"github.com/aws/eks-anywhere/pkg/providers"
)

func TestValidateSSHKeyPresentForUpgrade(t *testing.T) {
testCases := []struct {
name string
config *cluster.Config
want []string
}{
{
name: "all machines have keys",
config: &cluster.Config{
VSphereMachineConfigs: map[string]*anywherev1.VSphereMachineConfig{
"machine1": {
Spec: anywherev1.VSphereMachineConfigSpec{
Users: []anywherev1.UserConfiguration{
{
Name: "user1",
},
{
Name: "user2",
SshAuthorizedKeys: []string{
"mykey",
},
},
},
},
},
"machine2": {
Spec: anywherev1.VSphereMachineConfigSpec{
Users: []anywherev1.UserConfiguration{
{
Name: "user1",
SshAuthorizedKeys: []string{""},
},
{
Name: "user2",
SshAuthorizedKeys: []string{
"mykey",
},
},
},
},
},
},
},
},
{
name: "all machines are missing keys",
config: &cluster.Config{
VSphereMachineConfigs: map[string]*anywherev1.VSphereMachineConfig{
"machine1": {
TypeMeta: metav1.TypeMeta{
Kind: "VSphereMachineConfig",
},
ObjectMeta: metav1.ObjectMeta{
Name: "machine1",
},
Spec: anywherev1.VSphereMachineConfigSpec{
Users: []anywherev1.UserConfiguration{
{
Name: "user1",
},
{
Name: "user2",
SshAuthorizedKeys: []string{""},
},
},
},
},
"machine2": {
TypeMeta: metav1.TypeMeta{
Kind: "VSphereMachineConfig",
},
ObjectMeta: metav1.ObjectMeta{
Name: "machine2",
},
Spec: anywherev1.VSphereMachineConfigSpec{
Users: []anywherev1.UserConfiguration{
{
Name: "user1",
SshAuthorizedKeys: []string{""},
},
},
},
},
},
CloudStackMachineConfigs: map[string]*anywherev1.CloudStackMachineConfig{
"machine1": {
TypeMeta: metav1.TypeMeta{
Kind: "CloudStackMachineConfig",
},
ObjectMeta: metav1.ObjectMeta{
Name: "machine1",
},
},
},
NutanixMachineConfigs: map[string]*anywherev1.NutanixMachineConfig{
"machine1": {
TypeMeta: metav1.TypeMeta{
Kind: "NutanixMachineConfig",
},
ObjectMeta: metav1.ObjectMeta{
Name: "machine1",
},
},
},
TinkerbellMachineConfigs: map[string]*anywherev1.TinkerbellMachineConfig{
"machine1": {
TypeMeta: metav1.TypeMeta{
Kind: "TinkerbellMachineConfig",
},
ObjectMeta: metav1.ObjectMeta{
Name: "machine1",
},
},
},
},
want: []string{
"machine1 VSphereMachineConfig is invalid: it should contain at least one SSH key",
"machine2 VSphereMachineConfig is invalid: it should contain at least one SSH key",
"machine1 CloudStackMachineConfig is invalid: it should contain at least one SSH key",
"machine1 NutanixMachineConfig is invalid: it should contain at least one SSH key",
"machine1 TinkerbellMachineConfig is invalid: it should contain at least one SSH key",
},
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
ctx := context.Background()
g := NewWithT(t)
spec := &cluster.Spec{
Config: tc.config,
}

if len(tc.want) == 0 {
g.Expect(providers.ValidateSSHKeyPresentForUpgrade(ctx, spec)).To(Succeed())
} else {
g.Expect(providers.ValidateSSHKeyPresentForUpgrade(ctx, spec)).To(
MatchError(fmt.Sprintf("[%s]", strings.Join(tc.want, ", "))),
)
}
})
}
}
25 changes: 25 additions & 0 deletions pkg/validations/upgradevalidations/preflightvalidations.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import (

anywherev1 "github.com/aws/eks-anywhere/pkg/api/v1alpha1"
"github.com/aws/eks-anywhere/pkg/config"
"github.com/aws/eks-anywhere/pkg/providers"
"github.com/aws/eks-anywhere/pkg/types"
"github.com/aws/eks-anywhere/pkg/validation"
"github.com/aws/eks-anywhere/pkg/validations"
)

Expand All @@ -21,6 +23,12 @@ func (u *UpgradeValidations) PreflightValidations(ctx context.Context) []validat
KubeconfigFile: u.Opts.ManagementCluster.KubeconfigFile,
}
upgradeValidations := []validations.Validation{
func() *validations.ValidationResult {
return resultForRemediableValidation(
"SSH Keys present",
providers.ValidateSSHKeyPresentForUpgrade(ctx, u.Opts.Spec),
)
},
func() *validations.ValidationResult {
return &validations.ValidationResult{
Name: "validate OS is compatible with registry mirror configuration",
Expand Down Expand Up @@ -144,3 +152,20 @@ func (u *UpgradeValidations) PreflightValidations(ctx context.Context) []validat
}
return upgradeValidations
}

func resultForRemediableValidation(name string, err error) *validations.ValidationResult {
r := &validations.ValidationResult{
Name: name,
Err: err,
}

if r.Err == nil {
return r
}

if validation.IsRemediable(r.Err) {
r.Remediation = validation.Remediation(r.Err)
}

return r
}
2 changes: 1 addition & 1 deletion pkg/validations/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type ValidationResult struct {

func (v *ValidationResult) Report() {
if v.Err != nil {
logger.MarkFail("Validation failed", "validation", v.Name, "error", v.Err, "remediation", v.Remediation)
logger.MarkFail("Validation failed", "validation", v.Name, "error", v.Err.Error(), "remediation", v.Remediation)
return
}
if !v.Silent {
Expand Down

0 comments on commit 094fb01

Please sign in to comment.