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

store opensearch-dashboards configs in Secret (#426) #430

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

Conversation

rufdoSICKAG
Copy link

opensearch_dashboards.yml contains values like
opensearch.password or opensearch_security.openid.client_secret which should be stored in Secrets rather than ConfigMaps.

Description

Because opensearch_dashboards.yml may contain secret values like opensearch.password or opensearch_security.openid.client_secret, it should be stored as Secret rather than a ConfigMap.
For comparison, the securityConfig in the opensearch chart is already stored as a Secret.

Issues Resolved

#426

Check List

  • [x ] Commits are signed per the DCO using --signoff

For any changes to files within Helm chart directories:

  • [x ] Helm chart version bumped
  • [x ] Helm chart CHANGELOG.md updated to reflect change

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link
Member

@TheAlgo TheAlgo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @rufdoSICKAG , thanks a lot for your contributions. Can you also update the README.md w.r.t this change.

@ThoreKr
Copy link

ThoreKr commented Jul 15, 2023

Should it then also be possible to provide your own secret? e.g. when handling values through flux it would be desireable to not store these secret values in the configmap of the chart values.

@mustdiechik
Copy link

@rufdoSICKAG
could you also add metadata.annotation value to secret ?
It will be convenient to use with vault webhook to modify secret.

@rufdoSICKAG
Copy link
Author

@TheAlgo sorry for the late response. I added the requested change to the README now.

@ThoreKr @mustdiechik I think adding additional options should be handled in a separate PR after this change has been accepted.

@rufdoSICKAG rufdoSICKAG reopened this Sep 11, 2023
@bbarani
Copy link
Member

bbarani commented Dec 4, 2023

@TheAlgo @prudhvigodithi @peterzhuamazon Can we merge this PR?

@prudhvigodithi
Copy link
Member

Hey @rufdoSICKAG, thanks for your contribution and patience, this change looks good to me, can you please fix the conflicts and we can merge this.

opensearch_dashboards.yml contains values like
opensearch.password or opensearch_security.openid.client_secret
which should be stored in Secrets rather than ConfigMaps.

Signed-off-by: Dominik Ruf <[email protected]>
@rufdoSICKAG
Copy link
Author

@prudhvigodithi I rebased my changes

Copy link
Member

@TheAlgo TheAlgo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@TheAlgo
Copy link
Member

TheAlgo commented Aug 21, 2024

@prudhvigodithi can we merge this PR?

@prudhvigodithi
Copy link
Member

@rufdoSICKAG can you please increment the chart version (as a patch release) and update the change-log ? Sample PR https://github.com/opensearch-project/helm-charts/pull/569/files
Thanks
@TheAlgo @peterzhuamazon

@rufdoSICKAG
Copy link
Author

@prudhvigodithi wouldn't it make more sense if who every manages the next release would do that?!
I mean, this PR is more then a year old, who knows how many release will happen until it will actually be merged.
And each time the PR would have to be updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 📦 Backlog
Development

Successfully merging this pull request may close these issues.

6 participants