diff --git a/api/v1beta1/azuremanagedcontrolplane_webhook.go b/api/v1beta1/azuremanagedcontrolplane_webhook.go index 5637c32e3a3..ae145b4b0c0 100644 --- a/api/v1beta1/azuremanagedcontrolplane_webhook.go +++ b/api/v1beta1/azuremanagedcontrolplane_webhook.go @@ -244,6 +244,10 @@ func (mw *azureManagedControlPlaneWebhook) ValidateUpdate(ctx context.Context, o allErrs = append(allErrs, errs...) } + if errs := m.validateAddonProfilesUpdate(old); len(errs) > 0 { + allErrs = append(allErrs, errs...) + } + if errs := m.validateAPIServerAccessProfileUpdate(old); len(errs) > 0 { allErrs = append(allErrs, errs...) } @@ -523,6 +527,26 @@ func (m *AzureManagedControlPlane) validateAPIServerAccessProfileUpdate(old *Azu return allErrs } +// validateAddonProfilesUpdate validates update to AddonProfiles. +func (m *AzureManagedControlPlane) validateAddonProfilesUpdate(old *AzureManagedControlPlane) field.ErrorList { + var allErrs field.ErrorList + newAddonProfileMap := map[string]struct{}{} + if len(old.Spec.AddonProfiles) != 0 { + for _, addonProfile := range m.Spec.AddonProfiles { + newAddonProfileMap[addonProfile.Name] = struct{}{} + } + for i, addonProfile := range old.Spec.AddonProfiles { + if _, ok := newAddonProfileMap[addonProfile.Name]; !ok { + allErrs = append(allErrs, field.Invalid( + field.NewPath("Spec", "AddonProfiles"), + m.Spec.AddonProfiles, + fmt.Sprintf("cannot remove addonProfile %s, To disable this AddonProfile, update Spec.AddonProfiles[%v].Enabled to false", addonProfile.Name, i))) + } + } + } + return allErrs +} + // validateVirtualNetworkUpdate validates update to VirtualNetwork. func (m *AzureManagedControlPlane) validateVirtualNetworkUpdate(old *AzureManagedControlPlane) field.ErrorList { var allErrs field.ErrorList diff --git a/api/v1beta1/azuremanagedcontrolplane_webhook_test.go b/api/v1beta1/azuremanagedcontrolplane_webhook_test.go index ec5b8eaefe8..20f88d111b0 100644 --- a/api/v1beta1/azuremanagedcontrolplane_webhook_test.go +++ b/api/v1beta1/azuremanagedcontrolplane_webhook_test.go @@ -1036,6 +1036,102 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { amcp: createAzureManagedControlPlane("192.168.0.10", "1.999.9", generateSSHPublicKey(true)), wantErr: true, }, + { + name: "AzureManagedControlPlane AddonProfiles is mutable", + oldAMCP: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + Version: "v1.18.0", + }, + }, + amcp: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + Version: "v1.18.0", + AddonProfiles: []AddonProfile{ + { + Name: "first-addon-profile", + Enabled: true, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "AzureManagedControlPlane AddonProfiles can be disabled", + oldAMCP: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + AddonProfiles: []AddonProfile{ + { + Name: "first-addon-profile", + Enabled: true, + }, + }, + Version: "v1.18.0", + }, + }, + amcp: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + Version: "v1.18.0", + AddonProfiles: []AddonProfile{ + { + Name: "first-addon-profile", + Enabled: false, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "AzureManagedControlPlane AddonProfiles cannot update to empty array", + oldAMCP: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + AddonProfiles: []AddonProfile{ + { + Name: "first-addon-profile", + Enabled: true, + }, + }, + Version: "v1.18.0", + }, + }, + amcp: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + Version: "v1.18.0", + }, + }, + wantErr: true, + }, + { + name: "AzureManagedControlPlane AddonProfiles cannot be completely removed", + oldAMCP: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + AddonProfiles: []AddonProfile{ + { + Name: "first-addon-profile", + Enabled: true, + }, + { + Name: "second-addon-profile", + Enabled: true, + }, + }, + Version: "v1.18.0", + }, + }, + amcp: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + AddonProfiles: []AddonProfile{ + { + Name: "first-addon-profile", + Enabled: true, + }, + }, + Version: "v1.18.0", + }, + }, + wantErr: true, + }, { name: "AzureManagedControlPlane SubscriptionID is immutable", oldAMCP: &AzureManagedControlPlane{ diff --git a/azure/services/managedclusters/spec.go b/azure/services/managedclusters/spec.go index ac723ee0e72..a34b5bb338c 100644 --- a/azure/services/managedclusters/spec.go +++ b/azure/services/managedclusters/spec.go @@ -484,6 +484,21 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing interface{ return nil, azure.WithTransientError(errors.Errorf("Unable to update existing managed cluster in non-terminal state. Managed cluster must be in one of the following provisioning states: Canceled, Failed, or Succeeded. Actual state: %s", ps), 20*time.Second) } + if managedCluster.Properties.AddonProfiles == nil && existingMC.Properties.AddonProfiles != nil { + managedCluster.Properties.AddonProfiles = map[string]*armcontainerservice.ManagedClusterAddonProfile{} + } + for key, item := range existingMC.Properties.AddonProfiles { + if _, ok := managedCluster.Properties.AddonProfiles[key]; !ok { + addonProfile := &armcontainerservice.ManagedClusterAddonProfile{ + Enabled: ptr.To(false), + } + if item.Config != nil { + addonProfile.Config = item.Config + } + managedCluster.Properties.AddonProfiles[key] = addonProfile + } + existingMC.Properties.AddonProfiles[key].Identity = nil + } // Normalize the LoadBalancerProfile so the diff below doesn't get thrown off by AKS added properties. if managedCluster.Properties.NetworkProfile.LoadBalancerProfile == nil { // If our LoadBalancerProfile generated by the spec is nil, then don't worry about what AKS has added. @@ -631,6 +646,14 @@ func computeDiffOfNormalizedClusters(managedCluster armcontainerservice.ManagedC } } + if managedCluster.Properties.AddonProfiles != nil { + propertiesNormalized.AddonProfiles = managedCluster.Properties.AddonProfiles + } + + if existingMC.Properties.AddonProfiles != nil { + existingMCPropertiesNormalized.AddonProfiles = existingMC.Properties.AddonProfiles + } + if managedCluster.Properties.AutoScalerProfile != nil { propertiesNormalized.AutoScalerProfile = &armcontainerservice.ManagedClusterPropertiesAutoScalerProfile{ BalanceSimilarNodeGroups: managedCluster.Properties.AutoScalerProfile.BalanceSimilarNodeGroups, diff --git a/azure/services/managedclusters/spec_test.go b/azure/services/managedclusters/spec_test.go index 796f8cf19a3..5edd3f9d87b 100644 --- a/azure/services/managedclusters/spec_test.go +++ b/azure/services/managedclusters/spec_test.go @@ -485,6 +485,99 @@ func TestParameters(t *testing.T) { g.Expect(*result.(armcontainerservice.ManagedCluster).Properties.OidcIssuerProfile.Enabled).To(BeFalse()) }, }, + { + name: "do not update addon profile", + existing: getExistingClusterWithAddonProfile(), + spec: &ManagedClusterSpec{ + Name: "test-managedcluster", + ResourceGroup: "test-rg", + Location: "test-location", + Tags: map[string]string{ + "test-tag": "test-value", + }, + Version: "v1.22.0", + LoadBalancerSKU: "standard", + OIDCIssuerProfile: &OIDCIssuerProfile{ + Enabled: ptr.To(true), + }, + AddonProfiles: []AddonProfile{ + { + Name: "first-addon-profile", + Enabled: true, + }, + { + Name: "second-addon-profile", + Enabled: true, + }, + }, + }, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeNil()) + }, + }, + { + name: "update needed when addon profile enabled changes", + existing: getExistingClusterWithAddonProfile(), + spec: &ManagedClusterSpec{ + Name: "test-managedcluster", + ResourceGroup: "test-rg", + Location: "test-location", + Tags: map[string]string{ + "test-tag": "test-value", + }, + Version: "v1.22.0", + LoadBalancerSKU: "standard", + OIDCIssuerProfile: &OIDCIssuerProfile{ + Enabled: ptr.To(true), + }, + AddonProfiles: []AddonProfile{ + { + Name: "first-addon-profile", + Enabled: true, + }, + }, + }, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(armcontainerservice.ManagedCluster{})) + g.Expect(*result.(armcontainerservice.ManagedCluster).Properties.AddonProfiles["first-addon-profile"].Enabled).To(BeTrue()) + g.Expect(*result.(armcontainerservice.ManagedCluster).Properties.AddonProfiles["second-addon-profile"].Enabled).To(BeFalse()) + }, + }, + { + name: "update when we delete an addon profile", + existing: getExistingClusterWithAddonProfile(), + spec: &ManagedClusterSpec{ + Name: "test-managedcluster", + ResourceGroup: "test-rg", + Location: "test-location", + Tags: map[string]string{ + "test-tag": "test-value", + }, + Version: "v1.22.0", + LoadBalancerSKU: "standard", + OIDCIssuerProfile: &OIDCIssuerProfile{ + Enabled: ptr.To(true), + }, + AddonProfiles: []AddonProfile{ + { + Name: "first-addon-profile", + Enabled: true, + }, + { + Name: "second-addon-profile", + Enabled: true, + }, + { + Name: "third-addon-profile", + Enabled: true, + }, + }, + }, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(armcontainerservice.ManagedCluster{})) + g.Expect(*result.(armcontainerservice.ManagedCluster).Properties.AddonProfiles["third-addon-profile"].Enabled).To(BeTrue()) + }, + }, } for _, tc := range testcases { tc := tc @@ -569,6 +662,21 @@ func getExistingCluster() armcontainerservice.ManagedCluster { return mc } +func getExistingClusterWithAddonProfile() armcontainerservice.ManagedCluster { + mc := getSampleManagedCluster() + mc.Properties.ProvisioningState = ptr.To("Succeeded") + mc.ID = ptr.To("test-id") + mc.Properties.AddonProfiles = map[string]*armcontainerservice.ManagedClusterAddonProfile{ + "first-addon-profile": { + Enabled: ptr.To(true), + }, + "second-addon-profile": { + Enabled: ptr.To(true), + }, + } + return mc +} + func getExistingClusterWithUserAssignedIdentity() armcontainerservice.ManagedCluster { mc := getSampleManagedCluster() mc.Properties.ProvisioningState = ptr.To("Succeeded")