-
Notifications
You must be signed in to change notification settings - Fork 239
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
Updates HELM configuration to adapt to changes in demo admin user setup and bumps appVersion to 2.12.0 #503
Updates HELM configuration to adapt to changes in demo admin user setup and bumps appVersion to 2.12.0 #503
Conversation
…min password Signed-off-by: Darshit Chanpura <[email protected]>
abbb25e
to
ea94cd0
Compare
Signed-off-by: Darshit Chanpura <[email protected]>
Update: security plugin is not supporting password via txt file and hence this is not required. |
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
@prudhvigodithi @ruanyl Can I get more reviews on this? |
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add a CHANGELOG entry for opensearch on 2.18.0.
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
@prudhvigodithi Could you please review this? |
@prudhvigodithi Could you please add 1 more review? |
Thanks @DarshitChanpura, this PR should be merged after I would suggest to use a new parameter as With your change can you please test adding a Thanks |
@prudhvigodithi I'm unfamiliar with HELM and hence slightly confused at what the ask is here. We are not supplying a default password. User will always have to pass the password. Our intention is to fail cluster startup if no password was supplied. By adding a new parameter "initialAdminPassword" we are essentially supplying a default value. We are moving away from default hardcoded password for admin. |
…nd bumps appVersion to 2.12.0
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Thanks @DarshitChanpura for your contribution, there are some more changes required, I'm closing this PR in favor of #518. |
Description
This change adapts to the change in security plugin's demo installation script which now requires admin password to be setup when installing demo configuration. Without this password, the cluster will not spin up.
Issues Resolved
Check List
For any changes to files within Helm chart directories:
CHANGELOG.md
updated to reflect changeBy 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.