Pass in initial admin password and remove admin:admin references #631

derek-ho · 2023-12-14T15:25:04Z

Description

Starting with 2.12.0, security plugin requires an initial admin password to be set via env. variable when installing the demo configuration. This PR is to update documentation and CI to accommodate that change.

Issues Resolved

Closes: #759

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.

Signed-off-by: Derek Ho <dxho@amazon.com>

codecov · 2023-12-14T15:26:25Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.14%. Comparing base (ba715b9) to head (9d2c999).
Report is 26 commits behind head on main.

❗ Current head 9d2c999 differs from pull request most recent head e5cd848

Please upload reports for the commit e5cd848 to get more accurate results.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #631      +/-   ##
==========================================
+ Coverage   71.95%   72.14%   +0.19%     
==========================================
  Files          91       89       -2     
  Lines        8001     7945      -56     
==========================================
- Hits         5757     5732      -25     
+ Misses       2244     2213      -31

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

saimedhi · 2023-12-14T19:47:12Z

Hello @derek-ho,

Is this pull request ready for review?

Signed-off-by: Derek Ho <dxho@amazon.com>

derek-ho · 2023-12-28T21:03:55Z

Some logic may need to be added after 2.12.0 release in test files like here: https://github.com/opensearch-project/opensearch-py/blob/main/opensearchpy/helpers/test.py#L37 to replace this with new password conditionally, based on the version the CI is running against.

derek-ho · 2023-12-28T21:05:31Z

@saimedhi @VachaShah this PR is ready for review now - I am not sure what approach you folks think is appropriate. I think this PR should be able to be merged as is, but after 2.12.0 I would expect some tests to fail for 2.12.0 release, such as here: https://github.com/opensearch-project/opensearch-py/blob/main/opensearchpy/helpers/test.py#L37. What do you folks think?

saimedhi · 2024-01-09T18:43:31Z

@saimedhi @VachaShah this PR is ready for review now - I am not sure what approach you folks think is appropriate. I think this PR should be able to be merged as is, but after 2.12.0 I would expect some tests to fail for 2.12.0 release, such as here: https://github.com/opensearch-project/opensearch-py/blob/main/opensearchpy/helpers/test.py#L37. What do you folks think?

If it's not urgent, we can wait until the 2.12.0 release to make the code changes. This allows us to align with the latest release. @VachaShah what do you think?

VachaShah · 2024-01-09T21:51:54Z

I am wondering why we can't make all the changes now since we already test this client against the developing 2.x line. @derek-ho I believe you should be able to add the version checks in as well and test the code out. LMK if I am missing something here.

derek-ho · 2024-01-18T15:07:31Z

I am wondering why we can't make all the changes now since we already test this client against the developing 2.x line. @derek-ho I believe you should be able to add the version checks in as well and test the code out. LMK if I am missing something here.

@VachaShah I believe those tests are run against 2.x OpenSearch without security, so thus we are not seeing those fail yet.

derek-ho · 2024-01-18T15:08:13Z

@saimedhi @VachaShah this PR is ready for review now - I am not sure what approach you folks think is appropriate. I think this PR should be able to be merged as is, but after 2.12.0 I would expect some tests to fail for 2.12.0 release, such as here: https://github.com/opensearch-project/opensearch-py/blob/main/opensearchpy/helpers/test.py#L37. What do you folks think?

If it's not urgent, we can wait until the 2.12.0 release to make the code changes. This allows us to align with the latest release. @VachaShah what do you think?

I am ok with waiting for now, if the regular CI is not broken. I will turn this PR to draft for now.

dblock · 2024-01-18T16:18:49Z

@VachaShah I believe those tests are run against 2.x OpenSearch without security, so thus we are not seeing those fail yet.

Are you able to help enable that?

DarshitChanpura · 2024-02-22T21:30:12Z

Now that 2.12 is released this should be unblocked. Would the maintainers review and merge this?

VachaShah · 2024-02-22T22:33:36Z

@derek-ho Is this ready for review?

dblock · 2024-02-24T13:14:46Z

@DarshitChanpura also needs a rebase, please

DarshitChanpura · 2024-02-27T00:45:49Z

