code review

e2e/testdata/fn-eval/fnconfig-missing-name/pkg/.expected/config.yaml

@@ -16,4 +16,4 @@ testType: eval
 image: gcr.io/kpt-fn/set-namespace:v0.1
 fnConfig: ../../config.yaml
 exitCode: 1
-stdErr: "fnconfig-missing-name/config.yaml' is missing field `metadata.name`"
+stdErr: "error: resource must have `metadata.name`"

e2e/testdata/fn-eval/multiple-fn-config-one-file/config.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: v1
+kind: ConfigMap
+metadata:
+  name: config
+data:
+  namespace: staging
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: config
+data:
+  namespace: staging

e2e/testdata/fn-eval/multiple-fn-config-one-file/pkg/.expected/config.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.
+
+testType: eval
+image: gcr.io/kpt-fn/set-namespace:v0.1.3
+fnConfig: ../../config.yaml
+exitCode: 1
+stdErr: "must not contain more than 1 config, got 2"

e2e/testdata/fn-eval/multiple-fn-config-one-file/pkg/.krmignore

@@ -0,0 +1 @@
+.expected

e2e/testdata/fn-eval/multiple-fn-config-one-file/pkg/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

e2e/testdata/fn-render/fnconfig-missing-name/.expected/config.yaml

@@ -13,8 +13,4 @@
 # limitations under the License.
 
 exitCode: 1
-stdErr: |
-  Kptfile is invalid:
-  Field: `pipeline.mutators[1].configPath`
-  Value: "labelconfig1.yaml"
-  Reason: resource must have `metadata.name`
+stdErr: "resource must have `metadata.name`"

e2e/testdata/fn-render/fnconfig-multiple-config-one-file/.expected/config.yaml

@@ -0,0 +1,15 @@
+# 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: "must not contain more than 1 config, got 2"

e2e/testdata/fn-render/fnconfig-multiple-config-one-file/.krmignore

@@ -0,0 +1 @@
+.expected

e2e/testdata/fn-render/fnconfig-multiple-config-one-file/Kptfile

@@ -0,0 +1,8 @@
+apiVersion: kpt.dev/v1alpha2
+kind: Kptfile
+metadata:
+  name: app-with-db
+pipeline:
+  mutators:
+    - image: gcr.io/kpt-fn/set-labels:v0.1.4
+      configPath: labelconfig.yaml

e2e/testdata/fn-render/fnconfig-multiple-config-one-file/labelconfig.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: v1
+kind: ConfigMap
+metadata:
+  name: label-config
+data:
+  tier: db
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: label-config
+data:
+  tier: db

e2e/testdata/fn-render/fnconfig-multiple-config-one-file/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

e2e/testdata/fn-render/missing-fnconfig/.expected/config.yaml

@@ -18,6 +18,3 @@ stdErr: |
   Field: `pipeline.mutators[1].configPath`
   Value: "labelconfig1.yaml"
   Reason: functionConfig must exist in the current package
-stdOut: |
-  [RUNNING] "gcr.io/kpt-fn/set-labels:v0.1.4"
-  [PASS] "gcr.io/kpt-fn/set-labels:v0.1.4"

pkg/api/kptfile/v1alpha2/validation.go

@@ -16,13 +16,13 @@ package v1alpha2
 
 import (
 	"fmt"
-	"github.com/GoogleContainerTools/kpt/internal/types"
 	"os"
 	"path/filepath"
 	"regexp"
-	"sigs.k8s.io/kustomize/kyaml/kio"
 	"strings"
 
+	"github.com/GoogleContainerTools/kpt/internal/types"
+	"sigs.k8s.io/kustomize/kyaml/kio"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
@@ -83,7 +83,7 @@ func (f *Function) validate(fnType string, idx int, pkgPath types.UniquePath) er
 				Reason: err.Error(),
 			}
 		}
-		if _ , err := ValidateFnConfigPath(pkgPath, f.ConfigPath); err != nil {
+		if _ , err := GetValidatedFnConfigFromPath(pkgPath, f.ConfigPath); err != nil {
 			return &ValidateError{
 				Field:  fmt.Sprintf("pipeline.%s[%d].configPath", fnType, idx),
 				Value:  f.ConfigPath,
@@ -143,7 +143,7 @@ func validateFnConfigPathSyntax(p string) error {
 	return nil
 }
 
-func ValidateFnConfigPath(pkgPath types.UniquePath, configPath string) (*yaml.RNode, error) {
+func GetValidatedFnConfigFromPath(pkgPath types.UniquePath, configPath string) (*yaml.RNode, error) {
 	path := filepath.Join(string(pkgPath), configPath)
 	file, err := os.Open(path)
 	if err != nil {
@@ -155,7 +155,7 @@ func ValidateFnConfigPath(pkgPath types.UniquePath, configPath string) (*yaml.RN
 		return nil, fmt.Errorf("failed to read function config '%s': %w", configPath, err)
 	}
 	if len(nodes) > 1 {
-		return nil, fmt.Errorf("function config file '%s' must not contain more than 1 config", configPath)
+		return nil, fmt.Errorf("function config file '%s' must not contain more than 1 config, got %d", configPath, len(nodes))
 	}
 	if err := IsKrm(nodes[0]); err != nil {
 		return nil, err

pkg/api/kptfile/v1alpha2/validation_test.go

@@ -39,7 +39,6 @@ func TestKptfileValidate(t *testing.T) {
 					Mutators: []Function{
 						{
 							Image:      "patch-strategic-merge",
-							ConfigPath: "./patch.yaml",
 						},
 						{
 							Image: "gcr.io/kpt-fn/set-annotations:v0.1",
@@ -115,20 +114,6 @@ func TestKptfileValidate(t *testing.T) {
 			},
 			valid: false,
 		},
-		{
-			name: "pipeline: cleaned configpath does not contain ..",
-			kptfile: KptFile{
-				Pipeline: &Pipeline{
-					Mutators: []Function{
-						{
-							Image:      "image",
-							ConfigPath: "a/b/../config.yaml",
-						},
-					},
-				},
-			},
-			valid: true,
-		},
 		{
 			name: "pipeline: cleaned configpath contains ..",
 			kptfile: KptFile{
@@ -162,7 +147,7 @@ func TestKptfileValidate(t *testing.T) {
 	for _, c := range cases {
 		c := c
 		t.Run(c.name, func(t *testing.T) {
-			err := c.kptfile.Validate()
+			err := c.kptfile.Validate("")
 			if c.valid && err != nil {
 				t.Fatalf("kptfile should be valid, %s", err)
 			}
@@ -328,10 +313,14 @@ func TestValidatePath(t *testing.T) {
 			"\t \n",
 			false,
 		},
+		{
+			"a/b/../config.yaml",
+			true,
+		},
 	}
 
 	for _, c := range cases {
-		ret := validateFnConfigPath(c.Path)
+		ret := validateFnConfigPathSyntax(c.Path)
 		if (ret == nil) != c.Valid {
 			t.Fatalf("returned value for path %q should be %t, got %t",
 				c.Path, c.Valid, (ret == nil))

thirdparty/kyaml/runfn/runfn.go

@@ -389,7 +389,7 @@ func getUIDGID(asCurrentUser bool, currentUser currentUserFunc) (string, error)
 // getFunctionConfig returns yaml representation of functionConfig that can
 // be provided to a function as input.
 func (r *RunFns) getFunctionConfig() (*yaml.RNode, error) {
-	return v1alpha2.ValidateFnConfigPath("", r.FnConfigPath)
+	return v1alpha2.GetValidatedFnConfigFromPath("", r.FnConfigPath)
 }
 
 // defaultFnFilterProvider provides function filters