Skip to content

Commit f4c7a10

Browse files
author
Joshua Reed
committed
Temp fixup webhook validations.
1 parent ed7cc45 commit f4c7a10

File tree

2 files changed

+39
-36
lines changed

2 files changed

+39
-36
lines changed

‎api/v1beta2/cloudstackcluster_webhook.go

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package v1beta2
1818

1919
import (
2020
"fmt"
21+
"reflect"
2122

2223
"k8s.io/apimachinery/pkg/api/errors"
2324
"k8s.io/apimachinery/pkg/runtime"
@@ -67,17 +68,19 @@ func (r *CloudStackCluster) ValidateCreate() error {
6768
field.NewPath("spec", "account"), "specifying account requires additionally specifying domain"))
6869
}
6970

70-
// // Require Zones and their respective Networks.
71-
// if len(r.Spec.Zones) <= 0 {
72-
// errorList = append(errorList, field.Required(field.NewPath("spec", "Zones"), "Zones"))
73-
// } else {
74-
// for _, zone := range r.Spec.Zones {
75-
// if zone.Network.Name == "" && zone.Network.ID == "" {
76-
// errorList = append(errorList, field.Required(
77-
// field.NewPath("spec", "Zones", "Network"), "each Zone requires a Network specification"))
78-
// }
79-
// }
80-
// }
71+
// Require FailureDomains and their respective fields.
72+
// TODO flesh out verifications on FD fields.
73+
if len(r.Spec.FailureDomains) <= 0 {
74+
errorList = append(errorList, field.Required(field.NewPath("spec", "FailureDomains"), "FailureDomains"))
75+
} else {
76+
for _, fdSpec := range r.Spec.FailureDomains {
77+
if fdSpec.Zone.Network.Name == "" && fdSpec.Zone.Network.ID == "" {
78+
errorList = append(errorList, field.Required(
79+
field.NewPath("spec", "FailureDomains", "Zone", "Network"),
80+
"each Zone requires a Network specification"))
81+
}
82+
}
83+
}
8184

