breaking: Fix the inherit feature to support multi-inheritance (#674)

* breaking: Fix the inherit feature introduced in Helmfile v0.150.0 for multi-inheritance

Follow-up for #435
Addresses https://github.com/helmfile/helmfile/discussions/656#discussioncomment-4877360 towards Helmfile v1

Signed-off-by: Yusuke Kuoka <ykuoka@gmail.com>

* Print a deprecation warning on releases[].inherit of map

Signed-off-by: Yusuke Kuoka <ykuoka@gmail.com>

---------

Signed-off-by: Yusuke Kuoka <ykuoka@gmail.com>

pkg/app/app_template_test.go

@@ -435,23 +435,23 @@ func TestTemplate_CyclicInheritance(t *testing.T) {
 templates:
   a:
     inherit:
-      template: b
+    - template: b
     values:
     - a.yaml
   b:
     inherit:
-      template: c
+    - template: c
     values:
     - b.yaml
   c:
     inherit:
-      template: a
+    - template: a
     values:
     - c.yaml
 releases:
 - name: app1
   inherit:
-    template: a
+  - template: a
   chart: incubator/raw
 `,
 			}

pkg/state/state.go

@@ -213,6 +213,8 @@ type Inherit struct {
 	Except   []string `yaml:"except,omitempty"`
 }
 
+type Inherits []Inherit
+
 // ReleaseSpec defines the structure of a helm release
 type ReleaseSpec struct {
 	// Chart is the name of the chart being installed to create this release
@@ -353,7 +355,22 @@ type ReleaseSpec struct {
 	PostRenderer *string `yaml:"postRenderer,omitempty"`
 
 	// Inherit is used to inherit a release template from a release or another release template
-	Inherit Inherit `yaml:"inherit,omitempty"`
+	Inherit Inherits `yaml:"inherit,omitempty"`
+}
+
+func (r *Inherits) UnmarshalYAML(unmarshal func(interface{}) error) error {
+	var v0151 []Inherit
+	if err := unmarshal(&v0151); err != nil {
+		var v0150 Inherit
+		if err := unmarshal(&v0150); err != nil {
+			return err
+		}
+		fmt.Fprintf(os.Stderr, "releases[].inherit of map(%+v) has been deprecated and will be removed in v0.152.0. Wrap it into an array: %v\n", v0150, err)
+		*r = []Inherit{v0150}
+		return nil
+	}
+	*r = v0151
+	return nil
 }
 
 // ChartPathOrName returns ChartPath if it is non-empty, and returns Chart otherwise.

pkg/state/state_exec_tmpl.go

@@ -154,66 +154,68 @@ func (e CyclicReleaseTemplateInheritanceError) Error() string {
 // a cyclic relese template inheritance.
 // This functions fails with a CyclicReleaseTemplateInheritanceError if it finds a cyclic inheritance.
 func (st *HelmState) releaseWithInheritedTemplate(r *ReleaseSpec, inheritancePath []string) (*ReleaseSpec, error) {
-	templateName := r.Inherit.Template
-	if templateName == "" {
-		return r, nil
-	}
-
-	path := append([]string{}, inheritancePath...)
-	path = append(path, templateName)
-
-	var cycleFound bool
-	for _, t := range inheritancePath {
-		if t == templateName {
-			cycleFound = true
-			break
-		}
-	}
-
-	if cycleFound {
-		return nil, CyclicReleaseTemplateInheritanceError{Message: fmt.Sprintf("cyclic inheritance detected: %s", strings.Join(path, "->"))}
-	}
-
-	template, defined := st.Templates[templateName]
-	if !defined {
-		return nil, fmt.Errorf("release %q tried to inherit inexistent release template %q", r.Name, templateName)
-	}
-
-	src, err := st.releaseWithInheritedTemplate(&template.ReleaseSpec, path)
-	if err != nil {
-		return nil, fmt.Errorf("unable to load release template %q: %w", templateName, err)
-	}
-
-	for _, k := range r.Inherit.Except {
-		switch k {
-		case "labels":
-			src.Labels = map[string]string{}
-		case "values":
-			src.Values = nil
-		case "valuesTemplate":
-			src.ValuesTemplate = nil
-		case "setTemplate":
-			src.SetValuesTemplate = nil
-		case "set":
-			src.SetValues = nil
-		default:
-			return nil, fmt.Errorf("%q is not allowed under `inherit`. Allowed values are \"set\", \"setTemplate\", \"values\", \"valuesTemplate\", and \"labels\"", k)
-		}
-
-		st.logger.Debugf("excluded field %q when inheriting template %q to release %q", k, templateName, r.Name)
-	}
-
 	var merged ReleaseSpec
 
-	if err := mergo.Merge(&merged, src, mergo.WithAppendSlice, mergo.WithSliceDeepCopy); err != nil {
-		return nil, fmt.Errorf("unable to inherit release template %q: %w", templateName, err)
+	for _, inherit := range r.Inherit {
+		templateName := inherit.Template
+		if templateName == "" {
+			return r, nil
+		}
+
+		path := append([]string{}, inheritancePath...)
+		path = append(path, templateName)
+
+		var cycleFound bool
+		for _, t := range inheritancePath {
+			if t == templateName {
+				cycleFound = true
+				break
+			}
+		}
+
+		if cycleFound {
+			return nil, CyclicReleaseTemplateInheritanceError{Message: fmt.Sprintf("cyclic inheritance detected: %s", strings.Join(path, "->"))}
+		}
+
+		template, defined := st.Templates[templateName]
+		if !defined {
+			return nil, fmt.Errorf("release %q tried to inherit inexistent release template %q", r.Name, templateName)
+		}
+
+		src, err := st.releaseWithInheritedTemplate(&template.ReleaseSpec, path)
+		if err != nil {
+			return nil, fmt.Errorf("unable to load release template %q: %w", templateName, err)
+		}
+
+		for _, k := range inherit.Except {
+			switch k {
+			case "labels":
+				src.Labels = map[string]string{}
+			case "values":
+				src.Values = nil
+			case "valuesTemplate":
+				src.ValuesTemplate = nil
+			case "setTemplate":
+				src.SetValuesTemplate = nil
+			case "set":
+				src.SetValues = nil
+			default:
+				return nil, fmt.Errorf("%q is not allowed under `inherit`. Allowed values are \"set\", \"setTemplate\", \"values\", \"valuesTemplate\", and \"labels\"", k)
+			}
+
+			st.logger.Debugf("excluded field %q when inheriting template %q to release %q", k, templateName, r.Name)
+		}
+
+		if err := mergo.Merge(&merged, src, mergo.WithAppendSlice, mergo.WithSliceDeepCopy); err != nil {
+			return nil, fmt.Errorf("unable to inherit release template %q: %w", templateName, err)
+		}
 	}
 
 	if err := mergo.Merge(&merged, r, mergo.WithAppendSlice, mergo.WithSliceDeepCopy); err != nil {
 		return nil, fmt.Errorf("unable to load release %q: %w", r.Name, err)
 	}
 
-	merged.Inherit = Inherit{}
+	merged.Inherit = nil
 
 	return &merged, nil
 }

pkg/state/temp_test.go

@@ -38,39 +38,39 @@ func TestGenerateID(t *testing.T) {
 	run(testcase{
 		subject: "baseline",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
-		want:    "foo-values-5d85cdbb5d",
+		want:    "foo-values-7cd4757dfd",
 	})
 
 	run(testcase{
 		subject: "different bytes content",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
 		data:    []byte(`{"k":"v"}`),
-		want:    "foo-values-9548bdcd9",
+		want:    "foo-values-7bb85f848f",
 	})
 
 	run(testcase{
 		subject: "different map content",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
 		data:    map[string]interface{}{"k": "v"},
-		want:    "foo-values-5db884fb87",
+		want:    "foo-values-c57649655",
 	})
 
 	run(testcase{
 		subject: "different chart",
 		release: ReleaseSpec{Name: "foo", Chart: "stable/envoy"},
-		want:    "foo-values-6596c997cc",
+		want:    "foo-values-bf798c8f",
 	})
 
 	run(testcase{
 		subject: "different name",
 		release: ReleaseSpec{Name: "bar", Chart: "incubator/raw"},
-		want:    "bar-values-5ccfd9f8b5",
+		want:    "bar-values-5465f47b4f",
 	})
 
 	run(testcase{
 		subject: "specific ns",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw", Namespace: "myns"},
-		want:    "myns-foo-values-bc7cd5b87",
+		want:    "myns-foo-values-55f77767f5",
 	})
 
 	for id, n := range ids {

test/e2e/template/helmfile/testdata/snapshot/release_template_inheritance/input.yaml

@@ -17,7 +17,7 @@ templates:
     labels:
       template1: template1
     inherit:
-      template: base
+    - template: base
       except:
       - labels
   template2:
@@ -29,14 +29,14 @@ templates:
     labels:
       template2: template2
     inherit:
-      template: base
+    - template: base
       except:
       - valuesTemplate
 
 releases:
 - name: foo1
   inherit:
-    template: template1
+  - template: template1
   values:
   - templates:
     - |
@@ -49,7 +49,7 @@ releases:
         {{` {{ (unset .Values "templates") | toYaml | nindent 2 }} `}}
 - name: foo2
   inherit:
-    template: template2
+  - template: template2
   values:
   - templates:
     - |