-
Notifications
You must be signed in to change notification settings - Fork 755
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
feat: support expansion in gator verify #3650
base: master
Are you sure you want to change the base?
feat: support expansion in gator verify #3650
Conversation
Signed-off-by: David-Jaeyoon-Lee <[email protected]>
Signed-off-by: David-Jaeyoon-Lee <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3650 +/- ##
==========================================
- Coverage 54.49% 47.75% -6.75%
==========================================
Files 134 221 +87
Lines 12329 18756 +6427
==========================================
+ Hits 6719 8956 +2237
- Misses 5116 8949 +3833
- Partials 494 851 +357
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Are we going to add |
Signed-off-by: David-Jaeyoon-Lee <[email protected]>
Signed-off-by: David-Jaeyoon-Lee <[email protected]>
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.
couple small nits
@@ -9,6 +9,9 @@ var ( | |||
// ErrNotAConstraint indicates the user-indicated file does not contain a | |||
// Constraint. | |||
ErrNotAConstraint = errors.New("not a Constraint") | |||
// ErrNotAnExpansion indicates the user-indicated file does not contain a |
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.
nit: "an ExpansionTemplate"
} | ||
|
||
gvk := u.GroupVersionKind() | ||
if gvk.Group != "expansion.gatekeeper.sh" { |
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.
We should test both group and kind here (we don't do that for constraints because constraints have an unknown set of kinds that all live under the same group)
@@ -327,7 +364,32 @@ func (r *Runner) runReview(ctx context.Context, newClient func() (gator.Client, | |||
Object: *toReview, | |||
Source: mutationtypes.SourceTypeOriginal, | |||
} | |||
return c.Review(ctx, au, reviews.EnforcementPoint(util.GatorEnforcementPoint)) | |||
|
|||
review, err := c.Review(ctx, au, reviews.EnforcementPoint(util.GatorEnforcementPoint)) |
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.
As a general rule in golang you should return ASAP on error.
What this PR does / why we need it:
It allows for expansion in gator verify.
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when the PR gets merged):Fixes #3432
Special notes for your reviewer: