Skip to content

Commit

Permalink
fix containerized function mounts issue (#4489)
Browse files Browse the repository at this point in the history 
* fix containerized function mounts issue

* skip path test on windows

* move test out of temp dir

* update tests to deal with new working dir restrictions

* code review
  • Loading branch information
@natasha41575
natasha41575 committed Apr 18, 2022
1 parent cf89eae commit 9d5491c
Show file tree
Hide file tree
Showing 8 changed files with 252 additions and 56 deletions.
2 changes: 1 addition & 1 deletion Makefile
View file
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions api/internal/plugins/loader/loader.go
View file
Expand Up @@ -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())
Expand Down
231 changes: 199 additions & 32 deletions api/krusty/fnplugin_test.go
View file
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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")
}
19 changes: 13 additions & 6 deletions kyaml/fn/runtime/container/container.go
View file
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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())
}

Expand Down

0 comments on commit 9d5491c

Please sign in to comment.