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

[2.11] Enhancing the Install Demo Configuration Script with a clearer message #3490

Merged

Conversation

DarshitChanpura
Copy link
Member

Description

This changes remove the confusing log statement from 2.11 to unblock the build.

Issues Resolved

Testing

Manual testing:
opensearch-security ./tools/install_demo_configuration.sh -y -i -s
**************************************************************************
** This tool will be deprecated in the next major release of OpenSearch **
** https://github.com/opensearch-project/security/issues/1755           **
**************************************************************************
OpenSearch Security Demo Installer
 ** Warning: Do not use on production or public reachable systems **
Basedir: /Users/dchanp/Documents/amazon/testing-playground/test/binaries/opensearch-3.0.0-SNAPSHOT
OpenSearch install type: .tar.gz on
OpenSearch config dir: /Users/dchanp/Documents/amazon/testing-playground/test/binaries/opensearch-3.0.0-SNAPSHOT/config
OpenSearch config file: /Users/dchanp/Documents/amazon/testing-playground/test/binaries/opensearch-3.0.0-SNAPSHOT/config/opensearch.yml
OpenSearch bin dir: /Users/dchanp/Documents/amazon/testing-playground/test/binaries/opensearch-3.0.0-SNAPSHOT/bin
OpenSearch plugins dir: /Users/dchanp/Documents/amazon/testing-playground/test/binaries/opensearch-3.0.0-SNAPSHOT/plugins
OpenSearch lib dir: /Users/dchanp/Documents/amazon/testing-playground/test/binaries/opensearch-3.0.0-SNAPSHOT/lib
Detected OpenSearch Version: x-content-3.0.0-SNAPSHOT
Detected OpenSearch Security Version: 3.0.0.0-SNAPSHOT

Unable to find custom admin password. Continuing with the default password for admin.
   ***************************************************
   ***   ADMIN PASSWORD SET TO: admin    ***
   ***************************************************
**************************************************************************
** This tool will be deprecated in the next major release of OpenSearch **
** https://github.com/opensearch-project/security/issues/1755           **
**************************************************************************
### Success
### Execute this script now on all your nodes and then start all nodes
### OpenSearch Security will be automatically initialized.
### If you like to change the runtime configuration
### change the files in ../../../config/opensearch-security and execute:
"/Users/dchanp/Documents/amazon/testing-playground/test/binaries/opensearch-3.0.0-SNAPSHOT/plugins/opensearch-security/tools/securityadmin.sh" -cd "/Users/dchanp/Documents/amazon/testing-playground/test/binaries/opensearch-3.0.0-SNAPSHOT/config/opensearch-security" -icl -key "/Users/dchanp/Documents/amazon/testing-playground/test/binaries/opensearch-3.0.0-SNAPSHOT/config/kirk-key.pem" -cert "/Users/dchanp/Documents/amazon/testing-playground/test/binaries/opensearch-3.0.0-SNAPSHOT/config/kirk.pem" -cacert "/Users/dchanp/Documents/amazon/testing-playground/test/binaries/opensearch-3.0.0-SNAPSHOT/config/root-ca.pem" -nhnv
### or run ./securityadmin_demo.sh
### To use the Security Plugin ConfigurationGUI
### To access your secured cluster open https://<hostname>:<HTTP port> and log in with admin/admin.
### (Ignore the SSL certificate warning because we installed self-signed demo certificates)

Check List

- [ ] New functionality includes testing
- [ ] New functionality has been documented

  • Commits are signed per the DCO using --signoff

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.

@DarshitChanpura DarshitChanpura changed the title Replaces confusing log with a better error message [2.11] Replaces confusing log in install_demo_configuration script with a better error message Oct 6, 2023
Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

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

I don't think its better, maybe I'm too close to the issue, but as it was it states the action(s) that can be taken to resolve the 'no password' scenario.

This new message doesn't indicate where it was looked for, or what action a user should take to get resolution. What didn't work about the old message for you?

@DarshitChanpura
Copy link
Member Author

@peternied The old message was confusing in the sense that if I spin up a docker instance and run it without passing initialAdminPassword the script would exit and not finish execution, however the docker cluster will continue to spin up and create an admin user with admin password as it picks that up from internal_users.yml file.

This PR avoids the confusion by letting users know that the password will be defaulted to admin which is what is eventually happening.

I have updated the log messages to be more detailed.

@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

Merging #3490 (10bb68f) into 2.11 (ef1d27c) will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##               2.11    #3490      +/-   ##
============================================
- Coverage     64.69%   64.68%   -0.02%     
+ Complexity     3566     3565       -1     
============================================
  Files           267      267              
  Lines         19893    19893              
  Branches       3329     3329              
============================================
- Hits          12870    12868       -2     
- Misses         5383     5384       +1     
- Partials       1640     1641       +1     

see 1 file with indirect coverage changes

cwperks
cwperks previously approved these changes Oct 6, 2023
RyanL1997
RyanL1997 previously approved these changes Oct 6, 2023
@DarshitChanpura DarshitChanpura changed the title [2.11] Replaces confusing log in install_demo_configuration script with a better error message [2.11] Replaces confusing log in install_demo_configuration script with a better message Oct 6, 2023
@DarshitChanpura DarshitChanpura changed the title [2.11] Replaces confusing log in install_demo_configuration script with a better message [2.11] Enhancing the Install Demo Configuration Script with a clearer message Oct 6, 2023
@DarshitChanpura DarshitChanpura dismissed stale reviews from RyanL1997 and cwperks via 95b4813 October 6, 2023 21:14
@peternied peternied dismissed their stale review October 6, 2023 21:50

Unblocking this review - has 2 approvals

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
@DarshitChanpura DarshitChanpura merged commit 919589c into opensearch-project:2.11 Oct 7, 2023
104 of 105 checks passed
DarshitChanpura added a commit to DarshitChanpura/security that referenced this pull request Oct 9, 2023
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.

None yet

4 participants