Skip to content

Commit

Permalink
more code review
Browse files Browse the repository at this point in the history
  • Loading branch information
natasha41575 committed Aug 19, 2021
1 parent 16482c9 commit 05228bb
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 36 deletions.
2 changes: 1 addition & 1 deletion api/internal/plugins/fnplugin/fnplugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func NewFnPlugin(o *types.FnPluginLoadingOptions) *FnPlugin {
StorageMounts: toStorageMounts(o.Mounts),
Env: o.Env,
AsCurrentUser: o.AsCurrentUser,
WorkingDir: o.WorkingDir,
},
}
}
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion api/internal/target/kusttarget.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions api/resmap/resmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions cmd/config/internal/commands/run-fns.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package commands
import (
"fmt"
"io"
"os"
"strings"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
18 changes: 12 additions & 6 deletions cmd/config/internal/commands/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -201,6 +202,7 @@ apiVersion: v1
Path: "dir",
EnableStarlark: true,
Env: []string{},
WorkingDir: wd,
},
},
{
Expand Down Expand Up @@ -254,6 +256,7 @@ apiVersion: v1
Path: "dir",
ResultsDir: "foo/",
Env: []string{},
WorkingDir: wd,
},
expected: `
metadata:
Expand Down Expand Up @@ -286,18 +289,20 @@ 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,
},
},
{
name: "envs",
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,
},
},
{
Expand All @@ -308,6 +313,7 @@ apiVersion: v1
Path: "dir",
AsCurrentUser: true,
Env: []string{},
WorkingDir: wd,
},
},
}
Expand Down
10 changes: 10 additions & 0 deletions kyaml/fn/runtime/exec/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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()
Expand Down
29 changes: 5 additions & 24 deletions kyaml/runfn/runfn.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
}

0 comments on commit 05228bb

Please sign in to comment.