Skip to content

Commit

Permalink
⚠️ decouple webhooks from the API
Browse files Browse the repository at this point in the history
  • Loading branch information
camilamacedo86 committed Sep 10, 2024
1 parent 09e4b2d commit 4b5368a
Show file tree
Hide file tree
Showing 82 changed files with 955 additions and 620 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion docs/book/src/cronjob-tutorial/testdata/project/cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (

batchv1 "tutorial.kubebuilder.io/project/api/v1"
"tutorial.kubebuilder.io/project/internal/controller"
webhooksv1 "tutorial.kubebuilder.io/project/internal/webhooks/v1"
// +kubebuilder:scaffold:imports
)

Expand Down Expand Up @@ -183,7 +184,7 @@ func main() {
*/
// nolint:goconst
if os.Getenv("ENABLE_WEBHOOKS") != "false" {
if err = (&batchv1.CronJob{}).SetupWebhookWithManager(mgr); err != nil {
if err = webhooksv1.SetupCronJobWebhookWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create webhook", "webhook", "CronJob")
os.Exit(1)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ webhooks:
namespace: system
path: /mutate-batch-tutorial-kubebuilder-io-v1-cronjob
failurePolicy: Fail
name: mcronjob.kb.io
name: mcronjob-v1.kb.io
rules:
- apiGroups:
- batch.tutorial.kubebuilder.io
Expand Down Expand Up @@ -67,6 +67,7 @@ webhooks:
operations:
- CREATE
- UPDATE
- DELETE
resources:
- cronjobs
sideEffects: None
Expand All @@ -78,7 +79,7 @@ webhooks:
namespace: system
path: /validate-batch-tutorial-kubebuilder-io-v1-cronjob
failurePolicy: Fail
name: vcronjob.kb.io
name: vcronjob-v1.kb.io
rules:
- apiGroups:
- batch.tutorial.kubebuilder.io
Expand All @@ -87,7 +88,6 @@ webhooks:
operations:
- CREATE
- UPDATE
- DELETE
resources:
- cronjobs
sideEffects: None
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import (
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

batchv1 "tutorial.kubebuilder.io/project/api/v1"
)

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

// SetupWebhookWithManager will setup the manager to manage the webhooks.
func (r *CronJob) SetupWebhookWithManager(mgr ctrl.Manager) error {
return ctrl.NewWebhookManagedBy(mgr).
For(r).
// SetupCronJobWebhookWithManager registers the webhook for CronJob in the manager.
func SetupCronJobWebhookWithManager(mgr ctrl.Manager) error {
return ctrl.NewWebhookManagedBy(mgr).For(&batchv1.CronJob{}).
WithValidator(&CronJobCustomValidator{}).
WithDefaulter(&CronJobCustomDefaulter{
DefaultConcurrencyPolicy: AllowConcurrent,
DefaultConcurrencyPolicy: batchv1.AllowConcurrent,
DefaultSuspend: false,
DefaultSuccessfulJobsHistoryLimit: 3,
DefaultFailedJobsHistoryLimit: 1,
Expand All @@ -66,7 +67,7 @@ This marker is responsible for generating a mutating webhook manifest.
The meaning of each marker can be found [here](/reference/markers/webhook.md).
*/

// +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
// +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-v1.kb.io,sideEffects=None,admissionReviewVersions=v1

/*
We use the `webhook.CustomDefaulter` interface to set defaults to our CRD.
Expand All @@ -86,7 +87,7 @@ The `Default` method is expected to mutate the receiver, setting the defaults.
type CronJobCustomDefaulter struct {

// Default values for various CronJob fields
DefaultConcurrencyPolicy ConcurrencyPolicy
DefaultConcurrencyPolicy batchv1.ConcurrencyPolicy
DefaultSuspend bool
DefaultSuccessfulJobsHistoryLimit int32
DefaultFailedJobsHistoryLimit int32
Expand All @@ -96,40 +97,42 @@ var _ webhook.CustomDefaulter = &CronJobCustomDefaulter{}

// Default implements webhook.CustomDefaulter so a webhook will be registered for the Kind CronJob.
func (d *CronJobCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error {
cronjob, ok := obj.(*CronJob)
cronjob, ok := obj.(*batchv1.CronJob)

if !ok {
return fmt.Errorf("expected an CronJob object but got %T", obj)
}
cronjoblog.Info("Defaulting for CronJob", "name", cronjob.GetName())

// Set default values
cronjob.Default()

d.applyDefaults(cronjob)
return nil
}

func (r *CronJob) Default() {
if r.Spec.ConcurrencyPolicy == "" {
r.Spec.ConcurrencyPolicy = AllowConcurrent
// applyDefaults applies default values to CronJob fields.
func (d *CronJobCustomDefaulter) applyDefaults(cronJob *batchv1.CronJob) {
if cronJob.Spec.ConcurrencyPolicy == "" {
cronJob.Spec.ConcurrencyPolicy = d.DefaultConcurrencyPolicy
}
if r.Spec.Suspend == nil {
r.Spec.Suspend = new(bool)
if cronJob.Spec.Suspend == nil {
cronJob.Spec.Suspend = new(bool)
*cronJob.Spec.Suspend = d.DefaultSuspend
}
if r.Spec.SuccessfulJobsHistoryLimit == nil {
r.Spec.SuccessfulJobsHistoryLimit = new(int32)
*r.Spec.SuccessfulJobsHistoryLimit = 3
if cronJob.Spec.SuccessfulJobsHistoryLimit == nil {
cronJob.Spec.SuccessfulJobsHistoryLimit = new(int32)
*cronJob.Spec.SuccessfulJobsHistoryLimit = d.DefaultSuccessfulJobsHistoryLimit
}
if r.Spec.FailedJobsHistoryLimit == nil {
r.Spec.FailedJobsHistoryLimit = new(int32)
*r.Spec.FailedJobsHistoryLimit = 1
if cronJob.Spec.FailedJobsHistoryLimit == nil {
cronJob.Spec.FailedJobsHistoryLimit = new(int32)
*cronJob.Spec.FailedJobsHistoryLimit = d.DefaultFailedJobsHistoryLimit
}
}

/*
This marker is responsible for generating a validating webhook manifest.
*/

// +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
// +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-v1.kb.io,sideEffects=None,admissionReviewVersions=v1

/*
We can validate our CRD beyond what's possible with declarative
Expand Down Expand Up @@ -171,29 +174,29 @@ var _ webhook.CustomValidator = &CronJobCustomValidator{}

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

return nil, cronjob.validateCronJob()
return nil, validateCronJob(cronjob)
}

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

return nil, cronjob.validateCronJob()
return nil, validateCronJob(cronjob)
}

// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type CronJob.
func (v *CronJobCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
cronjob, ok := obj.(*CronJob)
cronjob, ok := obj.(*batchv1.CronJob)
if !ok {
return nil, fmt.Errorf("expected a CronJob object but got %T", obj)
}
Expand All @@ -208,21 +211,19 @@ func (v *CronJobCustomValidator) ValidateDelete(ctx context.Context, obj runtime
We validate the name and the spec of the CronJob.
*/

func (r *CronJob) validateCronJob() error {
// validateCronJob validates the fields of a CronJob object.
func validateCronJob(cronjob *batchv1.CronJob) error {
var allErrs field.ErrorList
if err := r.validateCronJobName(); err != nil {
if err := validateCronJobName(cronjob); err != nil {
allErrs = append(allErrs, err)
}
if err := r.validateCronJobSpec(); err != nil {
if err := validateCronJobSpec(cronjob); err != nil {
allErrs = append(allErrs, err)
}
if len(allErrs) == 0 {
return nil
}

return apierrors.NewInvalid(
schema.GroupKind{Group: "batch.tutorial.kubebuilder.io", Kind: "CronJob"},
r.Name, allErrs)
return apierrors.NewInvalid(schema.GroupKind{Group: "batch.tutorial.kubebuilder.io", Kind: "CronJob"}, cronjob.Name, allErrs)
}

/*
Expand All @@ -235,19 +236,17 @@ declaring validation by running `controller-gen crd -w`,
or [here](/reference/markers/crd-validation.md).
*/

func (r *CronJob) validateCronJobSpec() *field.Error {
// The field helpers from the kubernetes API machinery help us return nicely
// structured validation errors.
return validateScheduleFormat(
r.Spec.Schedule,
field.NewPath("spec").Child("schedule"))
// validateCronJobSpec validates the schedule field of the CronJob spec.
func validateCronJobSpec(cronjob *batchv1.CronJob) *field.Error {
return validateScheduleFormat(cronjob.Spec.Schedule, field.NewPath("spec").Child("schedule"))
}

/*
We'll need to validate the [cron](https://en.wikipedia.org/wiki/Cron) schedule
is well-formatted.
*/

// validateScheduleFormat validates that the schedule field follows the cron format.
func validateScheduleFormat(schedule string, fldPath *field.Path) *field.Error {
if _, err := cron.ParseStandard(schedule); err != nil {
return field.Invalid(fldPath, schedule, err.Error())
Expand All @@ -264,15 +263,10 @@ the apimachinery repo, so we can't declaratively validate it using
the validation schema.
*/

func (r *CronJob) validateCronJobName() *field.Error {
if len(r.ObjectMeta.Name) > validationutils.DNS1035LabelMaxLength-11 {
// The job name length is 63 characters like all Kubernetes objects
// (which must fit in a DNS subdomain). The cronjob controller appends
// a 11-character suffix to the cronjob (`-$TIMESTAMP`) when creating
// a job. The job name length limit is 63 characters. Therefore cronjob
// names must have length <= 63-11=52. If we don't validate this here,
// then job creation will fail later.
return field.Invalid(field.NewPath("metadata").Child("name"), r.ObjectMeta.Name, "must be no more than 52 characters")
// validateCronJobName validates the name of the CronJob object.
func validateCronJobName(cronjob *batchv1.CronJob) *field.Error {
if len(cronjob.ObjectMeta.Name) > validationutils.DNS1035LabelMaxLength-11 {
return field.Invalid(field.NewPath("metadata").Child("name"), cronjob.ObjectMeta.Name, "must be no more than 52 characters")
}
return nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,32 +19,35 @@ package v1
import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

batchv1 "tutorial.kubebuilder.io/project/api/v1"
// TODO (user): Add any additional imports if needed
)

var _ = Describe("CronJob Webhook", func() {
var (
obj *CronJob
oldObj *CronJob
obj = &batchv1.CronJob{}
oldObj = &batchv1.CronJob{}
validator CronJobCustomValidator
defaulter CronJobCustomDefaulter
)

BeforeEach(func() {
obj = &CronJob{
Spec: CronJobSpec{
obj = &batchv1.CronJob{
Spec: batchv1.CronJobSpec{
Schedule: "*/5 * * * *",
ConcurrencyPolicy: AllowConcurrent,
ConcurrencyPolicy: batchv1.AllowConcurrent,
SuccessfulJobsHistoryLimit: new(int32),
FailedJobsHistoryLimit: new(int32),
},
}
*obj.Spec.SuccessfulJobsHistoryLimit = 3
*obj.Spec.FailedJobsHistoryLimit = 1

oldObj = &CronJob{
Spec: CronJobSpec{
oldObj = &batchv1.CronJob{
Spec: batchv1.CronJobSpec{
Schedule: "*/5 * * * *",
ConcurrencyPolicy: AllowConcurrent,
ConcurrencyPolicy: batchv1.AllowConcurrent,
SuccessfulJobsHistoryLimit: new(int32),
FailedJobsHistoryLimit: new(int32),
},
Expand All @@ -53,6 +56,12 @@ var _ = Describe("CronJob Webhook", func() {
*oldObj.Spec.FailedJobsHistoryLimit = 1

validator = CronJobCustomValidator{}
defaulter = CronJobCustomDefaulter{
DefaultConcurrencyPolicy: batchv1.AllowConcurrent,
DefaultSuspend: false,
DefaultSuccessfulJobsHistoryLimit: 3,
DefaultFailedJobsHistoryLimit: 1,
}

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

By("calling the Default method to apply defaults")
obj.Default()
defaulter.Default(ctx, obj)

By("checking that the default values are set")
Expect(obj.Spec.ConcurrencyPolicy).To(Equal(AllowConcurrent), "Expected ConcurrencyPolicy to default to AllowConcurrent")
Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.AllowConcurrent), "Expected ConcurrencyPolicy to default to AllowConcurrent")
Expect(*obj.Spec.Suspend).To(BeFalse(), "Expected Suspend to default to false")
Expect(*obj.Spec.SuccessfulJobsHistoryLimit).To(Equal(int32(3)), "Expected SuccessfulJobsHistoryLimit to default to 3")
Expect(*obj.Spec.FailedJobsHistoryLimit).To(Equal(int32(1)), "Expected FailedJobsHistoryLimit to default to 1")
})

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

By("calling the Default method to apply defaults")
obj.Default()
defaulter.Default(ctx, obj)

By("checking that the fields were not overwritten")
Expect(obj.Spec.ConcurrencyPolicy).To(Equal(ForbidConcurrent), "Expected ConcurrencyPolicy to retain its set value")
Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.ForbidConcurrent), "Expected ConcurrencyPolicy to retain its set value")
Expect(*obj.Spec.Suspend).To(BeTrue(), "Expected Suspend to retain its set value")
Expect(*obj.Spec.SuccessfulJobsHistoryLimit).To(Equal(int32(5)), "Expected SuccessfulJobsHistoryLimit to retain its set value")
Expect(*obj.Spec.FailedJobsHistoryLimit).To(Equal(int32(2)), "Expected FailedJobsHistoryLimit to retain its set value")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ var _ = BeforeSuite(func() {
Expect(cfg).NotTo(BeNil())

scheme := apimachineryruntime.NewScheme()
err = AddToScheme(scheme)
err = admissionv1.AddToScheme(scheme)
Expect(err).NotTo(HaveOccurred())

err = admissionv1.AddToScheme(scheme)
Expand All @@ -115,7 +115,7 @@ var _ = BeforeSuite(func() {
})
Expect(err).NotTo(HaveOccurred())

err = (&CronJob{}).SetupWebhookWithManager(mgr)
err = SetupCronJobWebhookWithManager(mgr)
Expect(err).NotTo(HaveOccurred())

// +kubebuilder:scaffold:webhook
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 4b5368a

Please sign in to comment.