From 6057585fa442f16704c030f5acc4decc7319356b Mon Sep 17 00:00:00 2001 From: Suleiman Dibirov Date: Fri, 9 Feb 2024 19:54:00 +0200 Subject: [PATCH] fix: added list values merging mechanism Signed-off-by: Suleiman Dibirov --- cmd/helm/template_test.go | 11 +++-- ...late-chart-with-ingress-with-list-host.txt | 12 ++++++ ...plate-chart-with-ingress-with-map-host.txt | 39 ++++++++++++++++++ .../.helmignore | 21 ++++++++++ .../Chart.yaml | 6 +++ .../templates/ingress.yaml | 27 ++++++++++++ .../values.yaml | 14 +++++++ .../.helmignore | 21 ++++++++++ .../Chart.yaml | 6 +++ .../templates/ingress.yaml | 38 +++++++++++++++++ .../values.yaml | 26 ++++++++++++ pkg/chartutil/coalesce.go | 41 +++++++++++++++++++ pkg/chartutil/values.go | 6 +++ 13 files changed, 265 insertions(+), 3 deletions(-) create mode 100644 cmd/helm/testdata/output/template-chart-with-ingress-with-list-host.txt create mode 100644 cmd/helm/testdata/output/template-chart-with-ingress-with-map-host.txt create mode 100644 cmd/helm/testdata/testcharts/chart-with-ingress-with-list-host/.helmignore create mode 100644 cmd/helm/testdata/testcharts/chart-with-ingress-with-list-host/Chart.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-ingress-with-list-host/templates/ingress.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-ingress-with-list-host/values.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-ingress-with-map-host/.helmignore create mode 100644 cmd/helm/testdata/testcharts/chart-with-ingress-with-map-host/Chart.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-ingress-with-map-host/templates/ingress.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-ingress-with-map-host/values.yaml diff --git a/cmd/helm/template_test.go b/cmd/helm/template_test.go index e5b939879..c63c14545 100644 --- a/cmd/helm/template_test.go +++ b/cmd/helm/template_test.go @@ -68,11 +68,16 @@ func TestTemplateCmd(t *testing.T) { }, { name: "check chart with dependency which is an app chart acting as a library chart", - cmd: fmt.Sprintf("template '%s'", "testdata/testcharts/chart-with-template-lib-dep"), - golden: "output/template-chart-with-template-lib-dep.txt", + cmd: fmt.Sprintf("template '%s' --set ingress.enabled=true --set ingress.hosts[1].host=changed_host.com", "testdata/testcharts/chart-with-ingress-with-map-host"), + golden: "output/template-chart-with-ingress-with-map-host.txt", }, { - name: "check chart with dependency which is an app chart archive acting as a library chart", + name: "check chart with map type ingress host and set ingress.hosts[1].host=changed_host.com", + cmd: fmt.Sprintf("template '%s' --set ingress.enabled=true --set ingress.hosts[1].host=changed_host.com", "testdata/testcharts/chart-with-ingress-with-list-host"), + golden: "output/template-chart-with-ingress-with-list-host.txt", + }, + { + name: "check chart with list type ingress host and set ingress.hosts[1].host=changed_host.com", cmd: fmt.Sprintf("template '%s'", "testdata/testcharts/chart-with-template-lib-archive-dep"), golden: "output/template-chart-with-template-lib-archive-dep.txt", }, diff --git a/cmd/helm/testdata/output/template-chart-with-ingress-with-list-host.txt b/cmd/helm/testdata/output/template-chart-with-ingress-with-list-host.txt new file mode 100644 index 000000000..f81ea04df --- /dev/null +++ b/cmd/helm/testdata/output/template-chart-with-ingress-with-list-host.txt @@ -0,0 +1,12 @@ +--- +# Source: chart-with-ingress-with-list-host/templates/ingress.yaml +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: chart-with-ingress-with-list-host +spec: + ingressClassName: myclassname + rules: + - host: "myhost1.local" + - host: "changed_host.com" + - host: "myhost3.local" diff --git a/cmd/helm/testdata/output/template-chart-with-ingress-with-map-host.txt b/cmd/helm/testdata/output/template-chart-with-ingress-with-map-host.txt new file mode 100644 index 000000000..d1ee1c005 --- /dev/null +++ b/cmd/helm/testdata/output/template-chart-with-ingress-with-map-host.txt @@ -0,0 +1,39 @@ +--- +# Source: chart-with-ingress-with-map-host/templates/ingress.yaml +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: chart-with-ingress-with-map-host +spec: + ingressClassName: myclassname + rules: + - host: "myhost1.local" + http: + paths: + - path: / + pathType: mypathtype1 + backend: + service: + name: mypath1 + port: + number: 9898 + - host: "changed_host.com" + http: + paths: + - path: / + pathType: mypathtype2 + backend: + service: + name: mypath2 + port: + number: 9898 + - host: "myhost3.local" + http: + paths: + - path: / + pathType: mypathtype3 + backend: + service: + name: mypath3 + port: + number: 9898 diff --git a/cmd/helm/testdata/testcharts/chart-with-ingress-with-list-host/.helmignore b/cmd/helm/testdata/testcharts/chart-with-ingress-with-list-host/.helmignore new file mode 100644 index 000000000..f0c131944 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-ingress-with-list-host/.helmignore @@ -0,0 +1,21 @@ +# 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 diff --git a/cmd/helm/testdata/testcharts/chart-with-ingress-with-list-host/Chart.yaml b/cmd/helm/testdata/testcharts/chart-with-ingress-with-list-host/Chart.yaml new file mode 100644 index 000000000..44059cd86 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-ingress-with-list-host/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v1 +appVersion: "1.0" +description: A Helm chart for Kubernetes +name: chart-with-ingress-with-list-host +type: application +version: 0.1.0 diff --git a/cmd/helm/testdata/testcharts/chart-with-ingress-with-list-host/templates/ingress.yaml b/cmd/helm/testdata/testcharts/chart-with-ingress-with-list-host/templates/ingress.yaml new file mode 100644 index 000000000..99803d6b6 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-ingress-with-list-host/templates/ingress.yaml @@ -0,0 +1,27 @@ +{{- if .Values.ingress.enabled -}} +{{- $svcPort := .Values.service.externalPort -}} +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: {{ .Chart.Name }} + {{- with .Values.ingress.annotations }} + annotations: + {{- toYaml . | nindent 4 }} + {{- end }} +spec: + ingressClassName: {{ .Values.ingress.className }} + {{- if .Values.ingress.tls }} + tls: + {{- range .Values.ingress.tls }} + - hosts: + {{- range .hosts }} + - {{ . | quote }} + {{- end }} + secretName: {{ .secretName }} + {{- end }} + {{- end }} + rules: + {{- range .Values.ingress.hosts }} + - host: {{ .host | quote }} + {{- end }} +{{- end }} diff --git a/cmd/helm/testdata/testcharts/chart-with-ingress-with-list-host/values.yaml b/cmd/helm/testdata/testcharts/chart-with-ingress-with-list-host/values.yaml new file mode 100644 index 000000000..fc7c64b00 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-ingress-with-list-host/values.yaml @@ -0,0 +1,14 @@ +# Kubernetes Service settings +service: + externalPort: 9898 + +ingress: + enabled: true + className: myclassname + additionalLabels: {} + annotations: {} + hosts: + - host: myhost1.local + - host: myhost2.local + - host: myhost3.local + tls: [] diff --git a/cmd/helm/testdata/testcharts/chart-with-ingress-with-map-host/.helmignore b/cmd/helm/testdata/testcharts/chart-with-ingress-with-map-host/.helmignore new file mode 100644 index 000000000..f0c131944 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-ingress-with-map-host/.helmignore @@ -0,0 +1,21 @@ +# 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 diff --git a/cmd/helm/testdata/testcharts/chart-with-ingress-with-map-host/Chart.yaml b/cmd/helm/testdata/testcharts/chart-with-ingress-with-map-host/Chart.yaml new file mode 100644 index 000000000..fd2055766 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-ingress-with-map-host/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v1 +appVersion: "1.0" +description: A Helm chart for Kubernetes +name: chart-with-ingress-with-map-host +type: application +version: 0.1.0 diff --git a/cmd/helm/testdata/testcharts/chart-with-ingress-with-map-host/templates/ingress.yaml b/cmd/helm/testdata/testcharts/chart-with-ingress-with-map-host/templates/ingress.yaml new file mode 100644 index 000000000..b358da545 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-ingress-with-map-host/templates/ingress.yaml @@ -0,0 +1,38 @@ +{{- if .Values.ingress.enabled -}} +{{- $svcPort := .Values.service.externalPort -}} +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: {{ .Chart.Name }} + {{- with .Values.ingress.annotations }} + annotations: + {{- toYaml . | nindent 4 }} + {{- end }} +spec: + ingressClassName: {{ .Values.ingress.className }} + {{- if .Values.ingress.tls }} + tls: + {{- range .Values.ingress.tls }} + - hosts: + {{- range .hosts }} + - {{ . | quote }} + {{- end }} + secretName: {{ .secretName }} + {{- end }} + {{- end }} + rules: + {{- range .Values.ingress.hosts }} + - host: {{ .host | quote }} + http: + paths: + {{- range .paths }} + - path: {{ .path }} + pathType: {{ .pathType }} + backend: + service: + name: {{ .name }} + port: + number: {{ $svcPort }} + {{- end }} + {{- end }} +{{- end }} diff --git a/cmd/helm/testdata/testcharts/chart-with-ingress-with-map-host/values.yaml b/cmd/helm/testdata/testcharts/chart-with-ingress-with-map-host/values.yaml new file mode 100644 index 000000000..8b21c06b2 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-ingress-with-map-host/values.yaml @@ -0,0 +1,26 @@ +# Kubernetes Service settings +service: + externalPort: 9898 + +ingress: + enabled: true + className: myclassname + additionalLabels: {} + annotations: {} + hosts: + - host: myhost1.local + paths: + - path: / + name: mypath1 + pathType: mypathtype1 + - host: myhost2.local + paths: + - path: / + name: mypath2 + pathType: mypathtype2 + - host: myhost3.local + paths: + - path: / + name: mypath3 + pathType: mypathtype3 + tls: [] diff --git a/pkg/chartutil/coalesce.go b/pkg/chartutil/coalesce.go index df6cae6cb..26320f2b5 100644 --- a/pkg/chartutil/coalesce.go +++ b/pkg/chartutil/coalesce.go @@ -19,6 +19,7 @@ package chartutil import ( "fmt" "log" + "strconv" "github.com/mitchellh/copystructure" "github.com/pkg/errors" @@ -285,9 +286,49 @@ func coalesceTablesFullKey(printf printFn, dst, src map[string]interface{}, pref } else { printf("warning: cannot overwrite table with non table for %s (%v)", fullkey, val) } + } else if islist(val) { + if islist(dv) { + dst[key] = coalesceListsFullKey(printf, dv.([]interface{}), val.([]interface{}), fullkey, merge) + } else { + printf("warning: cannot overwrite list with non list for %s (%v)", fullkey, val) + } } else if istable(dv) && val != nil { printf("warning: destination for %s is a table. Ignoring non-table value (%v)", fullkey, val) } } return dst } + +// coalesceListsFullKey merges a source list into a destination list. +// +// dest is considered authoritative. +func coalesceListsFullKey(printf printFn, dst, src []interface{}, prefix string, merge bool) []interface{} { + // When --reuse-values is set but there are no modifications yet, return new values + if src == nil { + return dst + } + if dst == nil { + return src + } + // Because dest has higher precedence than src, dest values override src + // values. + for key, val := range src { + if key == len(dst) { + dst = append(dst, val) + } else if dst[key] == nil { + dst[key] = val + } else if istable(val) && istable(dst[key]) { + dst[key] = coalesceTablesFullKey( + printf, + dst[key].(map[string]interface{}), + val.(map[string]interface{}), + prefix, + merge, + ) + } else { + fullkey := concatPrefix(prefix, strconv.Itoa(key)) + printf("warning: cannot overwrite table with non table for %s (%v)", fullkey, val) + } + } + return dst +} diff --git a/pkg/chartutil/values.go b/pkg/chartutil/values.go index d41df6e7f..f6fbcb58f 100644 --- a/pkg/chartutil/values.go +++ b/pkg/chartutil/values.go @@ -180,6 +180,12 @@ func istable(v interface{}) bool { return ok } +// islist is a special-purpose function to see if the present thing matches the definition of a YAML list. +func islist(v interface{}) bool { + _, ok := v.([]interface{}) + return ok +} + // PathValue takes a path that traverses a YAML structure and returns the value at the end of that path. // The path starts at the root of the YAML structure and is comprised of YAML keys separated by periods. // Given the following YAML data the value at path "chapter.one.title" is "Loomings".