Skip to content

Commit

Permalink
Add type for matching Constraints (#193)
Browse files Browse the repository at this point in the history
* Implement Target cache hook

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>

* Fix merge conflicts

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

* Remove ability for caches to fail deleting

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>

* Make addition atomic

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>

* Make it impossible for handler caches to fail deletion

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>

* Add type for matching Constraints

Prep work for compiler sharding. This adds a store of the matchers
corresponding to Constraints by-target and by-kind.

We could do constriant matching per-target, but it's better for us to
fail fast rather than partially review objects when we're just going to
return errors. Thus, ConstraintsFor iterates over all targets itself
rather than requiring its caller to.

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

* Split out per-handler matching

Each Handler potentially has its own review type to be matched against,
so this makes requests for matching happen per-target.

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

* Resolve reviewer comments

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

* Fix Remove test code

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

* Fix other tests

Signed-off-by: Will Beason <willbeason@google.com>
  • Loading branch information
Will Beason committed Feb 14, 2022
1 parent 2c8fe2d commit 811b909
Show file tree
Hide file tree
Showing 4 changed files with 856 additions and 9 deletions.
173 changes: 173 additions & 0 deletions constraint/pkg/client/matchers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
package client

import (
"fmt"
"sort"
"sync"

"github.com/open-policy-agent/frameworks/constraint/pkg/client/errors"
"github.com/open-policy-agent/frameworks/constraint/pkg/core/constraints"
)

// matcherKey uniquely identifies a Matcher.
// For a given Constraint (uniquely identified by Kind/Name), there is at most
// one Matcher for each Target.
type matcherKey struct {
target string
kind string
name string
}

// constraintMatchers tracks the Matchers for each Constraint.
// Filters Constraints relevant to a passed review.
// Threadsafe.
type constraintMatchers struct {
// matchers is the set of Constraint matchers by their Target, Kind, and Name.
// matchers is a map from Target to a map from Kind to a map from Name to matcher.
matchers map[string]targetMatchers

mtx sync.RWMutex
}

// Add inserts the Matcher for the Constraint with kind and name.
// Replaces the current Matcher if one already exists.
func (c *constraintMatchers) Add(key matcherKey, matcher constraints.Matcher) {
c.mtx.Lock()
defer c.mtx.Unlock()

if c.matchers == nil {
c.matchers = make(map[string]targetMatchers)
}

target := c.matchers[key.target]
target.Add(key, matcher)

c.matchers[key.target] = target
}

// Remove deletes the Matcher for the Constraint with kind and name.
// Returns normally if no entry for the Constraint existed.
func (c *constraintMatchers) Remove(key matcherKey) {
c.mtx.Lock()
defer c.mtx.Unlock()

if len(c.matchers) == 0 {
return
}

target, ok := c.matchers[key.target]
if !ok {
return
}

target.Remove(key)

if len(target.matchers) == 0 {
delete(c.matchers, key.target)
} else {
c.matchers[key.target] = target
}
}

// RemoveKind removes all Matchers for Constraints with kind.
// Returns normally if no entry for the kind exists for any target.
func (c *constraintMatchers) RemoveKind(kind string) {
c.mtx.Lock()
defer c.mtx.Unlock()

if len(c.matchers) == 0 {
return
}

for name, target := range c.matchers {
target.RemoveKind(kind)

if len(target.matchers) == 0 {
// It is safe to delete keys from a map while traversing it.
delete(c.matchers, name)
} else {
c.matchers[name] = target
}
}

delete(c.matchers, kind)
}

// ConstraintsFor returns the set of Constraints which should run against review
// according to their Matchers. Returns a map from Kind to the names of the
// Constraints of that Kind which should be run against review.
//
// Returns errors for each Constraint which was unable to properly run match
// criteria.
func (c *constraintMatchers) ConstraintsFor(targetName string, review interface{}) (map[string][]string, error) {
c.mtx.RLock()
defer c.mtx.RUnlock()

result := make(map[string][]string)
target := c.matchers[targetName]
errs := errors.ErrorMap{}

for kind, kindMatchers := range target.matchers {
var resultKindMatchers []string

for name, matcher := range kindMatchers {
if matches, err := matcher.Match(review); err != nil {
// key uniquely identifies the Constraint whose matcher was unable to
// run, for use in debugging.
key := fmt.Sprintf("%s %s %s", targetName, kind, name)
errs[key] = err
} else if matches {
resultKindMatchers = append(resultKindMatchers, name)
}
}

sort.Strings(resultKindMatchers)
result[kind] = resultKindMatchers
}

if len(errs) > 0 {
return nil, &errs
}

return result, nil
}

// targetMatchers are the Matchers for Constraints for a specific target.
// Not threadsafe.
type targetMatchers struct {
matchers map[string]map[string]constraints.Matcher
}

func (t *targetMatchers) Add(key matcherKey, matcher constraints.Matcher) {
if t.matchers == nil {
t.matchers = make(map[string]map[string]constraints.Matcher)
}

kindMatchers := t.matchers[key.kind]
if kindMatchers == nil {
kindMatchers = make(map[string]constraints.Matcher)
}

kindMatchers[key.name] = matcher
t.matchers[key.kind] = kindMatchers
}

func (t *targetMatchers) Remove(key matcherKey) {
kindMatchers, ok := t.matchers[key.kind]
if !ok {
return
}

delete(kindMatchers, key.name)

// Remove empty parents to avoid memory leaks.
if len(kindMatchers) == 0 {
delete(t.matchers, key.kind)
} else {
t.matchers[key.kind] = kindMatchers
}
}

func (t *targetMatchers) RemoveKind(kind string) {
delete(t.matchers, kind)
}
Loading

0 comments on commit 811b909

Please sign in to comment.