The generated chart doesn't handle well environment variables created by the quarkus kubernetes extension for init tasks

[PierreBtz]

When adding quarkus-flyway to my project, quarkus-kubernetes creates an init task (see https://quarkus.io/guides/flyway#flyway-on-kubernetes). This init task comes with an environment variable QUARKUS_FLYWAY_ENABLED. It seems the type of the environment variable is incorrect, since when installing the chart generated with quarkus-helm, I end up with:

Error: Deployment in version "v1" cannot be handled as a Deployment: json: cannot unmarshal bool into Go struct field EnvVar.spec.template.spec.containers.env.value of type string

Looking into the resources generated by quarkus-kubernetes, I can see that the type looks correct:

    spec:
      containers:
        - env:
            - name: KUBERNETES_NAMESPACE
              valueFrom:
                fieldRef:
                  fieldPath: metadata.namespace
            - name: QUARKUS_FLYWAY_ENABLED
              value: "false"

In the chart generated by quarkus-helm I see:

    spec:
      containers:
        - env:
            - name: KUBERNETES_NAMESPACE
              valueFrom:
                fieldRef:
                  fieldPath: metadata.namespace
            - name: QUARKUS_FLYWAY_ENABLED
              value: {{ .Values.app.envs.QUARKUS_FLYWAY_ENABLED }}

With the following in the values file:

app:
  envs:
    QUARKUS_FLYWAY_ENABLED: "false"

(which looks good to me).

Finally the schema also looks good:

        "envs" : {
          "type" : "object",
          "properties" : {
            "QUARKUS_FLYWAY_ENABLED" : {
              "type" : "string"
            }
          }
        },

I tried overwriting the value when installing the chart with --set-string, with no luck...

I managed to reproduced on a minimal example:

• generate a project with the 4 extensions selected in this link https://code.quarkus.io/?e=kubernetes&e=kind&e=flyway&e=container-image-docker
• setup your kubectl context to a test cluster
• run mvn package && helm upgrade --install bug target/helm/kubernetes/<artifact>

For convenience I uploaded a reproducer: https://github.com/PierreBtz/quarkus-flyway-helm-bug.

Upon installing the chart, you should see:

Release "quarkus-flyway-helm-bug" does not exist. Installing it now.
Error: Deployment in version "v1" cannot be handled as a Deployment: json: cannot unmarshal bool into Go struct field EnvVar.spec.template.spec.containers.env.value of type string

[Sgitario]

Unfortunately, this is a limitation of the Helm client by Go.
From the Quarkus Helm extension, there is no way to improve this situation since we can't know if the env var value is a string or a boolean.
The only workaround I came up with is to add the following configuration that adds the quotes:

quarkus.helm.expressions.0.path=*.env.(name == QUARKUS_FLYWAY_ENABLED).value
quarkus.helm.expressions.0.expression={{ .Values.app.envs.QUARKUS_FLYWAY_ENABLED | quote }}

Let me know if this makes the generated Chart work for you.

I will keep the issue open just in case there are a better solution for this issue.

[PierreBtz]

I confirm the suggested solution works properly.

What I had in mind, was that for "well-known" environment variables generated by other extensions, quarkus helm could force the type.

It would introduce an indirect coupling with the quarkus-flyway extension (and other extensions in the future) so perhaps you do not want to go down this road (and I totally understand)!

Thanks for your work on this extension!

[Sgitario]

What I had in mind, was that for "well-known" environment variables generated by other extensions, quarkus helm could force the type.

It would introduce an indirect coupling with the quarkus-flyway extension (and other extensions in the future) so perhaps you do not want to go down this road (and I totally understand)!

Indeed, that's why I'm reluctant to do something like this. More taking into account that this is a really Helm CLI issue that they might fix in the future and also because it seems weird to me that the Quarkus Flyway (or Liquibase) extensions automatically add this env var property.

[PierreBtz]

I totally understand :) In fact, I need to dig a bit deeper when I have some time, but I think there is a problem with this approach, since quarkus.flyway.enabled is a build time property, so I have a hard time figuring out what it can do at the deployment/statefulset level.

Back to the issue at hand, as far as I'm concerned, you're proposed workaround is alright!

