Skip to content

Commit cb61164

Browse files
author
Joshua Reed
committed
Webhook tests pass now.
1 parent 4433992 commit cb61164

File tree

4 files changed

+112
-103
lines changed

4 files changed

+112
-103
lines changed

‎api/v1beta1/cloudstackcluster_webhook.go

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

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

2223
"github.com/aws/cluster-api-provider-cloudstack/pkg/webhookutil"
2324
"k8s.io/apimachinery/pkg/api/errors"
@@ -63,14 +64,20 @@ func (r *CloudStackCluster) ValidateCreate() error {
6364
}
6465

6566
if (r.Spec.Account != "") && (r.Spec.Domain == "") {
66-
errorList = append(errorList, field.Required(field.NewPath("spec", "account"), "specifying account requires additionally specifying domain"))
67+
errorList = append(errorList, field.Required(
68+
field.NewPath("spec", "account"), "specifying account requires additionally specifying domain"))
6769
}
6870

69-
// errorList = webhook_utilities.EnsureFieldExists(r.Spec.Zones, "Zone", errorList)
70-
71-
// Zone and Network are required fields
71+
// Require Zones and their respective Networks.
7272
if len(r.Spec.Zones) <= 0 {
73-
errorList = append(errorList, field.Required(field.NewPath("spec", "Zones"), "asdfasdfasdf"))
73+
errorList = append(errorList, field.Required(field.NewPath("spec", "Zones"), "Zones"))
74+
} else {
75+
for _, zone := range r.Spec.Zones {
76+
if zone.Network.Name == "" && zone.Network.ID == "" {
77+
errorList = append(errorList, field.Required(
78+
field.NewPath("spec", "Zones", "Network"), "each Zone requires a Network specification"))
79+
}
80+
}
7481
}
7582

7683
return webhookutil.AggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, errorList)
@@ -81,8 +88,7 @@ func (r *CloudStackCluster) ValidateUpdate(old runtime.Object) error {
8188
cloudstackclusterlog.Info("validate update", "name", r.Name)
8289

8390
var (
84-
errorList field.ErrorList
85-
spec = r.Spec
91+
spec = r.Spec
8692
)
8793

8894
oldCluster, ok := old.(*CloudStackCluster)
@@ -91,25 +97,29 @@ func (r *CloudStackCluster) ValidateUpdate(old runtime.Object) error {
9197
}
9298
oldSpec := oldCluster.Spec
9399

94-
// IdentityRefs must be Secrets.
95-
if spec.IdentityRef != nil && spec.IdentityRef.Kind != defaultIdentityRefKind {
96-
errorList = append(errorList, field.Forbidden(field.NewPath("spec", "identityRef", "kind"), "must be a Secret"))
100+
// No spec fields may be updated.
101+
errorList := field.ErrorList(nil)
102+
if !reflect.DeepEqual(oldSpec.Zones, spec.Zones) {
103+
errorList = append(errorList, field.Forbidden(
104+
field.NewPath("spec", "Zones"), "Zones and sub-attributes may not be modified after creation"))
97105
}
98-
99-
// No spec fields may be changed
100-
// errorList = webhook_utilities.EnsureStringFieldsAreEqual(spec.Zone, oldSpec.Zone, "zone", errorList)
101-
// errorList = webhook_utilities.EnsureStringFieldsAreEqual(spec.Network, oldSpec.Network, "network", errorList)
102106
if oldSpec.ControlPlaneEndpoint.Host != "" { // Need to allow one time endpoint setting via CAPC cluster controller.
103107
errorList = webhookutil.EnsureStringFieldsAreEqual(
104108
spec.ControlPlaneEndpoint.Host, oldSpec.ControlPlaneEndpoint.Host, "controlplaneendpointhost", errorList)
105109
errorList = webhookutil.EnsureStringFieldsAreEqual(
106110
string(spec.ControlPlaneEndpoint.Port), string(oldSpec.ControlPlaneEndpoint.Port), "controlplaneendpointport", errorList)
107111
}
108112
if spec.IdentityRef != nil && oldSpec.IdentityRef != nil {
109-
errorList = webhookutil.EnsureStringFieldsAreEqual(spec.IdentityRef.Kind, oldSpec.IdentityRef.Kind, "identityRef.Kind", errorList)
113+
errorList = webhookutil.EnsureStringFieldsAreEqual(
114+
spec.IdentityRef.Kind, oldSpec.IdentityRef.Kind, "identityRef.Kind", errorList)
110115
errorList = webhookutil.EnsureStringFieldsAreEqual(spec.IdentityRef.Name, oldSpec.IdentityRef.Name, "identityRef.Name", errorList)
111116
}
112117

118+
// IdentityRefs must be Secrets.
119+
if spec.IdentityRef != nil && spec.IdentityRef.Kind != defaultIdentityRefKind {
120+
errorList = append(errorList, field.Forbidden(field.NewPath("spec", "identityRef", "kind"), "must be a Secret"))
121+
}
122+
113123
return webhookutil.AggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, errorList)
114124
}
115125

