Skip to content

Commit 8cc7e3d

Browse files
committed
Adding webhook validation and test cases
1 parent 097255c commit 8cc7e3d

File tree

5 files changed

+208
-0
lines changed

5 files changed

+208
-0
lines changed

api/v1beta1/azurecluster_validation.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ func validateNetworkSpec(controlPlaneEnabled bool, networkSpec NetworkSpec, old
203203
lbType = networkSpec.APIServerLB.Type
204204
}
205205
allErrs = append(allErrs, validatePrivateDNSZoneName(networkSpec.PrivateDNSZoneName, controlPlaneEnabled, lbType, fldPath.Child("privateDNSZoneName"))...)
206+
allErrs = append(allErrs, validatePrivateDNSZoneResourceGroup(networkSpec.PrivateDNSZoneName, networkSpec.PrivateDNSZoneResourceGroup, fldPath.Child("privateDNSZoneResourceGroup"))...)
206207

207208
if len(allErrs) == 0 {
208209
return nil
@@ -556,6 +557,23 @@ func validatePrivateDNSZoneName(privateDNSZoneName string, controlPlaneEnabled b
556557
return allErrs
557558
}
558559

560+
// validatePrivateDNSZoneName validates the PrivateDNSZoneResourceGroup.
561+
func validatePrivateDNSZoneResourceGroup(privateDNSZoneName string, privateDNSZoneResourceGroup string, fldPath *field.Path) field.ErrorList {
562+
var allErrs field.ErrorList
563+
564+
if privateDNSZoneResourceGroup != "" {
565+
if valid.IsNull(privateDNSZoneName) {
566+
allErrs = append(allErrs, field.Invalid(fldPath, privateDNSZoneName,
567+
"PrivateDNSZoneResourceGroup can only be used when PrivateDNSZoneName is provided"))
568+
}
569+
if err := validateResourceGroup(privateDNSZoneResourceGroup, fldPath); err != nil {
570+
allErrs = append(allErrs, err)
571+
}
572+
}
573+
574+
return allErrs
575+
}
576+
559577
// validateCloudProviderConfigOverrides validates CloudProviderConfigOverrides.
560578
func validateCloudProviderConfigOverrides(oldConfig, newConfig *CloudProviderConfigOverrides, fldPath *field.Path) field.ErrorList {
561579
var allErrs field.ErrorList

api/v1beta1/azurecluster_validation_test.go

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,9 @@ func TestResourceGroupValid(t *testing.T) {
465465
err := validateResourceGroup(testCase.resourceGroup,
466466
field.NewPath("spec").Child("networkSpec").Child("vnet").Child("resourceGroup"))
467467
g.Expect(err).NotTo(HaveOccurred())
468+
err = validateResourceGroup(testCase.resourceGroup,
469+
field.NewPath("spec").Child("networkSpec").Child("privateDNSZoneResourceGroup"))
470+
g.Expect(err).NotTo(HaveOccurred())
468471
})
469472
}
470473

@@ -487,6 +490,12 @@ func TestResourceGroupInvalid(t *testing.T) {
487490
g.Expect(err.Type).To(Equal(field.ErrorTypeInvalid))
488491
g.Expect(err.Field).To(Equal("spec.networkSpec.vnet.resourceGroup"))
489492
g.Expect(err.BadValue).To(BeEquivalentTo(testCase.resourceGroup))
493+
err = validateResourceGroup(testCase.resourceGroup,
494+
field.NewPath("spec").Child("networkSpec").Child("privateDNSZoneResourceGroup"))
495+
g.Expect(err).NotTo(BeNil())
496+
g.Expect(err.Type).To(Equal(field.ErrorTypeInvalid))
497+
g.Expect(err.Field).To(Equal("spec.networkSpec.privateDNSZoneResourceGroup"))
498+
g.Expect(err.BadValue).To(BeEquivalentTo(testCase.resourceGroup))
490499
})
491500
}
492501

@@ -1371,6 +1380,81 @@ func TestPrivateDNSZoneName(t *testing.T) {
13711380
}
13721381
}
13731382

