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.

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

Implement Target cache hook #190

Merged
merged 8 commits into from
Feb 10, 2022

Conversation

willbeason
Copy link
Member

Targets sometimes need to maintain state about the system. For example,
the Gatekeeper target needs to track the set of current Namespaces on
the cluster in order to properly match objects to Constraints when Audit
is called.

This commit adds a Cache interface which Targets may choose to
implement. If they implement this interface, Client attempts to add and
remove objects from the Target cache just as it does for Driver caches.

These operations are not atomic, so it is possible for systems to get
into an inconsistent state. There isn't a good solution to this now -
I've opened #189 to solve this in the future. The implications are quite
complex and there's a lot of edge cases.

This commit also modifies the test target handler matchers - they now
access the test target's cache in order to function. These matchers
aren't called yet - we don't want to break Gatekeeper since Gatekeeper
Golang matchers are not yet implemented.

Signed-off-by: Will Beason willbeason@google.com

Targets sometimes need to maintain state about the system. For example,
the Gatekeeper target needs to track the set of current Namespaces on
the cluster in order to properly match objects to Constraints when Audit
is called.

This commit adds a Cache interface which Targets may choose to
implement. If they implement this interface, Client attempts to add and
remove objects from the Target cache just as it does for Driver caches.

These operations are not atomic, so it is possible for systems to get
into an inconsistent state. There isn't a good solution to this now -
I've opened open-policy-agent#189 to solve this in the future. The implications are quite
complex and there's a lot of edge cases.

This commit also modifies the test target handler matchers - they now
access the test target's cache in order to function. These matchers
aren't called yet - we don't want to break Gatekeeper since Gatekeeper
Golang matchers are not yet implemented.

Signed-off-by: Will Beason <willbeason@google.com>
Will Beason added 3 commits January 27, 2022 10:29
Signed-off-by: Will Beason <willbeason@google.com>
Otherwise it is easy to get into inconsistent cache states. There's lots
of edge cases that can cause unpredictable behaviors that we don't want
to allow.

Signed-off-by: Will Beason <willbeason@google.com>
Since adding data can fail in the target cache, remove data from the
driver cache.

Note that addition/deletion occur in opposite orders for AddData and
RemoveData - this is because we want to prioritize reversible over
potentially-irreversible operations. Removing data from the handler
cache can't fail, so it is safe to add it first.

Signed-off-by: Will Beason <willbeason@google.com>
@codecov-commenter
Copy link

codecov-commenter commented Jan 27, 2022

Codecov Report

Merging #190 (8623d48) into master (83f5c4f) will increase coverage by 0.20%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #190      +/-   ##
==========================================
+ Coverage   47.89%   48.10%   +0.20%     
==========================================
  Files          59       59              
  Lines        2856     2871      +15     
==========================================
+ Hits         1368     1381      +13     
- Misses       1239     1241       +2     
  Partials      249      249              
Flag Coverage Δ
unittests 48.10% <ø> (+0.20%) ⬆️

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

Impacted Files Coverage Δ
...y-agent/frameworks/constraint/pkg/client/client.go 74.22% <0.00%> (+0.55%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 83f5c4f...8623d48. Read the comment docs.

Will Beason added 2 commits January 28, 2022 09:53
Otherwise we can easily end up in very annoying inconsistent states. If
deleteion really, really needs to fail then the application should panic
rather than allow things to get in an inconsistent state.

Per discussion

Signed-off-by: Will Beason <willbeason@google.com>
@willbeason
Copy link
Member Author

Fixes #194

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.

LGTM

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.

LGTM

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

@willbeason willbeason merged commit cd1085b into open-policy-agent:master Feb 10, 2022
@willbeason willbeason deleted the golang-autoreject branch February 10, 2022 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants