Skip to content

Commit eae50c4

Browse files
dee077dee077
committed
fix: validate API server LB private IP is unchanged on update
1 parent 47cc48d commit eae50c4

2 files changed

Lines changed: 201 additions & 7 deletions

File tree

‎internal/webhooks/azurecluster_validation.go‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,18 @@ func validateSecurityRule(rule infrav1.SecurityRule, fldPath *field.Path) (allEr
392392
return allErrs
393393
}
394394

395+
func validateAPIServerLBPrivateIPUnchanged(lb, old *infrav1.LoadBalancerSpec, fldPath *field.Path) *field.Error {
396+
if lb == nil || old == nil || len(old.FrontendIPs) == 0 || len(lb.FrontendIPs) == 0 {
397+
return nil
398+
}
399+
if old.FrontendIPs[0].PrivateIPAddress != lb.FrontendIPs[0].PrivateIPAddress {
400+
return field.Forbidden(
401+
fldPath.Child("frontendIPConfigs").Index(0).Child("privateIP"),
402+
"API Server load balancer private IP should not be modified after AzureCluster creation.")
403+
}
404+
return nil
405+
}
406+
395407
func validateAPIServerLB(lb *infrav1.LoadBalancerSpec, old *infrav1.LoadBalancerSpec, cidrs []string, fldPath *field.Path) field.ErrorList {
396408
var allErrs field.ErrorList
397409

@@ -434,12 +446,13 @@ func validateAPIServerLB(lb *infrav1.LoadBalancerSpec, old *infrav1.LoadBalancer
434446
if err := validateInternalLBIPAddress(privateIP, cidrs, fldPath.Child("frontendIPConfigs").Index(0).Child("privateIP")); err != nil {
435447
allErrs = append(allErrs, err)
436448
}
437-
} else {
438-
// API Server LB should not have a Private IP if APIServerILB feature is disabled.
439-
if privateIPCount > 0 {
440-
allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPConfigs").Index(0).Child("privateIP"),
441-
"Public Load Balancers cannot have a Private IP"))
449+
if err := validateAPIServerLBPrivateIPUnchanged(lb, old, fldPath); err != nil {
450+
allErrs = append(allErrs, err)
442451
}
452+
} else if privateIPCount > 0 {
453+
// API Server LB should not have a Private IP if APIServerILB feature is disabled.
454+
allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPConfigs").Index(0).Child("privateIP"),
455+
"Public Load Balancers cannot have a Private IP"))
443456
}
444457
}
445458

@@ -458,8 +471,8 @@ func validateAPIServerLB(lb *infrav1.LoadBalancerSpec, old *infrav1.LoadBalancer
458471
allErrs = append(allErrs, err)
459472
}
460473

461-
if old != nil && len(old.FrontendIPs) != 0 && old.FrontendIPs[0].PrivateIPAddress != lb.FrontendIPs[0].PrivateIPAddress {
462-
allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer private IP should not be modified after AzureCluster creation."))
474+
if err := validateAPIServerLBPrivateIPUnchanged(lb, old, fldPath); err != nil {
475+
allErrs = append(allErrs, err)
463476
}
464477
}
465478
}