1383+
func TestPrivateDNSZoneResourceGroup(t *testing.T) {
1384+
testcases := []struct {
1385+
name string
1386+
network NetworkSpec
1387+
wantErr bool
1388+
expectedErr field.Error
1389+
}{
1390+
{
1391+
name: "testEmptyPrivateDNSZoneNameAndResourceGroup",
1392+
network: NetworkSpec{
1393+
NetworkClassSpec: NetworkClassSpec{
1394+
PrivateDNSZoneName: "",
1395+
PrivateDNSZoneResourceGroup: "",
1396+
},
1397+
},
1398+
wantErr: false,
1399+
},
1400+
{
1401+
name: "testValidPrivateDNSZoneNameAndResourceGroup",
1402+
network: NetworkSpec{
1403+
NetworkClassSpec: NetworkClassSpec{
1404+
PrivateDNSZoneName: "good.dns.io",
1405+
PrivateDNSZoneResourceGroup: "test-rg",
1406+
},
1407+
},
1408+
wantErr: false,
1409+
},
1410+
{
1411+
name: "testInvalidPrivateDNSZoneResourceGroup",
1412+
network: NetworkSpec{
1413+
NetworkClassSpec: NetworkClassSpec{
1414+
PrivateDNSZoneName: "good.dns.io",
1415+
PrivateDNSZoneResourceGroup: "inv@lid-rg",
1416+
},
1417+
},
1418+
expectedErr: field.Error{
1419+
Type: "FieldValueInvalid",
1420+
Field: "spec.networkSpec.privateDNSZoneResourceGroup",
1421+
BadValue: "inv@lid-rg",
1422+
Detail: "resourceGroup doesn't match regex ^[-\\w\\._\\(\\)]+$",
1423+
},
1424+
wantErr: true,
1425+
},
1426+
{
1427+
name: "testEmptyPrivateDNSZoneNameWithValidResourceGroup",
1428+
network: NetworkSpec{
1429+
NetworkClassSpec: NetworkClassSpec{
1430+
PrivateDNSZoneName: "",
1431+
PrivateDNSZoneResourceGroup: "test-rg",
1432+
},
1433+
},
1434+
expectedErr: field.Error{
1435+
Type: "FieldValueInvalid",
1436+
Field: "spec.networkSpec.privateDNSZoneResourceGroup",
1437+
BadValue: "",
1438+
Detail: "PrivateDNSZoneResourceGroup can only be used when PrivateDNSZoneName is provided",
1439+
},
1440+
wantErr: true,
1441+
},
1442+
}
1443+
1444+
for _, test := range testcases {
1445+
t.Run(test.name, func(t *testing.T) {
1446+
t.Parallel()
1447+
g := NewWithT(t)
1448+
err := validatePrivateDNSZoneResourceGroup(test.network.PrivateDNSZoneName, test.network.PrivateDNSZoneResourceGroup, field.NewPath("spec", "networkSpec", "privateDNSZoneResourceGroup"))
1449+
if test.wantErr {
1450+
g.Expect(err).To(ContainElement(MatchError(test.expectedErr.Error())))
1451+
} else {
1452+
g.Expect(err).To(BeEmpty())
1453+
}
1454+
})
1455+
}
1456+
}
1457+
13741458
func TestValidateNodeOutboundLB(t *testing.T) {
13751459
testcases := []struct {
13761460
name string

api/v1beta1/azurecluster_webhook.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,13 @@ func (c *AzureCluster) ValidateUpdate(oldRaw runtime.Object) (admission.Warnings
116116
allErrs = append(allErrs, err)
117117
}
118118

119+
if err := webhookutils.ValidateImmutable(
120+
field.NewPath("spec", "networkSpec", "privateDNSZoneResourceGroup"),
121+
old.Spec.NetworkSpec.PrivateDNSZoneResourceGroup,
122+
c.Spec.NetworkSpec.PrivateDNSZoneResourceGroup); err != nil {
123+
allErrs = append(allErrs, err)
124+
}
125+
119126
// Allow enabling azure bastion but avoid disabling it.
120127
if old.Spec.BastionSpec.AzureBastion != nil && !reflect.DeepEqual(old.Spec.BastionSpec.AzureBastion, c.Spec.BastionSpec.AzureBastion) {
121128
allErrs = append(allErrs,

api/v1beta1/azureclustertemplate_validation.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ func (c *AzureClusterTemplate) validateClusterTemplateSpec() field.ErrorList {
6262

6363
allErrs = append(allErrs, c.validatePrivateDNSZoneName()...)
6464

65+
allErrs = append(allErrs, c.validatePrivateDNSZoneResourceGroup()...)
66+
6567
return allErrs
6668
}
6769

@@ -169,3 +171,18 @@ func (c *AzureClusterTemplate) validatePrivateDNSZoneName() field.ErrorList {
169171

170172
return allErrs
171173
}
174+
175+
func (c *AzureClusterTemplate) validatePrivateDNSZoneResourceGroup() field.ErrorList {
176+
var allErrs field.ErrorList
177+
178+
fldPath := field.NewPath("spec").Child("template").Child("spec").Child("networkSpec").Child("privateDNSZoneResourceGroup")
179+
networkSpec := c.Spec.Template.Spec.NetworkSpec
180+
181+
allErrs = append(allErrs, validatePrivateDNSZoneResourceGroup(
182+
networkSpec.PrivateDNSZoneName,
183+
networkSpec.PrivateDNSZoneResourceGroup,
184+
fldPath,
185+
)...)
186+
187+
return allErrs
188+
}

api/v1beta1/azureclustertemplate_validation_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -822,6 +822,88 @@ func TestValidatePrivateDNSZoneName(t *testing.T) {
822822
})
823823
}
824824
}
825+
826+
func TestValidatePrivateDNSZoneResourceGroup(t *testing.T) {
827+
cases := []struct {
828+
name string
829+
clusterTemplate *AzureClusterTemplate
830+
expectValid bool
831+
}{
832+
{
833+
name: "not set",
834+
clusterTemplate: &AzureClusterTemplate{
835+
ObjectMeta: metav1.ObjectMeta{
836+
Name: "test-cluster-template",
837+
},
838+
Spec: AzureClusterTemplateSpec{
839+
Template: AzureClusterTemplateResource{
840+
Spec: AzureClusterTemplateResourceSpec{
841+
NetworkSpec: NetworkTemplateSpec{},
842+
},
843+
},
844+
},
845+
},
846+
expectValid: true,
847+
},
848+
{
849+
name: "show be set if PrivateDNSZoneName is given",
850+
clusterTemplate: &AzureClusterTemplate{
851+
ObjectMeta: metav1.ObjectMeta{
852+
Name: "test-cluster-template",
853+
},
854+
Spec: AzureClusterTemplateSpec{
855+
Template: AzureClusterTemplateResource{
856+
Spec: AzureClusterTemplateResourceSpec{
857+
NetworkSpec: NetworkTemplateSpec{
858+
NetworkClassSpec: NetworkClassSpec{
859+
PrivateDNSZoneName: "e.f.g",
860+
PrivateDNSZoneResourceGroup: "a.b.c",
861+
},
862+
},
863+
},
864+
},
865+
},
866+
},
867+
expectValid: true,
868+
},
869+
{
870+
name: "show not be set if PrivateDNSZoneName is not given",
871+
clusterTemplate: &AzureClusterTemplate{
872+
ObjectMeta: metav1.ObjectMeta{
873+
Name: "test-cluster-template",
874+
},
875+
Spec: AzureClusterTemplateSpec{
876+
Template: AzureClusterTemplateResource{
877+
Spec: AzureClusterTemplateResourceSpec{
878+
NetworkSpec: NetworkTemplateSpec{
879+
NetworkClassSpec: NetworkClassSpec{
880+
PrivateDNSZoneResourceGroup: "a.b.c",
881+
},
882+
},
883+
},
884+
},
885+
},
886+
},
887+
expectValid: false,
888+
},
889+
}
890+
891+
for _, c := range cases {
892+
tc := c
893+
t.Run(tc.name, func(t *testing.T) {
894+
t.Parallel()
895+
g := NewWithT(t)
896+
res := tc.clusterTemplate.validatePrivateDNSZoneResourceGroup()
897+
898+
if tc.expectValid {
899+
g.Expect(res).To(BeNil())
900+
} else {
901+
g.Expect(res).NotTo(BeNil())
902+
}
903+
})
904+
}
905+
}
906+
825907
func TestValidateNetworkSpec(t *testing.T) {
826908
cases := []struct {
827909
name string

0 commit comments

Comments
 (0)