:Merge branch 'master' into no-backend

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

constraint/pkg/client/client.go

@@ -58,6 +58,8 @@ func createDataPath(target, subpath string) string {
 // On error, the responses return value will still be populated so that
 // partial results can be analyzed.
 func (c *Client) AddData(ctx context.Context, data interface{}) (*types.Responses, error) {
+	// TODO(#189): Make AddData atomic across all Drivers/Targets.
+
 	resp := types.NewResponses()
 	errMap := make(clienterrors.ErrorMap)
 	for target, h := range c.targets {
@@ -69,12 +71,40 @@ func (c *Client) AddData(ctx context.Context, data interface{}) (*types.Response
 		if !handled {
 			continue
 		}
-		if err := c.driver.PutData(ctx, createDataPath(target, relPath), processedData); err != nil {
+
+		var cache handler.Cache
+		if cacher, ok := h.(handler.Cacher); ok {
+			cache = cacher.GetCache()
+		}
+
+		// Add to the target cache first because cache.Remove cannot fail. Thus, we
+		// can prevent the system from getting into an inconsistent state.
+		if cache != nil {
+			err = cache.Add(relPath, processedData)
+			if err != nil {
+				// Use a different key than the driver to avoid clobbering errors.
+				errMap[target] = err
+
+				continue
+			}
+		}
+
+		// paths passed to driver must be specific to the target to prevent key
+		// collisions.
+		driverPath := createDataPath(target, relPath)
+		err = c.driver.PutData(ctx, driverPath, processedData)
+		if err != nil {
 			errMap[target] = err
+
+			if cache != nil {
+				cache.Remove(relPath)
+			}
 			continue
 		}
+
 		resp.Handled[target] = true
 	}
+
 	if len(errMap) == 0 {
 		return resp, nil
 	}
@@ -96,15 +126,24 @@ func (c *Client) RemoveData(ctx context.Context, data interface{}) (*types.Respo
 		if !handled {
 			continue
 		}
+
 		if _, err := c.driver.DeleteData(ctx, createDataPath(target, relPath)); err != nil {
 			errMap[target] = err
 			continue
 		}
 		resp.Handled[target] = true
+
+		if cacher, ok := h.(handler.Cacher); ok {
+			cache := cacher.GetCache()
+
+			cache.Remove(relPath)
+		}
 	}
+
 	if len(errMap) == 0 {
 		return resp, nil
 	}
+
 	return resp, &errMap
 }
 

constraint/pkg/client/client_test.go

@@ -1548,3 +1548,178 @@ func TestClient_AddTemplate_Duplicate(t *testing.T) {
 		t.Fatal(diff)
 	}
 }
+
+func TestClient_AddData_Cache(t *testing.T) {
+	tests := []struct {
+		name    string
+		before  map[string]*handlertest.Object
+		add     interface{}
+		want    map[interface{}]interface{}
+		wantErr error
+	}{
+		{
+			name:   "add invalid type",
+			before: nil,
+			add:    "foo",
+			want:   nil,
+			wantErr: &clienterrors.ErrorMap{
+				handlertest.HandlerName: handlertest.ErrInvalidType,
+			},
+		},
+		{
+			name:   "add invalid Object",
+			before: nil,
+			add: &handlertest.Object{
+				Namespace: "",
+				Name:      "",
+			},
+			want: nil,
+			wantErr: &clienterrors.ErrorMap{
+				handlertest.HandlerName: handlertest.ErrInvalidObject,
+			},
+		},
+		{
+			name:   "add Object",
+			before: nil,
+			add: &handlertest.Object{
+				Namespace: "foo",
+				Name:      "bar",
+			},
+			want:    nil,
+			wantErr: nil,
+		},
+		{
+			name:   "add Namespace",
+			before: nil,
+			add: &handlertest.Object{
+				Namespace: "foo",
+			},
+			want: map[interface{}]interface{}{
+				"namespace/foo/": &handlertest.Object{
+					Namespace: "foo",
+				},
+			},
+			wantErr: nil,
+		},
+		{
+			name: "replace Namespace",
+			before: map[string]*handlertest.Object{
+				"namespace/foo/": {
+					Namespace: "foo",
+					Data:      "qux",
+				},
+			},
+			add: &handlertest.Object{
+				Namespace: "foo",
+				Data:      "bar",
+			},
+			want: map[interface{}]interface{}{
+				"namespace/foo/": &handlertest.Object{
+					Namespace: "foo",
+					Data:      "bar",
+				},
+			},
+			wantErr: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cache := &handlertest.Cache{}
+			h := &handlertest.Handler{Cache: cache}
+
+			c := clienttest.New(t, client.Targets(h))
+
+			ctx := context.Background()
+			for _, v := range tt.before {
+				_, err := c.AddData(ctx, v)
+				if err != nil {
+					t.Fatal(err)
+				}
+			}
+
+			_, err := c.AddData(ctx, tt.add)
+			if !errors.Is(err, tt.wantErr) {
+				t.Fatalf("got error: %#v,\nwant %#v", err, tt.wantErr)
+			}
+
+			got := make(map[interface{}]interface{})
+			cache.Namespaces.Range(func(key, value interface{}) bool {
+				got[key] = value
+				return true
+			})
+
+			if diff := cmp.Diff(tt.want, got, cmpopts.EquateEmpty()); diff != "" {
+				t.Error(diff)
+			}
+		})
+	}
+}
+
+func TestClient_RemoveData_Cache(t *testing.T) {
+	tests := []struct {
+		name    string
+		before  map[string]*handlertest.Object
+		remove  interface{}
+		want    map[interface{}]interface{}
+		wantErr error
+	}{
+		{
+			name:   "remove invalid",
+			before: nil,
+			remove: "foo",
+			want:   nil,
+			wantErr: &clienterrors.ErrorMap{
+				handlertest.HandlerName: handlertest.ErrInvalidType,
+			},
+		},
+		{
+			name:    "remove nonexistent",
+			before:  nil,
+			remove:  &handlertest.Object{Namespace: "foo"},
+			want:    nil,
+			wantErr: nil,
+		},
+		{
+			name: "remove Namespace",
+			before: map[string]*handlertest.Object{
+				"/namespace/foo": {Namespace: "foo"},
+			},
+			remove:  &handlertest.Object{Namespace: "foo"},
+			want:    nil,
+			wantErr: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cache := &handlertest.Cache{}
+			h := &handlertest.Handler{Cache: cache}
+
+			c := clienttest.New(t, client.Targets(h))
+
+			ctx := context.Background()
+			for _, v := range tt.before {
+				_, err := c.AddData(ctx, v)
+				if err != nil {
+					t.Fatal(err)
+				}
+			}
+
+			_, err := c.RemoveData(ctx, tt.remove)
+			if !errors.Is(err, tt.wantErr) {
+				t.Fatalf("got error: %v,\nwant %v", err, tt.wantErr)
+			}
+
+			got := make(map[interface{}]interface{})
+			cache.Namespaces.Range(func(key, value interface{}) bool {
+				got[key] = value
+				return true
+			})
+
+			if diff := cmp.Diff(tt.want, got, cmpopts.EquateEmpty()); diff != "" {
+				t.Error(diff)
+			}
+		})
+	}
+}

