diff --git a/pkg/webhook/parser.go b/pkg/webhook/parser.go index 832edebb0..2737562be 100644 --- a/pkg/webhook/parser.go +++ b/pkg/webhook/parser.go @@ -24,6 +24,7 @@ package webhook import ( "fmt" + "slices" "sort" "strings" @@ -514,8 +515,12 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error { } versionedWebhooks := make(map[string][]interface{}, len(supportedWebhookVersions)) + //nolint:dupl for _, version := range supportedWebhookVersions { if cfgs, ok := mutatingCfgs[version]; ok { + slices.SortFunc(cfgs, func(a, b admissionregv1.MutatingWebhook) int { + return strings.Compare(a.Name, b.Name) + }) var objRaw *admissionregv1.MutatingWebhookConfiguration if mutatingWebhookCfgs.Name != "" { objRaw = &mutatingWebhookCfgs @@ -553,6 +558,9 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error { } if cfgs, ok := validatingCfgs[version]; ok { + slices.SortFunc(cfgs, func(a, b admissionregv1.ValidatingWebhook) int { + return strings.Compare(a.Name, b.Name) + }) var objRaw *admissionregv1.ValidatingWebhookConfiguration if validatingWebhookCfgs.Name != "" { objRaw = &validatingWebhookCfgs diff --git a/pkg/webhook/parser_integration_test.go b/pkg/webhook/parser_integration_test.go index 4a1b111ac..ce2ca51b3 100644 --- a/pkg/webhook/parser_integration_test.go +++ b/pkg/webhook/parser_integration_test.go @@ -426,6 +426,76 @@ var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition", assertSame(actualValidating, expectedValidating) }) + It("should keep webhook order stable across package traversal orders", func() { + By("switching into testdata to appease go modules") + cwd, err := os.Getwd() + Expect(err).NotTo(HaveOccurred()) + Expect(os.Chdir("./testdata/valid-crosspkg-stable")).To(Succeed()) // go modules are directory-sensitive + defer func() { Expect(os.Chdir(cwd)).To(Succeed()) }() + + By("setting up the parser") + reg := &markers.Registry{} + Expect(reg.Register(webhook.ConfigDefinition)).To(Succeed()) + Expect(reg.Register(webhook.WebhookConfigDefinition)).To(Succeed()) + + By("loading the desired v1 YAML") + expectedFile, err := os.ReadFile("manifests.yaml") + Expect(err).NotTo(HaveOccurred()) + expectedManifest := &admissionregv1.ValidatingWebhookConfiguration{} + Expect(yaml.UnmarshalStrict(expectedFile, expectedManifest)).To(Succeed()) + + rootsOrders := []struct { + name string + outputDir string + roots []string + }{ + { + name: "v1 first", + outputDir: "webhook-integration-test-order-a", + roots: []string{ + "./v1", + "./v1alpha1", + }}, + { + name: "v1alpha1 first", + outputDir: "webhook-integration-test-order-b", + roots: []string{ + "./v1alpha1", + "./v1", + }}, + } + + for _, rootsOrder := range rootsOrders { + By("loading the roots in order " + rootsOrder.name) + pkgs, err := loader.LoadRoots(rootsOrder.roots...) + Expect(err).NotTo(HaveOccurred()) + Expect(pkgs).To(HaveLen(2)) + + By("requesting that the manifest be generated for order " + rootsOrder.name) + outputDir, err := os.MkdirTemp("", rootsOrder.outputDir) + Expect(err).NotTo(HaveOccurred()) + defer os.RemoveAll(outputDir) + genCtx := &genall.GenerationContext{ + Collector: &markers.Collector{Registry: reg}, + Roots: pkgs, + OutputRule: genall.OutputToDirectory(outputDir), + } + Expect(webhook.Generator{}.Generate(genCtx)).To(Succeed()) + for _, r := range genCtx.Roots { + Expect(r.Errors).To(HaveLen(0)) + } + + By("loading the generated v1 YAML for order " + rootsOrder.name) + actualFile, err := os.ReadFile(path.Join(outputDir, "manifests.yaml")) + Expect(err).NotTo(HaveOccurred()) + actualManifest := &admissionregv1.ValidatingWebhookConfiguration{} + Expect(yaml.UnmarshalStrict(actualFile, actualManifest)).To(Succeed()) + + By("comparing the manifest for order " + rootsOrder.name) + assertSame(actualManifest, expectedManifest) + } + }) + It("should fail to generate when there are multiple `kubebuilder:webhookconfiguration` markers of the same mutation type", func() { By("switching into testdata to appease go modules") cwd, err := os.Getwd() diff --git a/pkg/webhook/testdata/valid-crosspkg-stable/manifests.yaml b/pkg/webhook/testdata/valid-crosspkg-stable/manifests.yaml new file mode 100644 index 000000000..2e029fb3a --- /dev/null +++ b/pkg/webhook/testdata/valid-crosspkg-stable/manifests.yaml @@ -0,0 +1,50 @@ +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: validating-webhook-configuration +webhooks: + - admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-testdata-kubebuilder-io-v1alpha1-cronjob + failurePolicy: Fail + matchPolicy: Equivalent + name: a.cronjob.testdata.kubebuilder.io + rules: + - apiGroups: + - testdata.kubebuilder.io + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - cronjobs + sideEffects: None + - admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-testdata-kubebuilder-io-v1-cronjob + failurePolicy: Fail + matchPolicy: Equivalent + name: validation.cronjob.testdata.kubebuilder.io + rules: + - apiGroups: + - testdata.kubebuilder.io + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - cronjobs + sideEffects: None diff --git a/pkg/webhook/testdata/valid-crosspkg-stable/v1/cronjob_types.go b/pkg/webhook/testdata/valid-crosspkg-stable/v1/cronjob_types.go new file mode 100644 index 000000000..93bfdefa7 --- /dev/null +++ b/pkg/webhook/testdata/valid-crosspkg-stable/v1/cronjob_types.go @@ -0,0 +1,3 @@ +package v1 + +type CronJob struct{} diff --git a/pkg/webhook/testdata/valid-crosspkg-stable/v1/webhook.go b/pkg/webhook/testdata/valid-crosspkg-stable/v1/webhook.go new file mode 100644 index 000000000..226221c55 --- /dev/null +++ b/pkg/webhook/testdata/valid-crosspkg-stable/v1/webhook.go @@ -0,0 +1,3 @@ +package v1 + +// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuilder.io,resources=cronjobs,versions=v1,name=validation.cronjob.testdata.kubebuilder.io,sideEffects=None,admissionReviewVersions=v1;v1beta1,path=/validate-testdata-kubebuilder-io-v1-cronjob diff --git a/pkg/webhook/testdata/valid-crosspkg-stable/v1alpha1/cronjob_types.go b/pkg/webhook/testdata/valid-crosspkg-stable/v1alpha1/cronjob_types.go new file mode 100644 index 000000000..96a2d05c3 --- /dev/null +++ b/pkg/webhook/testdata/valid-crosspkg-stable/v1alpha1/cronjob_types.go @@ -0,0 +1,3 @@ +package v1alpha1 + +type CronJob struct{} diff --git a/pkg/webhook/testdata/valid-crosspkg-stable/v1alpha1/webhook.go b/pkg/webhook/testdata/valid-crosspkg-stable/v1alpha1/webhook.go new file mode 100644 index 000000000..722d035af --- /dev/null +++ b/pkg/webhook/testdata/valid-crosspkg-stable/v1alpha1/webhook.go @@ -0,0 +1,3 @@ +package v1alpha1 + +// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuilder.io,resources=cronjobs,versions=v1alpha1,name=a.cronjob.testdata.kubebuilder.io,sideEffects=None,admissionReviewVersions=v1;v1beta1,path=/validate-testdata-kubebuilder-io-v1alpha1-cronjob