add webhook validations for hookImagesURLPath and skipLoadBalancerDep…

…loyment for Tinkerbell
aws · Feb 15, 2024 · dc5ac52 · dc5ac52
1 parent 73209f6
commit dc5ac52
Show file tree

Hide file tree

Showing 17 changed files with 336 additions and 18 deletions.
diff --git a/cmd/eksctl-anywhere/cmd/upgradecluster.go b/cmd/eksctl-anywhere/cmd/upgradecluster.go
@@ -216,6 +216,7 @@ func (uc *upgradeClusterOptions) upgradeCluster(cmd *cobra.Command, args []strin
 
 	} else {
 		upgradeWorkloadCluster := workload.NewUpgrade(
+			deps.UnAuthKubeClient,
 			deps.Provider,
 			deps.ClusterManager,
 			deps.GitOpsFlux,

diff --git a/pkg/api/v1alpha1/cloudstackdatacenterconfig_types.go b/pkg/api/v1alpha1/cloudstackdatacenterconfig_types.go
@@ -159,6 +159,14 @@ func (v *CloudStackDatacenterConfig) ClearPauseAnnotation() {
 	}
 }
 
+// AddManagedByCLIAnnotation is a no-op for CloudStackDatacenterConfig.
+func (v *CloudStackDatacenterConfig) AddManagedByCLIAnnotation() {
+}
+
+// ClearManagedByCLIAnnotation is a no-op for CloudStackDatacenterConfig.
+func (v *CloudStackDatacenterConfig) ClearManagedByCLIAnnotation() {
+}
+
 func (v *CloudStackDatacenterConfig) ConvertConfigToConfigGenerateStruct() *CloudStackDatacenterConfigGenerate {
 	namespace := defaultEksaNamespace
 	if v.Namespace != "" {

diff --git a/pkg/api/v1alpha1/dockerdatacenterconfig_types.go b/pkg/api/v1alpha1/dockerdatacenterconfig_types.go
@@ -49,6 +49,14 @@ func (d *DockerDatacenterConfig) ClearPauseAnnotation() {
 	}
 }
 
+// AddManagedByCLIAnnotation is a no-op for DockerDatacenterConfig.
+func (d *DockerDatacenterConfig) AddManagedByCLIAnnotation() {
+}
+
+// ClearManagedByCLIAnnotation is a no-op for DockerDatacenterConfig.
+func (d *DockerDatacenterConfig) ClearManagedByCLIAnnotation() {
+}
+
 func (d *DockerDatacenterConfig) ConvertConfigToConfigGenerateStruct() *DockerDatacenterConfigGenerate {
 	namespace := defaultEksaNamespace
 	if d.Namespace != "" {

diff --git a/pkg/api/v1alpha1/nutanixdatacenterconfig_types.go b/pkg/api/v1alpha1/nutanixdatacenterconfig_types.go
@@ -88,6 +88,14 @@ func (in *NutanixDatacenterConfig) ClearPauseAnnotation() {
 	}
 }
 
+// AddManagedByCLIAnnotation is a no-op for NutanixDatacenterConfig.
+func (in *NutanixDatacenterConfig) AddManagedByCLIAnnotation() {
+}
+
+// ClearManagedByCLIAnnotation is a no-op for NutanixDatacenterConfig.
+func (in *NutanixDatacenterConfig) ClearManagedByCLIAnnotation() {
+}
+
 func (in *NutanixDatacenterConfig) ConvertConfigToConfigGenerateStruct() *NutanixDatacenterConfigGenerate {
 	namespace := defaultEksaNamespace
 	if in.Namespace != "" {

diff --git a/pkg/api/v1alpha1/snowdatacenterconfig_types.go b/pkg/api/v1alpha1/snowdatacenterconfig_types.go
@@ -57,6 +57,14 @@ func (s *SnowDatacenterConfig) ClearPauseAnnotation() {
 	}
 }
 
+// AddManagedByCLIAnnotation is a no-op for SnowDatacenterConfig.
+func (s *SnowDatacenterConfig) AddManagedByCLIAnnotation() {
+}
+
+// ClearManagedByCLIAnnotation is a no-op for SnowDatacenterConfig.
+func (s *SnowDatacenterConfig) ClearManagedByCLIAnnotation() {
+}
+
 func (s *SnowDatacenterConfig) Validate() error {
 	if len(s.Spec.IdentityRef.Name) == 0 {
 		return fmt.Errorf("SnowDatacenterConfig IdentityRef name must not be empty")

diff --git a/pkg/api/v1alpha1/tinkerbelldatacenterconfig_types.go b/pkg/api/v1alpha1/tinkerbelldatacenterconfig_types.go
@@ -71,6 +71,21 @@ func (t *TinkerbellDatacenterConfig) ClearPauseAnnotation() {
 	}
 }
 
+// AddManagedByCLIAnnotation adds the managed-by-cli annotation to the datacenterconfig.
+func (t *TinkerbellDatacenterConfig) AddManagedByCLIAnnotation() {
+	if t.Annotations == nil {
+		t.Annotations = map[string]string{}
+	}
+	t.Annotations[ManagedByCLIAnnotation] = "true"
+}
+
+// ClearManagedByCLIAnnotation removes the managed-by-cli annotation from the datacenterconfig.
+func (t *TinkerbellDatacenterConfig) ClearManagedByCLIAnnotation() {
+	if t.Annotations != nil {
+		delete(t.Annotations, ManagedByCLIAnnotation)
+	}
+}
+
 // Validate validates the Tinkerbell datacenter config.
 func (t *TinkerbellDatacenterConfig) Validate() error {
 	return validateDatacenterConfig(t)

diff --git a/pkg/api/v1alpha1/tinkerbelldatacenterconfig_types_test.go b/pkg/api/v1alpha1/tinkerbelldatacenterconfig_types_test.go
@@ -66,6 +66,28 @@ func TestTinkerbellDatacenterConfigValidateSuccess(t *testing.T) {
 	g.Expect(tinkDC.Validate()).To(Succeed())
 }
 
+func TestTinkerbellDatacenterConfig_AddManagedByCLIAnnotation(t *testing.T) {
+	g := NewWithT(t)
+	tinkDC := createTinkerbellDatacenterConfig()
+	tinkDC.Annotations = map[string]string{}
+	tinkDC.AddManagedByCLIAnnotation()
+	val, ok := tinkDC.Annotations[v1alpha1.ManagedByCLIAnnotation]
+
+	g.Expect(ok).To(BeTrue())
+	g.Expect(val).To(ContainSubstring("true"))
+}
+
+func TestTinkerbellDatacenterConfig_ClearManagedByCLIAnnotation(t *testing.T) {
+	g := NewWithT(t)
+	tinkDC := createTinkerbellDatacenterConfig()
+	tinkDC.Annotations = map[string]string{
+		v1alpha1.ManagedByCLIAnnotation: "true",
+	}
+	tinkDC.ClearManagedByCLIAnnotation()
+	_, ok := tinkDC.Annotations[v1alpha1.ManagedByCLIAnnotation]
+	g.Expect(ok).To(BeFalse())
+}
+
 func newTinkerbellDatacenterConfig(opts ...func(*v1alpha1.TinkerbellDatacenterConfig)) *v1alpha1.TinkerbellDatacenterConfig {
 	c := createTinkerbellDatacenterConfig()
 	for _, o := range opts {

diff --git a/pkg/api/v1alpha1/tinkerbelldatacenterconfig_webhook.go b/pkg/api/v1alpha1/tinkerbelldatacenterconfig_webhook.go
@@ -18,6 +18,7 @@ import (
 	"fmt"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -111,5 +112,19 @@ func validateImmutableFieldsTinkerbellDatacenterConfig(new, old *TinkerbellDatac
 		)
 	}
 
+	if new.Spec.HookImagesURLPath != old.Spec.HookImagesURLPath && !metav1.HasAnnotation(new.ObjectMeta, ManagedByCLIAnnotation) {
+		allErrs = append(
+			allErrs,
+			field.Forbidden(specPath.Child("hookImagesURLPath"), "field is immutable"),
+		)
+	}
+
+	if new.Spec.SkipLoadBalancerDeployment != old.Spec.SkipLoadBalancerDeployment && !metav1.HasAnnotation(new.ObjectMeta, ManagedByCLIAnnotation) {
+		allErrs = append(
+			allErrs,
+			field.Forbidden(specPath.Child("skipLoadBalancerDeployment"), "field is immutable"),
+		)
+	}
+
 	return allErrs
 }
diff --git a/pkg/api/v1alpha1/tinkerbelldatacenterconfig_webhook_test.go b/pkg/api/v1alpha1/tinkerbelldatacenterconfig_webhook_test.go
@@ -7,6 +7,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/aws/eks-anywhere/pkg/api/v1alpha1"
+	"github.com/aws/eks-anywhere/pkg/controller/clientutil"
 )
 
 func TestTinkerbellDatacenterValidateCreate(t *testing.T) {
@@ -51,14 +52,80 @@ func TestTinkerbellDatacenterValidateUpdateFailBadReq(t *testing.T) {
 	g.Expect(c.ValidateUpdate(cOld)).To(MatchError(ContainSubstring("expected a TinkerbellDatacenterConfig but got a *v1alpha1.Cluster")))
 }
 
-func TestTinkerbellDatacenterValidateUpdateImmutableTinkIP(t *testing.T) {
-	tOld := tinkerbellDatacenterConfig()
-	tOld.Spec.TinkerbellIP = "1.1.1.1"
-	tNew := tOld.DeepCopy()
+func TestTinkerbellDatacenterValidateUpdateImmutable(t *testing.T) {
+	tests := []struct {
+		name    string
+		old     v1alpha1.TinkerbellDatacenterConfig
+		new     v1alpha1.TinkerbellDatacenterConfig
+		wantErr string
+	}{
+		{
+			name: "updated tinkerbellIP",
+			old: tinkerbellDatacenterConfig(func(d *v1alpha1.TinkerbellDatacenterConfig) {
+				d.Spec.TinkerbellIP = "1.1.1.1"
+			}),
+			new: tinkerbellDatacenterConfig(func(d *v1alpha1.TinkerbellDatacenterConfig) {
+				d.Spec.TinkerbellIP = "1.1.1.2"
+			}),
+			wantErr: "spec.tinkerbellIP: Forbidden: field is immutable",
+		},
+		{
+			name: "updated hookImagesURLPath",
+			old: tinkerbellDatacenterConfig(func(d *v1alpha1.TinkerbellDatacenterConfig) {
+				d.Spec.HookImagesURLPath = "https://oldpath"
+			}),
+			new: tinkerbellDatacenterConfig(func(d *v1alpha1.TinkerbellDatacenterConfig) {
+				d.Spec.HookImagesURLPath = "https://newpath"
+			}),
+			wantErr: "spec.hookImagesURLPath: Forbidden: field is immutable",
+		},
+		{
+			name: "updated hookImagesURLPath, managed by cli",
+			old: tinkerbellDatacenterConfig(func(d *v1alpha1.TinkerbellDatacenterConfig) {
+				d.Spec.HookImagesURLPath = "https://oldpath"
+			}),
+			new: tinkerbellDatacenterConfig(func(d *v1alpha1.TinkerbellDatacenterConfig) {
+				d.Spec.HookImagesURLPath = "https://newpath"
+				clientutil.AddAnnotation(d, v1alpha1.ManagedByCLIAnnotation, "true")
+			}),
+			wantErr: "",
+		},
+		{
+			name: "updated skipLoadBalancerDeployment",
+			old: tinkerbellDatacenterConfig(func(d *v1alpha1.TinkerbellDatacenterConfig) {
+				d.Spec.SkipLoadBalancerDeployment = false
+			}),
+			new: tinkerbellDatacenterConfig(func(d *v1alpha1.TinkerbellDatacenterConfig) {
+				d.Spec.SkipLoadBalancerDeployment = true
+			}),
+			wantErr: "spec.skipLoadBalancerDeployment: Forbidden: field is immutable",
+		},
+		{
+			name: "updated skipLoadBalancerDeployment, managed by cli",
+			old: tinkerbellDatacenterConfig(func(d *v1alpha1.TinkerbellDatacenterConfig) {
+				d.Spec.SkipLoadBalancerDeployment = false
+			}),
+			new: tinkerbellDatacenterConfig(func(d *v1alpha1.TinkerbellDatacenterConfig) {
+				d.Spec.SkipLoadBalancerDeployment = true
+				clientutil.AddAnnotation(d, v1alpha1.ManagedByCLIAnnotation, "true")
+			}),
+			wantErr: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			err := tt.new.ValidateUpdate(&tt.old)
+			if tt.wantErr == "" {
+				g.Expect(err).To(BeNil())
+			} else {
+				g.Expect(err).To(MatchError(ContainSubstring(tt.wantErr)))
+			}
+		})
+	}
 
-	tNew.Spec.TinkerbellIP = "1.1.1.2"
-	g := NewWithT(t)
-	g.Expect(tNew.ValidateUpdate(&tOld)).To(MatchError(ContainSubstring("spec.tinkerbellIP: Forbidden: field is immutable")))
 }
 
 func TestTinkerbellDatacenterValidateDelete(t *testing.T) {
@@ -68,8 +135,10 @@ func TestTinkerbellDatacenterValidateDelete(t *testing.T) {
 	g.Expect(tOld.ValidateDelete()).To(Succeed())
 }
 
-func tinkerbellDatacenterConfig() v1alpha1.TinkerbellDatacenterConfig {
-	return v1alpha1.TinkerbellDatacenterConfig{
+type tinkerbellDatacenterConfigOpt func(*v1alpha1.TinkerbellDatacenterConfig)
+
+func tinkerbellDatacenterConfig(opts ...tinkerbellDatacenterConfigOpt) v1alpha1.TinkerbellDatacenterConfig {
+	d := v1alpha1.TinkerbellDatacenterConfig{
 		TypeMeta: metav1.TypeMeta{},
 		ObjectMeta: metav1.ObjectMeta{
 			Annotations: make(map[string]string, 1),
@@ -80,4 +149,10 @@ func tinkerbellDatacenterConfig() v1alpha1.TinkerbellDatacenterConfig {
 		},
 		Status: v1alpha1.TinkerbellDatacenterConfigStatus{},
 	}
+
+	for _, opt := range opts {
+		opt(&d)
+	}
+
+	return d
 }
diff --git a/pkg/api/v1alpha1/vspheredatacenterconfig_types.go b/pkg/api/v1alpha1/vspheredatacenterconfig_types.go
@@ -74,6 +74,14 @@ func (v *VSphereDatacenterConfig) ClearPauseAnnotation() {
 	}
 }
 
+// AddManagedByCLIAnnotation is a no-op for VSphereDatacenterConfig.
+func (v *VSphereDatacenterConfig) AddManagedByCLIAnnotation() {
+}
+
+// ClearManagedByCLIAnnotation is a no-op for VSphereDatacenterConfig.
+func (v *VSphereDatacenterConfig) ClearManagedByCLIAnnotation() {
+}
+
 func (v *VSphereDatacenterConfig) SetDefaults() {
 	v.Spec.Network = generateFullVCenterPath(networkFolderType, v.Spec.Network, v.Spec.Datacenter)
 

diff --git a/pkg/controller/clientutil/annotations_test.go b/pkg/controller/clientutil/annotations_test.go
@@ -126,3 +126,46 @@ func TestAddLabel(t *testing.T) {
 		})
 	}
 }
+
+func TestRemoveAnnotation(t *testing.T) {
+	tests := []struct {
+		name string
+		obj  client.Object
+		key  string
+	}{
+		{
+			name: "empty annotations",
+			obj:  &corev1.ConfigMap{},
+			key:  "my-annotation",
+		},
+		{
+			name: "annotations key present",
+			obj: &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"my-annotation": "b",
+					},
+				},
+			},
+			key: "my-annotation",
+		},
+		{
+			name: "annotation key not present",
+			obj: &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"a": "b",
+					},
+				},
+			},
+			key: "my-annotation",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+			clientutil.RemoveAnnotation(tt.obj, tt.key)
+			g.Expect(tt.obj.GetAnnotations()).ToNot(HaveKey(tt.key))
+		})
+	}
+}
diff --git a/pkg/providers/provider.go b/pkg/providers/provider.go
@@ -51,6 +51,8 @@ type DatacenterConfig interface {
 	Kind() string
 	PauseReconcile()
 	ClearPauseAnnotation()
+	AddManagedByCLIAnnotation()
+	ClearManagedByCLIAnnotation()
 	Marshallable() v1alpha1.Marshallable
 }
 

diff --git a/pkg/workflows/management/upgrade_cluster.go b/pkg/workflows/management/upgrade_cluster.go
@@ -3,6 +3,8 @@ package management
 import (
 	"context"
 
+	"github.com/aws/eks-anywhere/pkg/clients/kubernetes"
+	"github.com/aws/eks-anywhere/pkg/constants"
 	"github.com/aws/eks-anywhere/pkg/logger"
 	"github.com/aws/eks-anywhere/pkg/task"
 	"github.com/aws/eks-anywhere/pkg/workflows"
@@ -13,11 +15,28 @@ type upgradeCluster struct{}
 // Run upgradeCluster performs actions needed to upgrade the management cluster.
 func (s *upgradeCluster) Run(ctx context.Context, commandContext *task.CommandContext) task.Task {
 	logger.Info("Upgrading management cluster")
+	commandContext.Provider.DatacenterConfig(commandContext.ClusterSpec).AddManagedByCLIAnnotation()
 	if err := commandContext.ClusterUpgrader.Run(ctx, commandContext.ClusterSpec, *commandContext.ManagementCluster); err != nil {
 		commandContext.SetError(err)
 		return &workflows.CollectMgmtClusterDiagnosticsTask{}
 	}
 
+	client, err := commandContext.ClientFactory.BuildClientFromKubeconfig(commandContext.ManagementCluster.KubeconfigFile)
+	if err != nil {
+		commandContext.SetError(err)
+		return &workflows.CollectMgmtClusterDiagnosticsTask{}
+	}
+
+	commandContext.Provider.DatacenterConfig(commandContext.ClusterSpec).ClearManagedByCLIAnnotation()
+	if err := client.ApplyServerSide(ctx,
+		constants.EKSACLIFieldManager,
+		commandContext.ClusterSpec.TinkerbellDatacenter,
+		kubernetes.ApplyServerSideOptions{ForceOwnership: true},
+	); err != nil {
+		commandContext.SetError(err)
+		return &workflows.CollectMgmtClusterDiagnosticsTask{}
+	}
+
 	return &reconcileGitOps{}
 }