constraint/pkg/handler/cache.go

@@ -0,0 +1,44 @@
+package handler
+
+// Cacher is a type - usually a Handler - which needs to cache state.
+// Handlers only need implement this interface if they have need of a cache.
+// Handlers which do not implement Cacher are assumed to be stateless from
+// Client's perspective.
+type Cacher interface {
+	// GetCache returns the Cache. If nil, the Cacher is treated as having no
+	// cache.
+	GetCache() Cache
+}
+
+// Cache is an interface for Handlers to define which allows them to track
+// objects not currently under review. For example, this is required to make
+// referential constraints work, or to have Constraint match criteria which
+// relies on more than just the object under review.
+//
+// Implementations must satisfy the per-method requirements for Client to handle
+// the Cache properly.
+type Cache interface {
+	// Add inserts a new object into Cache with identifier key. If an object
+	// already exists, replaces the object at key.
+	Add(key string, object interface{}) error
+
+	// Remove deletes the object at key from Cache. Deletion succeeds if key
+	// does not exist.
+	// Remove always succeeds; if for some reason key cannot be deleted the application
+	// should panic.
+	Remove(key string)
+}
+
+type NoCache struct{}
+
+func (n NoCache) Add(key string, object interface{}) error {
+	return nil
+}
+
+func (n NoCache) Get(key string) (interface{}, error) {
+	return nil, nil
+}
+
+func (n NoCache) Remove(key string) {}
+
+var _ Cache = NoCache{}

