From 45ede3140275bed79a8051944a916f3652cf285c Mon Sep 17 00:00:00 2001 From: Nawaz Hussain K Date: Thu, 10 Aug 2023 14:03:58 -0700 Subject: [PATCH] validate if DNS Service IP is a .10 IP - Removed `validateDNSServiceIP()` from the validators because the DNS Service IP validation was being done in `validateManagedClusterNetwork()`. - update `validateManagedClusterNetwork()` to validate if DNS Service IP is a .10 IP - update mockClient.Get to return clusterv1.Cluster with Service CIDR : `192.168.0.0/26` - update amcp_webhook_test with valid DNS Service IPs and mock client for webhook tests --- api/v1beta1/azuremachine_default_test.go | 15 +- .../azuremanagedcontrolplane_webhook.go | 28 +-- .../azuremanagedcontrolplane_webhook_test.go | 215 +++++++++++++----- 3 files changed, 180 insertions(+), 78 deletions(-) diff --git a/api/v1beta1/azuremachine_default_test.go b/api/v1beta1/azuremachine_default_test.go index b18dbbea797..3d1f4346a13 100644 --- a/api/v1beta1/azuremachine_default_test.go +++ b/api/v1beta1/azuremachine_default_test.go @@ -521,10 +521,17 @@ func (m mockClient) Get(ctx context.Context, key client.ObjectKey, obj client.Ob case *AzureCluster: obj.Spec.SubscriptionID = "test-subscription-id" case *clusterv1.Cluster: - obj.Spec.InfrastructureRef = &corev1.ObjectReference{ - Kind: "AzureCluster", - Name: "test-cluster", - Namespace: "default", + obj.Spec = clusterv1.ClusterSpec{ + InfrastructureRef: &corev1.ObjectReference{ + Kind: "AzureCluster", + Name: "test-cluster", + Namespace: "default", + }, + ClusterNetwork: &clusterv1.ClusterNetwork{ + Services: &clusterv1.NetworkRanges{ + CIDRBlocks: []string{"192.168.0.0/26"}, + }, + }, } default: return errors.New("unexpected object type") diff --git a/api/v1beta1/azuremanagedcontrolplane_webhook.go b/api/v1beta1/azuremanagedcontrolplane_webhook.go index 8853d97ee5c..9035e010bcd 100644 --- a/api/v1beta1/azuremanagedcontrolplane_webhook.go +++ b/api/v1beta1/azuremanagedcontrolplane_webhook.go @@ -253,7 +253,6 @@ func (m *AzureManagedControlPlane) Validate(cli client.Client) error { validators := []func(client client.Client) error{ m.validateName, m.validateVersion, - m.validateDNSServiceIP, m.validateSSHKey, m.validateLoadBalancerProfile, m.validateAPIServerAccessProfile, @@ -271,17 +270,6 @@ func (m *AzureManagedControlPlane) Validate(cli client.Client) error { return kerrors.NewAggregate(errs) } -// validateDNSServiceIP validates the DNSServiceIP. -func (m *AzureManagedControlPlane) validateDNSServiceIP(_ client.Client) error { - if m.Spec.DNSServiceIP != nil { - if net.ParseIP(*m.Spec.DNSServiceIP) == nil { - return errors.New("DNSServiceIP must be a valid IP") - } - } - - return nil -} - // validateVersion validates the Kubernetes version. func (m *AzureManagedControlPlane) validateVersion(_ client.Client) error { if !kubeSemver.MatchString(m.Spec.Version) { @@ -421,10 +409,22 @@ func (m *AzureManagedControlPlane) validateManagedClusterNetwork(cli client.Clie if err != nil { allErrs = append(allErrs, field.Invalid(field.NewPath("Cluster", "Spec", "ClusterNetwork", "Services", "CIDRBlocks"), serviceCIDR, fmt.Sprintf("failed to parse cluster service cidr: %v", err))) } - ip := net.ParseIP(*m.Spec.DNSServiceIP) - if !cidr.Contains(ip) { + + dnsIP := net.ParseIP(*m.Spec.DNSServiceIP) + if dnsIP == nil { // dnsIP will be nil if the string is not a valid IP + allErrs = append(allErrs, field.Invalid(field.NewPath("Spec", "DNSServiceIP"), *m.Spec.DNSServiceIP, "must be a valid IP address")) + } + + if dnsIP != nil && !cidr.Contains(dnsIP) { allErrs = append(allErrs, field.Invalid(field.NewPath("Cluster", "Spec", "ClusterNetwork", "Services", "CIDRBlocks"), serviceCIDR, "DNSServiceIP must reside within the associated cluster serviceCIDR")) } + + // AKS only supports .10 as the last octet for the DNSServiceIP. + // Refer to: https://learn.microsoft.com/en-us/azure/aks/configure-kubenet#create-an-aks-cluster-with-system-assigned-managed-identities + targetSuffix := ".10" + if dnsIP != nil && !strings.HasSuffix(dnsIP.String(), targetSuffix) { + allErrs = append(allErrs, field.Invalid(field.NewPath("Spec", "DNSServiceIP"), *m.Spec.DNSServiceIP, fmt.Sprintf("must end with %q", targetSuffix))) + } } if errs := validatePrivateEndpoints(m.Spec.VirtualNetwork.Subnet.PrivateEndpoints, []string{m.Spec.VirtualNetwork.Subnet.CIDRBlock}, field.NewPath("Spec", "VirtualNetwork.Subnet.PrivateEndpoints")); len(errs) > 0 { diff --git a/api/v1beta1/azuremanagedcontrolplane_webhook_test.go b/api/v1beta1/azuremanagedcontrolplane_webhook_test.go index 4e46d28e377..f184fcce2b5 100644 --- a/api/v1beta1/azuremanagedcontrolplane_webhook_test.go +++ b/api/v1beta1/azuremanagedcontrolplane_webhook_test.go @@ -95,8 +95,9 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing valid DNSServiceIP", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.17.8", }, }, @@ -105,18 +106,41 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing invalid DNSServiceIP", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0.3"), + DNSServiceIP: pointer.String("192.168.0.10.3"), Version: "v1.17.8", }, }, expectErr: true, }, + { + name: "Testing invalid DNSServiceIP", + amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), + Spec: AzureManagedControlPlaneSpec{ + DNSServiceIP: pointer.String("192.168.0.11"), + Version: "v1.17.8", + }, + }, + expectErr: true, + }, + { + name: "Testing empty DNSServiceIP", + amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), + Spec: AzureManagedControlPlaneSpec{ + Version: "v1.17.8", + }, + }, + expectErr: false, + }, { name: "Invalid Version", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "honk", }, }, @@ -125,8 +149,9 @@ func TestValidatingWebhook(t *testing.T) { { name: "not following the Kubernetes Version pattern", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "1.19.0", }, }, @@ -135,8 +160,9 @@ func TestValidatingWebhook(t *testing.T) { { name: "Version not set", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "", }, }, @@ -145,8 +171,9 @@ func TestValidatingWebhook(t *testing.T) { { name: "Valid Version", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.17.8", }, }, @@ -155,6 +182,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Valid Managed AADProfile", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.21.2", AADProfile: &AADProfile{ @@ -170,6 +198,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Valid LoadBalancerProfile", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.21.2", LoadBalancerProfile: &LoadBalancerProfile{ @@ -184,6 +213,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Invalid LoadBalancerProfile.ManagedOutboundIPs", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.21.2", LoadBalancerProfile: &LoadBalancerProfile{ @@ -196,6 +226,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Invalid LoadBalancerProfile.AllocatedOutboundPorts", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.21.2", LoadBalancerProfile: &LoadBalancerProfile{ @@ -208,6 +239,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Invalid LoadBalancerProfile.IdleTimeoutInMinutes", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.21.2", LoadBalancerProfile: &LoadBalancerProfile{ @@ -220,6 +252,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "LoadBalancerProfile must specify at most one of ManagedOutboundIPs, OutboundIPPrefixes and OutboundIPs", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.21.2", LoadBalancerProfile: &LoadBalancerProfile{ @@ -235,6 +268,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Invalid CIDR for AuthorizedIPRanges", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.21.2", APIServerAccessProfile: &APIServerAccessProfile{ @@ -247,6 +281,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing valid AutoScalerProfile", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -275,6 +310,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing valid AutoScalerProfile.ExpanderRandom", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -287,6 +323,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing valid AutoScalerProfile.ExpanderLeastWaste", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -299,6 +336,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing valid AutoScalerProfile.ExpanderMostPods", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -311,6 +349,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing valid AutoScalerProfile.ExpanderPriority", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -323,6 +362,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing valid AutoScalerProfile.BalanceSimilarNodeGroupsTrue", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -335,6 +375,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing valid AutoScalerProfile.BalanceSimilarNodeGroupsFalse", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -347,6 +388,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing invalid AutoScalerProfile.MaxEmptyBulkDelete", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -359,6 +401,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing invalid AutoScalerProfile.MaxGracefulTerminationSec", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -371,6 +414,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing invalid AutoScalerProfile.MaxNodeProvisionTime", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -383,6 +427,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing invalid AutoScalerProfile.MaxTotalUnreadyPercentage", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -395,6 +440,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing invalid AutoScalerProfile.NewPodScaleUpDelay", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -407,6 +453,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing invalid AutoScalerProfile.OkTotalUnreadyCount", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -419,6 +466,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing invalid AutoScalerProfile.ScanInterval", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -431,6 +479,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing invalid AutoScalerProfile.ScaleDownDelayAfterAdd", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -443,6 +492,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing invalid AutoScalerProfile.ScaleDownDelayAfterDelete", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -455,6 +505,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing invalid AutoScalerProfile.ScaleDownDelayAfterFailure", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -467,6 +518,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing invalid AutoScalerProfile.ScaleDownUnneededTime", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -479,6 +531,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing invalid AutoScalerProfile.ScaleDownUnreadyTime", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -491,6 +544,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing invalid AutoScalerProfile.ScaleDownUtilizationThreshold", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -503,6 +557,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing valid AutoScalerProfile.SkipNodesWithLocalStorageTrue", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -515,6 +570,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing valid AutoScalerProfile.SkipNodesWithLocalStorageFalse", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -527,6 +583,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing valid AutoScalerProfile.SkipNodesWithSystemPodsTrue", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -539,6 +596,7 @@ func TestValidatingWebhook(t *testing.T) { { name: "Testing valid AutoScalerProfile.SkipNodesWithSystemPodsFalse", amcp: AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ Version: "v1.24.1", AutoScalerProfile: &AutoScalerProfile{ @@ -552,8 +610,12 @@ func TestValidatingWebhook(t *testing.T) { for _, tt := range tests { tt := tt + // client is used to fetch the AzureManagedControlPlane, we do not want to return an error on client.Get + client := mockClient{ReturnError: false} t.Run(tt.name, func(t *testing.T) { - mcpw := &azureManagedControlPlaneWebhook{} + mcpw := &azureManagedControlPlaneWebhook{ + Client: client, + } if tt.expectErr { g.Expect(mcpw.ValidateCreate(context.Background(), &tt.amcp)).NotTo(Succeed()) } else { @@ -582,25 +644,31 @@ func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) { }, { name: "invalid DNSServiceIP", - amcp: createAzureManagedControlPlane("192.168.0.0.3", "v1.18.0", generateSSHPublicKey(true)), + amcp: createAzureManagedControlPlane("192.168.0.10.3", "v1.18.0", generateSSHPublicKey(true)), + wantErr: true, + errorLen: 1, + }, + { + name: "invalid DNSServiceIP", + amcp: createAzureManagedControlPlane("192.168.0.11", "v1.18.0", generateSSHPublicKey(true)), wantErr: true, errorLen: 1, }, { name: "invalid sshKey", - amcp: createAzureManagedControlPlane("192.168.0.0", "v1.18.0", generateSSHPublicKey(false)), + amcp: createAzureManagedControlPlane("192.168.0.10", "v1.18.0", generateSSHPublicKey(false)), wantErr: true, errorLen: 1, }, { name: "invalid sshKey with a simple text and invalid DNSServiceIP", - amcp: createAzureManagedControlPlane("192.168.0.0.3", "v1.18.0", "invalid_sshkey_honk"), + amcp: createAzureManagedControlPlane("192.168.0.10.3", "v1.18.0", "invalid_sshkey_honk"), wantErr: true, errorLen: 2, }, { name: "invalid version", - amcp: createAzureManagedControlPlane("192.168.0.0", "honk.version", generateSSHPublicKey(true)), + amcp: createAzureManagedControlPlane("192.168.0.10", "honk.version", generateSSHPublicKey(true)), wantErr: true, errorLen: 1, }, @@ -612,7 +680,7 @@ func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) { }, Spec: AzureManagedControlPlaneSpec{ SSHPublicKey: generateSSHPublicKey(true), - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.23.5", }, }, @@ -627,7 +695,7 @@ func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) { }, Spec: AzureManagedControlPlaneSpec{ SSHPublicKey: generateSSHPublicKey(true), - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.23.5", }, }, @@ -641,7 +709,7 @@ func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) { ControlPlaneEndpoint: clusterv1.APIEndpoint{ Host: "my-host", }, - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", SSHPublicKey: generateSSHPublicKey(true), AADProfile: &AADProfile{ @@ -661,7 +729,7 @@ func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) { ControlPlaneEndpoint: clusterv1.APIEndpoint{ Port: 444, }, - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", SSHPublicKey: generateSSHPublicKey(true), AADProfile: &AADProfile{ @@ -675,9 +743,12 @@ func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) { wantErr: false, }, } + client := mockClient{ReturnError: false} for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - mcpw := &azureManagedControlPlaneWebhook{} + mcpw := &azureManagedControlPlaneWebhook{ + Client: client, + } err := mcpw.ValidateCreate(context.Background(), tc.amcp) if tc.wantErr { g.Expect(err).To(HaveOccurred()) @@ -710,10 +781,13 @@ func TestAzureManagedControlPlane_ValidateCreateFailure(t *testing.T) { deferFunc: func() {}, }, } + client := mockClient{ReturnError: false} for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { defer tc.deferFunc() - mcpw := &azureManagedControlPlaneWebhook{} + mcpw := &azureManagedControlPlaneWebhook{ + Client: client, + } err := mcpw.ValidateCreate(context.Background(), tc.amcp) g.Expect(err).To(HaveOccurred()) }) @@ -732,40 +806,46 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { }{ { name: "can't add a SSHPublicKey to an existing AzureManagedControlPlane", - oldAMCP: createAzureManagedControlPlane("192.168.0.0", "v1.18.0", ""), - amcp: createAzureManagedControlPlane("192.168.0.0", "v1.18.0", generateSSHPublicKey(true)), + oldAMCP: createAzureManagedControlPlane("192.168.0.10", "v1.18.0", ""), + amcp: createAzureManagedControlPlane("192.168.0.10", "v1.18.0", generateSSHPublicKey(true)), wantErr: true, }, { name: "same SSHPublicKey is valid", - oldAMCP: createAzureManagedControlPlane("192.168.0.0", "v1.18.0", commonSSHKey), - amcp: createAzureManagedControlPlane("192.168.0.0", "v1.18.0", commonSSHKey), + oldAMCP: createAzureManagedControlPlane("192.168.0.10", "v1.18.0", commonSSHKey), + amcp: createAzureManagedControlPlane("192.168.0.10", "v1.18.0", commonSSHKey), wantErr: false, }, { name: "AzureManagedControlPlane with invalid serviceIP", oldAMCP: createAzureManagedControlPlane("", "v1.18.0", ""), - amcp: createAzureManagedControlPlane("192.168.0.0.3", "v1.18.0", generateSSHPublicKey(true)), + amcp: createAzureManagedControlPlane("192.168.0.10.3", "v1.18.0", generateSSHPublicKey(true)), + wantErr: true, + }, + { + name: "AzureManagedControlPlane with invalid serviceIP", + oldAMCP: createAzureManagedControlPlane("", "v1.18.0", ""), + amcp: createAzureManagedControlPlane("192.168.0.11", "v1.18.0", generateSSHPublicKey(true)), wantErr: true, }, { name: "AzureManagedControlPlane with invalid version", oldAMCP: createAzureManagedControlPlane("", "v1.18.0", ""), - amcp: createAzureManagedControlPlane("192.168.0.0", "1.999.9", generateSSHPublicKey(true)), + amcp: createAzureManagedControlPlane("192.168.0.10", "1.999.9", generateSSHPublicKey(true)), wantErr: true, }, { name: "AzureManagedControlPlane SubscriptionID is immutable", oldAMCP: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), SubscriptionID: "212ec1q8", Version: "v1.18.0", }, }, amcp: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), SubscriptionID: "212ec1q9", Version: "v1.18.0", }, @@ -776,14 +856,14 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { name: "AzureManagedControlPlane ResourceGroupName is immutable", oldAMCP: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), ResourceGroupName: "hello-1", Version: "v1.18.0", }, }, amcp: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), ResourceGroupName: "hello-2", Version: "v1.18.0", }, @@ -794,14 +874,14 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { name: "AzureManagedControlPlane NodeResourceGroupName is immutable", oldAMCP: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), NodeResourceGroupName: "hello-1", Version: "v1.18.0", }, }, amcp: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), NodeResourceGroupName: "hello-2", Version: "v1.18.0", }, @@ -812,14 +892,14 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { name: "AzureManagedControlPlane Location is immutable", oldAMCP: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Location: "westeurope", Version: "v1.18.0", }, }, amcp: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Location: "eastus", Version: "v1.18.0", }, @@ -830,14 +910,14 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { name: "AzureManagedControlPlane SSHPublicKey is immutable", oldAMCP: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), SSHPublicKey: generateSSHPublicKey(true), Version: "v1.18.0", }, }, amcp: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), SSHPublicKey: generateSSHPublicKey(true), Version: "v1.18.0", }, @@ -848,13 +928,13 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { name: "AzureManagedControlPlane DNSServiceIP is immutable", oldAMCP: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", }, }, amcp: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.1"), + DNSServiceIP: pointer.String("192.168.1.1"), Version: "v1.18.0", }, }, @@ -864,7 +944,7 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { name: "AzureManagedControlPlane DNSServiceIP is immutable, unsetting is not allowed", oldAMCP: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", }, }, @@ -879,14 +959,14 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { name: "AzureManagedControlPlane NetworkPlugin is immutable", oldAMCP: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), NetworkPlugin: pointer.String("azure"), Version: "v1.18.0", }, }, amcp: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), NetworkPlugin: pointer.String("kubenet"), Version: "v1.18.0", }, @@ -897,14 +977,14 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { name: "AzureManagedControlPlane NetworkPlugin is immutable, unsetting is not allowed", oldAMCP: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), NetworkPlugin: pointer.String("azure"), Version: "v1.18.0", }, }, amcp: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", }, }, @@ -914,14 +994,14 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { name: "AzureManagedControlPlane NetworkPolicy is immutable", oldAMCP: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), NetworkPolicy: pointer.String("azure"), Version: "v1.18.0", }, }, amcp: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), NetworkPolicy: pointer.String("calico"), Version: "v1.18.0", }, @@ -932,14 +1012,14 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { name: "AzureManagedControlPlane NetworkPolicy is immutable, unsetting is not allowed", oldAMCP: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), NetworkPolicy: pointer.String("azure"), Version: "v1.18.0", }, }, amcp: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", }, }, @@ -949,14 +1029,14 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { name: "AzureManagedControlPlane LoadBalancerSKU is immutable", oldAMCP: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), LoadBalancerSKU: pointer.String("Standard"), Version: "v1.18.0", }, }, amcp: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), LoadBalancerSKU: pointer.String("Basic"), Version: "v1.18.0", }, @@ -967,14 +1047,14 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { name: "AzureManagedControlPlane LoadBalancerSKU is immutable, unsetting is not allowed", oldAMCP: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), LoadBalancerSKU: pointer.String("Standard"), Version: "v1.18.0", }, }, amcp: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", }, }, @@ -1095,13 +1175,13 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { name: "AzureManagedControlPlane EnablePrivateCluster is immutable", oldAMCP: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", }, }, amcp: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", APIServerAccessProfile: &APIServerAccessProfile{ EnablePrivateCluster: pointer.Bool(true), @@ -1114,13 +1194,13 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { name: "AzureManagedControlPlane AuthorizedIPRanges is mutable", oldAMCP: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", }, }, amcp: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", APIServerAccessProfile: &APIServerAccessProfile{ AuthorizedIPRanges: []string{"192.168.0.1/32"}, @@ -1136,7 +1216,7 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { Name: "test-cluster", }, Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", VirtualNetwork: ManagedControlPlaneVirtualNetwork{ Name: "test-network", @@ -1154,7 +1234,7 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { Name: "test-cluster", }, Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", }, }, @@ -1167,7 +1247,7 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { Name: "test-cluster", }, Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", }, }, @@ -1176,7 +1256,7 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { Name: "test-cluster", }, Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", VirtualNetwork: ManagedControlPlaneVirtualNetwork{ Name: "test-network", @@ -1198,7 +1278,7 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { Name: "test-cluster", }, Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", VirtualNetwork: ManagedControlPlaneVirtualNetwork{ Name: "test-network", @@ -1216,7 +1296,7 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { Name: "test-cluster", }, Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", VirtualNetwork: ManagedControlPlaneVirtualNetwork{ Name: "test-network", @@ -1252,9 +1332,12 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { wantErr: true, }, } + client := mockClient{ReturnError: false} for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - mcpw := &azureManagedControlPlaneWebhook{} + mcpw := &azureManagedControlPlaneWebhook{ + Client: client, + } err := mcpw.ValidateUpdate(context.Background(), tc.oldAMCP, tc.amcp) if tc.wantErr { g.Expect(err).To(HaveOccurred()) @@ -1267,6 +1350,7 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { func createAzureManagedControlPlane(serviceIP, version, sshKey string) *AzureManagedControlPlane { return &AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ SSHPublicKey: sshKey, DNSServiceIP: pointer.String(serviceIP), @@ -1277,8 +1361,9 @@ func createAzureManagedControlPlane(serviceIP, version, sshKey string) *AzureMan func getKnownValidAzureManagedControlPlane() *AzureManagedControlPlane { return &AzureManagedControlPlane{ + ObjectMeta: getAMCPMetaData(), Spec: AzureManagedControlPlaneSpec{ - DNSServiceIP: pointer.String("192.168.0.0"), + DNSServiceIP: pointer.String("192.168.0.10"), Version: "v1.18.0", SSHPublicKey: generateSSHPublicKey(true), AADProfile: &AADProfile{ @@ -1290,3 +1375,13 @@ func getKnownValidAzureManagedControlPlane() *AzureManagedControlPlane { }, } } + +func getAMCPMetaData() metav1.ObjectMeta { + return metav1.ObjectMeta{ + Name: "test-AMCP", + Labels: map[string]string{ + "cluster.x-k8s.io/cluster-name": "test-cluster", + }, + Namespace: "default", + } +}