From cf7a02fac75415c22e420072ed3c17dbd7b53cae Mon Sep 17 00:00:00 2001 From: Taylor Thomas Date: Thu, 6 Feb 2020 12:14:22 -0700 Subject: [PATCH] chore(*): Removes support for searching the plugin dir Signed-off-by: Taylor Thomas --- cmd/helm/flags.go | 2 +- pkg/postrender/exec.go | 60 ++++++++++++++++++------------------ pkg/postrender/exec_test.go | 61 +++++++++++++++++++------------------ 3 files changed, 63 insertions(+), 60 deletions(-) diff --git a/cmd/helm/flags.go b/cmd/helm/flags.go index dfaef04a1..86ed53d51 100644 --- a/cmd/helm/flags.go +++ b/cmd/helm/flags.go @@ -98,7 +98,7 @@ func (o *outputValue) Set(s string) error { } func bindPostRenderFlag(cmd *cobra.Command, varRef *postrender.PostRenderer) { - cmd.Flags().Var(&postRenderer{varRef}, postRenderFlag, "the path to an executable to be used for post rendering. If a non-absolute path is provided, the plugin directory and $PATH will be searched") + cmd.Flags().Var(&postRenderer{varRef}, postRenderFlag, "the path to an executable to be used for post rendering. If it exists in $PATH, the binary will be used, otherwise it will try to look for the executable at the given path") // Setup shell completion for the flag cmd.MarkFlagCustom(outputFlag, "__helm_output_options") } diff --git a/pkg/postrender/exec.go b/pkg/postrender/exec.go index bcf3ef64d..0860e7c35 100644 --- a/pkg/postrender/exec.go +++ b/pkg/postrender/exec.go @@ -19,14 +19,10 @@ package postrender import ( "bytes" "io" - "os" "os/exec" "path/filepath" - "strings" "github.com/pkg/errors" - - "helm.sh/helm/v3/pkg/helmpath" ) type execRender struct { @@ -34,10 +30,9 @@ type execRender struct { } // NewExec returns a PostRenderer implementation that calls the provided binary. -// It returns an error if the binary cannot be found. If the provided path does -// not contain any separators, it will search first in the plugins directory, -// then in $PATH, otherwise it will resolve any relative paths to a fully -// qualified path +// It returns an error if the binary cannot be found. If the path does not +// contain any separators, it will search in $PATH, otherwise it will resolve +// any relative paths to a fully qualified path func NewExec(binaryPath string) (PostRenderer, error) { fullPath, err := getFullPath(binaryPath) if err != nil { @@ -72,30 +67,35 @@ func (p *execRender) Run(renderedManifests *bytes.Buffer) (*bytes.Buffer, error) } // getFullPath returns the full filepath to the binary to execute. If the path -// does not contain any separators, it will search first in the plugins -// directory (or directories if multiple are specified. In which case, it will -// return the first result), then in $PATH, otherwise it will resolve any -// relative paths to a fully qualified path +// does not contain any separators, it will search in $PATH, otherwise it will +// resolve any relative paths to a fully qualified path func getFullPath(binaryPath string) (string, error) { + // NOTE(thomastaylor312): I am leaving this code commented out here. During + // the implementation of post-render, it was brought up that if we are + // relying on plguins, we should actually use the plugin system so it can + // properly handle multiple OSs. This will be a feature add in the future, + // so I left this code for reference. It can be deleted or reused once the + // feature is implemented + // Manually check the plugin dir first - if !strings.Contains(binaryPath, string(filepath.Separator)) { - // First check the plugin dir - pluginDir := helmpath.DataPath("plugins") // Default location - // If location for plugins is explicitly set, check there - if v, ok := os.LookupEnv("HELM_PLUGINS"); ok { - pluginDir = v - } - // The plugins variable can actually contain multple paths, so loop through those - for _, p := range filepath.SplitList(pluginDir) { - _, err := os.Stat(filepath.Join(p, binaryPath)) - if err != nil && !os.IsNotExist(err) { - return "", err - } else if err == nil { - binaryPath = filepath.Join(p, binaryPath) - break - } - } - } + // if !strings.Contains(binaryPath, string(filepath.Separator)) { + // // First check the plugin dir + // pluginDir := helmpath.DataPath("plugins") // Default location + // // If location for plugins is explicitly set, check there + // if v, ok := os.LookupEnv("HELM_PLUGINS"); ok { + // pluginDir = v + // } + // // The plugins variable can actually contain multple paths, so loop through those + // for _, p := range filepath.SplitList(pluginDir) { + // _, err := os.Stat(filepath.Join(p, binaryPath)) + // if err != nil && !os.IsNotExist(err) { + // return "", err + // } else if err == nil { + // binaryPath = filepath.Join(p, binaryPath) + // break + // } + // } + // } // Now check for the binary using the given path or check if it exists in // the path and is executable diff --git a/pkg/postrender/exec_test.go b/pkg/postrender/exec_test.go index 684a0642b..ef0956949 100644 --- a/pkg/postrender/exec_test.go +++ b/pkg/postrender/exec_test.go @@ -73,35 +73,38 @@ func TestGetFullPath(t *testing.T) { is.Equal(testpath, fullPath) }) - t.Run("binary in plugin path resolves correctly", func(t *testing.T) { - testpath, cleanup := setupTestingScript(t) - defer cleanup() - - realPath := os.Getenv("HELM_PLUGINS") - os.Setenv("HELM_PLUGINS", filepath.Dir(testpath)) - defer func() { - os.Setenv("HELM_PLUGINS", realPath) - }() - - fullPath, err := getFullPath(filepath.Base(testpath)) - is.NoError(err) - is.Equal(testpath, fullPath) - }) - - t.Run("binary in multiple plugin paths resolves correctly", func(t *testing.T) { - testpath, cleanup := setupTestingScript(t) - defer cleanup() - - realPath := os.Getenv("HELM_PLUGINS") - os.Setenv("HELM_PLUGINS", filepath.Dir(testpath)+string(os.PathListSeparator)+"/another/dir") - defer func() { - os.Setenv("HELM_PLUGINS", realPath) - }() - - fullPath, err := getFullPath(filepath.Base(testpath)) - is.NoError(err) - is.Equal(testpath, fullPath) - }) + // NOTE(thomastaylor312): See note in getFullPath for more details why this + // is here + + // t.Run("binary in plugin path resolves correctly", func(t *testing.T) { + // testpath, cleanup := setupTestingScript(t) + // defer cleanup() + + // realPath := os.Getenv("HELM_PLUGINS") + // os.Setenv("HELM_PLUGINS", filepath.Dir(testpath)) + // defer func() { + // os.Setenv("HELM_PLUGINS", realPath) + // }() + + // fullPath, err := getFullPath(filepath.Base(testpath)) + // is.NoError(err) + // is.Equal(testpath, fullPath) + // }) + + // t.Run("binary in multiple plugin paths resolves correctly", func(t *testing.T) { + // testpath, cleanup := setupTestingScript(t) + // defer cleanup() + + // realPath := os.Getenv("HELM_PLUGINS") + // os.Setenv("HELM_PLUGINS", filepath.Dir(testpath)+string(os.PathListSeparator)+"/another/dir") + // defer func() { + // os.Setenv("HELM_PLUGINS", realPath) + // }() + + // fullPath, err := getFullPath(filepath.Base(testpath)) + // is.NoError(err) + // is.Equal(testpath, fullPath) + // }) } func TestExecRun(t *testing.T) {