[go: nahoru, domu]
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.

Sign up for GitHub

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

Jump to bottom

feat: adding scopedenforcementactions #403

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

feat: adding scopedenforcementactions #403

wants to merge 13 commits into from

Conversation

JaydipGabani
Copy link
Contributor
@JaydipGabani JaydipGabani commented Mar 13, 2024 •
edited
Loading

This PR implements changes for multi ea/ep design.

The gists of the changes are:

  • ConstraintToBinding takes a list of enforcement actions instead of determining them (this will be handled by the reconciler code).
  • The client knows which enforcement points it can use. When adding constraints for templates, the constraints store precompiled enforcement actions for each known enforcement point.
  • One of the client's enforcement points must match the sourceEP in Review for enforcement. If no enforcement points are passed in Review, enforcement applies to all points known to the client.
  • Stored constraints are checked against the source of review requests (gator, audit, webhook). Constraints without scopedEnforcementActions are reviewed for all sources.

For gatekeeper changes and CI tests refer to PR gatekeeper/3321

@JaydipGabani JaydipGabani force-pushed the multi-ea branch 2 times, most recently from 0c29c4c to b09049c Compare March 18, 2024 23:01
@JaydipGabani JaydipGabani mentioned this pull request Mar 19, 2024
Open
@JaydipGabani JaydipGabani force-pushed the multi-ea branch 3 times, most recently from 7bea138 to 8a991c7 Compare March 20, 2024 20:28
@JaydipGabani
adding scopedenforcementactions
4aabc45 
Signed-off-by: Jaydip Gabani <gabanijaydip@gmail.com>
@JaydipGabani
adding tests
81b2d08 
Signed-off-by: Jaydip Gabani <gabanijaydip@gmail.com>
@JaydipGabani
fixing tests
9deaab2 
Signed-off-by: Jaydip Gabani <gabanijaydip@gmail.com>
sozercan
sozercan approved these changes Mar 27, 2024
View reviewed changes
Copy link
Member
@sozercan sozercan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

maxsmythe
maxsmythe reviewed Mar 28, 2024
View reviewed changes
Copy link
Contributor
@maxsmythe maxsmythe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! A few comments, but looking good.

constraint/pkg/apis/constraints/apis.go Outdated Show resolved Hide resolved
constraint/pkg/apis/constraints/apis.go Show resolved Hide resolved
constraint/pkg/apis/constraints/apis.go Outdated Show resolved Hide resolved
constraint/pkg/client/client.go Outdated Show resolved Hide resolved
constraint/pkg/client/client.go Show resolved Hide resolved
constraint/pkg/client/constraint_client.go Outdated Show resolved Hide resolved
constraint/pkg/client/drivers/k8scel/driver.go Outdated Show resolved Hide resolved
constraint/pkg/client/drivers/rego/driver.go Outdated Show resolved Hide resolved
constraint/pkg/client/drivers/to_result.go Outdated Show resolved Hide resolved
@JaydipGabani
refactoring template client and query
909437e 
Signed-off-by: Jaydip Gabani <gabanijaydip@gmail.com>
@codecov-commenter
Copy link
codecov-commenter commented Apr 8, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 83.90805% with 42 lines in your changes are missing coverage. Please review.

Project coverage is 53.93%. Comparing base (76869f8) to head (e054366).
Report is 13 commits behind head on master.

Files Patch % Lines
constraint/pkg/apis/constraints/apis.go 60.71% 16 Missing and 6 partials ⚠️
...onstraint/pkg/client/clienttest/cts/constraints.go 83.33% 4 Missing and 2 partials ⚠️
...lient/drivers/k8scel/transform/make_vap_objects.go 83.87% 4 Missing and 1 partial ⚠️
constraint/pkg/client/constraint_client.go 80.00% 2 Missing and 1 partial ⚠️
constraint/pkg/client/template_client.go 85.71% 2 Missing and 1 partial ⚠️
constraint/pkg/types/validation.go 0.00% 2 Missing ⚠️
constraint/pkg/client/clienttest/client.go 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #403      +/-   ##
==========================================
- Coverage   54.68%   53.93%   -0.75%     
==========================================
  Files          71      104      +33     
  Lines        5241     6726    +1485     
==========================================
+ Hits         2866     3628     +762     
- Misses       2073     2729     +656     
- Partials      302      369      +67
Flag Coverage Δ
unittests 53.93% <83.90%> (-0.75%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JaydipGabani
Copy link
Contributor Author

@maxsmythe Sorry for the delay. I have addressed all the comments and implemented the feedback. PTAL.

@JaydipGabani
Merge branch 'master' into multi-ea
e837a1e 
Signed-off-by: Jaydipkumar Arvindbhai Gabani <gabanijaydip@gmail.com>
maxsmythe
maxsmythe previously requested changes Apr 10, 2024
View reviewed changes
Copy link
Contributor
@maxsmythe maxsmythe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries! Sorry for the delay on my end. Getting close!

constraint/pkg/apis/constraints/apis.go Outdated Show resolved Hide resolved
constraint/pkg/apis/constraints/apis.go Show resolved Hide resolved
constraint/pkg/client/client.go Outdated Show resolved Hide resolved
constraint/pkg/client/client_opts.go Outdated Show resolved Hide resolved
constraint/pkg/client/drivers/k8scel/transform/make_vap_objects.go Outdated Show resolved Hide resolved
@JaydipGabani
refatoring queryopts to reviewopts
a6f1641 
Signed-off-by: Jaydip Gabani <gabanijaydip@gmail.com>
@JaydipGabani
updating ccomments
46a172b 
Signed-off-by: Jaydip Gabani <gabanijaydip@gmail.com>
@JaydipGabani
removing EP variables
7a2ff49 
Signed-off-by: Jaydip Gabani <gabanijaydip@gmail.com>
@JaydipGabani
generic webhook EP
e054366 
Signed-off-by: Jaydip Gabani <gabanijaydip@gmail.com>
@JaydipGabani JaydipGabani mentioned this pull request Jun 20, 2024
Merged
@maxsmythe maxsmythe mentioned this pull request Jul 2, 2024
Open
@JaydipGabani
Merge branch 'master' into multi-ea
076751a
@JaydipGabani
simplifying constraintToBinding
8f28a01 
Signed-off-by: Jaydip Gabani <gabanijaydip@gmail.com>
@JaydipGabani
adding comments
1c6b9d8 
Signed-off-by: Jaydip Gabani <gabanijaydip@gmail.com>
sozercan
sozercan approved these changes Jul 10, 2024
View reviewed changes
Copy link
Member
@sozercan sozercan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@JaydipGabani JaydipGabani dismissed maxsmythe’s stale review July 11, 2024 03:30

implemented changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sozercan sozercan sozercan approved these changes

@maxsmythe maxsmythe Awaiting requested review from maxsmythe

@ritazh ritazh Awaiting requested review from ritazh

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@JaydipGabani @codecov-commenter @sozercan @ritazh @maxsmythe