temporalio / helm-charts

Temporal Helm charts
MIT License
294 stars 321 forks source link

feat: add support for additionalEnv to schema jobs #408

Closed weisdd closed 3 months ago

weisdd commented 1 year ago

What was changed

Added support for additionalEnv to schema jobs (server-job.yaml).

Why?

Currently, the chart doesn't have a way to tune PostgreSQL TLS settings in the jobs that create/update schema. One of the ways to implement that support is through custom environment variables. E.g. for "required" mode, I need to pass:

        - name: SQL_TLS
           value: "true"
         - name: SQL_TLS_DISABLE_HOST_VERIFICATION
           value: "true"

Checklist

2. How was this tested:

Saved the values below to custom-values.yaml

schema:
  additionalEnv:
    - name: SQL_TLS
      value: "true"
    - name: SQL_TLS_DISABLE_HOST_VERIFICATION
      value: "true"

Then ran:

helm dependencies update
helm template . -f custom-values.yaml --show-only templates/server-job.yaml

The envs should appear in specs of the containers.

NOTE: It was added only to containers that already had section with environment variables. There's probably not much need for that for check-elasticsearch, create-elasticsearch-index, etc.

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

grzegorz8 commented 1 year ago

I've just created a PR for basically the same issue but in a different way :)

https://github.com/temporalio/helm-charts/pull/411

weisdd commented 1 year ago

@grzegorz8 that's nice :) I had actually thought of implementing something similar to what you added in the PR, though it was one of my last days on a project where temporal was planned to be used, so had to prefer the quicker option. Anyway, I'm happy that you had the time to make the chart more user-friendly :)