diff --git a/kyaml/kio/byteio_reader.go b/kyaml/kio/byteio_reader.go index a9cc0471b..7513f0d64 100644 --- a/kyaml/kio/byteio_reader.go +++ b/kyaml/kio/byteio_reader.go @@ -11,7 +11,6 @@ import ( "sort" "strings" - "sigs.k8s.io/kustomize/kyaml/copyutil" "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/kio/kioutil" "sigs.k8s.io/kustomize/kyaml/yaml" @@ -284,7 +283,7 @@ func (r *ByteReader) decode(originalYAML string, index int, decoder *yaml.Decode if r.PreserveSeqIndent { // derive and add the seqindent annotation - seqIndentStyle := seqIndentAnno(node, originalYAML) + seqIndentStyle := yaml.DeriveSeqIndentStyle(originalYAML) if seqIndentStyle != "" { r.SetAnnotations[kioutil.SeqIndentAnnotation] = fmt.Sprintf("%s", seqIndentStyle) } @@ -303,40 +302,3 @@ func (r *ByteReader) decode(originalYAML string, index int, decoder *yaml.Decode } return yaml.NewRNode(node), nil } - -// seqIndentAnno derives the sequence indentation annotation value for the resource, -// originalYAML is the input yaml string and node is the decoded equivalent of originalYAML, -// the annotation value is decided by deriving the existing sequence indentation of resource -func seqIndentAnno(node *yaml.Node, originalYAML string) string { - rNode := &yaml.RNode{} - rNode.SetYNode(node) - // step 1: derive the sequence indentation of the node - anno := rNode.GetAnnotations() - if anno[kioutil.SeqIndentAnnotation] != "" { - // the annotation already exists, so don't change it - return "" - } - - // marshal the node with 2 space sequence indentation and calculate the diff - out, err := yaml.MarshalWithOptions(rNode.Document(), &yaml.EncoderOptions{SeqIndent: yaml.WideSeqIndent}) - if err != nil { - return "" - } - twoSpaceIndentDiff := copyutil.PrettyFileDiff(string(out), originalYAML) - - // marshal the node with 0 space sequence indentation and calculate the diff - out, err = yaml.MarshalWithOptions(rNode.Document(), &yaml.EncoderOptions{SeqIndent: yaml.CompactSeqIndent}) - if err != nil { - return "" - } - noIndentDiff := copyutil.PrettyFileDiff(string(out), originalYAML) - - var style string - if len(noIndentDiff) <= len(twoSpaceIndentDiff) { - style = string(yaml.CompactSeqIndent) - } else { - style = string(yaml.WideSeqIndent) - } - - return style -} diff --git a/kyaml/kio/byteio_readwriter_test.go b/kyaml/kio/byteio_readwriter_test.go index 040b8d81d..4488cce6e 100644 --- a/kyaml/kio/byteio_readwriter_test.go +++ b/kyaml/kio/byteio_readwriter_test.go @@ -528,6 +528,31 @@ kind: Service spec: - foo - bar +`, + }, + { + name: "round_trip with mixed indentations in same resource, wide wins as it is first", + input: ` +apiVersion: apps/v1 +kind: Deployment +spec: + - foo + - bar +env: +- foo +- bar +- baz +`, + expectedOutput: ` +apiVersion: apps/v1 +kind: Deployment +spec: + - foo + - bar +env: + - foo + - bar + - baz `, }, } diff --git a/kyaml/yaml/util.go b/kyaml/yaml/util.go new file mode 100644 index 000000000..df6c79d33 --- /dev/null +++ b/kyaml/yaml/util.go @@ -0,0 +1,48 @@ +package yaml + +import ( + "strings" +) + +// DeriveSeqIndentStyle derives the sequence indentation annotation value for the resource, +// originalYAML is the input yaml string, +// the style is decided by deriving the existing sequence indentation of first sequence node +func DeriveSeqIndentStyle(originalYAML string) string { + lines := strings.Split(originalYAML, "\n") + for i, line := range lines { + elems := strings.SplitN(line, "- ", 2) + if len(elems) != 2 { + continue + } + // prefix of "- " must be sequence of spaces + if strings.Trim(elems[0], " ") != "" { + continue + } + numSpacesBeforeSeqElem := len(elems[0]) + + if i == 0 { + continue + } + + // keyLine is the line before the first sequence element + keyLine := lines[i-1] + + numSpacesBeforeKeyElem := len(keyLine) - len(strings.TrimLeft(keyLine, " ")) + trimmedKeyLine := strings.Trim(keyLine, " ") + if strings.HasSuffix(trimmedKeyLine, "|") || strings.HasSuffix(trimmedKeyLine, "|-") { + // this is not a sequence node, it is a wrapped sequence node string + // ignore it + continue + } + + if numSpacesBeforeSeqElem == numSpacesBeforeKeyElem { + return string(CompactSeqIndent) + } + + if numSpacesBeforeSeqElem-numSpacesBeforeKeyElem == 2 { + return string(WideSeqIndent) + } + } + + return string(CompactSeqIndent) +} diff --git a/kyaml/yaml/util_test.go b/kyaml/yaml/util_test.go new file mode 100644 index 000000000..a83725c28 --- /dev/null +++ b/kyaml/yaml/util_test.go @@ -0,0 +1,143 @@ +package yaml + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDeriveSeqIndentStyle(t *testing.T) { + type testCase struct { + name string + input string + expectedOutput string + } + + testCases := []testCase{ + { + name: "detect simple wide indent", + input: `apiVersion: apps/v1 +kind: Deployment +spec: + - foo + - bar + - baz +`, + expectedOutput: `wide`, + }, + { + name: "detect simple compact indent", + input: `apiVersion: apps/v1 +kind: Deployment +spec: +- foo +- bar +- baz +`, + expectedOutput: `compact`, + }, + { + name: "read with mixed indentation, wide first", + input: `apiVersion: apps/v1 +kind: Deployment +spec: + - foo + - bar + - baz +env: +- foo +- bar +`, + expectedOutput: `wide`, + }, + { + name: "read with mixed indentation, compact first", + input: `apiVersion: apps/v1 +kind: Deployment +spec: +- foo +- bar +- baz +env: + - foo + - bar +`, + expectedOutput: `compact`, + }, + { + name: "read with mixed indentation, compact first with less elements", + input: `apiVersion: apps/v1 +kind: Deployment +spec: +- foo +- bar +env: + - foo + - bar + - baz +`, + expectedOutput: `compact`, + }, + { + name: "skip wrapped sequence strings", + input: `apiVersion: apps/v1 +kind: Deployment +spec: |- + - foo + - bar +env: +- foo +- bar +- baz +`, + expectedOutput: `compact`, + }, + { + name: "nested wide vs compact", + input: `apiVersion: apps/v1 +kind: Deployment +spec: + foo: + bar: + baz: + bor: + - a + - b +abc: +- a +- b +`, + expectedOutput: `wide`, + }, + { + name: "invalid resource but valid yaml sequence", + input: ` - foo`, + expectedOutput: `compact`, + }, + { + name: "- within sequence element", + input: `apiVersion: apps/v1 +kind: Deployment +spec: + foo: + - - a`, + expectedOutput: `wide`, + }, + { + name: "- within non sequence element", + input: `apiVersion: apps/v1 +kind: Deployment +spec: + foo: + a: - b`, + expectedOutput: `compact`, + }, + } + + for i := range testCases { + tc := testCases[i] + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expectedOutput, DeriveSeqIndentStyle(tc.input)) + }) + } +}