Skip to content

Commit d3d5147

Browse files
nawazkhnawazkh
authored
Revert "make Private IP of the internal LB of the API Server configurable"
1 parent 8e06caf commit d3d5147

File tree

6 files changed

+79
-248
lines changed

6 files changed

+79
-248
lines changed

‎api/v1beta1/azurecluster_default.go‎

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -232,31 +232,17 @@ func (c *AzureCluster) setAPIServerLBDefaults() {
232232
if lb.Name == "" {
233233
lb.Name = generateInternalLBName(c.ObjectMeta.Name)
234234
}
235-
}
236-
237-
// create default private IP if not set
238-
privateIPFound := false
239-
for i := range lb.FrontendIPs {
240-
if lb.FrontendIPs[i].FrontendIPClass.PrivateIPAddress != "" {
241-
if lb.FrontendIPs[i].Name == "" {
242-
lb.FrontendIPs[i].Name = generateFrontendIPConfigName(lb.Name) + "-internal-ip"
235+
if len(lb.FrontendIPs) == 0 {
236+
lb.FrontendIPs = []FrontendIP{
237+
{
238+
Name: generateFrontendIPConfigName(lb.Name),
239+
FrontendIPClass: FrontendIPClass{
240+
PrivateIPAddress: DefaultInternalLBIPAddress,
241+
},
242+
},
243243
}
244-
privateIPFound = true
245-
break
246-
}
247-
}
248-
249-
// if no private IP found, create a default one
250-
if !privateIPFound {
251-
privateIP := FrontendIP{
252-
Name: generateFrontendIPConfigName(lb.Name) + "-internal-ip",
253-
FrontendIPClass: FrontendIPClass{
254-
PrivateIPAddress: DefaultInternalLBIPAddress,
255-
},
256244
}
257-
lb.FrontendIPs = append(lb.FrontendIPs, privateIP)
258245
}
259-
260246
c.SetAPIServerLBBackendPoolNameDefault()
261247
}
262248

‎api/v1beta1/azurecluster_default_test.go‎

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,8 @@ func TestVnetDefaults(t *testing.T) {
106106
Subnets: Subnets{
107107
{
108108
SubnetClassSpec: SubnetClassSpec{
109-
Role: SubnetControlPlane,
110-
Name: "control-plane-subnet",
111-
CIDRBlocks: []string{DefaultControlPlaneSubnetCIDR},
109+
Role: SubnetControlPlane,
110+
Name: "control-plane-subnet",
112111
},
113112

114113
SecurityGroup: SecurityGroup{},
@@ -133,12 +132,6 @@ func TestVnetDefaults(t *testing.T) {
133132
DNSName: "myfqdn.azure.com",
134133
},
135134
},
136-
{
137-
Name: "ip-config-internal-ip",
138-
FrontendIPClass: FrontendIPClass{
139-
PrivateIPAddress: DefaultInternalLBIPAddress,
140-
},
141-
},
142135
},
143136
LoadBalancerClassSpec: LoadBalancerClassSpec{
144137
SKU: SKUStandard,
@@ -1244,12 +1237,6 @@ func TestAPIServerLBDefaults(t *testing.T) {
12441237
DNSName: "",
12451238
},
12461239
},
1247-
{
1248-
Name: "cluster-test-public-lb-frontEnd-internal-ip",
1249-
FrontendIPClass: FrontendIPClass{
1250-
PrivateIPAddress: DefaultInternalLBIPAddress,
1251-
},
1252-
},
12531240
},
12541241
BackendPool: BackendPool{
12551242
Name: "cluster-test-public-lb-backendPool",
@@ -1289,7 +1276,7 @@ func TestAPIServerLBDefaults(t *testing.T) {
12891276
APIServerLB: LoadBalancerSpec{
12901277
FrontendIPs: []FrontendIP{
12911278
{
1292-
Name: "cluster-test-internal-lb-frontEnd-internal-ip",
1279+
Name: "cluster-test-internal-lb-frontEnd",
12931280
FrontendIPClass: FrontendIPClass{
12941281
PrivateIPAddress: DefaultInternalLBIPAddress,
12951282
},
@@ -1337,7 +1324,7 @@ func TestAPIServerLBDefaults(t *testing.T) {
13371324
APIServerLB: LoadBalancerSpec{
13381325
FrontendIPs: []FrontendIP{
13391326
{
1340-
Name: "cluster-test-internal-lb-frontEnd-internal-ip",
1327+
Name: "cluster-test-internal-lb-frontEnd",
13411328
FrontendIPClass: FrontendIPClass{
13421329
PrivateIPAddress: DefaultInternalLBIPAddress,
13431330
},

‎api/v1beta1/azurecluster_validation.go‎

Lines changed: 19 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -400,58 +400,33 @@ func validateAPIServerLB(lb LoadBalancerSpec, old LoadBalancerSpec, cidrs []stri
400400
allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer name should not be modified after AzureCluster creation."))
401401
}
402402

403-
publicIPCount := 0
404-
privateIPCount := 0
405-
newPrivateIP := ""
406-
for i := range lb.FrontendIPs {
407-
if lb.FrontendIPs[i].PublicIP != nil {
408-
publicIPCount++
409-
}
410-
if lb.FrontendIPs[i].PrivateIPAddress != "" {
411-
privateIPCount++
412-
newPrivateIP = lb.FrontendIPs[i].PrivateIPAddress
413-
}
414-
}
415-
416-
if lb.Type == Public {
417-
// public IP count should be 1 for public LB.
418-
if publicIPCount != 1 || ptr.Deref[int32](lb.FrontendIPsCount, 1) != 1 {
419-
allErrs = append(allErrs, field.Invalid(fldPath.Child("frontendIPConfigs"), lb.FrontendIPs,
420-
"API Server Load balancer should have 1 Frontend IP"))
421-
}
422-
}
423-
424-
// if Internal, IP config should not have a public IP.
425-
if lb.Type == Internal {
426-
if publicIPCount != 0 {
427-
allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPConfigs").Index(0).Child("publicIP"),
428-
"API Server's associated internal load balancer cannot have a Public IP"))
429-
}
430-
}
431-
432-
// private IP count should be 1 for public LB.
433-
if privateIPCount != 1 {
403+
// There should only be one IP config.
404+
if len(lb.FrontendIPs) != 1 || ptr.Deref[int32](lb.FrontendIPsCount, 1) != 1 {
434405
allErrs = append(allErrs, field.Invalid(fldPath.Child("frontendIPConfigs"), lb.FrontendIPs,
435-
"API Server Load balancer should have 1 private IP"))
406+
"API Server Load balancer should have 1 Frontend IP"))
436407
} else {
437-
for i := range lb.FrontendIPs {
438-
if lb.FrontendIPs[i].PrivateIPAddress != "" {
439-
if err := validateInternalLBIPAddress(lb.FrontendIPs[i].PrivateIPAddress, cidrs,
408+
// if Internal, IP config should not have a public IP.
409+
if lb.Type == Internal {
410+
if lb.FrontendIPs[0].PublicIP != nil {
411+
allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPConfigs").Index(0).Child("publicIP"),
412+
"Internal Load Balancers cannot have a Public IP"))
413+
}
414+
if lb.FrontendIPs[0].PrivateIPAddress != "" {
415+
if err := validateInternalLBIPAddress(lb.FrontendIPs[0].PrivateIPAddress, cidrs,
440416
fldPath.Child("frontendIPConfigs").Index(0).Child("privateIP")); err != nil {
441417
allErrs = append(allErrs, err)
442418
}
419+
if len(old.FrontendIPs) != 0 && old.FrontendIPs[0].PrivateIPAddress != lb.FrontendIPs[0].PrivateIPAddress {
420+
allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer private IP should not be modified after AzureCluster creation."))
421+
}
443422
}
444423
}
445424

446-
if len(old.FrontendIPs) != 0 {
447-
oldPrivateIP := ""
448-
for i := range old.FrontendIPs {
449-
if old.FrontendIPs[i].PrivateIPAddress != "" {
450-
oldPrivateIP = old.FrontendIPs[i].PrivateIPAddress
451-
}
452-
}
453-
if newPrivateIP != oldPrivateIP {
454-
allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer private IP should not be modified after AzureCluster creation."))
425+
// if Public, IP config should not have a private IP.
426+
if lb.Type == Public {
427+
if lb.FrontendIPs[0].PrivateIPAddress != "" {
428+
allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPConfigs").Index(0).Child("privateIP"),
429+
"Public Load Balancers cannot have a Private IP"))
455430
}
456431
}
457432
}

‎api/v1beta1/azurecluster_validation_test.go‎

Lines changed: 6 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -891,7 +891,6 @@ func TestValidateAPIServerLB(t *testing.T) {
891891
{
892892
name: "too many IP configs",
893893
lb: LoadBalancerSpec{
894-
Name: "my-valid-lb",
895894
FrontendIPs: []FrontendIP{
896895
{
897896
Name: "ip-1",
@@ -900,10 +899,6 @@ func TestValidateAPIServerLB(t *testing.T) {
900899
Name: "ip-2",
901900
},
902901
},
903-
LoadBalancerClassSpec: LoadBalancerClassSpec{
904-
Type: Public,
905-
SKU: SKUStandard,
906-
},
907902
},
908903
wantErr: true,
909904
expectedErr: field.Error{
@@ -921,80 +916,26 @@ func TestValidateAPIServerLB(t *testing.T) {
921916
},
922917
},
923918
{
924-
name: "too many private IP configs",
919+
name: "public LB with private IP",
925920
lb: LoadBalancerSpec{
926-
Name: "my-valid-lb",
927921
FrontendIPs: []FrontendIP{
928922
{
929923
Name: "ip-1",
930924
FrontendIPClass: FrontendIPClass{
931-
PrivateIPAddress: "10.0.0.100",
932-
},
933-
},
934-
{
935-
Name: "ip-2",
936-
FrontendIPClass: FrontendIPClass{
937-
PrivateIPAddress: "10.0.0.200",
925+
PrivateIPAddress: "10.0.0.4",
938926
},
939927
},
940-
{
941-
Name: "ip-3",
942-
},
943928
},
944929
LoadBalancerClassSpec: LoadBalancerClassSpec{
945930
Type: Public,
946-
SKU: SKUStandard,
947931
},
948932
},
949933
wantErr: true,
950934
expectedErr: field.Error{
951-
Type: "FieldValueInvalid",
952-
Field: "apiServerLB.frontendIPConfigs",
953-
BadValue: []FrontendIP{
954-
{
955-
Name: "ip-1",
956-
FrontendIPClass: FrontendIPClass{
957-
PrivateIPAddress: "10.0.0.100",
958-
},
959-
},
960-
{
961-
Name: "ip-2",
962-
FrontendIPClass: FrontendIPClass{
963-
PrivateIPAddress: "10.0.0.200",
964-
},
965-
},
966-
{
967-
Name: "ip-3",
968-
},
969-
},
970-
Detail: "API Server Load balancer should have 1 private IP",
971-
},
972-
},
973-
{
974-
name: "public LB with private IP",
975-
cpCIDRS: []string{"10.0.0.0/24"},
976-
lb: LoadBalancerSpec{
977-
Name: "my-valid-lb",
978-
FrontendIPs: []FrontendIP{
979-
{
980-
Name: "ip-1",
981-
PublicIP: &PublicIPSpec{
982-
Name: "my-valid-ip-name",
983-
},
984-
},
985-
{
986-
Name: "ip-1",
987-
FrontendIPClass: FrontendIPClass{
988-
PrivateIPAddress: "10.0.0.4",
989-
},
990-
},
991-
},
992-
LoadBalancerClassSpec: LoadBalancerClassSpec{
993-
Type: Public,
994-
SKU: SKUStandard,
995-
},
935+
Type: "FieldValueForbidden",
936+
Field: "apiServerLB.frontendIPConfigs[0].privateIP",
937+
Detail: "Public Load Balancers cannot have a Private IP",
996938
},
997-
wantErr: false,
998939
},
999940
{
1000941
name: "internal LB with public IP",
@@ -1015,7 +956,7 @@ func TestValidateAPIServerLB(t *testing.T) {
1015956
expectedErr: field.Error{
1016957
Type: "FieldValueForbidden",
1017958
Field: "apiServerLB.frontendIPConfigs[0].publicIP",
1018-
Detail: "API Server's associated internal load balancer cannot have a Public IP",
959+
Detail: "Internal Load Balancers cannot have a Public IP",
1019960
},
1020961
},
1021962
{
@@ -1542,18 +1483,12 @@ func createClusterNetworkSpec() NetworkSpec {
15421483
Vnet: VnetSpec{
15431484
ResourceGroup: "custom-vnet",
15441485
Name: "my-vnet",
1545-
VnetClassSpec: VnetClassSpec{
1546-
CIDRBlocks: []string{DefaultVnetCIDR},
1547-
},
15481486
},
15491487
Subnets: Subnets{
15501488
{
15511489
SubnetClassSpec: SubnetClassSpec{
15521490
Role: "cluster",
15531491
Name: "cluster-subnet",
1554-
CIDRBlocks: []string{
1555-
DefaultClusterSubnetCIDR,
1556-
},
15571492
},
15581493
},
15591494
},
@@ -1567,18 +1502,12 @@ func createValidNetworkSpecWithClusterSubnet() NetworkSpec {
15671502
Vnet: VnetSpec{
15681503
ResourceGroup: "custom-vnet",
15691504
Name: "my-vnet",
1570-
VnetClassSpec: VnetClassSpec{
1571-
CIDRBlocks: []string{DefaultVnetCIDR},
1572-
},
15731505
},
15741506
Subnets: Subnets{
15751507
{
15761508
SubnetClassSpec: SubnetClassSpec{
15771509
Role: "cluster",
15781510
Name: "cluster-subnet",
1579-
CIDRBlocks: []string{
1580-
DefaultClusterSubnetCIDR,
1581-
},
15821511
},
15831512
},
15841513
},
@@ -1592,9 +1521,6 @@ func createValidNetworkSpec() NetworkSpec {
15921521
Vnet: VnetSpec{
15931522
ResourceGroup: "custom-vnet",
15941523
Name: "my-vnet",
1595-
VnetClassSpec: VnetClassSpec{
1596-
CIDRBlocks: []string{DefaultVnetCIDR},
1597-
},
15981524
},
15991525
Subnets: createValidSubnets(),
16001526
APIServerLB: createValidAPIServerLB(),
@@ -1608,9 +1534,6 @@ func createValidSubnets() Subnets {
16081534
SubnetClassSpec: SubnetClassSpec{
16091535
Role: "control-plane",
16101536
Name: "control-plane-subnet",
1611-
CIDRBlocks: []string{
1612-
DefaultControlPlaneSubnetCIDR,
1613-
},
16141537
},
16151538
},
16161539
{
@@ -1643,12 +1566,6 @@ func createValidAPIServerLB() LoadBalancerSpec {
16431566
DNSName: "myfqdn.azure.com",
16441567
},
16451568
},
1646-
{
1647-
Name: "ip-config-internal-ip",
1648-
FrontendIPClass: FrontendIPClass{
1649-
PrivateIPAddress: DefaultInternalLBIPAddress,
1650-
},
1651-
},
16521569
},
16531570
LoadBalancerClassSpec: LoadBalancerClassSpec{
16541571
SKU: SKUStandard,

0 commit comments

Comments
 (0)