From 934e1bdcd4e562782e0e0573d5602756a7703b23 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 | 24 ++++++- azure/services/managedclusters/spec_test.go | 69 +++++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/azure/services/managedclusters/spec.go b/azure/services/managedclusters/spec.go index dc560e7b282..648089a2c2a 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" @@ -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 +} diff --git a/azure/services/managedclusters/spec_test.go b/azure/services/managedclusters/spec_test.go index 01def7113af..30757ac6f64 100644 --- a/azure/services/managedclusters/spec_test.go +++ b/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 +}