Skip to content

Commit d850054

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #4747 from willie-yao/use-existing-vnet-1.13
[release-1.13] use existing virtualNetwork from a different rg
2 parents b9b4ccb + 93648b6 commit d850054

File tree

9 files changed

+359
-6
lines changed

9 files changed

+359
-6
lines changed

‎api/v1beta1/azuremanagedcontrolplane_webhook.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,8 @@ func (m *AzureManagedControlPlane) Validate(cli client.Client) error {
314314

315315
allErrs = append(allErrs, validateAutoScalerProfile(m.Spec.AutoScalerProfile, field.NewPath("spec").Child("AutoScalerProfile"))...)
316316

317+
allErrs = append(allErrs, validateAMCPVirtualNetwork(m.Spec.VirtualNetwork, field.NewPath("spec").Child("VirtualNetwork"))...)
318+
317319
return allErrs.ToAggregate()
318320
}
319321

@@ -409,6 +411,26 @@ func validateLoadBalancerProfile(loadBalancerProfile *LoadBalancerProfile, fldPa
409411
return allErrs
410412
}
411413

414+
func validateAMCPVirtualNetwork(virtualNetwork ManagedControlPlaneVirtualNetwork, fldPath *field.Path) field.ErrorList {
415+
var allErrs field.ErrorList
416+
417+
// VirtualNetwork and the CIDR blocks get defaulted in the defaulting webhook, so we can assume they are always set.
418+
if !reflect.DeepEqual(virtualNetwork, ManagedControlPlaneVirtualNetwork{}) {
419+
_, parentNet, vnetErr := net.ParseCIDR(virtualNetwork.CIDRBlock)
420+
if vnetErr != nil {
421+
allErrs = append(allErrs, field.Invalid(fldPath.Child("CIDRBlock"), virtualNetwork.CIDRBlock, "pre-existing virtual networks CIDR block is invalid"))
422+
}
423+
subnetIP, _, subnetErr := net.ParseCIDR(virtualNetwork.Subnet.CIDRBlock)
424+
if subnetErr != nil {
425+
allErrs = append(allErrs, field.Invalid(fldPath.Child("Subnet", "CIDRBlock"), virtualNetwork.CIDRBlock, "pre-existing subnets CIDR block is invalid"))
426+
}
427+
if vnetErr == nil && subnetErr == nil && !parentNet.Contains(subnetIP) {
428+
allErrs = append(allErrs, field.Invalid(fldPath.Child("CIDRBlock"), virtualNetwork.CIDRBlock, "pre-existing virtual networks CIDR block should contain the subnet CIDR block"))
429+
}
430+
}
431+
return allErrs
432+
}
433+
412434
// validateAPIServerAccessProfile validates an APIServerAccessProfile.
413435
func (m *AzureManagedControlPlane) validateAPIServerAccessProfile(_ client.Client) field.ErrorList {
414436
if m.Spec.APIServerAccessProfile != nil {

‎api/v1beta1/azuremanagedcontrolplane_webhook_test.go‎

Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2712,3 +2712,222 @@ func getAMCPMetaData() metav1.ObjectMeta {
27122712
Namespace: "default",
27132713
}
27142714
}
2715+
2716+
func TestValidateAMCPVirtualNetwork(t *testing.T) {
2717+
tests := []struct {
2718+
name string
2719+
amcp *AzureManagedControlPlane
2720+
wantErr string
2721+
}{
2722+
{
2723+
name: "Testing valid VirtualNetwork in same resource group",
2724+
amcp: &AzureManagedControlPlane{
2725+
ObjectMeta: metav1.ObjectMeta{
2726+
Name: "fooName",
2727+
Labels: map[string]string{
2728+
clusterv1.ClusterNameLabel: "fooCluster",
2729+
},
2730+
},
2731+
Spec: AzureManagedControlPlaneSpec{
2732+
ResourceGroupName: "rg1",
2733+
AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{
2734+
VirtualNetwork: ManagedControlPlaneVirtualNetwork{
2735+
ResourceGroup: "rg1",
2736+
ManagedControlPlaneVirtualNetworkClassSpec: ManagedControlPlaneVirtualNetworkClassSpec{
2737+
Name: "vnet1",
2738+
CIDRBlock: defaultAKSVnetCIDR,
2739+
Subnet: ManagedControlPlaneSubnet{
2740+
Name: "subnet1",
2741+
CIDRBlock: defaultAKSNodeSubnetCIDR,
2742+
},
2743+
},
2744+
},
2745+
},
2746+
},
2747+
},
2748+
wantErr: "",
2749+
},
2750+
{
2751+
name: "Testing valid VirtualNetwork in different resource group",
2752+
amcp: &AzureManagedControlPlane{
2753+
ObjectMeta: metav1.ObjectMeta{
2754+
Name: "fooName",
2755+
Labels: map[string]string{
2756+
clusterv1.ClusterNameLabel: "fooCluster",
2757+
},
2758+
},
2759+
Spec: AzureManagedControlPlaneSpec{
2760+
ResourceGroupName: "rg1",
2761+
AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{
2762+
VirtualNetwork: ManagedControlPlaneVirtualNetwork{
2763+
ResourceGroup: "rg2",
2764+
ManagedControlPlaneVirtualNetworkClassSpec: ManagedControlPlaneVirtualNetworkClassSpec{
2765+
Name: "vnet1",
2766+
CIDRBlock: defaultAKSVnetCIDR,
2767+
Subnet: ManagedControlPlaneSubnet{
2768+
Name: "subnet1",
2769+
CIDRBlock: defaultAKSNodeSubnetCIDR,
2770+
},
2771+
},
2772+
},
2773+
},
2774+
},
2775+
},
2776+
wantErr: "",
2777+
},
2778+
{
2779+
name: "Testing invalid VirtualNetwork in different resource group: invalid subnet CIDR",
2780+
amcp: &AzureManagedControlPlane{
2781+
ObjectMeta: metav1.ObjectMeta{
2782+
Name: "fooName",
2783+
Labels: map[string]string{
2784+
clusterv1.ClusterNameLabel: "fooCluster",
2785+
},
2786+
},
2787+
Spec: AzureManagedControlPlaneSpec{
2788+
ResourceGroupName: "rg1",
2789+
AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{
2790+
VirtualNetwork: ManagedControlPlaneVirtualNetwork{
2791+
ResourceGroup: "rg2",
2792+
ManagedControlPlaneVirtualNetworkClassSpec: ManagedControlPlaneVirtualNetworkClassSpec{
2793+
Name: "vnet1",
2794+
CIDRBlock: "10.1.0.0/16",
2795+
Subnet: ManagedControlPlaneSubnet{
2796+
Name: "subnet1",
2797+
CIDRBlock: "10.0.0.0/24",
2798+
},
2799+
},
2800+
},
2801+
},
2802+
},
2803+
},
2804+
wantErr: "pre-existing virtual networks CIDR block should contain the subnet CIDR block",
2805+
},
2806+
{
2807+
name: "Testing invalid VirtualNetwork in different resource group: no subnet CIDR",
2808+
amcp: &AzureManagedControlPlane{
2809+
ObjectMeta: metav1.ObjectMeta{
2810+
Name: "fooName",
2811+
Labels: map[string]string{
2812+
clusterv1.ClusterNameLabel: "fooCluster",
2813+
},
2814+
},
2815+
Spec: AzureManagedControlPlaneSpec{
2816+
ResourceGroupName: "rg1",
2817+
AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{
2818+
VirtualNetwork: ManagedControlPlaneVirtualNetwork{
2819+
ResourceGroup: "rg2",
2820+
ManagedControlPlaneVirtualNetworkClassSpec: ManagedControlPlaneVirtualNetworkClassSpec{
2821+
Name: "vnet1",
2822+
CIDRBlock: "10.1.0.0/16",
2823+
Subnet: ManagedControlPlaneSubnet{
2824+
Name: "subnet1",
2825+
},
2826+
},
2827+
},
2828+
},
2829+
},
2830+
},
2831+
wantErr: "pre-existing virtual networks CIDR block should contain the subnet CIDR block",
2832+
},
2833+
{
2834+
name: "Testing invalid VirtualNetwork in different resource group: no VNet CIDR",
2835+
amcp: &AzureManagedControlPlane{
2836+
ObjectMeta: metav1.ObjectMeta{
2837+
Name: "fooName",
2838+
Labels: map[string]string{
2839+
clusterv1.ClusterNameLabel: "fooCluster",
2840+
},
2841+
},
2842+
Spec: AzureManagedControlPlaneSpec{
2843+
ResourceGroupName: "rg1",
2844+
AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{
2845+
VirtualNetwork: ManagedControlPlaneVirtualNetwork{
2846+
ResourceGroup: "rg2",
2847+
ManagedControlPlaneVirtualNetworkClassSpec: ManagedControlPlaneVirtualNetworkClassSpec{
2848+
Name: "vnet1",
2849+
Subnet: ManagedControlPlaneSubnet{
2850+
Name: "subnet1",
2851+
CIDRBlock: "11.0.0.0/24",
2852+
},
2853+
},
2854+
},
2855+
},
2856+
},
2857+
},
2858+
wantErr: "pre-existing virtual networks CIDR block should contain the subnet CIDR block",
2859+
},
2860+
{
2861+
name: "Testing invalid VirtualNetwork in different resource group: invalid VNet CIDR",
2862+
amcp: &AzureManagedControlPlane{
2863+
ObjectMeta: metav1.ObjectMeta{
2864+
Name: "fooName",
2865+
Labels: map[string]string{
2866+
clusterv1.ClusterNameLabel: "fooCluster",
2867+
},
2868+
},
2869+
Spec: AzureManagedControlPlaneSpec{
2870+
ResourceGroupName: "rg1",
2871+
AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{
2872+
VirtualNetwork: ManagedControlPlaneVirtualNetwork{
2873+
ResourceGroup: "rg2",
2874+
ManagedControlPlaneVirtualNetworkClassSpec: ManagedControlPlaneVirtualNetworkClassSpec{
2875+
Name: "vnet1",
2876+
CIDRBlock: "invalid_vnet_CIDR",
2877+
Subnet: ManagedControlPlaneSubnet{
2878+
Name: "subnet1",
2879+
CIDRBlock: "11.0.0.0/24",
2880+
},
2881+
},
2882+
},
2883+
},
2884+
},
2885+
},
2886+
wantErr: "pre-existing virtual networks CIDR block is invalid",
2887+
},
2888+
{
2889+
name: "Testing invalid VirtualNetwork in different resource group: invalid Subnet CIDR",
2890+
amcp: &AzureManagedControlPlane{
2891+
ObjectMeta: metav1.ObjectMeta{
2892+
Name: "fooName",
2893+
Labels: map[string]string{
2894+
clusterv1.ClusterNameLabel: "fooCluster",
2895+
},
2896+
},
2897+
Spec: AzureManagedControlPlaneSpec{
2898+
ResourceGroupName: "rg1",
2899+
AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{
2900+
VirtualNetwork: ManagedControlPlaneVirtualNetwork{
2901+
ResourceGroup: "rg2",
2902+
ManagedControlPlaneVirtualNetworkClassSpec: ManagedControlPlaneVirtualNetworkClassSpec{
2903+
Name: "vnet1",
2904+
Subnet: ManagedControlPlaneSubnet{
2905+
Name: "subnet1",
2906+
CIDRBlock: "invalid_subnet_CIDR",
2907+
},
2908+
},
2909+
},
2910+
},
2911+
},
2912+
},
2913+
wantErr: "pre-existing subnets CIDR block is invalid",
2914+
},
2915+
}
2916+
2917+
for _, tc := range tests {
2918+
t.Run(tc.name, func(t *testing.T) {
2919+
g := NewWithT(t)
2920+
mcpw := &azureManagedControlPlaneWebhook{}
2921+
err := mcpw.Default(context.Background(), tc.amcp)
2922+
g.Expect(err).NotTo(HaveOccurred())
2923+
2924+
errs := validateAMCPVirtualNetwork(tc.amcp.Spec.VirtualNetwork, field.NewPath("spec", "VirtualNetwork"))
2925+
if tc.wantErr != "" {
2926+
g.Expect(errs).ToNot(BeEmpty())
2927+
g.Expect(errs[0].Detail).To(Equal(tc.wantErr))
2928+
} else {
2929+
g.Expect(err).NotTo(HaveOccurred())
2930+
}
2931+
})
2932+
}
2933+
}

‎api/v1beta1/azuremanagedcontrolplanetemplate_webhook.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,8 @@ func (mcp *AzureManagedControlPlaneTemplate) validateManagedControlPlaneTemplate
203203

204204
allErrs = append(allErrs, validateAutoScalerProfile(mcp.Spec.Template.Spec.AutoScalerProfile, field.NewPath("spec").Child("template").Child("spec").Child("AutoScalerProfile"))...)
205205

206+
allErrs = append(allErrs, validateAMCPVirtualNetwork(mcp.Spec.Template.Spec.VirtualNetwork, field.NewPath("spec").Child("template").Child("spec").Child("VirtualNetwork"))...)
207+
206208
return allErrs.ToAggregate()
207209
}
208210

