Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added tls configuration for elasticsearch #547

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

0xHexE
Copy link

@0xHexE 0xHexE commented Aug 13, 2024

What was changed

Added TLS Configuration for external elasticsearch

Why?

If we have external elasticsearch then we should able to connect to elasticsearch

Checklist

  • Setup external elasticsearch using elastic operator example
  • Setup
  1. Closes [Feature Request] Add TLS support for es-visibility in the helm chart #447

  2. How was this tested:

  • Setup of elasticsearch
  • Install helm chart with this values
elasticsearch:
  enabled: false
  external: true
  host: "elasticsearch-es-http"
  port: "9200"
  version: "v7"
  scheme: "https"
  logLevel: "error"
  username: "#USER#"
  password: "#PASSWORD#"
  tls:
    enabled: true
    secretName: "elasticsearch-es-remote-ca"
    ca: "ca.crt"

@0xHexE 0xHexE requested a review from a team as a code owner August 13, 2024 13:16
@CLAassistant
Copy link

CLAassistant commented Aug 13, 2024

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@harsha2650
Copy link

harsha2650 commented Aug 25, 2024

did we modified helper functions to accommodate this change.? in code below, it should be something like $driverConfig.tls.secretName instead $driverConfig.secretName.? can someone confirm if it is thoroughly tested because we continue to see secrets not found error.

{{- define "temporal.persistence.elasticsearch.secretName" -}}
{{- $global := index . 0 -}}
{{- $store := index . 1 -}}
{{- $driverConfig := $global.Values.elasticsearch -}}
{{- if $driverConfig.existingSecret -}}
{{- print $driverConfig.existingSecret -}}
{{- else if $driverConfig.secretName -}}
{{- print $driverConfig.secretName -}}
{{- else -}}
{{- include "temporal.componentname" (list $global (printf "%s-store" $store)) -}}
{{- end -}}
{{- end -}}

Error:
MountVolume.SetUp failed for volume "elasticsearch-certs" : secret "elasticsearch-certs" not found

@0xHexE
Copy link
Author

0xHexE commented Aug 25, 2024

Hey @harsha2650,

It was more for external elastic server rather than the provided one I didn't test because the chart we are using it is deprecated.

I am happy to work on the provided elastic part too can you provide some example yaml file I can fix it?

@harsha2650
Copy link

Hi @0xHexE ,

We have tried several ways by including all the changes in this PR but continuing to see the error "MountVolume.SetUp failed for volume "elasticsearch-certs" : secret "elasticsearch-certs" not found". Can we please request team to test this helm chart version thoroughly.? Deployment file is still not recognising the secrets, we also noticed some open bug's around this issue, not sure if they were fixed. please advise

#529
#538

Thank you so much for looking into it.

@0xHexE
Copy link
Author

0xHexE commented Aug 31, 2024

Hey @harsha2650,

I will try to fix the issues you mentioned. Can you please share any example values yaml which are causing this issue.

And as I mentioned earlier the elasticsearch helm is deprecated we need to use operator version or bitnami version.

And one more thing we moved to to this operator https://github.com/alexandrevilain/temporal-operator we faced lot of issues with the chart you can try this too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Add TLS support for es-visibility in the helm chart
3 participants