diff --git a/pkg/loader/loader.go b/pkg/loader/loader.go index 35c271335..064efa30f 100644 --- a/pkg/loader/loader.go +++ b/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,12 @@ import ( "go/types" "io/ioutil" "os" + "path/filepath" + "regexp" "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,6 +332,40 @@ 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. +// +// This function will traverse Go module boundaries for roots that are file- +// system paths and end with "...". Please note this feature currently only +// supports roots that are filesystem paths. For more information, please +// refer to the high-level outline of this function's logic: +// +// 1. If no roots are provided then load the working directory and return +// early. +// +// 2. Otherwise sort the provided roots into two, distinct buckets: +// +// a. package/module names +// b. filesystem paths +// +// A filesystem path is distinguished from a Go package/module name by +// the same rules as followed by the "go" command. At a high level, a +// root is a filesystem path IFF it meets ANY of the following criteria: +// +// * is absolute +// * begins with . +// * begins with .. +// +// For more information please refer to the output of the command +// "go help packages". +// +// 3. Load the package/module roots as a single call to packages.Load. If +// there are no filesystem path roots then return early. +// +// 4. For filesystem path roots ending with "...", check to see if its +// descendants include any nested, Go modules. If so, add the directory +// that contains the nested Go module to the filesystem path roots. +// +// 5. Load the filesystem path roots and return the load packages for the +// package/module roots AND the filesystem path roots. func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, error) { l := &loader{ cfg: cfg, @@ -341,13 +378,208 @@ 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 + // 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, + // otherwise the package is only returned if the result of + // validatePkgFn(pkg.ID) is truthy + loadPackages := func(roots ...string) ([]*Package, error) { + rawPkgs, err := packages.Load(l.cfg, roots...) + if err != nil { + return nil, 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) + } + } + return pkgs, nil + } + + // if no roots were provided then load the current package and return early + if len(roots) == 0 { + pkgs, err := loadPackages() + if err != nil { + return nil, err + } + l.Roots = append(l.Roots, pkgs...) + return l.Roots, nil + } + + // pkgRoots is a slice of roots that are package/modules and fspRoots + // is a slice of roots that are local filesystem paths. + // + // please refer to this function's godoc comments for more information on + // how these two types of roots are distinguished from one another + var ( + pkgRoots []string + fspRoots []string + fspRootRx = regexp.MustCompile(`^\.{1,2}`) + ) + for _, r := range roots { + if filepath.IsAbs(r) || fspRootRx.MatchString(r) { + fspRoots = append(fspRoots, r) + } else { + pkgRoots = append(pkgRoots, r) + } + } + + // handle the package roots by sending them into the packages.Load function + // all at once. this is more efficient, but cannot be used for the file- + // system path roots due to them needing a custom, calculated value for the + // cfg.Dir field + if len(pkgRoots) > 0 { + pkgs, err := loadPackages(pkgRoots...) + if err != nil { + return nil, err + } + l.Roots = append(l.Roots, pkgs...) + } + + // if there are no filesystem path roots then go ahead and return early + if len(fspRoots) == 0 { + return l.Roots, nil + } + + // + // at this point we are handling filesystem path roots + // + + // 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) + + // 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 filesystem path 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" { + fspRoots = append(fspRoots, filepath.Join(filepath.Dir(p), "...")) + } + return nil } - for _, rawPkg := range rawPkgs { - l.Roots = append(l.Roots, l.packageFor(rawPkg)) + // in the first pass over the filesystem path roots we: + // + // 1. make the root into an absolute path + // + // 2. check to see if a root uses the nested path syntax, ex. ... + // + // 3. if so, walk the root's descendants, searching for any nested Go + // modules + // + // 4. if found then the directory containing the Go module is added to + // the list of the filesystem path roots + for i := range fspRoots { + r := fspRoots[i] + + // clean up the root + r = filepath.Clean(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 + } + } + + // update the root to be an absolute path + fspRoots[i] = r + + b, d := filepath.Base(r), filepath.Dir(r) + + // 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 file- + // system path roots to process + if b == "..." { + if err := filepath.WalkDir( + d, + addNestedGoModulesToRoots); err != nil { + + return nil, err + } + } + } + + // in the second pass over the filesystem path roots we: + // + // 1. determine the directory from which to execute the loader + // + // 2. update the loader config's Dir property to be the directory from + // step one + // + // 3. determine whether the root passed to the loader should be "./." + // or "./..." + // + // 4. execute the loader with the value from step three + 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: + // + // 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) + + // load the packages from the roots + pkgs, err := loadPackages(r) + if err != nil { + return nil, err + } + l.Roots = append(l.Roots, pkgs...) } return l.Roots, nil diff --git a/pkg/loader/loader_suite_test.go b/pkg/loader/loader_suite_test.go new file mode 100644 index 000000000..32cbffc87 --- /dev/null +++ b/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") +} diff --git a/pkg/loader/loader_test.go b/pkg/loader/loader_test.go new file mode 100644 index 000000000..a1ba6f339 --- /dev/null +++ b/pkg/loader/loader_test.go @@ -0,0 +1,191 @@ +/* +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 ( + "os" + + . "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 named packages/modules", func() { + + // we need to change into the directory of ./testmod in order to execute + // tests due to the inability to place the replace statement in the + // project's root go.mod file (this breaks "go install") + var previousWorkingDir string + BeforeEach(func() { + cwd, err := os.Getwd() + Expect(err).ToNot(HaveOccurred()) + Expect(cwd).ToNot(BeEmpty()) + previousWorkingDir = cwd + Expect(os.Chdir("./testmod")).To(Succeed()) + }) + AfterEach(func() { + Expect(os.Chdir(previousWorkingDir)).To(Succeed()) + }) + + Context("with roots=[sigs.k8s.io/controller-tools/pkg/loader/testmod/submod1]", func() { + It("should load one package", func() { + pkgs, err := loader.LoadRoots("sigs.k8s.io/controller-tools/pkg/loader/testmod/submod1") + Expect(err).ToNot(HaveOccurred()) + Expect(pkgs).To(HaveLen(1)) + assertPkgExists(testmodPkg+"/submod1", pkgs) + }) + }) + + Context("with roots=[sigs.k8s.io/controller-tools/pkg/loader/testmod/...]", func() { + It("should load six packages", func() { + pkgs, err := loader.LoadRoots("sigs.k8s.io/controller-tools/pkg/loader/testmod/...") + Expect(err).ToNot(HaveOccurred()) + Expect(pkgs).To(HaveLen(6)) + assertPkgExists(testmodPkg, pkgs) + assertPkgExists(testmodPkg+"/subdir1", pkgs) + assertPkgExists(testmodPkg+"/subdir1/subdir1", pkgs) + assertPkgExists(testmodPkg+"/subdir1/subdir2", pkgs) + assertPkgExists(testmodPkg+"/submod1", pkgs) + assertPkgExists(testmodPkg+"/submod1/subdir1", pkgs) + }) + }) + + Context("with roots=[sigs.k8s.io/controller-tools/pkg/loader/testmod/..., ./...]", func() { + It("should load seven packages", func() { + pkgs, err := loader.LoadRoots("sigs.k8s.io/controller-tools/pkg/loader/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=[../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 seven 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 seven 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) + }) + }) +}) diff --git a/pkg/loader/testmod/dummy.go b/pkg/loader/testmod/dummy.go new file mode 100644 index 000000000..a52e9a08c --- /dev/null +++ b/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 diff --git a/pkg/loader/testmod/go.mod b/pkg/loader/testmod/go.mod new file mode 100644 index 000000000..ac7d2b050 --- /dev/null +++ b/pkg/loader/testmod/go.mod @@ -0,0 +1,7 @@ +module sigs.k8s.io/controller-tools/pkg/loader/testmod + +go 1.17 + +replace sigs.k8s.io/controller-tools/pkg/loader/testmod/submod1 => ./submod1 + +require sigs.k8s.io/controller-tools/pkg/loader/testmod/submod1 v0.0.0-00010101000000-000000000000 // indirect diff --git a/pkg/loader/testmod/subdir1/dummy.go b/pkg/loader/testmod/subdir1/dummy.go new file mode 100644 index 000000000..a52e9a08c --- /dev/null +++ b/pkg/loader/testmod/subdir1/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 diff --git a/pkg/loader/testmod/subdir1/subdir1/dummy.go b/pkg/loader/testmod/subdir1/subdir1/dummy.go new file mode 100644 index 000000000..a52e9a08c --- /dev/null +++ b/pkg/loader/testmod/subdir1/subdir1/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 diff --git a/pkg/loader/testmod/subdir1/subdir2/dummy.go b/pkg/loader/testmod/subdir1/subdir2/dummy.go new file mode 100644 index 000000000..a52e9a08c --- /dev/null +++ b/pkg/loader/testmod/subdir1/subdir2/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 diff --git a/pkg/loader/testmod/subdir1/submod1/dummy.go b/pkg/loader/testmod/subdir1/submod1/dummy.go new file mode 100644 index 000000000..a52e9a08c --- /dev/null +++ b/pkg/loader/testmod/subdir1/submod1/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 diff --git a/pkg/loader/testmod/subdir1/submod1/go.mod b/pkg/loader/testmod/subdir1/submod1/go.mod new file mode 100644 index 000000000..04113b125 --- /dev/null +++ b/pkg/loader/testmod/subdir1/submod1/go.mod @@ -0,0 +1,3 @@ +module sigs.k8s.io/controller-tools/pkg/loader/testmod/subdir1/submod1 + +go 1.17 diff --git a/pkg/loader/testmod/submod1/dummy.go b/pkg/loader/testmod/submod1/dummy.go new file mode 100644 index 000000000..a52e9a08c --- /dev/null +++ b/pkg/loader/testmod/submod1/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 diff --git a/pkg/loader/testmod/submod1/go.mod b/pkg/loader/testmod/submod1/go.mod new file mode 100644 index 000000000..a1e0d72c6 --- /dev/null +++ b/pkg/loader/testmod/submod1/go.mod @@ -0,0 +1,3 @@ +module sigs.k8s.io/controller-tools/pkg/loader/testmod/submod1 + +go 1.17 diff --git a/pkg/loader/testmod/submod1/subdir1/dummy.go b/pkg/loader/testmod/submod1/subdir1/dummy.go new file mode 100644 index 000000000..a52e9a08c --- /dev/null +++ b/pkg/loader/testmod/submod1/subdir1/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