Command-line only plugin install action

[stephen-crawford]

Description

This PR builds on the recent PR for adding a plugin install action. This version drops the docker use from the Ubuntu runners and relies entirely on a standardized process for both operating systems.

Testing

Passes CI and successfully completes install action on Windows and Linux with JDK 11 and JDK 17.

Check List

• New functionality includes testing
• New functionality has been documented
• 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.

[codecov-commenter]

Codecov Report

Merging #2266 (10da06a) into main (e36d396) will decrease coverage by 0.07%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #2266      +/-   ##
============================================
- Coverage     61.13%   61.06%   -0.08%     
+ Complexity     3271     3270       -1     
============================================
  Files           259      259              
  Lines         18337    18337              
  Branches       3248     3248              
============================================
- Hits          11210    11197      -13     
- Misses         5542     5556      +14     
+ Partials       1585     1584       -1     

Impacted Files	Coverage Δ
...t/keybyoidc/AuthenticatorUnavailableException.java	0.00% <0.00%> (-20.00%)	⬇️
.../auth/http/jwt/keybyoidc/SelfRefreshingKeySet.java	59.85% <0.00%> (-8.46%)	⬇️
...ic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java	55.81% <0.00%> (-3.49%)	⬇️
...ecurity/configuration/ConfigurationRepository.java	74.31% <0.00%> (+2.18%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[DarshitChanpura]

Looks good @scrawfor99 !

A general comment: See if we can have a variable that is initialized with -SNAPSHOT or <empty-string> based on the parameter passed to the script and replace current use of -SNAPSHOT with that variable. Thoughts?

In action.yml:

..
..
runs:
  using: "composite"
  env:
     useSnapshot: ${{ fromJSON('{"false": "", "true": "-SNAPSHOT"}')[inputs.use-snapshot == 'true'] }}
     # Replace all -SNAPSHOT with this variable
..
..

In plugin-install.yml:

..
..
plugin-start-script: install_demo_configuration
use-snapshot: true
..
..

Maybe this works, maybe it doesn't. But if its takes long to make this change then its not worth it.

[peternied]

Why change the source? https://ci.opensearch.org/... [1] is the freshest place to get them from according to the OpenSearch-Build team [2]

[1] https://github.com/opensearch-project/opensearch-build/blob/main/README.md#latest-distribution-url
[2] opensearch-project/opensearch-build#2754

[stephen-crawford]

So I swapped the source for two reasons: 1) We do not have the same type of catalogue for Windows as we do Linux so what was available to Linux was not for Windows which made the processes for the two more divergent then I liked 2) Related to that, the most recent distributions seemed to have security already installed which would require that I make the script uninstall and reinstall the plugin to actually test what they are meant to test. Likewise, they probably would have had other plugins installed and I am not sure how much I could account for the differences in what those install and removal processes looked like. With the minimums snapshots, there is basically a barebones framework of OpenSearch to work with which I think makes it easier to install into but also gives you a better idea of whether your particular plugin is working right.

Good question though--I actually thought a bit about this.

[peternied]

the most recent distributions seemed to have security already installed

That doesn't sound right, the opensearch-min-* is suppose to be only the core version of OpenSearch with no plugins, could you see about making a bug on OpenSearch-Build if that is the case.

[stephen-crawford]

Update: I spent today trying to incorporate the ability to define a setup.sh/.bat script in the workflow into this command-line only install action. I got it working on Linux with no docker use. The issue I am running into that prevents Windows from working is described in detail here https://superuser.com/questions/1753564/how-to-pass-multiple-automatic-reponses-to-user-prompts-between-batch-files-usin.

If anyone would like to look at Linux version which does work it can be viewed on https://github.com/scrawfor99/security/tree/SetupForPluginInstall and then by swapping the os in the workflow matrix to 'ubuntu-latest' instead of windows. I also went ahead and created two files that can be used to replicate the scenario on a local windows machine or a remote desktop. The contents of the two are pasted below (they are really short)

test.bat:

@echo off
echo                                     
echo.
echo Yes or No? (y/n)
set /p Input=Enter Yes or No:
If /I "%Input%"=="y" goto yes1
goto no1
:yes1
echo You said yes 
goto yorn1
:no1
echo You said no
goto yorn1
:yorn1
echo Yes or No? (y/n)
set /p Input=Enter Yes or No:
If /I "%Input%"=="y" goto yes2
goto no2
:yes2
echo You said yes 
goto yorn2
:no2
echo You said no
goto yorn2
:yorn2
echo Yes or No? (y/n)
set /p Input=Enter Yes or No:
If /I "%Input%"=="y" goto yes3
goto no3
:yes3
echo You said yes 
goto exitprocess
:no3
echo You said no 
goto exitprocess
:exitprocess

run_test.bat:

(echo y&&echo y&&echo n) | .\test.bat

It should work if you can call .\run_test.bat in the same directory as the test.bat file and see responses that say you said 'yes' twice and 'no' once.

[stephen-crawford]

Looks good @scrawfor99 !

A general comment: See if we can have a variable that is initialized with -SNAPSHOT or <empty-string> based on the parameter passed to the script and replace current use of -SNAPSHOT with that variable. Thoughts?

In action.yml:

..
..
runs:
  using: "composite"
  env:
     useSnapshot: ${{ fromJSON('{"false": "", "true": "-SNAPSHOT"}')[inputs.use-snapshot == 'true'] }}
     # Replace all -SNAPSHOT with this variable
..
..

In plugin-install.yml:

..
..
plugin-start-script: install_demo_configuration
use-snapshot: true
..
..

Maybe this works, maybe it doesn't. But if its takes long to make this change then its not worth it.

Thank you for your review @DarshitChanpura!

For the SNAPSHOT, I think that it is always going to be there so long as we use the distributions I selected. If you check out the response I sent to Peter, there were a couple reasons I chose to make both Linux and Windows runners use the min distributions. I am not sure what you guys will think but for now I think they should always have SNAPSHOT as a suffix. I can swap to your suggestion if that changes though!

[DarshitChanpura]

Thank you for your review @DarshitChanpura!

For the SNAPSHOT, I think that it is always going to be there so long as we use the distributions I selected. If you check out the response I sent to Peter, there were a couple reasons I chose to make both Linux and Windows runners use the min distributions. I am not sure what you guys will think but for now I think they should always have SNAPSHOT as a suffix. I can swap to your suggestion if that changes though!

yes having SNAPSHOT is absolutely ok. My question is more in terms of defining a boolean variable that can be passed dynamically and based on that the script decides whether to append -SNAPSHOT after the version

[stephen-crawford]

Closing in favor of #2271