[#25] engine: Refactor ChainRouter interface

* Pass RequestTarget instead only namespace
* Refactor unit-tests and dependencies

Signed-off-by: Airat Arifullin <aarifullin@yadro.com>

iam/converter_test.go

@@ -833,7 +833,7 @@ func TestComplexNativeConditions(t *testing.T) {
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			req := testutil.NewRequest(tc.action, testutil.NewResource(tc.resource, tc.resourceMap), tc.requestMap)
-			status, _, err := s.IsAllowed("name", "ns", req)
+			status, _, err := s.IsAllowed("name", engine.NewRequestTargetWithNamespace("ns"), req)
 			require.NoError(t, err)
 			require.Equal(t, tc.status.String(), status.String())
 		})
@@ -1064,7 +1064,7 @@ func TestComplexS3Conditions(t *testing.T) {
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			req := testutil.NewRequest(tc.action, testutil.NewResource(tc.resource, tc.resourceMap), tc.requestMap)
-			status, _, err := s.IsAllowed("name", "ns", req)
+			status, _, err := s.IsAllowed("name", engine.NewRequestTargetWithNamespace("ns"), req)
 			require.NoError(t, err)
 			require.Equal(t, tc.status.String(), status.String())
 		})

pkg/engine/chain_router.go

@@ -24,56 +24,59 @@ func NewDefaultChainRouterWithLocalOverrides(morph MorphRuleChainStorage, local
 	}
 }
 
-func (dr *defaultChainRouter) IsAllowed(name chain.Name, namespace string, r resource.Request) (status chain.Status, ruleFound bool, err error) {
+func (dr *defaultChainRouter) IsAllowed(name chain.Name, rt RequestTarget, r resource.Request) (status chain.Status, ruleFound bool, err error) {
-	status, ruleFound, err = dr.checkLocal(name, namespace, r)
+	status, ruleFound, err = dr.checkLocal(name, rt, r)
 	if err != nil {
 		return chain.NoRuleFound, false, err
 	} else if ruleFound {
+		// The local overrides have the highest priority and thus
+		// morph rules are not considered if a local one is found.
 		return
 	}
 
-	status, ruleFound, err = dr.checkMorph(name, namespace, r)
+	status, ruleFound, err = dr.checkMorph(name, rt, r)
 	return
 }
 
-func (dr *defaultChainRouter) checkLocal(name chain.Name, namespace string, r resource.Request) (status chain.Status, ruleFound bool, err error) {
+func (dr *defaultChainRouter) checkLocal(name chain.Name, rt RequestTarget, r resource.Request) (status chain.Status, ruleFound bool, err error) {
 	if dr.local == nil {
 		return
 	}
-
+	var ruleFounds []bool
-	status, ruleFound, err = dr.matchLocalOverrides(name, ContainerTarget(r.Resource().Name()), r)
+	for _, target := range rt.Targets() {
-	if err != nil {
+		status, ruleFound, err = dr.matchLocalOverrides(name, target, r)
-		return chain.NoRuleFound, false, err
+		if err != nil || ruleFound && status != chain.Allow {
-	} else if ruleFound {
+			return
-		return
+		}
-	}
+		ruleFounds = append(ruleFounds, ruleFound)
-
-	status, ruleFound, err = dr.matchLocalOverrides(name, NamespaceTarget(namespace), r)
-	return
-}
-
-func (dr *defaultChainRouter) checkMorph(name chain.Name, namespace string, r resource.Request) (status chain.Status, ruleFound bool, err error) {
-	var namespaceRuleFound bool
-	status, namespaceRuleFound, err = dr.matchMorphRuleChains(name, NamespaceTarget(namespace), r)
-	if err != nil {
-		return
-	} else if namespaceRuleFound && status != chain.Allow {
-		ruleFound = true
-		return
-	}
-
-	var cnrRuleFound bool
-	status, cnrRuleFound, err = dr.matchMorphRuleChains(name, ContainerTarget(r.Resource().Name()), r)
-	if err != nil {
-		return
-	} else if cnrRuleFound && status != chain.Allow {
-		ruleFound = true
-		return
 	}
 
 	status = chain.NoRuleFound
-	if ruleFound = namespaceRuleFound || cnrRuleFound; ruleFound {
+	for _, ruleFound = range ruleFounds {
-		status = chain.Allow
+		if ruleFound {
+			status = chain.Allow
+			break
+		}
+	}
+	return
+}
+
+func (dr *defaultChainRouter) checkMorph(name chain.Name, rt RequestTarget, r resource.Request) (status chain.Status, ruleFound bool, err error) {
+	var ruleFounds []bool
+	for _, target := range rt.Targets() {
+		status, ruleFound, err = dr.matchMorphRuleChains(name, target, r)
+		if err != nil || ruleFound && status != chain.Allow {
+			return
+		}
+		ruleFounds = append(ruleFounds, ruleFound)
+	}
+
+	status = chain.NoRuleFound
+	for _, ruleFound = range ruleFounds {
+		if ruleFound {
+			status = chain.Allow
+			break
+		}
 	}
 	return
 }

pkg/engine/inmemory/inmemory.go

@@ -43,6 +43,6 @@ func (im *inmemory) MorphRuleChainStorage() engine.MorphRuleChainStorage {
 	return im.morph
 }
 
-func (im *inmemory) IsAllowed(name chain.Name, namespace string, r resource.Request) (status chain.Status, ruleFound bool, err error) {
+func (im *inmemory) IsAllowed(name chain.Name, rt engine.RequestTarget, r resource.Request) (status chain.Status, ruleFound bool, err error) {
-	return im.router.IsAllowed(name, namespace, r)
+	return im.router.IsAllowed(name, rt, r)
 }

pkg/engine/inmemory/inmemory_test.go

@@ -29,7 +29,7 @@ func TestInmemory(t *testing.T) {
 		"Actor":    actor1,
 	})
 
-	status, ok, _ := s.IsAllowed(chain.Ingress, namespace, reqGood)
+	status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTargetWithNamespace(namespace), reqGood)
 	require.Equal(t, chain.NoRuleFound, status)
 	require.False(t, ok)
 
@@ -103,7 +103,7 @@ func TestInmemory(t *testing.T) {
 			"SourceIP": "10.122.1.20",
 			"Actor":    actor1,
 		})
-		status, ok, _ := s.IsAllowed(chain.Ingress, namespace, reqBadIP)
+		status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadIP)
 		require.Equal(t, chain.AccessDenied, status)
 		require.True(t, ok)
 	})
