diff --git a/api/v1beta1/azurecluster_default.go b/api/v1beta1/azurecluster_default.go index a0898a51901..57cac7e74db 100644 --- a/api/v1beta1/azurecluster_default.go +++ b/api/v1beta1/azurecluster_default.go @@ -232,17 +232,31 @@ func (c *AzureCluster) setAPIServerLBDefaults() { if lb.Name == "" { lb.Name = generateInternalLBName(c.ObjectMeta.Name) } - if len(lb.FrontendIPs) == 0 { - lb.FrontendIPs = []FrontendIP{ - { - Name: generateFrontendIPConfigName(lb.Name), - FrontendIPClass: FrontendIPClass{ - PrivateIPAddress: DefaultInternalLBIPAddress, - }, - }, + } + + // 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" } + 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 8ab61074ad0..4d4b4b0fc07 100644 --- a/api/v1beta1/azurecluster_default_test.go +++ b/api/v1beta1/azurecluster_default_test.go @@ -106,8 +106,9 @@ func TestVnetDefaults(t *testing.T) { Subnets: Subnets{ { SubnetClassSpec: SubnetClassSpec{ - Role: SubnetControlPlane, - Name: "control-plane-subnet", + Role: SubnetControlPlane, + Name: "control-plane-subnet", + CIDRBlocks: []string{DefaultControlPlaneSubnetCIDR}, }, SecurityGroup: SecurityGroup{}, @@ -132,6 +133,12 @@ func TestVnetDefaults(t *testing.T) { DNSName: "myfqdn.azure.com", }, }, + { + Name: "ip-config-internal-ip", + FrontendIPClass: FrontendIPClass{ + PrivateIPAddress: DefaultInternalLBIPAddress, + }, + }, }, LoadBalancerClassSpec: LoadBalancerClassSpec{ SKU: SKUStandard, @@ -1237,6 +1244,12 @@ 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", @@ -1276,7 +1289,7 @@ func TestAPIServerLBDefaults(t *testing.T) { APIServerLB: LoadBalancerSpec{ FrontendIPs: []FrontendIP{ { - Name: "cluster-test-internal-lb-frontEnd", + Name: "cluster-test-internal-lb-frontEnd-internal-ip", FrontendIPClass: FrontendIPClass{ PrivateIPAddress: DefaultInternalLBIPAddress, }, @@ -1324,7 +1337,7 @@ func TestAPIServerLBDefaults(t *testing.T) { APIServerLB: LoadBalancerSpec{ FrontendIPs: []FrontendIP{ { - Name: "cluster-test-internal-lb-frontEnd", + Name: "cluster-test-internal-lb-frontEnd-internal-ip", FrontendIPClass: FrontendIPClass{ PrivateIPAddress: DefaultInternalLBIPAddress, }, diff --git a/api/v1beta1/azurecluster_validation.go b/api/v1beta1/azurecluster_validation.go index 51350662631..ec46bd699ca 100644 --- a/api/v1beta1/azurecluster_validation.go +++ b/api/v1beta1/azurecluster_validation.go @@ -400,33 +400,58 @@ 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.")) } - // There should only be one IP config. - if len(lb.FrontendIPs) != 1 || ptr.Deref[int32](lb.FrontendIPsCount, 1) != 1 { + 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 { allErrs = append(allErrs, field.Invalid(fldPath.Child("frontendIPConfigs"), lb.FrontendIPs, - "API Server Load balancer should have 1 Frontend IP")) + "API Server Load balancer should have 1 private IP")) } else { - // 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, + for i := range lb.FrontendIPs { + if lb.FrontendIPs[i].PrivateIPAddress != "" { + if err := validateInternalLBIPAddress(lb.FrontendIPs[i].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 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")) + 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.")) } } } diff --git a/api/v1beta1/azurecluster_validation_test.go b/api/v1beta1/azurecluster_validation_test.go index e4052d49045..72d7f37ba4e 100644 --- a/api/v1beta1/azurecluster_validation_test.go +++ b/api/v1beta1/azurecluster_validation_test.go @@ -891,6 +891,7 @@ func TestValidateAPIServerLB(t *testing.T) { { name: "too many IP configs", lb: LoadBalancerSpec{ + Name: "my-valid-lb", FrontendIPs: []FrontendIP{ { Name: "ip-1", @@ -899,6 +900,10 @@ func TestValidateAPIServerLB(t *testing.T) { Name: "ip-2", }, }, + LoadBalancerClassSpec: LoadBalancerClassSpec{ + Type: Public, + SKU: SKUStandard, + }, }, wantErr: true, expectedErr: field.Error{ @@ -916,26 +921,80 @@ func TestValidateAPIServerLB(t *testing.T) { }, }, { - name: "public LB with private IP", + name: "too many private IP configs", lb: LoadBalancerSpec{ + Name: "my-valid-lb", FrontendIPs: []FrontendIP{ { Name: "ip-1", FrontendIPClass: FrontendIPClass{ - PrivateIPAddress: "10.0.0.4", + PrivateIPAddress: "10.0.0.100", + }, + }, + { + Name: "ip-2", + FrontendIPClass: FrontendIPClass{ + PrivateIPAddress: "10.0.0.200", }, }, + { + Name: "ip-3", + }, }, LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Public, + SKU: SKUStandard, }, }, wantErr: true, expectedErr: field.Error{ - Type: "FieldValueForbidden", - Field: "apiServerLB.frontendIPConfigs[0].privateIP", - Detail: "Public Load Balancers cannot have a Private IP", + 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, + }, }, + wantErr: false, }, { name: "internal LB with public IP", @@ -956,7 +1015,7 @@ func TestValidateAPIServerLB(t *testing.T) { expectedErr: field.Error{ Type: "FieldValueForbidden", Field: "apiServerLB.frontendIPConfigs[0].publicIP", - Detail: "Internal Load Balancers cannot have a Public IP", + Detail: "API Server's associated internal load balancer cannot have a Public IP", }, }, { @@ -1483,12 +1542,18 @@ 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, + }, }, }, }, @@ -1502,12 +1567,18 @@ 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, + }, }, }, }, @@ -1521,6 +1592,9 @@ func createValidNetworkSpec() NetworkSpec { Vnet: VnetSpec{ ResourceGroup: "custom-vnet", Name: "my-vnet", + VnetClassSpec: VnetClassSpec{ + CIDRBlocks: []string{DefaultVnetCIDR}, + }, }, Subnets: createValidSubnets(), APIServerLB: createValidAPIServerLB(), @@ -1534,6 +1608,9 @@ func createValidSubnets() Subnets { SubnetClassSpec: SubnetClassSpec{ Role: "control-plane", Name: "control-plane-subnet", + CIDRBlocks: []string{ + DefaultControlPlaneSubnetCIDR, + }, }, }, { @@ -1566,6 +1643,12 @@ 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 c92b64885a3..3f12564fe7e 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -242,10 +242,52 @@ 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{ - &loadbalancers.LBSpec{ - // API Server LB - Name: s.APIServerLB().Name, + frontendLB, + } + + if s.APIServerLB().Type != infrav1.Internal { + internalLB := &loadbalancers.LBSpec{ + Name: s.APIServerLB().Name + "-internal", ResourceGroup: s.ResourceGroup(), SubscriptionID: s.SubscriptionID(), ClusterName: s.ClusterName(), @@ -254,36 +296,6 @@ 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, @@ -291,7 +303,11 @@ 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 217a0d645c2..2f773244ad5 100644 --- a/azure/scope/cluster_test.go +++ b/azure/scope/cluster_test.go @@ -2577,12 +2577,6 @@ 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", @@ -2601,6 +2595,14 @@ 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{ @@ -2683,9 +2685,9 @@ func TestClusterScope_LBSpecs(t *testing.T) { SubnetName: "cp-subnet", FrontendIPConfigs: []infrav1.FrontendIP{ { - Name: "api-server-lb-internal-frontEnd", + Name: "api-server-internal-lb-private-ip", FrontendIPClass: infrav1.FrontendIPClass{ - PrivateIPAddress: infrav1.DefaultInternalLBIPAddress, + PrivateIPAddress: "10.10.10.100", }, }, }, @@ -2792,6 +2794,14 @@ 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, + }, + }, + }, }, }, }, @@ -2813,6 +2823,14 @@ 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, + }, + }, + }, }, }, },