[Sgitario]

quarkus.flyway.enabled

I had not noticed that "quarkus.flyway.enabled" was a build-time property, then the env var seems wrong for sure. Actually, in Quarkus 3.2, this property was not a build-time property, so maybe this is a leftover.

Thanks for digging into this!

[PierreBtz]

Link quarkusio/quarkus#37040

[Sgitario]

Link quarkusio/quarkus#37040

Great, thanks!

[yrodiere]

@Sgitario I just hit this as well.

I don't get why there was mention of handling well-known environment variables. All environment variables must be strings anyway, that's what the error message is telling us.

With that in mind, can't the Helm extension at least force quoting for all environment variables?

What I mean is that it currently replaces this:

    spec:
      containers:
        - env:
            - name: QUARKUS_HIBERNATE_SEARCH_ORM_ELASTICSEARCH_HOSTS
              value: search-backend-0.search-backend:9200
            - name: INDEXING_QUEUE_COUNT
              value: "12"
            - name: HOME
              value: /tmp
            - name: INDEXING_BULK_SIZE
              value: "20"

with this:

    spec:
      containers:
        - env:
            - name: QUARKUS_HIBERNATE_SEARCH_ORM_ELASTICSEARCH_HOSTS
              value: {{ .Values.app.envs.QUARKUS_HIBERNATE_SEARCH_ORM_ELASTICSEARCH_HOSTS }}
            - name: INDEXING_QUEUE_COUNT
              value: {{ .Values.app.envs.INDEXING_QUEUE_COUNT }}
            - name: HOME
              value: {{ .Values.app.envs.HOME }}
            - name: INDEXING_BULK_SIZE
              value: {{ .Values.app.envs.INDEXING_BULK_SIZE }}

Can't it replace it with this instead?

    spec:
      containers:
        - env:
            - name: QUARKUS_HIBERNATE_SEARCH_ORM_ELASTICSEARCH_HOSTS
              value: {{ .Values.app.envs.QUARKUS_HIBERNATE_SEARCH_ORM_ELASTICSEARCH_HOSTS | quote }}
            - name: INDEXING_QUEUE_COUNT
              value: {{ .Values.app.envs.INDEXING_QUEUE_COUNT | quote }}
            - name: HOME
              value: {{ .Values.app.envs.HOME | quote }}
            - name: INDEXING_BULK_SIZE
              value: {{ .Values.app.envs.INDEXING_BULK_SIZE | quote }}

Or are you saying the Helm extension doesn't have the context it needs to detect, upon replacement, that it's replacing an environment variable?

[Sgitario]

@yrodiere the Quarkus Helm extension is already adding the | quote only when the value is not a string, and looking at:

- name: INDEXING_BULK_SIZE
              value: "20"

The "20" is a string, so that's why the quote is not added. However, the Helm client (again) interprets "20" as a number and hence we have the problem.

I was reluctant to always add the quote for env vars because (1) this is a limitation of the Helm client that we don't hit if we don't use it and (2) users might add a non-string env var and then it won't work as they expect.
But since this is being a pain for a lot of users, I think we can do it and for the users from (2), they can always provide a custom expression. Wdyt?

[yrodiere]

(1) this is a limitation of the Helm client that we don't hit if we don't use it

Do you mean there are other ways to use Helm charts than through the Helm client? 😕

(2) users might add a non-string env var and then it won't work as they expect.

Are you sure they can? Environment variables are always strings as far as I can tell... the Kubernetes YAML schema just doesn't allow anything other than strings there. See https://kubernetes.io/docs/reference/kubernetes-api/workload-resources/pod-v1/#environment-variables

But since this is being a pain for a lot of users, I think we can do it and for the users from (2), they can always provide a custom expression. Wdyt?

+1 from me, because I don't think users from (2) even exist :D

[Sgitario]

+1 from me, because I don't think users from (2) even exist :D

About (2), the problem I foresee is that users using boolean or integers would expect to inject types as such. Before adding the | quote, this is spotted before installing the chart. Now, they will realise that these types are now strings.

Having said that, I also expect (2) is not even a real concern!
#319 should fix this issue.