Cross-module support

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

pkg/crd/schema_test.go

@@ -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/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,
@@ -329,7 +331,7 @@ func LoadRoots(roots ...string) ([]*Package, error) {
 //
 // 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 +343,163 @@ 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)
+	//fmt.Printf("cfg.Dir=%s\n", cfg.Dir)
+
+	// ensure cfg.Dir is absolute. it just makes the rest of the code easier
+	// to debug when there are any issues
+	if cfg.Dir != "" {
+		if !filepath.IsAbs(cfg.Dir) {
+			ap, err := filepath.Abs(cfg.Dir)
+			if err != nil {
+				return nil, err
+			}
+			cfg.Dir = ap
+		}
+		//fmt.Printf("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
 	}
 
-	for _, rawPkg := range rawPkgs {
-		l.Roots = append(l.Roots, l.packageFor(rawPkg))
+	// 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 {
+		//fmt.Printf("r=%s\n", r)
+
+		// clean up the root
+		r := filepath.Clean(r)
+		//fmt.Printf("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
+			}
+		}
+		//fmt.Printf("r.abs=%s\n", r)
+
+		// update the root to be an absolute path
+		roots[i] = r
+
+		b, d := filepath.Base(r), filepath.Dir(r)
+		//fmt.Printf("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, but for nested, Go modules
+		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)
+		//fmt.Printf("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)
+		//fmt.Printf("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")
+}

pkg/loader/loader_test.go

@@ -0,0 +1,133 @@
+/*
+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 (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	"sigs.k8s.io/controller-tools/pkg/loader"
+)
+
+var _ = Describe("Loader parsing root module", func() {
+	const (
+		rootPkg    = "sigs.k8s.io/controller-tools"
+		pkgPkg     = rootPkg + "/pkg"
+		loaderPkg  = pkgPkg + "/loader"
+		testmodPkg = loaderPkg + "/testmod"
+	)
+
+	var indexOfPackage = func(pkgID string, pkgs []*loader.Package) int {
+		for i := range pkgs {
+			if pkgs[i].ID == pkgID {
+				return i
+			}
+		}
+		return -1
+	}
+
+	var assertPkgExists = func(pkgID string, pkgs []*loader.Package) {
+		ExpectWithOffset(1, indexOfPackage(pkgID, pkgs)).Should(BeNumerically(">", -1))
+	}
+
+	Context("with roots=[../crd/.]", func() {
+		It("should load one package", func() {
+			pkgs, err := loader.LoadRoots("../crd/.")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(pkgs).To(HaveLen(1))
+			assertPkgExists(pkgPkg+"/crd", pkgs)
+		})
+	})
+
+	Context("with roots=[./]", func() {
+		It("should load one package", func() {
+			pkgs, err := loader.LoadRoots("./")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(pkgs).To(HaveLen(1))
+			assertPkgExists(loaderPkg, pkgs)
+		})
+	})
+
+	Context("with roots=[../../pkg/loader]", func() {
+		It("should load one package", func() {
+			pkgs, err := loader.LoadRoots("../../pkg/loader")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(pkgs).To(HaveLen(1))
+			assertPkgExists(loaderPkg, pkgs)
+		})
+	})
+
+	Context("with roots=[../../pkg/loader/../loader/testmod/..., ./testmod/./../testmod//.]", func() {
+		It("should load six packages", func() {
+			pkgs, err := loader.LoadRoots(
+				"../../pkg/loader/../loader/testmod/...",
+				"./testmod/./../testmod//.")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(pkgs).To(HaveLen(7))
+			assertPkgExists(testmodPkg, pkgs)
+			assertPkgExists(testmodPkg+"/subdir1", pkgs)
+			assertPkgExists(testmodPkg+"/subdir1/subdir1", pkgs)
+			assertPkgExists(testmodPkg+"/subdir1/subdir2", pkgs)
+			assertPkgExists(testmodPkg+"/subdir1/submod1", pkgs)
+			assertPkgExists(testmodPkg+"/submod1", pkgs)
+			assertPkgExists(testmodPkg+"/submod1/subdir1", pkgs)
+		})
+	})
+
+	Context("with roots=[./testmod/...]", func() {
+		It("should load six packages", func() {
+			pkgs, err := loader.LoadRoots("./testmod/...")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(pkgs).To(HaveLen(7))
+			assertPkgExists(testmodPkg, pkgs)
+			assertPkgExists(testmodPkg+"/subdir1", pkgs)
+			assertPkgExists(testmodPkg+"/subdir1/subdir1", pkgs)
+			assertPkgExists(testmodPkg+"/subdir1/subdir2", pkgs)
+			assertPkgExists(testmodPkg+"/subdir1/submod1", pkgs)
+			assertPkgExists(testmodPkg+"/submod1", pkgs)
+			assertPkgExists(testmodPkg+"/submod1/subdir1", pkgs)
+		})
+	})
+
+	Context("with roots=[./testmod/subdir1/submod1/...]", func() {
+		It("should load one package", func() {
+			pkgs, err := loader.LoadRoots("./testmod/subdir1/submod1/...")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(pkgs).To(HaveLen(1))
+			assertPkgExists(testmodPkg+"/subdir1/submod1", pkgs)
+		})
+	})
+
+	Context("with roots=[./testmod, ./testmod/submod1]", func() {
+		It("should load two packages", func() {
+			pkgs, err := loader.LoadRoots("./testmod", "./testmod/submod1")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(pkgs).To(HaveLen(2))
+			assertPkgExists(testmodPkg, pkgs)
+			assertPkgExists(testmodPkg+"/submod1", pkgs)
+		})
+	})
+
+	Context("with roots=[./testmod/submod1/subdir1/]", func() {
+		It("should load one package", func() {
+			pkgs, err := loader.LoadRoots("./testmod/submod1/subdir1/")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(pkgs).To(HaveLen(1))
+			assertPkgExists(testmodPkg+"/submod1/subdir1", pkgs)
+		})
+	})
+})

pkg/loader/testmod/dummy.go

@@ -0,0 +1,17 @@
+/*
+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 dummy

pkg/loader/testmod/go.mod

@@ -0,0 +1,3 @@
+module sigs.k8s.io/controller-tools/pkg/loader/testmod
+
+go 1.17