From 1e1b9b484a836714b57b25c2cd47dda1780610e7 Mon Sep 17 00:00:00 2001
From: Natasha Sarkar <natashasarkar@google.com>
Date: Thu, 19 Aug 2021 20:15:24 -0700
Subject: [PATCH] exec function working dir is the kustomization that
 referenced it (#4125)

* exec function working dir is the kustomization that referenced it

* suggested changes

* more code review

* use a field instead of an annotation

* more code review
---
 api/internal/plugins/fnplugin/fnplugin.go |   1 +
 api/internal/plugins/loader/loader.go     |   5 +
 api/internal/target/kusttarget.go         |   5 +-
 api/krusty/fnplugin_test.go               | 149 ++++++++++++++++++++--
 api/krusty/fnplugin_test/fnexectest.sh    |  24 ----
 api/types/pluginrestrictions.go           |   2 +
 cmd/config/internal/commands/run-fns.go   |   8 ++
 cmd/config/internal/commands/run_test.go  |  18 ++-
 kyaml/fn/runtime/exec/exec.go             |  17 +++
 kyaml/runfn/runfn.go                      |  12 +-
 10 files changed, 199 insertions(+), 42 deletions(-)
 delete mode 100755 api/krusty/fnplugin_test/fnexectest.sh

diff --git a/api/internal/plugins/fnplugin/fnplugin.go b/api/internal/plugins/fnplugin/fnplugin.go
index 76be4b0760..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,
 		},
 	}
 }
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 18a38f3c48..45e021e245 100644
--- a/api/internal/target/kusttarget.go
+++ b/api/internal/target/kusttarget.go
@@ -40,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,
 	}
 }
 
@@ -295,7 +297,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/krusty/fnplugin_test.go b/api/krusty/fnplugin_test.go
index eca491056b..88fa8dee84 100644
--- a/api/krusty/fnplugin_test.go
+++ b/api/krusty/fnplugin_test.go
@@ -1,17 +1,52 @@
 package krusty_test
 
 import (
+	"os"
 	"os/exec"
+	"path/filepath"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
+	"sigs.k8s.io/kustomize/kyaml/filesys"
 )
 
