From d3d51471da6ee181d5b83036f02b04b39a9b061c Mon Sep 17 00:00:00 2001
From: Nawaz K <k.nawaz.h@gmail.com>
Date: Thu, 21 Nov 2024 10:05:43 -0800
Subject: [PATCH] Revert "make Private IP of the internal LB of the API Server
configurable"
---
api/v1beta1/azurecluster_default.go | 30 ++-----
api/v1beta1/azurecluster_default_test.go | 21 +----
api/v1beta1/azurecluster_validation.go | 63 +++++---------
api/v1beta1/azurecluster_validation_test.go | 95 ++-------------------
azure/scope/cluster.go | 84 ++++++++----------
azure/scope/cluster_test.go | 34 ++------
6 files changed, 79 insertions(+), 248 deletions(-)
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,
- },
- },
- },
},
},
},