-
Notifications
You must be signed in to change notification settings - Fork 474
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
🌱 Add branch protection evaluation #3759
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3759 +/- ##
==========================================
- Coverage 75.36% 68.60% -6.76%
==========================================
Files 232 234 +2
Lines 15682 15842 +160
==========================================
- Hits 11818 10868 -950
- Misses 3129 4302 +1173
+ Partials 735 672 -63 |
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.
only a partial review (ran out of time before lunch), but wanted to get a few of these questions to you for discussion. Particularly the "protected" branch value/probe discussion
99a2df2
to
ba40d03
Compare
#3759 (comment) fixed in ba40d03 |
7d8ce6f
to
b05d177
Compare
The failing integration test looks unrelated, but reliably reproduces:
|
I think this had to do with today's GitHub status incident. |
Signed-off-by: Adam Korczynski <adam@adalogics.com>
Signed-off-by: Adam Korczynski <adam@adalogics.com>
Signed-off-by: Adam Korczynski <adam@adalogics.com>
Signed-off-by: Adam Korczynski <adam@adalogics.com>
Signed-off-by: Adam Korczynski <adam@adalogics.com>
Signed-off-by: Adam Korczynski <adam@adalogics.com>
Signed-off-by: AdamKorcz <adam@adalogics.com>
Signed-off-by: AdamKorcz <adam@adalogics.com>
Signed-off-by: AdamKorcz <adam@adalogics.com>
4a9dcd4
to
a4896eb
Compare
I'm seeing weird reversions of commits from earlier discussions. For example: And one e2e test is still failing TOKEN_TYPE=PAT ginkgo --focus-file branch_protection_test.go e2e
[FAILED] branch protection accessible patch: (-expected +actual) utests.TestReturn{
Error: nil,
Score: 1,
- NumberOfWarn: 3,
+ NumberOfWarn: 6,
NumberOfInfo: 4,
- NumberOfDebug: 4,
+ NumberOfDebug: 8,
} |
This was discussed previously, but accidentally reverted Signed-off-by: Spencer Schrock <sschrock@google.com>
This reverts commit 00acf66. Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
this was previously included by the old evaluation code Signed-off-by: Spencer Schrock <sschrock@google.com>
requiring codeowners without a corresponding file used to give 1 INFO and 1 WARN now it only gives 1 WARN Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
/scdiff generate Branch-Protection |
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.
* 🌱 Add branch protection evaluation Signed-off-by: Adam Korczynski <adam@adalogics.com> * make helper for getting the branchName Signed-off-by: Adam Korczynski <adam@adalogics.com> * move check for branch name Signed-off-by: Adam Korczynski <adam@adalogics.com> * define size of slice Signed-off-by: Adam Korczynski <adam@adalogics.com> * add probe for protected branches. Signed-off-by: Adam Korczynski <adam@adalogics.com> * change 'basicNonAdminProtection' to 'deleteAndForcePushProtection' Signed-off-by: Adam Korczynski <adam@adalogics.com> * fix markdown in text field in def.yml Signed-off-by: Adam Korczynski <adam@adalogics.com> * remove duplicate conditional Signed-off-by: Adam Korczynski <adam@adalogics.com> * remove redundant 'protected' value from 'requiresCodeOwnersReview' probe Signed-off-by: Adam Korczynski <adam@adalogics.com> * remove protected values from probes Signed-off-by: Adam Korczynski <adam@adalogics.com> * Bring back negative outcome in case of 0 codeowners files Signed-off-by: Adam Korczynski <adam@adalogics.com> * log based on whether branches are protected Signed-off-by: Adam Korczynski <adam@adalogics.com> * remove unnecessary test Signed-off-by: Adam Korczynski <adam@adalogics.com> * debug failing tests Signed-off-by: Adam Korczynski <adam@adalogics.com> * Fix failing tests Signed-off-by: Adam Korczynski <adam@adalogics.com> * rename test Signed-off-by: Adam Korczynski <adam@adalogics.com> * update to with latest upstream changes Signed-off-by: AdamKorcz <adam@adalogics.com> * fix linting issues Signed-off-by: AdamKorcz <adam@adalogics.com> * remove tests that represent impossible scenarios Signed-off-by: AdamKorcz <adam@adalogics.com> * remove protected finding value This was discussed previously, but accidentally reverted Signed-off-by: Spencer Schrock <sschrock@google.com> * Revert "debug failing tests" This reverts commit 00acf66. Signed-off-by: Spencer Schrock <sschrock@google.com> * use branchName key for branch name Signed-off-by: Spencer Schrock <sschrock@google.com> * include number of reviews in INFO this was previously included by the old evaluation code Signed-off-by: Spencer Schrock <sschrock@google.com> * reduce info count by 1 requiring codeowners without a corresponding file used to give 1 INFO and 1 WARN now it only gives 1 WARN Signed-off-by: Spencer Schrock <sschrock@google.com> --------- Signed-off-by: Adam Korczynski <adam@adalogics.com> Signed-off-by: AdamKorcz <adam@adalogics.com> Signed-off-by: Spencer Schrock <sschrock@google.com> Co-authored-by: Spencer Schrock <sschrock@google.com>
What kind of change does this PR introduce?
(Is it a bug fix, feature, docs update, something else?)
What is the current behavior?
What is the new behavior (if this is a feature change)?**
Changes Branch Protection evaluation to process probe findings. Some of the probes were submitted here: 🌱 Add probes for Branch Protection #3691
Adds another probe
Rewrites a bunch of tests
Tests for the changes have been added (for bug fixes/features)
Which issue(s) this PR fixes
NONE
Special notes for your reviewer
Does this PR introduce a user-facing change?
NONE
For user-facing changes, please add a concise, human-readable release note to
the
release-note
(In particular, describe what changes users might need to make in their
application as a result of this pull request.)