Pass in strong password instead to fix integtest workflow #4302

derek-ho · 2023-12-21T18:41:06Z

Description

With recent changes in security plugin requiring a strong initial admin password to be set, I've seen a few autocuts with integtest failures. This PR attempts to resolve those by passing in a strong password and using that to run the tests

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.

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

derek-ho · 2023-12-21T18:43:41Z

Am I correct in assuming that we also need to change any repo which has its own integtest.sh file to use default admin:myStrongPassword123! with this change? How do we handle cases in which integtest pass in its own credentials, or we don't need to worry about those?

rishabh6788 · 2023-12-21T19:53:53Z

src/test_workflow/integ_test/distribution_deb.py

@@ -30,6 +30,10 @@ def install(self, bundle_name: str) -> None:
        logging.info("deb installation requires sudo, script will exit if current user does not have sudo access")
        deb_install_cmd = " ".join(
            [
+                'sudo'
+                'env'
+                'OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123!'


This will only set the env variable for the duration of the sudo env command execution and will not persist. Recommend to use export to set the environment variable.

rishabh6788 · 2023-12-21T19:54:06Z

src/test_workflow/integ_test/distribution_rpm.py

@@ -30,6 +30,10 @@ def install(self, bundle_name: str) -> None:
        logging.info("rpm installation requires sudo, script will exit if current user does not have sudo access")
        rpm_install_cmd = " ".join(
            [
+                'sudo'
+                'env'
+                'OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123!'


same as above.

rishabh6788 · 2023-12-21T19:54:44Z

src/test_workflow/integ_test/distribution_tar.py

@@ -33,7 +33,7 @@ def install(self, bundle_name: str) -> None:
    @property
    def start_cmd(self) -> str:
        start_cmd_map = {
-            "opensearch": "./opensearch-tar-install.sh",
+            "opensearch": "OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123! ./opensearch-tar-install.sh",


I believe this should be export OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123! && ./opensearch-tar-install.sh

rishabh6788 · 2023-12-21T19:55:41Z

Added a few comments, please address them.
Also please fix the failing tests.

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

rishabh6788 · 2023-12-21T21:53:03Z

scripts/default/integtest.sh

@@ -89,7 +89,7 @@ fi

 if [ -z "$CREDENTIAL" ]
 then
-  CREDENTIAL="admin:admin"
+  CREDENTIAL="admin:myStrongPassword123!"


This wouldn't work for OS version lower than 2.12.
It may not make much sense to run integration tests for released 2.x versions but it will definitely break integ tests for 1.x line.

Right! The code is generic for all versions as of today. Until we solve the branching problem.

To unblock this we can try adding if/else condition to differentiate with 1.x and above until we have this branching/versioning for the build repo solved. The script already has an argument OPENSEARCH_VERSION so adding if/else should be straight forward.

I think issue with that is what if patch version 2.0 < patch version < 2.12 come along? I was looking into bash script to compare semantic version, but it seems complex, not sure how do you folks feel about adding something like this: https://stackoverflow.com/questions/4023830/how-to-compare-two-strings-in-dot-separated-version-format-in-bash?

@rishabh6788 @gaiksaya @prudhvigodithi I changed the logic in the script itself to set a strong password if the version is after 2.12.0. Let me know if that makes sense to keep. I also included this logic in the integtest.sh files of repo's which have their own versions of integtest.sh.

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

codecov · 2023-12-28T21:41:43Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f92f571) 91.26% compared to head (00fa11f) 91.27%.
Report is 11 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4302   +/-   ##
=======================================
  Coverage   91.26%   91.27%           
=======================================
  Files         189      189           
  Lines        6124     6163   +39     
=======================================
+ Hits         5589     5625   +36     
- Misses        535      538    +3

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

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

peterzhuamazon

This is just the default integTest.sh, there are more in each plugin's repo.

scripts/default/integtest.sh

derek-ho · 2024-01-04T17:08:50Z

This is just the default integTest.sh, there are more in each plugin's repo.

I believe most plugins have removed their custom integTest.sh in favor of this default. There are two repos I found which need their own custom ones, we have open PRs in those, which should be merged prior to 2.12.0 release, correct me if my understanding is incorrect:
opensearch-project/cross-cluster-replication#1298
opensearch-project/opensearch-dashboards-functional-test#981
If you can take a look at these, that would also be awesome! I do not know if their repo's branching strategy is honored. @kavilla suggested we don't need logic in these repos to determine version since those repo's integtest.sh is run against their own branch.

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

peterzhuamazon

Waiting for final decision based on this issue before approval, since it includes both deb and rpm.
opensearch-project/security#3916

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

src/test_workflow/integ_test/distribution_tar.py

peterzhuamazon · 2024-01-09T00:38:57Z

You can either add docker or open a new PR for docker:
https://github.com/opensearch-project/opensearch-build/blob/main/docker/release/config/opensearch/opensearch-docker-entrypoint.sh#L33-L39

derek-ho · 2024-01-09T01:03:39Z

You can either add docker or open a new PR for docker: https://github.com/opensearch-project/opensearch-build/blob/main/docker/release/config/opensearch/opensearch-docker-entrypoint.sh#L33-L39

Is this guy being used in docker integtest? Can you point me to where docker integtest is run, if at all? I would rather merge in this PR as is, since the main purpose is to sniff out if this will solve the autocuts that have been cut against a lot of repos, and close any false alarm that is being raised.

prudhvigodithi · 2024-01-09T02:17:22Z

@derek-ho Merging the PR to make sure 2.12.0 with-security integ tests pass, please keep an eye on failures if there is anything breaking with this change.

DarshitChanpura · 2024-01-09T14:22:29Z

You can either add docker or open a new PR for docker:
https://github.com/opensearch-project/opensearch-build/blob/main/docker/release/config/opensearch/opensearch-docker-entrypoint.sh#L33-L39

This is addressed in this PR: #4274

* Remove instances of hard admin credentials opensearch-project/opensearch-build#4302 Signed-off-by: Derek Ho <[email protected]>

* Remove instances of hard admin credentials opensearch-project/opensearch-build#4302 Signed-off-by: Derek Ho <[email protected]> (cherry picked from commit 8391566)

* Remove instances of hard admin credentials opensearch-project/opensearch-build#4302 Signed-off-by: Derek Ho <[email protected]> (cherry picked from commit 8391566) Co-authored-by: Derek Ho <[email protected]>

Pass in strong password instead to fix integtest workflow

03a719b

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

derek-ho requested review from dblock, peterzhuamazon, bbarani, gaiksaya, rishabh6788, zelinh, jordarlu, prudhvigodithi, Divyaasm and tianleh as code owners December 21, 2023 18:41

github-actions bot added the first-time-contributor label Dec 21, 2023

derek-ho mentioned this pull request Dec 21, 2023

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

Closed

2 tasks

rishabh6788 reviewed Dec 21, 2023

View reviewed changes

derek-ho added 2 commits December 21, 2023 16:00

Modify commands and fix tests

1048e38

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

Fix linting issue

545ddd9

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

rishabh6788 reviewed Dec 21, 2023

View reviewed changes

derek-ho added 3 commits December 28, 2023 16:12

Add logic to differentiate between versions

6722ec8

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

Fix test formatting

e601645

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

Add in commas

48a40e2

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

derek-ho mentioned this pull request Dec 29, 2023

Pass in initial admin password in jenkins verification library opensearch-project/opensearch-build-libraries#357

Closed

Udpate script logic

e9b23ea

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

peterzhuamazon requested changes Jan 2, 2024

View reviewed changes

scripts/default/integtest.sh Outdated Show resolved Hide resolved

peternied mentioned this pull request Jan 5, 2024

[AUTOCUT] Integration Test failed for security: 2.12.0 tar distribution opensearch-project/security#3876

Closed

Add numeral comparison logic from Peter

7480dde

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

DarshitChanpura approved these changes Jan 8, 2024

View reviewed changes

peterzhuamazon reviewed Jan 8, 2024

View reviewed changes

Revert rpm/deb changes pending decision

00fa11f

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

prudhvigodithi approved these changes Jan 8, 2024

View reviewed changes

prudhvigodithi reviewed Jan 8, 2024

View reviewed changes

src/test_workflow/integ_test/distribution_tar.py Show resolved Hide resolved

peterzhuamazon approved these changes Jan 9, 2024

View reviewed changes

prudhvigodithi merged commit 8897650 into opensearch-project:main Jan 9, 2024
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass in strong password instead to fix integtest workflow #4302

Pass in strong password instead to fix integtest workflow #4302

derek-ho commented Dec 21, 2023

derek-ho commented Dec 21, 2023

rishabh6788 Dec 21, 2023

rishabh6788 Dec 21, 2023

rishabh6788 Dec 21, 2023

rishabh6788 commented Dec 21, 2023

rishabh6788 Dec 21, 2023

gaiksaya Dec 21, 2023

prudhvigodithi Dec 22, 2023

derek-ho Dec 22, 2023

derek-ho Dec 28, 2023

codecov bot commented Dec 28, 2023 •

edited

Loading

peterzhuamazon left a comment

derek-ho commented Jan 4, 2024

peterzhuamazon left a comment •

edited

Loading

peterzhuamazon commented Jan 9, 2024

derek-ho commented Jan 9, 2024

prudhvigodithi commented Jan 9, 2024

DarshitChanpura commented Jan 9, 2024

Pass in strong password instead to fix integtest workflow #4302

Pass in strong password instead to fix integtest workflow #4302

Conversation

derek-ho commented Dec 21, 2023

Description

Issues Resolved

derek-ho commented Dec 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rishabh6788 commented Dec 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Dec 28, 2023 • edited Loading

Codecov Report

peterzhuamazon left a comment

Choose a reason for hiding this comment

derek-ho commented Jan 4, 2024

peterzhuamazon left a comment • edited Loading

Choose a reason for hiding this comment

peterzhuamazon commented Jan 9, 2024

derek-ho commented Jan 9, 2024

prudhvigodithi commented Jan 9, 2024

DarshitChanpura commented Jan 9, 2024

codecov bot commented Dec 28, 2023 •

edited

Loading

peterzhuamazon left a comment •

edited

Loading