Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix containerized function mounts issue #4489

Merged
merged 5 commits into from Apr 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
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
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) {
natasha41575 marked this conversation as resolved.
Show resolved Hide resolved
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
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) {
KnVerey marked this conversation as resolved.
Show resolved Hide resolved
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
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