From e52bab4ddcd1e96093e12f25ac3d214de9d66d17 Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Tue, 17 Aug 2021 16:57:42 -0700 Subject: [PATCH 1/5] exec function working dir is the kustomization that referenced it --- api/internal/target/kusttarget.go | 8 +- api/krusty/fnplugin_test.go | 125 ++++++++++++++++++++++++++++++ api/resmap/resmap.go | 4 + api/resmap/reswrangler.go | 13 ++++ kyaml/fn/runtime/exec/exec.go | 13 ++++ kyaml/runfn/runfn.go | 14 +++- 6 files changed, 175 insertions(+), 2 deletions(-) diff --git a/api/internal/target/kusttarget.go b/api/internal/target/kusttarget.go index 18a38f3c48..046ed27a17 100644 --- a/api/internal/target/kusttarget.go +++ b/api/internal/target/kusttarget.go @@ -21,6 +21,7 @@ import ( "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/types" + "sigs.k8s.io/kustomize/kyaml/kio/kioutil" "sigs.k8s.io/kustomize/kyaml/openapi" "sigs.k8s.io/yaml" ) @@ -262,7 +263,12 @@ func (kt *KustTarget) configureExternalGenerators() ([]resmap.Generator, error) if err != nil { return nil, err } - return kt.pLdr.LoadGenerators(kt.ldr, kt.validator, ra.ResMap()) + m := ra.ResMap() + err = m.AnnotateAll(kioutil.PathAnnotation, kt.ldr.Root()) + if err != nil { + return nil, err + } + return kt.pLdr.LoadGenerators(kt.ldr, kt.validator, m) } func (kt *KustTarget) runTransformers(ra *accumulator.ResAccumulator) error { diff --git a/api/krusty/fnplugin_test.go b/api/krusty/fnplugin_test.go index eca491056b..554b5637f9 100644 --- a/api/krusty/fnplugin_test.go +++ b/api/krusty/fnplugin_test.go @@ -1,10 +1,16 @@ package krusty_test import ( + "os" "os/exec" + "path/filepath" "testing" + "github.com/stretchr/testify/assert" + "sigs.k8s.io/kustomize/api/internal/utils" + "sigs.k8s.io/kustomize/api/krusty" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" + "sigs.k8s.io/kustomize/kyaml/filesys" ) func TestFnExecGenerator(t *testing.T) { @@ -82,6 +88,125 @@ spec: `) } +func TestFnExecGeneratorWithOverlay(t *testing.T) { + fSys := filesys.MakeFsOnDisk() + th := kusttest_test.MakeHarness(t) + o := th.MakeOptionsPluginsEnabled() + o.PluginConfig.FnpLoadingOptions.EnableExec = true + b := krusty.MakeKustomizer(&o) + + tmpDir, err := filesys.NewTmpConfirmedDir() + assert.NoError(t, err) + base := filepath.Join(tmpDir.String(), "base") + prod := filepath.Join(tmpDir.String(), "prod") + assert.NoError(t, fSys.Mkdir(base)) + assert.NoError(t, fSys.Mkdir(prod)) + assert.NoError(t, fSys.WriteFile(filepath.Join(base, "kustomization.yaml"), []byte(` +resources: +- short_secret.yaml +generators: +- gener.yaml +`))) + assert.NoError(t, fSys.WriteFile(filepath.Join(prod, "kustomization.yaml"), []byte(` +resources: +- ../base +`))) + assert.NoError(t, fSys.WriteFile(filepath.Join(base, "short_secret.yaml"), []byte(` +apiVersion: v1 +kind: Secret +metadata: + labels: + airshipit.org/ephemeral-user-data: "true" + name: node1-bmc-secret +type: Opaque +stringData: + userData: | + bootcmd: + - mkdir /mnt/vda +`))) + assert.NoError(t, fSys.WriteFile(filepath.Join(base, "exec.sh"), []byte(`#!/bin/sh + +cat < Date: Wed, 18 Aug 2021 13:47:43 -0700 Subject: [PATCH 2/5] suggested changes --- api/internal/target/kusttarget.go | 11 ++- api/krusty/fnplugin_test.go | 118 +++++++++++++------------ api/krusty/fnplugin_test/fnexectest.sh | 24 ----- api/resmap/resmap.go | 4 - api/resmap/reswrangler.go | 13 --- kyaml/fn/runtime/exec/exec.go | 12 +-- kyaml/kio/kioutil/kioutil.go | 4 + kyaml/runfn/runfn.go | 28 ++++-- 8 files changed, 100 insertions(+), 114 deletions(-) delete mode 100755 api/krusty/fnplugin_test/fnexectest.sh diff --git a/api/internal/target/kusttarget.go b/api/internal/target/kusttarget.go index 046ed27a17..f601b42530 100644 --- a/api/internal/target/kusttarget.go +++ b/api/internal/target/kusttarget.go @@ -264,7 +264,7 @@ func (kt *KustTarget) configureExternalGenerators() ([]resmap.Generator, error) return nil, err } m := ra.ResMap() - err = m.AnnotateAll(kioutil.PathAnnotation, kt.ldr.Root()) + err = m.AnnotateAll(kioutil.WorkingDirAnnotation, kt.ldr.Root()) if err != nil { return nil, err } @@ -300,12 +300,17 @@ func (kt *KustTarget) configureExternalTransformers(transformers []string) ([]re } ra.AppendAll(rm) } - ra, err := kt.accumulateResources(ra, transformerPaths, &resource.Origin{}) + ra, err := kt.accumulateResources(ra, transformerPaths, &resource.Origin{}) + if err != nil { + return nil, err + } + m := ra.ResMap() + err = m.AnnotateAll(kioutil.WorkingDirAnnotation, kt.ldr.Root()) if err != nil { return nil, err } - return kt.pLdr.LoadTransformers(kt.ldr, kt.validator, ra.ResMap()) + return kt.pLdr.LoadTransformers(kt.ldr, kt.validator, m) } func (kt *KustTarget) runValidators(ra *accumulator.ResAccumulator) error { diff --git a/api/krusty/fnplugin_test.go b/api/krusty/fnplugin_test.go index 554b5637f9..d4d7057ccf 100644 --- a/api/krusty/fnplugin_test.go +++ b/api/krusty/fnplugin_test.go @@ -7,17 +7,46 @@ import ( "testing" "github.com/stretchr/testify/assert" - "sigs.k8s.io/kustomize/api/internal/utils" - "sigs.k8s.io/kustomize/api/krusty" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" "sigs.k8s.io/kustomize/kyaml/filesys" ) +const execFile = `#!/bin/sh + +cat < Date: Thu, 19 Aug 2021 13:08:33 -0700 Subject: [PATCH 3/5] more code review --- api/krusty/fnplugin_test.go | 14 +++++++------- api/resmap/reswrangler.go | 26 +++++++++++++------------- kyaml/runfn/runfn.go | 25 +++++++++++-------------- 3 files changed, 31 insertions(+), 34 deletions(-) diff --git a/api/krusty/fnplugin_test.go b/api/krusty/fnplugin_test.go index d4d7057ccf..88fa8dee84 100644 --- a/api/krusty/fnplugin_test.go +++ b/api/krusty/fnplugin_test.go @@ -11,7 +11,7 @@ import ( "sigs.k8s.io/kustomize/kyaml/filesys" ) -const execFile = `#!/bin/sh +const generateDeploymentDotSh = `#!/bin/sh cat < Date: Thu, 19 Aug 2021 13:48:12 -0700 Subject: [PATCH 4/5] use a field instead of an annotation --- api/internal/plugins/fnplugin/fnplugin.go | 1 + api/internal/plugins/loader/loader.go | 5 +++++ api/internal/target/kusttarget.go | 19 +++++-------------- api/resmap/resmap.go | 4 ++++ api/types/pluginrestrictions.go | 2 ++ kyaml/kio/kioutil/kioutil.go | 4 ---- kyaml/runfn/runfn.go | 14 ++++++++------ 7 files changed, 25 insertions(+), 24 deletions(-) diff --git a/api/internal/plugins/fnplugin/fnplugin.go b/api/internal/plugins/fnplugin/fnplugin.go index 76be4b0760..5ee53a282a 100644 --- a/api/internal/plugins/fnplugin/fnplugin.go +++ b/api/internal/plugins/fnplugin/fnplugin.go @@ -187,6 +187,7 @@ func (p *FnPlugin) invokePlugin(input []byte) ([]byte, error) { p.runFns.Input = bytes.NewReader(input) p.runFns.Functions = append(p.runFns.Functions, functionConfig) p.runFns.Output = &ouputBuffer + p.runFns.WorkDir = p.h.WorkingDir() err = p.runFns.Execute() if err != nil { diff --git a/api/internal/plugins/loader/loader.go b/api/internal/plugins/loader/loader.go index 7bcce43152..3fff946bba 100644 --- a/api/internal/plugins/loader/loader.go +++ b/api/internal/plugins/loader/loader.go @@ -47,6 +47,11 @@ func (l *Loader) Config() *types.PluginConfig { return l.pc } +// SetWorkDir sets the working directory for this loader's plugins +func (l *Loader) SetWorkDir(wd string) { + l.pc.FnpLoadingOptions.WorkingDir = wd +} + func (l *Loader) LoadGenerators( ldr ifc.Loader, v ifc.Validator, rm resmap.ResMap) ([]resmap.Generator, error) { var result []resmap.Generator diff --git a/api/internal/target/kusttarget.go b/api/internal/target/kusttarget.go index f601b42530..0141b992f7 100644 --- a/api/internal/target/kusttarget.go +++ b/api/internal/target/kusttarget.go @@ -21,7 +21,6 @@ import ( "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/types" - "sigs.k8s.io/kustomize/kyaml/kio/kioutil" "sigs.k8s.io/kustomize/kyaml/openapi" "sigs.k8s.io/yaml" ) @@ -41,11 +40,13 @@ func NewKustTarget( validator ifc.Validator, rFactory *resmap.Factory, pLdr *loader.Loader) *KustTarget { + pLdrCopy := *pLdr + pLdrCopy.SetWorkDir(ldr.Root()) return &KustTarget{ ldr: ldr, validator: validator, rFactory: rFactory, - pLdr: pLdr, + pLdr: &pLdrCopy, } } @@ -263,12 +264,7 @@ func (kt *KustTarget) configureExternalGenerators() ([]resmap.Generator, error) if err != nil { return nil, err } - m := ra.ResMap() - err = m.AnnotateAll(kioutil.WorkingDirAnnotation, kt.ldr.Root()) - if err != nil { - return nil, err - } - return kt.pLdr.LoadGenerators(kt.ldr, kt.validator, m) + return kt.pLdr.LoadGenerators(kt.ldr, kt.validator, ra.ResMap()) } func (kt *KustTarget) runTransformers(ra *accumulator.ResAccumulator) error { @@ -305,12 +301,7 @@ func (kt *KustTarget) configureExternalTransformers(transformers []string) ([]re if err != nil { return nil, err } - m := ra.ResMap() - err = m.AnnotateAll(kioutil.WorkingDirAnnotation, kt.ldr.Root()) - if err != nil { - return nil, err - } - return kt.pLdr.LoadTransformers(kt.ldr, kt.validator, m) + return kt.pLdr.LoadTransformers(kt.ldr, kt.validator, ra.ResMap()) } func (kt *KustTarget) runValidators(ra *accumulator.ResAccumulator) error { diff --git a/api/resmap/resmap.go b/api/resmap/resmap.go index 7c0c2ea802..8327543ae2 100644 --- a/api/resmap/resmap.go +++ b/api/resmap/resmap.go @@ -66,6 +66,10 @@ func (c *PluginHelpers) Validator() ifc.Validator { return c.v } +func (c *PluginHelpers) WorkingDir() string { + return c.pc.FnpLoadingOptions.WorkingDir +} + type GeneratorPlugin interface { Generator Configurable diff --git a/api/types/pluginrestrictions.go b/api/types/pluginrestrictions.go index b1ab2221f6..88b03b3f5b 100644 --- a/api/types/pluginrestrictions.go +++ b/api/types/pluginrestrictions.go @@ -57,4 +57,6 @@ type FnPluginLoadingOptions struct { Env []string // Run as uid and gid of the command executor AsCurrentUser bool + // Run in this working directory + WorkingDir string } diff --git a/kyaml/kio/kioutil/kioutil.go b/kyaml/kio/kioutil/kioutil.go index 16155e4b26..993cdfd848 100644 --- a/kyaml/kio/kioutil/kioutil.go +++ b/kyaml/kio/kioutil/kioutil.go @@ -25,10 +25,6 @@ const ( // SeqIndentAnnotation records the sequence nodes indentation of the input resource SeqIndentAnnotation AnnotationKey = "internal.config.kubernetes.io/seqindent" - - // WorkingDirAnnotation records the directory of the kustomization that - // refers to the resource - WorkingDirAnnotation AnnotationKey = "internal.config.kubernetes.io/working-dir" ) func GetFileAnnotations(rn *yaml.RNode) (string, string, error) { diff --git a/kyaml/runfn/runfn.go b/kyaml/runfn/runfn.go index 7904bf434f..f85788dfb9 100644 --- a/kyaml/runfn/runfn.go +++ b/kyaml/runfn/runfn.go @@ -101,6 +101,10 @@ type RunFns struct { // If it is true, the empty result will be provided as input to the next // function in the list. ContinueOnEmptyResult bool + + // WorkDir specifies which working directory an exec function should run in. + // If this is empty, fall back to the current working directory. + WorkDir string } // Execute runs the command @@ -507,13 +511,13 @@ func (r *RunFns) ffp(spec runtimeutil.FunctionSpec, api *yaml.RNode, currentUser } if r.EnableExec && spec.Exec.Path != "" { - wd, err := getWorkingDirectory(api) + wd, err := getWorkingDirectory(r.WorkDir) if err != nil { return nil, err } ef := &exec.Filter{ - Path: spec.Exec.Path, + Path: spec.Exec.Path, WorkingDir: wd, } @@ -527,10 +531,8 @@ func (r *RunFns) ffp(spec runtimeutil.FunctionSpec, api *yaml.RNode, currentUser return nil, nil } -func getWorkingDirectory(api *yaml.RNode) (string, error) { - annotations := api.GetAnnotations() - wd, ok := annotations[kioutil.WorkingDirAnnotation] - if !ok { +func getWorkingDirectory(wd string) (string, error) { + if wd == "" { return os.Getwd() } From 05228bbfed5bc4cb18903b2a7e8a5ee4cae27f5b Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Thu, 19 Aug 2021 15:10:33 -0700 Subject: [PATCH 5/5] more code review --- api/internal/plugins/fnplugin/fnplugin.go | 2 +- api/internal/target/kusttarget.go | 1 - api/resmap/resmap.go | 4 ---- cmd/config/internal/commands/run-fns.go | 8 +++++++ cmd/config/internal/commands/run_test.go | 18 +++++++++----- kyaml/fn/runtime/exec/exec.go | 10 ++++++++ kyaml/runfn/runfn.go | 29 ++++------------------- 7 files changed, 36 insertions(+), 36 deletions(-) diff --git a/api/internal/plugins/fnplugin/fnplugin.go b/api/internal/plugins/fnplugin/fnplugin.go index 5ee53a282a..84bc0ac059 100644 --- a/api/internal/plugins/fnplugin/fnplugin.go +++ b/api/internal/plugins/fnplugin/fnplugin.go @@ -79,6 +79,7 @@ func NewFnPlugin(o *types.FnPluginLoadingOptions) *FnPlugin { StorageMounts: toStorageMounts(o.Mounts), Env: o.Env, AsCurrentUser: o.AsCurrentUser, + WorkingDir: o.WorkingDir, }, } } @@ -187,7 +188,6 @@ func (p *FnPlugin) invokePlugin(input []byte) ([]byte, error) { p.runFns.Input = bytes.NewReader(input) p.runFns.Functions = append(p.runFns.Functions, functionConfig) p.runFns.Output = &ouputBuffer - p.runFns.WorkDir = p.h.WorkingDir() err = p.runFns.Execute() if err != nil { diff --git a/api/internal/target/kusttarget.go b/api/internal/target/kusttarget.go index 0141b992f7..45e021e245 100644 --- a/api/internal/target/kusttarget.go +++ b/api/internal/target/kusttarget.go @@ -296,7 +296,6 @@ func (kt *KustTarget) configureExternalTransformers(transformers []string) ([]re } ra.AppendAll(rm) } - ra, err := kt.accumulateResources(ra, transformerPaths, &resource.Origin{}) if err != nil { return nil, err diff --git a/api/resmap/resmap.go b/api/resmap/resmap.go index 8327543ae2..7c0c2ea802 100644 --- a/api/resmap/resmap.go +++ b/api/resmap/resmap.go @@ -66,10 +66,6 @@ func (c *PluginHelpers) Validator() ifc.Validator { return c.v } -func (c *PluginHelpers) WorkingDir() string { - return c.pc.FnpLoadingOptions.WorkingDir -} - type GeneratorPlugin interface { Generator Configurable diff --git a/cmd/config/internal/commands/run-fns.go b/cmd/config/internal/commands/run-fns.go index a28e0b2b9f..684d940c20 100644 --- a/cmd/config/internal/commands/run-fns.go +++ b/cmd/config/internal/commands/run-fns.go @@ -6,6 +6,7 @@ package commands import ( "fmt" "io" + "os" "strings" "github.com/spf13/cobra" @@ -73,6 +74,7 @@ func GetRunFnRunner(name string) *RunFnRunner { "a list of environment variables to be used by functions") r.Command.Flags().BoolVar( &r.AsCurrentUser, "as-current-user", false, "use the uid and gid of the command executor to run the function in the container") + return r } @@ -302,6 +304,11 @@ func (r *RunFnRunner) preRunE(c *cobra.Command, args []string) error { // parse mounts to set storageMounts storageMounts := toStorageMounts(r.Mounts) + wd, err := os.Getwd() + if err != nil { + return err + } + r.RunFns = runfn.RunFns{ FunctionPaths: r.FnPaths, GlobalScope: r.GlobalScope, @@ -317,6 +324,7 @@ func (r *RunFnRunner) preRunE(c *cobra.Command, args []string) error { LogSteps: r.LogSteps, Env: r.Env, AsCurrentUser: r.AsCurrentUser, + WorkingDir: wd, } // don't consider args for the function diff --git a/cmd/config/internal/commands/run_test.go b/cmd/config/internal/commands/run_test.go index bf9c682276..5a8ca8e268 100644 --- a/cmd/config/internal/commands/run_test.go +++ b/cmd/config/internal/commands/run_test.go @@ -11,13 +11,14 @@ import ( "github.com/spf13/cobra" "github.com/stretchr/testify/assert" - "sigs.k8s.io/kustomize/kyaml/runfn" ) // TestRunFnCommand_preRunE verifies that preRunE correctly parses the commandline // flags and arguments into the RunFns structure to be executed. func TestRunFnCommand_preRunE(t *testing.T) { + wd, err := os.Getwd() + assert.NoError(t, err) tests := []struct { name string args []string @@ -201,6 +202,7 @@ apiVersion: v1 Path: "dir", EnableStarlark: true, Env: []string{}, + WorkingDir: wd, }, }, { @@ -254,6 +256,7 @@ apiVersion: v1 Path: "dir", ResultsDir: "foo/", Env: []string{}, + WorkingDir: wd, }, expected: ` metadata: @@ -286,9 +289,10 @@ apiVersion: v1 args: []string{"run", "dir", "--log-steps"}, path: "dir", expectedStruct: &runfn.RunFns{ - Path: "dir", - LogSteps: true, - Env: []string{}, + Path: "dir", + LogSteps: true, + Env: []string{}, + WorkingDir: wd, }, }, { @@ -296,8 +300,9 @@ apiVersion: v1 args: []string{"run", "dir", "--env", "FOO=BAR", "-e", "BAR"}, path: "dir", expectedStruct: &runfn.RunFns{ - Path: "dir", - Env: []string{"FOO=BAR", "BAR"}, + Path: "dir", + Env: []string{"FOO=BAR", "BAR"}, + WorkingDir: wd, }, }, { @@ -308,6 +313,7 @@ apiVersion: v1 Path: "dir", AsCurrentUser: true, Env: []string{}, + WorkingDir: wd, }, }, } diff --git a/kyaml/fn/runtime/exec/exec.go b/kyaml/fn/runtime/exec/exec.go index 7bab904c6a..628d3dac8e 100644 --- a/kyaml/fn/runtime/exec/exec.go +++ b/kyaml/fn/runtime/exec/exec.go @@ -7,7 +7,9 @@ import ( "io" "os" "os/exec" + "path/filepath" + "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/fn/runtime/runtimeutil" "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -37,6 +39,14 @@ func (c *Filter) Run(reader io.Reader, writer io.Writer) error { cmd.Stdout = writer cmd.Stderr = os.Stderr if c.WorkingDir != "" { + if !filepath.IsAbs(c.WorkingDir) { + return errors.Errorf( + "relative working directory %s not allowed", c.WorkingDir) + } + if c.WorkingDir == "/" { + return errors.Errorf( + "root working directory '/' not allowed") + } cmd.Dir = c.WorkingDir } return cmd.Run() diff --git a/kyaml/runfn/runfn.go b/kyaml/runfn/runfn.go index f85788dfb9..05f57f8ff8 100644 --- a/kyaml/runfn/runfn.go +++ b/kyaml/runfn/runfn.go @@ -102,9 +102,8 @@ type RunFns struct { // function in the list. ContinueOnEmptyResult bool - // WorkDir specifies which working directory an exec function should run in. - // If this is empty, fall back to the current working directory. - WorkDir string + // WorkingDir specifies which working directory an exec function should run in. + WorkingDir string } // Execute runs the command @@ -511,14 +510,13 @@ func (r *RunFns) ffp(spec runtimeutil.FunctionSpec, api *yaml.RNode, currentUser } if r.EnableExec && spec.Exec.Path != "" { - wd, err := getWorkingDirectory(r.WorkDir) - if err != nil { - return nil, err + if r.WorkingDir == "" { + return nil, fmt.Errorf("no working directory set for exec function") } ef := &exec.Filter{ Path: spec.Exec.Path, - WorkingDir: wd, + WorkingDir: r.WorkingDir, } ef.FunctionConfig = api @@ -530,20 +528,3 @@ func (r *RunFns) ffp(spec runtimeutil.FunctionSpec, api *yaml.RNode, currentUser return nil, nil } - -func getWorkingDirectory(wd string) (string, error) { - if wd == "" { - return os.Getwd() - } - - if !filepath.IsAbs(wd) { - return "", errors.Errorf( - "relative working directory %s not allowed", wd) - } - if wd == "/" { - return "", errors.Errorf( - "root working directory '/' not allowed") - } - - return wd, nil -}