From 7c5df964a848b924e9bfff6f8d83d00de82c1ed9 Mon Sep 17 00:00:00 2001 From: Nawaz Hussain Khazielakha Date: Tue, 27 Feb 2024 14:01:10 -0800 Subject: [PATCH] check vnets for azure managed control plane and - check vnets for azure managed control plane templates - add tests to GroupSpecs - update webhooks and add webhook tests - add AzureName to aso groups - use AzureName when rgs are different - normalize Azure name to K8s name - propogate changes to ClusterScope --- .../azuremanagedcontrolplane_webhook.go | 23 ++ .../azuremanagedcontrolplane_webhook_test.go | 219 ++++++++++++++++++ ...zuremanagedcontrolplanetemplate_webhook.go | 2 + azure/defaults.go | 14 ++ azure/defaults_test.go | 76 ++++++ azure/scope/cluster.go | 6 +- azure/scope/cluster_test.go | 157 +++++++++++++ azure/scope/managedcontrolplane.go | 13 +- azure/scope/managedcontrolplane_test.go | 161 +++++++++++++ azure/services/groups/spec.go | 9 +- azure/services/groups/spec_test.go | 4 +- 11 files changed, 678 insertions(+), 6 deletions(-) diff --git a/api/v1beta1/azuremanagedcontrolplane_webhook.go b/api/v1beta1/azuremanagedcontrolplane_webhook.go index 2fdb0ce0c90..b020a752f09 100644 --- a/api/v1beta1/azuremanagedcontrolplane_webhook.go +++ b/api/v1beta1/azuremanagedcontrolplane_webhook.go @@ -288,6 +288,9 @@ func (m *AzureManagedControlPlane) Validate(cli client.Client) error { allErrs = append(allErrs, validateNetworkDataplane(m.Spec.NetworkDataplane, m.Spec.NetworkPolicy, m.Spec.NetworkPluginMode, field.NewPath("spec").Child("NetworkDataplane"))...) allErrs = append(allErrs, validateAPIServerAccessProfile(m.Spec.APIServerAccessProfile, field.NewPath("spec").Child("APIServerAccessProfile"))...) + + allErrs = append(allErrs, validateAMCPVirtualNetwork(m.Spec.VirtualNetwork, field.NewPath("spec").Child("VirtualNetwork"))...) + return allErrs.ToAggregate() } @@ -439,6 +442,26 @@ func validateLoadBalancerProfile(loadBalancerProfile *LoadBalancerProfile, fldPa return allErrs } +func validateAMCPVirtualNetwork(virtualNetwork ManagedControlPlaneVirtualNetwork, fldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + + // VirtualNetwork and the CIDR blocks get defaulted in the defaulting webhook, so we can assume they are always set. + if !reflect.DeepEqual(virtualNetwork, ManagedControlPlaneVirtualNetwork{}) { + _, parentNet, vnetErr := net.ParseCIDR(virtualNetwork.CIDRBlock) + if vnetErr != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("CIDRBlock"), virtualNetwork.CIDRBlock, "pre-existing virtual networks CIDR block is invalid")) + } + subnetIP, _, subnetErr := net.ParseCIDR(virtualNetwork.Subnet.CIDRBlock) + if subnetErr != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("Subnet", "CIDRBlock"), virtualNetwork.CIDRBlock, "pre-existing subnets CIDR block is invalid")) + } + if vnetErr == nil && subnetErr == nil && !parentNet.Contains(subnetIP) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("CIDRBlock"), virtualNetwork.CIDRBlock, "pre-existing virtual networks CIDR block should contain the subnet CIDR block")) + } + } + return allErrs +} + // validateAPIServerAccessProfile validates an APIServerAccessProfile. func validateAPIServerAccessProfile(apiServerAccessProfile *APIServerAccessProfile, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList diff --git a/api/v1beta1/azuremanagedcontrolplane_webhook_test.go b/api/v1beta1/azuremanagedcontrolplane_webhook_test.go index af735a75f52..82ea6ba1dbd 100644 --- a/api/v1beta1/azuremanagedcontrolplane_webhook_test.go +++ b/api/v1beta1/azuremanagedcontrolplane_webhook_test.go @@ -4003,3 +4003,222 @@ func TestValidateAPIServerAccessProfile(t *testing.T) { }) } } + +func TestValidateAMCPVirtualNetwork(t *testing.T) { + tests := []struct { + name string + amcp *AzureManagedControlPlane + wantErr string + }{ + { + name: "Testing valid VirtualNetwork in same resource group", + amcp: &AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fooName", + Labels: map[string]string{ + clusterv1.ClusterNameLabel: "fooCluster", + }, + }, + Spec: AzureManagedControlPlaneSpec{ + ResourceGroupName: "rg1", + AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{ + VirtualNetwork: ManagedControlPlaneVirtualNetwork{ + ResourceGroup: "rg1", + ManagedControlPlaneVirtualNetworkClassSpec: ManagedControlPlaneVirtualNetworkClassSpec{ + Name: "vnet1", + CIDRBlock: defaultAKSVnetCIDR, + Subnet: ManagedControlPlaneSubnet{ + Name: "subnet1", + CIDRBlock: defaultAKSNodeSubnetCIDR, + }, + }, + }, + }, + }, + }, + wantErr: "", + }, + { + name: "Testing valid VirtualNetwork in different resource group", + amcp: &AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fooName", + Labels: map[string]string{ + clusterv1.ClusterNameLabel: "fooCluster", + }, + }, + Spec: AzureManagedControlPlaneSpec{ + ResourceGroupName: "rg1", + AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{ + VirtualNetwork: ManagedControlPlaneVirtualNetwork{ + ResourceGroup: "rg2", + ManagedControlPlaneVirtualNetworkClassSpec: ManagedControlPlaneVirtualNetworkClassSpec{ + Name: "vnet1", + CIDRBlock: defaultAKSVnetCIDR, + Subnet: ManagedControlPlaneSubnet{ + Name: "subnet1", + CIDRBlock: defaultAKSNodeSubnetCIDR, + }, + }, + }, + }, + }, + }, + wantErr: "", + }, + { + name: "Testing invalid VirtualNetwork in different resource group: invalid subnet CIDR", + amcp: &AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fooName", + Labels: map[string]string{ + clusterv1.ClusterNameLabel: "fooCluster", + }, + }, + Spec: AzureManagedControlPlaneSpec{ + ResourceGroupName: "rg1", + AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{ + VirtualNetwork: ManagedControlPlaneVirtualNetwork{ + ResourceGroup: "rg2", + ManagedControlPlaneVirtualNetworkClassSpec: ManagedControlPlaneVirtualNetworkClassSpec{ + Name: "vnet1", + CIDRBlock: "10.1.0.0/16", + Subnet: ManagedControlPlaneSubnet{ + Name: "subnet1", + CIDRBlock: "10.0.0.0/24", + }, + }, + }, + }, + }, + }, + wantErr: "pre-existing virtual networks CIDR block should contain the subnet CIDR block", + }, + { + name: "Testing invalid VirtualNetwork in different resource group: no subnet CIDR", + amcp: &AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fooName", + Labels: map[string]string{ + clusterv1.ClusterNameLabel: "fooCluster", + }, + }, + Spec: AzureManagedControlPlaneSpec{ + ResourceGroupName: "rg1", + AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{ + VirtualNetwork: ManagedControlPlaneVirtualNetwork{ + ResourceGroup: "rg2", + ManagedControlPlaneVirtualNetworkClassSpec: ManagedControlPlaneVirtualNetworkClassSpec{ + Name: "vnet1", + CIDRBlock: "10.1.0.0/16", + Subnet: ManagedControlPlaneSubnet{ + Name: "subnet1", + }, + }, + }, + }, + }, + }, + wantErr: "pre-existing virtual networks CIDR block should contain the subnet CIDR block", + }, + { + name: "Testing invalid VirtualNetwork in different resource group: no VNet CIDR", + amcp: &AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fooName", + Labels: map[string]string{ + clusterv1.ClusterNameLabel: "fooCluster", + }, + }, + Spec: AzureManagedControlPlaneSpec{ + ResourceGroupName: "rg1", + AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{ + VirtualNetwork: ManagedControlPlaneVirtualNetwork{ + ResourceGroup: "rg2", + ManagedControlPlaneVirtualNetworkClassSpec: ManagedControlPlaneVirtualNetworkClassSpec{ + Name: "vnet1", + Subnet: ManagedControlPlaneSubnet{ + Name: "subnet1", + CIDRBlock: "11.0.0.0/24", + }, + }, + }, + }, + }, + }, + wantErr: "pre-existing virtual networks CIDR block should contain the subnet CIDR block", + }, + { + name: "Testing invalid VirtualNetwork in different resource group: invalid VNet CIDR", + amcp: &AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fooName", + Labels: map[string]string{ + clusterv1.ClusterNameLabel: "fooCluster", + }, + }, + Spec: AzureManagedControlPlaneSpec{ + ResourceGroupName: "rg1", + AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{ + VirtualNetwork: ManagedControlPlaneVirtualNetwork{ + ResourceGroup: "rg2", + ManagedControlPlaneVirtualNetworkClassSpec: ManagedControlPlaneVirtualNetworkClassSpec{ + Name: "vnet1", + CIDRBlock: "invalid_vnet_CIDR", + Subnet: ManagedControlPlaneSubnet{ + Name: "subnet1", + CIDRBlock: "11.0.0.0/24", + }, + }, + }, + }, + }, + }, + wantErr: "pre-existing virtual networks CIDR block is invalid", + }, + { + name: "Testing invalid VirtualNetwork in different resource group: invalid Subnet CIDR", + amcp: &AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fooName", + Labels: map[string]string{ + clusterv1.ClusterNameLabel: "fooCluster", + }, + }, + Spec: AzureManagedControlPlaneSpec{ + ResourceGroupName: "rg1", + AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{ + VirtualNetwork: ManagedControlPlaneVirtualNetwork{ + ResourceGroup: "rg2", + ManagedControlPlaneVirtualNetworkClassSpec: ManagedControlPlaneVirtualNetworkClassSpec{ + Name: "vnet1", + Subnet: ManagedControlPlaneSubnet{ + Name: "subnet1", + CIDRBlock: "invalid_subnet_CIDR", + }, + }, + }, + }, + }, + }, + wantErr: "pre-existing subnets CIDR block is invalid", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + mcpw := &azureManagedControlPlaneWebhook{} + err := mcpw.Default(context.Background(), tc.amcp) + g.Expect(err).NotTo(HaveOccurred()) + + errs := validateAMCPVirtualNetwork(tc.amcp.Spec.VirtualNetwork, field.NewPath("spec", "VirtualNetwork")) + if tc.wantErr != "" { + g.Expect(errs).ToNot(BeEmpty()) + g.Expect(errs[0].Detail).To(Equal(tc.wantErr)) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + }) + } +} diff --git a/api/v1beta1/azuremanagedcontrolplanetemplate_webhook.go b/api/v1beta1/azuremanagedcontrolplanetemplate_webhook.go index 3d1a6059424..e8b9c523cb3 100644 --- a/api/v1beta1/azuremanagedcontrolplanetemplate_webhook.go +++ b/api/v1beta1/azuremanagedcontrolplanetemplate_webhook.go @@ -228,6 +228,8 @@ func (mcp *AzureManagedControlPlaneTemplate) validateManagedControlPlaneTemplate allErrs = append(allErrs, validateAPIServerAccessProfile(mcp.Spec.Template.Spec.APIServerAccessProfile, field.NewPath("spec").Child("template").Child("spec").Child("APIServerAccessProfile"))...) + allErrs = append(allErrs, validateAMCPVirtualNetwork(mcp.Spec.Template.Spec.VirtualNetwork, field.NewPath("spec").Child("template").Child("spec").Child("VirtualNetwork"))...) + return allErrs.ToAggregate() } diff --git a/azure/defaults.go b/azure/defaults.go index 58e711379c7..9dcf227b149 100644 --- a/azure/defaults.go +++ b/azure/defaults.go @@ -19,6 +19,8 @@ package azure import ( "fmt" "net/http" + "regexp" + "strings" "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" "github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud" @@ -412,3 +414,15 @@ func (p CustomPutPatchHeaderPolicy) Do(req *policy.Request) (*http.Response, err return req.Next() } + +// GetNormalizedKubernetesName returns a normalized name for a Kubernetes resource. +func GetNormalizedKubernetesName(name string) string { + // Remove non-alphanumeric characters, convert to lowercase, and replace underscores with hyphens + name = strings.ToLower(name) + re := regexp.MustCompile(`[^a-z0-9\-]+`) + name = re.ReplaceAllString(name, "-") + + // Remove leading and trailing hyphens + name = strings.Trim(name, "-") + return name +} diff --git a/azure/defaults_test.go b/azure/defaults_test.go index 994a5bd43df..254e62a6a26 100644 --- a/azure/defaults_test.go +++ b/azure/defaults_test.go @@ -277,3 +277,79 @@ func TestGetBootstrappingVMExtension(t *testing.T) { }) } } + +func TestNormalizeAzureName(t *testing.T) { + testCases := []struct { + name string + input string + expected string + }{ + { + name: "should return lower case", + input: "Test", + expected: "test", + }, + { + name: "should return lower case with spaces replaced by hyphens", + input: "Test Name", + expected: "test-name", + }, + { + name: "should return lower case with spaces replaced by hyphens and non-alphanumeric characters removed", + input: "Test Name 1", + expected: "test-name-1", + }, + { + name: "should return lower case with spaces replaced by hyphens and non-alphanumeric characters removed", + input: "Test-Name-1-", + expected: "test-name-1", + }, + { + name: "should return lower case with spaces replaced by hyphens and non-alphanumeric characters removed", + input: "Test-Name-1-@", + expected: "test-name-1", + }, + { + name: "should return lower case with spaces replaced by hyphens and non-alphanumeric characters removed", + input: "Test-Name-1-@-", + expected: "test-name-1", + }, + { + name: "should return lower case with spaces replaced by hyphens and non-alphanumeric characters removed", + input: "Test-Name-1-@-@", + expected: "test-name-1", + }, + { + name: "should return lower case with underscores replaced by hyphens and non-alphanumeric characters removed", + input: "Test_Name_1-@-@", + expected: "test-name-1", + }, + { + name: "should return lower case with underscores replaced by hyphens and non-alphanumeric characters removed", + input: "0_Test_Name_1-@-@", + expected: "0-test-name-1", + }, + { + name: "should return lower case with underscores replaced by hyphens and non-alphanumeric characters removed", + input: "_Test_Name_1-@-@", + expected: "test-name-1", + }, + { + name: "should return lower case with name without hyphens", + input: "_Test_Name_1---", + expected: "test-name-1", + }, + { + name: "should not change the input since input is valid k8s name", + input: "test-name-1", + expected: "test-name-1", + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + normalizedNamed := GetNormalizedKubernetesName(tc.input) + g.Expect(normalizedNamed).To(Equal(tc.expected)) + }) + } +} diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index 6838d0e5a01..e9ebb814f01 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -425,14 +425,16 @@ func (s *ClusterScope) GroupSpecs() []azure.ASOResourceSpecGetter[*asoresourcesv specs := []azure.ASOResourceSpecGetter[*asoresourcesv1.ResourceGroup]{ &groups.GroupSpec{ Name: s.ResourceGroup(), + AzureName: s.ResourceGroup(), Location: s.Location(), ClusterName: s.ClusterName(), AdditionalTags: s.AdditionalTags(), }, } - if s.Vnet().ResourceGroup != s.ResourceGroup() { + if s.Vnet().ResourceGroup != "" && s.Vnet().ResourceGroup != s.ResourceGroup() { specs = append(specs, &groups.GroupSpec{ - Name: s.Vnet().ResourceGroup, + Name: azure.GetNormalizedKubernetesName(s.Vnet().ResourceGroup), + AzureName: s.Vnet().ResourceGroup, Location: s.Location(), ClusterName: s.ClusterName(), AdditionalTags: s.AdditionalTags(), diff --git a/azure/scope/cluster_test.go b/azure/scope/cluster_test.go index fa60e4262bf..8adf7235717 100644 --- a/azure/scope/cluster_test.go +++ b/azure/scope/cluster_test.go @@ -25,6 +25,7 @@ import ( asonetworkv1api20201101 "github.com/Azure/azure-service-operator/v2/api/network/v1api20201101" asonetworkv1api20220701 "github.com/Azure/azure-service-operator/v2/api/network/v1api20220701" + asoresourcesv1 "github.com/Azure/azure-service-operator/v2/api/resources/v1api20200601" "github.com/Azure/go-autorest/autorest/azure/auth" "github.com/google/go-cmp/cmp" . "github.com/onsi/gomega" @@ -35,6 +36,7 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" "sigs.k8s.io/cluster-api-provider-azure/azure/services/bastionhosts" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/groups" "sigs.k8s.io/cluster-api-provider-azure/azure/services/loadbalancers" "sigs.k8s.io/cluster-api-provider-azure/azure/services/natgateways" "sigs.k8s.io/cluster-api-provider-azure/azure/services/privateendpoints" @@ -3987,3 +3989,158 @@ func TestSetFailureDomain(t *testing.T) { }) } } + +func TestGroupSpecs(t *testing.T) { + cases := []struct { + name string + input ClusterScope + expected []azure.ASOResourceSpecGetter[*asoresourcesv1.ResourceGroup] + }{ + { + name: "virtualNetwork belongs to a different resource group", + input: ClusterScope{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + }, + }, + AzureCluster: &infrav1.AzureCluster{ + Spec: infrav1.AzureClusterSpec{ + ResourceGroup: "dummy-rg", + NetworkSpec: infrav1.NetworkSpec{ + Vnet: infrav1.VnetSpec{ + ResourceGroup: "different-rg", + }, + }, + }, + }, + }, + expected: []azure.ASOResourceSpecGetter[*asoresourcesv1.ResourceGroup]{ + &groups.GroupSpec{ + Name: "dummy-rg", + AzureName: "dummy-rg", + ClusterName: "cluster1", + Location: "", + AdditionalTags: make(infrav1.Tags, 0), + }, + &groups.GroupSpec{ + Name: "different-rg", + AzureName: "different-rg", + ClusterName: "cluster1", + Location: "", + AdditionalTags: make(infrav1.Tags, 0), + }, + }, + }, + { + name: "virtualNetwork belongs to a same resource group", + input: ClusterScope{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + }, + }, + AzureCluster: &infrav1.AzureCluster{ + Spec: infrav1.AzureClusterSpec{ + ResourceGroup: "dummy-rg", + NetworkSpec: infrav1.NetworkSpec{ + Vnet: infrav1.VnetSpec{ + ResourceGroup: "dummy-rg", + }, + }, + }, + }, + }, + expected: []azure.ASOResourceSpecGetter[*asoresourcesv1.ResourceGroup]{ + &groups.GroupSpec{ + Name: "dummy-rg", + AzureName: "dummy-rg", + ClusterName: "cluster1", + Location: "", + AdditionalTags: make(infrav1.Tags, 0), + }, + }, + }, + { + name: "virtualNetwork resource group not specified", + input: ClusterScope{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + AzureCluster: &infrav1.AzureCluster{ + Spec: infrav1.AzureClusterSpec{ + ResourceGroup: "dummy-rg", + NetworkSpec: infrav1.NetworkSpec{ + Vnet: infrav1.VnetSpec{ + Name: "vnet1", + }, + }, + }, + }, + }, + expected: []azure.ASOResourceSpecGetter[*asoresourcesv1.ResourceGroup]{ + &groups.GroupSpec{ + Name: "dummy-rg", + AzureName: "dummy-rg", + ClusterName: "cluster1", + Location: "", + AdditionalTags: make(infrav1.Tags, 0), + }, + }, + }, + { + name: "virtualNetwork belongs to different resource group with non-k8s name", + input: ClusterScope{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + AzureCluster: &infrav1.AzureCluster{ + Spec: infrav1.AzureClusterSpec{ + ResourceGroup: "dummy-rg", + NetworkSpec: infrav1.NetworkSpec{ + Vnet: infrav1.VnetSpec{ + ResourceGroup: "my_custom_rg", + Name: "vnet1", + }, + }, + }, + }, + }, + expected: []azure.ASOResourceSpecGetter[*asoresourcesv1.ResourceGroup]{ + &groups.GroupSpec{ + Name: "dummy-rg", + AzureName: "dummy-rg", + ClusterName: "cluster1", + Location: "", + AdditionalTags: make(infrav1.Tags, 0), + }, + &groups.GroupSpec{ + Name: "my-custom-rg", + AzureName: "my_custom_rg", + ClusterName: "cluster1", + Location: "", + AdditionalTags: make(infrav1.Tags, 0), + }, + }, + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + s := &ClusterScope{ + AzureCluster: c.input.AzureCluster, + Cluster: c.input.Cluster, + } + if got := s.GroupSpecs(); !reflect.DeepEqual(got, c.expected) { + t.Errorf("GroupSpecs() = %s, want %s", specArrayToString(got), specArrayToString(c.expected)) + } + }) + } +} diff --git a/azure/scope/managedcontrolplane.go b/azure/scope/managedcontrolplane.go index 6ae9192bf13..3f64e8c01c0 100644 --- a/azure/scope/managedcontrolplane.go +++ b/azure/scope/managedcontrolplane.go @@ -270,14 +270,25 @@ func (s *ManagedControlPlaneScope) Vnet() *infrav1.VnetSpec { // GroupSpecs returns the resource group spec. func (s *ManagedControlPlaneScope) GroupSpecs() []azure.ASOResourceSpecGetter[*asoresourcesv1.ResourceGroup] { - return []azure.ASOResourceSpecGetter[*asoresourcesv1.ResourceGroup]{ + specs := []azure.ASOResourceSpecGetter[*asoresourcesv1.ResourceGroup]{ &groups.GroupSpec{ Name: s.ResourceGroup(), + AzureName: s.ResourceGroup(), Location: s.Location(), ClusterName: s.ClusterName(), AdditionalTags: s.AdditionalTags(), }, } + if s.Vnet().ResourceGroup != "" && s.Vnet().ResourceGroup != s.ResourceGroup() { + specs = append(specs, &groups.GroupSpec{ + Name: azure.GetNormalizedKubernetesName(s.Vnet().ResourceGroup), + AzureName: s.Vnet().ResourceGroup, + Location: s.Location(), + ClusterName: s.ClusterName(), + AdditionalTags: s.AdditionalTags(), + }) + } + return specs } // VNetSpec returns the virtual network spec. diff --git a/azure/scope/managedcontrolplane_test.go b/azure/scope/managedcontrolplane_test.go index cfa6110a86e..6ed596527b4 100644 --- a/azure/scope/managedcontrolplane_test.go +++ b/azure/scope/managedcontrolplane_test.go @@ -24,6 +24,7 @@ import ( asocontainerservicev1 "github.com/Azure/azure-service-operator/v2/api/containerservice/v1api20231001" asokubernetesconfigurationv1 "github.com/Azure/azure-service-operator/v2/api/kubernetesconfiguration/v1api20230501" asonetworkv1 "github.com/Azure/azure-service-operator/v2/api/network/v1api20220701" + asoresourcesv1 "github.com/Azure/azure-service-operator/v2/api/resources/v1api20200601" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -33,6 +34,7 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/azure" "sigs.k8s.io/cluster-api-provider-azure/azure/services/agentpools" "sigs.k8s.io/cluster-api-provider-azure/azure/services/aksextensions" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/groups" "sigs.k8s.io/cluster-api-provider-azure/azure/services/managedclusters" "sigs.k8s.io/cluster-api-provider-azure/azure/services/privateendpoints" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -1619,3 +1621,162 @@ func TestManagedControlPlaneScope_AutoUpgradeProfile(t *testing.T) { }) } } + +func TestManagedControlPlaneScope_GroupSpecs(t *testing.T) { + cases := []struct { + name string + input ManagedControlPlaneScopeParams + expected []azure.ASOResourceSpecGetter[*asoresourcesv1.ResourceGroup] + }{ + { + name: "virtualNetwork belongs to a different resource group", + input: ManagedControlPlaneScopeParams{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + Spec: infrav1.AzureManagedControlPlaneSpec{ + ResourceGroupName: "dummy-rg", + AzureManagedControlPlaneClassSpec: infrav1.AzureManagedControlPlaneClassSpec{ + VirtualNetwork: infrav1.ManagedControlPlaneVirtualNetwork{ + ResourceGroup: "different-rg", + }, + }, + }, + }, + }, + expected: []azure.ASOResourceSpecGetter[*asoresourcesv1.ResourceGroup]{ + &groups.GroupSpec{ + Name: "dummy-rg", + AzureName: "dummy-rg", + ClusterName: "cluster1", + Location: "", + AdditionalTags: make(infrav1.Tags, 0), + }, + &groups.GroupSpec{ + Name: "different-rg", + AzureName: "different-rg", + ClusterName: "cluster1", + Location: "", + AdditionalTags: make(infrav1.Tags, 0), + }, + }, + }, + { + name: "virtualNetwork belongs to a same resource group", + input: ManagedControlPlaneScopeParams{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + Spec: infrav1.AzureManagedControlPlaneSpec{ + ResourceGroupName: "dummy-rg", + AzureManagedControlPlaneClassSpec: infrav1.AzureManagedControlPlaneClassSpec{ + VirtualNetwork: infrav1.ManagedControlPlaneVirtualNetwork{ + ResourceGroup: "dummy-rg", + }, + }, + }, + }, + }, + expected: []azure.ASOResourceSpecGetter[*asoresourcesv1.ResourceGroup]{ + &groups.GroupSpec{ + Name: "dummy-rg", + AzureName: "dummy-rg", + ClusterName: "cluster1", + Location: "", + AdditionalTags: make(infrav1.Tags, 0), + }, + }, + }, + { + name: "virtualNetwork resource group not specified", + input: ManagedControlPlaneScopeParams{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + Spec: infrav1.AzureManagedControlPlaneSpec{ + ResourceGroupName: "dummy-rg", + AzureManagedControlPlaneClassSpec: infrav1.AzureManagedControlPlaneClassSpec{ + VirtualNetwork: infrav1.ManagedControlPlaneVirtualNetwork{ + ManagedControlPlaneVirtualNetworkClassSpec: infrav1.ManagedControlPlaneVirtualNetworkClassSpec{ + Name: "vnet1", + }, + }, + }, + }, + }, + }, + expected: []azure.ASOResourceSpecGetter[*asoresourcesv1.ResourceGroup]{ + &groups.GroupSpec{ + Name: "dummy-rg", + AzureName: "dummy-rg", + ClusterName: "cluster1", + Location: "", + AdditionalTags: make(infrav1.Tags, 0), + }, + }, + }, + { + name: "virtualNetwork belongs to different resource group with non-k8s name", + input: ManagedControlPlaneScopeParams{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + Spec: infrav1.AzureManagedControlPlaneSpec{ + ResourceGroupName: "dummy-rg", + AzureManagedControlPlaneClassSpec: infrav1.AzureManagedControlPlaneClassSpec{ + VirtualNetwork: infrav1.ManagedControlPlaneVirtualNetwork{ + ResourceGroup: "my_custom_rg", + ManagedControlPlaneVirtualNetworkClassSpec: infrav1.ManagedControlPlaneVirtualNetworkClassSpec{ + Name: "vnet1", + }, + }, + }, + }, + }, + }, + expected: []azure.ASOResourceSpecGetter[*asoresourcesv1.ResourceGroup]{ + &groups.GroupSpec{ + Name: "dummy-rg", + AzureName: "dummy-rg", + ClusterName: "cluster1", + Location: "", + AdditionalTags: make(infrav1.Tags, 0), + }, + &groups.GroupSpec{ + Name: "my-custom-rg", + AzureName: "my_custom_rg", + ClusterName: "cluster1", + Location: "", + AdditionalTags: make(infrav1.Tags, 0), + }, + }, + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + s := &ManagedControlPlaneScope{ + ControlPlane: c.input.ControlPlane, + Cluster: c.input.Cluster, + } + if got := s.GroupSpecs(); !reflect.DeepEqual(got, c.expected) { + t.Errorf("GroupSpecs() = %s, want %s", specArrayToString(got), specArrayToString(c.expected)) + } + }) + } +} diff --git a/azure/services/groups/spec.go b/azure/services/groups/spec.go index 01263a0a3bf..9ac91d39d07 100644 --- a/azure/services/groups/spec.go +++ b/azure/services/groups/spec.go @@ -29,6 +29,7 @@ import ( // GroupSpec defines the specification for a Resource Group. type GroupSpec struct { Name string + AzureName string Location string ClusterName string AdditionalTags infrav1.Tags @@ -49,7 +50,7 @@ func (s *GroupSpec) Parameters(ctx context.Context, existing *asoresourcesv1.Res return existing, nil } - return &asoresourcesv1.ResourceGroup{ + resourceGroup := &asoresourcesv1.ResourceGroup{ Spec: asoresourcesv1.ResourceGroup_Spec{ Location: ptr.To(s.Location), Tags: infrav1.Build(infrav1.BuildParams{ @@ -60,7 +61,11 @@ func (s *GroupSpec) Parameters(ctx context.Context, existing *asoresourcesv1.Res Additional: s.AdditionalTags, }), }, - }, nil + } + if s.AzureName != "" { + resourceGroup.Spec.AzureName = s.AzureName + } + return resourceGroup, nil } // WasManaged implements azure.ASOResourceSpecGetter. diff --git a/azure/services/groups/spec_test.go b/azure/services/groups/spec_test.go index 2e660e44d65..9e286e30a47 100644 --- a/azure/services/groups/spec_test.go +++ b/azure/services/groups/spec_test.go @@ -38,6 +38,7 @@ func TestParameters(t *testing.T) { name: "no existing group", spec: &GroupSpec{ Name: "name", + AzureName: "azure-name", Location: "location", ClusterName: "cluster", AdditionalTags: infrav1.Tags{"some": "tags"}, @@ -45,7 +46,8 @@ func TestParameters(t *testing.T) { existing: nil, expected: &asoresourcesv1.ResourceGroup{ Spec: asoresourcesv1.ResourceGroup_Spec{ - Location: ptr.To("location"), + AzureName: "azure-name", + Location: ptr.To("location"), Tags: map[string]string{ "some": "tags", "sigs.k8s.io_cluster-api-provider-azure_cluster_cluster": "owned",