Fix controller-tools doesn't support single files as input

#837

pkg/crd/gen_single_file_integration_test.go

@@ -0,0 +1,108 @@
+/*
+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 crd_test
+
+import (
+	"bytes"
+	"os"
+	"path/filepath"
+
+	"github.com/google/go-cmp/cmp"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	"sigs.k8s.io/controller-tools/pkg/crd"
+	crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers"
+	"sigs.k8s.io/controller-tools/pkg/genall"
+	"sigs.k8s.io/controller-tools/pkg/loader"
+	"sigs.k8s.io/controller-tools/pkg/markers"
+)
+
+var _ = Describe("CRD Generation golang files", func() {
+	var (
+		ctx, ctx2 *genall.GenerationContext
+		out       *outputRule
+
+		genDir = filepath.Join("testdata", "multiple_files")
+	)
+
+	BeforeEach(func() {
+		By("switching into testdata to appease go modules")
+		cwd, err := os.Getwd()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(os.Chdir(genDir)).To(Succeed()) // go modules are directory-sensitive
+		defer func() { Expect(os.Chdir(cwd)).To(Succeed()) }()
+
+		By("loading the roots")
+		pkgs, err := loader.LoadRoots("file_one.go")
+		Expect(err).NotTo(HaveOccurred())
+		Expect(pkgs).To(HaveLen(1))
+		Expect(pkgs[0].GoFiles).To(HaveLen(1))
+		pkgs2, err := loader.LoadRoots("file_two.go", "file_two_reference.go")
+		Expect(err).NotTo(HaveOccurred())
+		Expect(pkgs2).To(HaveLen(1))
+		Expect(pkgs2[0].GoFiles).To(HaveLen(2))
+
+		By("setup up the context")
+		reg := &markers.Registry{}
+		Expect(crdmarkers.Register(reg)).To(Succeed())
+		out = &outputRule{
+			buf: &bytes.Buffer{},
+		}
+		ctx = &genall.GenerationContext{
+			Collector:  &markers.Collector{Registry: reg},
+			Roots:      pkgs,
+			Checker:    &loader.TypeChecker{},
+			OutputRule: out,
+		}
+		ctx2 = &genall.GenerationContext{
+			Collector:  &markers.Collector{Registry: reg},
+			Roots:      pkgs2,
+			Checker:    &loader.TypeChecker{},
+			OutputRule: out,
+		}
+	})
+
+	It("should have deterministic output for single golang file", func() {
+		By("calling Generate on single golang file")
+		gen := &crd.Generator{
+			CRDVersions: []string{"v1"},
+		}
+		Expect(gen.Generate(ctx)).NotTo(HaveOccurred())
+
+		By("loading the desired YAML")
+		expectedFileOne, err := os.ReadFile(filepath.Join(genDir, "one.example.com.yaml"))
+		Expect(err).NotTo(HaveOccurred())
+		expectedFileOne = fixAnnotations(expectedFileOne)
+		expectedOut := string(expectedFileOne)
+		Expect(out.buf.String()).To(Equal(expectedOut), cmp.Diff(out.buf.String(), expectedOut))
+	})
+	It("should have deterministic output for multiple golang files referencing other types", func() {
+		By("calling Generate on two golang files")
+		gen := &crd.Generator{
+			CRDVersions: []string{"v1"},
+		}
+		Expect(gen.Generate(ctx2)).NotTo(HaveOccurred())
+
+		By("loading the desired YAML file")
+		expectedFileTwo, err := os.ReadFile(filepath.Join(genDir, "two.example.com.yaml"))
+		Expect(err).NotTo(HaveOccurred())
+		expectedFileTwo = fixAnnotations(expectedFileTwo)
+		expectedOut := string(expectedFileTwo)
+		Expect(out.buf.String()).To(Equal(expectedOut), cmp.Diff(out.buf.String(), expectedOut))
+	})
+})

pkg/crd/testdata/multiple_files/file_one.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 multiplefiles
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	multiver "testdata.kubebuilder.io/cronjob/multiple_versions"
+)
+
+type OneResourceSpec struct {
+	Struct multiver.OuterStruct `json:"struct,omitempty"`
+}
+
+// +kubebuilder:object:root=true
+// +kubebuilder:subresource:status
+// +kubebuilder:resource:singular=oneresource
+
+type OneResource struct {
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+
+	Spec OneResourceSpec `json:"spec"`
+}
+
+// +kubebuilder:object:root=true
+
+type OneResourceList struct {
+	metav1.TypeMeta `json:",inline"`
+	metav1.ListMeta `json:"metadata,omitempty"`
+	Items           []OneResource `json:"items"`
+}

pkg/crd/testdata/multiple_files/file_two.go

@@ -0,0 +1,34 @@
+/*
+
+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 multiplefiles
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// +kubebuilder:object:root=true
+// +kubebuilder:subresource:status
+// +kubebuilder:resource:singular=tworesource
+
+type TwoResource struct {
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+
+	Spec TwoResourceSpec `json:"spec"`
+}
+
+// +kubebuilder:object:root=true

pkg/crd/testdata/multiple_files/file_two_reference.go

@@ -0,0 +1,31 @@
+/*
+
+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 multiplefiles
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	multiver "testdata.kubebuilder.io/cronjob/multiple_versions"
+)
+
+type TwoResourceSpec struct {
+	Struct multiver.OuterStruct `json:"struct,omitempty"`
+}
+
+type TwoResourceList struct {
+	metav1.TypeMeta `json:",inline"`
+	metav1.ListMeta `json:"metadata,omitempty"`
+	Items           []TwoResource `json:"items"`
+}

pkg/crd/testdata/multiple_files/one.example.com.yaml

@@ -0,0 +1,51 @@
+---
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  annotations:
+    controller-gen.kubebuilder.io/version: (unknown)
+  name: oneresources.testdata.kubebuilder.io
+spec:
+  group: testdata.kubebuilder.io
+  names:
+    kind: OneResource
+    listKind: OneResourceList
+    plural: oneresources
+    singular: oneresource
+  scope: Namespaced
+  versions:
+  - name: multiplefiles
+    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_files/two.example.com.yaml

@@ -0,0 +1,51 @@
+---
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  annotations:
+    controller-gen.kubebuilder.io/version: (unknown)
+  name: tworesources.testdata.kubebuilder.io
+spec:
+  group: testdata.kubebuilder.io
+  names:
+    kind: TwoResource
+    listKind: TwoResourceList
+    plural: tworesources
+    singular: tworesource
+  scope: Namespaced
+  versions:
+  - name: multiplefiles
+    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/loader/loader.go

@@ -389,12 +389,6 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, err
 		}
 	}()
 
-	// uniquePkgIDs is used to keep track of the discovered packages to be nice
-	// and try and prevent packages from showing up twice when nested module
-	// support is enabled. there is not harm that comes from this per se, but
-	// it makes testing easier when a known number of modules can be asserted
-	uniquePkgIDs := sets.String{}
-
 	// loadPackages returns the Go packages for the provided roots
 	//
 	// if validatePkgFn is nil, a package will be returned in the slice,
@@ -412,10 +406,7 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, err
 		var pkgs []*Package
 		for _, rp := range rawPkgs {
 			p := l.packageFor(rp)
-			if !uniquePkgIDs.Has(p.ID) {
-				pkgs = append(pkgs, p)
-				uniquePkgIDs.Insert(p.ID)
-			}
+			pkgs = append(pkgs, p)
 		}
 		return pkgs, nil
 	}
@@ -568,13 +559,14 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, err
 	for _, r := range fspRoots {
 		b, d := filepath.Base(r), filepath.Dir(r)
 
-		// we want the base part of the path to be either "..." or ".", except
-		// Go's filepath utilities clean paths during manipulation, removing the
-		// ".". thus, if not "...", let's update the path components so that:
+		// we want the base part of the path to be either "..." or ".", except Go's
+		// filepath utilities clean paths during manipulation or go file path,
+		// removing the ".". thus, if not "..." or go file, let's update the path
+		// components so that:
 		//
 		//   d = r
 		//   b = "."
-		if b != "..." {
+		if b != "..." && filepath.Ext(b) != ".go" {
 			d = r
 			b = "."
 		}