Skip to content

Commit

Permalink
Cross-module support for filesystem paths
Browse files Browse the repository at this point in the history
This patch updates the controller-gen package loader to support
loading packages across Go module boundaries for filesystem paths.

What does this actually mean? Imagine a project with the following
structure:

my-project/
|
|-- go.mod
|-- go.sum
|
|-- apis/
    |
    |-- v1alpha1/
        |
        |-- go.mod
        |-- go.sum

Prior to this patch, the following command, executed from the root of
"my-project," would *not* include the package my-project/apis/v1alpha1:

    controller-gen crd \
      paths=./apis/... \
      crd:trivialVersions=true \
      output:crd:dir=./config/crd/bases

The reason the above command would not parse the types from the package
my-project/apis/v1alpha1 is because it is a distinct Go module from the
root module, where the command was executed. In fact, while the above
command would silently fail to include those types, the following
command will fail with an error:

    controller-gen crd \
      paths=./apis/v1alpha1 \
      crd:trivialVersions=true \
      output:crd:dir=./config/crd/bases

The above command fails because the underlying logic to load packages
cannot load a path from one package when executed from the working
directory of another package.

With this patch, it is now possible for the first command to
successfully traverse Go module boundaries for roots that are file-
system paths ending with "..." with the following, high-level 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.
  • Loading branch information
akutz committed May 3, 2022
1 parent cb13ac5 commit 2f3e040
Show file tree
Hide file tree
Showing 14 changed files with 567 additions and 6 deletions.
3 changes: 3 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ module sigs.k8s.io/controller-tools

go 1.17

replace sigs.k8s.io/controller-tools/pkg/loader/testmod => ./pkg/loader/testmod

require (
github.com/fatih/color v1.12.0
github.com/gobuffalo/flect v0.2.5
Expand Down Expand Up @@ -40,6 +42,7 @@ require (
gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect
k8s.io/klog/v2 v2.60.1 // indirect
k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9 // indirect
sigs.k8s.io/controller-tools/pkg/loader/testmod v0.0.0-00010101000000-000000000000 // indirect
sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.2.1 // indirect
)
244 changes: 238 additions & 6 deletions pkg/loader/loader.go
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
29 changes: 29 additions & 0 deletions pkg/loader/loader_suite_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
Loading

0 comments on commit 2f3e040

Please sign in to comment.