diff --git a/pkg/parser/parser.go b/pkg/parser/parser.go index b464ae82e..ff30bd80a 100644 --- a/pkg/parser/parser.go +++ b/pkg/parser/parser.go @@ -22,6 +22,7 @@ import ( "context" "io" "strings" + "unicode" "github.com/spf13/afero" corev1 "k8s.io/api/core/v1" @@ -107,8 +108,7 @@ func (p *PackageParser) Parse(_ context.Context, reader io.ReadCloser) (*Package if errors.Is(err, io.EOF) { break } - content = cleanYAML(content) - if len(content) == 0 { + if isEmptyYAML(content) { continue } m, _, err := dm.Decode(content, nil, nil) @@ -130,27 +130,19 @@ func (p *PackageParser) Parse(_ context.Context, reader io.ReadCloser) (*Package return pkg, nil } -// cleanYAML cleans up YAML by removing empty and commented out lines which -// cause issues with decoding. -func cleanYAML(y []byte) []byte { - lines := []string{} - empty := true +// isEmptyYAML checks whether the provided YAML can be considered empty. This +// is useful for filtering out empty YAML documents that would otherwise +// cause issues when decoded. +func isEmptyYAML(y []byte) bool { for _, line := range strings.Split(string(y), "\n") { - trimmed := strings.TrimSpace(line) - if trimmed == "" || strings.HasPrefix(trimmed, "#") { - continue - } + trimmed := strings.TrimLeftFunc(line, unicode.IsSpace) // We don't want to return an empty document with only separators that // have nothing in-between. - if empty && trimmed != "---" && trimmed != "..." { - empty = false + if trimmed != "" && trimmed != "---" && trimmed != "..." && !strings.HasPrefix(trimmed, "#") { + return false } - lines = append(lines, line) - } - if empty { - return nil } - return []byte(strings.Join(lines, "\n")) + return true } // annotateErr annotates an error if the reader is an AnnotatedReadCloser. diff --git a/pkg/parser/parser_test.go b/pkg/parser/parser_test.go index 6d19698d4..c5d56d99d 100644 --- a/pkg/parser/parser_test.go +++ b/pkg/parser/parser_test.go @@ -56,7 +56,12 @@ metadata: deployBytes = []byte(`apiVersion: apps/v1 kind: Deployment metadata: - name: test`) + name: test + annotations: + crossplane.io/managed: | + #!/bin/bash some script + some script +`) commentedOutBytes = []byte(`# apiVersion: apps/v1 # kind: Deployment @@ -210,3 +215,91 @@ func TestParser(t *testing.T) { }) } } + +func TestCleanYAML(t *testing.T) { + type args struct { + in []byte + } + type want struct { + out bool + } + cases := map[string]struct { + reason string + args args + want want + }{ + "Empty": { + reason: "Should return true on empty input", + args: args{in: []byte("")}, + want: want{out: true}, + }, + "EmptyLine": { + reason: "Should return true on an input with an empty line", + args: args{in: []byte("\n")}, + want: want{out: true}, + }, + "WhitespaceOnly": { + reason: "Should return true on an input with only whitespaces", + args: args{in: []byte(" \n\t ")}, + want: want{out: true}, + }, + "OnlyYAMLSeparators": { + reason: "Should return true on an input with only YAML separators", + args: args{in: []byte("---\n...")}, + want: want{out: true}, + }, + "YAMLWithWhitespaceLineAndNonEmptyLine": { + reason: "Should return false on having whitespace and non empty line in the input", + args: args{in: []byte(" \nkey: value")}, + want: want{out: false}, + }, + "CommentedOut": { + reason: "Should return true on a fully commented out input", + args: args{in: []byte(`# apiVersion: apps/v1 +# kind: Deployment +# metadata: +# name: test`)}, + want: want{out: true}, + }, + "CommentedOutExceptSeparator": { + reason: "Should return true on a fully commented out input with a separator not commented", + args: args{in: []byte(`--- +# apiVersion: apps/v1 +# kind: Deployment +# metadata: +# name: test`)}, + want: want{out: true}, + }, + "NotFullyCommentedOut": { + reason: "Should return false on a partially commented out input", + args: args{in: []byte(`--- +# some comment +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test`)}, + want: want{out: false}, + }, + "ShebangAnnotation": { + reason: "Should return false with just a shebang annotation", + args: args{in: []byte(`--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test + annotations: + someScriptWithAShebang: | + #!/bin/bash + some script`)}, + want: want{out: false}, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got := isEmptyYAML(tc.args.in) + if diff := cmp.Diff(tc.want.out, got); diff != "" { + t.Errorf("isEmptyYAML: -want, +got:\n%s", diff) + } + }) + } +}