Skip to content

Commit defede3

Browse files
camilamacedo86camilamacedo86
authored
✨ (go/v4): improve the webhook tests by adding examples. Also, improve cronjob tutorial to clarify its usage and validate the changes (#4130)
improve the webhook tests by adding examples In this PR we are improving the webhook tests by adding further info and examples for the users. Either we are implementing them further for the cronjob tutorial as an example and to help us to validate and spot issues on related areas when/if we need to change them
1 parent df4e408 commit defede3

File tree

19 files changed

+933
-187
lines changed

19 files changed

+933
-187
lines changed

‎docs/book/src/cronjob-tutorial/testdata/project/api/v1/cronjob_webhook.go

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -100,22 +100,27 @@ func (d *CronJobCustomDefaulter) Default(ctx context.Context, obj runtime.Object
100100
}
101101
cronjoblog.Info("Defaulting for CronJob", "name", cronjob.GetName())
102102

103-
if cronjob.Spec.ConcurrencyPolicy == "" {
104-
cronjob.Spec.ConcurrencyPolicy = AllowConcurrent
103+
// Set default values
104+
cronjob.Default()
105+
106+
return nil
107+
}
108+
109+
func (r *CronJob) Default() {
110+
if r.Spec.ConcurrencyPolicy == "" {
111+
r.Spec.ConcurrencyPolicy = AllowConcurrent
105112
}
106-
if cronjob.Spec.Suspend == nil {
107-
cronjob.Spec.Suspend = new(bool)
113+
if r.Spec.Suspend == nil {
114+
r.Spec.Suspend = new(bool)
108115
}
109-
if cronjob.Spec.SuccessfulJobsHistoryLimit == nil {
110-
cronjob.Spec.SuccessfulJobsHistoryLimit = new(int32)
111-
*cronjob.Spec.SuccessfulJobsHistoryLimit = 3
116+
if r.Spec.SuccessfulJobsHistoryLimit == nil {
117+
r.Spec.SuccessfulJobsHistoryLimit = new(int32)
118+
*r.Spec.SuccessfulJobsHistoryLimit = 3
112119
}
113-
if cronjob.Spec.FailedJobsHistoryLimit == nil {
114-
cronjob.Spec.FailedJobsHistoryLimit = new(int32)
115-
*cronjob.Spec.FailedJobsHistoryLimit = 1
120+
if r.Spec.FailedJobsHistoryLimit == nil {
121+
r.Spec.FailedJobsHistoryLimit = new(int32)
122+
*r.Spec.FailedJobsHistoryLimit = 1
116123
}
117-
118-
return nil
119124
}
120125

121126
/*

‎docs/book/src/cronjob-tutorial/testdata/project/api/v1/cronjob_webhook_test.go

Lines changed: 125 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,29 +18,146 @@ package v1
1818

1919
import (
2020
. "github.com/onsi/ginkgo/v2"
21+
. "github.com/onsi/gomega"
22+
// TODO (user): Add any additional imports if needed
2123
)
2224

2325
var _ = Describe("CronJob Webhook", func() {
26+
var (
27+
obj *CronJob
28+
oldObj *CronJob
29+
validator CronJobCustomValidator
30+
)
31+
32+
BeforeEach(func() {
33+
obj = &CronJob{
34+
Spec: CronJobSpec{
35+
Schedule: "*/5 * * * *",
36+
ConcurrencyPolicy: AllowConcurrent,
37+
SuccessfulJobsHistoryLimit: new(int32),
38+
FailedJobsHistoryLimit: new(int32),
39+
},
40+
}
41+
*obj.Spec.SuccessfulJobsHistoryLimit = 3
42+
*obj.Spec.FailedJobsHistoryLimit = 1
43+
44+
oldObj = &CronJob{
45+
Spec: CronJobSpec{
46+
Schedule: "*/5 * * * *",
47+
ConcurrencyPolicy: AllowConcurrent,
48+
SuccessfulJobsHistoryLimit: new(int32),
49+
FailedJobsHistoryLimit: new(int32),
50+
},
51+
}
52+
*oldObj.Spec.SuccessfulJobsHistoryLimit = 3
53+
*oldObj.Spec.FailedJobsHistoryLimit = 1
54+
55+
validator = CronJobCustomValidator{}
56+
57+
Expect(obj).NotTo(BeNil(), "Expected obj to be initialized")
58+
Expect(oldObj).NotTo(BeNil(), "Expected oldObj to be initialized")
59+
})
2460

