Skip to content

Commit

Permalink
Move demandDirectoryRoot into kyaml/filesys (#4677)
Browse files Browse the repository at this point in the history
* Move demandDirectoryRoot into kyaml/filesys

* Make root directory platform-agnostic

Support windows root directory. Dogfood own error package.

* Use cleaner windows support implementation

* Address feedback

* Address feedback x2

* Re-apply go.sum changes to avoid CI errors
  • Loading branch information
annasong20 authored Jun 30, 2022
1 parent 7e0fd02 commit d1102fb
Show file tree
Hide file tree
Showing 49 changed files with 368 additions and 145 deletions.
1 change: 1 addition & 0 deletions api/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,7 @@ golang.org/x/sys v0.0.0-20210315160823-c6e025ad8005/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20210320140829-1e4c9ba3b0c4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210403161142-5e06dd20ab57/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210510120138-977fb7262007 h1:gG67DSER+11cZvqIMb8S8bt0vZtiN6xWYARwirrOSfE=
golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
Expand Down
2 changes: 1 addition & 1 deletion api/internal/target/kusttarget.go
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ func (kt *KustTarget) accumulateResources(
// try loading resource as file then as base (directory or git repository)
if errF := kt.accumulateFile(ra, path); errF != nil {
// not much we can do if the error is an HTTP error so we bail out
if errors.Is(errF, load.ErrorHTTP) {
if errors.Is(errF, load.ErrHTTP) {
return nil, errF
}
ldr, err := kt.ldr.New(path)
Expand Down
3 changes: 2 additions & 1 deletion api/krusty/component_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"testing"

"sigs.k8s.io/kustomize/api/konfig"
"sigs.k8s.io/kustomize/api/loader"
kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
)

Expand Down Expand Up @@ -551,7 +552,7 @@ components:
`),
},
runPath: "filesincomponents",
expectedError: "'/filesincomponents/stub.yaml' must be a directory so that it can used as a build root",
expectedError: fmt.Sprintf("%s: '%s'", loader.ErrRtNotDir.Error(), "/filesincomponents/stub.yaml"),
},
"invalid-component-api-version": {
input: []FileGen{writeTestBase, writeOverlayProd,
Expand Down
2 changes: 1 addition & 1 deletion api/krusty/remoteload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ func TestRemoteResourceWithHttpError(t *testing.T) {

_, err := b.Run(fSys, tmpDir.String())

httpErr := fmt.Errorf("%w: status code %d (%s)", loader.ErrorHTTP, 404, http.StatusText(404))
httpErr := fmt.Errorf("%w: status code %d (%s)", loader.ErrHTTP, 404, http.StatusText(404))
accuFromErr := fmt.Errorf("accumulating resources from '%s': %w", url404, httpErr)
expectedErr := fmt.Errorf("accumulating resources: %w", accuFromErr)
req.EqualErrorf(err, expectedErr.Error(), url404)
Expand Down
7 changes: 5 additions & 2 deletions api/loader/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

package loader

import "fmt"
import "sigs.k8s.io/kustomize/kyaml/errors"

var ErrorHTTP = fmt.Errorf("HTTP Error")
var (
ErrHTTP = errors.Errorf("HTTP Error")
ErrRtNotDir = errors.Errorf("must build at directory")
)
33 changes: 7 additions & 26 deletions api/loader/fileloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package loader

import (
"errors"
"fmt"
"io/ioutil"
"log"
Expand All @@ -15,6 +14,7 @@ import (

"sigs.k8s.io/kustomize/api/ifc"
"sigs.k8s.io/kustomize/api/internal/git"
"sigs.k8s.io/kustomize/kyaml/errors"
"sigs.k8s.io/kustomize/kyaml/filesys"
)

Expand Down Expand Up @@ -123,7 +123,7 @@ func (fl *fileLoader) Root() string {
func newLoaderOrDie(
lr LoadRestrictorFunc,
fSys filesys.FileSystem, path string) *fileLoader {
root, err := demandDirectoryRoot(fSys, path)
root, err := filesys.ConfirmDir(fSys, path)
if err != nil {
log.Fatalf("unable to make loader at '%s'; %v", path, err)
}
Expand All @@ -146,30 +146,11 @@ func newLoaderAtConfirmedDir(
}
}

// Assure that the given path is in fact a directory.
func demandDirectoryRoot(
fSys filesys.FileSystem, path string) (filesys.ConfirmedDir, error) {
if path == "" {
return "", fmt.Errorf(
"loader root cannot be empty")
}
d, f, err := fSys.CleanedAbs(path)
if err != nil {
return "", err
}
if f != "" {
return "", fmt.Errorf(
"'%s' must be a directory so that it can used as a build root",
path)
}
return d, nil
}

// New returns a new Loader, rooted relative to current loader,
// or rooted in a temp directory holding a git repo clone.
func (fl *fileLoader) New(path string) (ifc.Loader, error) {
if path == "" {
return nil, fmt.Errorf("new root cannot be empty")
return nil, errors.Errorf("new root cannot be empty")
}

repoSpec, err := git.NewRepoSpecFromURL(path)
Expand All @@ -185,9 +166,9 @@ func (fl *fileLoader) New(path string) (ifc.Loader, error) {
if filepath.IsAbs(path) {
return nil, fmt.Errorf("new root '%s' cannot be absolute", path)
}
root, err := demandDirectoryRoot(fl.fSys, fl.root.Join(path))
root, err := filesys.ConfirmDir(fl.fSys, fl.root.Join(path))
if err != nil {
return nil, err
return nil, errors.WrapPrefixf(err, ErrRtNotDir.Error())
}
if err = fl.errIfGitContainmentViolation(root); err != nil {
return nil, err
Expand Down Expand Up @@ -317,9 +298,9 @@ func (fl *fileLoader) Load(path string) ([]byte, error) {
if resp.StatusCode < 200 || resp.StatusCode > 299 {
_, err := git.NewRepoSpecFromURL(path)
if err == nil {
return nil, errors.New("URL is a git repository")
return nil, errors.Errorf("URL is a git repository")
}
return nil, fmt.Errorf("%w: status code %d (%s)", ErrorHTTP, resp.StatusCode, http.StatusText(resp.StatusCode))
return nil, fmt.Errorf("%w: status code %d (%s)", ErrHTTP, resp.StatusCode, http.StatusText(resp.StatusCode))
}
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
Expand Down
16 changes: 3 additions & 13 deletions api/loader/fileloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"testing"

"github.com/stretchr/testify/require"

"sigs.k8s.io/kustomize/api/ifc"
"sigs.k8s.io/kustomize/api/internal/git"
"sigs.k8s.io/kustomize/api/konfig"
Expand Down Expand Up @@ -408,11 +407,8 @@ func TestLocalLoaderReferencingGitBase(t *testing.T) {
fSys.MkdirAll(topDir)
fSys.MkdirAll(cloneRoot + "/foo/base")

root, err := demandDirectoryRoot(fSys, topDir)
require.NoError(err)

l1 := newLoaderAtConfirmedDir(
RestrictionRootOnly, root, fSys, nil,
RestrictionRootOnly, filesys.ConfirmedDir(topDir), fSys, nil,
git.DoNothingCloner(filesys.ConfirmedDir(cloneRoot)))
require.Equal(topDir, l1.Root())

Expand All @@ -430,11 +426,8 @@ func TestRepoDirectCycleDetection(t *testing.T) {
fSys.MkdirAll(topDir)
fSys.MkdirAll(cloneRoot)

root, err := demandDirectoryRoot(fSys, topDir)
require.NoError(err)

l1 := newLoaderAtConfirmedDir(
RestrictionRootOnly, root, fSys, nil,
RestrictionRootOnly, filesys.ConfirmedDir(topDir), fSys, nil,
git.DoNothingCloner(filesys.ConfirmedDir(cloneRoot)))
p1 := "github.com/someOrg/someRepo/foo"
rs1, err := git.NewRepoSpecFromURL(p1)
Expand All @@ -455,11 +448,8 @@ func TestRepoIndirectCycleDetection(t *testing.T) {
fSys.MkdirAll(topDir)
fSys.MkdirAll(cloneRoot)

root, err := demandDirectoryRoot(fSys, topDir)
require.NoError(err)

l0 := newLoaderAtConfirmedDir(
RestrictionRootOnly, root, fSys, nil,
RestrictionRootOnly, filesys.ConfirmedDir(topDir), fSys, nil,
git.DoNothingCloner(filesys.ConfirmedDir(cloneRoot)))

p1 := "github.com/someOrg/someRepo1"
Expand Down
5 changes: 3 additions & 2 deletions api/loader/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package loader
import (
"sigs.k8s.io/kustomize/api/ifc"
"sigs.k8s.io/kustomize/api/internal/git"
"sigs.k8s.io/kustomize/kyaml/errors"
"sigs.k8s.io/kustomize/kyaml/filesys"
)

Expand All @@ -25,9 +26,9 @@ func NewLoader(
return newLoaderAtGitClone(
repoSpec, fSys, nil, git.ClonerUsingGitExec)
}
root, err := demandDirectoryRoot(fSys, target)
root, err := filesys.ConfirmDir(fSys, target)
if err != nil {
return nil, err
return nil, errors.WrapPrefixf(err, ErrRtNotDir.Error())
}
return newLoaderAtConfirmedDir(
lr, root, fSys, nil, git.ClonerUsingGitExec), nil
Expand Down
1 change: 1 addition & 0 deletions cmd/config/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,7 @@ golang.org/x/sys v0.0.0-20210315160823-c6e025ad8005/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20210320140829-1e4c9ba3b0c4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210403161142-5e06dd20ab57/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210510120138-977fb7262007 h1:gG67DSER+11cZvqIMb8S8bt0vZtiN6xWYARwirrOSfE=
golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
Expand Down
1 change: 1 addition & 0 deletions cmd/pluginator/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ golang.org/x/sys v0.0.0-20210315160823-c6e025ad8005/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20210320140829-1e4c9ba3b0c4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210403161142-5e06dd20ab57/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210510120138-977fb7262007 h1:gG67DSER+11cZvqIMb8S8bt0vZtiN6xWYARwirrOSfE=
golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
Expand Down
1 change: 1 addition & 0 deletions kustomize/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ golang.org/x/sys v0.0.0-20210315160823-c6e025ad8005/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20210320140829-1e4c9ba3b0c4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210403161142-5e06dd20ab57/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210510120138-977fb7262007 h1:gG67DSER+11cZvqIMb8S8bt0vZtiN6xWYARwirrOSfE=
golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
Expand Down
33 changes: 33 additions & 0 deletions kyaml/filesys/filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
package filesys

import (
"fmt"
"path/filepath"

"sigs.k8s.io/kustomize/kyaml/errors"
)

const (
Expand All @@ -16,40 +19,70 @@ const (
// FileSystem groups basic os filesystem methods.
// It's supposed be functional subset of https://golang.org/pkg/os
type FileSystem interface {

// Create a file.
Create(path string) (File, error)

// MkDir makes a directory.
Mkdir(path string) error

// MkDirAll makes a directory path, creating intervening directories.
MkdirAll(path string) error

// RemoveAll removes path and any children it contains.
RemoveAll(path string) error

// Open opens the named file for reading.
Open(path string) (File, error)

// IsDir returns true if the path is a directory.
IsDir(path string) bool

// ReadDir returns a list of files and directories within a directory.
ReadDir(path string) ([]string, error)

// CleanedAbs converts the given path into a
// directory and a file name, where the directory
// is represented as a ConfirmedDir and all that implies.
// If the entire path is a directory, the file component
// is an empty string.
CleanedAbs(path string) (ConfirmedDir, string, error)

// Exists is true if the path exists in the file system.
Exists(path string) bool

// Glob returns the list of matching files,
// emulating https://golang.org/pkg/path/filepath/#Glob
Glob(pattern string) ([]string, error)

// ReadFile returns the contents of the file at the given path.
ReadFile(path string) ([]byte, error)

// WriteFile writes the data to a file at the given path,
// overwriting anything that's already there.
WriteFile(path string, data []byte) error

// Walk walks the file system with the given WalkFunc.
Walk(path string, walkFn filepath.WalkFunc) error
}

// ConfirmDir returns an error if the user-specified path is not an existing directory on fSys.
// Otherwise, ConfirmDir returns path, which can be relative, as a ConfirmedDir and all that implies.
func ConfirmDir(fSys FileSystem, path string) (ConfirmedDir, error) {
if path == "" {
return "", errors.Errorf("directory path cannot be empty")
}

d, f, err := fSys.CleanedAbs(path)
if err != nil {
return "", errors.WrapPrefixf(err, "not a valid directory")
}
if f != "" {
return "", errors.WrapPrefixf(errors.Errorf("file is not directory"), fmt.Sprintf("'%s'", path))
}
return d, nil
}

// FileSystemOrOnDisk satisfies the FileSystem interface by forwarding
// all of its method calls to the given FileSystem whenever it's not nil.
// If it's nil, the call is forwarded to the OS's underlying file system.
Expand Down
Loading

0 comments on commit d1102fb

Please sign in to comment.