🐛 fix: Don't flatten schemas for type idents we don't know about

pkg/schemapatcher/gen.go

@@ -75,6 +75,10 @@ type Generator struct {
 
 	// GenerateEmbeddedObjectMeta specifies if any embedded ObjectMeta in the CRD should be generated
 	GenerateEmbeddedObjectMeta *bool `marker:",optional"`
+
+	// AllowMultiPackageGroups specifies whether cases where a group is used for multiple packages should be allowed,
+	// rather than the default behavior of failing.
+	AllowMultiPackageGroups *bool `marker:",optional"`
 }
 
 var _ genall.Generator = &Generator{}
@@ -128,6 +132,12 @@ func (g Generator) Generate(ctx *genall.GenerationContext) (result error) {
 			}
 
 			typeIdent := crdgen.TypeIdent{Package: pkg, Name: groupKind.Kind}
+			if _, ok := parser.Types[typeIdent]; !ok {
+				if g.AllowMultiPackageGroups != nil && *g.AllowMultiPackageGroups {
+					continue
+				}
+				return fmt.Errorf("failed to find type for kind %s in package %s", groupKind.Kind, pkg.PkgPath)
+			}
 			parser.NeedFlattenedSchemaFor(typeIdent)
 
 			fullSchema := parser.FlattenedSchemata[typeIdent]

pkg/schemapatcher/gen_integration_test.go

