Remove instances of hard admin credentials #981

derek-ho · 2023-12-21T14:04:16Z

Description

In 3.0.0 and backported to 2.12.0 release is a change where security demo configuration requires an initial admin password to be passed in on setup. This PR does that.

Issues Resolved

[List any issues this PR will resolve]

Check List

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.

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

derek-ho · 2023-12-21T14:05:28Z

Can somebody add backport 2.x label?

derek-ho · 2023-12-21T14:06:57Z

Can somebody review my deletion of integtest.sh file? Is that unused in this repo? The idea is coming from: opensearch-project/opensearch-build#497, in which it seems this file should have been migrated to build repo. Additionally it contains reference to admin:admin so I removed it, but can bring it back if that's wrong/is actively being used by this repo somehow.

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

Hailong-am · 2023-12-21T15:12:17Z

.github/workflows/release-e2e-workflow-template-windows.yml

@@ -62,8 +62,8 @@ jobs:
          else
              echo "Keep OpenSearch Security"
              nohup ./opensearch-windows-install.bat &
-              timeout 900 bash -c 'while [[ "$(curl -o /dev/null -w ''%{http_code}'' -u admin:admin -k https://localhost:9200)" != "200" ]]; do echo sleeping 5; sleep 5; done'
-              curl -sk https://localhost:9200/_cluster/health?pretty -u admin:admin
+              timeout 900 bash -c 'while [[ "$(curl -o /dev/null -w ''%{http_code}'' -u admin:${{ env.OPENSEARCH_INITIAL_ADMIN_PASSWORD }} -k https://localhost:9200)" != "200" ]]; do echo sleeping 5; sleep 5; done'


seems that env.OPENSEARCH_INITIAL_ADMIN_PASSWORD is not defined

You're right, I was accidentally editing a backport branch, fixed now

Hailong-am · 2023-12-21T15:13:11Z

#978

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

kavilla · 2023-12-21T15:26:24Z

integtest.sh

whoops. this is the one jenkins executes.

source

Got it. I found this: https://github.com/opensearch-project/opensearch-build/blob/f92f571a42eecfc3279c217c5bd53922517054d0/scripts/components/functionalTestDashboards/build.sh. I will revert this deletion. @kavilla can you help me understand what/if any changes need to be made in integtest.sh in terms of the admin:admin credentials? I feel like there is some assumption about the setup of the OpenSearch backend that FTR would be running against, but I don't know where that is setup?

this line: https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/integtest.sh#L83

yeah that line just need to replace it with admin:urnewpassword if that is the expected password. basically the integtest.sh script gets executed after an OpenSearch and OpenSearch Dashboards of the same version and successful builds get scaled up. Then Jenkins will clone this repo and execute integtest.sh. So as long as you change that password in integtest.sh it will be the credential passed when the tests are executed.

Do you have any idea about where those successful builds are generated? It seems like less work to modify that place to get spun up with admin:admin, so none of the component integtest.sh need to be changed.

kavilla

integtest.sh

SuZhou-Joe · 2023-12-22T10:04:28Z

#982 for checking if all tests run successfully if security enabled.

SuZhou-Joe · 2023-12-25T02:23:47Z

@derek-ho seems the OSD in main fails to launch according to #982 , do you know who we can ask to take a look?

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

derek-ho · 2023-12-27T21:39:43Z

@kavilla @SuZhou-Joe can you take another look? Added logic to handle the logic of 2.12.0 release and onwards.

derek-ho · 2023-12-27T21:41:35Z

@derek-ho seems the OSD in main fails to launch according to #982 , do you know who we can ask to take a look?

Let me check this

SuZhou-Joe · 2023-12-28T03:02:10Z

@kavilla @SuZhou-Joe can you take another look? Added logic to handle the logic of 2.12.0 release and onwards.

LGTM

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

SuZhou-Joe · 2024-01-04T01:15:34Z

@kavilla , could you please take a look on this PR and unblock the requested change if it looks good to you?

kavilla · 2024-01-04T02:16:40Z

integtest.sh

-  USERNAME=`echo $CREDENTIAL | awk -F ':' '{print $1}'`
-  PASSWORD=`echo $CREDENTIAL | awk -F ':' '{print $2}'`
+  # 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


imo i dont think this need. I'm not positive if version is being passed I can check but ideally. This file will be checked out via branches or tags. So since this is on main and main is expected to 3.0.0. Which just need to make sure we backport this to 2.x then it's in the 2.12 release branch. And then dont backport any further.

