Merge pull request #4783 from derkoe/fix-requirement-condition

fix(pkg/chartutil): conditions for alias and umrella charts (#3734)

docs/charts.md

@@ -258,9 +258,11 @@ All charts are loaded by default. If `tags` or `condition` fields are present,
 they will be evaluated and used to control loading for the chart(s) they are applied to.
 
 Condition - The condition field holds one or more YAML paths (delimited by commas).
-If this path exists in the top parent's values and resolves to a boolean value,
+If this path exists in the parent's values and resolves to a boolean value,
 the chart will be enabled or disabled based on that boolean value. Only the first
-valid path found in the list is evaluated and if no paths exist then the condition has no effect.
+valid path found in the list is evaluated and if no paths exist then the condition 
+has no effect. For multiple level dependencies the condition is prependend by the 
+path to the parent chart.
 
 Tags - The tags field is a YAML list of labels to associate with this chart.
 In the top parent's values, all charts with tags can be enabled or disabled by
@@ -272,7 +274,7 @@ dependencies:
   - name: subchart1
     repository: http://localhost:10191
     version: 0.1.0
-    condition: subchart1.enabled,global.subchart1.enabled
+    condition: subchart1.enabled
     tags:
       - front-end
       - subchart1
@@ -280,11 +282,19 @@ dependencies:
   - name: subchart2
     repository: http://localhost:10191
     version: 0.1.0
-    condition: subchart2.enabled,global.subchart2.enabled
+    condition: subchart2.enabled
     tags:
       - back-end
       - subchart2
+```
 
+```yaml
+# subchart2/requirements.yaml
+dependencies:
+  - name: subsubchart
+    repository: http://localhost:10191
+    version: 0.1.0
+    condition: subsubchart.enabled
 ```
 
 ```yaml
@@ -292,6 +302,9 @@ dependencies:
 
 subchart1:
   enabled: true
+subchart2:
+  subsubchart:
+    enabled: false
 tags:
   front-end: false
   back-end: true
@@ -305,6 +318,9 @@ Since `subchart2` is tagged with `back-end` and that tag evaluates to `true`, `s
 enabled. Also note that although `subchart2` has a condition specified in `requirements.yaml`, there
 is no corresponding path and value in the parent's values so that condition has no effect.
 
+`subsubchart` is disabled by default but can be enabled by setting `subchart2.subsubchart.enabled=true`.
+Hint: disabling `subchart2` via tag will also disable all sub-charts (even if overriding the value `subchart2.subsubchart.enabled=true`).
+
 ##### Using the CLI with Tags and Conditions
 
 The `--set` parameter can be used as usual to alter tag and condition values.

pkg/chartutil/requirements.go

@@ -124,7 +124,7 @@ func LoadRequirementsLock(c *chart.Chart) (*RequirementsLock, error) {
 }
 
 // ProcessRequirementsConditions disables charts based on condition path value in values
-func ProcessRequirementsConditions(reqs *Requirements, cvals Values) {
+func ProcessRequirementsConditions(reqs *Requirements, cvals Values, cpath string) {
 	var cond string
 	var conds []string
 	if reqs == nil || len(reqs.Dependencies) == 0 {
@@ -143,7 +143,7 @@ func ProcessRequirementsConditions(reqs *Requirements, cvals Values) {
 			for _, c := range conds {
 				if len(c) > 0 {
 					// retrieve value
-					vv, err := cvals.PathValue(c)
+					vv, err := cvals.PathValue(cpath + c)
 					if err == nil {
 						// if not bool, warn
 						if bv, ok := vv.(bool); ok {
@@ -247,6 +247,10 @@ func getAliasDependency(charts []*chart.Chart, aliasChart *Dependency) *chart.Ch
 
 // ProcessRequirementsEnabled removes disabled charts from dependencies
 func ProcessRequirementsEnabled(c *chart.Chart, v *chart.Config) error {
+	return doProcessRequirementsEnabled(c, v, "")
+}
+
+func doProcessRequirementsEnabled(c *chart.Chart, v *chart.Config, path string) error {
 	reqs, err := LoadRequirements(c)
 	if err != nil {
 		// if not just missing requirements file, return error
@@ -303,7 +307,7 @@ func ProcessRequirementsEnabled(c *chart.Chart, v *chart.Config) error {
 	cc := chart.Config{Raw: yvals}
 	// flag dependencies as enabled/disabled
 	ProcessRequirementsTags(reqs, cvals)
-	ProcessRequirementsConditions(reqs, cvals)
+	ProcessRequirementsConditions(reqs, cvals, path)
 	// make a map of charts to remove
 	rm := map[string]bool{}
 	for _, r := range reqs.Dependencies {
@@ -323,7 +327,8 @@ func ProcessRequirementsEnabled(c *chart.Chart, v *chart.Config) error {
 	}
 	// recursively call self to process sub dependencies
 	for _, t := range cd {
-		err := ProcessRequirementsEnabled(t, &cc)
+		subpath := path + t.Metadata.Name + "."
+		err := doProcessRequirementsEnabled(t, &cc, subpath)
 		// if its not just missing requirements file, return error
 		if nerr, ok := err.(ErrNoRequirementsFile); !ok && err != nil {
 			return nerr

pkg/chartutil/requirements_test.go

@@ -158,7 +158,7 @@ func TestRequirementsCombinedDisabledL2(t *testing.T) {
 		t.Fatalf("Failed to load testdata: %s", err)
 	}
 	// tags enabling a parent/child group with condition disabling one child
-	v := &chart.Config{Raw: "subchartc:\n  enabled: false\ntags:\n  back-end: true\n"}
+	v := &chart.Config{Raw: "subchart2:\n  subchartc:\n    enabled: false\ntags:\n  back-end: true\n"}
 	// expected charts including duplicates in alphanumeric order
 	e := []string{"parentchart", "subchart1", "subchart2", "subcharta", "subchartb", "subchartb"}
 
@@ -176,6 +176,15 @@ func TestRequirementsCombinedDisabledL1(t *testing.T) {
 
 	verifyRequirementsEnabled(t, c, v, e)
 }
+func TestRequirementsAliasCondition(t *testing.T) {
+	c, err := Load("testdata/subpop")
+	if err != nil {
+		t.Fatalf("Failed to load testdata: %s", err)
+	}
+	v := &chart.Config{Raw: "subchart1:\n  enabled: false\nsubchart2alias:\n  enabled: true\n  subchartb:\n    enabled: true\n"}
+	e := []string{"parentchart", "subchart2alias", "subchartb"}
+	verifyRequirementsEnabled(t, c, v, e)
+}
 
 func verifyRequirementsEnabled(t *testing.T, c *chart.Chart, v *chart.Config, e []string) {
 	out := []*chart.Chart{}

pkg/chartutil/testdata/subpop/charts/subchart1/requirements.yaml

@@ -2,7 +2,7 @@ dependencies:
       - name: subcharta
         repository: http://localhost:10191
         version: 0.1.0
-        condition: subcharta.enabled,subchart1.subcharta.enabled
+        condition: subcharta.enabled
         tags:
           - front-end
           - subcharta

pkg/chartutil/testdata/subpop/charts/subchart2/requirements.yaml

@@ -2,7 +2,7 @@ dependencies:
       - name: subchartb
         repository: http://localhost:10191
         version: 0.1.0
-        condition: subchartb.enabled,subchart2.subchartb.enabled
+        condition: subchartb.enabled
         tags:
           - back-end
           - subchartb

pkg/chartutil/testdata/subpop/requirements.yaml

@@ -29,3 +29,9 @@ dependencies:
         tags:
           - back-end
           - subchart2
+
+      - name: subchart2
+        alias: subchart2alias
+        repository: http://localhost:10191
+        version: 0.1.0
+        condition: subchart2alias.enabled

pkg/chartutil/testdata/subpop/values.yaml

@@ -39,3 +39,5 @@ tags:
   front-end: true
   back-end: false
 
+subchart2alias:
+  enabled: false