‎azure/defaults.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ package azure
1919
import (
2020
"fmt"
2121
"net/http"
22+
"regexp"
23+
"strings"
2224

2325
"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
2426
"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
@@ -412,3 +414,15 @@ func (p CustomPutPatchHeaderPolicy) Do(req *policy.Request) (*http.Response, err
412414

413415
return req.Next()
414416
}
417+
418+
// GetNormalizedKubernetesName returns a normalized name for a Kubernetes resource.
419+
func GetNormalizedKubernetesName(name string) string {
420+
// Remove non-alphanumeric characters, convert to lowercase, and replace underscores with hyphens
421+
name = strings.ToLower(name)
422+
re := regexp.MustCompile(`[^a-z0-9\-]+`)
423+
name = re.ReplaceAllString(name, "-")
424+
425+
// Remove leading and trailing hyphens
426+
name = strings.Trim(name, "-")
427+
return name
428+
}

‎azure/defaults_test.go‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,3 +277,79 @@ func TestGetBootstrappingVMExtension(t *testing.T) {
277277
})
278278
}
279279
}
280+
281+
func TestNormalizeAzureName(t *testing.T) {
282+
testCases := []struct {
283+
name string
284+
input string
285+
expected string
286+
}{
287+
{
288+
name: "should return lower case",
289+
input: "Test",
290+
expected: "test",
291+
},
292+
{
293+
name: "should return lower case with spaces replaced by hyphens",
294+
input: "Test Name",
295+
expected: "test-name",
296+
},
297+
{
298+
name: "should return lower case with spaces replaced by hyphens and non-alphanumeric characters removed",
299+
input: "Test Name 1",
300+
expected: "test-name-1",
301+
},
302+
{
303+
name: "should return lower case with spaces replaced by hyphens and non-alphanumeric characters removed",
304+
input: "Test-Name-1-",
305+
expected: "test-name-1",
306+
},
307+
{
308+
name: "should return lower case with spaces replaced by hyphens and non-alphanumeric characters removed",
309+
input: "Test-Name-1-@",
310+
expected: "test-name-1",
311+
},
312+
{
313+
name: "should return lower case with spaces replaced by hyphens and non-alphanumeric characters removed",
314+
input: "Test-Name-1-@-",
315+
expected: "test-name-1",
316+
},
317+
{
318+
name: "should return lower case with spaces replaced by hyphens and non-alphanumeric characters removed",
319+
input: "Test-Name-1-@-@",
320+
expected: "test-name-1",
321+
},
322+
{
323+
name: "should return lower case with underscores replaced by hyphens and non-alphanumeric characters removed",
324+
input: "Test_Name_1-@-@",
325+
expected: "test-name-1",
326+
},
327+
{
328+
name: "should return lower case with underscores replaced by hyphens and non-alphanumeric characters removed",
329+
input: "0_Test_Name_1-@-@",
330+
expected: "0-test-name-1",
331+
},
332+
{
333+
name: "should return lower case with underscores replaced by hyphens and non-alphanumeric characters removed",
334+
input: "_Test_Name_1-@-@",
335+
expected: "test-name-1",
336+
},
337+
{
338+
name: "should return lower case with name without hyphens",
339+
input: "_Test_Name_1---",
340+
expected: "test-name-1",
341+
},
342+
{
343+
name: "should not change the input since input is valid k8s name",
344+
input: "test-name-1",
345+
expected: "test-name-1",
346+
},
347+
}
348+
for _, tc := range testCases {
349+
t.Run(tc.name, func(t *testing.T) {
350+
g := NewWithT(t)
351+
normalizedNamed := GetNormalizedKubernetesName(tc.input)
352+
g.Expect(normalizedNamed).To(Equal(tc.expected))
353+
})
354+
}
355+
}

