Merge pull request #1295 from HaraldNordgren/sort_version_2

🐛 sort manifest webhooks

Author: k8s-ci-robot

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

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()

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

pkg/webhook/testdata/valid-crosspkg-stable/v1/cronjob_types.go

@@ -0,0 +1,3 @@
+package v1
+
+type CronJob struct{}

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

pkg/webhook/testdata/valid-crosspkg-stable/v1alpha1/cronjob_types.go

@@ -0,0 +1,3 @@
+package v1alpha1
+
+type CronJob struct{}

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