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.

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

Create a Github Action to ensure variables in shell scripts are set #3343

Open
owaiskazi19 opened this issue May 16, 2022 · 10 comments
Open
Assignees
Labels
Build Libraries & Interfaces Build Build Tasks/Gradle Plugin, groovy scripts, build tools, Javadoc enforcement. enhancement Enhancement or improvement to existing feature or request good first issue Good for newcomers

Comments

@owaiskazi19
Copy link
Member

Is your feature request related to a problem? Please describe.
Coming from #3106 and #3278 (comment). Create a Github Action to run on pull request to ensure whether shell scripts variables are set.

Describe the solution you'd like
Can use https://github.com/marketplace/actions/shellcheck for the same.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

@owaiskazi19 owaiskazi19 added enhancement Enhancement or improvement to existing feature or request untriaged Build Libraries & Interfaces and removed untriaged labels May 16, 2022
@dblock
Copy link
Member

dblock commented May 16, 2022

Maybe there's a unit test framework for these shell scripts?

@owaiskazi19
Copy link
Member Author

owaiskazi19 commented May 16, 2022

yamlRestTest takes care of most of the shell scripts during gradle check. GHA will be an added layer to ensure the shell scripts are more secure.

@rursprung
Copy link
Contributor

note that my suggestion was not to create a check just for unset variables.

instead, my suggestion was to create a check which ensures that set -eu -o pipefile (or whatever is the defined standard for OpenSearch) is present in all .sh files.

i do like the ShellCheck action, though, it looks nice!

@minalsha
Copy link
Contributor

minalsha commented Jun 7, 2022

@owaiskazi19 any proposed solutions you have that you can document so that someone can pick it up?

@owaiskazi19
Copy link
Member Author

Hey @minalsha! The GHA which can be used to achieve this is in the description.

@laysauchoa
Copy link

Hi @owaiskazi19! I am a first time contributor to this project. I'm familiar with GitHub actions, but I am not sure if my PR is correct. Would be possible to guide me a bit on this? I'd love to contribute to the OpenSearch project. Many thanks!

@owaiskazi19
Copy link
Member Author

owaiskazi19 commented Jul 28, 2022

Hi @owaiskazi19! I am a first time contributor to this project. I'm familiar with GitHub actions, but I am not sure if my PR is correct. Would be possible to guide me a bit on this? I'd love to contribute to the OpenSearch project. Many thanks!

Sure thing @laysauchoa. Welcome to OpenSearch!
We wanted to have strict check for the new scripts which will get added later to the project. The way we handled for OpenSearch repo based on the suggestion by @rursprung (thanks for spending time on it) for the current shell scripts is by having flags set -e -o pipefail.
The Github Action should verify whether the above mentioned flags are present in the script in the PR, and should fail if not. 1. Can you start exploring more on verifying these flags if present in the script?
2. We wanted to have this GHA across all the projects under @opensearch-project.

@minalsha
Copy link
Contributor

minalsha commented Mar 3, 2023

HI @laysauchoa , just following up on this issue. Do you need any help contributing to this issue?

@bl1nkker
Copy link

bl1nkker commented Apr 7, 2023

Can I take care of this issue? looks like something interesting

@owaiskazi19 owaiskazi19 assigned bl1nkker and unassigned laysauchoa Apr 7, 2023
@owaiskazi19
Copy link
Member Author

Can I take care of this issue? looks like something interesting

@bl1nkker Sure. I have assigned you the issue and there's a closed PR attached for you to refer. Let us know if you need any additional information. Thanks

@peterzhuamazon peterzhuamazon added the Build Build Tasks/Gradle Plugin, groovy scripts, build tools, Javadoc enforcement. label Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Libraries & Interfaces Build Build Tasks/Gradle Plugin, groovy scripts, build tools, Javadoc enforcement. enhancement Enhancement or improvement to existing feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

7 participants