One other thing is I'm not positive how this password gets set. So when the integration tests get ran they get executed after the cluster is started.

Originally this takes from an environment variable that needs to be set pre opensearch-tar-install is that correct?

I am not sure. I asked this question in a comment in a PR in the build repo. However, this seems plausible to me. I think in the build repo the default one is used on all branches so we need to add that logic there. I removed it for now, pending confirmation from build team that this would be checked out per branch.

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

derek-ho · 2024-01-09T20:41:31Z

@kavilla can you re-review and merge if it looks good? I applied your suggestions and this seems to be blocking a few other PRs and workflows, thanks!

* 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]>

Remove instances of hard admin credentials

319519a

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

derek-ho requested review from xluo-aws, Hailong-am, SuZhou-Joe, ruanyl, wanglam and raintygao as code owners December 21, 2023 14:04

Fix typo

65de524

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

Hailong-am added the backport 2.x label Dec 21, 2023

Hailong-am reviewed Dec 21, 2023

View reviewed changes

Add env variable

a8f3ef5

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

Hailong-am approved these changes Dec 21, 2023

View reviewed changes

kavilla reviewed Dec 21, 2023

View reviewed changes

kavilla requested changes Dec 21, 2023

View reviewed changes

SuZhou-Joe approved these changes Dec 22, 2023

View reviewed changes

Add back integtest.sh file

b54cf8b

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

Update logic to be the same across all repos

3f726de

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

wanglam mentioned this pull request Dec 29, 2023

[BUG] Release signoff for bundled OpenSearch Dashboards with Dashboards plugins always failed #985

Closed

Revert logic

81ac58a

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

DarshitChanpura mentioned this pull request Jan 2, 2024

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

Closed

2 tasks

kavilla reviewed Jan 4, 2024

View reviewed changes

derek-ho mentioned this pull request Jan 4, 2024

Pass in strong password instead to fix integtest workflow opensearch-project/opensearch-build#4302

Merged

derek-ho added 2 commits January 5, 2024 10:55

Remove version logic

61bbb49

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

Merge branch 'main' into admin

a3c9a07

This was referenced Jan 9, 2024

Add functional test for datasources at observability dashboards plugin #987

Merged

Add minimal Cypress tests for integrations #990

Merged

kavilla self-requested a review January 10, 2024 17:56

kavilla approved these changes Jan 10, 2024

View reviewed changes

kavilla merged commit 8391566 into opensearch-project:main Jan 10, 2024
36 checks passed

opensearch-trigger-bot bot mentioned this pull request Jan 10, 2024

[Backport 2.x] Remove instances of hard admin credentials #992

Merged

xluo-aws mentioned this pull request Jan 23, 2024

Handle security related changes to default password #913

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove instances of hard admin credentials #981

Remove instances of hard admin credentials #981

derek-ho commented Dec 21, 2023 •

edited

Loading

derek-ho commented Dec 21, 2023

derek-ho commented Dec 21, 2023

Hailong-am Dec 21, 2023

derek-ho Dec 21, 2023

Hailong-am commented Dec 21, 2023

kavilla Dec 21, 2023 •

edited

Loading

derek-ho Dec 21, 2023

derek-ho Dec 21, 2023

kavilla Dec 21, 2023

derek-ho Dec 21, 2023

kavilla left a comment

SuZhou-Joe commented Dec 22, 2023

SuZhou-Joe commented Dec 25, 2023

derek-ho commented Dec 27, 2023

derek-ho commented Dec 27, 2023

SuZhou-Joe commented Dec 28, 2023

SuZhou-Joe commented Jan 4, 2024

kavilla Jan 4, 2024

derek-ho Jan 5, 2024

derek-ho commented Jan 9, 2024

Remove instances of hard admin credentials #981

Remove instances of hard admin credentials #981

Conversation

derek-ho commented Dec 21, 2023 • edited Loading

Description

Issues Resolved

Check List

derek-ho commented Dec 21, 2023

derek-ho commented Dec 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hailong-am commented Dec 21, 2023

kavilla Dec 21, 2023 • edited Loading

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

kavilla left a comment

Choose a reason for hiding this comment

SuZhou-Joe commented Dec 22, 2023

SuZhou-Joe commented Dec 25, 2023

derek-ho commented Dec 27, 2023

derek-ho commented Dec 27, 2023

SuZhou-Joe commented Dec 28, 2023

SuZhou-Joe commented Jan 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derek-ho commented Jan 9, 2024

derek-ho commented Dec 21, 2023 •

edited

Loading

kavilla Dec 21, 2023 •

edited

Loading