‎api/v1beta1/cloudstackcluster_webhook_test.go

Lines changed: 57 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -18,85 +18,87 @@ package v1beta1_test
1818

1919
import (
2020
"context"
21-
"fmt"
2221

2322
infrav1 "github.com/aws/cluster-api-provider-cloudstack/api/v1beta1"
2423
"github.com/aws/cluster-api-provider-cloudstack/test/dummies"
2524
. "github.com/onsi/ginkgo"
26-
. "github.com/onsi/ginkgo/extensions/table"
2725
. "github.com/onsi/gomega"
26+
"github.com/onsi/gomega/types"
2827
)
2928

30-
var _ = Describe("CloudStackCluster webhooks", func() {
29+
var errString = func(err error) string { return err.Error() }
30+
31+
func BeErrorAndMatchRegexp(regexp string, args ...interface{}) types.GomegaMatcher {
32+
return SatisfyAll(HaveOccurred(), WithTransform(errString, MatchRegexp(regexp, args)))
33+
}
3134

35+
var _ = Describe("CloudStackCluster webhooks", func() {
3236
var ctx context.Context
37+
forbiddenRegex := "admission webhook.*denied the request.*Forbidden\\: %s"
38+
requiredRegex := "admission webhook.*denied the request.*Required value\\: %s"
3339

3440
BeforeEach(func() { // Reset test vars to initial state.
35-
dummies.SetDummyVars()
3641
ctx = context.Background()
42+
dummies.SetDummyVars() // Reset cluster var.
43+
_ = k8sClient.Delete(ctx, dummies.CSCluster) // Delete any remnants.
44+
dummies.SetDummyVars() // Reset again since the k8s client can set this on delete.
3745
})
3846

39-
Context("When creating a CloudStackCluster with all validated attributes", func() {
40-
It("Should succeed", func() {
47+
Context("When creating a CloudStackCluster", func() {
48+
It("Should accept a CloudStackCluster with all attributes present", func() {
4149
Ω(k8sClient.Create(ctx, dummies.CSCluster)).Should(Succeed())
4250
})
43-
})
4451

45-
Context("When creating a CloudStackCluster with missing Network attribute", func() {
46-
It("Should be rejected by the validating webhooks", func() {
47-
Ω(k8sClient.Create(ctx, dummies.CSCluster).Error()).
48-
Should(MatchRegexp("admission webhook.*denied the request.*Required value\\: Network"))
52+
It("Should reject a CloudStackCluster with missing Zones.Network attribute", func() {
53+
dummies.CSCluster.Spec.Zones = []infrav1.Zone{{Name: "ZoneWNoNetwork"}}
54+
Ω(k8sClient.Create(ctx, dummies.CSCluster).Error()).Should(
55+
MatchRegexp(requiredRegex, "each Zone requires a Network specification"))
4956
})
50-
})
5157

52-
Context("When creating a CloudStackCluster with missing Zone attribute", func() {
53-
It("Should be rejected by the validating webhooks", func() {
54-
Ω(k8sClient.Create(ctx, dummies.CSCluster).Error()).
55-
Should(MatchRegexp("admission webhook.*denied the request.*Required value\\: Zone"))
58+
It("Should reject a CloudStackCluster with missing Zones attribute", func() {
59+
dummies.CSCluster.Spec.Zones = []infrav1.Zone{}
60+
Ω(k8sClient.Create(ctx, dummies.CSCluster).Error()).Should(MatchRegexp(requiredRegex, "Zones"))
5661
})
57-
})
5862

59-
Context("When creating a CloudStackCluster with the wrong kind of IdentityReference", func() {
60-
It("Should be rejected by the validating webhooks", func() {
61-
dummies.CSCluster.Spec.IdentityRef.Kind = "Wrong"
62-
Ω(k8sClient.Create(ctx, dummies.CSCluster).Error()).
63-
Should(MatchRegexp("admission webhook.*denied the request.*Forbidden\\: must be a Secret"))
63+
It("Should reject a CloudStackCluster with IdentityRef not of kind 'Secret'", func() {
64+
dummies.CSCluster.Spec.IdentityRef.Kind = "ConfigMap"
65+
Ω(k8sClient.Create(ctx, dummies.CSCluster).Error()).Should(MatchRegexp(forbiddenRegex, "must be a Secret"))
6466
})
6567
})
6668

6769
Context("When updating a CloudStackCluster", func() {
68-
type CSClusterModFunc func(*infrav1.CloudStackCluster)
69-
description := func() func(string, CSClusterModFunc) string {
70-
return func(field string, mod CSClusterModFunc) string {
71-
return fmt.Sprintf(
72-
"CloudStackCluster.Spec %s modification should be rejected by the validating webhooks", field)
73-
}
74-
}
75-
DescribeTable("Forbidden field modification",
76-
func(field string, mod CSClusterModFunc) {
77-
Ω(k8sClient.Create(ctx, dummies.CSCluster)).Should(Succeed())
78-
forbiddenRegex := "admission webhook.*denied the request.*Forbidden\\: %s"
79-
mod(dummies.CSCluster)
80-
Ω(k8sClient.Update(ctx, dummies.CSCluster).Error()).Should(MatchRegexp(forbiddenRegex, field))
81-
},
82-
Entry(description(), "zone", func(CSC *infrav1.CloudStackCluster) {
83-
CSC.Spec.Zones = []infrav1.Zone{dummies.Zone1}
84-
}),
85-
Entry(description(), "zonenetwork", func(CSC *infrav1.CloudStackCluster) {
86-
CSC.Spec.Zones[0].Network.Name = "ArbitraryNetworkName"
87-
}),
88-
Entry(description(), "controlplaneendpoint\\.host", func(CSC *infrav1.CloudStackCluster) {
89-
CSC.Spec.ControlPlaneEndpoint.Host = "1.1.1.1"
90-
}),
91-
Entry(description(), "identityRef\\.Kind", func(CSC *infrav1.CloudStackCluster) {
92-
CSC.Spec.IdentityRef.Kind = "ArbitraryKind"
93-
}),
94-
Entry(description(), "identityRef\\.Name", func(CSC *infrav1.CloudStackCluster) {
95-
CSC.Spec.IdentityRef.Name = "ArbitraryName"
96-
}),
97-
Entry(description(), "controlplaneendpoint\\.port", func(CSC *infrav1.CloudStackCluster) {
98-
CSC.Spec.ControlPlaneEndpoint.Port = int32(1234)
99-
}),
100-
)
70+
BeforeEach(func() {
71+
Ω(k8sClient.Create(ctx, dummies.CSCluster)).Should(Succeed())
72+
})
73+
74+
It("Should reject updates to CloudStackCluster Zones", func() {
75+
dummies.CSCluster.Spec.Zones = []infrav1.Zone{dummies.Zone1}
76+
Ω(k8sClient.Update(ctx, dummies.CSCluster)).Should(BeErrorAndMatchRegexp(forbiddenRegex, "Zones and sub"))
77+
})
78+
It("Should reject updates to CloudStackCluster Zones", func() {
79+
dummies.CSCluster.Spec.Zones[0].Network.Name = "ArbitraryUpdateNetworkName"
80+
Ω(k8sClient.Update(ctx, dummies.CSCluster)).Should(BeErrorAndMatchRegexp(forbiddenRegex, "Zones and sub"))
81+
})
82+
It("Should reject updates to CloudStackCluster controlplaneendpoint.host", func() {
83+
dummies.CSCluster.Spec.ControlPlaneEndpoint.Host = "1.1.1.1"
84+
Ω(k8sClient.Update(ctx, dummies.CSCluster)).
85+
Should(BeErrorAndMatchRegexp(forbiddenRegex, "controlplaneendpoint\\.host"))
86+
})
87+
88+
It("Should reject updates to CloudStackCluster controlplaneendpoint.port", func() {
89+
dummies.CSCluster.Spec.ControlPlaneEndpoint.Port = int32(1234)
90+
Ω(k8sClient.Update(ctx, dummies.CSCluster)).
91+
Should(BeErrorAndMatchRegexp(forbiddenRegex, "controlplaneendpoint\\.port"))
92+
})
93+
It("Should reject updates to the CloudStackCluster identity reference kind", func() {
94+
dummies.CSCluster.Spec.IdentityRef.Kind = "ConfigMap"
95+
Ω(k8sClient.Update(ctx, dummies.CSCluster)).
96+
Should(BeErrorAndMatchRegexp(forbiddenRegex, "identityRef\\.Kind"))
97+
})
98+
It("Should reject updates to the CloudStackCluster identity reference name", func() {
99+
dummies.CSCluster.Spec.IdentityRef.Name = "ConfigMap"
100+
Ω(k8sClient.Update(ctx, dummies.CSCluster)).
101+
Should(BeErrorAndMatchRegexp(forbiddenRegex, "identityRef\\.name"))
102+
})
101103
})
102104
})

‎api/v1beta1/cloudstackmachine_webhook_test.go

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -26,49 +26,39 @@ import (
2626

2727
var _ = Describe("CloudStackMachine webhook", func() {
2828
var ctx context.Context
29+
forbiddenRegex := "admission webhook.*denied the request.*Forbidden\\: %s"
30+
requiredRegex := "admission webhook.*denied the request.*Required value\\: %s"
2931

3032
BeforeEach(func() { // Reset test vars to initial state.
3133
dummies.SetDummyVars()
3234
ctx = context.Background()
33-
// Clear out any remaining machines. Ignore errors.
34-
_ = k8sClient.Delete(ctx, dummies.CSMachine1)
35+
_ = k8sClient.Delete(ctx, dummies.CSMachine1) // Clear out any remaining machines. Ignore errors.
3536
})
3637

37-
Context("When creating a CloudStackMachine with all attributes", func() {
38-
It("Should succeed", func() {
38+
Context("When creating a CloudStackMachine", func() {
39+
It("Should accept CloudStackMachine with all attributes", func() {
3940
Expect(k8sClient.Create(ctx, dummies.CSMachine1)).Should(Succeed())
4041
})
41-
})
4242

43-
Context("When creating a CloudStackMachine with missing Offering attribute", func() {
44-
It("Should be rejected by the validating webhooks", func() {
43+
It("Should reject a CloudStackMachine with missing Offering attribute", func() {
4544
dummies.CSMachine1.Spec.Offering = ""
46-
Expect(k8sClient.Create(ctx, dummies.CSMachine1).Error()).
47-
Should(MatchRegexp("admission webhook.*denied the request.*Required value\\: Offering"))
45+
Expect(k8sClient.Create(ctx, dummies.CSMachine1).Error()).Should(MatchRegexp(requiredRegex, "Offering"))
4846
})
49-
})
5047

51-
Context("When creating a CloudStackMachine with missing Template attribute", func() {
52-
It("Should be rejected by the validating webhooks", func() {
48+
It("Should be reject a CloudStackMachine with missint Template attribute", func() {
5349
dummies.CSMachine1.Spec.Template = ""
54-
Expect(k8sClient.Create(ctx, dummies.CSMachine1).Error()).
55-
Should(MatchRegexp("admission webhook.*denied the request.*Required value\\: Template"))
50+
Expect(k8sClient.Create(ctx, dummies.CSMachine1).Error()).Should(MatchRegexp(requiredRegex, "Template"))
5651
})
57-
})
5852

59-
Context("When creating a CloudStackMachine with the wrong kind of IdentityReference", func() {
60-
It("Should be rejected by the validating webhooks", func() {
53+
It("Should be reject a CloudStackMachine with IdentityRef not of kind 'Secret'", func() {
6154
dummies.CSMachine1.Spec.IdentityRef.Kind = "ConfigMap"
6255
Expect(k8sClient.Create(ctx, dummies.CSMachine1).Error()).
63-
Should(MatchRegexp("admission webhook.*denied the request.*Forbidden\\: must be a Secret"))
56+
Should(MatchRegexp(forbiddenRegex, "must be a Secret"))
6457
})
6558
})
6659

67-
// Need the `-- not template` here to make the context unique. Apparently ginkgo uses startswith.
6860
Context("When updating a CloudStackMachine", func() {
69-
forbiddenRegex := "admission webhook.*denied the request.*Forbidden\\: %s"
70-
71-
BeforeEach(func() { // Reset test vars to initial state.
61+
BeforeEach(func() {
7262
Ω(k8sClient.Create(ctx, dummies.CSMachine1)).Should(Succeed())
7363
})
7464

‎test/dummies/vars.go

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,15 @@ func SetTestTags() {
5050
func SetDummyVars() {
5151
// These need to be in order as they build upon eachother.
5252
SetDummyCAPCClusterVars()
53+
SetDummyCAPIClusterVars()
5354
SetDummyCSMachineTemplateVars()
5455
SetDummyCSMachineVars()
55-
SetDummyCAPIClusterVars()
5656
SetDummyTagVars()
5757
}
5858

5959
// SetDummyClusterSpecVars resets the values in each of the exported CloudStackMachines related dummy variables.
6060
func SetDummyCSMachineTemplateVars() {
61+
DomainID = "FakeDomainId"
6162
CSMachineTemplate1 = &infrav1.CloudStackMachineTemplate{
6263
TypeMeta: metav1.TypeMeta{
6364
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
@@ -91,6 +92,7 @@ func SetDummyCSMachineTemplateVars() {
9192

9293
// SetDummyClusterSpecVars resets the values in each of the exported CloudStackMachines related dummy variables.
9394
func SetDummyCSMachineVars() {
95+
DomainID = "FakeDomainId"
9496
CSMachine1 = &infrav1.CloudStackMachine{
9597
TypeMeta: metav1.TypeMeta{
9698
APIVersion: CSApiVersion,
@@ -127,24 +129,29 @@ func SetDummyCAPCClusterVars() {
127129
Name: "FakeAffinityGroup",
128130
Type: cloud.AffinityGroupType}
129131
Net1 = infrav1.Network{Name: "SharedGuestNet1"}
132+
Net2 = infrav1.Network{Name: "SharedGuestNet2"}
130133
Zone1 = infrav1.Zone{Name: "Zone1", Network: Net1}
131134
Zone2 = infrav1.Zone{Name: "Zone2", Network: Net2}
135+
132136
CSCluster = &infrav1.CloudStackCluster{
137+
TypeMeta: metav1.TypeMeta{
138+
APIVersion: CSApiVersion,
139+
Kind: CSClusterKind,
140+
},
141+
ObjectMeta: metav1.ObjectMeta{
142+
Name: CSClusterName,
143+
Namespace: "default",
144+
UID: "0",
145+
},
133146
Spec: infrav1.CloudStackClusterSpec{
134147
IdentityRef: &infrav1.CloudStackIdentityReference{
135148
Kind: "Secret",
136149
Name: "IdentitySecret",
137150
},
138-
Zones: []infrav1.Zone{Zone1, Zone2}},
139-
ObjectMeta: metav1.ObjectMeta{
140-
GenerateName: "cs-cluster-test1-",
141-
UID: "0",
142-
Namespace: "default"},
143-
TypeMeta: metav1.TypeMeta{
144-
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
145-
Kind: "CloudStackCluster"}}
146-
CAPIMachine = &capiv1.Machine{}
147-
DomainID = "FakeDomainId"
151+
ControlPlaneEndpoint: clusterv1.APIEndpoint{Host: "EndpointHost", Port: int32(8675309)},
152+
Zones: []infrav1.Zone{Zone1, Zone2},
153+
},
154+
}
148155
}
149156

150157
// SetDummyCapiCluster resets the values in each of the exported CAPICluster related dummy variables.

0 commit comments

Comments
 (0)