diff --git a/e2e/testdata/fn-render/fnconfig-cannot-refer-subpkgs/.expected/config.yaml b/e2e/testdata/fn-render/fnconfig-cannot-refer-subpkgs/.expected/config.yaml index d10d9e6a80..7f80366ed6 100644 --- a/e2e/testdata/fn-render/fnconfig-cannot-refer-subpkgs/.expected/config.yaml +++ b/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 \ No newline at end of file + 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 \ No newline at end of file diff --git a/e2e/testdata/fn-render/fnconfig-cannot-refer-subpkgs/.expected/diff.patch b/e2e/testdata/fn-render/fnconfig-cannot-refer-subpkgs/.expected/diff.patch index e367388b3d..a5e0287950 100644 --- a/e2e/testdata/fn-render/fnconfig-cannot-refer-subpkgs/.expected/diff.patch +++ b/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 diff --git a/e2e/testdata/fn-render/missing-fnconfig/.expected/config.yaml b/e2e/testdata/fn-render/missing-fnconfig/.expected/config.yaml index b0147d9a2f..fb7ccf8d89 100644 --- a/e2e/testdata/fn-render/missing-fnconfig/.expected/config.yaml +++ b/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 diff --git a/internal/pkg/pkg.go b/internal/pkg/pkg.go index ff10b27706..c917ffe9f7 100644 --- a/internal/pkg/pkg.go +++ b/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,66 @@ 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), + } +} + +// PipelineStepNameOrImage returns the name if it's not empty or extracts the base name of the image +func PipelineStepNameOrImage(name, image string) string { + if name != "" { + return name + } + + return ImageBaseName(image) +} + +// ImageBaseName extracts the "base name" from a full/partial image repository name +// +// TODO: This probably 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 diff --git a/internal/pkg/pkg_test.go b/internal/pkg/pkg_test.go index c70f0f4f51..531f1517d3 100644 --- a/internal/pkg/pkg_test.go +++ b/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) + }) + } +} diff --git a/pkg/api/kptfile/v1/validation.go b/pkg/api/kptfile/v1/validation.go index be2778b2bd..d01b71a8a5 100644 --- a/pkg/api/kptfile/v1/validation.go +++ b/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} diff --git a/pkg/test/runner/runner.go b/pkg/test/runner/runner.go index 4ad94b1ba9..2163c41a8e 100644 --- a/pkg/test/runner/runner.go +++ b/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(conditionedStderr) 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(`/?\w+/kpt-pipeline-e2e-\d+/`) + return re.ReplaceAllString(stderr, "") +} + func (r *Runner) Skip() bool { return r.testCase.Config.Skip }