From d6c8abd29c587e8331349678c3bcdd23c53c739e Mon Sep 17 00:00:00 2001 From: Nawaz Hussain K Date: Fri, 28 Jul 2023 13:19:39 -0700 Subject: [PATCH] update Authorized IP Ranges update logic - update AzureSDK once with empty slice and then ignore changes --- azure/services/managedclusters/spec.go | 21 ++++++- azure/services/managedclusters/spec_test.go | 69 +++++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/azure/services/managedclusters/spec.go b/azure/services/managedclusters/spec.go index 5a40918392b..9fad266fc84 100644 --- a/azure/services/managedclusters/spec.go +++ b/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" @@ -387,7 +388,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 } } @@ -422,6 +423,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") @@ -584,3 +595,11 @@ func computeDiffOfNormalizedClusters(managedCluster containerservice.ManagedClus diff := cmp.Diff(clusterNormalized, existingMCClusterNormalized) return diff } + +// 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 +} diff --git a/azure/services/managedclusters/spec_test.go b/azure/services/managedclusters/spec_test.go index cf2ca4ec5c4..6b240805f8b 100644 --- a/azure/services/managedclusters/spec_test.go +++ b/azure/services/managedclusters/spec_test.go @@ -177,6 +177,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 @@ -277,3 +338,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 +}