‎internal/webhooks/azurecluster_validation_test.go‎

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1286,6 +1286,187 @@ func TestValidateAPIServerLB(t *testing.T) {
12861286
Detail: "Internal LB IP address needs to be in control plane subnet range ([10.0.0.0/24 10.1.0.0/24])",
12871287
},
12881288
},
1289+
{
1290+
name: "public LB with APIServerILB enabled changing private IP after creation is forbidden",
1291+
featureGate: feature.APIServerILB,
1292+
old: &infrav1.LoadBalancerSpec{
1293+
FrontendIPs: []infrav1.FrontendIP{
1294+
{
1295+
Name: "ip-1",
1296+
FrontendIPClass: infrav1.FrontendIPClass{
1297+
PrivateIPAddress: "10.0.0.10",
1298+
},
1299+
},
1300+
{
1301+
Name: "ip-2",
1302+
PublicIP: &infrav1.PublicIPSpec{
1303+
Name: "my-valid-ip",
1304+
DNSName: "my-valid-ip",
1305+
},
1306+
},
1307+
},
1308+
LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{
1309+
Type: infrav1.Public,
1310+
SKU: infrav1.SKUStandard,
1311+
},
1312+
Name: "my-public-lb",
1313+
},
1314+
lb: &infrav1.LoadBalancerSpec{
1315+
FrontendIPs: []infrav1.FrontendIP{
1316+
{
1317+
Name: "ip-1",
1318+
FrontendIPClass: infrav1.FrontendIPClass{
1319+
PrivateIPAddress: "10.0.0.11",
1320+
},
1321+
},
1322+
{
1323+
Name: "ip-2",
1324+
PublicIP: &infrav1.PublicIPSpec{
1325+
Name: "my-valid-ip",
1326+
DNSName: "my-valid-ip",
1327+
},
1328+
},
1329+
},
1330+
LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{
1331+
Type: infrav1.Public,
1332+
SKU: infrav1.SKUStandard,
1333+
},
1334+
Name: "my-public-lb",
1335+
},
1336+
cpCIDRS: []string{"10.0.0.0/24"},
1337+
wantErr: true,
1338+
expectedErr: field.Error{
1339+
Type: "FieldValueForbidden",
1340+
Field: "apiServerLB.frontendIPConfigs[0].privateIP",
1341+
Detail: "API Server load balancer private IP should not be modified after AzureCluster creation.",
1342+
},
1343+
},
1344+
{
1345+
name: "internal LB: changing private IP after creation is forbidden",
1346+
old: &infrav1.LoadBalancerSpec{
1347+
FrontendIPs: []infrav1.FrontendIP{
1348+
{
1349+
Name: "ip-1",
1350+
FrontendIPClass: infrav1.FrontendIPClass{
1351+
PrivateIPAddress: "10.1.0.3",
1352+
},
1353+
},
1354+
},
1355+
LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{
1356+
Type: infrav1.Internal,
1357+
SKU: infrav1.SKUStandard,
1358+
},
1359+
Name: "my-private-lb",
1360+
},
1361+
lb: &infrav1.LoadBalancerSpec{
1362+
FrontendIPs: []infrav1.FrontendIP{
1363+
{
1364+
Name: "ip-1",
1365+
FrontendIPClass: infrav1.FrontendIPClass{
1366+
PrivateIPAddress: "10.1.0.4",
1367+
},
1368+
},
1369+
},
1370+
LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{
1371+
Type: infrav1.Internal,
1372+
SKU: infrav1.SKUStandard,
1373+
},
1374+
Name: "my-private-lb",
1375+
},
1376+
cpCIDRS: []string{"10.1.0.0/24"},
1377+
wantErr: true,
1378+
expectedErr: field.Error{
1379+
Type: "FieldValueForbidden",
1380+
Field: "apiServerLB.frontendIPConfigs[0].privateIP",
1381+
Detail: "API Server load balancer private IP should not be modified after AzureCluster creation.",
1382+
},
1383+
},
1384+
{
1385+
name: "public LB with APIServerILB enabled and unchanged private IP",
1386+
featureGate: feature.APIServerILB,
1387+
old: &infrav1.LoadBalancerSpec{
1388+
FrontendIPs: []infrav1.FrontendIP{
1389+
{
1390+
Name: "ip-1",
1391+
FrontendIPClass: infrav1.FrontendIPClass{
1392+
PrivateIPAddress: "10.0.0.10",
1393+
},
1394+
},
1395+
{
1396+
Name: "ip-2",
1397+
PublicIP: &infrav1.PublicIPSpec{
1398+
Name: "my-valid-ip",
1399+
DNSName: "my-valid-ip",
1400+
},
1401+
},
1402+
},
1403+
LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{
1404+
Type: infrav1.Public,
1405+
SKU: infrav1.SKUStandard,
1406+
},
1407+
Name: "my-public-lb",
1408+
},
1409+
lb: &infrav1.LoadBalancerSpec{
1410+
FrontendIPs: []infrav1.FrontendIP{
1411+
{
1412+
Name: "ip-1",
1413+
FrontendIPClass: infrav1.FrontendIPClass{
1414+
PrivateIPAddress: "10.0.0.10",
1415+
},
1416+
},
1417+
{
1418+
Name: "ip-2",
1419+
PublicIP: &infrav1.PublicIPSpec{
1420+
Name: "my-valid-ip",
1421+
DNSName: "my-valid-ip",
1422+
},
1423+
},
1424+
},
1425+
LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{
1426+
Type: infrav1.Public,
1427+
SKU: infrav1.SKUStandard,
1428+
},
1429+
Name: "my-public-lb",
1430+
},
1431+
cpCIDRS: []string{"10.0.0.0/24"},
1432+
wantErr: false,
1433+
},
1434+
{
1435+
name: "internal LB with APIServerILB enabled and unchanged private IP",
1436+
featureGate: feature.APIServerILB,
1437+
old: &infrav1.LoadBalancerSpec{
1438+
FrontendIPs: []infrav1.FrontendIP{
1439+
{
1440+
Name: "ip-1",
1441+
FrontendIPClass: infrav1.FrontendIPClass{
1442+
PrivateIPAddress: "10.1.0.3",
1443+
},
1444+
},
1445+
},
1446+
LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{
1447+
Type: infrav1.Internal,
1448+
SKU: infrav1.SKUStandard,
1449+
},
1450+
Name: "my-private-lb",
1451+
},
1452+
lb: &infrav1.LoadBalancerSpec{
1453+
FrontendIPs: []infrav1.FrontendIP{
1454+
{
1455+
Name: "ip-1",
1456+
FrontendIPClass: infrav1.FrontendIPClass{
1457+
PrivateIPAddress: "10.1.0.3",
1458+
},
1459+
},
1460+
},
1461+
LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{
1462+
Type: infrav1.Internal,
1463+
SKU: infrav1.SKUStandard,
1464+
},
1465+
Name: "my-private-lb",
1466+
},
1467+
cpCIDRS: []string{"10.1.0.0/24"},
1468+
wantErr: false,
1469+
},
12891470
}
12901471

12911472
for _, test := range testcases {

0 commit comments

Comments
 (0)