Skip to content

Commit

Permalink
Merge pull request #550 from vmware-tanzu/map-key-override-in-plain-yml
Browse files Browse the repository at this point in the history
(re)enable map key override for plain YAML
  • Loading branch information
pivotaljohn authored Dec 9, 2021
2 parents 3d2c71d + fd7535b commit fdbc6a0
Show file tree
Hide file tree
Showing 7 changed files with 212 additions and 19 deletions.
23 changes: 22 additions & 1 deletion pkg/cmd/template/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,10 +528,31 @@ func TestPlainYAMLNoTemplateProcessing(t *testing.T) {
yamlTplData := []byte(`
#@ load("funcs/funcs.lib.yml", "yamlfunc")
annotation: 5 #@ 1 + 2
text_template: (@= "string" @)`)
text_template: (@= "string" @)
versions:
- &version
name: v1alpha1
served: true
- << : *version
name: v1beta1
- << : *version
name: v1
storage: true
- << : *version
`)

expectedYAMLTplData := `annotation: 5
text_template: (@= "string" @)
versions:
- name: v1alpha1
served: true
- name: v1beta1
served: true
- name: v1
served: true
storage: true
- name: v1alpha1
served: true
`

filesToProcess := []*files.File{
Expand Down
14 changes: 12 additions & 2 deletions pkg/workspace/template_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,23 @@ func (l *TemplateLoader) EvalYAML(libraryCtx LibraryExecutionContext, file *file
return nil, nil, err
}

l.ui.Debugf("### ast\n")
docSet.Print(l.ui.DebugWriter())
l.ui.Debugf("### ast ")

// is this plain YAML?
if !file.IsTemplate() && !file.IsLibrary() || !yamltemplate.HasTemplating(docSet) {
// YAML spec requires map keys to be unique.
// Tools retain just the last instance: each subsequent map item overrides the value of any previous.
docSet.OverrideMapKeys()

l.ui.Debugf("(plain)\n")
docSet.Print(l.ui.DebugWriter())

return nil, docSet, nil
}

l.ui.Debugf("(templated)\n")
docSet.Print(l.ui.DebugWriter())

tplOpts := yamltemplate.TemplateOpts{
IgnoreUnknownComments: l.opts.IgnoreUnknownComments,
ImplicitMapKeyOverrides: l.opts.ImplicitMapKeyOverrides,
Expand Down
5 changes: 5 additions & 0 deletions pkg/yamlmeta/document_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,8 @@ func (ds *DocumentSet) AsBytesWithPrinter(printerFunc func(io.Writer) DocumentPr

return buf.Bytes(), nil
}

// OverrideMapKeys within any contained Map, where there is more than one MapItem with the same key, delete all but the last.
func (ds *DocumentSet) OverrideMapKeys() {
_ = Walk(ds, &overrideMapKeys{})
}
32 changes: 32 additions & 0 deletions pkg/yamlmeta/map_key_overrides.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright 2020 VMware, Inc.
// SPDX-License-Identifier: Apache-2.0

package yamlmeta

import (
"github.com/k14s/ytt/pkg/orderedmap"
)

type overrideMapKeys struct{}

// Visit if `node` is a Map, among its MapItem's that have duplicate keys, removes all but the last.
// This visitor always returns `nil`
func (r *overrideMapKeys) Visit(node Node) error {
mapNode, isMap := node.(*Map)
if !isMap {
return nil
}

lastItems := orderedmap.NewMap()
for _, item := range mapNode.Items {
lastItems.Set(item.Key, item)
}

var newItems []*MapItem
lastItems.Iterate(func(_, value interface{}) {
newItems = append(newItems, value.(*MapItem))
})
mapNode.Items = newItems

return nil
}
97 changes: 97 additions & 0 deletions pkg/yamlmeta/map_key_overrides_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
// Copyright 2020 VMware, Inc.
// SPDX-License-Identifier: Apache-2.0

package yamlmeta_test

import (
"testing"

"github.com/k14s/ytt/pkg/yamlmeta"
)

func TestMapKeyOverridePlainYAML(t *testing.T) {
t.Run("when no maps have duplicate keys, is a no op", func(t *testing.T) {
docSet := &yamlmeta.DocumentSet{
Items: []*yamlmeta.Document{{
Value: &yamlmeta.Map{
Items: []*yamlmeta.MapItem{
{Key: "foo", Value: 1},
{Key: "bar", Value: 2},
},
},
}},
}
expectedDocSet := docSet.DeepCopyAsNode()
docSet.OverrideMapKeys()

printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

result := printer.PrintStr(docSet)
expected := printer.PrintStr(expectedDocSet)
assertEqual(t, result, expected)
})
t.Run("when there are duplicates, last map item overrides", func(t *testing.T) {
docSet := &yamlmeta.DocumentSet{
Items: []*yamlmeta.Document{{
Value: &yamlmeta.Map{
Items: []*yamlmeta.MapItem{
{Key: "foo", Value: 1},
{Key: "foo", Value: 2},
{Key: "foo", Value: 3},
{Key: "foo", Value: 4},
},
},
}},
}
expectedDocSet := &yamlmeta.DocumentSet{
Items: []*yamlmeta.Document{{
Value: &yamlmeta.Map{
Items: []*yamlmeta.MapItem{
{Key: "foo", Value: 4},
},
},
}},
}
docSet.OverrideMapKeys()

printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

result := printer.PrintStr(docSet)
expected := printer.PrintStr(expectedDocSet)
assertEqual(t, result, expected)
})
t.Run("when there are multiple keys with duplicates, last map item for each key overrides the others", func(t *testing.T) {
docSet := &yamlmeta.DocumentSet{
Items: []*yamlmeta.Document{{
Value: &yamlmeta.Map{
Items: []*yamlmeta.MapItem{
{Key: "foo", Value: 1},
{Key: "bar", Value: 2},
{Key: "ree", Value: 3},
{Key: "ree", Value: 4},
{Key: "foo", Value: 5},
{Key: "bar", Value: 6},
},
},
}},
}
expectedDocSet := &yamlmeta.DocumentSet{
Items: []*yamlmeta.Document{{
Value: &yamlmeta.Map{
Items: []*yamlmeta.MapItem{
{Key: "foo", Value: 5},
{Key: "bar", Value: 6},
{Key: "ree", Value: 4},
},
},
}},
}
docSet.OverrideMapKeys()

printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

result := printer.PrintStr(docSet)
expected := printer.PrintStr(expectedDocSet)
assertEqual(t, result, expected)
})
}
31 changes: 15 additions & 16 deletions pkg/yamlmeta/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package yamlmeta_test

import (
"fmt"
"os"
"strings"
"testing"

Expand Down Expand Up @@ -33,7 +32,7 @@ func TestParserDocSetEmpty(t *testing.T) {
Position: filepos.NewUnknownPosition(),
}

printer := yamlmeta.NewPrinterWithOpts(os.Stdout, yamlmeta.PrinterOpts{ExcludeRefs: true})
printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

parsedValStr := printer.PrintStr(parsedVal)
expectedValStr := printer.PrintStr(expectedVal)
Expand All @@ -58,7 +57,7 @@ func TestParserDocSetNewline(t *testing.T) {
Position: filepos.NewUnknownPosition(),
}

printer := yamlmeta.NewPrinterWithOpts(os.Stdout, yamlmeta.PrinterOpts{ExcludeRefs: true})
printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

parsedValStr := printer.PrintStr(parsedVal)
expectedValStr := printer.PrintStr(expectedVal)
Expand Down Expand Up @@ -89,7 +88,7 @@ func TestParserOnlyComment(t *testing.T) {
Position: filepos.NewUnknownPosition(),
}

printer := yamlmeta.NewPrinterWithOpts(os.Stdout, yamlmeta.PrinterOpts{ExcludeRefs: true})
printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

parsedValStr := printer.PrintStr(parsedVal)
expectedValStr := printer.PrintStr(expectedVal)
Expand All @@ -116,7 +115,7 @@ func TestParserDoc(t *testing.T) {
Position: filepos.NewUnknownPosition(),
}

printer := yamlmeta.NewPrinterWithOpts(os.Stdout, yamlmeta.PrinterOpts{ExcludeRefs: true})
printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

parsedValStr := printer.PrintStr(parsedVal)
expectedValStr := printer.PrintStr(expectedVal)
Expand Down Expand Up @@ -147,7 +146,7 @@ func TestParserDocWithoutDashes(t *testing.T) {
Position: filepos.NewUnknownPosition(),
}

printer := yamlmeta.NewPrinterWithOpts(os.Stdout, yamlmeta.PrinterOpts{ExcludeRefs: true})
printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

parsedValStr := printer.PrintStr(parsedVal)
expectedValStr := printer.PrintStr(expectedVal)
Expand Down Expand Up @@ -310,7 +309,7 @@ array:
Position: filepos.NewUnknownPosition(),
}

printer := yamlmeta.NewPrinterWithOpts(os.Stdout, yamlmeta.PrinterOpts{ExcludeRefs: true})
printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

parsedValStr := printer.PrintStr(parsedVal)
expectedValStr := printer.PrintStr(expectedVal)
Expand Down Expand Up @@ -378,7 +377,7 @@ map:
Position: filepos.NewUnknownPosition(),
}

printer := yamlmeta.NewPrinterWithOpts(os.Stdout, yamlmeta.PrinterOpts{ExcludeRefs: true})
printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

parsedValStr := printer.PrintStr(parsedVal)
expectedValStr := printer.PrintStr(expectedVal)
Expand Down Expand Up @@ -489,7 +488,7 @@ array:
Position: filepos.NewUnknownPosition(),
}

printer := yamlmeta.NewPrinterWithOpts(os.Stdout, yamlmeta.PrinterOpts{ExcludeRefs: true})
printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

parsedValStr := printer.PrintStr(parsedVal)
expectedValStr := printer.PrintStr(expectedVal)
Expand Down Expand Up @@ -534,7 +533,7 @@ func TestParserDocSetComments(t *testing.T) {
Position: filepos.NewUnknownPosition(),
}

printer := yamlmeta.NewPrinterWithOpts(os.Stdout, yamlmeta.PrinterOpts{ExcludeRefs: true})
printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

parsedValStr := printer.PrintStr(parsedVal)
expectedValStr := printer.PrintStr(expectedVal)
Expand Down Expand Up @@ -565,7 +564,7 @@ func TestParserDocSetOnlyComments2(t *testing.T) {
Position: filepos.NewUnknownPosition(),
}

printer := yamlmeta.NewPrinterWithOpts(os.Stdout, yamlmeta.PrinterOpts{ExcludeRefs: true})
printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

parsedValStr := printer.PrintStr(parsedVal)
expectedValStr := printer.PrintStr(expectedVal)
Expand Down Expand Up @@ -593,7 +592,7 @@ func TestParserDocSetOnlyComments3(t *testing.T) {
Position: filepos.NewUnknownPosition(),
}

printer := yamlmeta.NewPrinterWithOpts(os.Stdout, yamlmeta.PrinterOpts{ExcludeRefs: true})
printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

parsedValStr := printer.PrintStr(parsedVal)
expectedValStr := printer.PrintStr(expectedVal)
Expand Down Expand Up @@ -624,7 +623,7 @@ func TestParserDocSetOnlyComments(t *testing.T) {
Position: filepos.NewUnknownPosition(),
}

printer := yamlmeta.NewPrinterWithOpts(os.Stdout, yamlmeta.PrinterOpts{ExcludeRefs: true})
printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

parsedValStr := printer.PrintStr(parsedVal)
expectedValStr := printer.PrintStr(expectedVal)
Expand Down Expand Up @@ -668,7 +667,7 @@ func TestParserDocSetCommentsNoFirstDashes(t *testing.T) {
Position: filepos.NewUnknownPosition(),
}

printer := yamlmeta.NewPrinterWithOpts(os.Stdout, yamlmeta.PrinterOpts{ExcludeRefs: true})
printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

parsedValStr := printer.PrintStr(parsedVal)
expectedValStr := printer.PrintStr(expectedVal)
Expand Down Expand Up @@ -725,7 +724,7 @@ key:
Position: filepos.NewUnknownPosition(),
}

printer := yamlmeta.NewPrinterWithOpts(os.Stdout, yamlmeta.PrinterOpts{ExcludeRefs: true})
printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

parsedValStr := printer.PrintStr(parsedVal)
expectedValStr := printer.PrintStr(expectedVal)
Expand Down Expand Up @@ -1024,7 +1023,7 @@ func (ex parserExample) checkDocSet(t *testing.T, parsedVal *yamlmeta.DocumentSe
t.Fatalf("error: %s", err)
}

printer := yamlmeta.NewPrinterWithOpts(os.Stdout, yamlmeta.PrinterOpts{ExcludeRefs: true})
printer := yamlmeta.NewPrinterWithOpts(nil, yamlmeta.PrinterOpts{ExcludeRefs: true})

parsedValStr := printer.PrintStr(parsedVal)
expectedValStr := printer.PrintStr(ex.Expected)
Expand Down
29 changes: 29 additions & 0 deletions pkg/yamlmeta/walk.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2020 VMware, Inc.
// SPDX-License-Identifier: Apache-2.0

package yamlmeta

// Visitor performs an operation on the given Node while traversing the AST.
// Typically defines the action taken during a Walk().
type Visitor interface {
Visit(Node) error
}

// Walk traverses the tree starting at `n`, recursively, depth-first, invoking `v` on each node.
// if `v` returns non-nil error, the traversal is aborted.
func Walk(n Node, v Visitor) error {
err := v.Visit(n)
if err != nil {
return err
}

for _, c := range n.GetValues() {
if cn, ok := c.(Node); ok {
err := Walk(cn, v)
if err != nil {
return err
}
}
}
return nil
}

0 comments on commit fdbc6a0

Please sign in to comment.