From f81201b74dc925572ca7066ea7f8f5b8575822b9 Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Thu, 8 Jul 2021 14:37:24 -0700 Subject: [PATCH] Add options, keep seqindent annotation equivalent to index annotation --- kyaml/kio/byteio_reader.go | 99 +++++++++++++++-------------- kyaml/kio/byteio_reader_test.go | 14 ++-- kyaml/kio/byteio_readwriter_test.go | 41 ------------ kyaml/kio/byteio_writer.go | 40 ++++++------ kyaml/kio/byteio_writer_test.go | 89 ++++++++++++++++++++++++++ kyaml/kio/pkgio_reader_test.go | 63 ++++++++++++++++++ kyaml/yaml/alias.go | 23 +++++-- kyaml/yaml/rnode.go | 8 --- 8 files changed, 248 insertions(+), 129 deletions(-) diff --git a/kyaml/kio/byteio_reader.go b/kyaml/kio/byteio_reader.go index 43250e0b13..620eea1bc9 100644 --- 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 index 7b74e009a9..e0ae8c0c2a 100644 --- 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 index bb1759c3c6..e7657e1bcc 100644 --- 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 index 4f87f794d5..628ce438a1 100644 --- a/kyaml/kio/byteio_writer.go +++ b/kyaml/kio/byteio_writer.go @@ -62,6 +62,12 @@ 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 { @@ -69,6 +75,11 @@ func (w ByteWriter) Write(inputNodes []*yaml.RNode) error { 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 { diff --git a/kyaml/kio/byteio_writer_test.go b/kyaml/kio/byteio_writer_test.go index 76d93d265c..241a9c31d9 100644 --- a/kyaml/kio/byteio_writer_test.go +++ b/kyaml/kio/byteio_writer_test.go @@ -311,6 +311,67 @@ metadata: `, }, + // + // Test Case + // + { + name: "keep_annotation_seqindent", + instance: ByteWriter{KeepReaderAnnotations: true}, + items: []string{ + `a: b #first +metadata: + annotations: + config.kubernetes.io/index: 0 + config.kubernetes.io/path: "a/b/a_test.yaml" + internal.config.kubernetes.io/index: "compact" +`, + `e: f +g: + h: + - i # has a list + - j +metadata: + annotations: + config.kubernetes.io/index: 0 + config.kubernetes.io/path: "a/b/b_test.yaml" + internal.config.kubernetes.io/seqindent: "wide" +`, + `c: d # second +metadata: + annotations: + config.kubernetes.io/index: 1 + config.kubernetes.io/path: "a/b/a_test.yaml" + internal.config.kubernetes.io/seqindent: "compact" +`, + }, + + expectedOutput: `a: b #first +metadata: + annotations: + config.kubernetes.io/index: 0 + config.kubernetes.io/path: "a/b/a_test.yaml" + internal.config.kubernetes.io/index: "compact" +--- +e: f +g: + h: + - i # has a list + - j +metadata: + annotations: + config.kubernetes.io/index: 0 + config.kubernetes.io/path: "a/b/b_test.yaml" + internal.config.kubernetes.io/seqindent: "wide" +--- +c: d # second +metadata: + annotations: + config.kubernetes.io/index: 1 + config.kubernetes.io/path: "a/b/a_test.yaml" + internal.config.kubernetes.io/seqindent: "compact" +`, + }, + // // Test Case // @@ -337,6 +398,34 @@ metadata: }`, }, + // + // Test Case + // + { + name: "encode_valid_json_remove_seqindent_annotation", + items: []string{ + `{ + "a": "a long string that would certainly see a newline introduced by the YAML marshaller abcd123", + metadata: { + annotations: { + "internal.config.kubernetes.io/seqindent": "compact", + "config.kubernetes.io/index": "0", + "config.kubernetes.io/path": "test.json" + } + } +}`, + }, + + expectedOutput: `{ + "a": "a long string that would certainly see a newline introduced by the YAML marshaller abcd123", + "metadata": { + "annotations": { + "config.kubernetes.io/path": "test.json" + } + } +}`, + }, + // // Test Case // diff --git a/kyaml/kio/pkgio_reader_test.go b/kyaml/kio/pkgio_reader_test.go index 1da5113bac..0ac6c434bc 100644 --- a/kyaml/kio/pkgio_reader_test.go +++ b/kyaml/kio/pkgio_reader_test.go @@ -337,6 +337,69 @@ g: } } +func TestLocalPackageReader_Read_addSeqIndentAnnotation(t *testing.T) { + s := SetupDirectories(t, filepath.Join("a", "b"), filepath.Join("a", "c")) + defer s.Clean() + s.WriteFile(t, filepath.Join("a_test.yaml"), readFileA) + s.WriteFile(t, filepath.Join("b_test.yaml"), readFileB) + + paths := []struct { + path string + }{ + {path: "./"}, + {path: s.Root}, + } + for _, p := range paths { + // empty path + rfr := LocalPackageReader{PackagePath: p.path, AddSeqIndentAnnotation: true} + nodes, err := rfr.Read() + if !assert.NoError(t, err) { + return + } + + if !assert.Len(t, nodes, 3) { + return + } + expected := []string{ + `a: b #first +metadata: + annotations: + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'a_test.yaml' + internal.config.kubernetes.io/seqindent: 'compact' +`, + `c: d # second +metadata: + annotations: + config.kubernetes.io/index: '1' + config.kubernetes.io/path: 'a_test.yaml' + internal.config.kubernetes.io/seqindent: 'compact' +`, + `# second thing +e: f +g: + h: + - i # has a list + - j +metadata: + annotations: + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'b_test.yaml' + internal.config.kubernetes.io/seqindent: 'compact' +`, + } + for i := range nodes { + val, err := nodes[i].String() + if !assert.NoError(t, err) { + return + } + if !assert.Equal(t, expected[i], val) { + return + } + } + } +} + func TestLocalPackageReader_Read_nestedDirs(t *testing.T) { s := SetupDirectories(t, filepath.Join("a", "b"), filepath.Join("a", "c")) defer s.Clean() diff --git a/kyaml/yaml/alias.go b/kyaml/yaml/alias.go index 41d5363211..6fb4f219a2 100644 --- a/kyaml/yaml/alias.go +++ b/kyaml/yaml/alias.go @@ -19,6 +19,15 @@ const ( // SeqIndentType holds the indentation style for sequence nodes type SeqIndentType string +// EncoderOptions are options that can be used to configure the encoder +type EncoderOptions struct { + // MapIndent is the indentation for YAML Mapping nodes + MapIndent int + + // SeqIndent is the indentation style for YAML Sequence nodes + SeqIndent SeqIndentType +} + // Expose the yaml.v3 functions so this package can be used as a replacement type Decoder = yaml.Decoder @@ -48,21 +57,21 @@ var NewEncoder = func(w io.Writer) *yaml.Encoder { return e } -// MarshalWithIndent marshals the input interface with provided indents -func MarshalWithIndent(in interface{}, mapIndent int, seqIndent SeqIndentType) ([]byte, error) { +// MarshalWithOptions marshals the input interface with provided options +func MarshalWithOptions(in interface{}, opts *EncoderOptions) ([]byte, error) { var buf bytes.Buffer - err := NewEncoderWithIndent(&buf, mapIndent, seqIndent).Encode(in) + err := NewEncoderWithOptions(&buf, opts).Encode(in) if err != nil { return nil, err } return buf.Bytes(), nil } -// NewEncoderWithIndent returns the encoder with configurable indents -func NewEncoderWithIndent(w io.Writer, mapIndent int, seqIndent SeqIndentType) *yaml.Encoder { +// NewEncoderWithOptions returns the encoder with provided options +func NewEncoderWithOptions(w io.Writer, opts *EncoderOptions) *yaml.Encoder { encoder := NewEncoder(w) - encoder.SetIndent(mapIndent) - if seqIndent == WideSeqIndent { + encoder.SetIndent(opts.MapIndent) + if opts.SeqIndent == WideSeqIndent { encoder.DefaultSeqIndent() } else { encoder.CompactSeqIndent() diff --git a/kyaml/yaml/rnode.go b/kyaml/yaml/rnode.go index f3ba5b0797..9d9cbcaf94 100644 --- a/kyaml/yaml/rnode.go +++ b/kyaml/yaml/rnode.go @@ -505,14 +505,6 @@ func (rn *RNode) SetAnnotations(m map[string]string) error { return rn.setMapInMetadata(m, AnnotationsField) } -// DeleteAnnotation tries to delete the annotation and clears the empty metadata field. -func (rn *RNode) DeleteAnnotation(annotation string) error { - if err := rn.PipeE(ClearAnnotation(annotation)); err != nil { - return err - } - return ClearEmptyAnnotations(rn) -} - // GetLabels gets the metadata labels field. // If the field is missing, returns an empty map. // Use another method to check for missing metadata.