Skip to content

Commit 1f52056

Browse files
k8s-infra-cherrypick-robotnawazkh
k8s-infra-cherrypick-robot
and
nawazkh
authored
[release-1.18] fix AzCluster_default and AzCluster_validation webhooks (#5616)
* remove unused defaulters * fix code * fix tests --------- Co-authored-by: Nawaz Hussain Khazielakha <k.nawaz.h@gmail.com>
1 parent 8f66d35 commit 1f52056

File tree

4 files changed

+64
-150
lines changed

4 files changed

+64
-150
lines changed

‎api/v1beta1/azurecluster_default.go

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ func (s *SubnetSpec) setClusterSubnetDefaults(clusterName string) {
200200
s.SecurityGroup.Name = generateClusterSecurityGroupName(clusterName)
201201
}
202202
if s.RouteTable.Name == "" {
203-
s.RouteTable.Name = generateClustereRouteTableName(clusterName)
203+
s.RouteTable.Name = generateClusterRouteTableName(clusterName)
204204
}
205205
if s.NatGateway.Name == "" {
206206
s.NatGateway.Name = generateClusterNatGatewayName(clusterName)
@@ -456,25 +456,6 @@ func (lb *LoadBalancerClassSpec) setOutboundLBDefaults() {
456456
}
457457
}
458458

459-
func setControlPlaneOutboundLBDefaults(lb *LoadBalancerClassSpec, apiserverLBType LBType) {
460-
// public clusters don't need control plane outbound lb
461-
if apiserverLBType == Public {
462-
return
463-
}
464-
465-
// private clusters can disable control plane outbound lb by setting it to nil.
466-
if lb == nil {
467-
return
468-
}
469-
470-
lb.Type = Public
471-
lb.SKU = SKUStandard
472-
473-
if lb.IdleTimeoutInMinutes == nil {
474-
lb.IdleTimeoutInMinutes = ptr.To[int32](DefaultOutboundRuleIdleTimeoutInMinutes)
475-
}
476-
}
477-
478459
// generateVnetName generates a virtual network name, based on the cluster name.
479460
func generateVnetName(clusterName string) string {
480461
return fmt.Sprintf("%s-%s", clusterName, "vnet")
@@ -520,8 +501,8 @@ func generateNodeSecurityGroupName(clusterName string) string {
520501
return fmt.Sprintf("%s-%s", clusterName, "node-nsg")
521502
}
522503

523-
// generateClustereRouteTableName generates a route table name, based on the cluster name.
524-
func generateClustereRouteTableName(clusterName string) string {
504+
// generateClusterRouteTableName generates a route table name, based on the cluster name.
505+
func generateClusterRouteTableName(clusterName string) string {
525506
return fmt.Sprintf("%s-%s", clusterName, "routetable")
526507
}
527508

@@ -555,7 +536,7 @@ func generateFrontendIPConfigName(lbName string) string {
555536
return fmt.Sprintf("%s-%s", lbName, "frontEnd")
556537
}
557538

558-
// generateFrontendIPConfigName generates a load balancer frontend IP config name.
539+
// generatePrivateIPConfigName generates a load balancer frontend private IP config name.
559540
func generatePrivateIPConfigName(lbName string) string {
560541
return fmt.Sprintf("%s-%s", lbName, "frontEnd-internal-ip")
561542
}

‎api/v1beta1/azurecluster_validation.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,6 @@ func validateSubnets(controlPlaneEnabled bool, subnets Subnets, vnet VnetSpec, f
231231
requiredSubnetRoles["control-plane"] = false
232232
}
233233
clusterSubnet := false
234-
numberofClusterSubnets := 0
235234
for i, subnet := range subnets {
236235
if err := validateSubnetName(subnet.Name, fldPath.Index(i).Child("name")); err != nil {
237236
allErrs = append(allErrs, err)
@@ -242,7 +241,6 @@ func validateSubnets(controlPlaneEnabled bool, subnets Subnets, vnet VnetSpec, f
242241
subnetNames[subnet.Name] = true
243242
if subnet.Role == SubnetCluster {
244243
clusterSubnet = true
245-
numberofClusterSubnets++
246244
} else {
247245
for role := range requiredSubnetRoles {
248246
if role == string(subnet.Role) {
@@ -251,10 +249,10 @@ func validateSubnets(controlPlaneEnabled bool, subnets Subnets, vnet VnetSpec, f
251249
}
252250
}
253251

254-
for _, rule := range subnet.SecurityGroup.SecurityRules {
252+
for j, rule := range subnet.SecurityGroup.SecurityRules {
255253
if err := validateSecurityRule(
256254
rule,
257-
fldPath.Index(i).Child("securityGroup").Child("securityRules").Index(i),
255+
fldPath.Index(i).Child("securityGroup").Child("securityRules").Index(j),
258256
); err != nil {
259257
allErrs = append(allErrs, err...)
260258
}
@@ -596,7 +594,7 @@ func validateClassSpecForAPIServerLB(lb LoadBalancerClassSpec, old *LoadBalancer
596594

597595
if lb.IdleTimeoutInMinutes != nil && (*lb.IdleTimeoutInMinutes < MinLBIdleTimeoutInMinutes || *lb.IdleTimeoutInMinutes > MaxLBIdleTimeoutInMinutes) {
598596
allErrs = append(allErrs, field.Invalid(apiServerLBPath.Child("idleTimeoutInMinutes"), *lb.IdleTimeoutInMinutes,
599-
fmt.Sprintf("Node outbound idle timeout should be between %d and %d minutes", MinLBIdleTimeoutInMinutes, MaxLoadBalancerOutboundIPs)))
597+
fmt.Sprintf("API Server load balancer idle timeout should be between %d and %d minutes", MinLBIdleTimeoutInMinutes, MaxLBIdleTimeoutInMinutes)))
600598
}
601599

602600
return allErrs
@@ -629,7 +627,7 @@ func validateClassSpecForNodeOutboundLB(lb *LoadBalancerClassSpec, old *LoadBala
629627

630628
if lb.IdleTimeoutInMinutes != nil && (*lb.IdleTimeoutInMinutes < MinLBIdleTimeoutInMinutes || *lb.IdleTimeoutInMinutes > MaxLBIdleTimeoutInMinutes) {
631629
allErrs = append(allErrs, field.Invalid(fldPath.Child("idleTimeoutInMinutes"), *lb.IdleTimeoutInMinutes,
632-
fmt.Sprintf("Node outbound idle timeout should be between %d and %d minutes", MinLBIdleTimeoutInMinutes, MaxLoadBalancerOutboundIPs)))
630+
fmt.Sprintf("Node outbound idle timeout should be between %d and %d minutes", MinLBIdleTimeoutInMinutes, MaxLBIdleTimeoutInMinutes)))
633631
}
634632

635633
return allErrs
@@ -651,7 +649,7 @@ func validateClassSpecForControlPlaneOutboundLB(lb *LoadBalancerClassSpec, apise
651649

652650
if lb.IdleTimeoutInMinutes != nil && (*lb.IdleTimeoutInMinutes < MinLBIdleTimeoutInMinutes || *lb.IdleTimeoutInMinutes > MaxLBIdleTimeoutInMinutes) {
653651
allErrs = append(allErrs, field.Invalid(fldPath.Child("idleTimeoutInMinutes"), *lb.IdleTimeoutInMinutes,
654-
fmt.Sprintf("Control plane outbound idle timeout should be between %d and %d minutes", MinLBIdleTimeoutInMinutes, MaxLoadBalancerOutboundIPs)))
652+
fmt.Sprintf("Control plane outbound idle timeout should be between %d and %d minutes", MinLBIdleTimeoutInMinutes, MaxLBIdleTimeoutInMinutes)))
655653
}
656654
}
657655

‎api/v1beta1/azurecluster_validation_test.go

Lines changed: 55 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -131,29 +131,40 @@ func TestClusterWithPreexistingVnetValid(t *testing.T) {
131131
}
132132

133133
func TestClusterWithPreexistingVnetInvalid(t *testing.T) {
134-
type test struct {
134+
tests := []struct {
135135
name string
136136
cluster *AzureCluster
137-
}
138-
139-
testCase := test{
140-
name: "azurecluster with pre-existing vnet - invalid",
141-
cluster: createValidCluster(),
142-
}
143-
144-
// invalid because it doesn't specify a controlplane subnet
145-
testCase.cluster.Spec.NetworkSpec.Subnets[0] = SubnetSpec{
146-
SubnetClassSpec: SubnetClassSpec{
147-
Role: "random",
148-
Name: "random-subnet",
137+
wantErr bool
138+
}{
139+
{
140+
name: "azurecluster with pre-existing vnet - invalid",
141+
cluster: func() *AzureCluster {
142+
cluster := createValidCluster()
143+
// invalid because it doesn't specify a controlplane subnet
144+
cluster.Spec.NetworkSpec.Subnets[0] = SubnetSpec{
145+
SubnetClassSpec: SubnetClassSpec{
146+
Role: "random",
147+
Name: "random-subnet",
148+
},
149+
}
150+
return cluster
151+
}(),
152+
wantErr: true,
149153
},
150154
}
151155

152-
t.Run(testCase.name, func(t *testing.T) {
153-
g := NewWithT(t)
154-
_, err := testCase.cluster.validateCluster(nil)
155-
g.Expect(err).To(HaveOccurred())
156-
})
156+
for _, tc := range tests {
157+
t.Run(tc.name, func(t *testing.T) {
158+
t.Parallel()
159+
g := NewWithT(t)
160+
_, err := tc.cluster.validateCluster(nil)
161+
if tc.wantErr {
162+
g.Expect(err).To(HaveOccurred())
163+
} else {
164+
g.Expect(err).NotTo(HaveOccurred())
165+
}
166+
})
167+
}
157168
}
158169

159170
func TestClusterWithoutPreexistingVnetValid(t *testing.T) {
@@ -1975,24 +1986,35 @@ func TestServiceEndpointsLackRequiredFieldLocations(t *testing.T) {
19751986
}
19761987

19771988
func TestClusterWithExtendedLocationInvalid(t *testing.T) {
1978-
type test struct {
1989+
tests := []struct {
19791990
name string
19801991
cluster *AzureCluster
1992+
wantErr bool
1993+
}{
1994+
{
1995+
name: "azurecluster spec with extended location but not enable EdgeZone feature gate flag",
1996+
cluster: func() *AzureCluster {
1997+
cluster := createValidCluster()
1998+
cluster.Spec.ExtendedLocation = &ExtendedLocationSpec{
1999+
Name: "rr4",
2000+
Type: "EdgeZone",
2001+
}
2002+
return cluster
2003+
}(),
2004+
wantErr: true,
2005+
},
19812006
}
19822007

1983-
testCase := test{
1984-
name: "azurecluster spec with extended location but not enable EdgeZone feature gate flag",
1985-
cluster: createValidCluster(),
1986-
}
1987-
1988-
testCase.cluster.Spec.ExtendedLocation = &ExtendedLocationSpec{
1989-
Name: "rr4",
1990-
Type: "EdgeZone",
2008+
for _, tc := range tests {
2009+
t.Run(tc.name, func(t *testing.T) {
2010+
t.Parallel()
2011+
g := NewWithT(t)
2012+
err := tc.cluster.validateClusterSpec(nil)
2013+
if tc.wantErr {
2014+
g.Expect(err).NotTo(BeNil())
2015+
} else {
2016+
g.Expect(err).To(BeNil())
2017+
}
2018+
})
19912019
}
1992-
1993-
t.Run(testCase.name, func(t *testing.T) {
1994-
g := NewWithT(t)
1995-
err := testCase.cluster.validateClusterSpec(nil)
1996-
g.Expect(err).NotTo(BeNil())
1997-
})
19982020
}

‎api/v1beta1/azureclustertemplate_default_test.go

Lines changed: 0 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,93 +1169,6 @@ func TestNodeOutboundLBClassDefaults(t *testing.T) {
11691169
}
11701170
}
11711171

1172-
func TestControlPlaneOutboundLBTemplateDefaults(t *testing.T) {
1173-
cases := []struct {
1174-
name string
1175-
clusterTemplate *AzureClusterTemplate
1176-
outputTemplate *AzureClusterTemplate
1177-
}{
1178-
{
1179-
name: "no cp lb for public clusters",
1180-
clusterTemplate: &AzureClusterTemplate{
1181-
ObjectMeta: metav1.ObjectMeta{
1182-
Name: "test-cluster-template",
1183-
},
1184-
Spec: AzureClusterTemplateSpec{
1185-
Template: AzureClusterTemplateResource{
1186-
Spec: AzureClusterTemplateResourceSpec{
1187-
NetworkSpec: NetworkTemplateSpec{
1188-
APIServerLB: LoadBalancerClassSpec{Type: Public},
1189-
},
1190-
},
1191-
},
1192-
},
1193-
},
1194-
outputTemplate: &AzureClusterTemplate{
1195-
ObjectMeta: metav1.ObjectMeta{
1196-
Name: "test-cluster-template",
1197-
},
1198-
Spec: AzureClusterTemplateSpec{
1199-
Template: AzureClusterTemplateResource{
1200-
Spec: AzureClusterTemplateResourceSpec{
1201-
NetworkSpec: NetworkTemplateSpec{
1202-
APIServerLB: LoadBalancerClassSpec{Type: Public},
1203-
},
1204-
},
1205-
},
1206-
},
1207-
},
1208-
},
1209-
{
1210-
name: "no cp lb for private clusters",
1211-
clusterTemplate: &AzureClusterTemplate{
1212-
ObjectMeta: metav1.ObjectMeta{
1213-
Name: "test-cluster-template",
1214-
},
1215-
Spec: AzureClusterTemplateSpec{
1216-
Template: AzureClusterTemplateResource{
1217-
Spec: AzureClusterTemplateResourceSpec{
1218-
NetworkSpec: NetworkTemplateSpec{
1219-
APIServerLB: LoadBalancerClassSpec{Type: Internal},
1220-
},
1221-
},
1222-
},
1223-
},
1224-
},
1225-
outputTemplate: &AzureClusterTemplate{
1226-
ObjectMeta: metav1.ObjectMeta{
1227-
Name: "test-cluster-template",
1228-
},
1229-
Spec: AzureClusterTemplateSpec{
1230-
Template: AzureClusterTemplateResource{
1231-
Spec: AzureClusterTemplateResourceSpec{
1232-
NetworkSpec: NetworkTemplateSpec{
1233-
APIServerLB: LoadBalancerClassSpec{Type: Internal},
1234-
},
1235-
},
1236-
},
1237-
},
1238-
},
1239-
},
1240-
}
1241-
1242-
for _, c := range cases {
1243-
tc := c
1244-
t.Run(tc.name, func(t *testing.T) {
1245-
t.Parallel()
1246-
setControlPlaneOutboundLBDefaults(
1247-
tc.clusterTemplate.Spec.Template.Spec.NetworkSpec.ControlPlaneOutboundLB,
1248-
tc.clusterTemplate.Spec.Template.Spec.NetworkSpec.APIServerLB.Type,
1249-
)
1250-
if !reflect.DeepEqual(tc.clusterTemplate, tc.outputTemplate) {
1251-
expected, _ := json.MarshalIndent(tc.outputTemplate, "", "\t")
1252-
actual, _ := json.MarshalIndent(tc.clusterTemplate, "", "\t")
1253-
t.Errorf("Expected %s, got %s", string(expected), string(actual))
1254-
}
1255-
})
1256-
}
1257-
}
1258-
12591172
func TestBastionTemplateDefaults(t *testing.T) {
12601173
cases := []struct {
12611174
name string

0 commit comments

Comments
 (0)