Repository: GoogleContainerTools/kpt
Pull request #2095 (2021-05-25 02:44:40)

Generator with subpackages
This PR is to add test case for generator function(upsert-resource) with subpackages.

https://github.com/GoogleContainerTools/kpt/issues/2050


Patch:
diff --git a/internal/fnruntime/runner.go b/internal/fnruntime/runner.go
index 86623d78..14b79361 100644
--- a/internal/fnruntime/runner.go
+++ b/internal/fnruntime/runner.go
@@ -29,7 +29,6 @@ import (
 	"github.com/GoogleContainerTools/kpt/internal/types"
 	fnresult "github.com/GoogleContainerTools/kpt/pkg/api/fnresult/v1alpha2"
 	kptfilev1alpha2 "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1alpha2"
-	"sigs.k8s.io/kustomize/kyaml/fn/framework"
 	"sigs.k8s.io/kustomize/kyaml/fn/runtime/runtimeutil"
 	"sigs.k8s.io/kustomize/kyaml/kio"
 	"sigs.k8s.io/kustomize/kyaml/kio/kioutil"
@@ -174,6 +173,54 @@ func parseStructuredResult(yml *yaml.RNode, fnResult *fnresult.Result) error {
 	if err != nil {
 		return err
 	}
+
+	return parseNameAndNamespace(yml, fnResult)
+}
+
+// parseNameAndNamespace populates name and namespace in fnResult.Result if a
+// function (e.g. using kyaml Go SDKs) gives results in a schema
+// that puts a resourceRef's name and namespace under a metadata field
+// TODO: fix upstream (https://github.com/GoogleContainerTools/kpt/issues/2091)
+func parseNameAndNamespace(yml *yaml.RNode, fnResult *fnresult.Result) error {
+	items, err := yml.Elements()
+	if err != nil {
+		return err
+	}
+
+	for i := range items {
+		if err := populateResourceRef(items[i], &fnResult.Results[i]); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func populateResourceRef(item *yaml.RNode, resultItem *fnresult.ResultItem) error {
+	r, err := item.Pipe(yaml.Lookup("resourceRef", "metadata"))
+	if err != nil {
+		return err
+	}
+	if r == nil {
+		return nil
+	}
+	nameNode, err := r.Pipe(yaml.Lookup("name"))
+	if err != nil {
+		return err
+	}
+	namespaceNode, err := r.Pipe(yaml.Lookup("namespace"))
+	if err != nil {
+		return err
+	}
+	if nameNode != nil {
+		resultItem.ResourceRef.Name = strings.TrimSpace(nameNode.MustString())
+	}
+	if namespaceNode != nil {
+		namespace := strings.TrimSpace(namespaceNode.MustString())
+		if namespace != "" && namespace != "''" {
+			resultItem.ResourceRef.Namespace = strings.TrimSpace(namespace)
+		}
+	}
 	return nil
 }
 
@@ -300,7 +347,7 @@ func (ri *multiLineFormatter) String() string {
 }
 
 // resultToString converts given structured result item to string format.
-func resultToString(result framework.ResultItem) string {
+func resultToString(result fnresult.ResultItem) string {
 	// TODO: Go SDK should implement Stringer method
 	// for framework.ResultItem. This is a temporary
 	// wrapper that will eventually be moved to Go SDK.
@@ -333,7 +380,7 @@ func resultToString(result framework.ResultItem) string {
 	return s.String()
 }
 
-func resourceRefToString(ref yaml.ResourceMeta) string {
+func resourceRefToString(ref yaml.ResourceIdentifier) string {
 	s := strings.Builder{}
 	if ref.APIVersion != "" {
 		s.WriteString(fmt.Sprintf("%s/", ref.APIVersion))
diff --git a/pkg/api/fnresult/v1alpha2/types.go b/pkg/api/fnresult/v1alpha2/types.go
index cc761ca1..8e521bda 100644
--- a/pkg/api/fnresult/v1alpha2/types.go
+++ b/pkg/api/fnresult/v1alpha2/types.go
@@ -36,7 +36,7 @@ type Result struct {
 	// ExitCode is the exit code from running the function
 	ExitCode int `yaml:"exitCode"`
 	// Results is the list of results for the function
-	Results []framework.ResultItem `yaml:"results,omitempty"`
+	Results []ResultItem `yaml:"results,omitempty"`
 }
 
 const (
@@ -73,3 +73,24 @@ func NewResultList() *ResultList {
 		Items: []Result{},
 	}
 }
+
+// ResultItem is a duplicate of framework.ResultItem, except that
+// ResourceRef uses yaml.ResourceIdentifier here, whereas framework.ResultItem
+// uses yaml.ResourceMeta. Eventually, we will need to fix it upstream.
+// TODO: https://github.com/GoogleContainerTools/kpt/issues/2091
+type ResultItem struct {
+	// Message is a human readable message
+	Message string `yaml:"message,omitempty"`
+
+	// Severity is the severity of this result
+	Severity framework.Severity `yaml:"severity,omitempty"`
+
+	// ResourceRef is a reference to a resource
+	ResourceRef yaml.ResourceIdentifier `yaml:"resourceRef,omitempty"`
+
+	// Field is a reference to the field in a resource this result refers to
+	Field framework.Field `yaml:"field,omitempty"`
+
+	// File references a file containing the resource this result refers to
+	File framework.File `yaml:"file,omitempty"`
+}


Test patch:
diff --git a/e2e/testdata/fn-eval/function-env/.expected/config.yaml b/e2e/testdata/fn-eval/function-env/.expected/config.yaml
new file mode 100644
index 00000000..0678c588
--- /dev/null
+++ b/e2e/testdata/fn-eval/function-env/.expected/config.yaml
@@ -0,0 +1,16 @@
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+exitCode: 1
+stdErr: "FOO=BAR EXPORT_ENV=export_env_value"
diff --git a/e2e/testdata/fn-eval/function-env/.expected/exec.sh b/e2e/testdata/fn-eval/function-env/.expected/exec.sh
new file mode 100644
index 00000000..37edc9f1
--- /dev/null
+++ b/e2e/testdata/fn-eval/function-env/.expected/exec.sh
@@ -0,0 +1,22 @@
+#! /bin/bash
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -eo pipefail
+
+IMAGE_TAG="gcr.io/kpt-fn-demo/printenv:v0.1"
+export EXPORT_ENV="export_env_value"
+
+kpt fn source \
+| kpt fn eval - --image $IMAGE_TAG -e EXPORT_ENV -e FOO=BAR
diff --git a/e2e/testdata/fn-eval/function-env/.krmginore b/e2e/testdata/fn-eval/function-env/.krmginore
new file mode 100644
index 00000000..9d7a4007
--- /dev/null
+++ b/e2e/testdata/fn-eval/function-env/.krmginore
@@ -0,0 +1,1 @@
+.expected
diff --git a/e2e/testdata/fn-eval/output-to-stdout/.expected/exec.sh b/e2e/testdata/fn-eval/output-to-stdout/.expected/exec.sh
index 7a989309..a662d1e3 100644
--- a/e2e/testdata/fn-eval/output-to-stdout/.expected/exec.sh
+++ b/e2e/testdata/fn-eval/output-to-stdout/.expected/exec.sh
@@ -1,4 +1,19 @@
 #! /bin/bash
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -eo pipefail
 
 kpt fn source |\
 kpt fn eval - --image gcr.io/kpt-fn/set-namespace:v0.1 -- namespace=staging
diff --git a/e2e/testdata/fn-render/fnresult-fn-failure/.expected/results.yaml b/e2e/testdata/fn-render/fnresult-fn-failure/.expected/results.yaml
index 023dcf41..f24539a7 100755
--- a/e2e/testdata/fn-render/fnresult-fn-failure/.expected/results.yaml
+++ b/e2e/testdata/fn-render/fnresult-fn-failure/.expected/results.yaml
@@ -21,6 +21,7 @@ items:
           resourceRef:
             apiVersion: apps/v1
             kind: Deployment
+            name: nginx-deployment
           field:
             path: selector
           file:
@@ -30,6 +31,7 @@ items:
           resourceRef:
             apiVersion: apps/v1
             kind: Deployment
+            name: nginx-deployment
           field:
             path: template
           file:
@@ -39,6 +41,7 @@ items:
           resourceRef:
             apiVersion: apps/v1
             kind: Deployment
+            name: nginx-deployment
           field:
             path: spec.replicas
           file:
@@ -52,6 +55,7 @@ items:
           resourceRef:
             apiVersion: custom.io/v1
             kind: Custom
+            name: custom
           file:
             path: resources.yaml
             index: 1
diff --git a/e2e/testdata/fn-render/generator-subpkgs/.expected/diff.patch b/e2e/testdata/fn-render/generator-subpkgs/.expected/diff.patch
new file mode 100644
index 00000000..50c85a58
--- /dev/null
+++ b/e2e/testdata/fn-render/generator-subpkgs/.expected/diff.patch
@@ -0,0 +1,25 @@
+diff --git a/db/configmap_db-map.yaml b/db/configmap_db-map.yaml
+new file mode 100644
+index 0000000..7a0f3af
+--- /dev/null
++++ b/db/configmap_db-map.yaml
+@@ -0,0 +1,6 @@
++apiVersion: v1
++kind: ConfigMap
++metadata:
++  name: db-map
++data:
++  db-key: db-value
+diff --git a/def/configmap_parent-map.yaml b/def/configmap_parent-map.yaml
+new file mode 100644
+index 0000000..fd1e17d
+--- /dev/null
++++ b/def/configmap_parent-map.yaml
+@@ -0,0 +1,7 @@
++apiVersion: v1
++kind: ConfigMap
++metadata:
++  name: parent-map
++  namespace: def
++data:
++  parent-key: parent-value
\ No newline at end of file
diff --git a/e2e/testdata/fn-render/generator-subpkgs/.krmignore b/e2e/testdata/fn-render/generator-subpkgs/.krmignore
new file mode 100644
index 00000000..9d7a4007
--- /dev/null
+++ b/e2e/testdata/fn-render/generator-subpkgs/.krmignore
@@ -0,0 +1,1 @@
+.expected
diff --git a/e2e/testdata/fn-render/generator-subpkgs/Kptfile b/e2e/testdata/fn-render/generator-subpkgs/Kptfile
new file mode 100644
index 00000000..9fa913c0
--- /dev/null
+++ b/e2e/testdata/fn-render/generator-subpkgs/Kptfile
@@ -0,0 +1,8 @@
+apiVersion: kpt.dev/v1alpha2
+kind: Kptfile
+metadata:
+  name: app-with-generator
+pipeline:
+  mutators:
+    - image: gcr.io/kpt-fn/upsert-resource:unstable
+      configPath: fn-config.yaml
diff --git a/e2e/testdata/fn-render/generator-subpkgs/db/Kptfile b/e2e/testdata/fn-render/generator-subpkgs/db/Kptfile
new file mode 100644
index 00000000..020dc8ac
--- /dev/null
+++ b/e2e/testdata/fn-render/generator-subpkgs/db/Kptfile
@@ -0,0 +1,8 @@
+apiVersion: kpt.dev/v1alpha2
+kind: Kptfile
+metadata:
+  name: db
+pipeline:
+  mutators:
+    - image: gcr.io/kpt-fn/upsert-resource:unstable
+      configPath: fn-config.yaml
diff --git a/e2e/testdata/fn-render/generator-subpkgs/db/fn-config.yaml b/e2e/testdata/fn-render/generator-subpkgs/db/fn-config.yaml
new file mode 100644
index 00000000..7a0f3af0
--- /dev/null
+++ b/e2e/testdata/fn-render/generator-subpkgs/db/fn-config.yaml
@@ -0,0 +1,6 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: db-map
+data:
+  db-key: db-value
diff --git a/e2e/testdata/fn-render/generator-subpkgs/db/resources.yaml b/e2e/testdata/fn-render/generator-subpkgs/db/resources.yaml
new file mode 100644
index 00000000..ac1fd966
--- /dev/null
+++ b/e2e/testdata/fn-render/generator-subpkgs/db/resources.yaml
@@ -0,0 +1,19 @@
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: db
+spec:
+  replicas: 3
diff --git a/e2e/testdata/fn-render/generator-subpkgs/fn-config.yaml b/e2e/testdata/fn-render/generator-subpkgs/fn-config.yaml
new file mode 100644
index 00000000..fd1e17d7
--- /dev/null
+++ b/e2e/testdata/fn-render/generator-subpkgs/fn-config.yaml
@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: parent-map
+  namespace: def
+data:
+  parent-key: parent-value
diff --git a/e2e/testdata/fn-render/generator-subpkgs/resources.yaml b/e2e/testdata/fn-render/generator-subpkgs/resources.yaml
new file mode 100644
index 00000000..7a494c9c
--- /dev/null
+++ b/e2e/testdata/fn-render/generator-subpkgs/resources.yaml
@@ -0,0 +1,26 @@
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  replicas: 3
+---
+apiVersion: custom.io/v1
+kind: Custom
+metadata:
+  name: custom
+spec:
+  image: nginx:1.2.3
diff --git a/e2e/testdata/fn-render/kubeval-failure/.expected/config.yaml b/e2e/testdata/fn-render/kubeval-failure/.expected/config.yaml
index b257d428..5e58752b 100644
--- a/e2e/testdata/fn-render/kubeval-failure/.expected/config.yaml
+++ b/e2e/testdata/fn-render/kubeval-failure/.expected/config.yaml
@@ -15,3 +15,4 @@
 # One of the functions in the pipeline fails resulting in
 # non-zero exit code and no changes in the resources.
 exitCode: 1
+stdOut: "[ERROR] selector is required in object \"apps/v1/Deployment/nginx-deployment\" in file \"resources.yaml\" in field \"selector\""
diff --git a/e2e/testdata/fn-render/kubeval-failure/.expected/results.yaml b/e2e/testdata/fn-render/kubeval-failure/.expected/results.yaml
index 929e572a..352d463d 100755
--- a/e2e/testdata/fn-render/kubeval-failure/.expected/results.yaml
+++ b/e2e/testdata/fn-render/kubeval-failure/.expected/results.yaml
@@ -12,6 +12,7 @@ items:
           resourceRef:
             apiVersion: apps/v1
             kind: Deployment
+            name: nginx-deployment
           field:
             path: selector
           file:
@@ -21,6 +22,7 @@ items:
           resourceRef:
             apiVersion: apps/v1
             kind: Deployment
+            name: nginx-deployment
           field:
             path: template
           file:
@@ -30,6 +32,7 @@ items:
           resourceRef:
             apiVersion: apps/v1
             kind: Deployment
+            name: nginx-deployment
           field:
             path: spec.replicas
           file:
@@ -41,6 +44,7 @@ items:
           resourceRef:
             apiVersion: custom.io/v1
             kind: Custom
+            name: custom
           file:
             path: resources.yaml
             index: 1
diff --git a/internal/fnruntime/runner_test.go b/internal/fnruntime/runner_test.go
index 3ec3dd6f..c2d36c4d 100644
--- a/internal/fnruntime/runner_test.go
+++ b/internal/fnruntime/runner_test.go
@@ -24,6 +24,8 @@ import (
 	"strings"
 	"testing"
 
+	fnresult "github.com/GoogleContainerTools/kpt/pkg/api/fnresult/v1alpha2"
+
 	"github.com/GoogleContainerTools/kpt/internal/types"
 	kptfilev1alpha2 "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1alpha2"
 	"github.com/stretchr/testify/assert"
@@ -447,3 +449,147 @@ spec:
 		}
 	}
 }
+
+func TestGetResourceRefMetadata(t *testing.T) {
+	tests := map[string]struct {
+		input    string // input
+		expected string // expected result
+	}{
+		"new format with name": {
+			input: `
+message: selector is required
+severity: error
+resourceRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: nginx-deployment
+field:
+    path: selector
+file:
+    path: resources.yaml
+`,
+			expected: `message: selector is required
+severity: error
+resourceRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: nginx-deployment
+field:
+    path: selector
+file:
+    path: resources.yaml
+`,
+		},
+		"new format with namespace": {
+			input: `
+message: selector is required
+severity: error
+resourceRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: nginx-deployment
+    namespace: my-namespace
+field:
+    path: selector
+file:
+    path: resources.yaml
+`,
+			expected: `message: selector is required
+severity: error
+resourceRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: nginx-deployment
+    namespace: my-namespace
+field:
+    path: selector
+file:
+    path: resources.yaml
+`,
+		},
+		"old format with name": {
+			input: `
+message: selector is required
+severity: error
+resourceRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    metadata:
+      name: nginx-deployment
+field:
+    path: selector
+file:
+    path: resources.yaml
+`,
+			expected: `message: selector is required
+severity: error
+resourceRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: nginx-deployment
+field:
+    path: selector
+file:
+    path: resources.yaml
+`,
+		},
+		"old format with namespace": {
+			input: `
+message: selector is required
+severity: error
+resourceRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    metadata:
+      name: nginx-deployment
+      namespace: my-namespace
+field:
+    path: selector
+file:
+    path: resources.yaml
+`,
+			expected: `message: selector is required
+severity: error
+resourceRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: nginx-deployment
+    namespace: my-namespace
+field:
+    path: selector
+file:
+    path: resources.yaml
+`,
+		},
+		"no resourceRef": {
+			input: `
+message: selector is required
+severity: error
+field:
+    path: selector
+file:
+    path: resources.yaml
+`,
+			expected: `message: selector is required
+severity: error
+field:
+    path: selector
+file:
+    path: resources.yaml
+`,
+		},
+	}
+	for _, tc := range tests {
+		yml, err := yaml.Parse(tc.input)
+		assert.NoError(t, err)
+
+		result := &fnresult.ResultItem{}
+		err = yaml.Unmarshal([]byte(tc.input), result)
+		assert.NoError(t, err)
+		assert.NoError(t, populateResourceRef(yml, result))
+
+		out, err := yaml.Marshal(result)
+		assert.NoError(t, err)
+		assert.Equal(t, tc.expected, string(out))
+	}
+}
diff --git a/pkg/test/runner/runner.go b/pkg/test/runner/runner.go
index 998ccf1c..7883db57 100644
--- a/pkg/test/runner/runner.go
+++ b/pkg/test/runner/runner.go
@@ -331,16 +331,16 @@ func (r *Runner) runFnRender() error {
 		if err != nil {
 			return err
 		}
-		// we passed result check, now we should break if the command error
-		// is expected
-		if fnErr != nil {
-			break
-		}
-
+		// we passed result check, now we should run teardown script and break
+		// if the command error is expected
 		err = r.runTearDownScript(pkgPath)
 		if err != nil {
 			return err
 		}
+		if fnErr != nil {
+			break
+		}
+
 	}
 	return nil
 }