From bb77b1cccd74df30c4773c844306126329b23a00 Mon Sep 17 00:00:00 2001
From: Evgenii Stratonikov <e.stratonikov@yadro.com>
Date: Tue, 20 Aug 2024 18:54:39 +0300
Subject: [PATCH] [#90] engine: Fix ruleFound return value

It can be false if the first targets allows operation and the last one
returns NoRuleFound.

Found by @mbiryukova.
Introduced in #86.

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
---
 pkg/engine/chain_router.go           | 10 +++----
 pkg/engine/inmemory/inmemory_test.go | 43 ++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/pkg/engine/chain_router.go b/pkg/engine/chain_router.go
index fe6c470..3eced4a 100644
--- a/pkg/engine/chain_router.go
+++ b/pkg/engine/chain_router.go
@@ -52,11 +52,10 @@ func (dr *defaultChainRouter) checkLocal(name chain.Name, rt RequestTarget, r re
 		hasAllow = hasAllow || ruleFound
 	}
 
-	status = chain.NoRuleFound
 	if hasAllow {
-		status = chain.Allow
+		return chain.Allow, true, nil
 	}
-	return
+	return chain.NoRuleFound, false, nil
 }
 
 func (dr *defaultChainRouter) checkMorph(name chain.Name, rt RequestTarget, r resource.Request) (status chain.Status, ruleFound bool, err error) {
@@ -69,11 +68,10 @@ func (dr *defaultChainRouter) checkMorph(name chain.Name, rt RequestTarget, r re
 		hasAllow = hasAllow || ruleFound
 	}
 
-	status = chain.NoRuleFound
 	if hasAllow {
-		status = chain.Allow
+		return chain.Allow, true, nil
 	}
-	return
+	return chain.NoRuleFound, false, nil
 }
 
 func (dr *defaultChainRouter) matchLocalOverrides(name chain.Name, target Target, r resource.Request) (status chain.Status, ruleFound bool, err error) {
diff --git a/pkg/engine/inmemory/inmemory_test.go b/pkg/engine/inmemory/inmemory_test.go
index fca6935..30bd7c3 100644
--- a/pkg/engine/inmemory/inmemory_test.go
+++ b/pkg/engine/inmemory/inmemory_test.go
@@ -31,6 +31,49 @@ func TestAddRootOverrides(t *testing.T) {
 	require.Equal(t, string(id), string(res[0].ID))
 }
 
+func TestInmemory_MultipleTargets(t *testing.T) {
+	const op = "ape::test::op"
+
+	targets := []engine.Target{
+		engine.NamespaceTarget("ns1"),
+		engine.ContainerTarget("cnr1"),
+		engine.GroupTarget("group1"),
+		engine.UserTarget("user1"),
+	}
+	req := resourcetest.NewRequest(op, resourcetest.NewResource("r", nil), nil)
+	target := engine.NewRequestTargetExtended("ns1", "cnr1", "user1", []string{"group1"})
+	for _, tt := range targets {
+		t.Run("morph", func(t *testing.T) {
+			s := NewInMemoryLocalOverrides()
+			s.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, tt, &chain.Chain{
+				Rules: []chain.Rule{{
+					Status:  chain.Allow,
+					Actions: chain.Actions{Names: []string{op}},
+				}},
+			})
+
+			status, found, err := s.IsAllowed(chain.Ingress, target, req)
+			require.NoError(t, err)
+			require.True(t, found)
+			require.Equal(t, chain.Allow, status)
+		})
+		t.Run("override", func(t *testing.T) {
+			s := NewInMemoryLocalOverrides()
+			s.LocalStorage().AddOverride(chain.Ingress, tt, &chain.Chain{
+				Rules: []chain.Rule{{
+					Status:  chain.Allow,
+					Actions: chain.Actions{Names: []string{op}},
+				}},
+			})
+
+			status, found, err := s.IsAllowed(chain.Ingress, target, req)
+			require.NoError(t, err)
+			require.True(t, found)
+			require.Equal(t, chain.Allow, status)
+		})
+	}
+}
+
 func TestInmemory(t *testing.T) {
 	const (
 		object     = "native::object::abc/xyz"
-- 
2.45.2