25-
Context("When creating CronJob under Defaulting Webhook", func() {
26-
It("Should fill in the default value if a required field is empty", func() {
61+
AfterEach(func() {
62+
// TODO (user): Add any teardown logic common to all tests
63+
})
2764

28-
// TODO(user): Add your logic here
65+
Context("When creating CronJob under Defaulting Webhook", func() {
66+
It("Should apply defaults when a required field is empty", func() {
67+
By("simulating a scenario where defaults should be applied")
68+
obj.Spec.ConcurrencyPolicy = "" // This should default to AllowConcurrent
69+
obj.Spec.Suspend = nil // This should default to false
70+
obj.Spec.SuccessfulJobsHistoryLimit = nil // This should default to 3
71+
obj.Spec.FailedJobsHistoryLimit = nil // This should default to 1
72+
73+
By("calling the Default method to apply defaults")
74+
obj.Default()
75+
76+
By("checking that the default values are set")
77+
Expect(obj.Spec.ConcurrencyPolicy).To(Equal(AllowConcurrent), "Expected ConcurrencyPolicy to default to AllowConcurrent")
78+
Expect(*obj.Spec.Suspend).To(BeFalse(), "Expected Suspend to default to false")
79+
Expect(*obj.Spec.SuccessfulJobsHistoryLimit).To(Equal(int32(3)), "Expected SuccessfulJobsHistoryLimit to default to 3")
80+
Expect(*obj.Spec.FailedJobsHistoryLimit).To(Equal(int32(1)), "Expected FailedJobsHistoryLimit to default to 1")
81+
})
2982

83+
It("Should not overwrite fields that are already set", func() {
84+
By("setting fields that would normally get a default")
85+
obj.Spec.ConcurrencyPolicy = ForbidConcurrent
86+
obj.Spec.Suspend = new(bool)
87+
*obj.Spec.Suspend = true
88+
obj.Spec.SuccessfulJobsHistoryLimit = new(int32)
89+
*obj.Spec.SuccessfulJobsHistoryLimit = 5
90+
obj.Spec.FailedJobsHistoryLimit = new(int32)
91+
*obj.Spec.FailedJobsHistoryLimit = 2
92+
93+
By("calling the Default method to apply defaults")
94+
obj.Default()
95+
96+
By("checking that the fields were not overwritten")
97+
Expect(obj.Spec.ConcurrencyPolicy).To(Equal(ForbidConcurrent), "Expected ConcurrencyPolicy to retain its set value")
98+
Expect(*obj.Spec.Suspend).To(BeTrue(), "Expected Suspend to retain its set value")
99+
Expect(*obj.Spec.SuccessfulJobsHistoryLimit).To(Equal(int32(5)), "Expected SuccessfulJobsHistoryLimit to retain its set value")
100+
Expect(*obj.Spec.FailedJobsHistoryLimit).To(Equal(int32(2)), "Expected FailedJobsHistoryLimit to retain its set value")
30101
})
31102
})
32103

33-
Context("When creating CronJob under Validating Webhook", func() {
34-
It("Should deny if a required field is empty", func() {
104+
Context("When creating or updating CronJob under Validating Webhook", func() {
105+
It("Should deny creation if the name is too long", func() {
106+
obj.ObjectMeta.Name = "this-name-is-way-too-long-and-should-fail-validation-because-it-is-way-too-long"
107+
warnings, err := validator.ValidateCreate(ctx, obj)
108+
Expect(err).To(HaveOccurred(), "Expected name validation to fail for a too-long name")
109+
Expect(warnings).To(BeNil())
110+
Expect(err.Error()).To(ContainSubstring("must be no more than 52 characters"))
111+
})
112+
113+
It("Should admit creation if the name is valid", func() {
114+
obj.ObjectMeta.Name = "valid-cronjob-name"
115+
warnings, err := validator.ValidateCreate(ctx, obj)
116+
Expect(err).NotTo(HaveOccurred(), "Expected name validation to pass for a valid name")
117+
Expect(warnings).To(BeNil())
118+
})
119+
120+
It("Should deny creation if the schedule is invalid", func() {
121+
obj.Spec.Schedule = "invalid-cron-schedule"
122+
warnings, err := validator.ValidateCreate(ctx, obj)
123+
Expect(err).To(HaveOccurred(), "Expected spec validation to fail for an invalid schedule")
124+
Expect(warnings).To(BeNil())
125+
Expect(err.Error()).To(ContainSubstring("Expected exactly 5 fields, found 1: invalid-cron-schedule"))
126+
})
127+
128+
It("Should admit creation if the schedule is valid", func() {
129+
obj.Spec.Schedule = "*/5 * * * *"
130+
warnings, err := validator.ValidateCreate(ctx, obj)
131+
Expect(err).NotTo(HaveOccurred(), "Expected spec validation to pass for a valid schedule")
132+
Expect(warnings).To(BeNil())
133+
})
134+
135+
It("Should deny update if both name and spec are invalid", func() {
136+
oldObj.ObjectMeta.Name = "valid-cronjob-name"
137+
oldObj.Spec.Schedule = "*/5 * * * *"
35138

36-
// TODO(user): Add your logic here
139+
By("simulating an update")
140+
obj.ObjectMeta.Name = "this-name-is-way-too-long-and-should-fail-validation-because-it-is-way-too-long"
141+
obj.Spec.Schedule = "invalid-cron-schedule"
37142

143+
By("validating an update")
144+
warnings, err := validator.ValidateUpdate(ctx, oldObj, obj)
145+
Expect(err).To(HaveOccurred(), "Expected validation to fail for both name and spec")
146+
Expect(warnings).To(BeNil())
38147
})
39148

40-
It("Should admit if all required fields are provided", func() {
149+
It("Should admit update if both name and spec are valid", func() {
150+
oldObj.ObjectMeta.Name = "valid-cronjob-name"
151+
oldObj.Spec.Schedule = "*/5 * * * *"
41152

42-
// TODO(user): Add your logic here
153+
By("simulating an update")
154+
obj.ObjectMeta.Name = "valid-cronjob-name-updated"
155+
obj.Spec.Schedule = "0 0 * * *"
43156

157+
By("validating an update")
158+
warnings, err := validator.ValidateUpdate(ctx, oldObj, obj)
159+
Expect(err).NotTo(HaveOccurred(), "Expected validation to pass for a valid update")
160+
Expect(warnings).To(BeNil())
44161
})
45162
})
46163

‎hack/docs/internal/cronjob-tutorial/generate_cronjob.go

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -85,26 +85,28 @@ func (sp *Sample) UpdateTutorial() {
8585
sp.updateSpec()
8686
// 2. update webhook
8787
sp.updateWebhook()
88-
// 3. update makefile
88+
// 3. update webhookTests
89+
sp.updateWebhookTests()
90+
// 4. update makefile
8991
sp.updateMakefile()
90-
// 4. generate extra files
92+
// 5. generate extra files
9193
sp.codeGen()
92-
// 5. compensate other intro in API
94+
// 6. compensate other intro in API
9395
sp.updateAPIStuff()
94-
// 6. update reconciliation and main.go
95-
// 6.1 update controller
96+
// 7. update reconciliation and main.go
97+
// 7.1 update controller
9698
sp.updateController()
97-
// 6.2 update main.go
99+
// 7.2 update main.go
98100
sp.updateMain()
99-
// 7. generate extra files
101+
// 8. generate extra files
100102
sp.codeGen()
101-
// 8. update suite_test explanation
103+
// 9. update suite_test explanation
102104
sp.updateSuiteTest()
103-
// 9. uncomment kustomization
105+
// 10. uncomment kustomization
104106
sp.updateKustomization()
105-
// 10. add example
107+
// 11. add example
106108
sp.updateExample()
107-
// 11. add test
109+
// 12. add test
108110
sp.addControllerTest()
109111
}
110112

@@ -372,6 +374,34 @@ manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and Cust
372374

373375
}
374376

377+
func (sp *Sample) updateWebhookTests() {
378+
file := filepath.Join(sp.ctx.Dir, "api/v1/cronjob_webhook_test.go")
379+
380+
err := pluginutil.InsertCode(file,
381+
`var _ = Describe("CronJob Webhook", func() {
382+
var (
383+
obj *CronJob`,
384+
`
385+
oldObj *CronJob
386+
validator CronJobCustomValidator`)
387+
hackutils.CheckError("insert global vars", err)
388+
389+
err = pluginutil.ReplaceInFile(file,
390+
webhookTestCreateDefaultingFragment,
391+
webhookTestCreateDefaultingReplaceFragment)
392+
hackutils.CheckError("replace create defaulting test", err)
393+
394+
err = pluginutil.ReplaceInFile(file,
395+
webhookTestingValidatingTodoFragment,
396+
webhookTestingValidatingExampleFragment)
397+
hackutils.CheckError("replace validating defaulting test", err)
398+
399+
err = pluginutil.ReplaceInFile(file,
400+
webhookTestsBeforeEachOriginal,
401+
webhookTestsBeforeEachChanged)
402+
hackutils.CheckError("replace validating defaulting test", err)
403+
}
404+
375405
func (sp *Sample) updateWebhook() {
376406
var err error
377407
err = pluginutil.InsertCode(

0 commit comments

Comments
 (0)