From dcfa119305b077573d483e9190259f2e7e18b8ac Mon Sep 17 00:00:00 2001 From: Cari Date: Thu, 1 Apr 2021 15:54:22 -0700 Subject: [PATCH 1/8] Add failing test for map overlay position - need to build in a way to test line positions `template_test.go` - run this test with `./hack/test-unit.sh -run TestYAMLTemplate TestYAMLTemplate.filetest=filetests/ytt-library/overlay/replace-key-position` --- .../overlay/replace-key-position.yml | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 pkg/yamltemplate/filetests/ytt-library/overlay/replace-key-position.yml diff --git a/pkg/yamltemplate/filetests/ytt-library/overlay/replace-key-position.yml b/pkg/yamltemplate/filetests/ytt-library/overlay/replace-key-position.yml new file mode 100644 index 00000000..d6789fa7 --- /dev/null +++ b/pkg/yamltemplate/filetests/ytt-library/overlay/replace-key-position.yml @@ -0,0 +1,34 @@ +#@ load("@ytt:overlay", "overlay") +#@ load("@ytt:template", "template") + +#@ def/end test1_left(): +--- +apiVersion: extensions/v1beta1 +kind: Ingress +metadata: + name: example-ingress1 + annotations: + key: 1 + +#@ def/end test1_right(): +#@overlay/match by=overlay.all +--- +metadata: + annotations: + key: 2 + +--- +test1 +--- #@ template.replace(overlay.apply(test1_left(), test1_right())) +--- ++++ + +OUTPUT POSITION: stdin:20 | [doc] + | test1 + stdin:5 | [doc] + stdin:6 | apiVersion: extensions/v1beta1 + stdin:7 | kind: Ingress + stdin:8 | metadata: + stdin:9 | name: example-ingress1 + stdin:10 | annotations: + stdin:18 | key: 2 From f21e1b0f24cd03f7a8e3e8851a1b9a184d149e13 Mon Sep 17 00:00:00 2001 From: Cari Date: Tue, 6 Apr 2021 15:20:38 -0700 Subject: [PATCH 2/8] Add ability to assert positions in template_test Co-authored-by: Garrett Cheadle --- pkg/yamltemplate/template_test.go | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/pkg/yamltemplate/template_test.go b/pkg/yamltemplate/template_test.go index 30897aea..2c1719a2 100644 --- a/pkg/yamltemplate/template_test.go +++ b/pkg/yamltemplate/template_test.go @@ -5,6 +5,7 @@ package yamltemplate_test import ( "fmt" + "io" "io/ioutil" "os" "path/filepath" @@ -67,10 +68,15 @@ func TestYAMLTemplate(t *testing.T) { if len(pieces) != 2 { t.Fatalf("expected file %s to include +++ separator", filePath) } - - resultStr, testErr := evalTemplate(t, pieces[0]) expectedStr := pieces[1] + includePositions := false + if strings.HasPrefix(expectedStr, "OUTPUT POSITION:") { + includePositions = true + } + + resultStr, testErr := evalTemplate(t, pieces[0], includePositions) + if strings.HasPrefix(expectedStr, "ERR: ") { if testErr == nil { err = fmt.Errorf("expected eval error, but did not receive it") @@ -79,6 +85,12 @@ func TestYAMLTemplate(t *testing.T) { resultStr = regexp.MustCompile("__ytt_tpl\\d+_").ReplaceAllString(resultStr, "__ytt_tplXXX_") err = expectEquals(t, resultStr, strings.ReplaceAll(strings.TrimPrefix(expectedStr, "ERR: "), "__YTT_VERSION__", version.Version)) } + } else if includePositions { + if testErr == nil { + err = expectEquals(t, resultStr, strings.ReplaceAll(strings.TrimPrefix(expectedStr, "OUTPUT POSITION:"), "__YTT_VERSION__", version.Version)) + } else { + err = testErr.TestErr() + } } else { if testErr == nil { err = expectEquals(t, resultStr, expectedStr) @@ -116,7 +128,7 @@ type testErr struct { func (e testErr) UserErr() error { return e.realErr } func (e testErr) TestErr() error { return e.testErr } -func evalTemplate(t *testing.T, data string) (string, *testErr) { +func evalTemplate(t *testing.T, data string, includePositions bool) (string, *testErr) { docSet, err := yamlmeta.NewDocumentSetFromBytes([]byte(data), yamlmeta.DocSetOpts{AssociatedName: "stdin"}) if err != nil { return "", &testErr{err, fmt.Errorf("unmarshal error: %v", err)} @@ -152,6 +164,18 @@ func evalTemplate(t *testing.T, data string) (string, *testErr) { typedNewVal.(*yamlmeta.DocumentSet).Print(os.Stdout) } + if includePositions { + printerFunc := func(w io.Writer) yamlmeta.DocumentPrinter { + return yamlmeta.WrappedFilePositionPrinter{yamlmeta.NewFilePositionPrinter(w)} + } + documentSet := typedNewVal.(*yamlmeta.DocumentSet) + combinedDocBytes, err := documentSet.AsBytesWithPrinter(printerFunc) + if err != nil { + return "", &testErr{err, fmt.Errorf("expected result docSet to be printable: %v", err)} + } + return string(combinedDocBytes), nil + } + resultBytes, err := typedNewVal.AsBytes() if err != nil { return "", &testErr{err, fmt.Errorf("marshal error: %v", err)} From 1b39c555d98095d4983d4f2d194fa6c7a775f000 Mon Sep 17 00:00:00 2001 From: Cari Date: Wed, 7 Apr 2021 16:49:30 -0700 Subject: [PATCH 3/8] Extract converting evaluated output as string - extract stringifying evaluated template into separate function --- pkg/yamltemplate/template_test.go | 79 ++++++++++++++++++------------- 1 file changed, 46 insertions(+), 33 deletions(-) diff --git a/pkg/yamltemplate/template_test.go b/pkg/yamltemplate/template_test.go index 2c1719a2..bc7669e2 100644 --- a/pkg/yamltemplate/template_test.go +++ b/pkg/yamltemplate/template_test.go @@ -70,12 +70,7 @@ func TestYAMLTemplate(t *testing.T) { } expectedStr := pieces[1] - includePositions := false - if strings.HasPrefix(expectedStr, "OUTPUT POSITION:") { - includePositions = true - } - - resultStr, testErr := evalTemplate(t, pieces[0], includePositions) + result, testErr := evalTemplate(t, pieces[0]) if strings.HasPrefix(expectedStr, "ERR: ") { if testErr == nil { @@ -85,15 +80,25 @@ func TestYAMLTemplate(t *testing.T) { resultStr = regexp.MustCompile("__ytt_tpl\\d+_").ReplaceAllString(resultStr, "__ytt_tplXXX_") err = expectEquals(t, resultStr, strings.ReplaceAll(strings.TrimPrefix(expectedStr, "ERR: "), "__YTT_VERSION__", version.Version)) } - } else if includePositions { + } else if strings.HasPrefix(expectedStr, "OUTPUT POSITION:") { if testErr == nil { - err = expectEquals(t, resultStr, strings.ReplaceAll(strings.TrimPrefix(expectedStr, "OUTPUT POSITION:"), "__YTT_VERSION__", version.Version)) + resultStr, strErr := asFilePositionsStr(result) + if strErr != nil { + err = strErr + } else { + err = expectEquals(t, resultStr, strings.ReplaceAll(strings.TrimPrefix(expectedStr, "OUTPUT POSITION:"), "__YTT_VERSION__", version.Version)) + } } else { err = testErr.TestErr() } } else { if testErr == nil { - err = expectEquals(t, resultStr, expectedStr) + resultStr, strErr := asString(result) + if strErr != nil { + err = strErr + } else { + err = expectEquals(t, resultStr, expectedStr) + } } else { err = testErr.TestErr() } @@ -120,6 +125,32 @@ func TestYAMLTemplate(t *testing.T) { } } +func asFilePositionsStr(result interface{}) (string, error) { + printerFunc := func(w io.Writer) yamlmeta.DocumentPrinter { + return yamlmeta.WrappedFilePositionPrinter{yamlmeta.NewFilePositionPrinter(w)} + } + docSet := result.(*yamlmeta.DocumentSet) + combinedDocBytes, err := docSet.AsBytesWithPrinter(printerFunc) + if err != nil { + return "", fmt.Errorf("expected result docSet to be printable: %v", err) + } + return string(combinedDocBytes), nil +} + +func asString(result interface{}) (string, error) { + typedNewVal, ok := result.(interface{ AsBytes() ([]byte, error) }) + if !ok { + return "", fmt.Errorf("expected eval result to be marshalable") + } + + resultBytes, err := typedNewVal.AsBytes() + if err != nil { + return "", fmt.Errorf("marshal error: %v", err) + } + + return string(resultBytes), nil +} + type testErr struct { realErr error // error returned to the user testErr error // error wrapped with helpful test context @@ -128,15 +159,15 @@ type testErr struct { func (e testErr) UserErr() error { return e.realErr } func (e testErr) TestErr() error { return e.testErr } -func evalTemplate(t *testing.T, data string, includePositions bool) (string, *testErr) { +func evalTemplate(t *testing.T, data string) (interface{}, *testErr) { docSet, err := yamlmeta.NewDocumentSetFromBytes([]byte(data), yamlmeta.DocSetOpts{AssociatedName: "stdin"}) if err != nil { - return "", &testErr{err, fmt.Errorf("unmarshal error: %v", err)} + return nil, &testErr{err, fmt.Errorf("unmarshal error: %v", err)} } compiledTemplate, err := yamltemplate.NewTemplate("stdin", yamltemplate.TemplateOpts{}).Compile(docSet) if err != nil { - return "", &testErr{err, fmt.Errorf("build error: %v", err)} + return nil, &testErr{err, fmt.Errorf("build error: %v", err)} } if showTemplateCode == "t" { @@ -151,37 +182,19 @@ func evalTemplate(t *testing.T, data string, includePositions bool) (string, *te _, newVal, err := compiledTemplate.Eval(thread, loader) if err != nil { - return "", &testErr{err, fmt.Errorf("eval error: %v\ncode:\n%s", err, compiledTemplate.DebugCodeAsString())} + return nil, &testErr{err, fmt.Errorf("eval error: %v\ncode:\n%s", err, compiledTemplate.DebugCodeAsString())} } typedNewVal, ok := newVal.(interface{ AsBytes() ([]byte, error) }) if !ok { - return "", &testErr{err, fmt.Errorf("expected eval result to be marshalable")} + return nil, &testErr{err, fmt.Errorf("expected eval result to be marshalable")} } if showTemplateCode == "t" { fmt.Printf("### result ast:\n") typedNewVal.(*yamlmeta.DocumentSet).Print(os.Stdout) } - - if includePositions { - printerFunc := func(w io.Writer) yamlmeta.DocumentPrinter { - return yamlmeta.WrappedFilePositionPrinter{yamlmeta.NewFilePositionPrinter(w)} - } - documentSet := typedNewVal.(*yamlmeta.DocumentSet) - combinedDocBytes, err := documentSet.AsBytesWithPrinter(printerFunc) - if err != nil { - return "", &testErr{err, fmt.Errorf("expected result docSet to be printable: %v", err)} - } - return string(combinedDocBytes), nil - } - - resultBytes, err := typedNewVal.AsBytes() - if err != nil { - return "", &testErr{err, fmt.Errorf("marshal error: %v", err)} - } - - return string(resultBytes), nil + return typedNewVal, nil } func expectEquals(t *testing.T, resultStr, expectedStr string) error { From 00f1f4d5eeaf1801d3453406e6123573fbf6440d Mon Sep 17 00:00:00 2001 From: Cari Dean Date: Thu, 8 Apr 2021 16:29:27 -0700 Subject: [PATCH 4/8] Test map output postions after overlay Signed-off-by: Garrett Cheadle --- .../overlay/map-output-position.yml | 84 +++++++++++++++++++ .../overlay/replace-key-position.yml | 34 -------- 2 files changed, 84 insertions(+), 34 deletions(-) create mode 100644 pkg/yamltemplate/filetests/ytt-library/overlay/map-output-position.yml delete mode 100644 pkg/yamltemplate/filetests/ytt-library/overlay/replace-key-position.yml diff --git a/pkg/yamltemplate/filetests/ytt-library/overlay/map-output-position.yml b/pkg/yamltemplate/filetests/ytt-library/overlay/map-output-position.yml new file mode 100644 index 00000000..06bb6d08 --- /dev/null +++ b/pkg/yamltemplate/filetests/ytt-library/overlay/map-output-position.yml @@ -0,0 +1,84 @@ +#@ load("@ytt:overlay", "overlay") +#@ load("@ytt:template", "template") + +#@ def/end test1_left(): +--- +apiVersion: extensions/v1beta1 +kind: Ingress +metadata: + name: example-ingress1 + annotations: + key: 1 + +#@ def/end test1_right(): +#@overlay/match by=overlay.all +--- +metadata: + annotations: + key: 2 + +#@ def/end test2_left(): +--- +apiVersion: extensions/v1beta1 +kind: Ingress +metadata: + name: example-ingress1 + annotations: + key: 1 + +#@ def/end test2_right(): +#@overlay/match by=overlay.all +--- +metadata: + #@overlay/replace + annotations: + keya: 2 + +#@ def/end test3_left(): +--- +foo: 1 + +#@ def return_something(): +#@ return "something" +#@ end + +#@ def/end test3_right(): +#@overlay/match by=overlay.all +--- +foo: #@ return_something() + +--- +test1 +--- #@ template.replace(overlay.apply(test1_left(), test1_right())) +--- +test2 +--- #@ template.replace(overlay.apply(test2_left(), test2_right())) +--- +test3 +--- #@ template.replace(overlay.apply(test3_left(), test3_right())) +--- + ++++ + +OUTPUT POSITION: stdin:50 | [doc] + | test1 + stdin:5 | [doc] + stdin:6 | apiVersion: extensions/v1beta1 + stdin:7 | kind: Ingress + stdin:8 | metadata: + stdin:9 | name: example-ingress1 + stdin:10 | annotations: + stdin:18 | key: 2 + stdin:53 | [doc] + | test2 + stdin:21 | [doc] + stdin:22 | apiVersion: extensions/v1beta1 + stdin:23 | kind: Ingress + stdin:24 | metadata: + stdin:25 | name: example-ingress1 + stdin:34 | annotations: + stdin:35 | keya: 2 + stdin:56 | [doc] + | test3 + stdin:38 | [doc] + stdin:48 | foo: something diff --git a/pkg/yamltemplate/filetests/ytt-library/overlay/replace-key-position.yml b/pkg/yamltemplate/filetests/ytt-library/overlay/replace-key-position.yml deleted file mode 100644 index d6789fa7..00000000 --- a/pkg/yamltemplate/filetests/ytt-library/overlay/replace-key-position.yml +++ /dev/null @@ -1,34 +0,0 @@ -#@ load("@ytt:overlay", "overlay") -#@ load("@ytt:template", "template") - -#@ def/end test1_left(): ---- -apiVersion: extensions/v1beta1 -kind: Ingress -metadata: - name: example-ingress1 - annotations: - key: 1 - -#@ def/end test1_right(): -#@overlay/match by=overlay.all ---- -metadata: - annotations: - key: 2 - ---- -test1 ---- #@ template.replace(overlay.apply(test1_left(), test1_right())) ---- -+++ - -OUTPUT POSITION: stdin:20 | [doc] - | test1 - stdin:5 | [doc] - stdin:6 | apiVersion: extensions/v1beta1 - stdin:7 | kind: Ingress - stdin:8 | metadata: - stdin:9 | name: example-ingress1 - stdin:10 | annotations: - stdin:18 | key: 2 From 0525aeb496c818a9b5465e8e34e5eb5a934336a2 Mon Sep 17 00:00:00 2001 From: Cari Dean Date: Mon, 12 Apr 2021 15:34:15 -0700 Subject: [PATCH 5/8] Test array ouput positions after overlay Signed-off-by: Garrett Cheadle --- .../overlay/array-output-position.yml | 74 +++++++++++++++++++ pkg/yttlibrary/overlay/array.go | 1 - pkg/yttlibrary/overlay/map.go | 1 - 3 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 pkg/yamltemplate/filetests/ytt-library/overlay/array-output-position.yml diff --git a/pkg/yamltemplate/filetests/ytt-library/overlay/array-output-position.yml b/pkg/yamltemplate/filetests/ytt-library/overlay/array-output-position.yml new file mode 100644 index 00000000..89b6f0d4 --- /dev/null +++ b/pkg/yamltemplate/filetests/ytt-library/overlay/array-output-position.yml @@ -0,0 +1,74 @@ +#@ load("@ytt:overlay", "overlay") +#@ load("@ytt:template", "template") + +#@ def/end test1_left(): +--- +clients: + - needsSecret: true + +#@ def/end test1_right(): +#@overlay/match by=overlay.all +--- +clients: + #@overlay/match by=overlay.subset({"needsSecret": True}) + - needsSecret: false + +#@ def/end test2_left(): +--- +clients: + - needsSecret: true + +#@ def/end test2_right(): +#@overlay/match by=overlay.all +--- +clients: + #@overlay/match by=overlay.subset({"needsSecret": True}) + #@overlay/replace + - secret: foo + +#@ def/end test3_left(): +--- +clients: + - needsSecret: true + +#@ def return_something(): +#@ return "something" +#@ end + +#@ def/end test3_right(): +#@overlay/match by=overlay.all +--- +clients: + #@overlay/match by=overlay.subset({"needsSecret": True}) + - #@ return_something() + +--- +test1 +--- #@ template.replace(overlay.apply(test1_left(), test1_right())) +--- +test2 +--- #@ template.replace(overlay.apply(test2_left(), test2_right())) +--- +test3 +--- #@ template.replace(overlay.apply(test3_left(), test3_right())) +--- + ++++ + +OUTPUT POSITION: stdin:45 | [doc] + | test1 + stdin:5 | [doc] + stdin:6 | clients: + stdin:7 | [0] + stdin:14 | needsSecret: false + stdin:48 | [doc] + | test2 + stdin:17 | [doc] + stdin:18 | clients: + stdin:27 | [0] + stdin:27 | secret: foo + stdin:51 | [doc] + | test3 + stdin:30 | [doc] + stdin:31 | clients: + stdin:43 | [0] something diff --git a/pkg/yttlibrary/overlay/array.go b/pkg/yttlibrary/overlay/array.go index 2c2d277b..64043746 100644 --- a/pkg/yttlibrary/overlay/array.go +++ b/pkg/yttlibrary/overlay/array.go @@ -43,7 +43,6 @@ func (o Op) mergeArrayItem( } if replace { leftArray.Items[leftIdx].Value = newItem.Value - //todo: should we be setting the position elsewhere? leftArray.Items[leftIdx].Position = newItem.Position } } diff --git a/pkg/yttlibrary/overlay/map.go b/pkg/yttlibrary/overlay/map.go index c259b147..f72e5a1c 100644 --- a/pkg/yttlibrary/overlay/map.go +++ b/pkg/yttlibrary/overlay/map.go @@ -44,7 +44,6 @@ func (o Op) mergeMapItem(leftMap *yamlmeta.Map, newItem *yamlmeta.MapItem, } if replace { leftMap.Items[leftIdx].Value = newItem.Value - //todo: should we be setting the position elsewhere? What about annotations? leftMap.Items[leftIdx].Position = newItem.Position } } From b7be5ea4d77b1106d988a5f882b82ac69247075d Mon Sep 17 00:00:00 2001 From: Cari Date: Tue, 13 Apr 2021 11:30:30 -0700 Subject: [PATCH 6/8] Add function to set Position of a Node - Change return and parameter type to a more strict interface Signed-off-by: Steven Locke --- pkg/yamlmeta/ast.go | 1 + pkg/yamlmeta/node.go | 7 +++++++ pkg/yamltemplate/template_test.go | 13 ++++--------- pkg/yttlibrary/overlay/array.go | 7 +++++-- pkg/yttlibrary/overlay/map.go | 7 +++++-- 5 files changed, 22 insertions(+), 13 deletions(-) diff --git a/pkg/yamlmeta/ast.go b/pkg/yamlmeta/ast.go index d94db6c7..b21d01a9 100644 --- a/pkg/yamlmeta/ast.go +++ b/pkg/yamlmeta/ast.go @@ -9,6 +9,7 @@ import ( type Node interface { GetPosition() *filepos.Position + SetPosition(*filepos.Position) GetValues() []interface{} // ie children SetValue(interface{}) error diff --git a/pkg/yamlmeta/node.go b/pkg/yamlmeta/node.go index 2be1296d..e43a5df7 100644 --- a/pkg/yamlmeta/node.go +++ b/pkg/yamlmeta/node.go @@ -20,6 +20,13 @@ func (a *Array) GetPosition() *filepos.Position { return a.Position } func (ai *ArrayItem) GetPosition() *filepos.Position { return ai.Position } func (s *Scalar) GetPosition() *filepos.Position { return s.Position } +func (ds *DocumentSet) SetPosition(position *filepos.Position) { ds.Position = position } +func (d *Document) SetPosition(position *filepos.Position) { d.Position = position } +func (m *Map) SetPosition(position *filepos.Position) { m.Position = position } +func (mi *MapItem) SetPosition(position *filepos.Position) { mi.Position = position } +func (a *Array) SetPosition(position *filepos.Position) { a.Position = position } +func (ai *ArrayItem) SetPosition(position *filepos.Position) { ai.Position = position } + func (ds *DocumentSet) ValueTypeAsString() string { return "documentSet" } func (d *Document) ValueTypeAsString() string { return typeToString(d.Value) } func (m *Map) ValueTypeAsString() string { return "map" } diff --git a/pkg/yamltemplate/template_test.go b/pkg/yamltemplate/template_test.go index bc7669e2..f87f8e94 100644 --- a/pkg/yamltemplate/template_test.go +++ b/pkg/yamltemplate/template_test.go @@ -125,7 +125,7 @@ func TestYAMLTemplate(t *testing.T) { } } -func asFilePositionsStr(result interface{}) (string, error) { +func asFilePositionsStr(result interface{ AsBytes() ([]byte, error) }) (string, error) { printerFunc := func(w io.Writer) yamlmeta.DocumentPrinter { return yamlmeta.WrappedFilePositionPrinter{yamlmeta.NewFilePositionPrinter(w)} } @@ -137,13 +137,8 @@ func asFilePositionsStr(result interface{}) (string, error) { return string(combinedDocBytes), nil } -func asString(result interface{}) (string, error) { - typedNewVal, ok := result.(interface{ AsBytes() ([]byte, error) }) - if !ok { - return "", fmt.Errorf("expected eval result to be marshalable") - } - - resultBytes, err := typedNewVal.AsBytes() +func asString(result interface{ AsBytes() ([]byte, error) }) (string, error) { + resultBytes, err := result.AsBytes() if err != nil { return "", fmt.Errorf("marshal error: %v", err) } @@ -159,7 +154,7 @@ type testErr struct { func (e testErr) UserErr() error { return e.realErr } func (e testErr) TestErr() error { return e.testErr } -func evalTemplate(t *testing.T, data string) (interface{}, *testErr) { +func evalTemplate(t *testing.T, data string) (interface{ AsBytes() ([]byte, error) }, *testErr) { docSet, err := yamlmeta.NewDocumentSetFromBytes([]byte(data), yamlmeta.DocSetOpts{AssociatedName: "stdin"}) if err != nil { return nil, &testErr{err, fmt.Errorf("unmarshal error: %v", err)} diff --git a/pkg/yttlibrary/overlay/array.go b/pkg/yttlibrary/overlay/array.go index 64043746..b0deff91 100644 --- a/pkg/yttlibrary/overlay/array.go +++ b/pkg/yttlibrary/overlay/array.go @@ -42,8 +42,11 @@ func (o Op) mergeArrayItem( } } if replace { - leftArray.Items[leftIdx].Value = newItem.Value - leftArray.Items[leftIdx].Position = newItem.Position + err := leftArray.Items[leftIdx].SetValue(newItem.Value) + if err != nil { + return err + } + leftArray.Items[leftIdx].SetPosition(newItem.Position) } } diff --git a/pkg/yttlibrary/overlay/map.go b/pkg/yttlibrary/overlay/map.go index f72e5a1c..d96c557a 100644 --- a/pkg/yttlibrary/overlay/map.go +++ b/pkg/yttlibrary/overlay/map.go @@ -43,8 +43,11 @@ func (o Op) mergeMapItem(leftMap *yamlmeta.Map, newItem *yamlmeta.MapItem, } } if replace { - leftMap.Items[leftIdx].Value = newItem.Value - leftMap.Items[leftIdx].Position = newItem.Position + err := leftMap.Items[leftIdx].SetValue(newItem.Value) + if err != nil { + return err + } + leftMap.Items[leftIdx].SetPosition(newItem.Position) } } From f3be30d4347e4de7e2daae7a602419e296b3fb60 Mon Sep 17 00:00:00 2001 From: Cari Date: Tue, 13 Apr 2021 13:40:16 -0700 Subject: [PATCH 7/8] Add newline in output position tests --- .../filetests/ytt-library/overlay/array-output-position.yml | 3 ++- .../filetests/ytt-library/overlay/map-output-position.yml | 3 ++- pkg/yamltemplate/template_test.go | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/yamltemplate/filetests/ytt-library/overlay/array-output-position.yml b/pkg/yamltemplate/filetests/ytt-library/overlay/array-output-position.yml index 89b6f0d4..1e22fe5d 100644 --- a/pkg/yamltemplate/filetests/ytt-library/overlay/array-output-position.yml +++ b/pkg/yamltemplate/filetests/ytt-library/overlay/array-output-position.yml @@ -55,7 +55,8 @@ test3 +++ -OUTPUT POSITION: stdin:45 | [doc] +OUTPUT POSITION: + stdin:45 | [doc] | test1 stdin:5 | [doc] stdin:6 | clients: diff --git a/pkg/yamltemplate/filetests/ytt-library/overlay/map-output-position.yml b/pkg/yamltemplate/filetests/ytt-library/overlay/map-output-position.yml index 06bb6d08..d88b337a 100644 --- a/pkg/yamltemplate/filetests/ytt-library/overlay/map-output-position.yml +++ b/pkg/yamltemplate/filetests/ytt-library/overlay/map-output-position.yml @@ -60,7 +60,8 @@ test3 +++ -OUTPUT POSITION: stdin:50 | [doc] +OUTPUT POSITION: + stdin:50 | [doc] | test1 stdin:5 | [doc] stdin:6 | apiVersion: extensions/v1beta1 diff --git a/pkg/yamltemplate/template_test.go b/pkg/yamltemplate/template_test.go index f87f8e94..605c6e55 100644 --- a/pkg/yamltemplate/template_test.go +++ b/pkg/yamltemplate/template_test.go @@ -86,7 +86,7 @@ func TestYAMLTemplate(t *testing.T) { if strErr != nil { err = strErr } else { - err = expectEquals(t, resultStr, strings.ReplaceAll(strings.TrimPrefix(expectedStr, "OUTPUT POSITION:"), "__YTT_VERSION__", version.Version)) + err = expectEquals(t, resultStr, strings.ReplaceAll(strings.TrimPrefix(expectedStr, "OUTPUT POSITION:\n"), "__YTT_VERSION__", version.Version)) } } else { err = testErr.TestErr() From b991e4fbbd13eadcf37c87ce1a92e7c0a7068910 Mon Sep 17 00:00:00 2001 From: Cari Date: Tue, 13 Apr 2021 15:58:31 -0700 Subject: [PATCH 8/8] Add comment that keeping fields is intentional - Preserving the Type and Metas fields on merge overlay operations is intentional in the anticipation that we may need them later for type checking - We did not change logic to preserve fields on overlay replace operations, but anticipate we may need to preserve some fields later on. Signed-off-by: John Ryan --- pkg/yttlibrary/overlay/array.go | 3 +++ pkg/yttlibrary/overlay/map.go | 3 +++ 2 files changed, 6 insertions(+) diff --git a/pkg/yttlibrary/overlay/array.go b/pkg/yttlibrary/overlay/array.go index b0deff91..6752e801 100644 --- a/pkg/yttlibrary/overlay/array.go +++ b/pkg/yttlibrary/overlay/array.go @@ -42,6 +42,7 @@ func (o Op) mergeArrayItem( } } if replace { + // left side type and metas are preserved err := leftArray.Items[leftIdx].SetValue(newItem.Value) if err != nil { return err @@ -116,6 +117,8 @@ func (o Op) replaceArrayItem( return err } + // left side fields are not preserved. + // probably need to rethink how to merge left and right once those fields are needed leftArray.Items[leftIdx] = newItem.DeepCopy() err = leftArray.Items[leftIdx].SetValue(newVal) if err != nil { diff --git a/pkg/yttlibrary/overlay/map.go b/pkg/yttlibrary/overlay/map.go index d96c557a..38df3c33 100644 --- a/pkg/yttlibrary/overlay/map.go +++ b/pkg/yttlibrary/overlay/map.go @@ -43,6 +43,7 @@ func (o Op) mergeMapItem(leftMap *yamlmeta.Map, newItem *yamlmeta.MapItem, } } if replace { + // left side type and metas are preserved err := leftMap.Items[leftIdx].SetValue(newItem.Value) if err != nil { return err @@ -115,6 +116,8 @@ func (o Op) replaceMapItem(leftMap *yamlmeta.Map, newItem *yamlmeta.MapItem, return err } + // left side fields are not preserved. + // probably need to rethink how to merge left and right once those fields are needed leftMap.Items[leftIdx] = newItem.DeepCopy() err = leftMap.Items[leftIdx].SetValue(newVal) if err != nil {