+const generateDeploymentDotSh = `#!/bin/sh
+
+cat <<EOF
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+  labels:
+    app: nginx
+  annotations:
+    tshirt-size: small # this injects the resource reservations
+spec:
+  selector:
+    matchLabels:
+      app: nginx
+  template:
+    metadata:
+      labels:
+        app: nginx
+    spec:
+      containers:
+      - name: nginx
+        image: nginx
+EOF
+`
+
 func TestFnExecGenerator(t *testing.T) {
-	// Function plugins should not need the env setup done by MakeEnhancedHarness
-	th := kusttest_test.MakeHarness(t)
+	fSys := filesys.MakeFsOnDisk()
 
-	th.WriteK(".", `
+	th := kusttest_test.MakeHarnessWithFs(t, fSys)
+	o := th.MakeOptionsPluginsEnabled()
+	o.PluginConfig.FnpLoadingOptions.EnableExec = true
+
+	tmpDir, err := filesys.NewTmpConfirmedDir()
+	assert.NoError(t, err)
+	th.WriteK(tmpDir.String(), `
 resources:
 - short_secret.yaml
 generators:
@@ -19,7 +54,8 @@ generators:
 `)
 
 	// Create some additional resource just to make sure everything is added
-	th.WriteF("short_secret.yaml", `
+	th.WriteF(filepath.Join(tmpDir.String(), "short_secret.yaml"),
+		`
 apiVersion: v1
 kind: Secret
 metadata:
@@ -32,23 +68,117 @@ stringData:
     bootcmd:
     - mkdir /mnt/vda
 `)
+	th.WriteF(filepath.Join(tmpDir.String(), "generateDeployment.sh"), generateDeploymentDotSh)
 
-	th.WriteF("gener.yaml", `
+	assert.NoError(t, os.Chmod(filepath.Join(tmpDir.String(), "generateDeployment.sh"), 0777))
+	th.WriteF(filepath.Join(tmpDir.String(), "gener.yaml"), `
 kind: executable
 metadata:
   name: demo
   annotations:
     config.kubernetes.io/function: |
       exec:
-        path: ./fnplugin_test/fnexectest.sh
+        path: ./generateDeployment.sh
 spec:
 `)
+
+	m := th.Run(tmpDir.String(), o)
+	assert.NoError(t, err)
+	yml, err := m.AsYaml()
+	assert.NoError(t, err)
+	assert.Equal(t, `apiVersion: v1
+kind: Secret
+metadata:
+  labels:
+    airshipit.org/ephemeral-user-data: "true"
+  name: node1-bmc-secret
+stringData:
+  userData: |
+    bootcmd:
+    - mkdir /mnt/vda
+type: Opaque
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  annotations:
+    tshirt-size: small
+  labels:
+    app: nginx
+  name: nginx
+spec:
+  selector:
+    matchLabels:
+      app: nginx
+  template:
+    metadata:
+      labels:
+        app: nginx
+    spec:
+      containers:
+      - image: nginx
+        name: nginx
+`, string(yml))
+	assert.NoError(t, fSys.RemoveAll(tmpDir.String()))
+}
+
+func TestFnExecGeneratorWithOverlay(t *testing.T) {
+	fSys := filesys.MakeFsOnDisk()
+
+	th := kusttest_test.MakeHarnessWithFs(t, fSys)
 	o := th.MakeOptionsPluginsEnabled()
 	o.PluginConfig.FnpLoadingOptions.EnableExec = true
-	m := th.Run(".", o)
-	th.AssertActualEqualsExpected(m, `
+
+	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))
+	th.WriteK(base, `
+resources:
+- short_secret.yaml
+generators:
+- gener.yaml
+`)
+	th.WriteK(prod, `
+resources:
+- ../base
+`)
+	th.WriteF(filepath.Join(base, "short_secret.yaml"),
+		`
 apiVersion: v1
 kind: Secret
+metadata:
+  labels:
+    airshipit.org/ephemeral-user-data: "true"
+  name: node1-bmc-secret
+type: Opaque
+stringData:
+  userData: |
+    bootcmd:
+    - mkdir /mnt/vda
+`)
+	th.WriteF(filepath.Join(base, "generateDeployment.sh"), generateDeploymentDotSh)
+
+	assert.NoError(t, os.Chmod(filepath.Join(base, "generateDeployment.sh"), 0777))
+	th.WriteF(filepath.Join(base, "gener.yaml"), `
+kind: executable
+metadata:
+  name: demo
+  annotations:
+    config.kubernetes.io/function: |
+      exec:
+        path: ./generateDeployment.sh
+spec:
+`)
+
+	m := th.Run(prod, o)
+	assert.NoError(t, err)
+	yml, err := m.AsYaml()
+	assert.NoError(t, err)
+	assert.Equal(t, `apiVersion: v1
+kind: Secret
 metadata:
   labels:
     airshipit.org/ephemeral-user-data: "true"
@@ -79,7 +209,8 @@ spec:
       containers:
       - image: nginx
         name: nginx
-`)
+`, string(yml))
+	assert.NoError(t, fSys.RemoveAll(tmpDir.String()))
 }
 
 func skipIfNoDocker(t *testing.T) {
diff --git a/api/krusty/fnplugin_test/fnexectest.sh b/api/krusty/fnplugin_test/fnexectest.sh
deleted file mode 100755
index d8e7c9eabb..0000000000
--- a/api/krusty/fnplugin_test/fnexectest.sh
+++ /dev/null
@@ -1,24 +0,0 @@
-#!/bin/sh
-
-cat <<EOF
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: nginx
-  labels:
-    app: nginx
-  annotations:
-    tshirt-size: small # this injects the resource reservations
-spec:
-  selector:
-    matchLabels:
-      app: nginx
-  template:
-    metadata:
-      labels:
-        app: nginx
-    spec:
-      containers:
-      - name: nginx
-        image: nginx
-EOF
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/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 3346bc998f..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"
 )
@@ -19,6 +21,10 @@ type Filter struct {
 	// Args are the arguments to the executable
 	Args []string `yaml:"args,omitempty"`
 
+	// WorkingDir is the working directory that the executable
+	// should run in
+	WorkingDir string
+
 	runtimeutil.FunctionFilter
 }
 
@@ -32,5 +38,16 @@ func (c *Filter) Run(reader io.Reader, writer io.Writer) error {
 	cmd.Stdin = reader
 	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 bfe1ac3f2f..05f57f8ff8 100644
--- a/kyaml/runfn/runfn.go
+++ b/kyaml/runfn/runfn.go
@@ -101,6 +101,9 @@ type RunFns struct {
 	// If it is true, the empty result will be provided as input to the next
 	// function in the list.
 	ContinueOnEmptyResult bool
+
+	// WorkingDir specifies which working directory an exec function should run in.
+	WorkingDir string
 }
 
 // Execute runs the command
@@ -507,7 +510,14 @@ func (r *RunFns) ffp(spec runtimeutil.FunctionSpec, api *yaml.RNode, currentUser
 	}
 
 	if r.EnableExec && spec.Exec.Path != "" {
-		ef := &exec.Filter{Path: spec.Exec.Path}
+		if r.WorkingDir == "" {
+			return nil, fmt.Errorf("no working directory set for exec function")
+		}
+
+		ef := &exec.Filter{
+			Path:       spec.Exec.Path,
+			WorkingDir: r.WorkingDir,
+		}
 
 		ef.FunctionConfig = api
 		ef.GlobalScope = r.GlobalScope