-
Notifications
You must be signed in to change notification settings - Fork 4
feat: run plugin-validator as part of CI (opt-in) #366
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
base: main
Are you sure you want to change the base?
Conversation
…ning from repo root
.github/workflows/ci.yml
Outdated
| echo "${PLUGIN_VALIDATOR_CONFIG_FILE}" > .plugin-validator.yaml | ||
| PLUGIN_VALIDATOR_CONFIG_PATH=.plugin-validator.yaml | ||
| cat ${PLUGIN_VALIDATOR_CONFIG_PATH} | ||
| elif [ -f "${PLUGIN_VALIDATOR_CONFIG_PATH}" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can silently fail, because if the user defined a value here but the file doesn't exits we are now going to use the default,
maybe if the user defines a path (not empty var) and it doesn't exist (-f) we fail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I have changed the inputs so they are considered in the following order:
- Raw yaml config content (if set) has the highest priority
- Then, config file path (if set) is checked. If the file doesn't exist, it now errors out rather than falling back to the hardcoded config. If it's a directory, it errors out as well
- If neither of the above is provided, the default hardcoded config is used instead
| mkdir -p /tmp/empty | ||
| # Do not run clamav because it takes too long (and because it would scan node_modules as well) | ||
| docker run --name=plugin-validator --pull=always \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we are not going to run clamav we might benefit from using npx directly which can be faster than pulling and running the whole docker image? just see if that might speed up things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about semgrep, osv-scanner and gosec? The readme specifies they should be installed:
https://github.com/grafana/plugin-validator?tab=readme-ov-file#security-tools
I am not sure about osv-scanner because it looks like it's being used as a library and it should work even if it's not installed on the system.
semgrep requires the executable: https://github.com/grafana/plugin-validator/blob/e985ef06cef56f260ed0af03fa08b94f495e952a/pkg/analysis/passes/coderules/coderules.go#L95
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are correct. semgrep and gosect require system install. osv-scanner uses the go library so works via npx
Part of #354.
Example runs can be found in the smoke tests for this PR, which have been updated to run the validator.
For example: https://github.com/grafana/plugin-ci-workflows/actions/runs/19139877974/job/54701694400?pr=366
(see the annotations at the top)