From 3e0de0dae9be9dd42386ab7e5a73abd9cc831204 Mon Sep 17 00:00:00 2001 From: Michelle Noorali <michellemolu@gmail.com> Date: Thu, 19 Jul 2018 15:54:47 -0400 Subject: [PATCH] fix(release_server): fix how we merge values resolves #4337 Merging maps inside of strings gets a bit tricky. When two strings consisting of "{}" were being added together, this resulted in "{}\n{}" instead of "{}" which is what we wanted. This led to YAML parsing errors and showed up when the `--reuse-values` flag was used when no overrides via `--set` were provided during install and/or upgrade. --- pkg/tiller/release_server.go | 4 +++- pkg/tiller/release_update_test.go | 40 +++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/pkg/tiller/release_server.go b/pkg/tiller/release_server.go index a75b7fc86..5964deeee 100644 --- a/pkg/tiller/release_server.go +++ b/pkg/tiller/release_server.go @@ -138,7 +138,9 @@ func (s *ReleaseServer) reuseValues(req *services.UpdateReleaseRequest, current } // merge new values with current - req.Values.Raw = current.Config.Raw + "\n" + req.Values.Raw + if current.Config != nil && current.Config.Raw != "" && current.Config.Raw != "{}\n" { + req.Values.Raw = current.Config.Raw + "\n" + req.Values.Raw + } req.Chart.Values = &chart.Config{Raw: nv} // yaml unmarshal and marshal to remove duplicate keys diff --git a/pkg/tiller/release_update_test.go b/pkg/tiller/release_update_test.go index 56dcca874..02bcb3842 100644 --- a/pkg/tiller/release_update_test.go +++ b/pkg/tiller/release_update_test.go @@ -129,6 +129,46 @@ func TestUpdateRelease_ResetValues(t *testing.T) { } } +func TestUpdateRelease_ReuseValuesWithNoValues(t *testing.T) { + c := helm.NewContext() + rs := rsFixture() + + installReq := &services.InstallReleaseRequest{ + Namespace: "spaced", + Chart: &chart.Chart{ + Metadata: &chart.Metadata{Name: "hello"}, + Templates: []*chart.Template{ + {Name: "templates/hello", Data: []byte("hello: world")}, + {Name: "templates/hooks", Data: []byte(manifestWithHook)}, + }, + Values: &chart.Config{Raw: "defaultFoo: defaultBar"}, + }, + } + + installResp, err := rs.InstallRelease(c, installReq) + if err != nil { + t.Fatal(err) + } + + rel := installResp.Release + req := &services.UpdateReleaseRequest{ + Name: rel.Name, + Chart: &chart.Chart{ + Metadata: &chart.Metadata{Name: "hello"}, + Templates: []*chart.Template{ + {Name: "templates/hello", Data: []byte("hello: world")}, + }, + }, + Values: &chart.Config{Raw: ""}, + ReuseValues: true, + } + + _, err = rs.UpdateRelease(c, req) + if err != nil { + t.Fatalf("Failed updated: %s", err) + } +} + // This is a regression test for bug found in issue #3655 func TestUpdateRelease_ComplexReuseValues(t *testing.T) { c := helm.NewContext() -- GitLab