temporalio / helm-charts

Temporal Helm charts
MIT License
294 stars 321 forks source link

[Bug] check-elasticsearch-index init container on main deployment is not using ES secret for credentials #529

Open csabatuz-chess opened 1 month ago

csabatuz-chess commented 1 month ago

What are you really trying to do?

We are trying to configure our in-house Temporal deployment with an external Elasticsearch, using authentication.

The credentials are put in the secret and key specified under:

elasticsearch:
  ...
  secretName: ...
  secretKey: ...

Describe the bug

The check-elasticsearch-index init container on the main deployment fails due to credential errors. https://github.com/temporalio/helm-charts/blob/main/charts/temporal/templates/server-deployment.yaml#L69-L75

Our own analysis

It appears that it expects the password under elasticsearch.password value in cleartext. While this is good for testing, in our actual deployment we do need to use it from the secret.

It appears that jobs are using the secret as expected, via _admintools-env.yaml: https://github.com/temporalio/helm-charts/blob/main/charts/temporal/templates/server-job.yaml#L44-L46

It's possible that the Deployment template's current state is a leftover after a previous refactor

Our naive fix attempt (untested for now): https://github.com/csabatuz-chess/temporal-helm-charts/compare/main..CST/elasticsearch-password-from-secret

Minimal Reproduction

Stock charts with

elasticsearch:
  enabled: false
  external: true
  ... other configs ...
  username: elastic
  secretName: <your secret>
  secretKey: password
apiVersion: v1
data:
  password: <your password>
kind: Secret

Environment/Versions

We are using Kubernetes, with 0.44.0 version of the chart.