8285
return webhookutil.AggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, errorList)
8386
}
@@ -98,10 +101,10 @@ func (r *CloudStackCluster) ValidateUpdate(old runtime.Object) error {
98101

99102
// No spec fields may be updated.
100103
errorList := field.ErrorList(nil)
101-
// if !reflect.DeepEqual(oldSpec.Zones, spec.Zones) {
102-
// errorList = append(errorList, field.Forbidden(
103-
// field.NewPath("spec", "Zones"), "Zones and sub-attributes may not be modified after creation"))
104-
// }
104+
if !reflect.DeepEqual(oldSpec.FailureDomains, spec.FailureDomains) {
105+
errorList = append(errorList, field.Forbidden(
106+
field.NewPath("spec", "Zones"), "Zones and sub-attributes may not be modified after creation"))
107+
}
105108
if oldSpec.ControlPlaneEndpoint.Host != "" { // Need to allow one time endpoint setting via CAPC cluster controller.
106109
errorList = webhookutil.EnsureStringFieldsAreEqual(
107110
spec.ControlPlaneEndpoint.Host, oldSpec.ControlPlaneEndpoint.Host, "controlplaneendpoint.host", errorList)

‎api/v1beta2/cloudstackcluster_webhook_test.go

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,14 @@ import (
2121

2222
. "github.com/onsi/ginkgo/v2"
2323
. "github.com/onsi/gomega"
24+
infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta2"
2425
dummies "sigs.k8s.io/cluster-api-provider-cloudstack/test/dummies/v1beta2"
2526
)
2627

2728
var _ = Describe("CloudStackCluster webhooks", func() {
2829
var ctx context.Context
2930
forbiddenRegex := "admission webhook.*denied the request.*Forbidden\\: %s"
30-
// requiredRegex := "admission webhook.*denied the request.*Required value\\: %s"
31+
requiredRegex := "admission webhook.*denied the request.*Required value\\: %s"
3132

3233
BeforeEach(func() { // Reset test vars to initial state.
3334
ctx = context.Background()
@@ -41,18 +42,18 @@ var _ = Describe("CloudStackCluster webhooks", func() {
4142
Ω(k8sClient.Create(ctx, dummies.CSCluster)).Should(Succeed())
4243
})
4344

44-
// TODO update for failure domains.
45-
// It("Should reject a CloudStackCluster with missing Zones.Network attribute", func() {
46-
// dummies.CSCluster.Spec.Zones = []infrav1.Zone{{}}
47-
// dummies.CSCluster.Spec.Zones[0].Name = "ZoneWNoNetwork"
48-
// Ω(k8sClient.Create(ctx, dummies.CSCluster)).Should(
49-
// MatchError(MatchRegexp(requiredRegex, "each Zone requires a Network specification")))
50-
// })
45+
It("Should reject a CloudStackCluster with missing Zones.Network attribute", func() {
46+
dummies.CSCluster.Spec.FailureDomains = []infrav1.CloudStackFailureDomainSpec{{}}
47+
dummies.CSCluster.Spec.FailureDomains[0].Zone.Name = "ZoneWNoNetwork"
48+
Ω(k8sClient.Create(ctx, dummies.CSCluster)).Should(
49+
MatchError(MatchRegexp(requiredRegex, "each Zone requires a Network specification")))
50+
})
5151

52-
// It("Should reject a CloudStackCluster with missing Zones attribute", func() {
53-
// dummies.CSCluster.Spec.Zones = []infrav1.Zone{}
54-
// Ω(k8sClient.Create(ctx, dummies.CSCluster)).Should(MatchError(MatchRegexp(requiredRegex, "Zones")))
55-
// })
52+
It("Should reject a CloudStackCluster with missing Zone attribute", func() {
53+
dummies.CSCluster.Spec.FailureDomains[0].Zone = infrav1.Zone{}
54+
Ω(k8sClient.Create(ctx, dummies.CSCluster)).Should(MatchError(MatchRegexp(requiredRegex,
55+
"each Zone requires a Network specification")))
56+
})
5657

5758
It("Should reject a CloudStackCluster with IdentityRef not of kind 'Secret'", func() {
5859
dummies.CSCluster.Spec.IdentityRef.Kind = "NewType"
@@ -66,15 +67,14 @@ var _ = Describe("CloudStackCluster webhooks", func() {
6667
Ω(k8sClient.Create(ctx, dummies.CSCluster)).Should(Succeed())
6768
})
6869

69-
// TODO update for failure domains.
70-
// It("Should reject updates to CloudStackCluster Zones", func() {
71-
// dummies.CSCluster.Spec.Zones = []infrav1.Zone{dummies.Zone1}
72-
// Ω(k8sClient.Update(ctx, dummies.CSCluster)).Should(MatchError(MatchRegexp(forbiddenRegex, "Zones and sub")))
73-
// })
74-
// It("Should reject updates to Networks specified in CloudStackCluster Zones", func() {
75-
// dummies.CSCluster.Spec.Zones[0].Network.Name = "ArbitraryUpdateNetworkName"
76-
// Ω(k8sClient.Update(ctx, dummies.CSCluster)).Should(MatchError(MatchRegexp(forbiddenRegex, "Zones and sub")))
77-
// })
70+
It("Should reject updates to CloudStackCluster FailureDomains", func() {
71+
dummies.CSCluster.Spec.FailureDomains[0].Zone.Name = "SomeRandomUpdate"
72+
Ω(k8sClient.Update(ctx, dummies.CSCluster)).Should(MatchError(MatchRegexp(forbiddenRegex, "Zones and sub")))
73+
})
74+
It("Should reject updates to Networks specified in CloudStackCluster Zones", func() {
75+
dummies.CSCluster.Spec.FailureDomains[0].Zone.Network.Name = "ArbitraryUpdateNetworkName"
76+
Ω(k8sClient.Update(ctx, dummies.CSCluster)).Should(MatchError(MatchRegexp(forbiddenRegex, "Zones and sub")))
77+
})
7878
It("Should reject updates to CloudStackCluster controlplaneendpoint.host", func() {
7979
dummies.CSCluster.Spec.ControlPlaneEndpoint.Host = "1.1.1.1"
8080
Ω(k8sClient.Update(ctx, dummies.CSCluster)).

0 commit comments

Comments
 (0)