diff --git a/pkg/chartutil/dependencies.go b/pkg/chartutil/dependencies.go index d2e7d6dc9..1dac34b0a 100644 --- a/pkg/chartutil/dependencies.go +++ b/pkg/chartutil/dependencies.go @@ -16,6 +16,7 @@ limitations under the License. package chartutil import ( + "go/build/constraint" "log" "strings" @@ -31,32 +32,100 @@ func ProcessDependencies(c *chart.Chart, v Values) error { } // processDependencyConditions disables charts based on condition path value in values +// +// Adapted from git@github.com:golang/go.git src/go/build/constraint/expr.go:394 +// +// In order to be compatible with the previous design, this is a bit different +// from the tag resolve logic of go. +// +// ' ' for OR, ',' for AND, '!' prefix for neg +// +// If an expr is not specified, it is not false, but null, that is, it will be ignored +// +// !cond1,cond2 cond3 +// cond1 = true, cond2 = true, cond3 = true +// Usually means (false && true || true) -> true +// +// but if +// cond1 = false, cond2 = null, cond3 = null +// the result is (true && null || null) -> true +// but not (true && false || false) -> false func processDependencyConditions(reqs []*chart.Dependency, cvals Values, cpath string) { if reqs == nil { return } for _, r := range reqs { - for _, c := range strings.Split(strings.TrimSpace(r.Condition), ",") { - if len(c) > 0 { + + var x constraint.Expr + for _, clause := range strings.Fields(r.Condition) { + var y constraint.Expr + for _, lit := range strings.Split(clause, ",") { + var z constraint.Expr + var neg bool + if strings.HasPrefix(lit, "!") { + lit = lit[len("!"):] + neg = true + } + // retrieve value - vv, err := cvals.PathValue(cpath + c) + vv, err := cvals.PathValue(cpath + lit) if err == nil { // if not bool, warn - if bv, ok := vv.(bool); ok { - r.Enabled = bv - break + if _, ok := vv.(bool); ok { + z = tag(lit) + if neg { + z = not(z) + } } else { - log.Printf("Warning: Condition path '%s' for chart %s returned non-bool value", c, r.Name) + log.Printf("Warning: Condition path '%s' for chart %s returned non-bool value", lit, r.Name) } } else if _, ok := err.(ErrNoValue); !ok { // this is a real error log.Printf("Warning: PathValue returned error %v", err) } + + if z != nil { + if y == nil { + y = z + } else { + y = and(y, z) + } + } + } + + if y != nil { + if x == nil { + x = y + } else { + x = or(x, y) + } + } + + } + + if x != nil { + r.Enabled = x.Eval( + func(tag string) bool { + + // This value has been checked before, so it is unlikely that an error will be reported here + vv, _ := cvals.PathValue(cpath + tag) + if bv, ok := vv.(bool); ok { + return bv + } + + return false + }, + ) } } } +func tag(tag string) constraint.Expr { return &constraint.TagExpr{Tag: tag} } +func not(x constraint.Expr) constraint.Expr { return &constraint.NotExpr{X: x} } +func and(x, y constraint.Expr) constraint.Expr { return &constraint.AndExpr{X: x, Y: y} } +func or(x, y constraint.Expr) constraint.Expr { return &constraint.OrExpr{X: x, Y: y} } + // processDependencyTags disables charts based on tags in values func processDependencyTags(reqs []*chart.Dependency, cvals Values) { if reqs == nil { diff --git a/pkg/chartutil/dependencies_test.go b/pkg/chartutil/dependencies_test.go index bcb1d40e5..2154157af 100644 --- a/pkg/chartutil/dependencies_test.go +++ b/pkg/chartutil/dependencies_test.go @@ -269,6 +269,134 @@ func TestProcessDependencyImportValuesForEnabledCharts(t *testing.T) { } } +func TestProcessDependencyImportComplexValuesForEnabledCharts(t *testing.T) { + + // # dev || !condition1 || condition2 && condition3 || condition 4 + // condition: dev.enabled !condition1.enabled condition2.enabled,condition3.enabled condition4.enabled + + // # prod && !condition4 || condition1 + // condition: prod.enabled,!condition4.enabled condition1.enabled + + type M = map[string]interface{} + tests := []struct { + name string + v M + e []string // expected charts including duplicates in alphanumeric order + }{{ + // dev: null + // prod: null + "tags with no effect", + M{"tags": M{"nothinguseful": false}}, + []string{"parent-chart", "parent-chart.dev", "parent-chart.prod"}, + }, { + // dev: dev + // prod: null + "tags with enable dev", + M{"dev": M{"enabled": true}}, + []string{"parent-chart", "parent-chart.dev", "parent-chart.prod"}, + }, { + // dev: null -> true + // prod: prod -> true + "tags with enable prod", + M{"prod": M{"enabled": true}}, + []string{"parent-chart", "parent-chart.dev", "parent-chart.prod"}, + }, { + // dev: !condition1 -> !true + // prod: condition1 -> true + "tags with enable condition1", + M{"condition1": M{"enabled": true}}, + []string{"parent-chart", "parent-chart.prod"}, + }, { + // dev: !condition1 || condition2 -> !true || true + // prod: condition1 -> true + "tags with enable condition1 and condition2", + M{"condition1": M{"enabled": true}, "condition2": M{"enabled": true}}, + []string{"parent-chart", "parent-chart.dev", "parent-chart.prod"}, + }, { + // dev: !condition1 || condition2 && condition3 -> !true || true && true + // prod: condition1 -> true + "tags with enable condition1, condition2 and condition3", + M{"condition1": M{"enabled": true}, "condition2": M{"enabled": true}, "condition3": M{"enabled": true}}, + []string{"parent-chart", "parent-chart.dev", "parent-chart.prod"}, + }, { + // dev: !condition1 || condition2 && condition3 -> !true || true && false + // prod: condition1 true + "tags with enable condition1 and condition2, disable condition3", + M{"condition1": M{"enabled": true}, "condition2": M{"enabled": true}, "condition3": M{"enabled": false}}, + []string{"parent-chart", "parent-chart.prod"}, + }, { + // dev: !condition1 || condition2 && condition3 -> !false || true && true + // prod: condition1 -> false + "tags with enable condition2 and condition3, disable condition1", + M{"condition1": M{"enabled": false}, "condition2": M{"enabled": true}, "condition3": M{"enabled": true}}, + []string{"parent-chart", "parent-chart.dev"}, + }, { + // dev: !condition1 || condition2 && condition3 -> !true || false && true + // prod: condition1 -> true + "tags with enable condition1 and condition3, disable condition2", + M{"condition1": M{"enabled": true}, "condition2": M{"enabled": false}, "condition3": M{"enabled": true}}, + []string{"parent-chart", "parent-chart.prod"}, + }, { + // dev: !condition1 || condition2 && condition3 || condition 4 -> !true || true && true || true + // prod: !condition 4 || condition1 -> !true || true + "tags with enable condition1, condition2, condition3 and condition4", + M{"condition1": M{"enabled": true}, "condition2": M{"enabled": true}, "condition3": M{"enabled": true}, "condition4": M{"enabled": true}}, + []string{"parent-chart", "parent-chart.dev", "parent-chart.prod"}, + }, { + // dev: !condition1 || condition2 && condition3 || condition 4 -> !true || true && true || false + // prod: !condition 4 || condition1 -> !false || true + "tags with enable condition1, condition2 and condition3, disable condition4", + M{"condition1": M{"enabled": true}, "condition2": M{"enabled": true}, "condition3": M{"enabled": true}, "condition4": M{"enabled": false}}, + []string{"parent-chart", "parent-chart.dev", "parent-chart.prod"}, + }, { + // dev: !condition1 || condition2 && condition3 || condition 4 -> !true || true && false || false + // prod: !condition 4 || condition1 -> !false || true + "tags with enable condition1 and condition2, disable condition3 and condition4", + M{"condition1": M{"enabled": true}, "condition2": M{"enabled": true}, "condition3": M{"enabled": false}, "condition4": M{"enabled": false}}, + []string{"parent-chart", "parent-chart.prod"}, + }, { + // dev: !condition1 || condition2 && condition3 || condition 4 -> !true || false && true || false + // prod: !condition 4 || condition1 -> !false || true + "tags with enable condition1 and condition3, disable condition2 and condition4", + M{"condition1": M{"enabled": true}, "condition2": M{"enabled": false}, "condition3": M{"enabled": true}, "condition4": M{"enabled": false}}, + []string{"parent-chart", "parent-chart.prod"}, + }, { + // dev: !condition1 || condition2 && condition3 || condition 4 -> !false || true && true || false + // prod: !condition 4 || condition1 -> !false || false + "tags with enable condition2 and condition3, disable condition1 and condition4", + M{"condition1": M{"enabled": false}, "condition2": M{"enabled": true}, "condition3": M{"enabled": true}, "condition4": M{"enabled": false}}, + []string{"parent-chart", "parent-chart.dev", "parent-chart.prod"}, + }, { + // dev: dev || !condition1 || condition2 && condition3 || condition 4 -> true || !false || true && true || true + // prod: prod && !condition 4 || condition1 -> true || !true || false + "tags with enable all and disable 1", + M{"condition1": M{"enabled": false}, "condition2": M{"enabled": true}, "condition3": M{"enabled": true}, "condition4": M{"enabled": true}, "prod": M{"enabled": true}, "dev": M{"enabled": false}}, + []string{"parent-chart", "parent-chart.dev"}, + }, + } + + for _, tc := range tests { + c := loadChart(t, "testdata/import-complex-values-from-enabled-subchart/parent-chart") + t.Run( + tc.name, func(t *testing.T) { + if err := processDependencyEnabled(c, tc.v, ""); err != nil { + t.Fatalf("error processing enabled dependencies %v", err) + } + + names := extractChartNames(c) + if len(names) != len(tc.e) { + t.Fatalf("slice lengths do not match got %v, expected %v", len(names), len(tc.e)) + } + for i := range names { + if names[i] != tc.e[i] { + t.Fatalf("slice values do not match got %v, expected %v", names, tc.e) + } + } + }, + ) + } +} + func TestGetAliasDependency(t *testing.T) { c := loadChart(t, "testdata/frobnitz") req := c.Metadata.Dependencies diff --git a/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/Chart.lock b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/Chart.lock new file mode 100644 index 000000000..b2f17fb39 --- /dev/null +++ b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/Chart.lock @@ -0,0 +1,9 @@ +dependencies: +- name: dev + repository: file://envs/dev + version: v0.1.0 +- name: prod + repository: file://envs/prod + version: v0.1.0 +digest: sha256:9403fc24f6cf9d6055820126cf7633b4bd1fed3c77e4880c674059f536346182 +generated: "2020-02-03T10:38:51.180474+01:00" diff --git a/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/Chart.yaml b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/Chart.yaml new file mode 100644 index 000000000..b13d3415b --- /dev/null +++ b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/Chart.yaml @@ -0,0 +1,32 @@ +apiVersion: v2 +name: parent-chart +version: v0.1.0 +appVersion: v0.1.0 +dependencies: + - name: dev + repository: "file://envs/dev" + version: ">= 0.0.1" + + # dev || !condition1 || condition2 && condition3 || condition 4 + condition: dev.enabled !condition1.enabled condition2.enabled,condition3.enabled condition4.enabled + tags: + - dev + - condition1 + - condition2 + - condition3 + - condition4 + import-values: + - data + + - name: prod + repository: "file://envs/prod" + version: ">= 0.0.1" + + # prod && !condition4 || condition1 + condition: prod.enabled,!condition4.enabled condition1.enabled + tags: + - prod + - condition1 + - condition4 + import-values: + - data \ No newline at end of file diff --git a/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/charts/dev-v0.1.0.tgz b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/charts/dev-v0.1.0.tgz new file mode 100644 index 000000000..d28e1621c Binary files /dev/null and b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/charts/dev-v0.1.0.tgz differ diff --git a/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/charts/prod-v0.1.0.tgz b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/charts/prod-v0.1.0.tgz new file mode 100644 index 000000000..a0c5aa84b Binary files /dev/null and b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/charts/prod-v0.1.0.tgz differ diff --git a/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/envs/dev/Chart.yaml b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/envs/dev/Chart.yaml new file mode 100644 index 000000000..80a52f538 --- /dev/null +++ b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/envs/dev/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v2 +name: dev +version: v0.1.0 +appVersion: v0.1.0 \ No newline at end of file diff --git a/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/envs/dev/values.yaml b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/envs/dev/values.yaml new file mode 100644 index 000000000..38f03484d --- /dev/null +++ b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/envs/dev/values.yaml @@ -0,0 +1,9 @@ +# Dev values parent-chart +nameOverride: parent-chart-dev +exports: + data: + resources: + autoscaler: + minReplicas: 1 + maxReplicas: 3 + targetCPUUtilizationPercentage: 80 diff --git a/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/envs/prod/Chart.yaml b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/envs/prod/Chart.yaml new file mode 100644 index 000000000..bda4be458 --- /dev/null +++ b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/envs/prod/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v2 +name: prod +version: v0.1.0 +appVersion: v0.1.0 \ No newline at end of file diff --git a/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/envs/prod/values.yaml b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/envs/prod/values.yaml new file mode 100644 index 000000000..10cc756b2 --- /dev/null +++ b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/envs/prod/values.yaml @@ -0,0 +1,9 @@ +# Prod values parent-chart +nameOverride: parent-chart-prod +exports: + data: + resources: + autoscaler: + minReplicas: 2 + maxReplicas: 5 + targetCPUUtilizationPercentage: 90 diff --git a/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/templates/autoscaler.yaml b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/templates/autoscaler.yaml new file mode 100644 index 000000000..976e5a8f1 --- /dev/null +++ b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/templates/autoscaler.yaml @@ -0,0 +1,16 @@ +################################################################################################### +# parent-chart horizontal pod autoscaler +################################################################################################### +apiVersion: autoscaling/v1 +kind: HorizontalPodAutoscaler +metadata: + name: {{ .Release.Name }}-autoscaler + namespace: {{ .Release.Namespace }} +spec: + scaleTargetRef: + apiVersion: apps/v1beta1 + kind: Deployment + name: {{ .Release.Name }} + minReplicas: {{ required "A valid .Values.resources.autoscaler.minReplicas entry required!" .Values.resources.autoscaler.minReplicas }} + maxReplicas: {{ required "A valid .Values.resources.autoscaler.maxReplicas entry required!" .Values.resources.autoscaler.maxReplicas }} + targetCPUUtilizationPercentage: {{ required "A valid .Values.resources.autoscaler.targetCPUUtilizationPercentage!" .Values.resources.autoscaler.targetCPUUtilizationPercentage }} \ No newline at end of file diff --git a/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/values.yaml b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/values.yaml new file mode 100644 index 000000000..225ca9b23 --- /dev/null +++ b/pkg/chartutil/testdata/import-complex-values-from-enabled-subchart/parent-chart/values.yaml @@ -0,0 +1,8 @@ +# Default values for parent-chart. +nameOverride: parent-chart + +resources: + autoscaler: + minReplicas: 0 + maxReplicas: 0 + targetCPUUtilizationPercentage: 99 \ No newline at end of file