diff --git a/api/v1beta1/azurecluster_default.go b/api/v1beta1/azurecluster_default.go index 57cac7e74db..a0898a51901 100644 --- a/api/v1beta1/azurecluster_default.go +++ b/api/v1beta1/azurecluster_default.go @@ -232,31 +232,17 @@ func (c *AzureCluster) setAPIServerLBDefaults() { if lb.Name == "" { lb.Name = generateInternalLBName(c.ObjectMeta.Name) } - } - - // create default private IP if not set - privateIPFound := false - for i := range lb.FrontendIPs { - if lb.FrontendIPs[i].FrontendIPClass.PrivateIPAddress != "" { - if lb.FrontendIPs[i].Name == "" { - lb.FrontendIPs[i].Name = generateFrontendIPConfigName(lb.Name) + "-internal-ip" + if len(lb.FrontendIPs) == 0 { + lb.FrontendIPs = []FrontendIP{ + { + Name: generateFrontendIPConfigName(lb.Name), + FrontendIPClass: FrontendIPClass{ + PrivateIPAddress: DefaultInternalLBIPAddress, + }, + }, } - privateIPFound = true - break - } - } - - // if no private IP found, create a default one - if !privateIPFound { - privateIP := FrontendIP{ - Name: generateFrontendIPConfigName(lb.Name) + "-internal-ip", - FrontendIPClass: FrontendIPClass{ - PrivateIPAddress: DefaultInternalLBIPAddress, - }, } - lb.FrontendIPs = append(lb.FrontendIPs, privateIP) } - c.SetAPIServerLBBackendPoolNameDefault() } diff --git a/api/v1beta1/azurecluster_default_test.go b/api/v1beta1/azurecluster_default_test.go index 4d4b4b0fc07..8ab61074ad0 100644 --- a/api/v1beta1/azurecluster_default_test.go +++ b/api/v1beta1/azurecluster_default_test.go @@ -106,9 +106,8 @@ func TestVnetDefaults(t *testing.T) { Subnets: Subnets{ { SubnetClassSpec: SubnetClassSpec{ - Role: SubnetControlPlane, - Name: "control-plane-subnet", - CIDRBlocks: []string{DefaultControlPlaneSubnetCIDR}, + Role: SubnetControlPlane, + Name: "control-plane-subnet", }, SecurityGroup: SecurityGroup{}, @@ -133,12 +132,6 @@ func TestVnetDefaults(t *testing.T) { DNSName: "myfqdn.azure.com", }, }, - { - Name: "ip-config-internal-ip", - FrontendIPClass: FrontendIPClass{ - PrivateIPAddress: DefaultInternalLBIPAddress, - }, - }, }, LoadBalancerClassSpec: LoadBalancerClassSpec{ SKU: SKUStandard, @@ -1244,12 +1237,6 @@ func TestAPIServerLBDefaults(t *testing.T) { DNSName: "", }, }, - { - Name: "cluster-test-public-lb-frontEnd-internal-ip", - FrontendIPClass: FrontendIPClass{ - PrivateIPAddress: DefaultInternalLBIPAddress, - }, - }, }, BackendPool: BackendPool{ Name: "cluster-test-public-lb-backendPool", @@ -1289,7 +1276,7 @@ func TestAPIServerLBDefaults(t *testing.T) { APIServerLB: LoadBalancerSpec{ FrontendIPs: []FrontendIP{ { - Name: "cluster-test-internal-lb-frontEnd-internal-ip", + Name: "cluster-test-internal-lb-frontEnd", FrontendIPClass: FrontendIPClass{ PrivateIPAddress: DefaultInternalLBIPAddress, }, @@ -1337,7 +1324,7 @@ func TestAPIServerLBDefaults(t *testing.T) { APIServerLB: LoadBalancerSpec{ FrontendIPs: []FrontendIP{ { - Name: "cluster-test-internal-lb-frontEnd-internal-ip", + Name: "cluster-test-internal-lb-frontEnd", FrontendIPClass: FrontendIPClass{ PrivateIPAddress: DefaultInternalLBIPAddress, }, diff --git a/api/v1beta1/azurecluster_validation.go b/api/v1beta1/azurecluster_validation.go index ec46bd699ca..51350662631 100644 --- a/api/v1beta1/azurecluster_validation.go +++ b/api/v1beta1/azurecluster_validation.go @@ -400,58 +400,33 @@ func validateAPIServerLB(lb LoadBalancerSpec, old LoadBalancerSpec, cidrs []stri allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer name should not be modified after AzureCluster creation.")) } - publicIPCount := 0 - privateIPCount := 0 - newPrivateIP := "" - for i := range lb.FrontendIPs { - if lb.FrontendIPs[i].PublicIP != nil { - publicIPCount++ - } - if lb.FrontendIPs[i].PrivateIPAddress != "" { - privateIPCount++ - newPrivateIP = lb.FrontendIPs[i].PrivateIPAddress - } - } - - if lb.Type == Public { - // public IP count should be 1 for public LB. - if publicIPCount != 1 || ptr.Deref[int32](lb.FrontendIPsCount, 1) != 1 { - allErrs = append(allErrs, field.Invalid(fldPath.Child("frontendIPConfigs"), lb.FrontendIPs, - "API Server Load balancer should have 1 Frontend IP")) - } - } - - // if Internal, IP config should not have a public IP. - if lb.Type == Internal { - if publicIPCount != 0 { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPConfigs").Index(0).Child("publicIP"), - "API Server's associated internal load balancer cannot have a Public IP")) - } - } - - // private IP count should be 1 for public LB. - if privateIPCount != 1 { + // There should only be one IP config. + if len(lb.FrontendIPs) != 1 || ptr.Deref[int32](lb.FrontendIPsCount, 1) != 1 { allErrs = append(allErrs, field.Invalid(fldPath.Child("frontendIPConfigs"), lb.FrontendIPs, - "API Server Load balancer should have 1 private IP")) + "API Server Load balancer should have 1 Frontend IP")) } else { - for i := range lb.FrontendIPs { - if lb.FrontendIPs[i].PrivateIPAddress != "" { - if err := validateInternalLBIPAddress(lb.FrontendIPs[i].PrivateIPAddress, cidrs, + // if Internal, IP config should not have a public IP. + if lb.Type == Internal { + if lb.FrontendIPs[0].PublicIP != nil { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPConfigs").Index(0).Child("publicIP"), + "Internal Load Balancers cannot have a Public IP")) + } + if lb.FrontendIPs[0].PrivateIPAddress != "" { + if err := validateInternalLBIPAddress(lb.FrontendIPs[0].PrivateIPAddress, cidrs, fldPath.Child("frontendIPConfigs").Index(0).Child("privateIP")); err != nil { allErrs = append(allErrs, err) } + if len(old.FrontendIPs) != 0 && old.FrontendIPs[0].PrivateIPAddress != lb.FrontendIPs[0].PrivateIPAddress { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer private IP should not be modified after AzureCluster creation.")) + } } } - if len(old.FrontendIPs) != 0 { - oldPrivateIP := "" - for i := range old.FrontendIPs { - if old.FrontendIPs[i].PrivateIPAddress != "" { - oldPrivateIP = old.FrontendIPs[i].PrivateIPAddress - } - } - if newPrivateIP != oldPrivateIP { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer private IP should not be modified after AzureCluster creation.")) + // if Public, IP config should not have a private IP. + if lb.Type == Public { + if lb.FrontendIPs[0].PrivateIPAddress != "" { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPConfigs").Index(0).Child("privateIP"), + "Public Load Balancers cannot have a Private IP")) } } } diff --git a/api/v1beta1/azurecluster_validation_test.go b/api/v1beta1/azurecluster_validation_test.go index 72d7f37ba4e..e4052d49045 100644 --- a/api/v1beta1/azurecluster_validation_test.go +++ b/api/v1beta1/azurecluster_validation_test.go @@ -891,7 +891,6 @@ func TestValidateAPIServerLB(t *testing.T) { { name: "too many IP configs", lb: LoadBalancerSpec{ - Name: "my-valid-lb", FrontendIPs: []FrontendIP{ { Name: "ip-1", @@ -900,10 +899,6 @@ func TestValidateAPIServerLB(t *testing.T) { Name: "ip-2", }, }, - LoadBalancerClassSpec: LoadBalancerClassSpec{ - Type: Public, - SKU: SKUStandard, - }, }, wantErr: true, expectedErr: field.Error{ @@ -921,80 +916,26 @@ func TestValidateAPIServerLB(t *testing.T) { }, }, { - name: "too many private IP configs", + name: "public LB with private IP", lb: LoadBalancerSpec{ - Name: "my-valid-lb", FrontendIPs: []FrontendIP{ { Name: "ip-1", FrontendIPClass: FrontendIPClass{ - PrivateIPAddress: "10.0.0.100", - }, - }, - { - Name: "ip-2", - FrontendIPClass: FrontendIPClass{ - PrivateIPAddress: "10.0.0.200", + PrivateIPAddress: "10.0.0.4", }, }, - { - Name: "ip-3", - }, }, LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Public, - SKU: SKUStandard, }, }, wantErr: true, expectedErr: field.Error{ - Type: "FieldValueInvalid", - Field: "apiServerLB.frontendIPConfigs", - BadValue: []FrontendIP{ - { - Name: "ip-1", - FrontendIPClass: FrontendIPClass{ - PrivateIPAddress: "10.0.0.100", - }, - }, - { - Name: "ip-2", - FrontendIPClass: FrontendIPClass{ - PrivateIPAddress: "10.0.0.200", - }, - }, - { - Name: "ip-3", - }, - }, - Detail: "API Server Load balancer should have 1 private IP", - }, - }, - { - name: "public LB with private IP", - cpCIDRS: []string{"10.0.0.0/24"}, - lb: LoadBalancerSpec{ - Name: "my-valid-lb", - FrontendIPs: []FrontendIP{ - { - Name: "ip-1", - PublicIP: &PublicIPSpec{ - Name: "my-valid-ip-name", - }, - }, - { - Name: "ip-1", - FrontendIPClass: FrontendIPClass{ - PrivateIPAddress: "10.0.0.4", - }, - }, - }, - LoadBalancerClassSpec: LoadBalancerClassSpec{ - Type: Public, - SKU: SKUStandard, - }, + Type: "FieldValueForbidden", + Field: "apiServerLB.frontendIPConfigs[0].privateIP", + Detail: "Public Load Balancers cannot have a Private IP", }, - wantErr: false, }, { name: "internal LB with public IP", @@ -1015,7 +956,7 @@ func TestValidateAPIServerLB(t *testing.T) { expectedErr: field.Error{ Type: "FieldValueForbidden", Field: "apiServerLB.frontendIPConfigs[0].publicIP", - Detail: "API Server's associated internal load balancer cannot have a Public IP", + Detail: "Internal Load Balancers cannot have a Public IP", }, }, { @@ -1542,18 +1483,12 @@ func createClusterNetworkSpec() NetworkSpec { Vnet: VnetSpec{ ResourceGroup: "custom-vnet", Name: "my-vnet", - VnetClassSpec: VnetClassSpec{ - CIDRBlocks: []string{DefaultVnetCIDR}, - }, }, Subnets: Subnets{ { SubnetClassSpec: SubnetClassSpec{ Role: "cluster", Name: "cluster-subnet", - CIDRBlocks: []string{ - DefaultClusterSubnetCIDR, - }, }, }, }, @@ -1567,18 +1502,12 @@ func createValidNetworkSpecWithClusterSubnet() NetworkSpec { Vnet: VnetSpec{ ResourceGroup: "custom-vnet", Name: "my-vnet", - VnetClassSpec: VnetClassSpec{ - CIDRBlocks: []string{DefaultVnetCIDR}, - }, }, Subnets: Subnets{ { SubnetClassSpec: SubnetClassSpec{ Role: "cluster", Name: "cluster-subnet", - CIDRBlocks: []string{ - DefaultClusterSubnetCIDR, - }, }, }, }, @@ -1592,9 +1521,6 @@ func createValidNetworkSpec() NetworkSpec { Vnet: VnetSpec{ ResourceGroup: "custom-vnet", Name: "my-vnet", - VnetClassSpec: VnetClassSpec{ - CIDRBlocks: []string{DefaultVnetCIDR}, - }, }, Subnets: createValidSubnets(), APIServerLB: createValidAPIServerLB(), @@ -1608,9 +1534,6 @@ func createValidSubnets() Subnets { SubnetClassSpec: SubnetClassSpec{ Role: "control-plane", Name: "control-plane-subnet", - CIDRBlocks: []string{ - DefaultControlPlaneSubnetCIDR, - }, }, }, { @@ -1643,12 +1566,6 @@ func createValidAPIServerLB() LoadBalancerSpec { DNSName: "myfqdn.azure.com", }, }, - { - Name: "ip-config-internal-ip", - FrontendIPClass: FrontendIPClass{ - PrivateIPAddress: DefaultInternalLBIPAddress, - }, - }, }, LoadBalancerClassSpec: LoadBalancerClassSpec{ SKU: SKUStandard, diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index 725d993f98d..5f80bc0238a 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -243,52 +243,10 @@ func (s *ClusterScope) PublicIPSpecs() []azure.ResourceSpecGetter { // LBSpecs returns the load balancer specs. func (s *ClusterScope) LBSpecs() []azure.ResourceSpecGetter { - // construct the frontend LB - frontendLB := &loadbalancers.LBSpec{ - Name: s.APIServerLB().Name, - ResourceGroup: s.ResourceGroup(), - SubscriptionID: s.SubscriptionID(), - ClusterName: s.ClusterName(), - Location: s.Location(), - ExtendedLocation: s.ExtendedLocation(), - VNetName: s.Vnet().Name, - VNetResourceGroup: s.Vnet().ResourceGroup, - SubnetName: s.ControlPlaneSubnet().Name, - APIServerPort: s.APIServerPort(), - Type: s.APIServerLB().Type, - SKU: s.APIServerLB().SKU, - Role: infrav1.APIServerRole, - BackendPoolName: s.APIServerLB().BackendPool.Name, - IdleTimeoutInMinutes: s.APIServerLB().IdleTimeoutInMinutes, - AdditionalTags: s.AdditionalTags(), - } - - // get the internal LB IP and the public LB IPs - apiServerInternalLBIP := infrav1.FrontendIP{} - apiServerFrontendLBIP := make([]infrav1.FrontendIP, 0) - if s.APIServerLB().FrontendIPs != nil { - for _, frontendIP := range s.APIServerLB().FrontendIPs { - // save the public IPs for the frontend LB - // or if the LB is of the type internal, save the only IP allowed for the frontend LB - if frontendIP.PublicIP != nil || frontendLB.Type == infrav1.Internal { - apiServerFrontendLBIP = append(apiServerFrontendLBIP, frontendIP) - } - - if frontendIP.PrivateIPAddress != "" { - apiServerInternalLBIP = frontendIP - } - } - } - - // set the frontend IPs for the frontend LB - frontendLB.FrontendIPConfigs = apiServerFrontendLBIP specs := []azure.ResourceSpecGetter{ - frontendLB, - } - - if s.APIServerLB().Type != infrav1.Internal { - internalLB := &loadbalancers.LBSpec{ - Name: s.APIServerLB().Name + "-internal", + &loadbalancers.LBSpec{ + // API Server LB + Name: s.APIServerLB().Name, ResourceGroup: s.ResourceGroup(), SubscriptionID: s.SubscriptionID(), ClusterName: s.ClusterName(), @@ -297,6 +255,36 @@ func (s *ClusterScope) LBSpecs() []azure.ResourceSpecGetter { VNetName: s.Vnet().Name, VNetResourceGroup: s.Vnet().ResourceGroup, SubnetName: s.ControlPlaneSubnet().Name, + FrontendIPConfigs: s.APIServerLB().FrontendIPs, + APIServerPort: s.APIServerPort(), + Type: s.APIServerLB().Type, + SKU: s.APIServerLB().SKU, + Role: infrav1.APIServerRole, + BackendPoolName: s.APIServerLB().BackendPool.Name, + IdleTimeoutInMinutes: s.APIServerLB().IdleTimeoutInMinutes, + AdditionalTags: s.AdditionalTags(), + }, + } + + if s.APIServerLB().Type != infrav1.Internal { + specs = append(specs, &loadbalancers.LBSpec{ + Name: s.APIServerLB().Name + "-internal", + ResourceGroup: s.ResourceGroup(), + SubscriptionID: s.SubscriptionID(), + ClusterName: s.ClusterName(), + Location: s.Location(), + ExtendedLocation: s.ExtendedLocation(), + VNetName: s.Vnet().Name, + VNetResourceGroup: s.Vnet().ResourceGroup, + SubnetName: s.ControlPlaneSubnet().Name, + FrontendIPConfigs: []infrav1.FrontendIP{ + { + Name: s.APIServerLB().Name + "-internal-frontEnd", // TODO: improve this name. + FrontendIPClass: infrav1.FrontendIPClass{ + PrivateIPAddress: infrav1.DefaultInternalLBIPAddress, + }, + }, + }, APIServerPort: s.APIServerPort(), Type: infrav1.Internal, SKU: s.APIServerLB().SKU, @@ -304,11 +292,7 @@ func (s *ClusterScope) LBSpecs() []azure.ResourceSpecGetter { BackendPoolName: s.APIServerLB().BackendPool.Name + "-internal", IdleTimeoutInMinutes: s.APIServerLB().IdleTimeoutInMinutes, AdditionalTags: s.AdditionalTags(), - } - - // set the internal IP for the internal LB - internalLB.FrontendIPConfigs = []infrav1.FrontendIP{apiServerInternalLBIP} - specs = append(specs, internalLB) + }) } // Node outbound LB diff --git a/azure/scope/cluster_test.go b/azure/scope/cluster_test.go index 4bd147e81d0..7d2b3aab723 100644 --- a/azure/scope/cluster_test.go +++ b/azure/scope/cluster_test.go @@ -2578,6 +2578,12 @@ func TestClusterScope_LBSpecs(t *testing.T) { Role: infrav1.SubnetControlPlane, }, }, + { + SubnetClassSpec: infrav1.SubnetClassSpec{ + Name: "node-subnet", + Role: infrav1.SubnetNode, + }, + }, }, APIServerLB: infrav1.LoadBalancerSpec{ Name: "api-server-lb", @@ -2596,14 +2602,6 @@ func TestClusterScope_LBSpecs(t *testing.T) { Name: "api-server-lb-frontend-ip", }, }, - { - // The private IP for the internal LB is set by the Defaulting Webhook - // since no defaulters are called here, we have to update the test - Name: "api-server-internal-lb-private-ip", - FrontendIPClass: infrav1.FrontendIPClass{ - PrivateIPAddress: "10.10.10.100", - }, - }, }, }, ControlPlaneOutboundLB: &infrav1.LoadBalancerSpec{ @@ -2686,9 +2684,9 @@ func TestClusterScope_LBSpecs(t *testing.T) { SubnetName: "cp-subnet", FrontendIPConfigs: []infrav1.FrontendIP{ { - Name: "api-server-internal-lb-private-ip", + Name: "api-server-lb-internal-frontEnd", FrontendIPClass: infrav1.FrontendIPClass{ - PrivateIPAddress: "10.10.10.100", + PrivateIPAddress: infrav1.DefaultInternalLBIPAddress, }, }, }, @@ -2795,14 +2793,6 @@ func TestClusterScope_LBSpecs(t *testing.T) { IdleTimeoutInMinutes: ptr.To[int32](30), SKU: infrav1.SKUStandard, }, - FrontendIPs: []infrav1.FrontendIP{ - { - Name: "api-server-lb-internal-ip", - FrontendIPClass: infrav1.FrontendIPClass{ - PrivateIPAddress: infrav1.DefaultInternalLBIPAddress, - }, - }, - }, }, }, }, @@ -2824,14 +2814,6 @@ func TestClusterScope_LBSpecs(t *testing.T) { BackendPoolName: "api-server-lb-backend-pool", IdleTimeoutInMinutes: ptr.To[int32](30), AdditionalTags: infrav1.Tags{}, - FrontendIPConfigs: []infrav1.FrontendIP{ - { - Name: "api-server-lb-internal-ip", - FrontendIPClass: infrav1.FrontendIPClass{ - PrivateIPAddress: infrav1.DefaultInternalLBIPAddress, - }, - }, - }, }, }, },