Installing the stack using helm fails: ’,’ should be escaped
Closed this issue · 13 comments
Expected Behaviour
The "TL;DR" provided there https://github.com/tinkerbell/charts/blob/main/tinkerbell/stack/README.md#tldr does not work.
Current Behaviour
The given shell command to create the "trusted_proxies" string does not escape the commas.
Possible Solution
The last tr command used in kubectl get nodes -o jsonpath='{.items[*].spec.podCIDR}' | tr ' ' ',' should escape the commas.
trusted_proxies=$(kubectl get nodes -o jsonpath='{.items[*].spec.podCIDR}' | tr ' ' '\\,')Steps to Reproduce (for bugs)
helm dependency build stack/trusted_proxies=$(kubectl get nodes -o jsonpath='{.items[*].spec.podCIDR}' | tr ' ' ',')helm install stack-release stack/ --create-namespace --namespace tink-system --wait --set "boots.trustedProxies=${trusted_proxies}" --set "hegel.trustedProxies=${trusted_proxies}"
The result:
Error: INSTALLATION FAILED: failed parsing --set data: key "0/24" has no value (cannot end with ,)
Context
Your Environment
$ helm version
helm versionversion.BuildInfo{Version:"v3.7.0", GitCommit:"eeac83883cb4014fe60267ec6373570374ce770b", GitTreeState:"clean", GoVersion:"go1.16.8"}
$ kubectl version --short
Flag --short has been deprecated, and will be removed in the future. The --short output will become the default.
Client Version: v1.26.0
Kustomize Version: v4.5.7
Server Version: v1.26.0Hi @mgrzybek, thanks for raising the issue. Could you share the output of the kubectl command to get podCIDR?
Hi,
The output is:
$ kubectl get nodes -o jsonpath='{.items[*].spec.podCIDR}' | tr ' ' ','
10.244.3.0/24,10.244.2.0/24,10.244.5.0/24,10.244.0.0/24,10.244.1.0/24,10.244.4.0/24Output of my script:
$ make deploy
kubectl apply -f tink-ns.yml
namespace/tink-system unchanged
cd charts/tinkerbell && helm dependency build stack/
Saving 4 charts
Deleting outdated charts
cd charts/tinkerbell && helm install stack-release stack/ \
--namespace tink-system \
--wait \
--set "boots.trustedProxies=10.244.3.0/24,10.244.2.0/24,10.244.5.0/24,10.244.0.0/24,10.244.1.0/24,10.244.4.0/24" \
--set "hegel.trustedProxies=10.244.3.0/24,10.244.2.0/24,10.244.5.0/24,10.244.0.0/24,10.244.1.0/24,10.244.4.0/24"
Error: INSTALLATION FAILED: failed parsing --set data: key "0/24" has no value (cannot end with ,)
make: *** [Makefile:16: deploy] Error 1
$
TIL you can set multiple values with --set.
@mgrzybek the fix you raised looks good if you'd like to re-open.
I think my patch was wrong. The commas were not escaped by Helm interpretor.
According to https://stackoverflow.com/questions/48316330/how-to-set-multiple-values-with-helm the right way to write comma-separated lists should look like:
helm install stack-release stack/ --namespace tink-system --wait \
--set "boots.trustedProxies={10.244.3.0/24,10.244.2.0/24}" \
--set "hegel.trustedProxies={10.244.3.0/24,10.244.2.0/24}"
I managed to make it work using "join" in deployment.yaml
$ git diff
diff --git a/tinkerbell/boots/templates/deployment.yaml b/tinkerbell/boots/templates/deployment.yaml
index 07ba8e2..8b046de 100644
--- a/tinkerbell/boots/templates/deployment.yaml
+++ b/tinkerbell/boots/templates/deployment.yaml
@@ -37,7 +37,7 @@ spec:
{{- end }}
env:
- name: TRUSTED_PROXIES
- value: {{ required "missing trustedProxies" .Values.trustedProxies | quote }}
+ value: {{ join "," .Values.trustedProxies }}
{{- range $i, $env := .Values.env }}
- name: {{ $env.name | quote }}
value: {{ $env.value | quote }}
diff --git a/tinkerbell/hegel/templates/deployment.yaml b/tinkerbell/hegel/templates/deployment.yaml
index 81432b6..eb69e4e 100644
--- a/tinkerbell/hegel/templates/deployment.yaml
+++ b/tinkerbell/hegel/templates/deployment.yaml
@@ -33,7 +33,7 @@ spec:
{{- end }}
env:
- name: HEGEL_TRUSTED_PROXIES
- value: {{ required "missing trustedProxies" .Values.trustedProxies | quote }}
+ value: {{ join "," .Values.trustedProxies }}
{{- range $i, $env := .Values.env }}
- name: {{ $env.name | quote }}
value: {{ $env.value | quote }}
diff --git a/tinkerbell/hegel/values.yaml b/tinkerbell/hegel/values.yaml
index ae6d2b6..1af08fa 100644
--- a/tinkerbell/hegel/values.yaml
+++ b/tinkerbell/hegel/values.yaml
@@ -1,5 +1,5 @@
deploy: true
-trustedProxies: ""
+trustedProxies: []
name: hegel
image: quay.io/tinkerbell/hegel:v0.10.1
imagePullPolicy: IfNotPresent
$What do you think?
I think we can change the proxies to arrays, that aligns with the direction we want to take the charts anyway.
Could you also update Hegel values and Boots values to reflect them being arrays (just swap out the "" for []).
I think we also still want the required piece. If you can't use something like require ... | join "," .Values.trustedProxies you can add {{- $_ := required ... }} above the TRUSTED_PROXIES env definition.
Hey @mgrzybek, thanks for catching and reporting this! I'm able to get everything deploy successfully without any code changes by using this trusted_proxies=$(kubectl get nodes -o jsonpath='{.items[*].spec.podCIDR}' | sed 's/ /\\,/g') instead of this trusted_proxies=$(kubectl get nodes -o jsonpath='{.items[*].spec.podCIDR}' | tr ' ' ',').
It seems like we could just update the docs to resolve this?
CC @chrisdoherty4
@jacobweinstock We can update the docs instead. I still like the idea of transitioning to an array though. It removes the need for the consumer to care about argument formatting and instead focus on formatting YAML, the document they're adjusting. I think this might go hand in hand with some of the other changes we spoke about regarding exposure of configuration in values YAML. What do you think?
Ah yeah, true. An array does feel like it improves the understand-ability.
The PR has been closed. Another solution is on the way.
I guess I can wait for #32 to be merged to close the issue.
Following the docs, I ran into this again:
$ helm version
version.BuildInfo{Version:"v3.10.1", GitCommit:"9f88ccb6aee40b9a0535fcc7efea6055e1ef72c9", GitTreeState:"clean", GoVersion:"go1.18.7"}
$ trusted_proxies=$(kubectl get nodes -o jsonpath='{.items[*].spec.podCIDR}' | tr ' ' ',')
$ ~/src/tinkerbell/charts/tinkerbell/main$ echo xxx${trusted_proxies}xxx
xxx10.42.0.0/24,10.42.3.0/24,10.42.1.0/24xxx
$ helm install stack-release stack/ --create-namespace --namespace tink-system --wait --set "boots.trustedProxies=${trusted_proxies}" --set "hegel.trustedProxies=${trusted_proxies}"
Error: INSTALLATION FAILED: failed parsing --set data: key "0/24" has no value (cannot end with ,)
Changing to the alternate syntax @jacobweinstock suggested:
$ trusted_proxies=$(kubectl get nodes -o jsonpath='{.items[*].spec.podCIDR}' | sed 's/ /\\,/g')
$ echo xxx${trusted_proxies}xxx
xxx10.42.1.0/24\,10.42.0.0/24\,10.42.3.0/24xxx
$ helm install stack-release stack/ --create-namespace --namespace tink-system --wait --set "boots.trustedProxies=${trusted_proxies}" --set "hegel.trustedProxies=${trusted_proxies}"
Error: INSTALLATION FAILED: timed out waiting for the condition
Now that's better :-)
rufio failed with exec /manager: exec format error
tink-stack-relay with
Normal Started 45m (x4 over 46m) kubelet Started container macvlan-interface
Normal Pulling 44m (x5 over 46m) kubelet Pulling image "alpine"
Normal Pulled 44m kubelet Successfully pulled image "alpine" in 724.920905ms (724.995317ms including waiting)
Normal Created 44m (x5 over 46m) kubelet Created container macvlan-interface
Warning BackOff 104s (x205 over 45m) kubelet Back-off restarting failed container macvlan-interface in pod tink-stack-relay-7f9b46b754-4pt6j_tink-system(b33e67bf-f6b1-4063-9678-7c311ef95b21)