Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 3812; Error message changed and check for MalformedYamlError #4497

Merged
merged 5 commits into from Mar 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 20 additions & 0 deletions api/internal/kusterr/yamlformaterror.go
Expand Up @@ -19,6 +19,16 @@ func (e YamlFormatError) Error() string {
return fmt.Sprintf("YAML file [%s] encounters a format error.\n%s\n", e.Path, e.ErrorMsg)
}

// MalformedYamlError represents an error that occurred while trying to decode a given YAML.
type MalformedYamlError struct {
Path string
ErrorMsg string
}

func (e MalformedYamlError) Error() string {
return fmt.Sprintf("%s in File: %s", e.ErrorMsg, e.Path)
}

// Handler handles YamlFormatError
func Handler(e error, path string) error {
if isYAMLSyntaxError(e) {
Expand All @@ -27,9 +37,19 @@ func Handler(e error, path string) error {
ErrorMsg: e.Error(),
}
}
if IsMalformedYAMLError(e) {
return MalformedYamlError{
Path: path,
ErrorMsg: e.Error(),
}
}
return e
}

func isYAMLSyntaxError(e error) bool {
return strings.Contains(e.Error(), "error converting YAML to JSON") || strings.Contains(e.Error(), "error unmarshaling JSON")
}

func IsMalformedYAMLError(e error) bool {
return strings.Contains(e.Error(), "MalformedYAMLError")
}
4 changes: 4 additions & 0 deletions api/internal/target/kusttarget.go
Expand Up @@ -14,6 +14,7 @@ import (
"sigs.k8s.io/kustomize/api/ifc"
"sigs.k8s.io/kustomize/api/internal/accumulator"
"sigs.k8s.io/kustomize/api/internal/builtins"
"sigs.k8s.io/kustomize/api/internal/kusterr"
"sigs.k8s.io/kustomize/api/internal/plugins/builtinconfig"
"sigs.k8s.io/kustomize/api/internal/plugins/builtinhelpers"
"sigs.k8s.io/kustomize/api/internal/plugins/loader"
Expand Down Expand Up @@ -405,6 +406,9 @@ func (kt *KustTarget) accumulateResources(
if errors.Is(errF, load.ErrorHTTP) {
return nil, errF
}
if kusterr.IsMalformedYAMLError(errF) { // Some error occurred while tyring to decode YAML file
return nil, errF
}
ldr, err := kt.ldr.New(path)
if err != nil {
return nil, errors.Wrapf(
Expand Down
33 changes: 33 additions & 0 deletions api/krusty/basic_io_test.go
Expand Up @@ -6,6 +6,7 @@ package krusty_test
import (
"testing"

"sigs.k8s.io/kustomize/api/internal/kusterr"
kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
)

Expand Down Expand Up @@ -80,3 +81,35 @@ spec:
clusterIP: None
`)
}

//test for https://github.com/kubernetes-sigs/kustomize/issues/3812#issuecomment-862339267
func TestBasicIO3812(t *testing.T) {
th := kusttest_test.MakeHarness(t)
th.WriteK(".", `
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- service.yaml
`)

th.WriteF("service.yaml", `
apiVersion: v1
kind: Service
metadata:
name: kapacitor
labels:
app.kubernetes.io/name: tick-kapacitor
spec:
selector:
app.kubernetes.io/name: tick-kapacitor
- name: http
port: 9092
protocol: TCP
type: ClusterIP
`)

err := th.RunWithErr(".", th.MakeDefaultOptions())
if !kusterr.IsMalformedYAMLError(err) {
t.Fatalf("unexpected error: %q", err)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add a check for the error contents, e.g. assert.Equal(t, err.Error(), expectedErr)

}
2 changes: 1 addition & 1 deletion api/krusty/component_test.go
Expand Up @@ -551,7 +551,7 @@ components:
`),
},
runPath: "filesincomponents",
expectedError: "'/filesincomponents/stub.yaml' must be a directory to be a root",
expectedError: "'/filesincomponents/stub.yaml' must be a directory so that it can used as a build root",
},
"invalid-component-api-version": {
input: []FileGen{writeTestBase, writeOverlayProd,
Expand Down
4 changes: 2 additions & 2 deletions api/loader/fileloader.go
Expand Up @@ -159,8 +159,8 @@ func demandDirectoryRoot(
}
if f != "" {
return "", fmt.Errorf(
"got file '%s', but '%s' must be a directory to be a root",
f, path)
"'%s' must be a directory so that it can used as a build root",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so much better, thank you!

path)
}
return d, nil
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/config/internal/commands/fmt_test.go
Expand Up @@ -164,7 +164,7 @@ func TestCmd_failFileContents(t *testing.T) {
err := r.Command.Execute()

// expect an error
assert.EqualError(t, err, "yaml: line 1: did not find expected node content")
assert.EqualError(t, err, "MalformedYAMLError: yaml: line 1: did not find expected node content")
}

func TestFmtSubPackages(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion kyaml/kio/byteio_reader.go
Expand Up @@ -294,7 +294,7 @@ func (r *ByteReader) decode(originalYAML string, index int, decoder *yaml.Decode
return nil, io.EOF
}
if err != nil {
return nil, errors.Wrap(err)
return nil, errors.WrapPrefixf(err, "MalformedYAMLError")
}

if yaml.IsYNodeEmptyDoc(node) {
Expand Down
2 changes: 1 addition & 1 deletion kyaml/kio/filters/fmtr_test.go
Expand Up @@ -673,7 +673,7 @@ apiVersion: example.com/v1beta1
`

_, err := FormatInput(strings.NewReader(y))
assert.EqualError(t, err, "yaml: line 15: found character that cannot start any token")
assert.EqualError(t, err, "MalformedYAMLError: yaml: line 15: found character that cannot start any token")
}

// TestFormatFileOrDirectory_yamlExtFile verifies that FormatFileOrDirectory will format a file
Expand Down