@@ -41,7 +41,7 @@ var _ = Describe("CRD Patching From Parsing to Editing", func() {
 		var crdSchemaGen genall.Generator = &Generator{
 			ManifestsPath: "./invalid",
 		}
-		rt, err := genall.Generators{&crdSchemaGen}.ForRoots("./...")
+		rt, err := genall.Generators{&crdSchemaGen}.ForRoots("./apis/kubebuilder/...", "./apis/legacy/...")
 		Expect(err).NotTo(HaveOccurred())
 
 		outputDir, err := ioutil.TempDir("", "controller-tools-test")
@@ -67,7 +67,7 @@ var _ = Describe("CRD Patching From Parsing to Editing", func() {
 		var crdSchemaGen genall.Generator = &Generator{
 			ManifestsPath: "./valid",
 		}
-		rt, err := genall.Generators{&crdSchemaGen}.ForRoots("./...")
+		rt, err := genall.Generators{&crdSchemaGen}.ForRoots("./apis/kubebuilder/...", "./apis/legacy/...")
 		Expect(err).NotTo(HaveOccurred())
 
 		outputDir, err := ioutil.TempDir("", "controller-tools-test")
@@ -95,4 +95,52 @@ var _ = Describe("CRD Patching From Parsing to Editing", func() {
 			Expect(actualContents).To(Equal(expectedContents), "contents not as expected, check pkg/schemapatcher/testdata/README.md for more details.\n\nDiff:\n\n%s", cmp.Diff(string(actualContents), string(expectedContents)))
 		}
 	})
+
+	It("should fail by default with a group used in multiple packages", func() {
+		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 generation runtime")
+		var crdSchemaGen genall.Generator = &Generator{
+			ManifestsPath: "./multipackagegroup",
+		}
+		rt, err := genall.Generators{&crdSchemaGen}.ForRoots("./...")
+		Expect(err).NotTo(HaveOccurred())
+
+		outputDir, err := ioutil.TempDir("", "controller-tools-test")
+		Expect(err).NotTo(HaveOccurred())
+		defer os.RemoveAll(outputDir)
+		rt.OutputRules.Default = genall.OutputToDirectory(outputDir)
+
+		By("running the generator")
+		Expect(rt.Run()).To(BeTrue(), "unexpectedly succeeded")
+	})
+
+	It("should succeed with a group used in multiple packages when allowMultiPackageGroup is specified", func() {
+		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 generation runtime")
+		trueBool := true
+		var crdSchemaGen genall.Generator = &Generator{
+			ManifestsPath:           "./multipackagegroup",
+			AllowMultiPackageGroups: &trueBool,
+		}
+		rt, err := genall.Generators{&crdSchemaGen}.ForRoots("./...")
+		Expect(err).NotTo(HaveOccurred())
+
+		outputDir, err := ioutil.TempDir("", "controller-tools-test")
+		Expect(err).NotTo(HaveOccurred())
+		defer os.RemoveAll(outputDir)
+		rt.OutputRules.Default = genall.OutputToDirectory(outputDir)
+
+		By("running the generator")
+		Expect(rt.Run()).To(BeFalse(), "unexpectedly had errors")
+	})
 })

pkg/schemapatcher/testdata/apis/otherpkg/v1/doc.go

@@ -0,0 +1,21 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+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.
+*/
+
+// +kubebuilder:object:generate=true
+// +groupName=kubebuilder.schemapatcher.controller-tools.sigs.k8s.io
+
+// Package v1 is the v1 version of the API. It uses kubebuilder markers with a different package and kind than in the kubebuilder package.
+package v1

pkg/schemapatcher/testdata/apis/otherpkg/v1/types_otherkind.go

@@ -0,0 +1,47 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+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.
+*/
+
+package v1
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// +kubebuilder:object:root=true
+
+// OtherKind is a kind without schema changes in a different package from kubebuilder but the same group.
+type OtherKind struct {
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+	// +required
+	Spec OtherUnchangedSpec `json:"spec"`
+}
+
+type OtherUnchangedSpec struct {
+	// foo contains foo.
+	Foo string `json:"foo"`
+	// foo contains foo.
+	Bar string `json:"bar"`
+}
+
+// +kubebuilder:object:root=true
+
+// OtherUnchangedList contains a list of OtherKind.
+type OtherUnchangedList struct {
+	metav1.TypeMeta `json:",inline"`
+	metav1.ListMeta `json:"metadata"`
+	Items           []OtherKind `json:"items"`
+}

pkg/schemapatcher/testdata/multipackagegroup/kubebuilder-example-crd.v1.yaml

@@ -0,0 +1,40 @@
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: example.kubebuilder.schemapatcher.controller-tools.sigs.k8s.io
+spec:
+  group: kubebuilder.schemapatcher.controller-tools.sigs.k8s.io
+  scope: Cluster
+  names:
+    kind: Example
+    singular: example
+    plural: examples
+    listKind: ExampleList
+  versions:
+    - name: v1
+      schema:
+        openAPIV3Schema:
+          description: Example is a kind with schema changes.
+          type: object
+          required:
+            - spec
+          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:
+              type: object
+              required:
+                - bar
+              properties:
+                bar:
+                  description: foo contains foo.
+                  type: string
+                foo:
+                  description: foo contains foo.
+                  type: string

pkg/schemapatcher/testdata/multipackagegroup/kubebuilder-unchanged-crd.yaml

@@ -0,0 +1,45 @@
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: unchanged.kubebuilder.schemapatcher.controller-tools.sigs.k8s.io
+spec:
+  group: kubebuilder.schemapatcher.controller-tools.sigs.k8s.io
+  scope: Cluster
+  names:
+    kind: Unchanged
+    singular: unchanged
+    plural: unchangeds
+    listKind: UnchangedList
+  versions:
+  - name: v1
+    schema:
+      openAPIV3Schema:
+        description: Unchanged is a kind without schema changes.
+        type: object
+        required:
+        - spec
+        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/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/api-conventions.md#types-kinds'
+            type: string
+          metadata:
+            type: object
+          spec:
+            type: object
+            required:
+            - bar
+            - foo
+            properties:
+              bar:
+                description: foo contains foo.
+                type: string
+              foo:
+                description: foo contains foo.
+                type: string

pkg/schemapatcher/testdata/multipackagegroup/otherpkg-unchanged-crd.yaml

@@ -0,0 +1,41 @@
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: otherunchanged.kubebuilder.schemapatcher.controller-tools.sigs.k8s.io
+spec:
+  group: kubebuilder.schemapatcher.controller-tools.sigs.k8s.io
+  scope: Cluster
+  names:
+    kind: OtherKind
+    singular: otherkind
+    plural: otherkinds
+    listKind: OtherKindList
+  versions:
+    - name: v1
+      schema:
+        openAPIV3Schema:
+          description: OtherKind is a kind without schema changes in a different package from kubebuilder but the same group.
+          type: object
+          required:
+            - spec
+          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:
+              type: object
+              required:
+                - bar
+                - foo
+              properties:
+                bar:
+                  description: foo contains foo.
+                  type: string
+                foo:
+                  description: foo contains foo.
+                  type: string