‎azure/scope/cluster.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -424,16 +424,18 @@ func (s *ClusterScope) GroupSpecs() []azure.ASOResourceSpecGetter[*asoresourcesv
424424
specs := []azure.ASOResourceSpecGetter[*asoresourcesv1.ResourceGroup]{
425425
&groups.GroupSpec{
426426
Name: s.ResourceGroup(),
427+
AzureName: s.ResourceGroup(),
427428
Namespace: s.Namespace(),
428429
Location: s.Location(),
429430
ClusterName: s.ClusterName(),
430431
AdditionalTags: s.AdditionalTags(),
431432
Owner: owner,
432433
},
433434
}
434-
if s.Vnet().ResourceGroup != s.ResourceGroup() {
435+
if s.Vnet().ResourceGroup != "" && s.Vnet().ResourceGroup != s.ResourceGroup() {
435436
specs = append(specs, &groups.GroupSpec{
436-
Name: s.Vnet().ResourceGroup,
437+
Name: azure.GetNormalizedKubernetesName(s.Vnet().ResourceGroup),
438+
AzureName: s.Vnet().ResourceGroup,
437439
Namespace: s.Namespace(),
438440
Location: s.Location(),
439441
ClusterName: s.ClusterName(),

0 commit comments

Comments
 (0)