Skip to content

Commit be97e19

Browse files
camilamacedo86camilamacedo86
committed
fix: improve CronJob and multiversion tutorial by refining replaces without overwriting webhook markers
Ensure we are more precise with operations to avoid overwriting the default scaffolding. Previously, the markers in the scaffold were unintentionally overwritten when injecting code and documentation, which could override crucial changes. This commit ensures the fix applied in PR #4122 is preserved by refining the tutorial instructions to avoid overwriting webhook markers while still injecting the necessary information. Either we should not export the consts used in the hack/docs since those are internal implementations, therefore the consts used in this area affected have their name changed
1 parent f1a44b0 commit be97e19

File tree

6 files changed

+71
-161
lines changed

6 files changed

+71
-161
lines changed

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

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,8 @@ This marker is responsible for generating a mutating webhook manifest.
6666
The meaning of each marker can be found [here](/reference/markers/webhook.md).
6767
*/
6868

69-
// +kubebuilder:webhook:path=/mutate-batch-tutorial-kubebuilder-io-v1-cronjob,mutating=true,failurePolicy=fail,groups=batch.tutorial.kubebuilder.io,resources=cronjobs,verbs=create;update,versions=v1,name=mcronjob.kb.io,sideEffects=None,admissionReviewVersions=v1
70-
7169
/*
72-
We use the `webhook.CustomDefaulter` interface to set defaults to our CRD.
73-
A webhook will automatically be served that calls this defaulting.
74-
75-
The `Default` method is expected to mutate the receiver, setting the defaults.
70+
This marker is responsible for generating a mutation webhook manifest.
7671
*/
7772

