diff --git a/controller/aks-cluster-config-handler.go b/controller/aks-cluster-config-handler.go index 16d559f9..23529d45 100644 --- a/controller/aks-cluster-config-handler.go +++ b/controller/aks-cluster-config-handler.go @@ -698,6 +698,7 @@ func (h *Handler) buildUpstreamClusterState(ctx context.Context, credentials *ak // set API server access profile upstreamSpec.PrivateCluster = to.Ptr(false) + upstreamSpec.AuthorizedIPRanges = utils.ConvertToPointerOfSlice([]*string{}) if clusterState.Properties.APIServerAccessProfile != nil { if clusterState.Properties.APIServerAccessProfile.EnablePrivateCluster != nil { upstreamSpec.PrivateCluster = clusterState.Properties.APIServerAccessProfile.EnablePrivateCluster diff --git a/pkg/aks/update.go b/pkg/aks/update.go index cf8df7f5..50e2e8f7 100644 --- a/pkg/aks/update.go +++ b/pkg/aks/update.go @@ -48,7 +48,7 @@ func UpdateCluster(ctx context.Context, cred *Credentials, clusterClient service ctx, spec.ResourceGroup, spec.ClusterName, - updateCluster(*desiredCluster, actualCluster.ManagedCluster), + updateCluster(*desiredCluster, actualCluster.ManagedCluster, spec.Imported), nil, ) if err != nil { @@ -64,7 +64,7 @@ func UpdateCluster(ctx context.Context, cred *Credentials, clusterClient service return err } -func updateCluster(desiredCluster armcontainerservice.ManagedCluster, actualCluster armcontainerservice.ManagedCluster) armcontainerservice.ManagedCluster { +func updateCluster(desiredCluster armcontainerservice.ManagedCluster, actualCluster armcontainerservice.ManagedCluster, imported bool) armcontainerservice.ManagedCluster { if !validateUpdate(desiredCluster, actualCluster) { logrus.Warn("Not all cluster properties can be updated.") } @@ -108,14 +108,23 @@ func updateCluster(desiredCluster armcontainerservice.ManagedCluster, actualClus } } + // If the cluster is imported, we may not have the authorized IP ranges set on the spec. if desiredCluster.Properties.APIServerAccessProfile != nil && desiredCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges != nil { - for i := range desiredCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges { - ipRange := (desiredCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges)[i] - - if !hasAuthorizedIPRange(String(ipRange), actualCluster.Properties.APIServerAccessProfile) { - actualCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges = append(actualCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges, ipRange) + if !imported { + actualCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges = desiredCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges + } else { + for i := range desiredCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges { + ipRange := (desiredCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges)[i] + + if !hasAuthorizedIPRange(String(ipRange), actualCluster.Properties.APIServerAccessProfile) { + actualCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges = append(actualCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges, ipRange) + } } } + } else { + if !imported && actualCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges != nil { + actualCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges = []*string{} + } } // Linux profile diff --git a/pkg/aks/update_test.go b/pkg/aks/update_test.go index 8ce728fc..520a009b 100644 --- a/pkg/aks/update_test.go +++ b/pkg/aks/update_test.go @@ -72,7 +72,7 @@ var _ = Describe("updateCluster", func() { desiredCluster, err := createManagedCluster(ctx, cred, workplacesClientMock, clusterSpec, "phase") Expect(err).ToNot(HaveOccurred()) - updatedCluster := updateCluster(*desiredCluster, *actualCluster) + updatedCluster := updateCluster(*desiredCluster, *actualCluster, false) Expect(updatedCluster.Properties.KubernetesVersion).To(Equal(clusterSpec.KubernetesVersion)) Expect(updatedCluster.Properties.AddonProfiles).To(HaveKey("test-addon")) Expect(updatedCluster.Properties.AddonProfiles).To(HaveKey("httpApplicationRouting")) @@ -113,7 +113,7 @@ var _ = Describe("updateCluster", func() { desiredCluster, err := createManagedCluster(ctx, cred, workplacesClientMock, clusterSpec, "phase") Expect(err).ToNot(HaveOccurred()) - updatedCluster := updateCluster(*desiredCluster, *actualCluster) + updatedCluster := updateCluster(*desiredCluster, *actualCluster, false) Expect(updatedCluster.Properties.KubernetesVersion).To(BeNil()) }) @@ -126,36 +126,106 @@ var _ = Describe("updateCluster", func() { desiredCluster, err := createManagedCluster(ctx, cred, workplacesClientMock, clusterSpec, "phase") Expect(err).ToNot(HaveOccurred()) - updatedCluster := updateCluster(*desiredCluster, *actualCluster) + updatedCluster := updateCluster(*desiredCluster, *actualCluster, false) agentPoolProfiles := updatedCluster.Properties.AgentPoolProfiles Expect(agentPoolProfiles).To(HaveLen(1)) }) - It("shouldn't set authorized IP ranges if not specified in cluster spec", func() { + It("should clear authorized IP ranges for non imported clusters", func() { + actualCluster.Properties.APIServerAccessProfile = &armcontainerservice.ManagedClusterAPIServerAccessProfile{ + AuthorizedIPRanges: []*string{to.Ptr("test-ip-range"), to.Ptr("test-ip-range-2")}, + } + clusterSpec.AuthorizedIPRanges = nil desiredCluster, err := createManagedCluster(ctx, cred, workplacesClientMock, clusterSpec, "phase") Expect(err).ToNot(HaveOccurred()) - updatedCluster := updateCluster(*desiredCluster, *actualCluster) + updatedCluster := updateCluster(*desiredCluster, *actualCluster, false) Expect(updatedCluster.Properties.APIServerAccessProfile).ToNot(BeNil()) Expect(updatedCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges).ToNot(BeNil()) authorizedIPranges := updatedCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges Expect(authorizedIPranges).To(HaveLen(0)) + + clusterSpec.AuthorizedIPRanges = to.Ptr([]string{}) + desiredCluster, err = createManagedCluster(ctx, cred, workplacesClientMock, clusterSpec, "phase") + Expect(err).ToNot(HaveOccurred()) + + updatedCluster = updateCluster(*desiredCluster, *actualCluster, false) + Expect(updatedCluster.Properties.APIServerAccessProfile).ToNot(BeNil()) + Expect(updatedCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges).ToNot(BeNil()) + authorizedIPranges = updatedCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges + Expect(authorizedIPranges).To(HaveLen(0)) }) - It("shoudn't add new authorized IP range if it already exists ", func() { + It("should overwrite authorized IP range for non imported clusters", func() { + clusterSpec.AuthorizedIPRanges = to.Ptr([]string{"test-ip-range-new1", "test-ip-range-new2"}) + actualCluster.Properties.APIServerAccessProfile = &armcontainerservice.ManagedClusterAPIServerAccessProfile{ - AuthorizedIPRanges: []*string{to.Ptr("test-ip-range")}, + AuthorizedIPRanges: []*string{to.Ptr("test-ip-range"), to.Ptr("test-ip-range-2")}, } + desiredCluster, err := createManagedCluster(ctx, cred, workplacesClientMock, clusterSpec, "phase") Expect(err).ToNot(HaveOccurred()) - updatedCluster := updateCluster(*desiredCluster, *actualCluster) + updatedCluster := updateCluster(*desiredCluster, *actualCluster, false) Expect(updatedCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges).To(Equal(actualCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges)) Expect(updatedCluster.Properties.APIServerAccessProfile).ToNot(BeNil()) authorizedIPranges := updatedCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges - Expect(authorizedIPranges).To(HaveLen(1)) + Expect(authorizedIPranges).To(HaveLen(2)) + Expect(*authorizedIPranges[0]).To(Equal("test-ip-range-new1")) + Expect(*authorizedIPranges[1]).To(Equal("test-ip-range-new2")) + + clusterSpec.AuthorizedIPRanges = to.Ptr([]string{"test-ip-range-new1", "test-ip-range-new2"}) + }) + + It("should merge authorized IP range if it already exists for imported cluster", func() { + clusterSpec.AuthorizedIPRanges = to.Ptr([]string{"test-ip-range-new1", "test-ip-range-new2", "test-ip-range"}) + + actualCluster.Properties.APIServerAccessProfile = &armcontainerservice.ManagedClusterAPIServerAccessProfile{ + AuthorizedIPRanges: []*string{to.Ptr("test-ip-range"), to.Ptr("test-ip-range-2")}, + } + + desiredCluster, err := createManagedCluster(ctx, cred, workplacesClientMock, clusterSpec, "phase") + Expect(err).ToNot(HaveOccurred()) + + updatedCluster := updateCluster(*desiredCluster, *actualCluster, true) + Expect(updatedCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges).To(Equal(actualCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges)) + Expect(updatedCluster.Properties.APIServerAccessProfile).ToNot(BeNil()) + authorizedIPranges := updatedCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges + Expect(authorizedIPranges).To(HaveLen(4)) + Expect(*authorizedIPranges[0]).To(Equal("test-ip-range")) + Expect(*authorizedIPranges[1]).To(Equal("test-ip-range-2")) + Expect(*authorizedIPranges[2]).To(Equal("test-ip-range-new1")) + Expect(*authorizedIPranges[3]).To(Equal("test-ip-range-new2")) + }) + + It("shoudn't change authorized IP range if it is empty or nil to imported cluster ", func() { + actualCluster.Properties.APIServerAccessProfile = &armcontainerservice.ManagedClusterAPIServerAccessProfile{ + AuthorizedIPRanges: []*string{to.Ptr("test-ip-range"), to.Ptr("test-ip-range-2")}, + } + clusterSpec.AuthorizedIPRanges = nil + desiredCluster, err := createManagedCluster(ctx, cred, workplacesClientMock, clusterSpec, "phase") + Expect(err).ToNot(HaveOccurred()) + + updatedCluster := updateCluster(*desiredCluster, *actualCluster, true) + Expect(updatedCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges).To(Equal(actualCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges)) + Expect(updatedCluster.Properties.APIServerAccessProfile).ToNot(BeNil()) + authorizedIPranges := updatedCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges + Expect(authorizedIPranges).To(HaveLen(2)) + Expect(*authorizedIPranges[0]).To(Equal("test-ip-range")) + Expect(*authorizedIPranges[1]).To(Equal("test-ip-range-2")) + + clusterSpec.AuthorizedIPRanges = to.Ptr([]string{}) + desiredCluster, err = createManagedCluster(ctx, cred, workplacesClientMock, clusterSpec, "phase") + Expect(err).ToNot(HaveOccurred()) + + updatedCluster = updateCluster(*desiredCluster, *actualCluster, true) + Expect(updatedCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges).To(Equal(actualCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges)) + Expect(updatedCluster.Properties.APIServerAccessProfile).ToNot(BeNil()) + authorizedIPranges = updatedCluster.Properties.APIServerAccessProfile.AuthorizedIPRanges + Expect(authorizedIPranges).To(HaveLen(2)) Expect(*authorizedIPranges[0]).To(Equal("test-ip-range")) + Expect(*authorizedIPranges[1]).To(Equal("test-ip-range-2")) }) It("shouldn't update linux profile if it's not specified", func() { @@ -164,7 +234,7 @@ var _ = Describe("updateCluster", func() { desiredCluster, err := createManagedCluster(ctx, cred, workplacesClientMock, clusterSpec, "phase") Expect(err).ToNot(HaveOccurred()) - updatedCluster := updateCluster(*desiredCluster, *actualCluster) + updatedCluster := updateCluster(*desiredCluster, *actualCluster, false) Expect(updatedCluster.Properties.LinuxProfile).To(BeNil()) }) @@ -172,7 +242,7 @@ var _ = Describe("updateCluster", func() { desiredCluster, err := createManagedCluster(ctx, cred, workplacesClientMock, clusterSpec, "active") Expect(err).ToNot(HaveOccurred()) - updatedCluster := updateCluster(*desiredCluster, *actualCluster) + updatedCluster := updateCluster(*desiredCluster, *actualCluster, false) Expect(updatedCluster.Properties.ServicePrincipalProfile).To(BeNil()) }) @@ -181,7 +251,7 @@ var _ = Describe("updateCluster", func() { desiredCluster, err := createManagedCluster(ctx, cred, workplacesClientMock, clusterSpec, "phase") Expect(err).ToNot(HaveOccurred()) - updatedCluster := updateCluster(*desiredCluster, *actualCluster) + updatedCluster := updateCluster(*desiredCluster, *actualCluster, false) Expect(updatedCluster.Tags).To(HaveLen(0)) }) })