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

Remove references to admin:admin #1298

Merged
merged 8 commits into from
Jan 19, 2024
Merged

Conversation

derek-ho
Copy link
Contributor

Description

Recently, security repo removed the hard coded admin:admin that comes from the demo configuration setup script. This makes the corresponding change. Integtest.sh is not used, and should be removed as per: opensearch-project/opensearch-build#497

Issues Resolved

[List any issues this PR will resolve]

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • 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.

@@ -682,6 +682,7 @@ clusters.each { name ->
if (securityEnabled) {
plugin(provider(securityPluginOld))
cliSetup("opensearch-security/install_demo_configuration.sh", "-y")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DarshitChanpura how is this being used? How do we pass in "admin" as the password here? -t?

Copy link
Member

Choose a reason for hiding this comment

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

we don't pass it here.

We have to export it as a separate command.

And if you want to use admin as password you can pass in option -t in addition to -y

Copy link

codecov bot commented Dec 15, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (be24bfa) 74.03% compared to head (2925971) 74.57%.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1298      +/-   ##
============================================
+ Coverage     74.03%   74.57%   +0.54%     
- Complexity     1022     1028       +6     
============================================
  Files           141      141              
  Lines          4783     4783              
  Branches        526      526              
============================================
+ Hits           3541     3567      +26     
+ Misses          889      869      -20     
+ Partials        353      347       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@monusingh-1
Copy link
Collaborator

@derek-ho the default integtest.sh present in opensearch-build will not run the complete integ test suite as it not handle multi-node multi-cluster topology for integtest runs.

@monusingh-1
Copy link
Collaborator

from opensearch-project/opensearch-build#497
However, if a plugin requires some custom logic to run integtests, which the standard tool doesn't provide, they can continue maintaining this integtest.sh in their own repo.

I think we should continue to maintain integtest.sh in this repo.

@derek-ho
Copy link
Contributor Author

derek-ho commented Dec 18, 2023

Ack. I will revert the deletion of the file. @monusingh-1 can you double check there seems to be a security CI failure not related to this change. This change has been backported to 2.x line, so we can try and resolve any CI failures in this PR related to the change. Thanks!

build.gradle Outdated
@@ -681,7 +682,8 @@ clusters.each { name ->

if (securityEnabled) {
plugin(provider(securityPluginOld))
cliSetup("opensearch-security/install_demo_configuration.sh", "-y")
cliSetup("opensearch-security/install_demo_configuration.sh", "-y", "-t")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@monusingh-1 can you help me understand how you folks are using this line?

Comment on lines 79 to 84
# Starting in 2.12.0, security demo configuration script requires an initial admin password
if (( ${version_array[0]} > 2 || (${version_array[0]} == 2 && ${version_array[1]} >= 12) )); then
CREDENTIAL="admin:myStrongPassword123!"
else
CREDENTIAL="admin:admin"
fi
Copy link
Contributor Author

Choose a reason for hiding this comment

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

logic to use strong password in 2.12.0 onwards

@derek-ho
Copy link
Contributor Author

@monusingh-1 can you take another look? Is security tests a known issue/related to admin credential changes?

@DarshitChanpura DarshitChanpura added backport 2.x v2.12.0 Issues targeting release v2.12.0 labels Jan 3, 2024
@@ -34,6 +34,7 @@ import org.opensearch.gradle.test.RestIntegTestTask

buildscript {
ext {
System.setProperty("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.

we should be setting this value based on versions. i.e. for 2.11 and lower, it should be admin else use myStrongPassword123. Tests for 2.11 and lower would fail otherwise as the integtest.sh is modified to followed the same conditional assignment

@monusingh-1 monusingh-1 merged commit 3ea239b into opensearch-project:main Jan 19, 2024
16 of 17 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jan 19, 2024
* Remove references to admin:admin

Signed-off-by: Derek Ho <[email protected]>

* Try to pass in initial admin password via env variable

Signed-off-by: Derek Ho <[email protected]>

* Try using the env variable

Signed-off-by: Derek Ho <[email protected]>

* Revert file deletion and add logic for integration tests

Signed-off-by: Derek Ho <[email protected]>

* fix env variable

Signed-off-by: Derek Ho <[email protected]>

* Update logic to be the same across all repos

Signed-off-by: Derek Ho <[email protected]>

* Keep old logic

Signed-off-by: Derek Ho <[email protected]>

* Change variable name

Signed-off-by: Derek Ho <[email protected]>

---------

Signed-off-by: Derek Ho <[email protected]>
(cherry picked from commit 3ea239b)
DarshitChanpura pushed a commit to DarshitChanpura/cross-cluster-replication that referenced this pull request Feb 9, 2024
* Remove references to admin:admin

Signed-off-by: Derek Ho <[email protected]>

* Try to pass in initial admin password via env variable

Signed-off-by: Derek Ho <[email protected]>

* Try using the env variable

Signed-off-by: Derek Ho <[email protected]>

* Revert file deletion and add logic for integration tests

Signed-off-by: Derek Ho <[email protected]>

* fix env variable

Signed-off-by: Derek Ho <[email protected]>

* Update logic to be the same across all repos

Signed-off-by: Derek Ho <[email protected]>

* Keep old logic

Signed-off-by: Derek Ho <[email protected]>

* Change variable name

Signed-off-by: Derek Ho <[email protected]>

---------

Signed-off-by: Derek Ho <[email protected]>
(cherry picked from commit 3ea239b)
DarshitChanpura pushed a commit to DarshitChanpura/cross-cluster-replication that referenced this pull request Feb 9, 2024
* Remove references to admin:admin

Signed-off-by: Derek Ho <[email protected]>

* Try to pass in initial admin password via env variable

Signed-off-by: Derek Ho <[email protected]>

* Try using the env variable

Signed-off-by: Derek Ho <[email protected]>

* Revert file deletion and add logic for integration tests

Signed-off-by: Derek Ho <[email protected]>

* fix env variable

Signed-off-by: Derek Ho <[email protected]>

* Update logic to be the same across all repos

Signed-off-by: Derek Ho <[email protected]>

* Keep old logic

Signed-off-by: Derek Ho <[email protected]>

* Change variable name

Signed-off-by: Derek Ho <[email protected]>

---------

Signed-off-by: Derek Ho <[email protected]>
(cherry picked from commit 3ea239b)
DarshitChanpura pushed a commit to DarshitChanpura/cross-cluster-replication that referenced this pull request Feb 9, 2024
* Remove references to admin:admin

Signed-off-by: Derek Ho <[email protected]>

* Try to pass in initial admin password via env variable

Signed-off-by: Derek Ho <[email protected]>

* Try using the env variable

Signed-off-by: Derek Ho <[email protected]>

* Revert file deletion and add logic for integration tests

Signed-off-by: Derek Ho <[email protected]>

* fix env variable

Signed-off-by: Derek Ho <[email protected]>

* Update logic to be the same across all repos

Signed-off-by: Derek Ho <[email protected]>

* Keep old logic

Signed-off-by: Derek Ho <[email protected]>

* Change variable name

Signed-off-by: Derek Ho <[email protected]>

---------

Signed-off-by: Derek Ho <[email protected]>
(cherry picked from commit 3ea239b)
DarshitChanpura pushed a commit to DarshitChanpura/cross-cluster-replication that referenced this pull request Feb 9, 2024
* Remove references to admin:admin

Signed-off-by: Derek Ho <[email protected]>

* Try to pass in initial admin password via env variable

Signed-off-by: Derek Ho <[email protected]>

* Try using the env variable

Signed-off-by: Derek Ho <[email protected]>

* Revert file deletion and add logic for integration tests

Signed-off-by: Derek Ho <[email protected]>

* fix env variable

Signed-off-by: Derek Ho <[email protected]>

* Update logic to be the same across all repos

Signed-off-by: Derek Ho <[email protected]>

* Keep old logic

Signed-off-by: Derek Ho <[email protected]>

* Change variable name

Signed-off-by: Derek Ho <[email protected]>

---------

Signed-off-by: Derek Ho <[email protected]>
(cherry picked from commit 3ea239b)
DarshitChanpura pushed a commit to DarshitChanpura/cross-cluster-replication that referenced this pull request Feb 9, 2024
* Remove references to admin:admin

Signed-off-by: Derek Ho <[email protected]>

* Try to pass in initial admin password via env variable

Signed-off-by: Derek Ho <[email protected]>

* Try using the env variable

Signed-off-by: Derek Ho <[email protected]>

* Revert file deletion and add logic for integration tests

Signed-off-by: Derek Ho <[email protected]>

* fix env variable

Signed-off-by: Derek Ho <[email protected]>

* Update logic to be the same across all repos

Signed-off-by: Derek Ho <[email protected]>

* Keep old logic

Signed-off-by: Derek Ho <[email protected]>

* Change variable name

Signed-off-by: Derek Ho <[email protected]>

---------

Signed-off-by: Derek Ho <[email protected]>
(cherry picked from commit 3ea239b)
Signed-off-by: Darshit Chanpura <[email protected]>
monusingh-1 pushed a commit that referenced this pull request Feb 12, 2024
…et the password accordingly and removes admin:admin reference from the Handbook (#1327)

* Remove references to admin:admin (#1298)

* Remove references to admin:admin

Signed-off-by: Derek Ho <[email protected]>

* Try to pass in initial admin password via env variable

Signed-off-by: Derek Ho <[email protected]>

* Try using the env variable

Signed-off-by: Derek Ho <[email protected]>

* Revert file deletion and add logic for integration tests

Signed-off-by: Derek Ho <[email protected]>

* fix env variable

Signed-off-by: Derek Ho <[email protected]>

* Update logic to be the same across all repos

Signed-off-by: Derek Ho <[email protected]>

* Keep old logic

Signed-off-by: Derek Ho <[email protected]>

* Change variable name

Signed-off-by: Derek Ho <[email protected]>

---------

Signed-off-by: Derek Ho <[email protected]>
(cherry picked from commit 3ea239b)
Signed-off-by: Darshit Chanpura <[email protected]>

* Updates integTest behavior to accept the version and set the password accordingly

Signed-off-by: Darshit Chanpura <[email protected]>
(cherry picked from commit d703887)

---------

Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Co-authored-by: Derek Ho <[email protected]>
monusingh-1 pushed a commit that referenced this pull request Feb 12, 2024
…set the password accordingly and removes admin:admin reference from the Handbook (#1328)

* Remove references to admin:admin (#1298)

* Remove references to admin:admin

Signed-off-by: Derek Ho <[email protected]>

* Try to pass in initial admin password via env variable

Signed-off-by: Derek Ho <[email protected]>

* Try using the env variable

Signed-off-by: Derek Ho <[email protected]>

* Revert file deletion and add logic for integration tests

Signed-off-by: Derek Ho <[email protected]>

* fix env variable

Signed-off-by: Derek Ho <[email protected]>

* Update logic to be the same across all repos

Signed-off-by: Derek Ho <[email protected]>

* Keep old logic

Signed-off-by: Derek Ho <[email protected]>

* Change variable name

Signed-off-by: Derek Ho <[email protected]>

---------

Signed-off-by: Derek Ho <[email protected]>
(cherry picked from commit 3ea239b)

* Updates integTest behavior to accept the version and set the password accordingly

Signed-off-by: Darshit Chanpura <[email protected]>
(cherry picked from commit d703887)

---------

Co-authored-by: Derek Ho <[email protected]>
skumarp7 pushed a commit to nokia/opensearch-project-cross-cluster-replication that referenced this pull request Jul 22, 2024
…et the password accordingly and removes admin:admin reference from the Handbook (opensearch-project#1327)

* Remove references to admin:admin (opensearch-project#1298)

* Remove references to admin:admin

Signed-off-by: Derek Ho <[email protected]>

* Try to pass in initial admin password via env variable

Signed-off-by: Derek Ho <[email protected]>

* Try using the env variable

Signed-off-by: Derek Ho <[email protected]>

* Revert file deletion and add logic for integration tests

Signed-off-by: Derek Ho <[email protected]>

* fix env variable

Signed-off-by: Derek Ho <[email protected]>

* Update logic to be the same across all repos

Signed-off-by: Derek Ho <[email protected]>

* Keep old logic

Signed-off-by: Derek Ho <[email protected]>

* Change variable name

Signed-off-by: Derek Ho <[email protected]>

---------

Signed-off-by: Derek Ho <[email protected]>
(cherry picked from commit 3ea239b)
Signed-off-by: Darshit Chanpura <[email protected]>

* Updates integTest behavior to accept the version and set the password accordingly

Signed-off-by: Darshit Chanpura <[email protected]>
(cherry picked from commit d703887)

---------

Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Co-authored-by: Derek Ho <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x v2.12.0 Issues targeting release v2.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants