Improve FunctionConfig related validation errors

e2e/testdata/fn-render/fnconfig-cannot-refer-subpkgs/.expected/config.yaml

@@ -17,4 +17,4 @@ stdErr: |
   Kptfile is invalid:
   Field: `pipeline.mutators[0].configPath`
   Value: "db/labelconfig.yaml"
-  Reason: functionConfig must exist in the current package
+  Reason: functionConfig resource for mutator "set-labels" is missing from path "db/labelconfig.yaml", this also occurs if "db/labelconfig.yaml" is part of a subpackage

e2e/testdata/fn-render/fnconfig-cannot-refer-subpkgs/.expected/diff.patch

@@ -1,5 +1,5 @@
 diff --git a/Kptfile b/Kptfile
-index 0bfdbb0..10e2d27 100644
+index 0bfdbb0..b4a6160 100644
 --- a/Kptfile
 +++ b/Kptfile
 @@ -6,3 +6,13 @@ pipeline:
@@ -15,4 +15,4 @@ index 0bfdbb0..10e2d27 100644
 +        pkg.render: pkg .: Kptfile is invalid:
 +        Field: `pipeline.mutators[0].configPath`
 +        Value: "db/labelconfig.yaml"
-+        Reason: functionConfig must exist in the current package
++        Reason: functionConfig resource for mutator "set-labels" is missing from path "db/labelconfig.yaml", this also occurs if "db/labelconfig.yaml" is part of a subpackage

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

@@ -17,4 +17,4 @@ stdErr: |
   Kptfile is invalid:
   Field: `pipeline.mutators[1].configPath`
   Value: "labelconfig1.yaml"
-  Reason: functionConfig must exist in the current package
+  Reason: failed to open functionConfig from path "labelconfig1.yaml": open missing-fnconfig/labelconfig1.yaml: no such file or directory

internal/pkg/pkg.go

