Add options, keep seqindent annotation equivalent to index annotation

kubernetes-sigs · Jul 8, 2021 · f81201b · f81201b
1 parent 6dbc74b
commit f81201b
Show file tree

Hide file tree

Showing 8 changed files with 248 additions and 129 deletions.
diff --git a/kyaml/kio/byteio_reader.go b/kyaml/kio/byteio_reader.go
@@ -199,17 +199,11 @@ func (r *ByteReader) Read() ([]*yaml.RNode, error) {
 			values[i] += "\n"
 		}
 		decoder := yaml.NewDecoder(bytes.NewBufferString(values[i]))
-		node, err := r.decode(index, decoder)
+		node, err := r.decode(values[i], index, decoder)
 		if err == io.EOF {
 			continue
 		}
 
-		if r.AddSeqIndentAnnotation {
-			if err := addSeqIndentAnno(values[i], node); err != nil {
-				return nil, errors.Wrap(err)
-			}
-		}
-
 		if err != nil {
 			return nil, errors.Wrap(err)
 		}
@@ -260,45 +254,7 @@ func (r *ByteReader) Read() ([]*yaml.RNode, error) {
 	return output, nil
 }
 
-// addSeqIndentAnno adds the sequence indentation annotation to the resource
-// value is the input yaml string and node is the decoded equivalent of value
-// the annotation value is decided by deriving the existing sequence indentation of resource
-func addSeqIndentAnno(value string, node *yaml.RNode) error {
-	// step 1: derive the sequence indentation of the node
-	anno := node.GetAnnotations()
-	if anno[kioutil.SeqIndentAnnotation] != "" {
-		// the annotation already exists, so don't change it
-		return nil
-	}
-
-	// marshal the node with 2 space sequence indentation and calculate the diff
-	out, err := yaml.MarshalWithIndent(node.Document(), yaml.DefaultMapIndent, yaml.WideSeqIndent)
-	if err != nil {
-		return err
-	}
-	twoSpaceIndentDiff := copyutil.PrettyFileDiff(string(out), value)
-
-	// marshal the node with 0 space sequence indentation and calculate the diff
-	out, err = yaml.MarshalWithIndent(node.Document(), yaml.DefaultMapIndent, yaml.CompactSeqIndent)
-	if err != nil {
-		return err
-	}
-	noIndentDiff := copyutil.PrettyFileDiff(string(out), value)
-
-	var style string
-	if len(noIndentDiff) <= len(twoSpaceIndentDiff) {
-		style = string(yaml.CompactSeqIndent)
-	} else {
-		style = string(yaml.WideSeqIndent)
-	}
-
-	// step 2: add the annotation to the node
-	return node.PipeE(
-		yaml.LookupCreate(yaml.MappingNode, yaml.MetadataField, yaml.AnnotationsField),
-		yaml.SetField(kioutil.SeqIndentAnnotation, yaml.NewScalarRNode(style)))
-}
-
-func (r *ByteReader) decode(index int, decoder *yaml.Decoder) (*yaml.RNode, error) {
+func (r *ByteReader) decode(originalYAML string, index int, decoder *yaml.Decoder) (*yaml.RNode, error) {
 	node := &yaml.Node{}
 	err := decoder.Decode(node)
 	if err == io.EOF {
@@ -321,6 +277,14 @@ func (r *ByteReader) decode(index int, decoder *yaml.Decoder) (*yaml.RNode, erro
 	}
 	if !r.OmitReaderAnnotations {
 		r.SetAnnotations[kioutil.IndexAnnotation] = fmt.Sprintf("%d", index)
+
+		if r.AddSeqIndentAnnotation {
+			// derive and add the seqindent annotation
+			seqIndentStyle := seqIndentAnno(node, originalYAML)
+			if seqIndentStyle != "" {
+				r.SetAnnotations[kioutil.SeqIndentAnnotation] = fmt.Sprintf("%s", seqIndentStyle)
+			}
+		}
 	}
 	var keys []string
 	for k := range r.SetAnnotations {
@@ -335,3 +299,46 @@ func (r *ByteReader) decode(index int, decoder *yaml.Decoder) (*yaml.RNode, erro
 	}
 	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{
+		MapIndent: yaml.DefaultMapIndent,
+		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{
+		MapIndent: yaml.DefaultMapIndent,
+		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_reader_test.go b/kyaml/kio/byteio_reader_test.go
@@ -832,7 +832,8 @@ spec:
 - baz
 metadata:
   annotations:
-    internal.config.kubernetes.io/seqindent: wide
+    config.kubernetes.io/index: '0'
+    internal.config.kubernetes.io/seqindent: 'wide'
 `,
 		},
 		{
@@ -852,7 +853,8 @@ spec:
 - baz
 metadata:
   annotations:
-    internal.config.kubernetes.io/seqindent: compact
+    config.kubernetes.io/index: '0'
+    internal.config.kubernetes.io/seqindent: 'compact'
 `,
 		},
 		{
@@ -878,7 +880,8 @@ env:
 - bar
 metadata:
   annotations:
-    internal.config.kubernetes.io/seqindent: wide
+    config.kubernetes.io/index: '0'
+    internal.config.kubernetes.io/seqindent: 'wide'
 `,
 		},
 		{
@@ -904,7 +907,8 @@ env:
 - bar
 metadata:
   annotations:
-    internal.config.kubernetes.io/seqindent: compact
+    config.kubernetes.io/index: '0'
+    internal.config.kubernetes.io/seqindent: 'compact'
 `,
 		},
 	}
@@ -913,7 +917,7 @@ metadata:
 		tc := testCases[i]
 		t.Run(tc.name, func(t *testing.T) {
 			rNodes, err := (&ByteReader{
-				OmitReaderAnnotations:  true,
+				OmitReaderAnnotations:  false,
 				AddSeqIndentAnnotation: true,
 				Reader:                 bytes.NewBuffer([]byte(tc.input)),
 			}).Read()

diff --git a/kyaml/kio/byteio_readwriter_test.go b/kyaml/kio/byteio_readwriter_test.go
@@ -289,47 +289,6 @@ functionConfig:
 `,
 			instance: kio.ByteReadWriter{FunctionConfig: yaml.MustParse(`c: d`)},
 		},
-		{
-			name: "ResourceList indentation doesn't matter",
-			input: `
-apiVersion: config.kubernetes.io/v1alpha1
-kind: ResourceList
-items:
-  - kind: Deployment
-    metadata:
-      annotations:
-        internal.config.kubernetes.io/seqindent: "compact"
-    spec:
-      - foo
-      - bar
-  - kind: Service
-    metadata:
-      annotations:
-        internal.config.kubernetes.io/seqindent: "wide"
-    spec:
-      - foo
-      - bar
-`,
-			expectedOutput: `
-apiVersion: config.kubernetes.io/v1alpha1
-kind: ResourceList
-items:
-- kind: Deployment
-  metadata:
-    annotations:
-      internal.config.kubernetes.io/seqindent: "compact"
-  spec:
-  - foo
-  - bar
-- kind: Service
-  metadata:
-    annotations:
-      internal.config.kubernetes.io/seqindent: "wide"
-  spec:
-  - foo
-  - bar
-`,
-		},
 	}
 
 	for i := range testCases {

diff --git a/kyaml/kio/byteio_writer.go b/kyaml/kio/byteio_writer.go
@@ -62,13 +62,24 @@ func (w ByteWriter) Write(inputNodes []*yaml.RNode) error {
 	// Even though we use the this value further down we must check this before removing annotations
 	jsonEncodeSingleBareNode := w.shouldJSONEncodeSingleBareNode(nodes)
 
+	// store seqindent annotation value for each node in order to set the encoder indentation
+	var seqIndentsForNodes []string
+	for i := range nodes {
+		seqIndentsForNodes = append(seqIndentsForNodes, nodes[i].GetAnnotations()[kioutil.SeqIndentAnnotation])
+	}
+
 	for i := range nodes {
 		// clean resources by removing annotations set by the Reader
 		if !w.KeepReaderAnnotations {
 			_, err := nodes[i].Pipe(yaml.ClearAnnotation(kioutil.IndexAnnotation))
 			if err != nil {
 				return errors.Wrap(err)
 			}
+
+			_, err = nodes[i].Pipe(yaml.ClearAnnotation(kioutil.SeqIndentAnnotation))
+			if err != nil {
+				return errors.Wrap(err)
+			}
 		}
 		for _, a := range w.ClearAnnotations {
 			_, err := nodes[i].Pipe(yaml.ClearAnnotation(a))
@@ -89,9 +100,6 @@ func (w ByteWriter) Write(inputNodes []*yaml.RNode) error {
 	if jsonEncodeSingleBareNode {
 		encoder := json.NewEncoder(w.Writer)
 		encoder.SetIndent("", "  ")
-		if err := nodes[0].DeleteAnnotation(kioutil.SeqIndentAnnotation); err != nil {
-			return errors.Wrap(err)
-		}
 		return errors.Wrap(encoder.Encode(nodes[0]))
 	}
 
@@ -100,8 +108,13 @@ func (w ByteWriter) Write(inputNodes []*yaml.RNode) error {
 	// don't wrap the elements
 	if w.WrappingKind == "" {
 		for i := range nodes {
-			if err := unwrapAndEncodeYAML(nodes[i], encoder); err != nil {
-				return err
+			if seqIndentsForNodes[i] == string(yaml.WideSeqIndent) {
+				encoder.DefaultSeqIndent()
+			} else {
+				encoder.CompactSeqIndent()
+			}
+			if err := encoder.Encode(nodes[i].Document()); err != nil {
+				return errors.Wrap(err)
 			}
 		}
 		return nil
@@ -137,23 +150,6 @@ func (w ByteWriter) Write(inputNodes []*yaml.RNode) error {
 	return encoder.Encode(doc)
 }
 
-// unwrapAndEncodeYAML encodes the yaml RNode in unwrapped format,
-// as a pre-step, it clears the sets the sequence indentation for encoder,
-// based on the kioutil.SeqIndentAnnotation and clears it before encoding.
-func unwrapAndEncodeYAML(node *yaml.RNode, encoder *yaml.Encoder) error {
-	anno := node.GetAnnotations()
-	seqIndent := anno[kioutil.SeqIndentAnnotation]
-	if seqIndent == string(yaml.WideSeqIndent) {
-		encoder.DefaultSeqIndent()
-	} else {
-		encoder.CompactSeqIndent()
-	}
-	if err := node.DeleteAnnotation(kioutil.SeqIndentAnnotation); err != nil {
-		return errors.Wrap(err)
-	}
-	return encoder.Encode(node.Document())
-}
-
 func copyRNodes(in []*yaml.RNode) []*yaml.RNode {
 	out := make([]*yaml.RNode, len(in))
 	for i := range in {