[#25] engine: Refactor ChainRouter interface

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

Signed-off-by: Airat Arifullin <aarifullin@yadro.com>
This commit is contained in:
aarifullin 2023-12-01 18:12:57 +03:00 committed by Evgenii Stratonikov
parent 4d8242584a
commit 2d4a9fc6dc
6 changed files with 101 additions and 53 deletions

View file

@ -834,7 +834,7 @@ func TestComplexNativeConditions(t *testing.T) {
} { } {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
req := testutil.NewRequest(tc.action, testutil.NewResource(tc.resource, tc.resourceMap), tc.requestMap) 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.NoError(t, err)
require.Equal(t, tc.status.String(), status.String()) require.Equal(t, tc.status.String(), status.String())
}) })
@ -1065,7 +1065,7 @@ func TestComplexS3Conditions(t *testing.T) {
} { } {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
req := testutil.NewRequest(tc.action, testutil.NewResource(tc.resource, tc.resourceMap), tc.requestMap) 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.NoError(t, err)
require.Equal(t, tc.status.String(), status.String()) require.Equal(t, tc.status.String(), status.String())
}) })

View file

@ -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 { if err != nil {
return chain.NoRuleFound, false, err return chain.NoRuleFound, false, err
} else if ruleFound { } else if ruleFound {
// The local overrides have the highest priority and thus
// morph rules are not considered if a local one is found.
return return
} }
status, ruleFound, err = dr.checkMorph(name, namespace, r) status, ruleFound, err = dr.checkMorph(name, rt, r)
return 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 { if dr.local == nil {
return 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 status = chain.NoRuleFound
if ruleFound = namespaceRuleFound || cnrRuleFound; ruleFound { for _, ruleFound = range ruleFounds {
if ruleFound {
status = chain.Allow 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 return
} }

View file

@ -43,6 +43,6 @@ func (im *inmemory) MorphRuleChainStorage() engine.MorphRuleChainStorage {
return im.morph 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)
} }

View file

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

View file

@ -52,6 +52,12 @@ func (s *inmemoryLocalStorage) AddOverride(name chain.Name, target engine.Target
s.nameToResourceChains[name] = make(targetToChain) s.nameToResourceChains[name] = make(targetToChain)
} }
rc := s.nameToResourceChains[name] 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) rc[target] = append(rc[target], c)
return c.ID, nil return c.ID, nil
} }

View file

@ -8,7 +8,7 @@ import (
type ChainRouter interface { type ChainRouter interface {
// IsAllowed returns status for the operation after all checks. // IsAllowed returns status for the operation after all checks.
// The second return value signifies whether a matching rule was found. // 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 // LocalOverrideStorage is the interface to manage local overrides defined
@ -37,6 +37,45 @@ type Target struct {
Name string 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 { func NamespaceTarget(namespace string) Target {
return Target{ return Target{
Type: Namespace, Type: Namespace,