edx / edx-arch-experiments

A plugin to include applications under development by the architecture team at edx
GNU Affero General Public License v3.0
0 stars 3 forks source link

Can't set boolean environment variables in Helm chart #627

Open timmc-edx opened 5 months ago

timmc-edx commented 5 months ago

We can't set boolean-looking environment variables for containers using the extraEnvs config provided by the django-ida Helm chart.

Setting a value to true or false results in a sync failure with an error message like unrecognized type: string following a large blob of config. Inspecting the config, we can see it includes something like {\"name\":\"DD_DJANGO_USE_LEGACY_RESOURCE_FORMAT\",\"value\":true}—the boolean true rather than the string "true". This happens even if the boolean was originally quoted as a string. Likely we're falling prey to Jinja-templated-YAML failing to escape the values it writes, inadvertently converting strings to bools.

Setting a (quoted) boolean directly in the env subtree of the django-ida Helm chart works fine, so likely what we need to do is stringify and quote the env var values in the loop where we apply extraEnvs.

A/C:

github-actions[bot] commented 4 months ago

This is now https://2u-internal.atlassian.net/browse/GSRE-1799

jristau1984 commented 2 months ago

@timmc-edx your response in the linked GSRE ticket looks like this has been resolved. Can you confirm that this ticket can be closed?

timmc-edx commented 2 months ago

It was fixed in https://github.com/edx/helm-charts/pull/150/files but I may want to re-fix it—the function they used was quote, which is appropriate for use in shell scripts rather than for YAML files. I think we want string instead. It's relatively low priority but should be a quick fix (now that I have documented how to test helm-charts changes locally!)