🐛 crdgen: compare metav1 pkg by ID

pkg/crd/gen.go

@@ -247,7 +247,12 @@ func FindKubeKinds(parser *Parser, metav1Pkg *loader.Package) map[schema.GroupKi
 			}
 			fieldPkgPath := loader.NonVendorPath(namedField.Obj().Pkg().Path())
 			fieldPkg := pkg.Imports()[fieldPkgPath]
-			if fieldPkg != metav1Pkg {
+
+			// Compare the metav1 package by ID and not by the actual instance
+			// of the object. The objects in memory could be different due to
+			// loading from different root paths, even when they both refer to
+			// the same metav1 package.
+			if fieldPkg == nil || fieldPkg.ID != metav1Pkg.ID {
 				continue
 			}
 

pkg/crd/parser_integration_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package crd_test
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 
@@ -52,63 +53,112 @@ func packageErrors(pkg *loader.Package, filterKinds ...packages.ErrorKind) error
 }
 
 var _ = Describe("CRD Generation From Parsing to CustomResourceDefinition", func() {
-	It("should properly generate and flatten the rewritten CronJob schema", func() {
-		// TODO(directxman12): test generation across multiple versions (right
-		// now, we're trusting k/k's conversion code, though, which is probably
-		// fine for the time being)
-		By("switching into testdata to appease go modules")
-		cwd, err := os.Getwd()
-		Expect(err).NotTo(HaveOccurred())
-		Expect(os.Chdir("./testdata")).To(Succeed()) // go modules are directory-sensitive
-		defer func() { Expect(os.Chdir(cwd)).To(Succeed()) }()
-
-		By("loading the roots")
-		pkgs, err := loader.LoadRoots("./", "./unserved", "./deprecated")
-		Expect(err).NotTo(HaveOccurred())
-		Expect(pkgs).To(HaveLen(3))
-		cronJobPkg := pkgs[0]
-
-		By("setting up the parser")
-		reg := &markers.Registry{}
-		Expect(crdmarkers.Register(reg)).To(Succeed())
-		parser := &crd.Parser{
-			Collector:              &markers.Collector{Registry: reg},
-			Checker:                &loader.TypeChecker{},
-			IgnoreUnexportedFields: true,
-			AllowDangerousTypes:    true, // need to allow “dangerous types” in this file for testing
-		}
-		crd.AddKnownTypes(parser)
+	Context("should properly generate and flatten the rewritten schemas", func() {
+
+		var (
+			prevCwd   string
+			pkgPaths  []string
+			pkgs      []*loader.Package
+			reg       *markers.Registry
+			parser    *crd.Parser
+			expPkgLen int
+		)
+
+		BeforeEach(func() {
+			var err error
+			By("switching into testdata to appease go modules")
+			prevCwd, err = os.Getwd()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(prevCwd).To(BeADirectory())
+			Expect(os.Chdir("./testdata")).To(Succeed())
+
+			By("setting up the parser")
+			reg = &markers.Registry{}
+			Expect(crdmarkers.Register(reg)).To(Succeed())
+			parser = &crd.Parser{
+				Collector:              &markers.Collector{Registry: reg},
+				Checker:                &loader.TypeChecker{},
+				IgnoreUnexportedFields: true,
+				AllowDangerousTypes:    true, // need to allow “dangerous types” in this file for testing
+			}
+			crd.AddKnownTypes(parser)
+		})
+
+		AfterEach(func() {
+			Expect(os.Chdir(prevCwd)).To(Succeed())
+		})
+
+		JustBeforeEach(func() {
+			var err error
+			By("loading the roots")
+			pkgs, err = loader.LoadRoots(pkgPaths...)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(pkgs).To(HaveLen(expPkgLen))
+
+			By("requesting that the packages be parsed")
+			for _, p := range pkgs {
+				parser.NeedPackage(p)
+			}
+		})
 
-		By("requesting that the packages be parsed")
-		for _, pkg := range pkgs {
-			parser.NeedPackage(pkg)
-		}
+		assertCRD := func(pkg *loader.Package, kind, fileName string) {
+			By(fmt.Sprintf("requesting that the %s CRD be generated", kind))
+			groupKind := schema.GroupKind{Kind: kind, Group: "testdata.kubebuilder.io"}
+			parser.NeedCRDFor(groupKind, nil)
 
-		By("requesting that the CRD be generated")
-		groupKind := schema.GroupKind{Kind: "CronJob", Group: "testdata.kubebuilder.io"}
-		parser.NeedCRDFor(groupKind, nil)
+			By(fmt.Sprintf("fixing top level ObjectMeta on the %s CRD", kind))
+			crd.FixTopLevelMetadata(parser.CustomResourceDefinitions[groupKind])
 
-		By("fixing top level ObjectMeta on the CRD")
-		crd.FixTopLevelMetadata(parser.CustomResourceDefinitions[groupKind])
+			By("checking that no errors occurred along the way (expect for type errors)")
+			ExpectWithOffset(1, packageErrors(pkg, packages.TypeError)).NotTo(HaveOccurred())
 
-		By("checking that no errors occurred along the way (expect for type errors)")
-		Expect(packageErrors(cronJobPkg, packages.TypeError)).NotTo(HaveOccurred())
+			By(fmt.Sprintf("checking that the %s CRD is present", kind))
+			ExpectWithOffset(1, parser.CustomResourceDefinitions).To(HaveKey(groupKind))
 
-		By("checking that the CRD is present")
-		Expect(parser.CustomResourceDefinitions).To(HaveKey(groupKind))
+			By(fmt.Sprintf("loading the desired %s YAML", kind))
+			expectedFile, err := ioutil.ReadFile(fileName)
+			ExpectWithOffset(1, err).NotTo(HaveOccurred())
 
-		By("loading the desired YAML")
-		expectedFile, err := ioutil.ReadFile("testdata.kubebuilder.io_cronjobs.yaml")
-		Expect(err).NotTo(HaveOccurred())
+			By(fmt.Sprintf("parsing the desired %s YAML", kind))
+			var crd apiext.CustomResourceDefinition
+			ExpectWithOffset(1, yaml.Unmarshal(expectedFile, &crd)).To(Succeed())
+			// clear the annotations -- we don't care about the attribution annotation
+			crd.Annotations = nil
 
-		By("parsing the desired YAML")
-		var crd apiext.CustomResourceDefinition
-		Expect(yaml.Unmarshal(expectedFile, &crd)).To(Succeed())
-		// clear the annotations -- we don't care about the attribution annotation
-		crd.Annotations = nil
+			By(fmt.Sprintf("comparing the two %s CRDs", kind))
+			ExpectWithOffset(1, parser.CustomResourceDefinitions[groupKind]).To(Equal(crd), "type not as expected, check pkg/crd/testdata/README.md for more details.\n\nDiff:\n\n%s", cmp.Diff(parser.CustomResourceDefinitions[groupKind], crd))
+		}
 
-		By("comparing the two")
-		Expect(parser.CustomResourceDefinitions[groupKind]).To(Equal(crd), "type not as expected, check pkg/crd/testdata/README.md for more details.\n\nDiff:\n\n%s", cmp.Diff(parser.CustomResourceDefinitions[groupKind], crd))
+		Context("CronJob API", func() {
+			BeforeEach(func() {
+				pkgPaths = []string{"./", "./unserved", "./deprecated"}
+				expPkgLen = 3
+			})
+			It("should successfully generate the CronJob CRD", func() {
+				assertCRD(pkgs[0], "CronJob", "testdata.kubebuilder.io_cronjobs.yaml")
+			})
+		})
+
+		Context("Job API", func() {
+			BeforeEach(func() {
+				pkgPaths = []string{"./job/..."}
+				expPkgLen = 1
+			})
+			It("should successfully generate the Job CRD", func() {
+				assertCRD(pkgs[0], "Job", "testdata.kubebuilder.io_jobs.yaml")
+			})
+		})
+
+		Context("CronJob and Job API", func() {
+			BeforeEach(func() {
+				pkgPaths = []string{"./", "./unserved", "./deprecated", "./job/..."}
+				expPkgLen = 4
+			})
+			It("should successfully generate the CronJob and Job CRDs", func() {
+				assertCRD(pkgs[0], "CronJob", "testdata.kubebuilder.io_cronjobs.yaml")
+				assertCRD(pkgs[3], "Job", "testdata.kubebuilder.io_jobs.yaml")
+			})
+		})
 	})
 
 	It("should generate plural words for Kind correctly", func() {

pkg/crd/testdata/cronjob_types.go

@@ -16,7 +16,7 @@ limitations under the License.
 // TODO(directxman12): test this across both versions (right now we're just
 // trusting k/k conversion, which is probably fine though)
 
-//go:generate ../../../.run-controller-gen.sh crd:ignoreUnexportedFields=true,allowDangerousTypes=true paths=./;./deprecated;./unserved output:dir=.
+//go:generate ../../../.run-controller-gen.sh crd:ignoreUnexportedFields=true,allowDangerousTypes=true paths=./;./deprecated;./unserved;./job/... output:dir=.
 
 // +groupName=testdata.kubebuilder.io
 // +versionName=v1

pkg/crd/testdata/job/types.go

@@ -0,0 +1,64 @@
+/*
+
+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.
+*/
+
+// +groupName=testdata.kubebuilder.io
+// +versionName=v1beta1
+package job
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"testdata.kubebuilder.io/cronjob/unserved"
+)
+
+// +kubebuilder:object:root=true
+// +kubebuilder:subresource:status
+// +kubebuilder:resource:singular=job
+
+// JobSpec is the spec for the jobs API.
+type JobSpec struct {
+	// FriendlyName is the friendly name for the job.
+	//
+	// +kubebuilder:validation:MinLength=5
+	FriendlyName string `json:"friendlyName"`
+
+	// Count is the number of times a job may be executed.
+	//
+	// +kubebuilder:validation:Minimum=0
+	// +kubebuilder:validation:Maximum=10
+	Count int32 `json:"count"`
+
+	// CronJob is the spec for the related CrongJob.
+	CronnJob unserved.CronJobSpec `json:"crongJob"`
+}
+
+// Job is the Schema for the jobs API
+type Job struct {
+	/*
+	 */
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+
+	Spec JobSpec `json:"spec"`
+}
+
+// +kubebuilder:object:root=true
+
+// JobList contains a list of Job
+type JobList struct {
+	metav1.TypeMeta `json:",inline"`
+	metav1.ListMeta `json:"metadata,omitempty"`
+	Items           []Job `json:"items"`
+}

pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml

@@ -7504,6 +7504,24 @@ spec:
             type: string
           metadata:
             type: object
+          spec:
+            description: CronJobSpec defines the desired state of CronJob
+            properties:
+              int32WithValidations:
+                format: int32
+                maximum: 2
+                minimum: -2
+                multipleOf: 2
+                type: integer
+              twoOfAKindPart0:
+                description: This tests that markers that are allowed on both fields
+                  and types are applied to fields
+                minLength: 4
+                type: string
+            required:
+            - int32WithValidations
+            - twoOfAKindPart0
+            type: object
         type: object
     served: false
     storage: false

pkg/crd/testdata/testdata.kubebuilder.io_jobs.yaml

@@ -0,0 +1,75 @@
+---
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  annotations:
+    controller-gen.kubebuilder.io/version: (devel)
+  creationTimestamp: null
+  name: jobs.testdata.kubebuilder.io
+spec:
+  group: testdata.kubebuilder.io
+  names:
+    kind: Job
+    listKind: JobList
+    plural: jobs
+    singular: job
+  scope: Namespaced
+  versions:
+  - name: v1beta1
+    schema:
+      openAPIV3Schema:
+        description: Job is the Schema for the jobs API
+        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: JobSpec is the spec for the jobs API.
+            properties:
+              count:
+                description: Count is the number of times a job may be executed.
+                format: int32
+                maximum: 10
+                minimum: 0
+                type: integer
+              crongJob:
+                description: CronJob is the spec for the related CrongJob.
+                properties:
+                  int32WithValidations:
+                    format: int32
+                    maximum: 2
+                    minimum: -2
+                    multipleOf: 2
+                    type: integer
+                  twoOfAKindPart0:
+                    description: This tests that markers that are allowed on both
+                      fields and types are applied to fields
+                    minLength: 4
+                    type: string
+                required:
+                - int32WithValidations
+                - twoOfAKindPart0
+                type: object
+              friendlyName:
+                description: FriendlyName is the friendly name for the job.
+                minLength: 5
+                type: string
+            required:
+            - count
+            - crongJob
+            - friendlyName
+            type: object
+        required:
+        - spec
+        type: object
+    served: true
+    storage: true

pkg/crd/testdata/unserved/types.go

@@ -21,6 +21,18 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+// CronJobSpec defines the desired state of CronJob
+type CronJobSpec struct {
+	// This tests that markers that are allowed on both fields and types are applied to fields
+	// +kubebuilder:validation:MinLength=4
+	TwoOfAKindPart0 string `json:"twoOfAKindPart0"`
+
+	// +kubebuilder:validation:Minimum=-2
+	// +kubebuilder:validation:Maximum=2
+	// +kubebuilder:validation:MultipleOf=2
+	Int32WithValidations int32 `json:"int32WithValidations"`
+}
+
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 // +kubebuilder:resource:singular=mycronjob
@@ -32,6 +44,8 @@ type CronJob struct {
 	 */
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
+
+	Spec CronJobSpec `json:"spec,omitempty"`
 }
 
 // +kubebuilder:object:root=true