diff --git a/e2e/testdata/fn-eval/missing-fn-image/.expected/config.yaml b/e2e/testdata/fn-eval/missing-fn-image/.expected/config.yaml index bde0350440..6093efcd87 100644 --- a/e2e/testdata/fn-eval/missing-fn-image/.expected/config.yaml +++ b/e2e/testdata/fn-eval/missing-fn-image/.expected/config.yaml @@ -17,7 +17,7 @@ exitCode: 1 image: gcr.io/kpt-fn/dne # non-existing image args: namespace: staging -stdErr: "error: function image \"gcr.io/kpt-fn/dne\" doesn't exist" +stdErr: 'Function image "gcr.io/kpt-fn/dne" doesn''t exist' stdOut: | [RUNNING] "gcr.io/kpt-fn/dne" [FAIL] "gcr.io/kpt-fn/dne" diff --git a/e2e/testdata/fn-render/invalid-inline-fnconfig/.expected/config.yaml b/e2e/testdata/fn-render/invalid-inline-fnconfig/.expected/config.yaml index baaa6ab545..89dcbcda4b 100644 --- a/e2e/testdata/fn-render/invalid-inline-fnconfig/.expected/config.yaml +++ b/e2e/testdata/fn-render/invalid-inline-fnconfig/.expected/config.yaml @@ -13,4 +13,4 @@ # limitations under the License. exitCode: 1 -stdErr: 'Error: invalid pipeline: function "gcr.io/kpt-fn/set-labels:unstable": functionConfig must be a valid KRM resource with `apiVersion` and `kind` fields' +stdErr: "functionConfig must be a valid KRM resource with `apiVersion` and `kind` fields" diff --git a/e2e/testdata/fn-render/missing-fn-image/.expected/config.yaml b/e2e/testdata/fn-render/missing-fn-image/.expected/config.yaml index 20d2b71de0..fc5ed75bcd 100644 --- a/e2e/testdata/fn-render/missing-fn-image/.expected/config.yaml +++ b/e2e/testdata/fn-render/missing-fn-image/.expected/config.yaml @@ -13,7 +13,7 @@ # limitations under the License. exitCode: 1 -stdErr: "Error: function image \"gcr.io/kpt-fn/set-labels1\" doesn't exist" +stdErr: 'Error: Function image "gcr.io/kpt-fn/dne" doesn''t exist' stdOut: | [RUNNING] "gcr.io/kpt-fn/set-namespace:unstable" [PASS] "gcr.io/kpt-fn/set-namespace:unstable" diff --git a/e2e/testdata/fn-render/missing-fn-image/Kptfile b/e2e/testdata/fn-render/missing-fn-image/Kptfile index 7cc1617fed..2c18b4fddf 100644 --- a/e2e/testdata/fn-render/missing-fn-image/Kptfile +++ b/e2e/testdata/fn-render/missing-fn-image/Kptfile @@ -7,6 +7,6 @@ pipeline: - image: gcr.io/kpt-fn/set-namespace:unstable configMap: namespace: staging - - image: gcr.io/kpt-fn/set-labels1 # non-existing image + - image: gcr.io/kpt-fn/dne # non-existing image configMap: - tier: backend \ No newline at end of file + tier: backend diff --git a/e2e/testdata/fn-render/multiple-fnconfig/.expected/config.yaml b/e2e/testdata/fn-render/multiple-fnconfig/.expected/config.yaml index 5f3e6ce046..188ee54259 100644 --- a/e2e/testdata/fn-render/multiple-fnconfig/.expected/config.yaml +++ b/e2e/testdata/fn-render/multiple-fnconfig/.expected/config.yaml @@ -13,4 +13,4 @@ # limitations under the License. exitCode: 1 -stdErr: "following fields are mutually exclusive: 'config', 'configMap', 'configPath'. Got \"configPath, configMap\"" +stdErr: 'only one of ''config'', ''configMap'', ''configPath'' can be specified. Got "configPath, configMap"' diff --git a/internal/errors/resolver/container.go b/internal/errors/resolver/container.go new file mode 100644 index 0000000000..6f2dedeb77 --- /dev/null +++ b/internal/errors/resolver/container.go @@ -0,0 +1,41 @@ +// 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. + +package resolver + +import ( + "errors" + + "github.com/GoogleContainerTools/kpt/internal/fnruntime" +) + +//nolint:gochecknoinits +func init() { + AddErrorResolver(&containerImageErrorResolver{}) +} + +// containerImageErrorResolver is an implementation of the ErrorResolver interface +// to resolve container image errors. +type containerImageErrorResolver struct{} + +func (*containerImageErrorResolver) Resolve(err error) (ResolvedResult, bool) { + var containerImageError *fnruntime.ContainerImageError + if !errors.As(err, &containerImageError) { + return ResolvedResult{}, false + } + return ResolvedResult{ + Message: containerImageError.Error(), + ExitCode: 1, + }, true +} diff --git a/internal/errors/resolver/pkg.go b/internal/errors/resolver/pkg.go index d43fea8404..c05079be35 100644 --- a/internal/errors/resolver/pkg.go +++ b/internal/errors/resolver/pkg.go @@ -19,6 +19,7 @@ import ( "github.com/GoogleContainerTools/kpt/internal/errors" "github.com/GoogleContainerTools/kpt/internal/pkg" + kptfile "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1alpha2" ) //nolint:gochecknoinits @@ -44,6 +45,7 @@ type pkgErrorResolver struct{} func (*pkgErrorResolver) Resolve(err error) (ResolvedResult, bool) { var kptfileError *pkg.KptfileError + var validateError *kptfile.ValidateError if errors.As(err, &kptfileError) { path := kptfileError.Path tmplArgs := map[string]interface{}{ @@ -63,6 +65,12 @@ func (*pkgErrorResolver) Resolve(err error) (ResolvedResult, bool) { ExitCode: 1, }, true } + if errors.As(err, &validateError) { + return ResolvedResult{ + Message: validateError.Error(), + ExitCode: 1, + }, true + } return ResolvedResult{}, false } diff --git a/internal/fnruntime/container.go b/internal/fnruntime/container.go index 595e8ac3dd..7a7daeae36 100644 --- a/internal/fnruntime/container.go +++ b/internal/fnruntime/container.go @@ -163,7 +163,10 @@ func (f *ContainerFn) prepareImage() error { var output []byte var err error if output, err = cmd.CombinedOutput(); err != nil { - return fmt.Errorf("failed to check local function image %q: %w", f.Image, err) + return &ContainerImageError{ + Image: f.Image, + Output: string(output), + } } if strings.Contains(string(output), strings.Split(f.Image, ":")[0]) { // image exists locally @@ -178,8 +181,22 @@ func (f *ContainerFn) prepareImage() error { ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() cmd = exec.CommandContext(ctx, dockerBin, args...) - if _, err := cmd.CombinedOutput(); err != nil { - return fmt.Errorf("function image %q doesn't exist", f.Image) + if output, err := cmd.CombinedOutput(); err != nil { + return &ContainerImageError{ + Image: f.Image, + Output: string(output), + } } return nil } + +// ContainerImageError is an error type which will be returned when +// the container run time cannot verify docker image. +type ContainerImageError struct { + Image string + Output string +} + +func (e *ContainerImageError) Error() string { + return fmt.Sprintf("Function image %q doesn't exist.", e.Image) +} diff --git a/internal/util/addmergecomment/addmergecomment.go b/internal/util/addmergecomment/addmergecomment.go index 4b6aff88c4..03a43f1cee 100644 --- a/internal/util/addmergecomment/addmergecomment.go +++ b/internal/util/addmergecomment/addmergecomment.go @@ -1,3 +1,17 @@ +// 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. + package addmergecomment import ( diff --git a/internal/util/addmergecomment/addmergecomment_test.go b/internal/util/addmergecomment/addmergecomment_test.go index 94858297a8..14662e4104 100644 --- a/internal/util/addmergecomment/addmergecomment_test.go +++ b/internal/util/addmergecomment/addmergecomment_test.go @@ -1,3 +1,17 @@ +// 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. + package addmergecomment import ( diff --git a/pkg/api/kptfile/v1alpha2/validation.go b/pkg/api/kptfile/v1alpha2/validation.go index 579bc995da..703f9a1071 100644 --- a/pkg/api/kptfile/v1alpha2/validation.go +++ b/pkg/api/kptfile/v1alpha2/validation.go @@ -41,12 +41,16 @@ func (p *Pipeline) validate() error { if p == nil { return nil } - fns := []Function{} - fns = append(fns, p.Mutators...) - fns = append(fns, p.Validators...) - for i := range fns { - f := fns[i] - err := f.validate() + for i := range p.Mutators { + f := p.Mutators[i] + err := f.validate("mutators", i) + if err != nil { + return fmt.Errorf("function %q: %w", f.Image, err) + } + } + for i := range p.Validators { + f := p.Validators[i] + err := f.validate("validators", i) if err != nil { return fmt.Errorf("function %q: %w", f.Image, err) } @@ -54,16 +58,24 @@ func (p *Pipeline) validate() error { return nil } -func (f *Function) validate() error { +func (f *Function) validate(fnType string, idx int) error { err := validateFunctionName(f.Image) if err != nil { - return fmt.Errorf("'image' is invalid: %w", err) + return &ValidateError{ + Field: fmt.Sprintf("pipeline.%s[%d].image", fnType, idx), + Value: f.Image, + Reason: err.Error(), + } } var configFields []string if f.ConfigPath != "" { if err := validatePath(f.ConfigPath); err != nil { - return fmt.Errorf("'configPath' %q is invalid: %w", f.ConfigPath, err) + return &ValidateError{ + Field: fmt.Sprintf("pipeline.%s[%d].configPath", fnType, idx), + Value: f.ConfigPath, + Reason: err.Error(), + } } configFields = append(configFields, "configPath") } @@ -73,13 +85,19 @@ func (f *Function) validate() error { if !IsNodeZero(&f.Config) { config := yaml.NewRNode(&f.Config) if _, err := config.GetMeta(); err != nil { - return fmt.Errorf("functionConfig must be a valid KRM resource with `apiVersion` and `kind` fields") + return &ValidateError{ + Field: fmt.Sprintf("pipeline.%s[%d].config", fnType, idx), + Reason: "functionConfig must be a valid KRM resource with `apiVersion` and `kind` fields", + } } configFields = append(configFields, "config") } if len(configFields) > 1 { - return fmt.Errorf("following fields are mutually exclusive: 'config', 'configMap', 'configPath'. Got %q", - strings.Join(configFields, ", ")) + return &ValidateError{ + Field: fmt.Sprintf("pipeline.%s[%d]", fnType, idx), + Reason: fmt.Sprintf("only one of 'config', 'configMap', 'configPath' can be specified. Got %q", + strings.Join(configFields, ", ")), + } } return nil @@ -148,3 +166,23 @@ func validatePath(p string) error { } return nil } + +// ValidateError is the error returned when validation fails. +type ValidateError struct { + // Field is the field that causes error + Field string + // Value is the value of invalid field + Value string + // Reason is the reson for the error + Reason string +} + +func (e *ValidateError) Error() string { + var sb strings.Builder + sb.WriteString(fmt.Sprintf("Kptfile is invalid:\nField: `%s`\n", e.Field)) + if e.Value != "" { + sb.WriteString(fmt.Sprintf("Value: %q\n", e.Value)) + } + sb.WriteString(fmt.Sprintf("Reason: %s\n", e.Reason)) + return sb.String() +}