From d08690a6aa5dd07f402761e3985d884326994828 Mon Sep 17 00:00:00 2001 From: jregan Date: Sun, 3 Nov 2019 10:04:45 -0800 Subject: [PATCH] Improve plugin home defaulting. --- api/pgmconfig/pluginconfig.go | 71 ++++++++++-- api/pgmconfig/pluginconfig_test.go | 121 +++++++++++++++++---- api/plugins/compiler/compiler.go | 58 +++++----- api/plugins/compiler/compiler_test.go | 3 +- api/plugins/loader/loader_test.go | 7 +- api/target/diamondcomposition_test.go | 2 +- api/target/plugindir_test.go | 6 +- api/testutils/kusttest/kusttestharness.go | 12 +- api/testutils/kusttest/plugintestenv.go | 3 +- api/types/errunabletofind.go | 40 +++++++ kustomize/internal/commands/build/build.go | 7 +- 11 files changed, 263 insertions(+), 67 deletions(-) create mode 100644 api/types/errunabletofind.go diff --git a/api/pgmconfig/pluginconfig.go b/api/pgmconfig/pluginconfig.go index 18667c191..dc286303e 100644 --- a/api/pgmconfig/pluginconfig.go +++ b/api/pgmconfig/pluginconfig.go @@ -8,6 +8,7 @@ import ( "path/filepath" "runtime" + "sigs.k8s.io/kustomize/api/filesys" "sigs.k8s.io/kustomize/api/types" ) @@ -36,9 +37,12 @@ const ( DomainName = "sigs.k8s.io" ) -func EnabledPluginConfig() *types.PluginConfig { - return MakePluginConfig( - types.PluginRestrictionsNone, DefaultAbsPluginHome()) +func EnabledPluginConfig() (*types.PluginConfig, error) { + dir, err := DefaultAbsPluginHome(filesys.MakeFsOnDisk()) + if err != nil { + return nil, err + } + return MakePluginConfig(types.PluginRestrictionsNone, dir), nil } func DisabledPluginConfig() *types.PluginConfig { @@ -57,18 +61,61 @@ func MakePluginConfig( // Use an obviously erroneous path, in case it's accidentally used. const NoPluginHomeSentinal = "/no/non-builtin/plugins!" -func DefaultAbsPluginHome() string { - return filepath.Join(configRoot(), RelPluginHome) +type NotedFunc struct { + Note string + F func() string } -// Use https://github.com/kirsle/configdir instead? -func configRoot() string { - dir := os.Getenv(XdgConfigHomeEnv) - if len(dir) == 0 { - dir = filepath.Join( - HomeDir(), XdgConfigHomeEnvDefault) +func DefaultAbsPluginHome(fSys filesys.FileSystem) (string, error) { + return FirstDirThatExistsElseError( + "plugin home directory", fSys, []NotedFunc{ + { + Note: "homed in $" + KustomizePluginHomeEnv, + F: func() string { + return os.Getenv(KustomizePluginHomeEnv) + }, + }, + { + Note: "homed in $" + XdgConfigHomeEnv, + F: func() string { + return filepath.Join( + os.Getenv(XdgConfigHomeEnv), + ProgramName, RelPluginHome) + }, + }, + { + Note: "homed in default value of $" + XdgConfigHomeEnv, + F: func() string { + return filepath.Join( + HomeDir(), XdgConfigHomeEnvDefault, + ProgramName, RelPluginHome) + }, + }, + { + Note: "homed in home directory", + F: func() string { + return filepath.Join( + HomeDir(), ProgramName, RelPluginHome) + }, + }, + }) +} + +// FirstDirThatExistsElseError tests different path functions for +// existence, returning the first that works, else error if all fail. +func FirstDirThatExistsElseError( + what string, + fSys filesys.FileSystem, + pathFuncs []NotedFunc) (string, error) { + var nope []types.Pair + for _, dt := range pathFuncs { + dir := dt.F() + if fSys.Exists(dir) { + return dir, nil + } + nope = append(nope, types.Pair{Key: dt.Note, Value: dir}) } - return filepath.Join(dir, ProgramName) + return "", types.NewErrUnableToFind(what, nope) } func HomeDir() string { diff --git a/api/pgmconfig/pluginconfig_test.go b/api/pgmconfig/pluginconfig_test.go index c132b07df..c29bc3716 100644 --- a/api/pgmconfig/pluginconfig_test.go +++ b/api/pgmconfig/pluginconfig_test.go @@ -6,38 +6,121 @@ package pgmconfig import ( "os" "path/filepath" - "strings" "testing" + + "sigs.k8s.io/kustomize/api/filesys" + "sigs.k8s.io/kustomize/api/types" ) -func TestConfigDirNoXdg(t *testing.T) { - xdg, isSet := os.LookupEnv(XdgConfigHomeEnv) +func TestDefaultAbsPluginHome_NoKustomizePluginHomeEnv(t *testing.T) { + fSys := filesys.MakeFsInMemory() + keep, isSet := os.LookupEnv(KustomizePluginHomeEnv) if isSet { - os.Unsetenv(XdgConfigHomeEnv) + _ = os.Unsetenv(KustomizePluginHomeEnv) } - s := configRoot() + _, err := DefaultAbsPluginHome(fSys) if isSet { - os.Setenv(XdgConfigHomeEnv, xdg) + os.Setenv(KustomizePluginHomeEnv, keep) } - if !strings.HasSuffix( - s, - rootedPath(XdgConfigHomeEnvDefault, ProgramName)) { + if err == nil { + t.Fatalf("expected err") + } + if !types.IsErrUnableToFind(err) { + t.Fatalf("unexpected err: %v", err) + } +} + +func TestDefaultAbsPluginHome_WithKustomizePluginHomeEnv(t *testing.T) { + fSys := filesys.MakeFsInMemory() + keep, isSet := os.LookupEnv(KustomizePluginHomeEnv) + if !isSet { + keep = "whatever" + os.Setenv(KustomizePluginHomeEnv, keep) + } + fSys.Mkdir(keep) + h, err := DefaultAbsPluginHome(fSys) + if !isSet { + _ = os.Unsetenv(KustomizePluginHomeEnv) + } + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if h != keep { + t.Fatalf("unexpected config dir: %s", h) + } +} + +func TestDefaultAbsPluginHomeWithXdg(t *testing.T) { + fSys := filesys.MakeFsInMemory() + keep, isSet := os.LookupEnv(XdgConfigHomeEnv) + if !isSet { + keep = "whatever" + os.Setenv(XdgConfigHomeEnv, keep) + } + configDir := filepath.Join(keep, ProgramName, RelPluginHome) + fSys.Mkdir(configDir) + h, err := DefaultAbsPluginHome(fSys) + if !isSet { + _ = os.Unsetenv(XdgConfigHomeEnv) + } + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if h != configDir { + t.Fatalf("unexpected config dir: %s", h) + } +} + +func TestDefaultAbsPluginHomeNoConfig(t *testing.T) { + fSys := filesys.MakeFsInMemory() + keep, isSet := os.LookupEnv(XdgConfigHomeEnv) + if isSet { + _ = os.Unsetenv(XdgConfigHomeEnv) + } + _, err := DefaultAbsPluginHome(fSys) + if isSet { + os.Setenv(XdgConfigHomeEnv, keep) + } + if err == nil { + t.Fatalf("expected err") + } + if !types.IsErrUnableToFind(err) { + t.Fatalf("unexpected err: %v", err) + } +} + +func TestDefaultAbsPluginHomeNoXdgWithDotConfig(t *testing.T) { + fSys := filesys.MakeFsInMemory() + configDir := filepath.Join( + HomeDir(), XdgConfigHomeEnvDefault, ProgramName, RelPluginHome) + fSys.Mkdir(configDir) + keep, isSet := os.LookupEnv(XdgConfigHomeEnv) + if isSet { + _ = os.Unsetenv(XdgConfigHomeEnv) + } + s, _ := DefaultAbsPluginHome(fSys) + if isSet { + os.Setenv(XdgConfigHomeEnv, keep) + } + if s != configDir { t.Fatalf("unexpected config dir: %s", s) } } -func rootedPath(elem ...string) string { - return string(filepath.Separator) + filepath.Join(elem...) -} - -func TestConfigDirWithXdg(t *testing.T) { - xdg, isSet := os.LookupEnv(XdgConfigHomeEnv) - os.Setenv(XdgConfigHomeEnv, rootedPath("blah")) - s := configRoot() +func TestDefaultAbsPluginHomeNoXdgJustHomeDir(t *testing.T) { + fSys := filesys.MakeFsInMemory() + configDir := filepath.Join( + HomeDir(), ProgramName, RelPluginHome) + fSys.Mkdir(configDir) + keep, isSet := os.LookupEnv(XdgConfigHomeEnv) if isSet { - os.Setenv(XdgConfigHomeEnv, xdg) + _ = os.Unsetenv(XdgConfigHomeEnv) } - if s != rootedPath("blah", ProgramName) { + s, _ := DefaultAbsPluginHome(fSys) + if isSet { + os.Setenv(XdgConfigHomeEnv, keep) + } + if s != configDir { t.Fatalf("unexpected config dir: %s", s) } } diff --git a/api/plugins/compiler/compiler.go b/api/plugins/compiler/compiler.go index 4f1df36b6..64e3d93ae 100644 --- a/api/plugins/compiler/compiler.go +++ b/api/plugins/compiler/compiler.go @@ -12,6 +12,7 @@ import ( "strings" "time" + "sigs.k8s.io/kustomize/api/filesys" "sigs.k8s.io/kustomize/api/pgmconfig" ) @@ -29,33 +30,36 @@ type Compiler struct { // DefaultSrcRoot guesses where the user // has her ${g}/${v}/$lower(${k})/${k}.go files. -func DefaultSrcRoot() (string, error) { - var nope []string - var root string - - root = filepath.Join( - os.Getenv("GOPATH"), "src", - pgmconfig.DomainName, pgmconfig.ProgramName, pgmconfig.RelPluginHome) - if FileExists(root) { - return root, nil - } - nope = append(nope, root) - - root = pgmconfig.DefaultAbsPluginHome() - if FileExists(root) { - return root, nil - } - nope = append(nope, root) - - root = filepath.Join( - pgmconfig.HomeDir(), pgmconfig.ProgramName, pgmconfig.RelPluginHome) - if FileExists(root) { - return root, nil - } - nope = append(nope, root) - - return "", fmt.Errorf( - "no default src root; tried %v", nope) +func DefaultSrcRoot(fSys filesys.FileSystem) (string, error) { + return pgmconfig.FirstDirThatExistsElseError( + "source directory", fSys, []pgmconfig.NotedFunc{ + { + Note: "old style $GOPATH", + F: func() string { + return filepath.Join( + os.Getenv("GOPATH"), + "src", pgmconfig.DomainName, + pgmconfig.ProgramName, pgmconfig.RelPluginHome) + }, + }, + { + Note: "HOME with literal 'gopath'", + F: func() string { + return filepath.Join( + pgmconfig.HomeDir(), "gopath", + "src", pgmconfig.DomainName, + pgmconfig.ProgramName, pgmconfig.RelPluginHome) + }, + }, + { + Note: "home directory", + F: func() string { + return filepath.Join( + pgmconfig.HomeDir(), pgmconfig.DomainName, + pgmconfig.ProgramName, pgmconfig.RelPluginHome) + }, + }, + }) } // NewCompiler returns a new compiler instance. diff --git a/api/plugins/compiler/compiler_test.go b/api/plugins/compiler/compiler_test.go index 1bf3501ea..022a51b86 100644 --- a/api/plugins/compiler/compiler_test.go +++ b/api/plugins/compiler/compiler_test.go @@ -9,6 +9,7 @@ import ( "path/filepath" "testing" + "sigs.k8s.io/kustomize/api/filesys" . "sigs.k8s.io/kustomize/api/plugins/compiler" ) @@ -18,7 +19,7 @@ func TestCompiler(t *testing.T) { if err != nil { t.Errorf("failed to make temp dir: %v", err) } - srcRoot, err := DefaultSrcRoot() + srcRoot, err := DefaultSrcRoot(filesys.MakeFsOnDisk()) if err != nil { t.Error(err) } diff --git a/api/plugins/loader/loader_test.go b/api/plugins/loader/loader_test.go index 3a4373030..604c8bc26 100644 --- a/api/plugins/loader/loader_test.go +++ b/api/plugins/loader/loader_test.go @@ -57,7 +57,12 @@ func TestLoader(t *testing.T) { ldr := loadertest.NewFakeLoader("/foo") - pLdr := NewLoader(pgmconfig.EnabledPluginConfig(), rmF) + c, err := pgmconfig.EnabledPluginConfig() + if err != nil { + t.Fatal(err) + } + + pLdr := NewLoader(c, rmF) if pLdr == nil { t.Fatal("expect non-nil loader") } diff --git a/api/target/diamondcomposition_test.go b/api/target/diamondcomposition_test.go index 224e3c462..a69192cac 100644 --- a/api/target/diamondcomposition_test.go +++ b/api/target/diamondcomposition_test.go @@ -231,7 +231,7 @@ func definePatchDirStructure(th *kusttest_test.KustTestHarness) { // Fails due to file load restrictor. func TestIssue1251_Patches_ProdVsDev_Failure(t *testing.T) { - th := kusttest_test.NewKustTestHarnessAllowPlugins(t, "/app/prod") + th := kusttest_test.NewKustTestHarness(t, "/app/prod") definePatchDirStructure(th) th.WriteK("/app/prod", ` diff --git a/api/target/plugindir_test.go b/api/target/plugindir_test.go index 5c41c440a..c6c58a4be 100644 --- a/api/target/plugindir_test.go +++ b/api/target/plugindir_test.go @@ -65,7 +65,11 @@ metadata: rf := resmap.NewFactory(resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()), nil) - pl := pLdr.NewLoader(pgmconfig.EnabledPluginConfig(), rf) + c, err := pgmconfig.EnabledPluginConfig() + if err != nil { + t.Fatal(err) + } + pl := pLdr.NewLoader(c, rf) tg, err := target.NewKustTarget( ldr, valtest_test.MakeFakeValidator(), rf, transformer.NewFactoryImpl(), pl) if err != nil { diff --git a/api/testutils/kusttest/kusttestharness.go b/api/testutils/kusttest/kusttestharness.go index c106ef660..c942b4ae0 100644 --- a/api/testutils/kusttest/kusttestharness.go +++ b/api/testutils/kusttest/kusttestharness.go @@ -40,8 +40,11 @@ func NewKustTestHarness(t *testing.T, path string) *KustTestHarness { } func NewKustTestHarnessAllowPlugins(t *testing.T, path string) *KustTestHarness { - return NewKustTestHarnessFull( - t, path, fLdr.RestrictionRootOnly, pgmconfig.EnabledPluginConfig()) + c, err := pgmconfig.EnabledPluginConfig() + if err != nil { + t.Fatal(err) + } + return NewKustTestHarnessFull(t, path, fLdr.RestrictionRootOnly, c) } func NewKustTestHarnessNoLoadRestrictor(t *testing.T, path string) *KustTestHarness { @@ -100,7 +103,10 @@ func (th *KustTestHarness) FromMap(m map[string]interface{}) *resource.Resource return th.rf.RF().FromMap(m) } -func (th *KustTestHarness) FromMapAndOption(m map[string]interface{}, args *types.GeneratorArgs, option *types.GeneratorOptions) *resource.Resource { +func (th *KustTestHarness) FromMapAndOption( + m map[string]interface{}, + args *types.GeneratorArgs, + option *types.GeneratorOptions) *resource.Resource { return th.rf.RF().FromMapAndOption(m, args, option) } diff --git a/api/testutils/kusttest/plugintestenv.go b/api/testutils/kusttest/plugintestenv.go index 8ea261701..25154468d 100644 --- a/api/testutils/kusttest/plugintestenv.go +++ b/api/testutils/kusttest/plugintestenv.go @@ -11,6 +11,7 @@ import ( "strings" "testing" + "sigs.k8s.io/kustomize/api/filesys" "sigs.k8s.io/kustomize/api/pgmconfig" "sigs.k8s.io/kustomize/api/plugins/compiler" ) @@ -75,7 +76,7 @@ func (x *PluginTestEnv) makeCompiler() *compiler.Compiler { if err != nil { x.t.Error(err) } - srcRoot, err := compiler.DefaultSrcRoot() + srcRoot, err := compiler.DefaultSrcRoot(filesys.MakeFsOnDisk()) if err != nil { x.t.Error(err) } diff --git a/api/types/errunabletofind.go b/api/types/errunabletofind.go new file mode 100644 index 000000000..a39e9a385 --- /dev/null +++ b/api/types/errunabletofind.go @@ -0,0 +1,40 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package types + +import ( + "fmt" + "strings" + + "github.com/pkg/errors" +) + +type errUnableToFind struct { + // What are we unable to find? + what string + // What things did we try? + attempts []Pair +} + +func (e *errUnableToFind) Error() string { + var m []string + for _, p := range e.attempts { + m = append(m, "('"+p.Value+"'; "+p.Key+")") + } + return fmt.Sprintf( + "unable to find plugin root - tried: %s", strings.Join(m, ", ")) +} + +func NewErrUnableToFind(w string, a []Pair) *errUnableToFind { + return &errUnableToFind{what: w, attempts: a} +} + +func IsErrUnableToFind(err error) bool { + _, ok := err.(*errUnableToFind) + if ok { + return true + } + _, ok = errors.Cause(err).(*errUnableToFind) + return ok +} diff --git a/kustomize/internal/commands/build/build.go b/kustomize/internal/commands/build/build.go index ac1d462c6..b5aa89687 100644 --- a/kustomize/internal/commands/build/build.go +++ b/kustomize/internal/commands/build/build.go @@ -5,6 +5,7 @@ package build import ( "io" + "log" "path/filepath" "strings" @@ -108,7 +109,11 @@ func (o *Options) makeOptions() *krusty.Options { DoPrune: false, } if isFlagEnablePluginsSet() { - opts.PluginConfig = pgmconfig.EnabledPluginConfig() + c, err := pgmconfig.EnabledPluginConfig() + if err != nil { + log.Fatal(err) + } + opts.PluginConfig = c } else { opts.PluginConfig = pgmconfig.DisabledPluginConfig() }