Default admin password as admin

[derek-ho]

Description

Fixes the CI to pass in "admin" as the admin password if the version is >= 2.12. This is necessary since 2.12 and onwards requires an initial admin password to be set in order to run the security demo configuration.

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].

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.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.71%. Comparing base (360e88c) to head (4240ad3).
Report is 11 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #234      +/-   ##
==========================================
+ Coverage   74.53%   79.71%   +5.18%     
==========================================
  Files         413      418       +5     
  Lines       65509    71104    +5595     
==========================================
+ Hits        48824    56682    +7858     
+ Misses      16685    14422    -2263     

Flag	Coverage Δ
integration	79.71% <ø> (+5.18%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[derek-ho]

https://github.com/opensearch-project/opensearch-rs/blob/main/.github/workflows/test.yml#L36 - if this becomes 2.12.0 or onwards, then this change is necessary. For now this is unnecessary. Should the tests be run with a matrix of different opensearch versions to ensure compatibility?

[Xtansia]

https://github.com/opensearch-project/opensearch-rs/blob/main/.github/workflows/test.yml#L36 - if this becomes 2.12.0 or onwards, then this change is necessary. For now this is unnecessary. Should the tests be run with a matrix of different opensearch versions to ensure compatibility?

The linked line is only used for a handful of very basic tests, the real integration tests where we use core's YAML tests is this matrix here: https://github.com/opensearch-project/opensearch-rs/blob/main/.github/workflows/test.yml#L66

[derek-ho]

Got it. @Xtansia this change is starting in 2.12.0 onwards. I am seeing in the matrix this wouldn't be tested. Can you suggest how you think I can change this PR to best pickup the changes from 2.12.0 onwards?

[derek-ho]

@Xtansia @dblock can you confirm what is the plan to add 2.12.0 when released into that matrix, so I can modify this PR to add support for the new admin credential changes? If there is no plan then I think we can close this PR and mark this repo as compliant with the new credential changes, thanks!

[dblock]

@derek-ho AFAIK when 2.12 is available we'll add it to CI. I think you can mark this as draft as other PRs?

[DarshitChanpura]

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

[Xtansia]

@derek-ho Could you please add 2.12.0 to the various workflow matrices so that we can confirm things are working as expected?

[Xtansia]

This is causing all versions other than 2.12.0 to fail as the old versions of the script throw an error on the unrecognised parameter.

Could do this via something like:

SECURITY_VERSION=$(cat $OPENSEARCH_HOME/plugins/opensearch-security/plugin-descriptor.properties | grep '^version=' | cut -d'=' -f 2)
SECURITY_VERSION_COMPONENTS=(${SECURITY_VERSION//./ })
SECURITY_MAJOR="${SECURITY_VERSION_COMPONENTS[0]}"
SECURITY_MINOR="${SECURITY_VERSION_COMPONENTS[1]}"
        
if (( $SECURITY_MAJOR > 2 || ( $SECURITY_MAJOR == 2 && $SECURITY_MINOR >= 12 ) )); then
    # with -t
else
    # without -t
fi

[derek-ho]

@Xtansia thanks for the suggestion. I used your logic to extract the version, but used the same logic as opensearch-build: https://github.com/opensearch-project/opensearch-build/blob/main/scripts/default/integtest.sh to determine whether it is >= 2.12, just to be consistent. It looks like tests are passing and PR is in a good state. Please merge when it looks good, thanks!

[derek-ho]

Ideally >=2.12.0 should use a strong password, but since this repo is using the scriptfile, it is possible to add this -t parameter and still use admin. This minimizes the surface area of this change.

[Xtansia]

Thanks @derek-ho!