Add a warning info and hard stop for security demo setup

[RyanL1997]

Description

Add a warning info and hard stop for security demo setup

Issues Resolved

• Relate [Security] Demo configuration script requires admin password  #4094

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

Merging #4105 (3203dd1) into main (0fe2554) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #4105   +/-   ##
=======================================
  Coverage   93.27%   93.27%           
=======================================
  Files         187      187           
  Lines        5753     5753           
=======================================
  Hits         5366     5366           
  Misses        387      387           

[gaiksaya]

Hi @RyanL1997

Is this change being backported to 1.x too?

If not @bbarani we need to branch this repository to accommodate these changes.
Also @peterzhuamazon I believe you have better knowledge what to do in terms of rpm, debian, windows zip and docker.

Thanks!

[DarshitChanpura]

This change is meant for 2.11 and above.
There will be subsequent PRs for RPM, DEB, windows and docker.

This PR intentionally only targets TAR to test and verify the change.

[RyanL1997]

Hi @gaiksaya, thanks for double checking. @DarshitChanpura 's reply is right, we do not need this change in 1.x, since the original change in security is only backported into 2.x.

[bbarani]

Hi @gaiksaya, thanks for double checking. @DarshitChanpura 's reply is right, we do not need this change in 1.x, since the original change in security is only backported into 2.x.

This will force us to branch the build workflows as well since its not setup that way at this point in time.

[gaiksaya]

Hi @gaiksaya, thanks for double checking. @DarshitChanpura 's reply is right, we do not need this change in 1.x, since the original change in security is only backported into 2.x.

This will force us to branch the build workflows as well since its not setup that way at this point in time.

+1

Today distribution workflows only runs on main as the code is generic. For config files we have 1.x and 2.x folders specifying the related versions.
With this change, security plugin being a crucial one for running tests, etc, our source code for running integrations tests, bwc tests, etc would change as well. For example, we need to make sure this is taken care before triggering any tests in our test jobs.

Please define an environment variable 'initialAdminPassword' with a password string or create a file 'initialAdminPassword.txt' with a single line that contains the password string and place it under $OPENSEARCH_PATH_CONF 

Also since this only applies to 2.x and not 1.x need to handle that scenario as well. Either via CI or via source code.

[DarshitChanpura]

Today distribution workflows only runs on main as the code is generic. For config files we have 1.x and 2.x folders specifying the related versions. With this change, security plugin being a crucial one for running tests, etc, our source code for running integrations tests, bwc tests, etc would change as well. For example, we need to make sure this is taken care before triggering any tests in our test jobs.

Please define an environment variable 'initialAdminPassword' with a password string or create a file 'initialAdminPassword.txt' with a single line that contains the password string and place it under $OPENSEARCH_PATH_CONF 

Also since this only applies to 2.x and not 1.x need to handle that scenario as well. Either via CI or via source code.

Could you give us some pointers on how to handle and test this?

[gaiksaya]

Could you give us some pointers on how to handle and test this?

I believe we need to plan it properly and handle it from infra side. Branching being the key element here (whether to do it or not OR handle the scenario efficiently based on versions.)
This is the code for all tests https://github.com/opensearch-project/opensearch-build/tree/main/src/test_workflow
You can try to run integration tests on any component and test it out. Instructions should be in the readme.
Basically, setting up the testing cluster for any tests would need some changes.

P.S: Did a rough search on admin keyword. Yielded these results: https://github.com/search?q=repo%3Aopensearch-project%2Fopensearch-build+admin+language%3APython&type=code&l=Python

[rishabh6788]

We can use the new setting to set the password admin to avoid updates to the python code. Right? @gaiksaya

[gaiksaya]

We can use the new setting to set the password admin to avoid updates to the python code. Right? @gaiksaya

That's what I was thinking as well. However, what password you use is not the issue. We need to make code compatible like if version is 1.x run install.sh normally. If 2.x/3.x use env variable or txt file.
Also, if not branching another way to go around is separate all specfiles/config scripts into 1.x and 2.x like you said. That should include everything from tarball, rpm,deb,windows and docker.
There might be duplicates which is okay rather than risking wrong version config.

[DarshitChanpura]

@gaiksaya @bbarani @peterzhuamazon @rishabh6788 What is the path forward to accommodate this change?

[DarshitChanpura]

Is it a possibility that we get this change in now for 2.11, and then fix it later when we release 1.x tarball. This will address the immediate need which is to get it in for 2.x, and we can create a follow-up to fix it for 1.x before next 1.x (which should give enough time to fix it before release i.e. 1.3.14 is scheduled to be release on December 5th)