@derek-ho Would you please rebase this and mark as ready for review? I don't have write access to push to your branch.

Signed-off-by: Derek Ho <derek01778@gmail.com>

derek-ho · 2024-03-13T18:35:50Z

@derek-ho Please fix these failing tests https://github.com/opensearch-project/opensearch-py/actions/runs/8072055726/job/22087332746

I am pretty sure the without security tests are not because of this change - I see them failing on other PRs as well. I am working on fixing the 2.12 with security test case, but running into issues with extracting version information within the test, as it seems to only be available outside of the test case. I will try a bit longer, but may need maintainers help for that.

saimedhi · 2024-03-13T21:29:41Z

@derek-ho Please fix these failing tests https://github.com/opensearch-project/opensearch-py/actions/runs/8072055726/job/22087332746

I am pretty sure the without security tests are not because of this change - I see them failing on other PRs as well. I am working on fixing the 2.12 with security test case, but running into issues with extracting version information within the test, as it seems to only be available outside of the test case. I will try a bit longer, but may need maintainers help for that.

Yes, other failures are unrelated to this PR. Apologies for any confusion caused. I specifically referred to version (2.12.0, true) as seen here: link. Thank you :)

saimedhi · 2024-05-23T06:11:08Z

Hello @derek-ho,

Could you please take a look at finishing this PR when you have a moment?

Thanks a lot!

saimedhi · 2024-06-17T20:27:26Z

Hello @derek-ho, could you please take a look at this PR and let me know if it's ready for review? opensearch-py currently lacks compatibility with the latest OpenSearch versions 2.12.0, 2.13.0, and 2.14.0. It would be great if we could resolve this issue and get the PR merged soon. Are you facing any difficulties or need assistance in this regard?

dblock · 2024-06-17T21:05:52Z

.ci/run-opensearch.sh

+else
+  CREDENTIAL="admin:myStrongPassword123!"
+fi
+


This won't work for 2.13, will it?

It should since admin credentials changes were released in 2.12. If you are referencing script execution in general, it works as expected returning admin:admin for versions <2.12 and admin:myStronPassword123! for those >= 2.12

Ok, but wouldn't it be simpler to just set OPENSEARCH_USERNAME and OPENSEARCH_PASSWORD in env: in the workflow?

yes, it should be, however on line 64 we make a status check call that would require the correct credentials to be passed.

Signed-off-by: Derek Ho <dxho@amazon.com>

DarshitChanpura · 2024-06-18T14:44:42Z

.ci/run-opensearch.sh

+else
+  CREDENTIAL="admin:myStrongPassword123!"
+fi
+


It should since admin credentials changes were released in 2.12. If you are referencing script execution in general, it works as expected returning admin:admin for versions <2.12 and admin:myStronPassword123! for those >= 2.12

Signed-off-by: Derek Ho <dxho@amazon.com>

dblock · 2024-06-18T15:23:59Z

@derek-ho I'm good with what's here if you can get it to green.

Without trying it, I think we can specify the username/password in .github workflow and remove all the logic from the .sh scripts. We can also do this later.

derek-ho · 2024-06-18T15:24:59Z

Agree @dblock there is probably a simpler way to do it, but I am not an expert in Python and just focused on getting it to a good state that we can start testing against the newer versions. It can be cleaned up in a follow up, but I may need assistance for that part.

dblock · 2024-06-18T15:25:30Z

.ci/run-repository.sh

@@ -39,9 +59,11 @@ docker run \
  --env "PYTHON_CONNECTION_CLASS=${PYTHON_CONNECTION_CLASS}" \
  --env "TEST_TYPE=server" \
  --env "TEST_PATTERN=${TEST_PATTERN}" \
+  --env "OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123!" \


You could avoid the entire if, versions < 2.12 don't care if you set this.

Yea, we can skip it but I was having trouble reading the env correctly within the testing code itself to determine which credentials to use when setting up the connection. It seems that I needed to set the env. in the sh file right before the tests ran for it to work. Not sure if there is a simpler way.

dblock · 2024-06-18T15:26:40Z

Agree @dblock there is probably a simpler way to do it, but I am not an expert in Python and just focused on getting it to a good state that we can start testing against the newer versions. It can be cleaned up in a follow up, but I may need assistance for that part.

I'm good with whatever works. We can improve later.

derek-ho · 2024-06-18T15:28:55Z

@dblock the CI seems to pass with the latest changes, can we merge this as is? Do you want me to create a follow up issue to clean up the logic/code for CI?

derek-ho · 2024-06-18T15:34:14Z

Created follow up issue here: #764

…nsearch-project#631) * Update to pass in initial admin password Signed-off-by: Derek Ho <dxho@amazon.com> * Add changelog and logic to distinguish between versions Signed-off-by: Derek Ho <dxho@amazon.com> * fix syntax Signed-off-by: Derek Ho <dxho@amazon.com> * Revert tests Signed-off-by: Derek Ho <dxho@amazon.com> * Add 2.12 to the matrix and fix testing logic Signed-off-by: Derek Ho <dxho@amazon.com> * Fix version logic Signed-off-by: Derek Ho <dxho@amazon.com> * Try to split job into two batches Signed-off-by: Derek Ho <dxho@amazon.com> * Fix lint Signed-off-by: Derek Ho <dxho@amazon.com> * Change name Signed-off-by: Derek Ho <dxho@amazon.com> * Remove period Signed-off-by: Derek Ho <dxho@amazon.com> * Pull password dynamically Signed-off-by: Derek Ho <dxho@amazon.com> * Change to proper env var Signed-off-by: Derek Ho <dxho@amazon.com> * Try passing through Signed-off-by: Derek Ho <dxho@amazon.com> --------- Signed-off-by: Derek Ho <dxho@amazon.com> Signed-off-by: Derek Ho <derek01778@gmail.com>

Update to pass in initial admin password

5a70790

Signed-off-by: Derek Ho <dxho@amazon.com>

derek-ho requested review from VachaShah, dblock, harshavamsi, axeoman, Shephalimittal, saimedhi and florianvazelle as code owners December 14, 2023 15:25

derek-ho marked this pull request as draft December 14, 2023 15:33

DarshitChanpura mentioned this pull request Dec 15, 2023

[META] Update all instances of admin:admin across different repositories opensearch-project/security#3624

Closed

2 tasks

derek-ho added 3 commits December 28, 2023 15:55

Add changelog and logic to distinguish between versions

065e1bc

Signed-off-by: Derek Ho <dxho@amazon.com>

fix syntax

b91a8ec

Signed-off-by: Derek Ho <dxho@amazon.com>

Revert tests

db0e15a

Signed-off-by: Derek Ho <dxho@amazon.com>

derek-ho marked this pull request as ready for review December 28, 2023 21:04

derek-ho marked this pull request as draft January 18, 2024 15:08

Merge branch 'main' into main

50970d8

Signed-off-by: Derek Ho <derek01778@gmail.com>

derek-ho marked this pull request as ready for review February 27, 2024 22:08

saimedhi added the opensearch-py 2.6.0 label Apr 23, 2024

saimedhi removed the opensearch-py 2.6.0 label May 28, 2024

saimedhi mentioned this pull request Jun 17, 2024

[BUG] Support Latest OpenSearch versions 2.12, 2.13, 2.14 #759

Closed

Merge branch 'main' of github.com:opensearch-project/opensearch-py

05eaded

dblock reviewed Jun 17, 2024

View reviewed changes

derek-ho added 5 commits June 17, 2024 17:17

Try to split job into two batches

7892fdc

Signed-off-by: Derek Ho <dxho@amazon.com>

Fix lint

2639850

Signed-off-by: Derek Ho <dxho@amazon.com>

Change name

85b923c

Signed-off-by: Derek Ho <dxho@amazon.com>

Remove period

38a676f

Signed-off-by: Derek Ho <dxho@amazon.com>

Pull password dynamically

cf5602f

Signed-off-by: Derek Ho <dxho@amazon.com>

DarshitChanpura approved these changes Jun 18, 2024

View reviewed changes

Change to proper env var

ac76aae

