From 72b87109e96acb4ad5c85a8eb427b48bb620b09f Mon Sep 17 00:00:00 2001 From: Carlos Salas Date: Mon, 21 Oct 2024 10:05:02 +0200 Subject: [PATCH 1/3] test: add managed cp webhook test cases Signed-off-by: Carlos Salas --- .../gcpmanagedcontrolplane_webhook_test.go | 252 ++++++++++++++++++ 1 file changed, 252 insertions(+) create mode 100644 exp/api/v1beta1/gcpmanagedcontrolplane_webhook_test.go diff --git a/exp/api/v1beta1/gcpmanagedcontrolplane_webhook_test.go b/exp/api/v1beta1/gcpmanagedcontrolplane_webhook_test.go new file mode 100644 index 000000000..1d5ea1208 --- /dev/null +++ b/exp/api/v1beta1/gcpmanagedcontrolplane_webhook_test.go @@ -0,0 +1,252 @@ +/* +Copyright 2024 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 v1beta1 + +import ( + "strings" + "testing" + + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var ( + vV1_27_1 = "v1.27.1" + releaseChannel = Rapid +) + +func TestGCPManagedControlPlaneDefaultingWebhook(t *testing.T) { + tests := []struct { + name string + resourceName string + resourceNS string + spec GCPManagedControlPlaneSpec + expectSpec GCPManagedControlPlaneSpec + expetError bool + expectHash bool + }{ + { + name: "valid cluster name", + resourceName: "cluster1", + resourceNS: "default", + spec: GCPManagedControlPlaneSpec{ + ClusterName: "default_cluster1", + }, + expectSpec: GCPManagedControlPlaneSpec{ClusterName: "default_cluster1"}, + }, + { + name: "no cluster name should generate a valid one", + resourceName: "cluster1", + resourceNS: "default", + spec: GCPManagedControlPlaneSpec{ + ClusterName: "", + }, + expectSpec: GCPManagedControlPlaneSpec{ClusterName: "default-cluster1"}, + }, + { + name: "invalid cluster name (too long)", + resourceName: strings.Repeat("A", maxClusterNameLength+1), + resourceNS: "default", + spec: GCPManagedControlPlaneSpec{ + ClusterName: "", + }, + expectSpec: GCPManagedControlPlaneSpec{ClusterName: "capg-"}, + expectHash: true, + }, + { + name: "with kubernetes version", + resourceName: "cluster1", + resourceNS: "default", + spec: GCPManagedControlPlaneSpec{ + ClusterName: "cluster1_27_1", + ControlPlaneVersion: &vV1_27_1, + }, + expectSpec: GCPManagedControlPlaneSpec{ClusterName: "cluster1_27_1", ControlPlaneVersion: &vV1_27_1}, + }, + { + name: "with autopilot enabled", + resourceName: "cluster1", + resourceNS: "default", + spec: GCPManagedControlPlaneSpec{ + ClusterName: "cluster1_autopilot", + ControlPlaneVersion: &vV1_27_1, + EnableAutopilot: true, + }, + expectSpec: GCPManagedControlPlaneSpec{ClusterName: "cluster1_autopilot", ControlPlaneVersion: &vV1_27_1, EnableAutopilot: true}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + mcp := &GCPManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: tc.resourceName, + Namespace: tc.resourceNS, + }, + Spec: tc.spec, + } + mcp.Default() + + g.Expect(mcp.Spec).ToNot(BeNil()) + g.Expect(mcp.Spec.ClusterName).ToNot(BeEmpty()) + + if tc.expectHash { + g.Expect(strings.HasPrefix(mcp.Spec.ClusterName, "capg-")).To(BeTrue()) + // We don't care about the exact name + tc.expectSpec.ClusterName = mcp.Spec.ClusterName + } + g.Expect(mcp.Spec).To(Equal(tc.expectSpec)) + }) + } +} + +func TestGCPManagedControlPlaneValidatingWebhookCreate(t *testing.T) { + tests := []struct { + name string + expectError bool + spec GCPManagedControlPlaneSpec + }{ + { + name: "cluster name too long should cause an error", + expectError: true, + spec: GCPManagedControlPlaneSpec{ + ClusterName: strings.Repeat("A", maxClusterNameLength+1), + }, + }, + { + name: "autopilot enabled without release channel should cause an error", + expectError: true, + spec: GCPManagedControlPlaneSpec{ + ClusterName: "", + EnableAutopilot: true, + ReleaseChannel: nil, + }, + }, + { + name: "autopilot enabled with release channel", + expectError: false, + spec: GCPManagedControlPlaneSpec{ + ClusterName: "", + EnableAutopilot: true, + ReleaseChannel: &releaseChannel, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + mcp := &GCPManagedControlPlane{ + Spec: tc.spec, + } + warn, err := mcp.ValidateCreate() + + if tc.expectError { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + // Nothing emits warnings yet + g.Expect(warn).To(BeEmpty()) + }) + } +} + +func TestGCPManagedControlPlaneValidatingWebhookUpdate(t *testing.T) { + tests := []struct { + name string + expectError bool + spec GCPManagedControlPlaneSpec + }{ + { + name: "request to change cluster name should cause an error", + expectError: true, + spec: GCPManagedControlPlaneSpec{ + ClusterName: "default_cluster2", + }, + }, + { + name: "request to change project should cause an error", + expectError: true, + spec: GCPManagedControlPlaneSpec{ + ClusterName: "default_cluster1", + Project: "new-project", + }, + }, + { + name: "request to change location should cause an error", + expectError: true, + spec: GCPManagedControlPlaneSpec{ + ClusterName: "default_cluster1", + Location: "us-west4", + }, + }, + { + name: "request to enable/disable autopilot should cause an error", + expectError: true, + spec: GCPManagedControlPlaneSpec{ + ClusterName: "default_cluster1", + EnableAutopilot: true, + }, + }, + { + name: "request to change network should not cause an error", + expectError: false, + spec: GCPManagedControlPlaneSpec{ + ClusterName: "default_cluster1", + ClusterNetwork: &ClusterNetwork{ + PrivateCluster: &PrivateCluster{ + EnablePrivateEndpoint: false, + }, + }, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + newMCP := &GCPManagedControlPlane{ + Spec: tc.spec, + } + oldMCP := &GCPManagedControlPlane{ + Spec: GCPManagedControlPlaneSpec{ + ClusterName: "default_cluster1", + ClusterNetwork: &ClusterNetwork{ + PrivateCluster: &PrivateCluster{ + EnablePrivateEndpoint: true, + }, + }, + }, + } + + warn, err := newMCP.ValidateUpdate(oldMCP) + + if tc.expectError { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + // Nothing emits warnings yet + g.Expect(warn).To(BeEmpty()) + }) + } +} From 2d45b1887ca9e4eea3e63fd9027b551e4bc5ff50 Mon Sep 17 00:00:00 2001 From: Carlos Salas Date: Mon, 21 Oct 2024 10:05:29 +0200 Subject: [PATCH 2/3] test: align managed mp webhook tests Signed-off-by: Carlos Salas --- .../gcpmanagedmachinepool_webhook_test.go | 294 ++++++++++-------- 1 file changed, 166 insertions(+), 128 deletions(-) diff --git a/exp/api/v1beta1/gcpmanagedmachinepool_webhook_test.go b/exp/api/v1beta1/gcpmanagedmachinepool_webhook_test.go index 3dacba740..410e76937 100644 --- a/exp/api/v1beta1/gcpmanagedmachinepool_webhook_test.go +++ b/exp/api/v1beta1/gcpmanagedmachinepool_webhook_test.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Kubernetes Authors. +Copyright 2024 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. @@ -18,149 +18,187 @@ package v1beta1 import ( "strings" + "testing" - . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" infrav1 "sigs.k8s.io/cluster-api-provider-gcp/api/v1beta1" ) -var gcpmmp *GCPManagedMachinePool - -var _ = Describe("Test GCPManagedMachinePool Webhooks", func() { - BeforeEach(func() { - machineType := "e2-medium" - diskSizeGb := int32(100) +var ( + minCount = int32(1) + maxCount = int32(3) + invalidMinCount = int32(-1) + enableAutoscaling = false + diskSizeGb = int32(200) + maxPods = int64(10) + localSsds = int32(0) + invalidDiskSizeGb = int32(-200) + invalidMaxPods = int64(-10) + invalidLocalSsds = int32(-0) +) - gcpmmp = &GCPManagedMachinePool{ - Spec: GCPManagedMachinePoolSpec{ - NodePoolName: "test-gke-pool", - MachineType: &machineType, - DiskSizeGb: &diskSizeGb, +func TestGCPManagedMachinePoolValidatingWebhookCreate(t *testing.T) { + tests := []struct { + name string + spec GCPManagedMachinePoolSpec + expectError bool + }{ + { + name: "valid node pool name", + spec: GCPManagedMachinePoolSpec{ + NodePoolName: "nodepool1", }, - } - }) - - Context("Test validateSpec", func() { - It("should error when node pool name is too long", func() { - gcpmmp.Spec.NodePoolName = strings.Repeat("A", maxNodePoolNameLength+1) - errs := gcpmmp.validateSpec() - Expect(errs).ToNot(BeEmpty()) - }) - It("should pass when node pool name is within limit", func() { - gcpmmp.Spec.NodePoolName = strings.Repeat("A", maxNodePoolNameLength) - errs := gcpmmp.validateSpec() - Expect(errs).To(BeEmpty()) - }) - }) + expectError: false, + }, + { + name: "node pool name is too long", + spec: GCPManagedMachinePoolSpec{ + NodePoolName: strings.Repeat("A", maxNodePoolNameLength+1), + }, + expectError: true, + }, + { + name: "scaling with valid min/max count", + spec: GCPManagedMachinePoolSpec{ + NodePoolName: "nodepool1", + Scaling: &NodePoolAutoScaling{ + MinCount: &minCount, + MaxCount: &maxCount, + }, + }, + expectError: false, + }, + { + name: "scaling with invalid min/max count", + spec: GCPManagedMachinePoolSpec{ + NodePoolName: "nodepool1", + Scaling: &NodePoolAutoScaling{ + MinCount: &invalidMinCount, + MaxCount: &maxCount, + }, + }, + expectError: true, + }, + { + name: "scaling with max < min count", + spec: GCPManagedMachinePoolSpec{ + NodePoolName: "nodepool1", + Scaling: &NodePoolAutoScaling{ + MinCount: &maxCount, + MaxCount: &minCount, + }, + }, + expectError: true, + }, + { + name: "autoscaling disabled and min/max provided", + spec: GCPManagedMachinePoolSpec{ + NodePoolName: "nodepool1", + Scaling: &NodePoolAutoScaling{ + EnableAutoscaling: &enableAutoscaling, + MinCount: &minCount, + MaxCount: &maxCount, + }, + }, + expectError: true, + }, + { + name: "valid non-negative values", + spec: GCPManagedMachinePoolSpec{ + NodePoolName: "nodepool1", + DiskSizeGb: &diskSizeGb, + MaxPodsPerNode: &maxPods, + LocalSsdCount: &localSsds, + }, + expectError: false, + }, + { + name: "invalid negative values", + spec: GCPManagedMachinePoolSpec{ + NodePoolName: "nodepool1", + DiskSizeGb: &invalidDiskSizeGb, + MaxPodsPerNode: &invalidMaxPods, + LocalSsdCount: &invalidLocalSsds, + }, + expectError: true, + }, + } - Context("Test validateScaling", func() { - It("should pass when scaling is not specified", func() { - errs := gcpmmp.validateScaling() - Expect(errs).To(BeEmpty()) - }) - It("should pass when min/max count is valid", func() { - minCount := int32(1) - maxCount := int32(3) - gcpmmp.Spec.Scaling = &NodePoolAutoScaling{ - MinCount: &minCount, - MaxCount: &maxCount, - } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) - errs := gcpmmp.validateScaling() - Expect(errs).To(BeEmpty()) - }) - It("should fail when min is negative", func() { - minCount := int32(-1) - gcpmmp.Spec.Scaling = &NodePoolAutoScaling{ - MinCount: &minCount, + mmp := &GCPManagedMachinePool{ + Spec: tc.spec, } + warn, err := mmp.ValidateCreate() - errs := gcpmmp.validateScaling() - Expect(errs).ToNot(BeEmpty()) - }) - It("should fail when min > max", func() { - minCount := int32(3) - maxCount := int32(1) - gcpmmp.Spec.Scaling = &NodePoolAutoScaling{ - MinCount: &minCount, - MaxCount: &maxCount, + if tc.expectError { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) } - - errs := gcpmmp.validateScaling() - Expect(errs).ToNot(BeEmpty()) + // Nothing emits warnings yet + g.Expect(warn).To(BeEmpty()) }) - It("should fail when autoscaling is disabled and min/max is specified", func() { - minCount := int32(1) - maxCount := int32(3) - enabled := false - locationPolicy := ManagedNodePoolLocationPolicyAny - gcpmmp.Spec.Scaling = &NodePoolAutoScaling{ - MinCount: &minCount, - MaxCount: &maxCount, - EnableAutoscaling: &enabled, - LocationPolicy: &locationPolicy, - } + } +} + +func TestGCPManagedMachinePoolValidatingWebhookUpdate(t *testing.T) { + tests := []struct { + name string + spec GCPManagedMachinePoolSpec + expectError bool + }{ + { + name: "node pool is not mutated", + spec: GCPManagedMachinePoolSpec{ + NodePoolName: "nodepool1", + }, + expectError: false, + }, + { + name: "mutable fields are mutated", + spec: GCPManagedMachinePoolSpec{ + NodePoolName: "nodepool1", + AdditionalLabels: infrav1.Labels{ + "testKey": "testVal", + }, + }, + expectError: false, + }, + { + name: "immutable field disk size is mutated", + spec: GCPManagedMachinePoolSpec{ + NodePoolName: "nodepool1", + DiskSizeGb: &diskSizeGb, + }, + expectError: true, + }, + } - errs := gcpmmp.validateScaling() - Expect(errs).To(HaveLen(3)) - }) - }) - Context("Test validateImmutable", func() { - It("should pass when node pool is not mutated", func() { - old := gcpmmp.DeepCopy() - errs := gcpmmp.validateImmutable(old) - Expect(errs).To(BeEmpty()) - }) - It("should pass when mutable fields are mutated", func() { - old := gcpmmp.DeepCopy() - gcpmmp.Spec.AdditionalLabels = infrav1.Labels{ - "testKey": "testVal", - } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) - errs := gcpmmp.validateImmutable(old) - Expect(errs).To(BeEmpty()) - }) - It("should fail when immutable fields are mutated", func() { - old := gcpmmp.DeepCopy() - diskSizeGb := int32(200) - gcpmmp.Spec.DiskSizeGb = &diskSizeGb - gcpmmp.Spec.NodePoolName = "new-name" - gcpmmp.Spec.Management = &NodePoolManagement{ - AutoUpgrade: false, - AutoRepair: false, + newMMP := &GCPManagedMachinePool{ + Spec: tc.spec, + } + oldMMP := &GCPManagedMachinePool{ + Spec: GCPManagedMachinePoolSpec{ + NodePoolName: "nodepool1", + }, } - errs := gcpmmp.validateImmutable(old) - Expect(errs).To(HaveLen(3)) - }) - }) + warn, err := newMMP.ValidateUpdate(oldMMP) - Context("Test validateNonNegative", func() { - It("should pass when number fields are not specified", func() { - errs := gcpmmp.validateNonNegative() - Expect(errs).To(BeEmpty()) - }) - It("should pass when number fields are non-negative", func() { - maxPods := int64(10) - localSsds := int32(0) - diskSize := int32(200) - gcpmmp.Spec.MaxPodsPerNode = &maxPods - gcpmmp.Spec.LocalSsdCount = &localSsds - gcpmmp.Spec.DiskSizeGb = &diskSize - - errs := gcpmmp.validateNonNegative() - Expect(errs).To(BeEmpty()) - }) - It("should pass when some number fields are negative", func() { - maxPods := int64(-1) - localSsds := int32(0) - diskSize := int32(-100) - gcpmmp.Spec.MaxPodsPerNode = &maxPods - gcpmmp.Spec.LocalSsdCount = &localSsds - gcpmmp.Spec.DiskSizeGb = &diskSize - - errs := gcpmmp.validateNonNegative() - Expect(errs).To(HaveLen(2)) + if tc.expectError { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + // Nothing emits warnings yet + g.Expect(warn).To(BeEmpty()) }) - }) -}) + } +} From 37520779fa15d4443d5ce4c7199a67bbfc6cdc13 Mon Sep 17 00:00:00 2001 From: Carlos Salas Date: Mon, 21 Oct 2024 10:05:52 +0200 Subject: [PATCH 3/3] test: add managed cluster webhook test cases Signed-off-by: Carlos Salas --- .../v1beta1/gcpmanagedcluster_webhook_test.go | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 exp/api/v1beta1/gcpmanagedcluster_webhook_test.go diff --git a/exp/api/v1beta1/gcpmanagedcluster_webhook_test.go b/exp/api/v1beta1/gcpmanagedcluster_webhook_test.go new file mode 100644 index 000000000..d82b57b7c --- /dev/null +++ b/exp/api/v1beta1/gcpmanagedcluster_webhook_test.go @@ -0,0 +1,114 @@ +/* +Copyright 2024 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 v1beta1 + +import ( + "testing" + + . "github.com/onsi/gomega" + infrav1 "sigs.k8s.io/cluster-api-provider-gcp/api/v1beta1" +) + +func TestGCPManagedClusterValidatingWebhookUpdate(t *testing.T) { + tests := []struct { + name string + expectError bool + spec GCPManagedClusterSpec + }{ + { + name: "request to change mutable field additional labels", + expectError: false, + spec: GCPManagedClusterSpec{ + Project: "old-project", + Region: "us-west1", + CredentialsRef: &infrav1.ObjectReference{ + Namespace: "default", + Name: "credsref", + }, + AdditionalLabels: map[string]string{ + "testKey": "testVal", + }, + }, + }, + { + name: "request to change immutable field project", + expectError: true, + spec: GCPManagedClusterSpec{ + Project: "new-project", + Region: "us-west1", + CredentialsRef: &infrav1.ObjectReference{ + Namespace: "default", + Name: "credsref", + }, + }, + }, + { + name: "request to change immutable field region", + expectError: true, + spec: GCPManagedClusterSpec{ + Project: "old-project", + Region: "us-central1", + CredentialsRef: &infrav1.ObjectReference{ + Namespace: "default", + Name: "credsref", + }, + }, + }, + { + name: "request to change immutable field credentials ref", + expectError: true, + spec: GCPManagedClusterSpec{ + Project: "old-project", + Region: "us-central1", + CredentialsRef: &infrav1.ObjectReference{ + Namespace: "new-ns", + Name: "new-name", + }, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + newMC := &GCPManagedCluster{ + Spec: tc.spec, + } + oldMC := &GCPManagedCluster{ + Spec: GCPManagedClusterSpec{ + Project: "old-project", + Region: "us-west1", + CredentialsRef: &infrav1.ObjectReference{ + Namespace: "default", + Name: "credsref", + }, + }, + } + + warn, err := newMC.ValidateUpdate(oldMC) + + if tc.expectError { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + // Nothing emits warnings yet + g.Expect(warn).To(BeEmpty()) + }) + } +}