Skip to content

Commit

Permalink
Merge pull request #4606 from nawazkh/use_existing_vnet
Browse files Browse the repository at this point in the history
use existing virtualNetwork from a different rg
  • Loading branch information
k8s-ci-robot authored Mar 4, 2024
2 parents c5fe91c + 7c5df96 commit 2251753
Show file tree
Hide file tree
Showing 11 changed files with 678 additions and 6 deletions.
23 changes: 23 additions & 0 deletions api/v1beta1/azuremanagedcontrolplane_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down Expand Up @@ -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
Expand Down
219 changes: 219 additions & 0 deletions api/v1beta1/azuremanagedcontrolplane_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
})
}
}
2 changes: 2 additions & 0 deletions api/v1beta1/azuremanagedcontrolplanetemplate_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
14 changes: 14 additions & 0 deletions azure/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
76 changes: 76 additions & 0 deletions azure/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
}
}
6 changes: 4 additions & 2 deletions azure/scope/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
Loading

0 comments on commit 2251753

Please sign in to comment.