Signed-off-by: Derek Ho <dxho@amazon.com>

dblock previously approved these changes Jun 18, 2024

View reviewed changes

Try passing through

e5cd848

Signed-off-by: Derek Ho <dxho@amazon.com>

derek-ho dismissed dblock’s stale review via e5cd848 June 18, 2024 15:24

dblock reviewed Jun 18, 2024

View reviewed changes

derek-ho mentioned this pull request Jun 18, 2024

[FEATURE] Clean up logic behind admin credential changes in 2.12 onwards #764

Open

dblock approved these changes Jun 18, 2024

View reviewed changes

dblock merged commit d3177a8 into opensearch-project:main Jun 18, 2024
61 checks passed

saimedhi mentioned this pull request Jun 18, 2024

Updated compatibility matrix to include latest OpenSearch versions #765

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass in initial admin password and remove admin:admin references #631

Pass in initial admin password and remove admin:admin references #631

derek-ho commented Dec 14, 2023 •

edited

Loading

codecov bot commented Dec 14, 2023 •

edited

Loading

saimedhi commented Dec 14, 2023 •

edited

Loading

derek-ho commented Dec 28, 2023

derek-ho commented Dec 28, 2023

saimedhi commented Jan 9, 2024

VachaShah commented Jan 9, 2024

derek-ho commented Jan 18, 2024

derek-ho commented Jan 18, 2024

dblock commented Jan 18, 2024

DarshitChanpura commented Feb 22, 2024

VachaShah commented Feb 22, 2024

dblock commented Feb 24, 2024

DarshitChanpura commented Feb 27, 2024

derek-ho commented Mar 13, 2024

saimedhi commented Mar 13, 2024

saimedhi commented May 23, 2024

saimedhi commented Jun 17, 2024

dblock Jun 17, 2024

DarshitChanpura Jun 18, 2024

dblock Jun 18, 2024

DarshitChanpura Jun 18, 2024

DarshitChanpura Jun 18, 2024

dblock commented Jun 18, 2024

derek-ho commented Jun 18, 2024

dblock Jun 18, 2024

derek-ho Jun 18, 2024

dblock commented Jun 18, 2024

derek-ho commented Jun 18, 2024

derek-ho commented Jun 18, 2024

Pass in initial admin password and remove admin:admin references #631

Pass in initial admin password and remove admin:admin references #631

Conversation

derek-ho commented Dec 14, 2023 • edited Loading

Description

Issues Resolved

codecov bot commented Dec 14, 2023 • edited Loading

Codecov Report

saimedhi commented Dec 14, 2023 • edited Loading

derek-ho commented Dec 28, 2023

derek-ho commented Dec 28, 2023

saimedhi commented Jan 9, 2024

VachaShah commented Jan 9, 2024

derek-ho commented Jan 18, 2024

derek-ho commented Jan 18, 2024

dblock commented Jan 18, 2024

DarshitChanpura commented Feb 22, 2024

VachaShah commented Feb 22, 2024

dblock commented Feb 24, 2024

DarshitChanpura commented Feb 27, 2024

derek-ho commented Mar 13, 2024

saimedhi commented Mar 13, 2024

saimedhi commented May 23, 2024

saimedhi commented Jun 17, 2024

dblock Jun 17, 2024

Choose a reason for hiding this comment

DarshitChanpura Jun 18, 2024

Choose a reason for hiding this comment

dblock Jun 18, 2024

Choose a reason for hiding this comment

DarshitChanpura Jun 18, 2024

Choose a reason for hiding this comment

DarshitChanpura Jun 18, 2024

Choose a reason for hiding this comment

dblock commented Jun 18, 2024

derek-ho commented Jun 18, 2024

dblock Jun 18, 2024

Choose a reason for hiding this comment

derek-ho Jun 18, 2024

Choose a reason for hiding this comment

dblock commented Jun 18, 2024

derek-ho commented Jun 18, 2024

derek-ho commented Jun 18, 2024

derek-ho commented Dec 14, 2023 •

edited

Loading

codecov bot commented Dec 14, 2023 •

edited

Loading

saimedhi commented Dec 14, 2023 •

edited

Loading