✨ crd/gen: sort FindKubeKinds (#694)

* bug: crd/gen: Sort findGroupKinds

* remove binary

Co-authored-by: Ghasem Shirazi <[email protected]>

Author: k8s-infra-cherrypick-robot

pkg/crd/gen.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"go/ast"
 	"go/types"
+	"sort"
 
 	apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -127,7 +128,7 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error {
 		crdVersions = []string{defaultVersion}
 	}
 
-	for groupKind := range kubeKinds {
+	for _, groupKind := range kubeKinds {
 		parser.NeedCRDFor(groupKind, g.MaxDescLen)
 		crdRaw := parser.CustomResourceDefinitions[groupKind]
 		addAttribution(&crdRaw)
@@ -216,7 +217,7 @@ func FindMetav1(roots []*loader.Package) *loader.Package {
 // FindKubeKinds locates all types that contain TypeMeta and ObjectMeta
 // (and thus may be a Kubernetes object), and returns the corresponding
 // group-kinds.
-func FindKubeKinds(parser *Parser, metav1Pkg *loader.Package) map[schema.GroupKind]struct{} {
+func FindKubeKinds(parser *Parser, metav1Pkg *loader.Package) []schema.GroupKind {
 	// TODO(directxman12): technically, we should be finding metav1 per-package
 	kubeKinds := map[schema.GroupKind]struct{}{}
 	for typeIdent, info := range parser.Types {
@@ -275,7 +276,15 @@ func FindKubeKinds(parser *Parser, metav1Pkg *loader.Package) map[schema.GroupKi
 		kubeKinds[groupKind] = struct{}{}
 	}
 
-	return kubeKinds
+	groupKindList := make([]schema.GroupKind, 0, len(kubeKinds))
+	for groupKind := range kubeKinds {
+		groupKindList = append(groupKindList, groupKind)
+	}
+	sort.Slice(groupKindList, func(i, j int) bool {
+		return groupKindList[i].String() < groupKindList[j].String()
+	})
+
+	return groupKindList
 }
 
 // filterTypesForCRDs filters out all nodes that aren't used in CRD generation,

pkg/crd/gen_integration_test.go

@@ -36,8 +36,8 @@ import (
 
 var _ = Describe("CRD Generation proper defaulting", func() {
 	var (
-		ctx *genall.GenerationContext
-		out *outputRule
+		ctx, ctx2 *genall.GenerationContext
+		out       *outputRule
 
 		genDir = filepath.Join("testdata", "gen")
 	)
@@ -53,6 +53,9 @@ var _ = Describe("CRD Generation proper defaulting", func() {
 		pkgs, err := loader.LoadRoots(".")
 		Expect(err).NotTo(HaveOccurred())
 		Expect(pkgs).To(HaveLen(1))
+		pkgs2, err := loader.LoadRoots("./...")
+		Expect(err).NotTo(HaveOccurred())
+		Expect(pkgs2).To(HaveLen(2))
 
 		By("setup up the context")
 		reg := &markers.Registry{}
@@ -66,6 +69,12 @@ var _ = Describe("CRD Generation proper defaulting", func() {
 			Checker:    &loader.TypeChecker{},
 			OutputRule: out,
 		}
+		ctx2 = &genall.GenerationContext{
+			Collector:  &markers.Collector{Registry: reg},
+			Roots:      pkgs2,
+			Checker:    &loader.TypeChecker{},
+			OutputRule: out,
+		}
 	})
 
 	It("should fail to generate v1beta1 CRDs", func() {
@@ -91,6 +100,26 @@ var _ = Describe("CRD Generation proper defaulting", func() {
 		By("comparing the two")
 		Expect(out.buf.String()).To(Equal(string(expectedFile)), cmp.Diff(out.buf.String(), string(expectedFile)))
 	})
+
+	It("should have deterministic output", func() {
+		By("calling Generate on multple packages")
+		gen := &crd.Generator{
+			CRDVersions: []string{"v1"},
+		}
+		Expect(gen.Generate(ctx2)).NotTo(HaveOccurred())
+
+		By("loading the desired YAMLs")
+		expectedFileFoos, err := ioutil.ReadFile(filepath.Join(genDir, "bar.example.com_foos.yaml"))
+		Expect(err).NotTo(HaveOccurred())
+		expectedFileFoos = fixAnnotations(expectedFileFoos)
+		expectedFileZoos, err := ioutil.ReadFile(filepath.Join(genDir, "zoo", "bar.example.com_zooes.yaml"))
+		Expect(err).NotTo(HaveOccurred())
+		expectedFileZoos = fixAnnotations(expectedFileZoos)
+
+		By("comparing the two, output must be deterministic because groupKinds are sorted")
+		expectedOut := string(expectedFileFoos) + string(expectedFileZoos)
+		Expect(out.buf.String()).To(Equal(expectedOut), cmp.Diff(out.buf.String(), expectedOut))
+	})
 })
 
 // fixAnnotations fixes the attribution annotation for tests.

pkg/crd/testdata/gen/zoo/bar.example.com_zooes.v1beta1.yaml

@@ -0,0 +1,50 @@
+---
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  annotations:
+    controller-gen.kubebuilder.io/version: (devel)
+  creationTimestamp: null
+  name: zoos.bar.example.com
+spec:
+  group: bar.example.com
+  names:
+    kind: Zoo
+    listKind: ZooList
+    plural: zooes
+    singular: zoo
+  scope: Namespaced
+  validation:
+    openAPIV3Schema:
+      properties:
+        apiVersion:
+          description: 'APIVersion defines the versioned schema of this representation
+            of an object. Servers should convert recognized schemas to the latest
+            internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
+          type: string
+        kind:
+          description: 'Kind is a string value representing the REST resource this
+            object represents. Servers may infer this from the endpoint the client
+            submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
+          type: string
+        metadata:
+          type: object
+        spec:
+          description: Spec comments SHOULD appear in the CRD spec
+          properties:
+            defaultedString:
+              description: This tests that defaulted fields are stripped for v1beta1,
+                but not for v1
+              type: string
+          required:
+          - defaultedString
+          type: object
+        status:
+          description: Status comments SHOULD appear in the CRD spec
+          type: object
+      type: object
+  version: zoo
+  versions:
+  - name: zoo
+    served: true
+    storage: true

pkg/crd/testdata/gen/zoo/bar.example.com_zooes.yaml

@@ -0,0 +1,50 @@
+---
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  annotations:
+    controller-gen.kubebuilder.io/version: (devel)
+  creationTimestamp: null
+  name: zooes.bar.example.com
+spec:
+  group: bar.example.com
+  names:
+    kind: Zoo
+    listKind: ZooList
+    plural: zooes
+    singular: zoo
+  scope: Namespaced
+  versions:
+  - name: zoo
+    schema:
+      openAPIV3Schema:
+        properties:
+          apiVersion:
+            description: 'APIVersion defines the versioned schema of this representation
+              of an object. Servers should convert recognized schemas to the latest
+              internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
+            type: string
+          kind:
+            description: 'Kind is a string value representing the REST resource this
+              object represents. Servers may infer this from the endpoint the client
+              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
+            type: string
+          metadata:
+            type: object
+          spec:
+            description: Spec comments SHOULD appear in the CRD spec
+            properties:
+              defaultedString:
+                default: zooDefaultString
+                description: This tests that defaulted fields are stripped for v1beta1,
+                  but not for v1
+                type: string
+            required:
+            - defaultedString
+            type: object
+          status:
+            description: Status comments SHOULD appear in the CRD spec
+            type: object
+        type: object
+    served: true
+    storage: true

pkg/crd/testdata/gen/zoo/zoo_types.go

@@ -0,0 +1,45 @@
+/*
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+//go:generate ../../../../.run-controller-gen.sh crd:crdVersions=v1beta1 paths=. output:dir=.
+//go:generate mv bar.example.com_zooes.yaml bar.example.com_zooes.v1beta1.yaml
+//go:generate ../../../../.run-controller-gen.sh crd:crdVersions=v1 paths=. output:dir=.
+
+// +groupName=bar.example.com
+package zoo
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+type ZooSpec struct {
+	// This tests that defaulted fields are stripped for v1beta1,
+	// but not for v1
+	// +kubebuilder:default=zooDefaultString
+	DefaultedString string `json:"defaultedString"`
+}
+type ZooStatus struct{}
+
+type Zoo struct {
+	// TypeMeta comments should NOT appear in the CRD spec
+	metav1.TypeMeta `json:",inline"`
+	// ObjectMeta comments should NOT appear in the CRD spec
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+
+	// Spec comments SHOULD appear in the CRD spec
+	Spec ZooSpec `json:"spec,omitempty"`
+	// Status comments SHOULD appear in the CRD spec
+	Status ZooStatus `json:"status,omitempty"`
+}

pkg/schemapatcher/gen.go

@@ -113,7 +113,7 @@ func (g Generator) Generate(ctx *genall.GenerationContext) (result error) {
 	}
 
 	// generate schemata for the types we care about, and save them to be written later.
-	for groupKind := range crdgen.FindKubeKinds(parser, metav1Pkg) {
+	for _, groupKind := range crdgen.FindKubeKinds(parser, metav1Pkg) {
 		existingSet, wanted := partialCRDSets[groupKind]
 		if !wanted {
 			continue