From b7d2947d1010a5abf8f986886b3189d6e55b1d36 Mon Sep 17 00:00:00 2001 From: Fernando Antivero <fla@clariusconsulting.net> Date: Fri, 6 Mar 2020 19:04:29 -0300 Subject: [PATCH] fix(requirements): refactor to use common codepath for table coalescing (#7047) * align both formats behaviors and now they will just differ in how to discover their paths * add coverage for exports format and fix expected assertions for parent-child format to match the logic child values always wins * just partially revert dda8497, this way parents values could be overridden when coalescing * after getting better coverage we were able to refact both formats behaviors by merging their propagation logics into a single code path. --- pkg/chartutil/requirements.go | 48 ++++++++----------- pkg/chartutil/requirements_test.go | 31 ++++++------ .../subpop/charts/subchart3/.helmignore | 22 +++++++++ .../subpop/charts/subchart3/Chart.yaml | 5 ++ .../subpop/charts/subchart3/values.yaml | 4 ++ .../testdata/subpop/requirements.yaml | 7 +++ pkg/chartutil/testdata/subpop/values.yaml | 9 +++- 7 files changed, 83 insertions(+), 43 deletions(-) create mode 100644 pkg/chartutil/testdata/subpop/charts/subchart3/.helmignore create mode 100644 pkg/chartutil/testdata/subpop/charts/subchart3/Chart.yaml create mode 100644 pkg/chartutil/testdata/subpop/charts/subchart3/values.yaml diff --git a/pkg/chartutil/requirements.go b/pkg/chartutil/requirements.go index 4c9713233..d72aa9da5 100644 --- a/pkg/chartutil/requirements.go +++ b/pkg/chartutil/requirements.go @@ -396,7 +396,7 @@ func processImportValues(c *chart.Chart) error { if err != nil { return err } - b := cvals.AsMap() + b := make(map[string]interface{}, 0) // import values from each dependency if specified in import-values for _, r := range reqs.Dependencies { // only process raw requirement that is found in chart's dependencies (enabled) @@ -417,42 +417,34 @@ func processImportValues(c *chart.Chart) error { if len(r.ImportValues) > 0 { var outiv []interface{} for _, riv := range r.ImportValues { + nm := make(map[string]string, 0) switch iv := riv.(type) { case map[string]interface{}: - nm := map[string]string{ - "child": iv["child"].(string), - "parent": iv["parent"].(string), - } - outiv = append(outiv, nm) - s := name + "." + nm["child"] - // get child table - vv, err := cvals.Table(s) - if err != nil { - log.Printf("Warning: ImportValues missing table: %v", err) - continue - } - // create value map from child to be merged into parent - vm := pathToMap(nm["parent"], vv.AsMap()) - b = coalesceTables(b, vm, c.Metadata.Name) + nm["child"] = iv["child"].(string) + nm["parent"] = iv["parent"].(string) case string: - nm := map[string]string{ - "child": "exports." + iv, - "parent": ".", - } - outiv = append(outiv, nm) - s := name + "." + nm["child"] - vm, err := cvals.Table(s) - if err != nil { - log.Printf("Warning: ImportValues missing table: %v", err) - continue - } - b = coalesceTables(b, vm.AsMap(), c.Metadata.Name) + nm["child"] = "exports." + iv + nm["parent"] = "." } + + outiv = append(outiv, nm) + s := name + "." + nm["child"] + // get child table + vv, err := cvals.Table(s) + if err != nil { + log.Printf("Warning: ImportValues missing table: %v", err) + continue + } + // create value map from child to be merged into parent + vm := pathToMap(nm["parent"], vv.AsMap()) + b = coalesceTables(b, vm, c.Metadata.Name) + } // set our formatted import values r.ImportValues = outiv } } + b = coalesceTables(b, cvals, c.Metadata.Name) y, err := yaml.Marshal(b) if err != nil { return err diff --git a/pkg/chartutil/requirements_test.go b/pkg/chartutil/requirements_test.go index f1b0ebdb3..640987fdd 100644 --- a/pkg/chartutil/requirements_test.go +++ b/pkg/chartutil/requirements_test.go @@ -259,10 +259,10 @@ func TestProcessRequirementsImportValues(t *testing.T) { e["imported-chartA-B.SPextra5"] = "k8s" e["imported-chartA-B.SC1extra5"] = "tiller" - e["overridden-chart1.SC1bool"] = "false" - e["overridden-chart1.SC1float"] = "3.141592" - e["overridden-chart1.SC1int"] = "99" - e["overridden-chart1.SC1string"] = "pollywog" + e["overridden-chart1.SC1bool"] = "true" + e["overridden-chart1.SC1float"] = "3.14" + e["overridden-chart1.SC1int"] = "100" + e["overridden-chart1.SC1string"] = "dollywood" e["overridden-chart1.SPextra2"] = "42" e["overridden-chartA.SCAbool"] = "true" @@ -272,13 +272,13 @@ func TestProcessRequirementsImportValues(t *testing.T) { e["overridden-chartA.SPextra4"] = "true" e["overridden-chartA-B.SCAbool"] = "true" - e["overridden-chartA-B.SCAfloat"] = "41.3" - e["overridden-chartA-B.SCAint"] = "808" - e["overridden-chartA-B.SCAstring"] = "jaberwocky" - e["overridden-chartA-B.SCBbool"] = "false" - e["overridden-chartA-B.SCBfloat"] = "1.99" - e["overridden-chartA-B.SCBint"] = "77" - e["overridden-chartA-B.SCBstring"] = "jango" + e["overridden-chartA-B.SCAfloat"] = "3.33" + e["overridden-chartA-B.SCAint"] = "555" + e["overridden-chartA-B.SCAstring"] = "wormwood" + e["overridden-chartA-B.SCBbool"] = "true" + e["overridden-chartA-B.SCBfloat"] = "0.25" + e["overridden-chartA-B.SCBint"] = "98" + e["overridden-chartA-B.SCBstring"] = "murkwood" e["overridden-chartA-B.SPextra6"] = "111" e["overridden-chartA-B.SCAextra1"] = "23" e["overridden-chartA-B.SCBextra1"] = "13" @@ -290,6 +290,9 @@ func TestProcessRequirementsImportValues(t *testing.T) { e["SCBexported2A"] = "blaster" e["global.SC1exported2.all.SC1exported3"] = "SC1expstr" + e["SCCdata.SCCstring"] = "mugwort" + e["SCCdata.SCCint"] = "42" + verifyRequirementsImportValues(t, c, v, e) } func verifyRequirementsImportValues(t *testing.T, c *chart.Chart, v *chart.Config, e map[string]string) { @@ -314,18 +317,18 @@ func verifyRequirementsImportValues(t *testing.T, c *chart.Chart, v *chart.Confi case float64: s := strconv.FormatFloat(pv.(float64), 'f', -1, 64) if s != vv { - t.Errorf("Failed to match imported float value %v with expected %v", s, vv) + t.Errorf("Failed to match imported float field %v with value %v with expected %v", kk, s, vv) return } case bool: b := strconv.FormatBool(pv.(bool)) if b != vv { - t.Errorf("Failed to match imported bool value %v with expected %v", b, vv) + t.Errorf("Failed to match imported bool field %v with value %v with expected %v", kk, b, vv) return } default: if pv.(string) != vv { - t.Errorf("Failed to match imported string value %v with expected %v", pv, vv) + t.Errorf("Failed to match imported string field %v with value %v with expected %v", kk, pv, vv) return } } diff --git a/pkg/chartutil/testdata/subpop/charts/subchart3/.helmignore b/pkg/chartutil/testdata/subpop/charts/subchart3/.helmignore new file mode 100644 index 000000000..50af03172 --- /dev/null +++ b/pkg/chartutil/testdata/subpop/charts/subchart3/.helmignore @@ -0,0 +1,22 @@ +# Patterns to ignore when building packages. +# This supports shell glob matching, relative path matching, and +# negation (prefixed with !). Only one pattern per line. +.DS_Store +# Common VCS dirs +.git/ +.gitignore +.bzr/ +.bzrignore +.hg/ +.hgignore +.svn/ +# Common backup files +*.swp +*.bak +*.tmp +*~ +# Various IDEs +.project +.idea/ +*.tmproj +.vscode/ diff --git a/pkg/chartutil/testdata/subpop/charts/subchart3/Chart.yaml b/pkg/chartutil/testdata/subpop/charts/subchart3/Chart.yaml new file mode 100644 index 000000000..0dc60c27c --- /dev/null +++ b/pkg/chartutil/testdata/subpop/charts/subchart3/Chart.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +appVersion: "1.0" +description: A Helm chart for Kubernetes +name: subchart3 +version: 0.1.0 diff --git a/pkg/chartutil/testdata/subpop/charts/subchart3/values.yaml b/pkg/chartutil/testdata/subpop/charts/subchart3/values.yaml new file mode 100644 index 000000000..e371bd074 --- /dev/null +++ b/pkg/chartutil/testdata/subpop/charts/subchart3/values.yaml @@ -0,0 +1,4 @@ +exports: + data: + SCCdata: + SCCstring: "mugwort" diff --git a/pkg/chartutil/testdata/subpop/requirements.yaml b/pkg/chartutil/testdata/subpop/requirements.yaml index a6ee20f07..10c97818b 100644 --- a/pkg/chartutil/testdata/subpop/requirements.yaml +++ b/pkg/chartutil/testdata/subpop/requirements.yaml @@ -35,3 +35,10 @@ dependencies: repository: http://localhost:10191 version: 0.1.0 condition: subchart2alias.enabled + + - name: subchart3 + repository: http://localhost:10191 + version: 0.1.0 + condition: subchart3.enabled + import-values: + - data diff --git a/pkg/chartutil/testdata/subpop/values.yaml b/pkg/chartutil/testdata/subpop/values.yaml index 68eb1323c..c5da8c511 100644 --- a/pkg/chartutil/testdata/subpop/values.yaml +++ b/pkg/chartutil/testdata/subpop/values.yaml @@ -35,9 +35,16 @@ overridden-chartA-B: SCBstring: "jango" SPextra6: 111 +SCCdata: + SCCstring: "jaberwocky" + SCCint: 42 + tags: front-end: true back-end: false +subchart3: + enabled: false + subchart2alias: - enabled: false \ No newline at end of file + enabled: false -- GitLab