Installing the stack using helm fails: ’,’ should be escaped

[mgrzybek]

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)

1. helm dependency build stack/
2. trusted_proxies=$(kubectl get nodes -o jsonpath='{.items[*].spec.podCIDR}' | tr ' ' ',')
3. 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.0

[chrisdoherty4]

Hi @mgrzybek, thanks for raising the issue. Could you share the output of the kubectl command to get podCIDR?

[mgrzybek]

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/24

Output 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
$

[chrisdoherty4]

TIL you can set multiple values with --set.

[chrisdoherty4]

@mgrzybek the fix you raised looks good if you'd like to re-open.

[mgrzybek]

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}"

[mgrzybek]

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?

[chrisdoherty4]

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.

[jacobweinstock]

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

[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?

[jacobweinstock]

Ah yeah, true. An array does feel like it improves the understand-ability.

[mgrzybek]

The PR has been closed. Another solution is on the way. I guess I can wait for https://github.com/tinkerbell/charts/pull/32 to be merged to close the issue.

[chrisdoherty4]

@mgrzybek With #34 and #32 this should be patched. Let us know if you have any issues.

[displague]

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)