Cross-module support

This patch updates the controller-gen package loader to support
loading packages across Go module boundaries.

pkg/crd/schema_test.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2019 The Kubernetes Authors.
+Copyright 2019-2022 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.
@@ -46,7 +46,7 @@ func transform(t *testing.T, expr string) *apiext.JSONSchemaProps {
 		},
 	}
 
-	pkgs, exported, err := testloader.LoadFakeRoots(pkgstest.Modules, modules, moduleName)
+	pkgs, exported, err := testloader.LoadFakeRoots(pkgstest.Modules, modules, "./.")
 	if exported != nil {
 		t.Cleanup(exported.Cleanup)
 	}

pkg/deepcopy/deepcopy_integration_test.go

@@ -86,9 +86,30 @@ var _ = Describe("CRD Generation From Parsing to CustomResourceDefinition", func
 		Expect(err).NotTo(HaveOccurred())
 		rt.OutputRules = genall.OutputRules{Default: output}
 
+		// TODO(akutz) This did not used to be necessary for some reason, as it
+		//             seems pkg/loader used to auto-magically act against the
+		//             working directory, even if not roots were specified. this
+		//             behavior is not documented anywhere, and I was torn as to
+		//             put the commented-out code *here* or update the loader to
+		//             assume roots=["."] if len(roots) == 0. For the sake of
+		//             backwards-compatibility I opted for the latter.
+		//
+		//             this is a reminder to sort this out and determine what
+		//             the expected, desired behavior should be when roots is
+		//             omitted
+		//
+		// By("loading the roots")
+		// pkgs, err := loader.LoadRoots(".")
+		// Expect(err).NotTo(HaveOccurred())
+		// Expect(pkgs).To(HaveLen(1))
+		// rt.Roots = pkgs
+
 		By("running the generator and checking for errors")
 		hadErrs := rt.Run()
 
+		By("checking for errors")
+		Expect(hadErrs).To(BeFalse())
+
 		By("checking that we got output contents")
 		Expect(output.fileList()).To(ContainElement("zz_generated.deepcopy.go")) // Don't use HaveKey--the output is too verbose to be usable
 		outFile := output["zz_generated.deepcopy.go"]
@@ -102,8 +123,5 @@ var _ = Describe("CRD Generation From Parsing to CustomResourceDefinition", func
 
 		By("comparing the two")
 		Expect(string(outContents)).To(Equal(string(expectedFile)), "generated code not as expected, check pkg/deepcopy/testdata/README.md for more details.\n\nDiff:\n\n%s", cmp.Diff(outContents, expectedFile))
-
-		By("checking for errors")
-		Expect(hadErrs).To(BeFalse())
 	})
 })

pkg/loader/loader.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2019 The Kubernetes Authors.
+Copyright 2019-2022 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.
@@ -25,9 +25,11 @@ import (
 	"go/types"
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"sync"
 
 	"golang.org/x/tools/go/packages"
+	"k8s.io/apimachinery/pkg/util/sets"
 )
 
 // Much of this is strongly inspired by the contents of go/packages,
@@ -323,13 +325,17 @@ func LoadRoots(roots ...string) ([]*Package, error) {
 	return LoadRootsWithConfig(&packages.Config{}, roots...)
 }
 
+func debugPrintf(format string, args ...interface{}) {
+	fmt.Printf(format, args...)
+}
+
 // LoadRootsWithConfig functions like LoadRoots, except that it allows passing
 // a custom loading config.  The config will be modified to suit the needs of
 // the loader.
 //
 // This is generally only useful for use in testing when you need to modify
 // loading settings to load from a fake location.