@@ -17,6 +17,7 @@ package pkg
 
 import (
 	"bytes"
+	stderrors "errors"
 	"fmt"
 	"io"
 	"os"
@@ -31,6 +32,7 @@ import (
 	rgfilev1alpha1 "github.com/kptdev/kpt/pkg/api/resourcegroup/v1alpha1"
 	"github.com/kptdev/kpt/pkg/kptfile/kptfileutil"
 	"github.com/kptdev/kpt/pkg/lib/errors"
+	regclientref "github.com/regclient/regclient/types/ref"
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 	"sigs.k8s.io/kustomize/kyaml/kio"
 	"sigs.k8s.io/kustomize/kyaml/kio/kioutil"
@@ -430,33 +432,63 @@ func (p *Pkg) ValidatePipeline() error {
 
 	resourcesByPath := sets.String{}
 
+	// TODO: should we use go.uber.org/multierr instead?
+	var errs []error
 	for _, r := range resources {
 		rPath, _, err := kioutil.GetFileAnnotations(r)
 		if err != nil {
-			return fmt.Errorf("resource missing path annotation err: %w", err)
+			errs = append(errs, fmt.Errorf("resource %q missing path annotation err: %w", r.GetName(), err))
 		}
 		resourcesByPath.Insert(filepath.Clean(rPath))
 	}
 
+	if len(errs) > 0 {
+		return stderrors.Join(errs...)
+	}
+
 	for i, fn := range pl.Mutators {
 		if fn.ConfigPath != "" && !resourcesByPath.Has(filepath.Clean(fn.ConfigPath)) {
-			return &kptfilev1.ValidateError{
-				Field:  fmt.Sprintf("pipeline.%s[%d].configPath", "mutators", i),
-				Value:  fn.ConfigPath,
-				Reason: "functionConfig must exist in the current package",
-			}
+			errs = append(errs, makeStepValidationErr("mutator", i, fn.Name, fn.Image, fn.ConfigPath))
 		}
 	}
 	for i, fn := range pl.Validators {
 		if fn.ConfigPath != "" && !resourcesByPath.Has(filepath.Clean(fn.ConfigPath)) {
-			return &kptfilev1.ValidateError{
-				Field:  fmt.Sprintf("pipeline.%s[%d].configPath", "validators", i),
-				Value:  fn.ConfigPath,
-				Reason: "functionConfig must exist in the current package",
-			}
+			errs = append(errs, makeStepValidationErr("validator", i, fn.Name, fn.Image, fn.ConfigPath))
 		}
 	}
-	return nil
+	return stderrors.Join(errs...)
+}
+
+func makeStepValidationErr(stepType string, i int, fnName, fnImage, configPath string) *kptfilev1.ValidateError {
+	return &kptfilev1.ValidateError{
+		Field: fmt.Sprintf("pipeline.%s[%d].configPath", stepType+"s", i),
+		Value: configPath,
+		Reason: fmt.Sprintf("functionConfig resource for %s %q is missing from path %q, this also occurs if %q is part of a subpackage",
+			stepType, PipelineStepNameOrImage(fnName, fnImage), configPath, configPath),
+	}
+}
+
+func PipelineStepNameOrImage(name, image string) string {
+	if name != "" {
+		return name
+	}
+
+	return ImageBaseName(image)
+}
+
+// TODO: I feel like this must already exist somewhere
+func ImageBaseName(fullName string) string {
+	ref, err := regclientref.New(fullName)
+	if err != nil {
+		return fullName
+	}
+
+	i := strings.LastIndex(ref.Repository, "/")
+	if i == -1 {
+		return ref.Repository
+	}
+
+	return ref.Repository[i+1:]
 }
 
 // GetPkgPathAnnotation returns the package path annotation on

internal/pkg/pkg_test.go

@@ -535,3 +535,54 @@ func Chdir(t *testing.T, path string) func() {
 	}
 	return revertFunc
 }
+
+func TestPipelineStepNameOrImage(t *testing.T) {
+	const expected = "image"
+
+	testCases := map[string]struct {
+		name  string
+		image string
+	}{
+		"has name": {
+			name:  "image",
+			image: "different-image",
+		},
+		"no registry, no path, no tag": {
+			image: "image",
+		},
+		"no registry, no path, with tag": {
+			image: "image:v0.1.0",
+		},
+		"no registry, with path, no tag": {
+			image: "kptdev/krm-functions-catalog/image",
+		},
+		"no registry, with path, with tag": {
+			image: "kptdev/krm-functions-catalog/image:v0.1.0",
+		},
+		"with registry, no path, no tag": {
+			image: "my-registry.com/image",
+		},
+		"with registry, no path, with tag": {
+			image: "my-registry.com/image:v0.1.0",
+		},
+		"with registry, with path, no tag": {
+			image: "my-registry.com/kptdev/krm-functions-catalog/image",
+		},
+		"with registry, with path, with tag": {
+			image: "my-registry.com/kptdev/krm-functions-catalog/image:v0.1.0",
+		},
+		"with digest, no tag": {
+			image: "my-registry.com/kptdev/krm-functions-catalog/image@sha256:7d89a74f106241391f687fc2985c8e6de597bb21f0d0014def5edc730618d9cc",
+		},
+		"with digest, with tag": {
+			image: "my-registry.com/kptdev/krm-functions-catalog/image:v0.1.0@sha256:7d89a74f106241391f687fc2985c8e6de597bb21f0d0014def5edc730618d9cc",
+		},
+	}
+
+	for tn, tc := range testCases {
+		t.Run(tn, func(t *testing.T) {
+			actual := PipelineStepNameOrImage(tc.name, tc.image)
+			assert.Equal(t, expected, actual)
+		})
+	}
+}

pkg/api/kptfile/v1/validation.go

@@ -177,7 +177,7 @@ func GetValidatedFnConfigFromPath(fsys filesys.FileSystem, pkgPath types.UniqueP
 	path := filepath.Join(string(pkgPath), configPath)
 	file, err := fsys.Open(path)
 	if err != nil {
-		return nil, fmt.Errorf("functionConfig must exist in the current package")
+		return nil, fmt.Errorf("failed to open functionConfig from path %q: %w", configPath, err)
 	}
 	defer file.Close()
 	reader := kio.ByteReader{Reader: file, PreserveSeqIndent: true, WrapBareSeqNode: true, DisableUnwrapping: true}

pkg/test/runner/runner.go

@@ -503,6 +503,7 @@ func (r *Runner) compareResult(exitErr error, stdout string, inStderr string, tm
 func (r *Runner) compareOutput(stdout string, stderr string) error {
 	expectedStderr := r.testCase.Config.StdErr
 	conditionedStderr := removeArmPlatformWarning(stderr)
+	conditionedStderr = removeTmpDirPrefix(stderr)
 
 	if !strings.Contains(conditionedStderr, expectedStderr) {
 		r.t.Logf("stderr diff is %s", cmp.Diff(expectedStderr, conditionedStderr))
@@ -526,6 +527,12 @@ func (r *Runner) compareOutput(stdout string, stderr string) error {
 	return nil
 }
 
+// removeTmpDirPrefix so that error messages can be compared properly
+func removeTmpDirPrefix(stderr string) string {
+	re := regexp.MustCompile(`/tmp/kpt-pipeline-e2e-\d+/`)
+	return re.ReplaceAllString(stderr, "")
+}
+
 func (r *Runner) Skip() bool {
 	return r.testCase.Config.Skip
 }