-
Notifications
You must be signed in to change notification settings - Fork 100
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
Feature Request: file-by-file checks for Pull Requests #31
base: master
Are you sure you want to change the base?
Conversation
Merge upstream 1.0.512
Feat/pr only scans changed files
Fix/update checkov version to latest as of Feb 2, 2021
update to latest version
Is this something that could be optional with a flag? For us monorepo folks this could be quite time-consuming. |
@@ -1,7 +1,7 @@ | |||
# action.yml | |||
name: 'Checkov Github Action' | |||
author: 'Chris Mavrakis' | |||
description: 'Run Checkov against Terraform/CloudFormation infrastructure code, as a pre-packaged Github Action.' | |||
author: 'Chris Mavrakis, Libertyy' |
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.
Contributors should not add themselves as author
for making singular changes.
do | ||
SCAN_FILES_FLAG="$SCAN_FILES_FLAG -f $f" | ||
done | ||
checkov $SCAN_FILES_FLAG $CHECK_FLAG $SKIP_CHECK_FLAG $QUIET_FLAG $SOFT_FAIL_FLAG $FRAMEWORK_FLAG $EXTCHECK_DIRS_FLAG $EXTCHECK_REPOS_FLAG $OUTPUT_FLAG |
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.
Wouldn't it be more efficient to copy the $DIFF_FILES
to a temporary directory and scan all of them at once instead of individually? For mono-repos this could be a very time consuming (and thus costly) change.
echo "running checkov on directory: $1" | ||
checkov -d $INPUT_DIRECTORY $CHECK_FLAG $SKIP_CHECK_FLAG $QUIET_FLAG $SOFT_FAIL_FLAG $FRAMEWORK_FLAG $EXTCHECK_DIRS_FLAG $EXTCHECK_REPOS_FLAG $OUTPUT_FLAG | ||
|
||
echo $(checkov --version ) |
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.
Why are you arbitrarily echoing the version here with no added context. If this was intended for debugging, it is best to remove it.
@libertyy Hey, are you still planning on working on this PR, or can I close it? |
@krewenki I would say go for it, if there are competing PRs its all about which one gets there (as in gets past approvals) and does the job the best way, first. One thing I'd contribute, is make this type of scan optional, possibly via an input or envar that sets a flag. |
This github action will do full scans against the target repo when a commit is merged into a main/release branch. This action will scan only those files changed in a PullRequest.
this resolves: #16
To test:
In a GitHub PR, this action will report that it scanned only those files (or no files scanned) changed in a PR. When the PR is merged into main, the action will run a full-scan on the target repo.