Skip to content

Commit

Permalink
improve the webhook tests by adding examples
Browse files Browse the repository at this point in the history
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
  • Loading branch information
camilamacedo86 committed Sep 3, 2024
1 parent 33a2f3d commit b4e13b8
Show file tree
Hide file tree
Showing 19 changed files with 933 additions and 187 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -100,22 +100,27 @@ func (d *CronJobCustomDefaulter) Default(ctx context.Context, obj runtime.Object
}
cronjoblog.Info("Defaulting for CronJob", "name", cronjob.GetName())

if cronjob.Spec.ConcurrencyPolicy == "" {
cronjob.Spec.ConcurrencyPolicy = AllowConcurrent
// Set default values
cronjob.Default()

return nil
}

func (r *CronJob) Default() {
if r.Spec.ConcurrencyPolicy == "" {
r.Spec.ConcurrencyPolicy = AllowConcurrent
}
if cronjob.Spec.Suspend == nil {
cronjob.Spec.Suspend = new(bool)
if r.Spec.Suspend == nil {
r.Spec.Suspend = new(bool)
}
if cronjob.Spec.SuccessfulJobsHistoryLimit == nil {
cronjob.Spec.SuccessfulJobsHistoryLimit = new(int32)
*cronjob.Spec.SuccessfulJobsHistoryLimit = 3
if r.Spec.SuccessfulJobsHistoryLimit == nil {
r.Spec.SuccessfulJobsHistoryLimit = new(int32)
*r.Spec.SuccessfulJobsHistoryLimit = 3
}
if cronjob.Spec.FailedJobsHistoryLimit == nil {
cronjob.Spec.FailedJobsHistoryLimit = new(int32)
*cronjob.Spec.FailedJobsHistoryLimit = 1
if r.Spec.FailedJobsHistoryLimit == nil {
r.Spec.FailedJobsHistoryLimit = new(int32)
*r.Spec.FailedJobsHistoryLimit = 1
}

return nil
}

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,146 @@ package v1

import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
// TODO (user): Add any additional imports if needed
)

var _ = Describe("CronJob Webhook", func() {
var (
obj *CronJob
oldObj *CronJob
validator CronJobCustomValidator
)

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

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

validator = CronJobCustomValidator{}

Expect(obj).NotTo(BeNil(), "Expected obj to be initialized")
Expect(oldObj).NotTo(BeNil(), "Expected oldObj to be initialized")
})

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

// TODO(user): Add your logic here
Context("When creating CronJob under Defaulting Webhook", func() {
It("Should apply defaults when a required field is empty", func() {
By("simulating a scenario where defaults should be applied")
obj.Spec.ConcurrencyPolicy = "" // This should default to AllowConcurrent
obj.Spec.Suspend = nil // This should default to false
obj.Spec.SuccessfulJobsHistoryLimit = nil // This should default to 3
obj.Spec.FailedJobsHistoryLimit = nil // This should default to 1

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

By("checking that the default values are set")
Expect(obj.Spec.ConcurrencyPolicy).To(Equal(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.Suspend = new(bool)
*obj.Spec.Suspend = true
obj.Spec.SuccessfulJobsHistoryLimit = new(int32)
*obj.Spec.SuccessfulJobsHistoryLimit = 5
obj.Spec.FailedJobsHistoryLimit = new(int32)
*obj.Spec.FailedJobsHistoryLimit = 2

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

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.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")
})
})

Context("When creating CronJob under Validating Webhook", func() {
It("Should deny if a required field is empty", func() {
Context("When creating or updating CronJob under Validating Webhook", func() {
It("Should deny creation if the name is too long", func() {
obj.ObjectMeta.Name = "this-name-is-way-too-long-and-should-fail-validation-because-it-is-way-too-long"
warnings, err := validator.ValidateCreate(ctx, obj)
Expect(err).To(HaveOccurred(), "Expected name validation to fail for a too-long name")
Expect(warnings).To(BeNil())
Expect(err.Error()).To(ContainSubstring("must be no more than 52 characters"))
})

It("Should admit creation if the name is valid", func() {
obj.ObjectMeta.Name = "valid-cronjob-name"
warnings, err := validator.ValidateCreate(ctx, obj)
Expect(err).NotTo(HaveOccurred(), "Expected name validation to pass for a valid name")
Expect(warnings).To(BeNil())
})

It("Should deny creation if the schedule is invalid", func() {
obj.Spec.Schedule = "invalid-cron-schedule"
warnings, err := validator.ValidateCreate(ctx, obj)
Expect(err).To(HaveOccurred(), "Expected spec validation to fail for an invalid schedule")
Expect(warnings).To(BeNil())
Expect(err.Error()).To(ContainSubstring("Expected exactly 5 fields, found 1: invalid-cron-schedule"))
})

It("Should admit creation if the schedule is valid", func() {
obj.Spec.Schedule = "*/5 * * * *"
warnings, err := validator.ValidateCreate(ctx, obj)
Expect(err).NotTo(HaveOccurred(), "Expected spec validation to pass for a valid schedule")
Expect(warnings).To(BeNil())
})

It("Should deny update if both name and spec are invalid", func() {
oldObj.ObjectMeta.Name = "valid-cronjob-name"
oldObj.Spec.Schedule = "*/5 * * * *"

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

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

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

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

By("validating an update")
warnings, err := validator.ValidateUpdate(ctx, oldObj, obj)
Expect(err).NotTo(HaveOccurred(), "Expected validation to pass for a valid update")
Expect(warnings).To(BeNil())
})
})

Expand Down
52 changes: 41 additions & 11 deletions hack/docs/internal/cronjob-tutorial/generate_cronjob.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,26 +85,28 @@ func (sp *Sample) UpdateTutorial() {
sp.updateSpec()
// 2. update webhook
sp.updateWebhook()
// 3. update makefile
// 3. update webhookTests
sp.updateWebhookTests()
// 4. update makefile
sp.updateMakefile()
// 4. generate extra files
// 5. generate extra files
sp.codeGen()
// 5. compensate other intro in API
// 6. compensate other intro in API
sp.updateAPIStuff()
// 6. update reconciliation and main.go
// 6.1 update controller
// 7. update reconciliation and main.go
// 7.1 update controller
sp.updateController()
// 6.2 update main.go
// 7.2 update main.go
sp.updateMain()
// 7. generate extra files
// 8. generate extra files
sp.codeGen()
// 8. update suite_test explanation
// 9. update suite_test explanation
sp.updateSuiteTest()
// 9. uncomment kustomization
// 10. uncomment kustomization
sp.updateKustomization()
// 10. add example
// 11. add example
sp.updateExample()
// 11. add test
// 12. add test
sp.addControllerTest()
}

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

}

func (sp *Sample) updateWebhookTests() {
file := filepath.Join(sp.ctx.Dir, "api/v1/cronjob_webhook_test.go")

err := pluginutil.InsertCode(file,
`var _ = Describe("CronJob Webhook", func() {
var (
obj *CronJob`,
`
oldObj *CronJob
validator CronJobCustomValidator`)
hackutils.CheckError("insert global vars", err)

err = pluginutil.ReplaceInFile(file,
webhookTestCreateDefaultingFragment,
webhookTestCreateDefaultingReplaceFragment)
hackutils.CheckError("replace create defaulting test", err)

err = pluginutil.ReplaceInFile(file,
webhookTestingValidatingTodoFragment,
webhookTestingValidatingExampleFragment)
hackutils.CheckError("replace validating defaulting test", err)

err = pluginutil.ReplaceInFile(file,
webhookTestsBeforeEachOriginal,
webhookTestsBeforeEachChanged)
hackutils.CheckError("replace validating defaulting test", err)
}

func (sp *Sample) updateWebhook() {
var err error
err = pluginutil.InsertCode(
Expand Down
Loading

0 comments on commit b4e13b8

Please sign in to comment.