7873
// +kubebuilder:webhook:path=/mutate-batch-tutorial-kubebuilder-io-v1-cronjob,mutating=true,failurePolicy=fail,sideEffects=None,groups=batch.tutorial.kubebuilder.io,resources=cronjobs,verbs=create;update,versions=v1,name=mcronjob-v1.kb.io,admissionReviewVersions=v1
@@ -94,6 +89,13 @@ type CronJobCustomDefaulter struct {
9489

9590
var _ webhook.CustomDefaulter = &CronJobCustomDefaulter{}
9691

92+
/*
93+
We use the `webhook.CustomDefaulter`interface to set defaults to our CRD.
94+
A webhook will automatically be served that calls this defaulting.
95+
96+
The `Default`method is expected to mutate the receiver, setting the defaults.
97+
*/
98+
9799
// Default implements webhook.CustomDefaulter so a webhook will be registered for the Kind CronJob.
98100
func (d *CronJobCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error {
99101
cronjob, ok := obj.(*CronJob)
@@ -125,12 +127,6 @@ func (r *CronJob) Default() {
125127
}
126128
}
127129

128-
/*
129-
This marker is responsible for generating a validating webhook manifest.
130-
*/
131-
132-
// +kubebuilder:webhook:verbs=create;update;delete,path=/validate-batch-tutorial-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,groups=batch.tutorial.kubebuilder.io,resources=cronjobs,versions=v1,name=vcronjob.kb.io,sideEffects=None,admissionReviewVersions=v1
133-
134130
/*
135131
We can validate our CRD beyond what's possible with declarative
136132
validation. Generally, declarative validation should be sufficient, but
@@ -153,8 +149,9 @@ Here, however, we just use the same shared validation for `ValidateCreate` and
153149
validate anything on deletion.
154150
*/
155151

156-
// NOTE: The 'path' attribute must follow a specific pattern and should not be modified directly here.
157-
// Modifying the path for an invalid path can cause API server errors; failing to locate the webhook.
152+
/*
153+
This marker is responsible for generating a validation webhook manifest.
154+
*/
158155
// +kubebuilder:webhook:path=/validate-batch-tutorial-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,sideEffects=None,groups=batch.tutorial.kubebuilder.io,resources=cronjobs,verbs=create;update,versions=v1,name=vcronjob-v1.kb.io,admissionReviewVersions=v1
159156

160157
// +kubebuilder:object:generate=false

‎docs/book/src/cronjob-tutorial/testdata/project/config/webhook/manifests.yaml‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -24,26 +24,6 @@ webhooks:
2424
resources:
2525
- cronjobs
2626
sideEffects: None
27-
- admissionReviewVersions:
28-
- v1
29-
clientConfig:
30-
service:
31-
name: webhook-service
32-
namespace: system
33-
path: /mutate-batch-tutorial-kubebuilder-io-v1-cronjob
34-
failurePolicy: Fail
35-
name: mcronjob.kb.io
36-
rules:
37-
- apiGroups:
38-
- batch.tutorial.kubebuilder.io
39-
apiVersions:
40-
- v1
41-
operations:
42-
- CREATE
43-
- UPDATE
44-
resources:
45-
- cronjobs
46-
sideEffects: None
4727
---
4828
apiVersion: admissionregistration.k8s.io/v1
4929
kind: ValidatingWebhookConfiguration
@@ -70,24 +50,3 @@ webhooks:
7050
resources:
7151
- cronjobs
7252
sideEffects: None
73-
- admissionReviewVersions:
74-
- v1
75-
clientConfig:
76-
service:
77-
name: webhook-service
78-
namespace: system
79-
path: /validate-batch-tutorial-kubebuilder-io-v1-cronjob
80-
failurePolicy: Fail
81-
name: vcronjob.kb.io
82-
rules:
83-
- apiGroups:
84-
- batch.tutorial.kubebuilder.io
85-
apiVersions:
86-
- v1
87-
operations:
88-
- CREATE
89-
- UPDATE
90-
- DELETE
91-
resources:
92-
- cronjobs
93-
sideEffects: None

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

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,8 @@ This marker is responsible for generating a mutating webhook manifest.
7070
The meaning of each marker can be found [here](/reference/markers/webhook.md).
7171
*/
7272

73-
// +kubebuilder:webhook:path=/mutate-batch-tutorial-kubebuilder-io-v1-cronjob,mutating=true,failurePolicy=fail,groups=batch.tutorial.kubebuilder.io,resources=cronjobs,verbs=create;update,versions=v1,name=mcronjob.kb.io,sideEffects=None,admissionReviewVersions=v1
74-
7573
/*
76-
We use the `webhook.CustomDefaulter` interface to set defaults to our CRD.
77-
A webhook will automatically be served that calls this defaulting.
78-
79-
The `Default` method is expected to mutate the receiver, setting the defaults.
74+
This marker is responsible for generating a mutation webhook manifest.
8075
*/
8176

8277
// +kubebuilder:webhook:path=/mutate-batch-tutorial-kubebuilder-io-v1-cronjob,mutating=true,failurePolicy=fail,sideEffects=None,groups=batch.tutorial.kubebuilder.io,resources=cronjobs,verbs=create;update,versions=v1,name=mcronjob-v1.kb.io,admissionReviewVersions=v1
@@ -98,6 +93,13 @@ type CronJobCustomDefaulter struct {
9893

9994
var _ webhook.CustomDefaulter = &CronJobCustomDefaulter{}
10095

96+
/*
97+
We use the `webhook.CustomDefaulter`interface to set defaults to our CRD.
98+
A webhook will automatically be served that calls this defaulting.
99+
100+
The `Default`method is expected to mutate the receiver, setting the defaults.
101+
*/
102+
101103
// Default implements webhook.CustomDefaulter so a webhook will be registered for the Kind CronJob.
102104
func (d *CronJobCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error {
103105
cronjob, ok := obj.(*CronJob)
@@ -129,12 +131,6 @@ func (r *CronJob) Default() {
129131
}
130132
}
131133

132-
/*
133-
This marker is responsible for generating a validating webhook manifest.
134-
*/
135-
136-
// +kubebuilder:webhook:verbs=create;update;delete,path=/validate-batch-tutorial-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,groups=batch.tutorial.kubebuilder.io,resources=cronjobs,versions=v1,name=vcronjob.kb.io,sideEffects=None,admissionReviewVersions=v1
137-
138134
/*
139135
We can validate our CRD beyond what's possible with declarative
140136
validation. Generally, declarative validation should be sufficient, but
@@ -157,8 +153,9 @@ Here, however, we just use the same shared validation for `ValidateCreate` and
157153
validate anything on deletion.
158154
*/
159155

160-
// NOTE: The 'path' attribute must follow a specific pattern and should not be modified directly here.
161-
// Modifying the path for an invalid path can cause API server errors; failing to locate the webhook.
156+
/*
157+
This marker is responsible for generating a validation webhook manifest.
158+
*/
162159
// +kubebuilder:webhook:path=/validate-batch-tutorial-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,sideEffects=None,groups=batch.tutorial.kubebuilder.io,resources=cronjobs,verbs=create;update,versions=v1,name=vcronjob-v1.kb.io,admissionReviewVersions=v1
163160

164161
// +kubebuilder:object:generate=false

‎docs/book/src/multiversion-tutorial/testdata/project/config/webhook/manifests.yaml‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -24,26 +24,6 @@ webhooks:
2424
resources:
2525
- cronjobs
2626
sideEffects: None
27-
- admissionReviewVersions:
28-
- v1
29-
clientConfig:
30-
service:
31-
name: webhook-service
32-
namespace: system
33-
path: /mutate-batch-tutorial-kubebuilder-io-v1-cronjob
34-
failurePolicy: Fail
35-
name: mcronjob.kb.io
36-
rules:
37-
- apiGroups:
38-
- batch.tutorial.kubebuilder.io
39-
apiVersions:
40-
- v1
41-
operations:
42-
- CREATE
43-
- UPDATE
44-
resources:
45-
- cronjobs
46-
sideEffects: None
4727
- admissionReviewVersions:
4828
- v1
4929
clientConfig:
@@ -90,27 +70,6 @@ webhooks:
9070
resources:
9171
- cronjobs
9272
sideEffects: None
93-
- admissionReviewVersions:
94-
- v1
95-
clientConfig:
96-
service:
97-
name: webhook-service
98-
namespace: system
99-
path: /validate-batch-tutorial-kubebuilder-io-v1-cronjob
100-
failurePolicy: Fail
101-
name: vcronjob.kb.io
102-
rules:
103-
- apiGroups:
104-
- batch.tutorial.kubebuilder.io
105-
apiVersions:
106-
- v1
107-
operations:
108-
- CREATE
109-
- UPDATE
110-
- DELETE
111-
resources:
112-
- cronjobs
113-
sideEffects: None
11473
- admissionReviewVersions:
11574
- v1
11675
clientConfig:

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

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ func (sp *Sample) updateWebhook() {
433433
434434
// nolint:unused
435435
// log is for logging in this package.
436-
`, WebhookIntro)
436+
`, webhookIntro)
437437
hackutils.CheckError("fixing cronjob_webhook.go", err)
438438

439439
err = pluginutil.InsertCode(
@@ -447,8 +447,14 @@ Then, we set up the webhook with the manager.
447447

448448
err = pluginutil.ReplaceInFile(
449449
filepath.Join(sp.ctx.Dir, "api/v1/cronjob_webhook.go"),
450-
`// TODO(user): EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!`, WebhookMarker)
451-
hackutils.CheckError("fixing cronjob_webhook.go by replacing TODO", err)
450+
`// TODO(user): EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!`, webhooksNoticeMarker)
451+
hackutils.CheckError("fixing cronjob_webhook.go by replacing note about path attribute", err)
452+
453+
err = pluginutil.ReplaceInFile(
454+
filepath.Join(sp.ctx.Dir, "api/v1/cronjob_webhook.go"),
455+
`// NOTE: The 'path' attribute must follow a specific pattern and should not be modified directly here.
456+
// Modifying the path for an invalid path can cause API server errors; failing to locate the webhook.`, explanationValidateCRD)
457+
hackutils.CheckError("fixing cronjob_webhook.go by replacing note about path attribute", err)
452458

453459
err = pluginutil.ReplaceInFile(
454460
filepath.Join(sp.ctx.Dir, "api/v1/cronjob_webhook.go"),
@@ -474,48 +480,37 @@ Then, we set up the webhook with the manager.
474480
err = pluginutil.ReplaceInFile(
475481
filepath.Join(sp.ctx.Dir, "api/v1/cronjob_webhook.go"),
476482
`// TODO(user): fill in your defaulting logic.
477-
`, WebhookDefaultingSettings)
483+
484+
return nil
485+
}`, webhookDefaultingSettings)
478486
hackutils.CheckError("fixing cronjob_webhook.go by adding logic", err)
479487

480488
err = pluginutil.ReplaceInFile(
481489
filepath.Join(sp.ctx.Dir, "api/v1/cronjob_webhook.go"),
482490
`// TODO(user): fill in your validation logic upon object creation.
483491
484492
return nil, nil`,
485-
`
486-
return nil, cronjob.validateCronJob()`)
493+
`return nil, cronjob.validateCronJob()`)
487494
hackutils.CheckError("fixing cronjob_webhook.go by fill in your validation", err)
488495

489496
err = pluginutil.ReplaceInFile(
490497
filepath.Join(sp.ctx.Dir, "api/v1/cronjob_webhook.go"),
491498
`// TODO(user): fill in your validation logic upon object update.
492499
493500
return nil, nil`,
494-
`
495-
return nil, cronjob.validateCronJob()`)
501+
`return nil, cronjob.validateCronJob()`)
496502
hackutils.CheckError("fixing cronjob_webhook.go by adding validation logic upon object update", err)
497503

498-
err = pluginutil.InsertCode(
499-
filepath.Join(sp.ctx.Dir, "api/v1/cronjob_webhook.go"),
500-
`// TODO(user): fill in your validation logic upon object deletion.
501-
502-
return nil, nil
503-
}`, WebhookValidateSpec)
504-
505-
hackutils.CheckError("fixing cronjob_webhook.go upon object deletion", err)
506-
507504
err = pluginutil.ReplaceInFile(
508505
filepath.Join(sp.ctx.Dir, "api/v1/cronjob_webhook.go"),
509-
`validate anything on deletion.
510-
*/
511-
512-
return nil
513-
}`, `validate anything on deletion.
514-
*/
515-
516-
`)
517-
hackutils.CheckError("fixing cronjob_webhook.go by removing wrong return nil", err)
506+
`// Default implements webhook.CustomDefaulter so a webhook will be registered for the Kind CronJob.`,
507+
customInterfaceDefaultInfo)
508+
hackutils.CheckError("fixing cronjob_webhook.go by adding validation logic upon object update", err)
518509

510+
err = pluginutil.AppendCodeAtTheEnd(
511+
filepath.Join(sp.ctx.Dir, "api/v1/cronjob_webhook.go"),
512+
webhookValidateSpecMethods)
513+
hackutils.CheckError("adding validation spec methods at the end", err)
519514
}
520515

