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

Update docker for default admin creds update #114

Merged
merged 5 commits into from
Feb 6, 2024

Conversation

derek-ho
Copy link

Changes

Passes in an initial admin password via env variable, implemented in security plugin main and 2.x lines, for 2.12.0 release.

Merge Requirements

For new features contributions please make sure you have completed the following
essential items:

  • CHANGELOG.md updated to document new feature additions
  • Appropriate documentation updates in the docs
  • Appropriate Helm chart updates in the helm-charts

Maintainers will not merge until the above have been completed. If you're unsure
which docs need to be changed ping the
@open-telemetry/demo-approvers.

@derek-ho
Copy link
Author

@YANG-DB can you help me understand what other changes this repo needs? It doesn't seem to follow the branching strategy of main/2.x - so do we need to handle logic to determine whether to use the default admin password, or the env variable password in the ES_PASSWORD field? Any other places you see that would need changes? Thanks!

@derek-ho
Copy link
Author

@YANG-DB
Copy link
Member

YANG-DB commented Dec 29, 2023

@derek-ho please use the .env for all the password’s place holders

Copy link

github-actions bot commented Jan 6, 2024

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jan 6, 2024
Copy link

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@DarshitChanpura
Copy link
Member

@derek-ho please use the .env for all the password’s place holders

@derek-ho Could you please address this comment.

@derek-ho
Copy link
Author

Marking this as draft until variable OPENSEARCH_VERSION changes to >= 2.12.0.

Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: Derek Ho <[email protected]>
@derek-ho
Copy link
Author

Done. @YANG-DB can you take another look? I also changed the image from staging to the actual released docker images, similar to the dashboards image (which is not using staging).

@@ -732,6 +732,7 @@ services:
- cluster.initial_cluster_manager_nodes=opensearch
- bootstrap.memory_lock=true
- "OPENSEARCH_JAVA_OPTS=-Xms512m -Xmx512m"
- "OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123!"
Copy link
Member

Choose a reason for hiding this comment

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

please use an env var for this field

Signed-off-by: Derek Ho <[email protected]>
@github-actions github-actions bot removed the Stale label Jan 19, 2024
@@ -723,7 +723,7 @@ services:

# OpenSearch store - engine
opensearch:
image: opensearchstaging/opensearch:${OPENSEARCH_VERSION}
Copy link
Member

Choose a reason for hiding this comment

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

is this change needed?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it is a mismatch between OS backend and OSD image

@derek-ho
Copy link
Author

@YANG-DB can you double check again and we can merge if everything looks good?

Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jan 30, 2024
@DarshitChanpura
Copy link
Member

@YANG-DB @pjfitzgibbons @ps48 @kavithacm @joshuali925 @dai-chen @rupal-bq @mengweieric @vamsi-amazon @Swiddis Could we get this reviewed as we are targeting 2.12 for this change?

@github-actions github-actions bot removed the Stale label Feb 1, 2024
@DarshitChanpura
Copy link
Member

@YANG-DB @pjfitzgibbons @ps48 @kavithacm @joshuali925 @dai-chen @rupal-bq @mengweieric @vamsi-amazon @Swiddis Still waiting for reviews. Could we get this reviewed as we are targeting 2.12 for this change?

@derek-ho derek-ho merged commit bc493fd into opensearch-project:main Feb 6, 2024
23 of 28 checks passed
@derek-ho derek-ho deleted the admin branch February 6, 2024 16:43
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.

3 participants