First sequence indent wins

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
-}

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
 `,
 		},
 	}

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)
+}

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))
+		})
+	}
+}