521516
func (sp *Sample) updateSuiteTest() {

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

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ limitations under the License.
1616

1717
package cronjob
1818

19-
const WebhookIntro = `"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
19+
const webhookIntro = `"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
2020
)
2121
2222
// +kubebuilder:docs-gen:collapse=Go imports
@@ -27,26 +27,7 @@ Next, we'll setup a logger for the webhooks.
2727
2828
`
2929

30-
const WebhookMarker = `/*
31-
Notice that we use kubebuilder markers to generate webhook manifests.
32-
This marker is responsible for generating a mutating webhook manifest.
33-
34-
The meaning of each marker can be found [here](/reference/markers/webhook.md).
35-
*/
36-
37-
// +kubebuilder:webhook:path=/mutate-batch-tutorial-kubebuilder-io-v1-cronjob,mutating=true,failurePolicy=fail,groups=batch.tutorial.kubebuilder.io,resources=cronjobs,verbs=create;update,versions=v1,name=mcronjob.kb.io,sideEffects=None,admissionReviewVersions=v1
38-
39-
/*
40-
We use the` + " `" + `webhook.CustomDefaulter` + "`" + ` interface to set defaults to our CRD.
41-
A webhook will automatically be served that calls this defaulting.
42-
43-
The` + " `" + `Default` + "`" + ` method is expected to mutate the receiver, setting the defaults.
44-
*/
45-
`
46-
47-
const WebhookDefaultingSettings = `
48-
49-
// Set default values
30+
const webhookDefaultingSettings = `// Set default values
5031
cronjob.Default()
5132
5233
return nil
@@ -68,13 +49,22 @@ func (r *CronJob) Default() {
6849
*r.Spec.FailedJobsHistoryLimit = 1
6950
}
7051
}
52+
`
7153