[bbarani]

Is it a possibility that we get this change in now for 2.11, and then fix it later when we release 1.x tarball. This will address the immediate need which is to get it in for 2.x, and we can create a follow-up to fix it for 1.x before next 1.x (which should give enough time to fix it before release i.e. 1.3.14 is scheduled to be release on December 5th)

It would basically break our automation (nightly builds, benchmarking etc..) as well as for anyone using this code to build 1.x on their own. Also, this would hinder our ability to release patch release in timely manner for 1.x version (if we need to address any critical CVE's) before planned patch release timeline.

[RyanL1997]

Hi @bbarani, thank you and the other folks for all the inputs on this. Yes, we can see the problem now. It seems like we need to address the 1.x's build breaking before we implement the script changes like this since we currently don't have branching strategy for the build flow. (cc: @DarshitChanpura @peternied @davidlago )

[DarshitChanpura]

It would basically break our automation (nightly builds, benchmarking etc..) as well as for anyone using this code to build 1.x on their own. Also, this would hinder our ability to release patch release in timely manner for 1.x version (if we need to address any critical CVE's) before planned patch release timeline.

Makes sense. How do we move forward with this change?

[bbarani]

Hi @bbarani, thank you and the other folks for all the inputs on this. Yes, we can see the problem now. It seems like we need to address the 1.x's build breaking before we implement the script changes like this since we currently don't have branching strategy for the build flow. (cc: @DarshitChanpura @peternied @davidlago )

These are the possible options I can infer from the discussion here,

1.) Create branch for 1.x and 2.x to isolate the build workflow changes to 2.x versions
2.) Separate all specfiles / config scripts into 1.x and 2.x.

Both the above changes would also involve CI, workflow changes as well to consume the code / configs from the right location for corresponding versions.

@gaiksaya @peterzhuamazon @rishabh6788 what do you think is the level of effort for either of the approaches?

[gaiksaya]

These are the possible options I can infer from the discussion here,

1.) Create branch for 1.x and 2.x to isolate the build workflow changes to 2.x versions 2.) Separate all specfiles / config scripts into 1.x and 2.x.

Both the above changes would also involve CI, workflow changes as well to consume the code / configs from the right location for corresponding versions.

@gaiksaya @peterzhuamazon @rishabh6788 what do you think is the level of effort for either of the approaches?

I like option 1. In this way whatever test, build code we have for 1.x remains intact. Also whatever new changes come up (today its 2.11, in future 3.0.0) the way we set up test clusters and run test can be defined and modified per major version.
Level of effort compared to both is also less for option 1. We however need sometime to set up CI and make sure its working as expected.

Option 2 is good in terms of config files. However this does not solve our code compatibility issues (integration, perf tests as well as validation workflows that were newly added recently). Whatever deploys clusters with security would need to be compatible with both 1.x and 2.x. (Too many if else everywhere) Hence more efforts in terms of code changes and compatibilities. Since we do not have 1.x release coming up soon catching the edge cases would be critical too. That would need additional time in testing.

[peternied]

I don't think we need the version specific text - so long as if the script returns a non-zero code the docker install fails, the console output should include the information needed to troubleshoot the issue.

I think this would avoid the git branches or version detection in the near term, what do you think of this notion @gaiksaya?

[RyanL1997]

I was checking with @peterzhuamazon, and it seems like we need to add these to warn the user before we run the flow from build script too, even we had our own lines inside our own script.

I think this would avoid the git branches or version detection in the near term, what do you think of this notion @gaiksaya?

For this, if I was understanding this correct yesterday, we need the branching strategy from the build team no matter we have these specific lines or not because the build team is using their workflows on main to build our 3.x,2.x and 1.x. In another word, everything is relying on main for build repo. Since we are not having our password change in our 1.x line, if we make the change like this, their 1.x build will be broken because the build flow should not be failed/exit on 1.x. (Please correct me if I'm wrong. cc: @gaiksaya @rishabh6788 )

[peternied]

We need the branching strategy from the build team no matter we have these specific lines or not because the build team is using their workflows on main to build our 3.x,2.x and 1.x. In another word, everything is relying on main for build repo. Since we are not having our password change in our 1.x line, if we make the change like this, their 1.x build will be broken because the build flow should not be failed/exit on 1.x

@RyanL1997 Based on this comment it seems like we should close out this pull request and create an issue, then add proposals for how to address these concerns. Then before implementing, make sure to review the plan with folks from opensearch-build. How does that sound?

[DarshitChanpura]

Let's reconvene on how to bring the demo configuration changes in. Until then let's close this PR. We can re-open this once we resume work.