Merge pull request #792 from ntoofu/fix-783

🐛 Change the type of `markers.Collector.byPackage`'s key from `string` to `*loader.Package`

pkg/crd/parser_integration_test.go

@@ -266,4 +266,52 @@ var _ = Describe("CRD Generation From Parsing to CustomResourceDefinition", func
 		By("checking that no errors occurred along the way (expect for type errors)")
 		Expect(packageErrors(cronJobPkg, packages.TypeError)).NotTo(HaveOccurred())
 	})
+
+	It("should generate markers properly among several package versions", func() {
+		By("switching into testdata to appease go modules")
+		cwd, err := os.Getwd()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(os.Chdir("./testdata/multiple_versions")).To(Succeed())
+		defer func() { Expect(os.Chdir(cwd)).To(Succeed()) }()
+
+		By("loading the roots")
+		pkgs, err := loader.LoadRoots("./v1beta1", "./v1beta2")
+		Expect(err).NotTo(HaveOccurred())
+		Expect(pkgs).To(HaveLen(2))
+
+		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{},
+		}
+		crd.AddKnownTypes(parser)
+
+		By("requesting that the package be parsed")
+		for _, pkg := range pkgs {
+			parser.NeedPackage(pkg)
+		}
+
+		By("requesting that the CRD be generated")
+		groupKind := schema.GroupKind{Kind: "VersionedResource", Group: "testdata.kubebuilder.io"}
+		parser.NeedCRDFor(groupKind, nil)
+
+		By("fixing top level ObjectMeta on the CRD")
+		crd.FixTopLevelMetadata(parser.CustomResourceDefinitions[groupKind])
+
+		By("loading the desired YAML")
+		expectedFile, err := ioutil.ReadFile("testdata.kubebuilder.io_versionedresources.yaml")
+		Expect(err).NotTo(HaveOccurred())
+
+		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("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))
+	})
+
 })

pkg/crd/testdata/multiple_versions/common.go

@@ -0,0 +1,10 @@
+package multiple_versions
+
+type InnerStruct struct {
+	Foo string `json:"foo,omitempty"`
+}
+
+type OuterStruct struct {
+	// +structType=atomic
+	Struct InnerStruct `json:"struct,omitempty"`
+}

pkg/crd/testdata/multiple_versions/testdata.kubebuilder.io_versionedresources.yaml

@@ -0,0 +1,87 @@
+---
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  annotations:
+    controller-gen.kubebuilder.io/version: (devel)
+  creationTimestamp: null
+  name: versionedresources.testdata.kubebuilder.io
+spec:
+  group: testdata.kubebuilder.io
+  names:
+    kind: VersionedResource
+    listKind: VersionedResourceList
+    plural: versionedresources
+    singular: versionedresource
+  scope: Namespaced
+  versions:
+  - name: v1beta1
+    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:
+            properties:
+              struct:
+                properties:
+                  struct:
+                    properties:
+                      foo:
+                        type: string
+                    type: object
+                    x-kubernetes-map-type: atomic
+                type: object
+            type: object
+        required:
+        - spec
+        type: object
+    served: true
+    storage: false
+    subresources:
+      status: {}
+  - name: v1beta2
+    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:
+            properties:
+              struct:
+                properties:
+                  struct:
+                    properties:
+                      foo:
+                        type: string
+                    type: object
+                    x-kubernetes-map-type: atomic
+                type: object
+            type: object
+        required:
+        - spec
+        type: object
+    served: true
+    storage: true
+    subresources:
+      status: {}

pkg/crd/testdata/multiple_versions/v1beta1/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.
+*/
+
+// +groupName=testdata.kubebuilder.io
+package v1beta1
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	multiver "testdata.kubebuilder.io/cronjob/multiple_versions"
+)
+
+type VersionedResourceSpec struct {
+	Struct multiver.OuterStruct `json:"struct,omitempty"`
+}
+
+// +kubebuilder:object:root=true
+// +kubebuilder:subresource:status
+// +kubebuilder:resource:singular=versionedresource
+
+type VersionedResource struct {
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+
+	Spec VersionedResourceSpec `json:"spec"`
+}
+
+// +kubebuilder:object:root=true
+
+type VersionedResourceList struct {
+	metav1.TypeMeta `json:",inline"`
+	metav1.ListMeta `json:"metadata,omitempty"`
+	Items           []VersionedResource `json:"items"`
+}

pkg/crd/testdata/multiple_versions/v1beta2/types.go

@@ -0,0 +1,46 @@
+/*
+
+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
+package v1beta2
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	multiver "testdata.kubebuilder.io/cronjob/multiple_versions"
+)
+
+type VersionedResourceSpec struct {
+	Struct multiver.OuterStruct `json:"struct,omitempty"`
+}
+
+// +kubebuilder:storageversion
+// +kubebuilder:object:root=true
+// +kubebuilder:subresource:status
+// +kubebuilder:resource:singular=versionedresource
+
+type VersionedResource struct {
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+
+	Spec VersionedResourceSpec `json:"spec"`
+}
+
+// +kubebuilder:object:root=true
+
+type VersionedResourceList struct {
+	metav1.TypeMeta `json:",inline"`
+	metav1.ListMeta `json:"metadata,omitempty"`
+	Items           []VersionedResource `json:"items"`
+}

pkg/markers/collect.go

@@ -31,7 +31,7 @@ import (
 type Collector struct {
 	*Registry
 
-	byPackage map[string]map[ast.Node]MarkerValues
+	byPackage map[*loader.Package]map[ast.Node]MarkerValues
 	mu        sync.Mutex
 }
 
@@ -53,7 +53,7 @@ func (c *Collector) init() {
 		c.Registry = &Registry{}
 	}
 	if c.byPackage == nil {
-		c.byPackage = make(map[string]map[ast.Node]MarkerValues)
+		c.byPackage = make(map[*loader.Package]map[ast.Node]MarkerValues)
 	}
 }
 
@@ -75,7 +75,7 @@ func (c *Collector) init() {
 func (c *Collector) MarkersInPackage(pkg *loader.Package) (map[ast.Node]MarkerValues, error) {
 	c.mu.Lock()
 	c.init()
-	if markers, exist := c.byPackage[pkg.ID]; exist {
+	if markers, exist := c.byPackage[pkg]; exist {
 		c.mu.Unlock()
 		return markers, nil
 	}
@@ -91,8 +91,7 @@ func (c *Collector) MarkersInPackage(pkg *loader.Package) (map[ast.Node]MarkerVa
 
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	c.byPackage[pkg.ID] = markers
-
+	c.byPackage[pkg] = markers
 	return markers, nil
 }