54+
const webhooksNoticeMarker = `
7255
/*
73-
This marker is responsible for generating a validating webhook manifest.
56+
Notice that we use kubebuilder markers to generate webhook manifests.
57+
This marker is responsible for generating a mutating webhook manifest.
58+
59+
The meaning of each marker can be found [here](/reference/markers/webhook.md).
7460
*/
7561
76-
// +kubebuilder:webhook:verbs=create;update;delete,path=/validate-batch-tutorial-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,groups=batch.tutorial.kubebuilder.io,resources=cronjobs,versions=v1,name=vcronjob.kb.io,sideEffects=None,admissionReviewVersions=v1
62+
/*
63+
This marker is responsible for generating a mutation webhook manifest.
64+
*/
65+
`
7766

67+
const explanationValidateCRD = `
7868
/*
7969
We can validate our CRD beyond what's possible with declarative
8070
validation. Generally, declarative validation should be sufficient, but
@@ -96,8 +86,21 @@ Here, however, we just use the same shared validation for` + " `" + `ValidateCre
9686
` + "`" + `ValidateUpdate` + "`" + `. And we do nothing in` + " `" + `ValidateDelete` + "`" + `, since we don't need to
9787
validate anything on deletion.
9888
*/
99-
`
100-
const WebhookValidateSpec = `
89+
90+
/*
91+
This marker is responsible for generating a validation webhook manifest.
92+
*/`
93+
94+
const customInterfaceDefaultInfo = `/*
95+
We use the ` + "`" + `webhook.CustomDefaulter` + "`" + `interface to set defaults to our CRD.
96+
A webhook will automatically be served that calls this defaulting.
97+
98+
The ` + "`" + `Default` + "`" + `method is expected to mutate the receiver, setting the defaults.
99+
*/
100+
101+
// Default implements webhook.CustomDefaulter so a webhook will be registered for the Kind CronJob.`
102+
103+
const webhookValidateSpecMethods = `
101104
/*
102105
We validate the name and the spec of the CronJob.
103106
*/

0 commit comments

Comments
 (0)