Add sow machine config immutale field validation (#2713)

pkg/providers/snow/snow.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"k8s.io/apimachinery/pkg/api/equality"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 
 	"github.com/aws/eks-anywhere/pkg/api/v1alpha1"
@@ -47,6 +48,7 @@ type SnowProvider struct {
 
 type KubeUnAuthClient interface {
 	KubeconfigClient(kubeconfig string) kubernetes.Client
+	Get(ctx context.Context, name, namespace, kubeconfig string, obj runtime.Object) error
 	Delete(ctx context.Context, name, namespace, kubeconfig string, obj runtime.Object) error
 }
 
@@ -221,7 +223,25 @@ func (p *SnowProvider) MachineConfigs(clusterSpec *cluster.Spec) []providers.Mac
 	return configs
 }
 
+// ValidateNewSpec validates the immutability of snow machine config by comparing it with the existing one in cluster.
 func (p *SnowProvider) ValidateNewSpec(ctx context.Context, cluster *types.Cluster, clusterSpec *cluster.Spec) error {
+	for _, mc := range clusterSpec.SnowMachineConfigs {
+		oldMc := &v1alpha1.SnowMachineConfig{}
+		err := p.kubeUnAuthClient.Get(ctx, mc.GetName(), mc.GetNamespace(), cluster.KubeconfigFile, oldMc)
+
+		// if machine config object does not exist in cluster, it means user defines new machine config. Skip comparison.
+		if apierrors.IsNotFound(err) {
+			continue
+		}
+		if err != nil {
+			return err
+		}
+
+		if oldMc.Spec.SshKeyName != mc.Spec.SshKeyName {
+			return fmt.Errorf("spec.sshKeyName is immutable. Previous value %s, new value %s", oldMc.Spec.SshKeyName, mc.Spec.SshKeyName)
+		}
+
+	}
 	return nil
 }
 

pkg/providers/snow/snow_test.go

@@ -2,6 +2,7 @@ package snow_test
 
 import (
 	"context"
+	"errors"
 	"os"
 	"testing"
 
@@ -337,6 +338,94 @@ func TestSetupAndValidateDeleteClusterNoCertsEnv(t *testing.T) {
 	tt.Expect(err).To(MatchError(ContainSubstring("'EKSA_AWS_CA_BUNDLES_FILE' is not set or is empty")))
 }
 
+func TestValidateNewSpecSuccess(t *testing.T) {
+	tt := newSnowTest(t)
+	tt.kubeUnAuthClient.EXPECT().
+		Get(
+			tt.ctx,
+			tt.clusterSpec.SnowMachineConfigs["test-cp"].Name,
+			tt.clusterSpec.SnowMachineConfigs["test-cp"].Namespace,
+			tt.cluster.KubeconfigFile,
+			&v1alpha1.SnowMachineConfig{},
+		).
+		DoAndReturn(func(_ context.Context, _, _, _ string, obj *v1alpha1.SnowMachineConfig) error {
+			tt.clusterSpec.SnowMachineConfigs["test-cp"].DeepCopyInto(obj)
+			return nil
+		})
+	tt.kubeUnAuthClient.EXPECT().
+		Get(
+			tt.ctx,
+			tt.clusterSpec.SnowMachineConfigs["test-wn"].Name,
+			tt.clusterSpec.SnowMachineConfigs["test-wn"].Namespace,
+			tt.cluster.KubeconfigFile,
+			&v1alpha1.SnowMachineConfig{},
+		).
+		DoAndReturn(func(_ context.Context, _, _, _ string, obj *v1alpha1.SnowMachineConfig) error {
+			tt.clusterSpec.SnowMachineConfigs["test-wn"].DeepCopyInto(obj)
+			return nil
+		})
+	err := tt.provider.ValidateNewSpec(tt.ctx, tt.cluster, tt.clusterSpec)
+	tt.Expect(err).To(Succeed())
+}
+
+func TestValidateNewSpecOldMachineConfigNotFound(t *testing.T) {
+	tt := newSnowTest(t)
+	tt.kubeUnAuthClient.EXPECT().
+		Get(
+			tt.ctx,
+			tt.clusterSpec.SnowMachineConfigs["test-cp"].Name,
+			tt.clusterSpec.SnowMachineConfigs["test-cp"].Namespace,
+			tt.cluster.KubeconfigFile,
+			&v1alpha1.SnowMachineConfig{},
+		).
+		Return(apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: ""}, ""))
+	tt.kubeUnAuthClient.EXPECT().
+		Get(
+			tt.ctx,
+			tt.clusterSpec.SnowMachineConfigs["test-wn"].Name,
+			tt.clusterSpec.SnowMachineConfigs["test-wn"].Namespace,
+			tt.cluster.KubeconfigFile,
+			&v1alpha1.SnowMachineConfig{},
+		).
+		Return(apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: ""}, ""))
+	err := tt.provider.ValidateNewSpec(tt.ctx, tt.cluster, tt.clusterSpec)
+	tt.Expect(err).To(Succeed())
+}
+
+func TestValidateNewSpecFetchOldMachineConfigError(t *testing.T) {
+	tt := newSnowTest(t)
+	tt.kubeUnAuthClient.EXPECT().
+		Get(
+			tt.ctx,
+			tt.clusterSpec.SnowMachineConfigs["test-cp"].Name,
+			tt.clusterSpec.SnowMachineConfigs["test-cp"].Namespace,
+			tt.cluster.KubeconfigFile,
+			&v1alpha1.SnowMachineConfig{},
+		).
+		Return(errors.New("get mc error"))
+	err := tt.provider.ValidateNewSpec(tt.ctx, tt.cluster, tt.clusterSpec)
+	tt.Expect(err).NotTo(Succeed())
+}
+
+func TestValidateNewSpecSshKeyNameChanged(t *testing.T) {
+	tt := newSnowTest(t)
+	tt.kubeUnAuthClient.EXPECT().
+		Get(
+			tt.ctx,
+			tt.clusterSpec.SnowMachineConfigs["test-cp"].Name,
+			tt.clusterSpec.SnowMachineConfigs["test-cp"].Namespace,
+			tt.cluster.KubeconfigFile,
+			&v1alpha1.SnowMachineConfig{},
+		).
+		DoAndReturn(func(_ context.Context, _, _, _ string, obj *v1alpha1.SnowMachineConfig) error {
+			tt.clusterSpec.SnowMachineConfigs["test-cp"].DeepCopyInto(obj)
+			obj.Spec.SshKeyName = "key-2"
+			return nil
+		})
+	err := tt.provider.ValidateNewSpec(tt.ctx, tt.cluster, tt.clusterSpec)
+	tt.Expect(err).To(MatchError(ContainSubstring("spec.sshKeyName is immutable")))
+}
+
 // TODO: add more tests (multi worker node groups, unstacked etcd, etc.)
 func TestGenerateCAPISpecForCreate(t *testing.T) {
 	tt := newSnowTest(t)