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.

Sign up for GitHub

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

Jump to bottom

fix containerized function mounts issue #4489

Merged
merged 5 commits into from Apr 18, 2022
Merged

fix containerized function mounts issue #4489

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6ccb493
fix containerized function mounts issue
natasha41575 Feb 25, 2022
4fdbebf
skip path test on windows
natasha41575 Apr 11, 2022
96977a1
move test out of temp dir
natasha41575 Apr 11, 2022
dfaa43c
update tests to deal with new working dir restrictions
natasha41575 Apr 14, 2022
e0166b5
code review
natasha41575 Apr 15, 2022
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
4 changes: 2 additions & 2 deletions Makefile
View file
Open in desktop
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 Expand Up @@ -132,7 +132,7 @@ test-unit-all: \

# This target is used by our Github Actions CI to run unit tests for all non-plugin modules in multiple GOOS environments.
.PHONY: test-unit-non-plugin
test-unit-non-plugin:
test-unit-non-plugin: kustomize
./hack/for-each-module.sh "make test" "./plugin/*" 15

.PHONY: build-non-plugin-all
Expand Down
11 changes: 11 additions & 0 deletions api/internal/plugins/loader/loader.go
View file
Open in desktop
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
256 changes: 223 additions & 33 deletions api/krusty/fnplugin_test.go
View file
Open in desktop
Expand Up @@ -4,12 +4,14 @@
package krusty_test

import (
"bytes"
"os"
"os/exec"
"path/filepath"
"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 +537,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 +574,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 +594,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 +628,198 @@ 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 TestFnContainerMounts(t *testing.T) {
skipIfNoDocker(t)

path, err := os.Getwd()
assert.NoError(t, err)

testDir := filepath.Join(path, "testdata", "fnmounts")
command := exec.Command("kustomize", "build", testDir, "--enable-alpha-plugins")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this test exec'ing Kustomize instead of using the test framework? How do we know the binary that will be found corresponds to the version of the code we think we're testing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally I was trying to copy the entire testdata/fnmounts directory into the temp directory where the test framework was running, which turned out to be a headache.

But with a pared down helm chart (per your other suggestion), this is much easier to do with the test framework. Updated!

var stdout bytes.Buffer
var stderr bytes.Buffer
command.Stdout = &stdout
command.Stderr = &stderr
t.Log(stderr.String())

assert.NoError(t, command.Run())
assert.Equal(t, `apiVersion: v1
kind: ServiceAccount
metadata:
labels:
app.kubernetes.io/instance: test
app.kubernetes.io/managed-by: Helm
app.kubernetes.io/name: helloworld-chart
app.kubernetes.io/version: 1.16.0
helm.sh/chart: helloworld-chart-0.1.0
name: test-helloworld-chart
---
apiVersion: v1
kind: Service
metadata:
labels:
app.kubernetes.io/instance: test
app.kubernetes.io/managed-by: Helm
app.kubernetes.io/name: helloworld-chart
app.kubernetes.io/version: 1.16.0
helm.sh/chart: helloworld-chart-0.1.0
name: test-helloworld-chart
spec:
ports:
- name: http
port: 80
protocol: TCP
targetPort: http
selector:
app.kubernetes.io/instance: test
app.kubernetes.io/name: helloworld-chart
type: ClusterIP
---
apiVersion: apps/v1
kind: Deployment
metadata:
labels:
app.kubernetes.io/instance: test
app.kubernetes.io/managed-by: Helm
app.kubernetes.io/name: helloworld-chart
app.kubernetes.io/version: 1.16.0
helm.sh/chart: helloworld-chart-0.1.0
name: test-helloworld-chart
spec:
replicas: 5
selector:
matchLabels:
app.kubernetes.io/instance: test
app.kubernetes.io/name: helloworld-chart
template:
metadata:
labels:
app.kubernetes.io/instance: test
app.kubernetes.io/name: helloworld-chart
spec:
containers:
- image: nginx:1.16.0
imagePullPolicy: Always
livenessProbe:
httpGet:
path: /
port: http
name: helloworld-chart
ports:
- containerPort: 80
name: http
protocol: TCP
readinessProbe:
httpGet:
path: /
port: http
resources: {}
securityContext: {}
securityContext: {}
serviceAccountName: test-helloworld-chart
---
apiVersion: v1
kind: Pod
metadata:
annotations:
helm.sh/hook: test-success
labels:
app.kubernetes.io/instance: test
app.kubernetes.io/managed-by: Helm
app.kubernetes.io/name: helloworld-chart
app.kubernetes.io/version: 1.16.0
helm.sh/chart: helloworld-chart-0.1.0
name: test-helloworld-chart-test-connection
spec:
containers:
- args:
- test-helloworld-chart:80
command:
- wget
image: busybox
name: wget
restartPolicy: Never
`, stdout.String())
}

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")
}
6 changes: 6 additions & 0 deletions api/krusty/testdata/fnmounts/charts/helloworld-chart/Chart.yaml
View file
Open in desktop
@@ -0,0 +1,6 @@
apiVersion: v2
name: helloworld-chart
description: A Helm chart for Kubernetes
type: application
version: 0.1.0
appVersion: 1.16.0
21 changes: 21 additions & 0 deletions api/krusty/testdata/fnmounts/charts/helloworld-chart/templates/NOTES.txt
View file
Open in desktop
@@ -0,0 +1,21 @@
1. Get the application URL by running these commands:
{{- if .Values.ingress.enabled }}
{{- range $host := .Values.ingress.hosts }}
{{- range .paths }}
http{{ if $.Values.ingress.tls }}s{{ end }}://{{ $host.host }}{{ . }}
{{- end }}
{{- end }}
{{- else if contains "NodePort" .Values.service.type }}
export NODE_PORT=$(kubectl get --namespace {{ .Release.Namespace }} -o jsonpath="{.spec.ports[0].nodePort}" services {{ include "helloworld-chart.fullname" . }})
export NODE_IP=$(kubectl get nodes --namespace {{ .Release.Namespace }} -o jsonpath="{.items[0].status.addresses[0].address}")
echo http://$NODE_IP:$NODE_PORT
{{- else if contains "LoadBalancer" .Values.service.type }}
NOTE: It may take a few minutes for the LoadBalancer IP to be available.
You can watch the status of by running 'kubectl get --namespace {{ .Release.Namespace }} svc -w {{ include "helloworld-chart.fullname" . }}'
export SERVICE_IP=$(kubectl get svc --namespace {{ .Release.Namespace }} {{ include "helloworld-chart.fullname" . }} --template "{{"{{ range (index .status.loadBalancer.ingress 0) }}{{.}}{{ end }}"}}")
echo http://$SERVICE_IP:{{ .Values.service.port }}
{{- else if contains "ClusterIP" .Values.service.type }}
export POD_NAME=$(kubectl get pods --namespace {{ .Release.Namespace }} -l "app.kubernetes.io/name={{ include "helloworld-chart.name" . }},app.kubernetes.io/instance={{ .Release.Name }}" -o jsonpath="{.items[0].metadata.name}")
echo "Visit http://127.0.0.1:8080 to use your application"
kubectl --namespace {{ .Release.Namespace }} port-forward $POD_NAME 8080:80
{{- end }}