Skip to content

Commit 8c0d1f8

Browse files
⚠️ decouple webhooks from APIs
1 parent 0dcb3a2 commit 8c0d1f8

File tree

85 files changed

+751
-1495
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

85 files changed

+751
-1495
lines changed

docs/book/src/cronjob-tutorial/testdata/project/api/v1/zz_generated.deepcopy.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/book/src/cronjob-tutorial/testdata/project/cmd/main.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838

3939
batchv1 "tutorial.kubebuilder.io/project/api/v1"
4040
"tutorial.kubebuilder.io/project/internal/controller"
41+
webhooksv1 "tutorial.kubebuilder.io/project/internal/webhooks/v1"
4142
// +kubebuilder:scaffold:imports
4243
)
4344

@@ -183,7 +184,7 @@ func main() {
183184
*/
184185
// nolint:goconst
185186
if os.Getenv("ENABLE_WEBHOOKS") != "false" {
186-
if err = (&batchv1.CronJob{}).SetupWebhookWithManager(mgr); err != nil {
187+
if err = webhooksv1.SetupCronJobWebhookWithManager(mgr); err != nil {
187188
setupLog.Error(err, "unable to create webhook", "webhook", "CronJob")
188189
os.Exit(1)
189190
}

docs/book/src/cronjob-tutorial/testdata/project/api/v1/cronjob_webhook.go renamed to docs/book/src/cronjob-tutorial/testdata/project/internal/webhooks/v1/cronjob_webhook.go

Lines changed: 39 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ import (
3131
logf "sigs.k8s.io/controller-runtime/pkg/log"
3232
"sigs.k8s.io/controller-runtime/pkg/webhook"
3333
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
34+
35+
batchv1 "tutorial.kubebuilder.io/project/api/v1"
3436
)
3537

3638
// +kubebuilder:docs-gen:collapse=Go imports
@@ -45,13 +47,12 @@ var cronjoblog = logf.Log.WithName("cronjob-resource")
4547
Then, we set up the webhook with the manager.
4648
*/
4749

48-
// SetupWebhookWithManager will setup the manager to manage the webhooks.
49-
func (r *CronJob) SetupWebhookWithManager(mgr ctrl.Manager) error {
50-
return ctrl.NewWebhookManagedBy(mgr).
51-
For(r).
50+
// SetupCronJobWebhookWithManager registers the webhook for CronJob in the manager.
51+
func SetupCronJobWebhookWithManager(mgr ctrl.Manager) error {
52+
return ctrl.NewWebhookManagedBy(mgr).For(&batchv1.CronJob{}).
5253
WithValidator(&CronJobCustomValidator{}).
5354
WithDefaulter(&CronJobCustomDefaulter{
54-
DefaultConcurrencyPolicy: AllowConcurrent,
55+
DefaultConcurrencyPolicy: batchv1.AllowConcurrent,
5556
DefaultSuspend: false,
5657
DefaultSuccessfulJobsHistoryLimit: 3,
5758
DefaultFailedJobsHistoryLimit: 1,
@@ -81,7 +82,7 @@ This marker is responsible for generating a mutation webhook manifest.
8182
type CronJobCustomDefaulter struct {
8283

8384
// Default values for various CronJob fields
84-
DefaultConcurrencyPolicy ConcurrencyPolicy
85+
DefaultConcurrencyPolicy batchv1.ConcurrencyPolicy
8586
DefaultSuspend bool
8687
DefaultSuccessfulJobsHistoryLimit int32
8788
DefaultFailedJobsHistoryLimit int32
@@ -98,32 +99,34 @@ The `Default`method is expected to mutate the receiver, setting the defaults.
9899

99100
// Default implements webhook.CustomDefaulter so a webhook will be registered for the Kind CronJob.
100101
func (d *CronJobCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error {
101-
cronjob, ok := obj.(*CronJob)
102+
cronjob, ok := obj.(*batchv1.CronJob)
103+
102104
if !ok {
103105
return fmt.Errorf("expected an CronJob object but got %T", obj)
104106
}
105107
cronjoblog.Info("Defaulting for CronJob", "name", cronjob.GetName())
106108

107109
// Set default values
108-
cronjob.Default()
109-
110+
d.applyDefaults(cronjob)
110111
return nil
111112
}
112113

113-
func (r *CronJob) Default() {
114-
if r.Spec.ConcurrencyPolicy == "" {
115-
r.Spec.ConcurrencyPolicy = AllowConcurrent
114+
// applyDefaults applies default values to CronJob fields.
115+
func (d *CronJobCustomDefaulter) applyDefaults(cronJob *batchv1.CronJob) {
116+
if cronJob.Spec.ConcurrencyPolicy == "" {
117+
cronJob.Spec.ConcurrencyPolicy = d.DefaultConcurrencyPolicy
116118
}
117-
if r.Spec.Suspend == nil {
118-
r.Spec.Suspend = new(bool)
119+
if cronJob.Spec.Suspend == nil {
120+
cronJob.Spec.Suspend = new(bool)
121+
*cronJob.Spec.Suspend = d.DefaultSuspend
119122
}
120-
if r.Spec.SuccessfulJobsHistoryLimit == nil {
121-
r.Spec.SuccessfulJobsHistoryLimit = new(int32)
122-
*r.Spec.SuccessfulJobsHistoryLimit = 3
123+
if cronJob.Spec.SuccessfulJobsHistoryLimit == nil {
124+
cronJob.Spec.SuccessfulJobsHistoryLimit = new(int32)
125+
*cronJob.Spec.SuccessfulJobsHistoryLimit = d.DefaultSuccessfulJobsHistoryLimit
123126
}
124-
if r.Spec.FailedJobsHistoryLimit == nil {
125-
r.Spec.FailedJobsHistoryLimit = new(int32)
126-
*r.Spec.FailedJobsHistoryLimit = 1
127+
if cronJob.Spec.FailedJobsHistoryLimit == nil {
128+
cronJob.Spec.FailedJobsHistoryLimit = new(int32)
129+
*cronJob.Spec.FailedJobsHistoryLimit = d.DefaultFailedJobsHistoryLimit
127130
}
128131
}
129132

@@ -168,29 +171,29 @@ var _ webhook.CustomValidator = &CronJobCustomValidator{}
168171

169172
// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type CronJob.
170173
func (v *CronJobCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
171-
cronjob, ok := obj.(*CronJob)
174+
cronjob, ok := obj.(*batchv1.CronJob)
172175
if !ok {
173176
return nil, fmt.Errorf("expected a CronJob object but got %T", obj)
174177
}
175178
cronjoblog.Info("Validation for CronJob upon creation", "name", cronjob.GetName())
176179

177-
return nil, cronjob.validateCronJob()
180+
return nil, validateCronJob(cronjob)
178181
}
179182

180183
// ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type CronJob.
181184
func (v *CronJobCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
182-
cronjob, ok := newObj.(*CronJob)
185+
cronjob, ok := newObj.(*batchv1.CronJob)
183186
if !ok {
184-
return nil, fmt.Errorf("expected a CronJob object but got %T", newObj)
187+
return nil, fmt.Errorf("expected a CronJob object for the newObj but got got %T", newObj)
185188
}
186189
cronjoblog.Info("Validation for CronJob upon update", "name", cronjob.GetName())
187190

188-
return nil, cronjob.validateCronJob()
191+
return nil, validateCronJob(cronjob)
189192
}
190193

191194
// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type CronJob.
192195
func (v *CronJobCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
193-
cronjob, ok := obj.(*CronJob)
196+
cronjob, ok := obj.(*batchv1.CronJob)
194197
if !ok {
195198
return nil, fmt.Errorf("expected a CronJob object but got %T", obj)
196199
}
@@ -205,12 +208,13 @@ func (v *CronJobCustomValidator) ValidateDelete(ctx context.Context, obj runtime
205208
We validate the name and the spec of the CronJob.
206209
*/
207210

208-
func (r *CronJob) validateCronJob() error {
211+
// validateCronJob validates the fields of a CronJob object.
212+
func validateCronJob(cronjob *batchv1.CronJob) error {
209213
var allErrs field.ErrorList
210-
if err := r.validateCronJobName(); err != nil {
214+
if err := validateCronJobName(cronjob); err != nil {
211215
allErrs = append(allErrs, err)
212216
}
213-
if err := r.validateCronJobSpec(); err != nil {
217+
if err := validateCronJobSpec(cronjob); err != nil {
214218
allErrs = append(allErrs, err)
215219
}
216220
if len(allErrs) == 0 {
@@ -219,7 +223,7 @@ func (r *CronJob) validateCronJob() error {
219223

220224
return apierrors.NewInvalid(
221225
schema.GroupKind{Group: "batch.tutorial.kubebuilder.io", Kind: "CronJob"},
222-
r.Name, allErrs)
226+
cronjob.Name, allErrs)
223227
}
224228

225229
/*
@@ -232,11 +236,11 @@ declaring validation by running `controller-gen crd -w`,
232236
or [here](/reference/markers/crd-validation.md).
233237
*/
234238

235-
func (r *CronJob) validateCronJobSpec() *field.Error {
239+
func validateCronJobSpec(cronjob *batchv1.CronJob) *field.Error {
236240
// The field helpers from the kubernetes API machinery help us return nicely
237241
// structured validation errors.
238242
return validateScheduleFormat(
239-
r.Spec.Schedule,
243+
cronjob.Spec.Schedule,
240244
field.NewPath("spec").Child("schedule"))
241245
}
242246

@@ -261,15 +265,15 @@ the apimachinery repo, so we can't declaratively validate it using
261265
the validation schema.
262266
*/
263267

264-
func (r *CronJob) validateCronJobName() *field.Error {
265-
if len(r.ObjectMeta.Name) > validationutils.DNS1035LabelMaxLength-11 {
268+
func validateCronJobName(cronjob *batchv1.CronJob) *field.Error {
269+
if len(cronjob.ObjectMeta.Name) > validationutils.DNS1035LabelMaxLength-11 {
266270
// The job name length is 63 characters like all Kubernetes objects
267271
// (which must fit in a DNS subdomain). The cronjob controller appends
268272
// a 11-character suffix to the cronjob (`-$TIMESTAMP`) when creating
269273
// a job. The job name length limit is 63 characters. Therefore cronjob
270274
// names must have length <= 63-11=52. If we don't validate this here,
271275
// then job creation will fail later.
272-
return field.Invalid(field.NewPath("metadata").Child("name"), r.ObjectMeta.Name, "must be no more than 52 characters")
276+
return field.Invalid(field.NewPath("metadata").Child("name"), cronjob.ObjectMeta.Name, "must be no more than 52 characters")
273277
}
274278
return nil
275279
}

docs/book/src/cronjob-tutorial/testdata/project/api/v1/cronjob_webhook_test.go renamed to docs/book/src/cronjob-tutorial/testdata/project/internal/webhooks/v1/cronjob_webhook_test.go

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,32 +19,35 @@ package v1
1919
import (
2020
. "github.com/onsi/ginkgo/v2"
2121
. "github.com/onsi/gomega"
22+
23+
batchv1 "tutorial.kubebuilder.io/project/api/v1"
2224
// TODO (user): Add any additional imports if needed
2325
)
2426

2527
var _ = Describe("CronJob Webhook", func() {
2628
var (
27-
obj *CronJob
28-
oldObj *CronJob
29+
obj = &batchv1.CronJob{}
30+
oldObj = &batchv1.CronJob{}
2931
validator CronJobCustomValidator
32+
defaulter CronJobCustomDefaulter
3033
)
3134

3235
BeforeEach(func() {
33-
obj = &CronJob{
34-
Spec: CronJobSpec{
36+
obj = &batchv1.CronJob{
37+
Spec: batchv1.CronJobSpec{
3538
Schedule: "*/5 * * * *",
36-
ConcurrencyPolicy: AllowConcurrent,
39+
ConcurrencyPolicy: batchv1.AllowConcurrent,
3740
SuccessfulJobsHistoryLimit: new(int32),
3841
FailedJobsHistoryLimit: new(int32),
3942
},
4043
}
4144
*obj.Spec.SuccessfulJobsHistoryLimit = 3
4245
*obj.Spec.FailedJobsHistoryLimit = 1
4346

44-
oldObj = &CronJob{
45-
Spec: CronJobSpec{
47+
oldObj = &batchv1.CronJob{
48+
Spec: batchv1.CronJobSpec{
4649
Schedule: "*/5 * * * *",
47-
ConcurrencyPolicy: AllowConcurrent,
50+
ConcurrencyPolicy: batchv1.AllowConcurrent,
4851
SuccessfulJobsHistoryLimit: new(int32),
4952
FailedJobsHistoryLimit: new(int32),
5053
},
@@ -53,6 +56,12 @@ var _ = Describe("CronJob Webhook", func() {
5356
*oldObj.Spec.FailedJobsHistoryLimit = 1
5457

5558
validator = CronJobCustomValidator{}
59+
defaulter = CronJobCustomDefaulter{
60+
DefaultConcurrencyPolicy: batchv1.AllowConcurrent,
61+
DefaultSuspend: false,
62+
DefaultSuccessfulJobsHistoryLimit: 3,
63+
DefaultFailedJobsHistoryLimit: 1,
64+
}
5665

5766
Expect(obj).NotTo(BeNil(), "Expected obj to be initialized")
5867
Expect(oldObj).NotTo(BeNil(), "Expected oldObj to be initialized")
@@ -71,18 +80,18 @@ var _ = Describe("CronJob Webhook", func() {
7180
obj.Spec.FailedJobsHistoryLimit = nil // This should default to 1
7281

7382
By("calling the Default method to apply defaults")
74-
obj.Default()
83+
defaulter.Default(ctx, obj)
7584

7685
By("checking that the default values are set")
77-
Expect(obj.Spec.ConcurrencyPolicy).To(Equal(AllowConcurrent), "Expected ConcurrencyPolicy to default to AllowConcurrent")
86+
Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.AllowConcurrent), "Expected ConcurrencyPolicy to default to AllowConcurrent")
7887
Expect(*obj.Spec.Suspend).To(BeFalse(), "Expected Suspend to default to false")
7988
Expect(*obj.Spec.SuccessfulJobsHistoryLimit).To(Equal(int32(3)), "Expected SuccessfulJobsHistoryLimit to default to 3")
8089
Expect(*obj.Spec.FailedJobsHistoryLimit).To(Equal(int32(1)), "Expected FailedJobsHistoryLimit to default to 1")
8190
})
8291

8392
It("Should not overwrite fields that are already set", func() {
8493
By("setting fields that would normally get a default")
85-
obj.Spec.ConcurrencyPolicy = ForbidConcurrent
94+
obj.Spec.ConcurrencyPolicy = batchv1.ForbidConcurrent
8695
obj.Spec.Suspend = new(bool)
8796
*obj.Spec.Suspend = true
8897
obj.Spec.SuccessfulJobsHistoryLimit = new(int32)
@@ -91,10 +100,10 @@ var _ = Describe("CronJob Webhook", func() {
91100
*obj.Spec.FailedJobsHistoryLimit = 2
92101

93102
By("calling the Default method to apply defaults")
94-
obj.Default()
103+
defaulter.Default(ctx, obj)
95104

96105
By("checking that the fields were not overwritten")
97-
Expect(obj.Spec.ConcurrencyPolicy).To(Equal(ForbidConcurrent), "Expected ConcurrencyPolicy to retain its set value")
106+
Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.ForbidConcurrent), "Expected ConcurrencyPolicy to retain its set value")
98107
Expect(*obj.Spec.Suspend).To(BeTrue(), "Expected Suspend to retain its set value")
99108
Expect(*obj.Spec.SuccessfulJobsHistoryLimit).To(Equal(int32(5)), "Expected SuccessfulJobsHistoryLimit to retain its set value")
100109
Expect(*obj.Spec.FailedJobsHistoryLimit).To(Equal(int32(2)), "Expected FailedJobsHistoryLimit to retain its set value")

docs/book/src/cronjob-tutorial/testdata/project/api/v1/webhook_suite_test.go renamed to docs/book/src/cronjob-tutorial/testdata/project/internal/webhooks/v1/webhook_suite_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ var _ = BeforeSuite(func() {
8989
Expect(cfg).NotTo(BeNil())
9090

9191
scheme := apimachineryruntime.NewScheme()
92-
err = AddToScheme(scheme)
92+
err = admissionv1.AddToScheme(scheme)
9393
Expect(err).NotTo(HaveOccurred())
9494

9595
err = admissionv1.AddToScheme(scheme)
@@ -115,7 +115,7 @@ var _ = BeforeSuite(func() {
115115
})
116116
Expect(err).NotTo(HaveOccurred())
117117

118-
err = (&CronJob{}).SetupWebhookWithManager(mgr)
118+
err = SetupCronJobWebhookWithManager(mgr)
119119
Expect(err).NotTo(HaveOccurred())
120120

121121
// +kubebuilder:scaffold:webhook

docs/book/src/multiversion-tutorial/testdata/project/api/v1/zz_generated.deepcopy.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/book/src/multiversion-tutorial/testdata/project/api/v2/zz_generated.deepcopy.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/book/src/multiversion-tutorial/testdata/project/cmd/main.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ import (
4040
batchv1 "tutorial.kubebuilder.io/project/api/v1"
4141
batchv2 "tutorial.kubebuilder.io/project/api/v2"
4242
"tutorial.kubebuilder.io/project/internal/controller"
43+
webhooksv1 "tutorial.kubebuilder.io/project/internal/webhooks/v1"
44+
webhooksv2 "tutorial.kubebuilder.io/project/internal/webhooks/v2"
4345
// +kubebuilder:scaffold:imports
4446
)
4547

@@ -175,14 +177,14 @@ func main() {
175177
*/
176178
// nolint:goconst
177179
if os.Getenv("ENABLE_WEBHOOKS") != "false" {
178-
if err = (&batchv1.CronJob{}).SetupWebhookWithManager(mgr); err != nil {
180+
if err = webhooksv1.SetupCronJobWebhookWithManager(mgr); err != nil {
179181
setupLog.Error(err, "unable to create webhook", "webhook", "CronJob")
180182
os.Exit(1)
181183
}
182184
}
183185
// nolint:goconst
184186
if os.Getenv("ENABLE_WEBHOOKS") != "false" {
185-
if err = (&batchv2.CronJob{}).SetupWebhookWithManager(mgr); err != nil {
187+
if err = webhooksv2.SetupCronJobWebhookWithManager(mgr); err != nil {
186188
setupLog.Error(err, "unable to create webhook", "webhook", "CronJob")
187189
os.Exit(1)
188190
}

docs/book/src/multiversion-tutorial/testdata/project/api/v1/cronjob_conversion.go renamed to docs/book/src/multiversion-tutorial/testdata/project/internal/webhooks/v1/cronjob_conversion.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,18 @@ method called `Hub()` to serve as a
2222
We could also just put this inline in our `cronjob_types.go` file.
2323
*/
2424

25+
import (
26+
batchv1 "tutorial.kubebuilder.io/project/api/v1" // Import CronJob from API package
27+
)
28+
29+
/*
30+
ConversionCronJob wraps the API's CronJob and implements the Hub interface for conversion.
31+
*/
32+
33+
// ConversionCronJob wraps the existing CronJob type from the API.
34+
type ConversionCronJob struct {
35+
batchv1.CronJob // Embed the original CronJob type
36+
}
37+
2538
// Hub marks this type as a conversion hub.
26-
func (*CronJob) Hub() {}
39+
func (*ConversionCronJob) Hub() {}

0 commit comments

Comments
 (0)