[release-1.10] fix AuthorizedIPRanges disable -> enable bug

azure/services/managedclusters/spec.go

@@ -21,6 +21,7 @@ import (
 	"encoding/base64"
 	"fmt"
 	"net"
+	"reflect"
 	"time"
 
 	"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2022-03-01/containerservice"
@@ -266,6 +267,9 @@ func buildAutoScalerProfile(autoScalerProfile *AutoScalerProfile) *containerserv
 }
 
 // Parameters returns the parameters for the managed clusters.
+// TODO: remove nolint after refactoring this function
+//
+//nolint:gocyclo // Function requires a lot of nil checks that raise complexity.
 func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing interface{}) (params interface{}, err error) {
 	ctx, log, done := tele.StartSpanWithLogger(ctx, "managedclusters.Service.Parameters")
 	defer done()
@@ -377,7 +381,7 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing interface{
 			EnablePrivateClusterPublicFQDN: s.APIServerAccessProfile.EnablePrivateClusterPublicFQDN,
 		}
 
-		if len(s.APIServerAccessProfile.AuthorizedIPRanges) > 0 {
+		if s.APIServerAccessProfile.AuthorizedIPRanges != nil {
 			managedCluster.APIServerAccessProfile.AuthorizedIPRanges = &s.APIServerAccessProfile.AuthorizedIPRanges
 		}
 	}
@@ -427,6 +431,16 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing interface{
 		// AgentPool changes are managed through AMMP.
 		managedCluster.AgentPoolProfiles = existingMC.AgentPoolProfiles
 
+		// if the AuthorizedIPRanges is nil in the user-updated spec, but not nil in the existing spec, then
+		// we need to set the AuthorizedIPRanges to empty array (&[]string{}) once so that the Azure API will
+		// update the existing authorized IP ranges to nil.
+		if !isAuthIPRangesNilOrEmpty(existingMC) && isAuthIPRangesNilOrEmpty(managedCluster) {
+			log.V(4).Info("managed cluster spec has nil AuthorizedIPRanges, updating existing authorized IP ranges to an empty list")
+			managedCluster.APIServerAccessProfile = &containerservice.ManagedClusterAPIServerAccessProfile{
+				AuthorizedIPRanges: &[]string{},
+			}
+		}
+
 		diff := computeDiffOfNormalizedClusters(managedCluster, existingMC)
 		if diff == "" {
 			log.V(4).Info("no changes found between user-updated spec and existing spec")
@@ -662,3 +676,11 @@ func getIdentity(identity *infrav1.Identity) (managedClusterIdentity *containers
 	}
 	return
 }
+
+// isAuthIPRangesNilOrEmpty returns true if the managed cluster's APIServerAccessProfile or AuthorizedIPRanges is nil or if AuthorizedIPRanges is empty.
+func isAuthIPRangesNilOrEmpty(managedCluster containerservice.ManagedCluster) bool {
+	if managedCluster.APIServerAccessProfile == nil || managedCluster.APIServerAccessProfile.AuthorizedIPRanges == nil || reflect.DeepEqual(managedCluster.APIServerAccessProfile.AuthorizedIPRanges, &[]string{}) {
+		return true
+	}
+	return false
+}

azure/services/managedclusters/spec_test.go

@@ -186,6 +186,67 @@ func TestParameters(t *testing.T) {
 				g.Expect(result).To(BeNil())
 			},
 		},
+		{
+			name:     "update authorized IP ranges with empty struct if spec does not have authorized IP ranges but existing cluster has authorized IP ranges",
+			existing: getExistingClusterWithAuthorizedIPRanges(),
+			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",
+			},
+			expect: func(g *WithT, result interface{}) {
+				g.Expect(result).To(BeAssignableToTypeOf(containerservice.ManagedCluster{}))
+				g.Expect(result.(containerservice.ManagedCluster).APIServerAccessProfile).To(Not(BeNil()))
+				g.Expect(result.(containerservice.ManagedCluster).APIServerAccessProfile.AuthorizedIPRanges).To(Equal(&[]string{}))
+			},
+		},
+		{
+			name:     "update authorized IP ranges with authorized IPs spec has authorized IP ranges but existing cluster does not have authorized IP ranges",
+			existing: getExistingCluster(),
+			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",
+				APIServerAccessProfile: &APIServerAccessProfile{
+					AuthorizedIPRanges: []string{"192.168.0.1/32, 192.168.0.2/32, 192.168.0.3/32"},
+				},
+			},
+			expect: func(g *WithT, result interface{}) {
+				g.Expect(result).To(BeAssignableToTypeOf(containerservice.ManagedCluster{}))
+				g.Expect(result.(containerservice.ManagedCluster).APIServerAccessProfile).To(Not(BeNil()))
+				g.Expect(result.(containerservice.ManagedCluster).APIServerAccessProfile.AuthorizedIPRanges).To(Equal(&[]string{"192.168.0.1/32, 192.168.0.2/32, 192.168.0.3/32"}))
+			},
+		},
+		{
+			name:     "no update needed when authorized IP ranges when both clusters have the same authorized IP ranges",
+			existing: getExistingClusterWithAuthorizedIPRanges(),
+			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",
+				APIServerAccessProfile: &APIServerAccessProfile{
+					AuthorizedIPRanges: []string{"192.168.0.1/32, 192.168.0.2/32, 192.168.0.3/32"},
+				},
+			},
+			expect: func(g *WithT, result interface{}) {
+				g.Expect(result).To(BeNil())
+			},
+		},
 	}
 	for _, tc := range testcases {
 		tc := tc
@@ -336,3 +397,11 @@ func getSampleManagedCluster() containerservice.ManagedCluster {
 		})),
 	}
 }
+
+func getExistingClusterWithAuthorizedIPRanges() containerservice.ManagedCluster {
+	mc := getExistingCluster()
+	mc.APIServerAccessProfile = &containerservice.ManagedClusterAPIServerAccessProfile{
+		AuthorizedIPRanges: &[]string{"192.168.0.1/32, 192.168.0.2/32, 192.168.0.3/32"},
+	}
+	return mc
+}