acl: Avoid shared cache with different parents

acl/cache.go

@@ -15,7 +15,7 @@ type FaultFunc func(id string) (string, string, error)
 type aclEntry struct {
 	ACL    ACL
 	Parent string
-	PolicyID string
+	RuleID string
 }
 
 // Cache is used to implement policy and ACL caching
@@ -71,13 +71,18 @@ func (c *Cache) ruleID(rules string) string {
 	return fmt.Sprintf("%x", md5.Sum([]byte(rules)))
 }
 
+// policyID returns the cache ID for a policy
+func (c *Cache) policyID(parent, ruleID string) string {
+	return parent + ":" + ruleID
+}
+
 // GetACLPolicy is used to get the potentially cached ACL
 // policy. If not cached, it will be generated and then cached.
 func (c *Cache) GetACLPolicy(id string) (string, *Policy, error) {
 	// Check for a cached acl
 	if raw, ok := c.aclCache.Get(id); ok {
 		cached := raw.(aclEntry)
-		if raw, ok := c.ruleCache.Get(cached.PolicyID); ok {
+		if raw, ok := c.ruleCache.Get(cached.RuleID); ok {
 			return cached.Parent, raw.(*Policy), nil
 		}
 	}
@@ -110,8 +115,9 @@ func (c *Cache) GetACL(id string) (ACL, error) {
 	ruleID := c.ruleID(rules)
 
 	// Check for a compiled ACL
+	policyID := c.policyID(parentID, ruleID)
 	var compiled ACL
-	if raw, ok := c.policyCache.Get(ruleID); ok {
+	if raw, ok := c.policyCache.Get(policyID); ok {
 		compiled = raw.(ACL)
 	} else {
 		// Get the policy
@@ -136,7 +142,7 @@ func (c *Cache) GetACL(id string) (ACL, error) {
 		}
 
 		// Cache the compiled ACL
-		c.policyCache.Add(ruleID, acl)
+		c.policyCache.Add(policyID, acl)
 		compiled = acl
 	}
 

acl/cache_test.go

@@ -114,7 +114,7 @@ func TestCache_ClearACL(t *testing.T) {
 	c.ClearACL("foo")
 
 	// Clear the policy cache
-	c.policyCache.Remove(c.ruleID(testSimplePolicy))
+	c.policyCache.Purge()
 
 	acl2, err := c.GetACL("foo")
 	if err != nil {
@@ -238,6 +238,53 @@ func TestCache_GetACL_Parent(t *testing.T) {
 	}
 }
 
+func TestCache_GetACL_ParentCache(t *testing.T) {
+	// Same rules, different parent
+	faultfn := func(id string) (string, string, error) {
+		switch id {
+		case "foo":
+			return "allow", testSimplePolicy, nil
+		case "bar":
+			return "deny", testSimplePolicy, nil
+		}
+		t.Fatalf("bad case")
+		return "", "", nil
+	}
+
+	c, err := NewCache(16, faultfn)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	acl, err := c.GetACL("foo")
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	if !acl.KeyRead("bar/test") {
+		t.Fatalf("should allow")
+	}
+	if !acl.KeyRead("foo/test") {
+		t.Fatalf("should allow")
+	}
+
+	acl2, err := c.GetACL("bar")
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	if acl == acl2 {
+		t.Fatalf("should not match")
+	}
+
+	if acl2.KeyRead("bar/test") {
+		t.Fatalf("should not allow")
+	}
+	if !acl2.KeyRead("foo/test") {
+		t.Fatalf("should allow")
+	}
+}
+
 var testSimplePolicy = `
 key "foo/" {
 	policy = "read"