From e8d4e5bcd389dcbbca1479c43fc436e72b5a984c Mon Sep 17 00:00:00 2001 From: Dong Gang Date: Wed, 5 Aug 2020 10:25:44 +0800 Subject: [PATCH] fix(render): fix invalid subchart global value change after rendering other subchart Signed-off-by: Dong Gang --- cmd/helm/template_test.go | 5 ++ .../template-with-subchart-global-value.txt | 48 ++++++++++++++++++ .../chart-with-subchart-global/Chart.lock | 9 ++++ .../chart-with-subchart-global/Chart.yaml | 15 ++++++ .../charts/dep1-0.1.0.tgz | Bin 0 -> 1637 bytes .../charts/dep2-0.1.0.tgz | Bin 0 -> 1638 bytes .../dep1/Chart.yaml | 23 +++++++++ .../dep1/templates/ingress.yaml | 42 +++++++++++++++ .../dep1/values.yaml | 23 +++++++++ .../dep2/Chart.yaml | 23 +++++++++ .../dep2/templates/ingress.yaml | 42 +++++++++++++++ .../dep2/values.yaml | 24 +++++++++ .../chart-with-subchart-global/values.yaml | 22 ++++++++ pkg/engine/engine.go | 7 ++- 14 files changed, 282 insertions(+), 1 deletion(-) create mode 100644 cmd/helm/testdata/output/template-with-subchart-global-value.txt create mode 100644 cmd/helm/testdata/testcharts/chart-with-subchart-global/Chart.lock create mode 100644 cmd/helm/testdata/testcharts/chart-with-subchart-global/Chart.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-subchart-global/charts/dep1-0.1.0.tgz create mode 100644 cmd/helm/testdata/testcharts/chart-with-subchart-global/charts/dep2-0.1.0.tgz create mode 100644 cmd/helm/testdata/testcharts/chart-with-subchart-global/dep1/Chart.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-subchart-global/dep1/templates/ingress.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-subchart-global/dep1/values.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-subchart-global/dep2/Chart.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-subchart-global/dep2/templates/ingress.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-subchart-global/dep2/values.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-subchart-global/values.yaml diff --git a/cmd/helm/template_test.go b/cmd/helm/template_test.go index f8cd31347..3d27baa99 100644 --- a/cmd/helm/template_test.go +++ b/cmd/helm/template_test.go @@ -94,6 +94,11 @@ func TestTemplateCmd(t *testing.T) { cmd: fmt.Sprintf("template '%s' --show-only templates/service.yaml --show-only charts/subcharta/templates/service.yaml", chartPath), golden: "output/template-show-only-multiple.txt", }, + { + name: "check chart with multiple dependencies that share global value", + cmd: fmt.Sprintf("template '%s' --set global.ingress.annotations.test=global", "testdata/testcharts/chart-with-subchart-global"), + golden: "output/template-with-subchart-global-value.txt", + }, { name: "template with show-only glob", cmd: fmt.Sprintf("template '%s' --show-only templates/subdir/role*", chartPath), diff --git a/cmd/helm/testdata/output/template-with-subchart-global-value.txt b/cmd/helm/testdata/output/template-with-subchart-global-value.txt new file mode 100644 index 000000000..395703614 --- /dev/null +++ b/cmd/helm/testdata/output/template-with-subchart-global-value.txt @@ -0,0 +1,48 @@ +--- +# Source: chart-with-subchart-global/charts/dep1/templates/ingress.yaml +apiVersion: networking.k8s.io/v1beta1 +kind: Ingress +metadata: + name: RELEASE-NAME-dep1 + labels: + helm.sh/chart: dep1-0.1.0 + app.kubernetes.io/name: dep1 + app.kubernetes.io/instance: RELEASE-NAME + app.kubernetes.io/version: "1.16.0" + app.kubernetes.io/managed-by: Helm + annotations: + some-annotation: dep1 + test: global +spec: + rules: + - host: "localhost1" + http: + paths: + - path: / + backend: + serviceName: RELEASE-NAME-dep1 + servicePort: 80 +--- +# Source: chart-with-subchart-global/charts/dep2/templates/ingress.yaml +apiVersion: networking.k8s.io/v1beta1 +kind: Ingress +metadata: + name: RELEASE-NAME-dep2 + labels: + helm.sh/chart: dep2-0.1.0 + app.kubernetes.io/name: dep2 + app.kubernetes.io/instance: RELEASE-NAME + app.kubernetes.io/version: "1.16.0" + app.kubernetes.io/managed-by: Helm + annotations: + some-annotation: dep2 + test: global +spec: + rules: + - host: "localhost2" + http: + paths: + - path: / + backend: + serviceName: RELEASE-NAME-dep2 + servicePort: 80 diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart-global/Chart.lock b/cmd/helm/testdata/testcharts/chart-with-subchart-global/Chart.lock new file mode 100644 index 000000000..ddd8169de --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-subchart-global/Chart.lock @@ -0,0 +1,9 @@ +dependencies: +- name: dep1 + repository: file://../dep1 + version: 0.1.0 +- name: dep2 + repository: file://../dep2 + version: 0.1.0 +digest: sha256:3278a7071cc4f6cecc966ec807fd378018d2f7f07f5f136b538596c1d9066c51 +generated: "2020-08-03T18:59:45.008862+03:00" diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart-global/Chart.yaml b/cmd/helm/testdata/testcharts/chart-with-subchart-global/Chart.yaml new file mode 100644 index 000000000..9a4b93203 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-subchart-global/Chart.yaml @@ -0,0 +1,15 @@ +apiVersion: v2 +name: chart-with-subchart-global +description: A Helm chart for Kubernetes +type: application +version: 0.1.0 +appVersion: 1.16.0 + +dependencies: + - name: dep1 + version: 0.1.0 + repository: file://./dep1 + - name: dep2 + version: 0.1.0 + repository: file://./dep2 + diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart-global/charts/dep1-0.1.0.tgz b/cmd/helm/testdata/testcharts/chart-with-subchart-global/charts/dep1-0.1.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..d870e96ccc87f26a71f4394949863177368636ac GIT binary patch literal 1637 zcmV-r2AcUFiwG0|00000|0w_~VMtOiV@ORlOnEsqVl!4SWK%V1T2nbTPgYhoO;>Dc zVQyr3R8em|NM&qo0PGm;ZsWKyzxfnX`3~4kYg&%I&OyKySY!`v&>|PqMT(**7PK^$ zc%djANjXWA^w|rNk}b)0lf!P37V!MADRMZR8O|pYEXSj>j44C!ST2rU0>Us1XVa`7rXhK2u@6v#^ZykS73voY08jbLB3`L1bQ&WDsQ1FERPbR0cp8rp$hy4FK zaDP8Kh852v&}d-E1zHFFm|L~P8AfnCdVC}ni~M+7GMRwM5Uh(sv2Lr>e`8+(EkK*IuKR%rxXJRHJY7`ioLjAcWNL$uXOZI8qgV;4zmb9C^*R90;{f;dU@Jm$~>`A&!C1;YQ zm<$(po={^cb6=|L)YQCY{=e(0FXC|L`Mz7)(vNp*x|?S%wP7;GF2`->*=3{sfL#}3 zz~Z>5q_L5#(zsU`hy2OoUhE4xT{If|lY3$RU02ji6r=C=5t41$T7KS;4C+DfECO4; z{MOm`?V(ryH5K$XZ3UmG|4zcw{`=2#I{9#@|6T)ne%#!D-F~fI{X6Td7j@#_>&6>& zy^1djWh{2jF{y3$+95<^zCv|Ybe$Q+oY_PZ#3_8aCd9kp33DH7M;+cX54)xaKm6;N0tl16DvORN*$14u3tlm@1LqeZ^qCW9PRx~Zjni@v^^RwKN1ZS6Hj zvfVYW88XG+SuXZ*P|IzL*r=Y`VIW4-Cz!UVipdt^TewYYUGM1aaw`p@N49CMozdI7 zYCio$yX&@_Hs1|SXG`ZpRRW)sp~GDBKwwX@qiq&5W6F(?{<&cN**V9pwPmy9?W8B) zT>)6I_!?yrbta+7=@)xUatG>VQ5SD(U+rM_G`vrHZyml4@Ri{5KeZ)2Pi3L-CC+&2 z{rAJQ&yQ*!)oGJq?lcu{FM|NNsSv{c2<7#l))f^6i` jRq->SD;*aJjtDc zVQyr3R8em|NM&qo0PGm;ZsWKyzxfnX`3~4kYg+zT??J#8SY!`v&>|PqMT(**7PK^$ zc%djANjXWA^w|rNk}b)0lfzw-7V!MADRMZR8O|pYEGMJ0j44C!ST0Up0>Us1=d+pn z55utkACBiAPsY=a<4HK3jOWvna6AboA5I{A5pH@=X~Wb>_>0f#3-^BltmIdyG%sWX z>j`0Hx%nK^@tlSvK^-ez8aMYD{)i%n*cuG13I%_vmZ&5e)I_ozBd`WV%w35Qu(A|9 zW_Bl8H&jD9rXhK2v5!!L^Zzvy73voY08jbbY>S zutQ*Ig!V|S!v>z-aPL{KasST$4d$g_2KDG`hN48JsVRTGQ1FERPp2Q}J^!E0j`{y} z;QoGe3TvK6pwYmJ3$zaUDYt5gGmPML^!P|D7Ww|RWHJGhAy^lOV&%rpaw)0thLGTj zON78$7PW{047W?H5(cO*IIWWaz4FPY9W~$qjH+Y|^Qjy1d{M1d{1$>?05vU?i5+^@ z&J;3(z&0n?ICKX;RZQ>|$HbN*j%iB%hMu;=H}(W8fQBW;tkMX&$gx$`cu`jzMWTTz z1i^D|FacA5$+(7h%R8rrFBc1o%Vi3>#P~fW=POVskm*`qaUl)kQrFd!o8tyJ zsI=u%19QyR-0s(E6J@uW<*SV%`ysed*unMZC|l?aoo^iD(WmF%=9b5e3Oo_|7q1_` zx8buh`7@a-6_;iO!MA$&tqyv${Grdct^LD}#kH=|`bJ&l+G}*zS=)xk5^Ra$(3Lur z?S1y;Xsg(d>oOf4Kw&9Zj1c@bfZ%KJ^PQ*2^Fn&|^a1w#qUSF|kyD+G9Lo{h-vgJi zsFI#2)F0Mc)F?1Eh5B<%khZL&mhAJU2eE6cENM4+Z(51_`#oD8;HRoE*pqaZO3oxp zF&QrJJfX%?7QR&3tEolJ{D0R~U&P_w^L@9pqaW|qbU)8rYQtoVU5?w%v(HBR0lO~7 zfW>i9Nn;~frSYIJ4*9dky*Lzfx@a`^C-=hsyRN9)C`R8OA|%_gwfwvx8PtQ|Sp;@` z`K`0>+e5GZYbxkpwH17#{+om!`|m%q*>rxa|6T)ne%#!D-F|Ic{Tu777j@!4=*C-g zy^b#nWh{2jF{y3$+95<^zD9Lcbe$Q+oZKPZ#4Q8aCdBkp33DH7M;+cX54)xaKm6;N0tl16DvOKcL~14u3tlm=#gqeZ^qCW9PRx~-*ri@v#)6K_!?yrbta+7=@$n~au4cdQ5SC;U+rM_G<-;VZymml@C(7^e`-s5p2|YuOPul4 z`|pS8hn@G|>G+ucUjqhk$&5iIwe17E4Zux?5|$Mg37099EWTzbYDxz1*Nkh>RcVvB z2A!c0VDl5?%)}X&>F+?HV1}>J1+msVlL;As#MGrDcvmW1@moxM%KOcG3O`732ZeMy zElW_M0>LFxLcc7&E=-}24502;ug(^baHR=Nxf!`%&px5cf2ooCZ6>mGWPh4Zy_Tab zAsbSw(!G6Ya!U10NlxjKU6WI4ayu>*Ps!=O$pEgH;zgz5{LA;6&{7pYVr&TI39^w# kSH+Knu60}_I66khBOKuf|I6^700030|1lU5uK*$d05|F?DF6Tf literal 0 HcmV?d00001 diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep1/Chart.yaml b/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep1/Chart.yaml new file mode 100644 index 000000000..fbdd3e7dc --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep1/Chart.yaml @@ -0,0 +1,23 @@ +apiVersion: v2 +name: dep1 +description: A Helm chart for Kubernetes + +# A chart can be either an 'application' or a 'library' chart. +# +# Application charts are a collection of templates that can be packaged into versioned archives +# to be deployed. +# +# Library charts provide useful utilities or functions for the chart developer. They're included as +# a dependency of application charts to inject those utilities and functions into the rendering +# pipeline. Library charts do not define any templates and therefore cannot be deployed. +type: application + +# This is the chart version. This version number should be incremented each time you make changes +# to the chart and its templates, including the app version. +# Versions are expected to follow Semantic Versioning (https://semver.org/) +version: 0.1.0 + +# This is the version number of the application being deployed. This version number should be +# incremented each time you make changes to the application. Versions are not expected to +# follow Semantic Versioning. They should reflect the version the application is using. +appVersion: 1.16.0 diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep1/templates/ingress.yaml b/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep1/templates/ingress.yaml new file mode 100644 index 000000000..d22c917bd --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep1/templates/ingress.yaml @@ -0,0 +1,42 @@ +{{- if .Values.ingress.enabled -}} +{{- $fullName := include "dep1.fullname" . -}} +{{- $svcPort := .Values.service.port -}} +{{- if semverCompare ">=1.14-0" .Capabilities.KubeVersion.GitVersion -}} +apiVersion: networking.k8s.io/v1beta1 +{{- else -}} +apiVersion: extensions/v1beta1 +{{- end }} +kind: Ingress +metadata: + name: {{ $fullName }} + labels: + {{- include "dep1.labels" . | nindent 4 }} +{{/* // Here I'm try to merge global & unique for chart annotations*/}} + {{- with merge .Values.global.ingress.annotations .Values.ingress.annotations }} + annotations: + {{- toYaml . | nindent 4 }} + {{- end }} +spec: + {{- 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: {{ . }} + backend: + serviceName: {{ $fullName }} + servicePort: {{ $svcPort }} + {{- end }} + {{- end }} + {{- end }} diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep1/values.yaml b/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep1/values.yaml new file mode 100644 index 000000000..41bd48bb5 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep1/values.yaml @@ -0,0 +1,23 @@ + + +imagePullSecrets: [] +nameOverride: "" +fullnameOverride: "" + + +service: + type: ClusterIP + port: 80 + +ingress: + enabled: true + annotations: {} + # kubernetes.io/ingress.class: nginx + # kubernetes.io/tls-acme: "true" + hosts: + - host: chart-example.local + paths: [] + tls: [] + # - secretName: chart-example-tls + # hosts: + # - chart-example.local diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep2/Chart.yaml b/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep2/Chart.yaml new file mode 100644 index 000000000..2bd659297 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep2/Chart.yaml @@ -0,0 +1,23 @@ +apiVersion: v2 +name: dep2 +description: A Helm chart for Kubernetes + +# A chart can be either an 'application' or a 'library' chart. +# +# Application charts are a collection of templates that can be packaged into versioned archives +# to be deployed. +# +# Library charts provide useful utilities or functions for the chart developer. They're included as +# a dependency of application charts to inject those utilities and functions into the rendering +# pipeline. Library charts do not define any templates and therefore cannot be deployed. +type: application + +# This is the chart version. This version number should be incremented each time you make changes +# to the chart and its templates, including the app version. +# Versions are expected to follow Semantic Versioning (https://semver.org/) +version: 0.1.0 + +# This is the version number of the application being deployed. This version number should be +# incremented each time you make changes to the application. Versions are not expected to +# follow Semantic Versioning. They should reflect the version the application is using. +appVersion: 1.16.0 diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep2/templates/ingress.yaml b/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep2/templates/ingress.yaml new file mode 100644 index 000000000..2dce86651 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep2/templates/ingress.yaml @@ -0,0 +1,42 @@ +{{- if .Values.ingress.enabled -}} +{{- $fullName := include "dep2.fullname" . -}} +{{- $svcPort := .Values.service.port -}} +{{- if semverCompare ">=1.14-0" .Capabilities.KubeVersion.GitVersion -}} +apiVersion: networking.k8s.io/v1beta1 +{{- else -}} +apiVersion: extensions/v1beta1 +{{- end }} +kind: Ingress +metadata: + name: {{ $fullName }} + labels: + {{- include "dep2.labels" . | nindent 4 }} + {{/* // Here I'm try to merge global & unique for chart annotations*/}} + {{- with merge .Values.global.ingress.annotations .Values.ingress.annotations }} + annotations: + {{- toYaml . | nindent 4 }} + {{- end }} +spec: + {{- 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: {{ . }} + backend: + serviceName: {{ $fullName }} + servicePort: {{ $svcPort }} + {{- end }} + {{- end }} + {{- end }} diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep2/values.yaml b/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep2/values.yaml new file mode 100644 index 000000000..116b40067 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-subchart-global/dep2/values.yaml @@ -0,0 +1,24 @@ + + + +imagePullSecrets: [] +nameOverride: "" +fullnameOverride: "" + + +service: + type: ClusterIP + port: 80 + +ingress: + enabled: true + annotations: {} + # kubernetes.io/ingress.class: nginx + # kubernetes.io/tls-acme: "true" + hosts: + - host: chart-example.local + paths: [] + tls: [] + # - secretName: chart-example-tls + # hosts: + # - chart-example.local diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart-global/values.yaml b/cmd/helm/testdata/testcharts/chart-with-subchart-global/values.yaml new file mode 100644 index 000000000..c80094219 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-subchart-global/values.yaml @@ -0,0 +1,22 @@ +global: + ingress: + annotations: {} + +dep1: + ingress: + enabled: true + annotations: + "some-annotation": "dep1" + hosts: + - host: localhost1 + paths: + - / +dep2: + ingress: + enabled: true + annotations: + "some-annotation": "dep2" + hosts: + - host: localhost2 + paths: + - / diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 5aa0ed8ec..a581a4d33 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -26,6 +26,7 @@ import ( "strings" "text/template" + "github.com/mitchellh/copystructure" "github.com/pkg/errors" "k8s.io/client-go/rest" @@ -244,7 +245,11 @@ func (e Engine) renderWithReferences(tpls, referenceTpls map[string]renderable) continue } // At render time, add information about the template that is being rendered. - vals := tpls[filename].vals + structureCopy, err := copystructure.Copy(tpls[filename].vals) + if err != nil { + return map[string]string{}, err + } + vals := structureCopy.(chartutil.Values) vals["Template"] = chartutil.Values{"Name": filename, "BasePath": tpls[filename].basePath} var buf strings.Builder if err := t.ExecuteTemplate(&buf, filename, vals); err != nil {