-func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, error) {
+func LoadRootsWithConfig(cfg *packages.Config, roots ...string) (pkgs []*Package, retErr error) {
 	l := &loader{
 		cfg:      cfg,
 		packages: make(map[*packages.Package]*Package),
@@ -341,13 +347,174 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, err
 	// put our build flags first so that callers can override them
 	l.cfg.BuildFlags = append([]string{"-tags", "ignore_autogenerated"}, l.cfg.BuildFlags...)
 
-	rawPkgs, err := packages.Load(l.cfg, roots...)
-	if err != nil {
-		return nil, err
+	// ensure the cfg.Dir field is reset to its original value upon
+	// returning from this function. it should honestly be fine if it is
+	// not given most callers will not send in the cfg parameter directly,
+	// as it's largely for testing, but still, let's be good stewards.
+	defer func(d string) {
+		cfg.Dir = d
+	}(cfg.Dir)
+	debugPrintf("cfg.Dir=%s\n", cfg.Dir)
+
+	// TODO(akutz) there was an undocumented expectation in pkg/deepcopy's
+	//             integration tests that if this function's roots parameter was
+	//             omitted, then the loader would act against the current
+	//             directory. while the command line flags would prevent this,
+	//             for purposes of backwards-compatibility, the following check
+	//             is performed here instead of in the deepcopy test.
+	//
+	//             this is a reminder to sort this out and determine what the
+	//             expected, desired behavior should be when roots is omitted
+	if len(roots) == 0 {
+		debugPrintf("roots=%+v\n", roots)
+		roots = []string{"."}
 	}
 
-	for _, rawPkg := range rawPkgs {
-		l.Roots = append(l.Roots, l.packageFor(rawPkg))
+	// ensure cfg.Dir is absolute. it just makes the rest of the code easier
+	// to debug when there are any issues
+	if cfg.Dir != "" && !filepath.IsAbs(cfg.Dir) {
+		ap, err := filepath.Abs(cfg.Dir)
+		if err != nil {
+			return nil, err
+		}
+		cfg.Dir = ap
+		debugPrintf("cfg.Dir.abs=%s\n", cfg.Dir)
+	}
+
+	// store the value of cfg.Dir so we can use it later if it is non-empty.
+	// we need to store it now as the value of cfg.Dir will be updated by
+	// a loop below
+	cfgDir := cfg.Dir
+
+	// addNestedGoModulesToRoots is given to filepath.WalkDir
+	// and adds the directory part of p to the list of roots
+	// IFF p is the path to a file named "go.mod"
+	addNestedGoModulesToRoots := func(
+		p string,
+		d os.DirEntry,
+		e error) error {
+
+		if e != nil {
+			return e
+		}
+		if !d.IsDir() && filepath.Base(p) == "go.mod" {
+			roots = append(roots, filepath.Join(filepath.Dir(p), "..."))
+		}
+		return nil
+	}
+
+	// uniqueRoots 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
+	uniqueRoots := sets.String{}
+
+	// the basic loader logic is as follows:
+	//
+	//    1. iterate over the list of provided roots
+	//
+	//    2. if a root uses the nested path syntax, ex. ..., then walk the
+	//       root's descendants to search for any any nested Go modules, and if
+	//       found, load them to the list of roots
+	//
+	//    3. iterate over the list of updated roots
+	//
+	//    4. update the loader config's Dir property to be the directory element
+	//       of the current root
+	//
+	//    5. execute the loader on the base element of the current root, which
+	//       will be either "./." or "./..."
+	//
+	// the following range operation is parts 1-2
+	for i, r := range roots {
+		debugPrintf("r=%s\n", r)
+
+		// clean up the root
+		r := filepath.Clean(r)
+		debugPrintf("r.cleaned=%s\n", r)
+
+		// get the absolute path of the root
+		if !filepath.IsAbs(r) {
+
+			// if the initial value of cfg.Dir was non-empty then use it when
+			// building the absolute path to this root. otherwise use the
+			// filepath.Abs function to get the absolute path of the root based
+			// on the working directory
+			if cfgDir != "" {
+				r = filepath.Join(cfgDir, r)
+			} else {
+				ar, err := filepath.Abs(r)
+				if err != nil {
+					return nil, err
+				}
+				r = ar
+			}
+		}
+		debugPrintf("r.abs=%s\n", r)
+
+		// update the root to be an absolute path
+		roots[i] = r
+
+		b, d := filepath.Base(r), filepath.Dir(r)
+		debugPrintf("r=%s,d=%s,b=%s\n", r, d, b)
+
+		// if the base element is "..." then it means nested traversal is
+		// activated. this can be passed directly to the loader. however, if
+		// specified we also want to traverse the path manually to determine if
+		// there are any nested Go modules we want to add to the list of roots
+		// to process
+		if b == "..." {
+			if err := filepath.WalkDir(
+				d,
+				addNestedGoModulesToRoots); err != nil {
+
+				return nil, err
+			}
+		}
+	}
+
+	// this range operation is parts 3-5 from above.
+	for _, r := range roots {
+		b, d := filepath.Base(r), filepath.Dir(r)
+		debugPrintf("r=%s,d=%s,b=%s\n", r, d, b)
+
+		// 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:
+		//
+		//   d = r
+		//   b = "."
+		if b != "..." {
+			d = r
+			b = "."
+		}
+
+		// update the loader configuration's Dir field to the directory part of
+		// the root
+		l.cfg.Dir = d
+
+		// update the root to be "./..." or "./."
+		// (with OS-specific filepath separator). please note filepath.Join
+		// would clean up the trailing "." character that we want preserved,
+		// hence the more manual path concatenation logic
+		r = fmt.Sprintf(".%s%s", string(filepath.Separator), b)
+		debugPrintf("r=%s,d=%s,b=%s\n", r, d, b)
+
+		// load the root and gets its raw packages
+		rawPkgs, err := packages.Load(l.cfg, r)
+		if err != nil {
+			return nil, err
+		}
+
+		// get the package path for each raw package, retaining only those
+		// packages with unique IDs
+		for _, rawPkg := range rawPkgs {
+			pkg := l.packageFor(rawPkg)
+			if !uniqueRoots.Has(pkg.ID) {
+				l.Roots = append(l.Roots, pkg)
+				uniqueRoots.Insert(pkg.ID)
+			}
+		}
 	}
 
 	return l.Roots, nil

pkg/loader/loader_suite_test.go

@@ -0,0 +1,29 @@
+/*
+Copyright 2022 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 loader_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func TestLoader(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Loader Patching Suite")
+}