diff --git a/Makefile b/Makefile index 8709fa0fe0..05cd90c4a8 100644 --- a/Makefile +++ b/Makefile @@ -82,7 +82,7 @@ kustomize: $(MYGOBIN)/kustomize # Used to add non-default compilation flags when experimenting with # plugin-to-api compatibility checks. .PHONY: build-kustomize-api -build-kustomize-api: $(builtinplugins) +build-kustomize-api: $(MYGOBIN)/goimports $(builtinplugins) cd api; $(MAKE) build .PHONY: generate-kustomize-api diff --git a/api/internal/plugins/loader/loader.go b/api/internal/plugins/loader/loader.go index 93c8559d23..82cb93965c 100644 --- a/api/internal/plugins/loader/loader.go +++ b/api/internal/plugins/loader/loader.go @@ -228,6 +228,17 @@ func (l *Loader) makeBuiltinPlugin(r resid.Gvk) (resmap.Configurable, error) { func (l *Loader) loadPlugin(res *resource.Resource) (resmap.Configurable, error) { spec := fnplugin.GetFunctionSpec(res) if spec != nil { + // validation check that function mounts are under the current kustomization directory + for _, mount := range spec.Container.StorageMounts { + if filepath.IsAbs(mount.Src) { + return nil, errors.New(fmt.Sprintf("plugin %s with mount path '%s' is not permitted; "+ + "mount paths must be relative to the current kustomization directory", res.OrgId(), mount.Src)) + } + if strings.HasPrefix(filepath.Clean(mount.Src), "../") { + return nil, errors.New(fmt.Sprintf("plugin %s with mount path '%s' is not permitted; "+ + "mount paths must be under the current kustomization directory", res.OrgId(), mount.Src)) + } + } return fnplugin.NewFnPlugin(&l.pc.FnpLoadingOptions), nil } return l.loadExecOrGoPlugin(res.OrgId()) diff --git a/api/krusty/fnplugin_test.go b/api/krusty/fnplugin_test.go index b865a7e46f..173d9e4b1e 100644 --- a/api/krusty/fnplugin_test.go +++ b/api/krusty/fnplugin_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + . "sigs.k8s.io/kustomize/api/krusty" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" "sigs.k8s.io/kustomize/kyaml/filesys" ) @@ -535,30 +536,33 @@ spec: func TestFnContainerTransformerWithConfig(t *testing.T) { skipIfNoDocker(t) - - // Function plugins should not need the env setup done by MakeEnhancedHarness th := kusttest_test.MakeHarness(t) - - th.WriteK(".", ` + o := th.MakeOptionsPluginsEnabled() + fSys := filesys.MakeFsOnDisk() + b := MakeKustomizer(&o) + tmpDir, err := filesys.NewTmpConfirmedDir() + assert.NoError(t, err) + assert.NoError(t, fSys.WriteFile(filepath.Join(tmpDir.String(), "kustomization.yaml"), []byte(` resources: - data1.yaml - data2.yaml transformers: - label_namespace.yaml -`) - - th.WriteF("data1.yaml", `apiVersion: v1 +`))) + assert.NoError(t, fSys.WriteFile(filepath.Join(tmpDir.String(), "data1.yaml"), []byte(` +apiVersion: v1 kind: Namespace metadata: name: my-namespace -`) - th.WriteF("data2.yaml", `apiVersion: v1 +`))) + assert.NoError(t, fSys.WriteFile(filepath.Join(tmpDir.String(), "data2.yaml"), []byte(` +apiVersion: v1 kind: Namespace metadata: name: another-namespace -`) - - th.WriteF("label_namespace.yaml", `apiVersion: v1 +`))) + assert.NoError(t, fSys.WriteFile(filepath.Join(tmpDir.String(), "label_namespace.yaml"), []byte(` +apiVersion: v1 kind: ConfigMap metadata: name: label_namespace @@ -569,11 +573,14 @@ metadata: data: label_name: my-ns-name label_value: function-test -`) - - m := th.Run(".", th.MakeOptionsPluginsEnabled()) - th.AssertActualEqualsExpected(m, ` -apiVersion: v1 +`))) + m, err := b.Run( + fSys, + tmpDir.String()) + assert.NoError(t, err) + actual, err := m.AsYaml() + assert.NoError(t, err) + assert.Equal(t, `apiVersion: v1 kind: Namespace metadata: labels: @@ -586,24 +593,22 @@ metadata: labels: my-ns-name: function-test name: another-namespace -`) +`, string(actual)) } func TestFnContainerEnvVars(t *testing.T) { skipIfNoDocker(t) - - // Function plugins should not need the env setup done by MakeEnhancedHarness th := kusttest_test.MakeHarness(t) - - th.WriteK(".", ` + o := th.MakeOptionsPluginsEnabled() + fSys := filesys.MakeFsOnDisk() + b := MakeKustomizer(&o) + tmpDir, err := filesys.NewTmpConfirmedDir() + assert.NoError(t, err) + assert.NoError(t, fSys.WriteFile(filepath.Join(tmpDir.String(), "kustomization.yaml"), []byte(` generators: - gener.yaml -`) - - // TODO: cheange image to gcr.io/kpt-functions/templater:stable - // when https://github.com/GoogleContainerTools/kpt-functions-catalog/pull/103 - // is merged - th.WriteF("gener.yaml", ` +`))) + assert.NoError(t, fSys.WriteFile(filepath.Join(tmpDir.String(), "gener.yaml"), []byte(` apiVersion: v1 kind: ConfigMap metadata: @@ -622,14 +627,176 @@ data: name: env data: value: '{{ env "TESTTEMPLATE" }}' -`) - m := th.Run(".", th.MakeOptionsPluginsEnabled()) - th.AssertActualEqualsExpected(m, ` -apiVersion: v1 +`))) + m, err := b.Run( + fSys, + tmpDir.String()) + assert.NoError(t, err) + actual, err := m.AsYaml() + assert.NoError(t, err) + assert.Equal(t, `apiVersion: v1 data: value: value kind: ConfigMap metadata: name: env +`, string(actual)) +} + +func TestFnContainerFnMounts(t *testing.T) { + skipIfNoDocker(t) + th := kusttest_test.MakeHarness(t) + o := th.MakeOptionsPluginsEnabled() + fSys := filesys.MakeFsOnDisk() + b := MakeKustomizer(&o) + tmpDir, err := filesys.NewTmpConfirmedDir() + assert.NoError(t, err) + assert.NoError(t, fSys.WriteFile(filepath.Join(tmpDir.String(), "kustomization.yaml"), []byte(` +generators: +- gener.yaml +`))) + assert.NoError(t, fSys.WriteFile(filepath.Join(tmpDir.String(), "gener.yaml"), []byte(` +apiVersion: v1alpha1 +kind: RenderHelmChart +metadata: + name: demo + annotations: + config.kubernetes.io/function: | + container: + image: gcr.io/kpt-fn/render-helm-chart:v0.1.0 + mounts: + - type: "bind" + src: "./charts" + dst: "/tmp/charts" +helmCharts: +- name: helloworld-chart + releaseName: test + valuesFile: /tmp/charts/helloworld-values/values.yaml +`))) + assert.NoError(t, fSys.MkdirAll(filepath.Join(tmpDir.String(), "charts", "helloworld-chart", "templates"))) + assert.NoError(t, fSys.MkdirAll(filepath.Join(tmpDir.String(), "charts", "helloworld-values"))) + assert.NoError(t, fSys.WriteFile(filepath.Join(tmpDir.String(), "charts", "helloworld-chart", "Chart.yaml"), []byte(` +apiVersion: v2 +name: helloworld-chart +description: A Helm chart for Kubernetes +type: application +version: 0.1.0 +appVersion: 1.16.0 +`))) + assert.NoError(t, fSys.WriteFile(filepath.Join(tmpDir.String(), "charts", "helloworld-chart", "templates", "deployment.yaml"), []byte(` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: name +spec: + replicas: {{ .Values.replicaCount }} +`))) + assert.NoError(t, fSys.WriteFile(filepath.Join(tmpDir.String(), "charts", "helloworld-values", "values.yaml"), []byte(` +replicaCount: 5 +`))) + m, err := b.Run( + fSys, + tmpDir.String()) + assert.NoError(t, err) + actual, err := m.AsYaml() + assert.NoError(t, err) + assert.Equal(t, `apiVersion: apps/v1 +kind: Deployment +metadata: + name: name +spec: + replicas: 5 +`, string(actual)) +} + +func TestFnContainerMountsLoadRestrictions_absolute(t *testing.T) { + skipIfNoDocker(t) + th := kusttest_test.MakeHarness(t) + o := th.MakeOptionsPluginsEnabled() + fSys := filesys.MakeFsOnDisk() + b := MakeKustomizer(&o) + tmpDir, err := filesys.NewTmpConfirmedDir() + assert.NoError(t, err) + assert.NoError(t, fSys.WriteFile(filepath.Join(tmpDir.String(), "kustomization.yaml"), []byte(` +generators: + - |- + apiVersion: v1alpha1 + kind: RenderHelmChart + metadata: + name: demo + annotations: + config.kubernetes.io/function: | + container: + image: gcr.io/kpt-fn/render-helm-chart:v0.1.0 + mounts: + - type: "bind" + src: "/tmp/dir" + dst: "/tmp/charts" +`))) + _, err = b.Run( + fSys, + tmpDir.String()) + assert.Error(t, err) + assert.Contains(t, err.Error(), "loading generator plugins: plugin RenderHelmChart."+ + "v1alpha1.[noGrp]/demo.[noNs] with mount path '/tmp/dir' is not permitted; mount paths must"+ + " be relative to the current kustomization directory") +} + +func TestFnContainerMountsLoadRestrictions_outsideCurrentDir(t *testing.T) { + skipIfNoDocker(t) + th := kusttest_test.MakeHarness(t) + o := th.MakeOptionsPluginsEnabled() + fSys := filesys.MakeFsOnDisk() + b := MakeKustomizer(&o) + tmpDir, err := filesys.NewTmpConfirmedDir() + assert.NoError(t, err) + assert.NoError(t, fSys.WriteFile(filepath.Join(tmpDir.String(), "kustomization.yaml"), []byte(` +generators: + - |- + apiVersion: v1alpha1 + kind: RenderHelmChart + metadata: + name: demo + annotations: + config.kubernetes.io/function: | + container: + image: gcr.io/kpt-fn/render-helm-chart:v0.1.0 + mounts: + - type: "bind" + src: "./tmp/../../dir" + dst: "/tmp/charts" +`))) + _, err = b.Run( + fSys, + tmpDir.String()) + assert.Error(t, err) + assert.Contains(t, err.Error(), "loading generator plugins: plugin RenderHelmChart."+ + "v1alpha1.[noGrp]/demo.[noNs] with mount path './tmp/../../dir' is not permitted; mount paths must "+ + "be under the current kustomization directory") +} + +func TestFnContainerMountsLoadRestrictions_root(t *testing.T) { + skipIfNoDocker(t) + th := kusttest_test.MakeHarness(t) + + th.WriteK(".", ` +generators: +- gener.yaml +`) + // Create generator config + th.WriteF("gener.yaml", ` +apiVersion: examples.config.kubernetes.io/v1beta1 +kind: CockroachDB +metadata: + name: demo + annotations: + config.kubernetes.io/function: | + container: + image: gcr.io/kustomize-functions/example-cockroachdb:v0.1.0 +spec: + replicas: 3 `) + err := th.RunWithErr(".", th.MakeOptionsPluginsEnabled()) + assert.Error(t, err) + assert.EqualError(t, err, "couldn't execute function: root working directory '/' not allowed") } diff --git a/kyaml/fn/runtime/container/container.go b/kyaml/fn/runtime/container/container.go index c7cda7dbda..76b51a8c50 100644 --- a/kyaml/fn/runtime/container/container.go +++ b/kyaml/fn/runtime/container/container.go @@ -6,10 +6,11 @@ package container import ( "fmt" "os" + "path/filepath" + "sigs.k8s.io/kustomize/kyaml/errors" runtimeexec "sigs.k8s.io/kustomize/kyaml/fn/runtime/exec" "sigs.k8s.io/kustomize/kyaml/fn/runtime/runtimeutil" - "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -151,11 +152,14 @@ func (c *Filter) setupExec() error { if c.Exec.Path != "" { return nil } - wd, err := os.Getwd() - if err != nil { - return err + + if c.Exec.WorkingDir == "" { + wd, err := os.Getwd() + if err != nil { + return errors.Wrap(err) + } + c.Exec.WorkingDir = wd } - c.Exec.WorkingDir = wd path, args := c.getCommand() c.Exec.Path = path @@ -183,8 +187,11 @@ func (c *Filter) getCommand() (string, []string) { // note: don't make fs readonly because things like heredoc rely on writing tmp files } - // TODO(joncwong): Allow StorageMount fields to have default values. for _, storageMount := range c.StorageMounts { + // convert declarative relative paths to absolute (otherwise docker will throw an error) + if !filepath.IsAbs(storageMount.Src) { + storageMount.Src = filepath.Join(c.Exec.WorkingDir, storageMount.Src) + } args = append(args, "--mount", storageMount.String()) } diff --git a/kyaml/fn/runtime/container/container_test.go b/kyaml/fn/runtime/container/container_test.go index 06c3a7dd23..f5ff0259b2 100644 --- a/kyaml/fn/runtime/container/container_test.go +++ b/kyaml/fn/runtime/container/container_test.go @@ -7,6 +7,7 @@ import ( "bytes" "fmt" "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -81,18 +82,19 @@ metadata: "--network", "none", "--user", "nobody", "--security-opt=no-new-privileges", - "--mount", fmt.Sprintf("type=%s,source=%s,target=%s,readonly", "bind", "/mount/path", "/local/"), - "--mount", fmt.Sprintf("type=%s,source=%s,target=%s", "bind", "/mount/pathrw", "/localrw/"), - "--mount", fmt.Sprintf("type=%s,source=%s,target=%s,readonly", "volume", "myvol", "/local/"), - "--mount", fmt.Sprintf("type=%s,source=%s,target=%s,readonly", "tmpfs", "", "/local/"), + // use filepath.Join for Windows filepath handling + "--mount", fmt.Sprintf("type=%s,source=%s,target=%s,readonly", "bind", getAbsFilePath(string(filepath.Separator), "mount", "path"), "/local/"), + "--mount", fmt.Sprintf("type=%s,source=%s,target=%s", "bind", getAbsFilePath(string(filepath.Separator), "mount", "pathrw"), "/localrw/"), + "--mount", fmt.Sprintf("type=%s,source=%s,target=%s,readonly", "volume", getAbsFilePath(string(filepath.Separator), "myvol"), "/local/"), + "--mount", fmt.Sprintf("type=%s,source=%s,target=%s,readonly", "tmpfs", getAbsFilePath(string(filepath.Separator)), "/local/"), }, containerSpec: runtimeutil.ContainerSpec{ Image: "example.com:version", StorageMounts: []runtimeutil.StorageMount{ - {MountType: "bind", Src: "/mount/path", DstPath: "/local/"}, - {MountType: "bind", Src: "/mount/pathrw", DstPath: "/localrw/", ReadWriteMode: true}, - {MountType: "volume", Src: "myvol", DstPath: "/local/"}, - {MountType: "tmpfs", Src: "", DstPath: "/local/"}, + {MountType: "bind", Src: getAbsFilePath(string(filepath.Separator), "mount", "path"), DstPath: "/local/"}, + {MountType: "bind", Src: getAbsFilePath(string(filepath.Separator), "mount", "pathrw"), DstPath: "/localrw/", ReadWriteMode: true}, + {MountType: "volume", Src: getAbsFilePath(string(filepath.Separator), "myvol"), DstPath: "/local/"}, + {MountType: "tmpfs", Src: getAbsFilePath(string(filepath.Separator)), DstPath: "/local/"}, }, }, UIDGID: "nobody", @@ -247,3 +249,8 @@ func getWorkingDir(t *testing.T) string { require.NoError(t, err) return wd } + +func getAbsFilePath(args ...string) string { + path, _ := filepath.Abs(filepath.Join(args...)) + return path +} diff --git a/kyaml/fn/runtime/runtimeutil/functiontypes.go b/kyaml/fn/runtime/runtimeutil/functiontypes.go index 83e7ff0eca..39cb241959 100644 --- a/kyaml/fn/runtime/runtimeutil/functiontypes.go +++ b/kyaml/fn/runtime/runtimeutil/functiontypes.go @@ -136,9 +136,6 @@ type FunctionSpec struct { // ExecSpec is the spec for running a function as an executable Exec ExecSpec `json:"exec,omitempty" yaml:"exec,omitempty"` - - // Mounts are the storage or directories to mount into the container - StorageMounts []StorageMount `json:"mounts,omitempty" yaml:"mounts,omitempty"` } type ExecSpec struct { @@ -208,9 +205,7 @@ func GetFunctionSpec(n *yaml.RNode) *FunctionSpec { if err != nil { return nil } - if fn := getFunctionSpecFromAnnotation(n, meta); fn != nil { - fn.StorageMounts = []StorageMount{} return fn } diff --git a/kyaml/runfn/runfn.go b/kyaml/runfn/runfn.go index eda2c29025..c855144d73 100644 --- a/kyaml/runfn/runfn.go +++ b/kyaml/runfn/runfn.go @@ -329,8 +329,11 @@ func (r RunFns) getFunctionFilters(global bool, fns ...*yaml.RNode) ( continue } cf, ok := c.(*container.Filter) - if global && ok { - cf.Exec.GlobalScope = true + if ok { + if global { + cf.Exec.GlobalScope = true + } + cf.Exec.WorkingDir = r.WorkingDir } fltrs = append(fltrs, c) } @@ -468,11 +471,17 @@ func (r *RunFns) ffp(spec runtimeutil.FunctionSpec, api *yaml.RNode, currentUser if err != nil { return nil, err } + + // Storage mounts can either come from kustomize fn run --mounts, + // or from the declarative function mounts field. + storageMounts := spec.Container.StorageMounts + storageMounts = append(storageMounts, r.StorageMounts...) + c := container.NewContainer( runtimeutil.ContainerSpec{ Image: spec.Container.Image, Network: spec.Container.Network, - StorageMounts: r.StorageMounts, + StorageMounts: storageMounts, Env: spec.Container.Env, }, uidgid, diff --git a/plugin/builtin/hashtransformer/HashTransformer_test.go b/plugin/builtin/hashtransformer/HashTransformer_test.go index 3d62541ac0..27972250cd 100644 --- a/plugin/builtin/hashtransformer/HashTransformer_test.go +++ b/plugin/builtin/hashtransformer/HashTransformer_test.go @@ -6,7 +6,7 @@ package main_test import ( "testing" - "sigs.k8s.io/kustomize/api/testutils/kusttest" + kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" ) func TestHashTransformer(t *testing.T) {