constraint/pkg/handler/handlertest/cache.go

@@ -0,0 +1,43 @@
+package handlertest
+
+import (
+	"errors"
+	"fmt"
+	"sync"
+
+	"github.com/open-policy-agent/frameworks/constraint/pkg/handler"
+)
+
+var ErrInvalidObject = errors.New("invalid object")
+
+// Cache is a threadsafe Cache for the test Handler which keeps track of
+// Namespaces.
+type Cache struct {
+	Namespaces sync.Map
+}
+
+var _ handler.Cache = &Cache{}
+
+// Add inserts object into Cache if object is a Namespace.
+func (c *Cache) Add(key string, object interface{}) error {
+	obj, ok := object.(*Object)
+	if !ok {
+		return fmt.Errorf("%w: got object type %T, want %T", ErrInvalidType, object, &Object{})
+	}
+
+	if obj.Name != "" {
+		return nil
+	}
+
+	if obj.Namespace == "" {
+		return fmt.Errorf("%w: must specify one of Name or Namespace", ErrInvalidObject)
+	}
+
+	c.Namespaces.Store(key, object)
+
+	return nil
+}
+
+func (c *Cache) Remove(key string) {
+	c.Namespaces.Delete(key)
+}

constraint/pkg/handler/handlertest/handler.go

@@ -14,6 +14,8 @@ import (
 
 var _ handler.TargetHandler = &Handler{}
 
+var _ handler.Cacher = &Handler{}
+
 // HandlerName is the default handler name.
 const HandlerName = "test.target"
 
@@ -28,6 +30,8 @@ type Handler struct {
 	// ProcessDataError is the error to return when ProcessData is called.
 	// If nil returns no error.
 	ProcessDataError error
+
+	Cache *Cache
 }
 
 func (h *Handler) GetName() string {
@@ -112,13 +116,10 @@ func (h *Handler) ProcessData(obj interface{}) (bool, string, interface{}, error
 			return false, "", nil, nil
 		}
 
-		if o.Namespace == "" {
-			return true, fmt.Sprintf("cluster/%s", o.Name), obj, nil
-		}
-		return true, fmt.Sprintf("namespace/%s/%s", o.Namespace, o.Name), obj, nil
+		return true, o.Key(), obj, nil
 	default:
-		return false, "", nil, fmt.Errorf("unrecognized type %T, want %T",
-			obj, &Object{})
+		return false, "", nil, fmt.Errorf("%w: got object type %T, want %T",
+			ErrInvalidType, obj, &Object{})
 	}
 }
 
@@ -167,5 +168,13 @@ func (h *Handler) ToMatcher(constraint *unstructured.Unstructured) (constraints.
 		return nil, fmt.Errorf("unable to get spec.matchNamespace: %w", err)
 	}
 
-	return Matcher{namespace: ns}, nil
+	return Matcher{namespace: ns, cache: h.Cache}, nil
+}
+
+func (h *Handler) GetCache() handler.Cache {
+	if h.Cache == nil {
+		return handler.NoCache{}
+	}
+
+	return h.Cache
 }