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.

Sign up for GitHub

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

Pass in strong password instead to fix integtest workflow #4302

Merged
merged 9 commits into from
Jan 9, 2024
Merged

Pass in strong password instead to fix integtest workflow #4302

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
03a719b
Pass in strong password instead to fix integtest workflow
derek-ho Dec 21, 2023
1048e38
Modify commands and fix tests
derek-ho Dec 21, 2023
545ddd9
Fix linting issue
derek-ho Dec 21, 2023
6722ec8
Add logic to differentiate between versions
derek-ho Dec 28, 2023
e601645
Fix test formatting
derek-ho Dec 28, 2023
48a40e2
Add in commas
derek-ho Dec 28, 2023
e9b23ea
Udpate script logic
derek-ho Dec 29, 2023
7480dde
Add numeral comparison logic from Peter
derek-ho Jan 8, 2024
00fa11f
Revert rpm/deb changes pending decision
derek-ho Jan 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scripts/default/integtest.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ fi

if [ -z "$CREDENTIAL" ]
then
CREDENTIAL="admin:admin"
CREDENTIAL="admin:myStrongPassword123!"
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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.

fi

USERNAME=`echo $CREDENTIAL | awk -F ':' '{print $1}'`
Expand Down
4 changes: 4 additions & 0 deletions src/test_workflow/integ_test/distribution_deb.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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!'
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

'&&'
'sudo',
'dpkg',
'--purge',
Expand Down
4 changes: 4 additions & 0 deletions src/test_workflow/integ_test/distribution_rpm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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!'
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above.

'&&'
'sudo',
'yum',
'remove',
Expand Down
2 changes: 1 addition & 1 deletion src/test_workflow/integ_test/distribution_tar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

"opensearch-dashboards": "./opensearch-dashboards",
}
return start_cmd_map[self.filename]
Expand Down
2 changes: 1 addition & 1 deletion src/test_workflow/integ_test/distribution_zip.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def install(self, bundle_name: str) -> None:
@property
def start_cmd(self) -> str:
start_cmd_map = {
"opensearch": ".\\opensearch-windows-install.bat",
"opensearch": "set OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123! .\\opensearch-windows-install.bat",
"opensearch-dashboards": ".\\opensearch-dashboards.bat",
}
return start_cmd_map[self.filename]
Expand Down