Merge pull request #1677 from benmoss/cluster-validation

✨ Add validations to AWSCluster
kubernetes-sigs · Apr 6, 2020 · e2c994b · e2c994b
2 parents ca38f8a + ff52703
commit e2c994b
Show file tree

Hide file tree

Showing 3 changed files with 179 additions and 0 deletions.
diff --git a/api/v1alpha3/awscluster_webhook.go b/api/v1alpha3/awscluster_webhook.go
@@ -17,8 +17,16 @@ limitations under the License.
 package v1alpha3
 
 import (
+	"fmt"
+	"reflect"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
 )
 
 // log is for logging in this package.
@@ -29,3 +37,46 @@ func (r *AWSCluster) SetupWebhookWithManager(mgr ctrl.Manager) error {
 		For(r).
 		Complete()
 }
+
+// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha3-awscluster,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=awsclusters,versions=v1alpha3,name=validation.awscluster.infrastructure.cluster.x-k8s.io
+
+var _ webhook.Validator = &AWSCluster{}
+
+func (r *AWSCluster) ValidateCreate() error {
+	return nil
+}
+
+func (r *AWSCluster) ValidateDelete() error {
+	return nil
+}
+
+func (r *AWSCluster) ValidateUpdate(old runtime.Object) error {
+	var allErrs field.ErrorList
+
+	oldC, ok := old.(*AWSCluster)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected an AWSCluster but got a %T", old))
+	}
+
+	if r.Spec.Region != oldC.Spec.Region {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "region"), r.Spec.Region, "field is immutable"),
+		)
+	}
+
+	if !reflect.DeepEqual(r.Spec.ControlPlaneLoadBalancer, oldC.Spec.ControlPlaneLoadBalancer) {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer"),
+				r.Spec.ControlPlaneLoadBalancer, "field is immutable"),
+		)
+	}
+
+	if !reflect.DeepEqual(oldC.Spec.ControlPlaneEndpoint, clusterv1.APIEndpoint{}) &&
+		!reflect.DeepEqual(r.Spec.ControlPlaneEndpoint, oldC.Spec.ControlPlaneEndpoint) {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "controlPlaneEndpoint"), r.Spec.ControlPlaneEndpoint, "field is immutable"),
+		)
+	}
+
+	return aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs)
+}
diff --git a/api/v1alpha3/awscluster_webhook_test.go b/api/v1alpha3/awscluster_webhook_test.go
@@ -0,0 +1,109 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1alpha3
+
+import (
+	"testing"
+
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
+)
+
+func TestAWSCluster_ValidateUpdate(t *testing.T) {
+	tests := []struct {
+		name       string
+		oldCluster *AWSCluster
+		newCluster *AWSCluster
+		wantErr    bool
+	}{
+		{
+			name: "region is immutable",
+			oldCluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					Region: "us-east-1",
+				},
+			},
+			newCluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					Region: "us-east-2",
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "controlPlaneLoadBalancer is immutable",
+			oldCluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
+						Scheme: &ClassicELBSchemeInternal,
+					},
+				},
+			},
+			newCluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
+						Scheme: &ClassicELBSchemeInternetFacing,
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "controlPlaneEndpoint is immutable",
+			oldCluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneEndpoint: clusterv1.APIEndpoint{
+						Host: "example.com",
+						Port: int32(8000),
+					},
+				},
+			},
+			newCluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneEndpoint: clusterv1.APIEndpoint{
+						Host: "foo.example.com",
+						Port: int32(9000),
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "controlPlaneEndpoint can be updated if it is empty",
+			oldCluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneEndpoint: clusterv1.APIEndpoint{},
+				},
+			},
+			newCluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneEndpoint: clusterv1.APIEndpoint{
+						Host: "example.com",
+						Port: int32(8000),
+					},
+				},
+			},
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := tt.newCluster.ValidateUpdate(tt.oldCluster); (err != nil) != tt.wantErr {
+				t.Errorf("ValidateUpdate() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml
@@ -6,6 +6,25 @@ metadata:
   creationTimestamp: null
   name: validating-webhook-configuration
 webhooks:
+- clientConfig:
+    caBundle: Cg==
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-infrastructure-cluster-x-k8s-io-v1alpha3-awscluster
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: validation.awscluster.infrastructure.cluster.x-k8s.io
+  rules:
+  - apiGroups:
+    - infrastructure.cluster.x-k8s.io
+    apiVersions:
+    - v1alpha3
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - awsclusters
 - clientConfig:
     caBundle: Cg==
     service: