engine: Fix ruleFound return value
Some checks failed
DCO action / DCO (pull_request) Failing after 2m58s
Tests and linters / Tests (1.20) (pull_request) Failing after 3m13s
Tests and linters / Tests (1.21) (pull_request) Failing after 3m10s
Tests and linters / Tests with -race (pull_request) Failing after 3m14s
Tests and linters / Lint (pull_request) Successful in 3m54s
Tests and linters / Staticcheck (pull_request) Successful in 3m54s

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

Found by @mbiryukova .

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
This commit is contained in:
Evgenii Stratonikov 2024-08-20 18:54:39 +03:00
parent a11e80e2c7
commit 3df9afdb30
2 changed files with 54 additions and 6 deletions

View file

@ -52,11 +52,10 @@ func (dr *defaultChainRouter) checkLocal(name chain.Name, rt RequestTarget, r re
hasAllow = hasAllow || ruleFound hasAllow = hasAllow || ruleFound
} }
status = chain.NoRuleFound
if hasAllow { 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) { 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 hasAllow = hasAllow || ruleFound
} }
status = chain.NoRuleFound
if hasAllow { 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) { func (dr *defaultChainRouter) matchLocalOverrides(name chain.Name, target Target, r resource.Request) (status chain.Status, ruleFound bool, err error) {

View file

@ -31,6 +31,49 @@ func TestAddRootOverrides(t *testing.T) {
require.Equal(t, string(id), string(res[0].ID)) 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"),
}
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}},
}},
})
req := resourcetest.NewRequest(op, resourcetest.NewResource("r", nil), nil)
status, found, err := s.IsAllowed(chain.Ingress, engine.NewRequestTargetExtended("ns1", "cnr1", "user1", []string{"group1"}), 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}},
}},
})
req := resourcetest.NewRequest(op, resourcetest.NewResource("r", nil), nil)
status, found, err := s.IsAllowed(chain.Ingress, engine.NewRequestTargetExtended("ns1", "cnr1", "user1", []string{"group1"}), req)
require.NoError(t, err)
require.True(t, found)
require.Equal(t, chain.Allow, status)
})
}
}
func TestInmemory(t *testing.T) { func TestInmemory(t *testing.T) {
const ( const (
object = "native::object::abc/xyz" object = "native::object::abc/xyz"
@ -204,6 +247,13 @@ func TestInmemory(t *testing.T) {
require.Equal(t, chain.NoRuleFound, status) require.Equal(t, chain.NoRuleFound, status)
require.False(t, ok) require.False(t, ok)
status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), resourcetest.NewRequest("native::object::get", res, map[string]string{
"Department": "HR",
"Actor": actor2,
}))
require.Equal(t, chain.Allow, status)
require.True(t, ok)
t.Run("quota on a different container", func(t *testing.T) { t.Run("quota on a different container", func(t *testing.T) {
s.LocalStorage().AddOverride(chain.Ingress, engine.ContainerTarget(container), &chain.Chain{ s.LocalStorage().AddOverride(chain.Ingress, engine.ContainerTarget(container), &chain.Chain{
Rules: []chain.Rule{{ Rules: []chain.Rule{{