@@ -113,7 +113,7 @@ func TestInmemory(t *testing.T) {
 			"SourceIP": "10.1.1.13",
 			"Actor":    actor2,
 		})
-		status, ok, _ := s.IsAllowed(chain.Ingress, namespace, reqBadActor)
+		status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadActor)
 		require.Equal(t, chain.AccessDenied, status)
 		require.True(t, ok)
 	})
@@ -121,14 +121,14 @@ func TestInmemory(t *testing.T) {
 		objGood := resourcetest.NewResource("native::object::abc/id1", map[string]string{"Department": "HR"})
 		objBadAttr := resourcetest.NewResource("native::object::abc/id2", map[string]string{"Department": "Support"})
 
-		status, ok, _ := s.IsAllowed(chain.Ingress, namespace, resourcetest.NewRequest("native::object::get", objGood, map[string]string{
+		status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), resourcetest.NewRequest("native::object::get", objGood, map[string]string{
 			"SourceIP": "10.1.1.14",
 			"Actor":    actor2,
 		}))
 		require.Equal(t, chain.Allow, status)
 		require.True(t, ok)
 
-		status, ok, _ = s.IsAllowed(chain.Ingress, namespace, resourcetest.NewRequest("native::object::get", objBadAttr, map[string]string{
+		status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), resourcetest.NewRequest("native::object::get", objBadAttr, map[string]string{
 			"SourceIP": "10.1.1.14",
 			"Actor":    actor2,
 		}))
@@ -141,28 +141,28 @@ func TestInmemory(t *testing.T) {
 			"SourceIP": "10.1.1.12",
 			"Actor":    actor1,
 		})
-		status, ok, _ := s.IsAllowed(chain.Ingress, namespace, reqBadOperation)
+		status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadOperation)
 		require.Equal(t, chain.AccessDenied, status)
 		require.True(t, ok)
 	})
 	t.Run("inverted rules", func(t *testing.T) {
 		req := resourcetest.NewRequest("native::object::put", resourcetest.NewResource(object, nil), nil)
-		status, ok, _ = s.IsAllowed(chain.Ingress, namespace2, req)
+		status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace2, container), req)
 		require.Equal(t, chain.NoRuleFound, status)
 		require.False(t, ok)
 
 		req = resourcetest.NewRequest("native::object::put", resourcetest.NewResource("native::object::cba/def", nil), nil)
-		status, ok, _ = s.IsAllowed(chain.Ingress, namespace2, req)
+		status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace2, container), req)
 		require.Equal(t, chain.AccessDenied, status)
 		require.True(t, ok)
 
 		req = resourcetest.NewRequest("native::object::get", resourcetest.NewResource("native::object::cba/def", nil), nil)
-		status, ok, _ = s.IsAllowed(chain.Ingress, namespace2, req)
+		status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace2, container), req)
 		require.Equal(t, chain.NoRuleFound, status)
 		require.False(t, ok)
 	})
 	t.Run("good", func(t *testing.T) {
-		status, ok, _ = s.IsAllowed(chain.Ingress, namespace, reqGood)
+		status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood)
 		require.Equal(t, chain.NoRuleFound, status)
 		require.False(t, ok)
 
@@ -175,7 +175,7 @@ func TestInmemory(t *testing.T) {
 				}},
 			})
 
-			status, ok, _ = s.IsAllowed(chain.Ingress, namespace, reqGood)
+			status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood)
 			require.Equal(t, chain.NoRuleFound, status)
 			require.False(t, ok)
 		})
@@ -190,7 +190,7 @@ func TestInmemory(t *testing.T) {
 				}},
 			})
 
-			status, ok, _ = s.IsAllowed(chain.Ingress, namespace, reqGood)
+			status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood)
 			require.Equal(t, chain.QuotaLimitReached, status)
 			require.True(t, ok)
 		})
@@ -198,7 +198,7 @@ func TestInmemory(t *testing.T) {
 			err := s.LocalStorage().RemoveOverride(chain.Ingress, engine.ContainerTarget(container), quotaRuleChainID)
 			require.NoError(t, err)
 
-			status, ok, _ = s.IsAllowed(chain.Ingress, namespace, reqGood)
+			status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood)
 			require.Equal(t, chain.NoRuleFound, status)
 			require.False(t, ok)
 		})

pkg/engine/inmemory/local_storage.go

@@ -52,6 +52,12 @@ func (s *inmemoryLocalStorage) AddOverride(name chain.Name, target engine.Target
 		s.nameToResourceChains[name] = make(targetToChain)
 	}
 	rc := s.nameToResourceChains[name]
+	for i := range rc[target] {
+		if rc[target][i].ID == c.ID {
+			rc[target][i] = c
+			return c.ID, nil
+		}
+	}
 	rc[target] = append(rc[target], c)
 	return c.ID, nil
 }

pkg/engine/interface.go

@@ -8,7 +8,7 @@ import (
 type ChainRouter interface {
 	// IsAllowed returns status for the operation after all checks.
 	// The second return value signifies whether a matching rule was found.
-	IsAllowed(name chain.Name, target string, r resource.Request) (status chain.Status, found bool, err error)
+	IsAllowed(name chain.Name, reqTarget RequestTarget, r resource.Request) (status chain.Status, found bool, err error)
 }
 
 // LocalOverrideStorage is the interface to manage local overrides defined
@@ -37,6 +37,45 @@ type Target struct {
 	Name string
 }
 
+// RequestTarget combines several targets on which the request is performed.
+type RequestTarget struct {
+	Namespace *Target
+	Container *Target
+}
+
+func NewRequestTargetWithNamespace(namespace string) RequestTarget {
+	nt := NamespaceTarget(namespace)
+	return RequestTarget{
+		Namespace: &nt,
+	}
+}
+
+func NewRequestTargetWithContainer(container string) RequestTarget {
+	ct := ContainerTarget(container)
+	return RequestTarget{
+		Container: &ct,
+	}
+}
+
+func NewRequestTarget(namespace, container string) RequestTarget {
+	nt := NamespaceTarget(namespace)
+	ct := ContainerTarget(container)
+	return RequestTarget{
+		Namespace: &nt,
+		Container: &ct,
+	}
+}
+
+func (rt *RequestTarget) Targets() (targets []Target) {
+	if rt.Container != nil {
+		targets = append(targets, *rt.Container)
+	}
+	if rt.Namespace != nil {
+		targets = append(targets, *rt.Namespace)
+	}
+	return
+}
+
 func NamespaceTarget(namespace string) Target {
 	return Target{
 		Type: Namespace,