From 4d8242584a88ac378585899f6a89abe4015baf87 Mon Sep 17 00:00:00 2001 From: aarifullin Date: Fri, 1 Dec 2023 16:08:52 +0300 Subject: [PATCH] [#25] engine: Refactor LocalOverrideStorage * Make LocalOverrideStorage methods to receive Target type instead resource * Refactor unit-tests and dependencies * Make default chain router check local overrides not only for container but also for namespaces Signed-off-by: Airat Arifullin --- pkg/engine/chain_router.go | 60 ++++++++++++----------- pkg/engine/inmemory/inmemory_test.go | 6 +-- pkg/engine/inmemory/local_storage.go | 31 ++++++------ pkg/engine/inmemory/local_storage_test.go | 6 ++- pkg/engine/inmemory/morph_storage.go | 12 ++--- pkg/engine/interface.go | 8 +-- 6 files changed, 67 insertions(+), 56 deletions(-) diff --git a/pkg/engine/chain_router.go b/pkg/engine/chain_router.go index c798793..f31f327 100644 --- a/pkg/engine/chain_router.go +++ b/pkg/engine/chain_router.go @@ -25,19 +25,36 @@ func NewDefaultChainRouterWithLocalOverrides(morph MorphRuleChainStorage, local } func (dr *defaultChainRouter) IsAllowed(name chain.Name, namespace string, r resource.Request) (status chain.Status, ruleFound bool, err error) { - if dr.local != nil { - var localRuleFound bool - status, localRuleFound, err = dr.checkLocalOverrides(name, r) - if err != nil { - return chain.NoRuleFound, false, err - } else if localRuleFound { - ruleFound = true - return - } + status, ruleFound, err = dr.checkLocal(name, namespace, r) + if err != nil { + return chain.NoRuleFound, false, err + } else if ruleFound { + return } + status, ruleFound, err = dr.checkMorph(name, namespace, r) + return +} + +func (dr *defaultChainRouter) checkLocal(name chain.Name, namespace string, r resource.Request) (status chain.Status, ruleFound bool, err error) { + if dr.local == nil { + return + } + + status, ruleFound, err = dr.matchLocalOverrides(name, ContainerTarget(r.Resource().Name()), r) + if err != nil { + return chain.NoRuleFound, false, err + } else if ruleFound { + return + } + + 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.checkNamespaceChains(name, namespace, r) + status, namespaceRuleFound, err = dr.matchMorphRuleChains(name, NamespaceTarget(namespace), r) if err != nil { return } else if namespaceRuleFound && status != chain.Allow { @@ -46,7 +63,7 @@ func (dr *defaultChainRouter) IsAllowed(name chain.Name, namespace string, r res } var cnrRuleFound bool - status, cnrRuleFound, err = dr.checkContainerChains(name, r.Resource().Name(), r) + status, cnrRuleFound, err = dr.matchMorphRuleChains(name, ContainerTarget(r.Resource().Name()), r) if err != nil { return } else if cnrRuleFound && status != chain.Allow { @@ -61,8 +78,8 @@ func (dr *defaultChainRouter) IsAllowed(name chain.Name, namespace string, r res return } -func (dr *defaultChainRouter) checkLocalOverrides(name chain.Name, r resource.Request) (status chain.Status, ruleFound bool, err error) { - localOverrides, err := dr.local.ListOverrides(name, r.Resource().Name()) +func (dr *defaultChainRouter) matchLocalOverrides(name chain.Name, target Target, r resource.Request) (status chain.Status, ruleFound bool, err error) { + localOverrides, err := dr.local.ListOverrides(name, target) if err != nil { return } @@ -74,8 +91,8 @@ func (dr *defaultChainRouter) checkLocalOverrides(name chain.Name, r resource.Re return } -func (dr *defaultChainRouter) checkNamespaceChains(name chain.Name, namespace string, r resource.Request) (status chain.Status, ruleFound bool, err error) { - namespaceChains, err := dr.morph.ListMorphRuleChains(name, NamespaceTarget(namespace)) +func (dr *defaultChainRouter) matchMorphRuleChains(name chain.Name, target Target, r resource.Request) (status chain.Status, ruleFound bool, err error) { + namespaceChains, err := dr.morph.ListMorphRuleChains(name, target) if err != nil { return } @@ -86,16 +103,3 @@ func (dr *defaultChainRouter) checkNamespaceChains(name chain.Name, namespace st } return } - -func (dr *defaultChainRouter) checkContainerChains(name chain.Name, container string, r resource.Request) (status chain.Status, ruleFound bool, err error) { - containerChains, err := dr.morph.ListMorphRuleChains(name, ContainerTarget(container)) - if err != nil { - return - } - for _, c := range containerChains { - if status, ruleFound = c.Match(r); ruleFound { - return - } - } - return -} diff --git a/pkg/engine/inmemory/inmemory_test.go b/pkg/engine/inmemory/inmemory_test.go index bc47313..acc9168 100644 --- a/pkg/engine/inmemory/inmemory_test.go +++ b/pkg/engine/inmemory/inmemory_test.go @@ -167,7 +167,7 @@ func TestInmemory(t *testing.T) { require.False(t, ok) t.Run("quota on a different container", func(t *testing.T) { - s.LocalStorage().AddOverride(chain.Ingress, container, &chain.Chain{ + s.LocalStorage().AddOverride(chain.Ingress, engine.ContainerTarget(container), &chain.Chain{ Rules: []chain.Rule{{ Status: chain.QuotaLimitReached, Actions: chain.Actions{Names: []string{"native::object::put"}}, @@ -182,7 +182,7 @@ func TestInmemory(t *testing.T) { var quotaRuleChainID chain.ID t.Run("quota on the request container", func(t *testing.T) { - quotaRuleChainID, _ = s.LocalStorage().AddOverride(chain.Ingress, container, &chain.Chain{ + quotaRuleChainID, _ = s.LocalStorage().AddOverride(chain.Ingress, engine.ContainerTarget(container), &chain.Chain{ Rules: []chain.Rule{{ Status: chain.QuotaLimitReached, Actions: chain.Actions{Names: []string{"native::object::put"}}, @@ -195,7 +195,7 @@ func TestInmemory(t *testing.T) { require.True(t, ok) }) t.Run("removed quota on the request container", func(t *testing.T) { - err := s.LocalStorage().RemoveOverride(chain.Ingress, container, quotaRuleChainID) + err := s.LocalStorage().RemoveOverride(chain.Ingress, engine.ContainerTarget(container), quotaRuleChainID) require.NoError(t, err) status, ok, _ = s.IsAllowed(chain.Ingress, namespace, reqGood) diff --git a/pkg/engine/inmemory/local_storage.go b/pkg/engine/inmemory/local_storage.go index 3c3f8ba..dae9c73 100644 --- a/pkg/engine/inmemory/local_storage.go +++ b/pkg/engine/inmemory/local_storage.go @@ -10,7 +10,7 @@ import ( "git.frostfs.info/TrueCloudLab/policy-engine/util" ) -type targetToChain map[string][]*chain.Chain +type targetToChain map[engine.Target][]*chain.Chain type inmemoryLocalStorage struct { usedChainID map[chain.ID]struct{} @@ -24,11 +24,11 @@ func NewInmemoryLocalStorage() engine.LocalOverrideStorage { } } -func (s *inmemoryLocalStorage) generateChainID(name chain.Name, resource string) chain.ID { +func (s *inmemoryLocalStorage) generateChainID(name chain.Name, target engine.Target) chain.ID { var id chain.ID for { suffix := rand.Uint32() % 100 - sid := fmt.Sprintf("%s:%s/%d", name, resource, suffix) + sid := fmt.Sprintf("%s:%s/%d", name, target.Name, suffix) sid = strings.ReplaceAll(sid, "*", "") sid = strings.ReplaceAll(sid, "/", ":") sid = strings.ReplaceAll(sid, "::", ":") @@ -43,24 +43,24 @@ func (s *inmemoryLocalStorage) generateChainID(name chain.Name, resource string) return id } -func (s *inmemoryLocalStorage) AddOverride(name chain.Name, resource string, c *chain.Chain) (chain.ID, error) { +func (s *inmemoryLocalStorage) AddOverride(name chain.Name, target engine.Target, c *chain.Chain) (chain.ID, error) { // AddOverride assigns generated chain ID if it has not been assigned. if c.ID == "" { - c.ID = s.generateChainID(name, resource) + c.ID = s.generateChainID(name, target) } if s.nameToResourceChains[name] == nil { s.nameToResourceChains[name] = make(targetToChain) } rc := s.nameToResourceChains[name] - rc[resource] = append(rc[resource], c) + rc[target] = append(rc[target], c) return c.ID, nil } -func (s *inmemoryLocalStorage) GetOverride(name chain.Name, resource string, chainID chain.ID) (*chain.Chain, error) { +func (s *inmemoryLocalStorage) GetOverride(name chain.Name, target engine.Target, chainID chain.ID) (*chain.Chain, error) { if _, ok := s.nameToResourceChains[name]; !ok { return nil, engine.ErrChainNameNotFound } - chains, ok := s.nameToResourceChains[name][resource] + chains, ok := s.nameToResourceChains[name][target] if !ok { return nil, engine.ErrResourceNotFound } @@ -72,30 +72,33 @@ func (s *inmemoryLocalStorage) GetOverride(name chain.Name, resource string, cha return nil, engine.ErrChainNotFound } -func (s *inmemoryLocalStorage) RemoveOverride(name chain.Name, resource string, chainID chain.ID) error { +func (s *inmemoryLocalStorage) RemoveOverride(name chain.Name, target engine.Target, chainID chain.ID) error { if _, ok := s.nameToResourceChains[name]; !ok { return engine.ErrChainNameNotFound } - chains, ok := s.nameToResourceChains[name][resource] + chains, ok := s.nameToResourceChains[name][target] if !ok { return engine.ErrResourceNotFound } for i, c := range chains { if c.ID == chainID { - s.nameToResourceChains[name][resource] = append(chains[:i], chains[i+1:]...) + s.nameToResourceChains[name][target] = append(chains[:i], chains[i+1:]...) return nil } } return engine.ErrChainNotFound } -func (s *inmemoryLocalStorage) ListOverrides(name chain.Name, resource string) ([]*chain.Chain, error) { +func (s *inmemoryLocalStorage) ListOverrides(name chain.Name, target engine.Target) ([]*chain.Chain, error) { rcs, ok := s.nameToResourceChains[name] if !ok { return []*chain.Chain{}, nil } - for container, chains := range rcs { - if !util.GlobMatch(resource, container) { + for t, chains := range rcs { + if t.Type != target.Type { + continue + } + if !util.GlobMatch(target.Name, t.Name) { continue } return chains, nil diff --git a/pkg/engine/inmemory/local_storage_test.go b/pkg/engine/inmemory/local_storage_test.go index 2fbe26d..3609070 100644 --- a/pkg/engine/inmemory/local_storage_test.go +++ b/pkg/engine/inmemory/local_storage_test.go @@ -9,11 +9,15 @@ import ( ) const ( - resrc = "native:::object/ExYw/*" + container = "native:::object/ExYw/*" chainID = "ingress:ExYw" nonExistChainId = "ingress:LxGyWyL" ) +var ( + resrc = engine.ContainerTarget(container) +) + func testInmemLocalStorage() *inmemoryLocalStorage { return NewInmemoryLocalStorage().(*inmemoryLocalStorage) } diff --git a/pkg/engine/inmemory/morph_storage.go b/pkg/engine/inmemory/morph_storage.go index 2f802fb..cd4b5a9 100644 --- a/pkg/engine/inmemory/morph_storage.go +++ b/pkg/engine/inmemory/morph_storage.go @@ -20,9 +20,9 @@ func NewInmemoryMorphRuleChainStorage() engine.MorphRuleChainStorage { func (s *inmemoryMorphRuleChainStorage) AddMorphRuleChain(name chain.Name, target engine.Target, c *chain.Chain) (err error) { switch target.Type { case engine.Namespace: - _, err = s.nameToNamespaceChains.AddOverride(name, target.Name, c) + _, err = s.nameToNamespaceChains.AddOverride(name, target, c) case engine.Container: - _, err = s.nameToContainerChains.AddOverride(name, target.Name, c) + _, err = s.nameToContainerChains.AddOverride(name, target, c) default: err = engine.ErrUnknownTarget } @@ -32,9 +32,9 @@ func (s *inmemoryMorphRuleChainStorage) AddMorphRuleChain(name chain.Name, targe func (s *inmemoryMorphRuleChainStorage) RemoveMorphRuleChain(name chain.Name, target engine.Target, chainID chain.ID) error { switch target.Type { case engine.Namespace: - return s.nameToNamespaceChains.RemoveOverride(name, target.Name, chainID) + return s.nameToNamespaceChains.RemoveOverride(name, target, chainID) case engine.Container: - return s.nameToContainerChains.RemoveOverride(name, target.Name, chainID) + return s.nameToContainerChains.RemoveOverride(name, target, chainID) default: return engine.ErrUnknownTarget } @@ -43,9 +43,9 @@ func (s *inmemoryMorphRuleChainStorage) RemoveMorphRuleChain(name chain.Name, ta func (s *inmemoryMorphRuleChainStorage) ListMorphRuleChains(name chain.Name, target engine.Target) ([]*chain.Chain, error) { switch target.Type { case engine.Namespace: - return s.nameToNamespaceChains.ListOverrides(name, target.Name) + return s.nameToNamespaceChains.ListOverrides(name, target) case engine.Container: - return s.nameToContainerChains.ListOverrides(name, target.Name) + return s.nameToContainerChains.ListOverrides(name, target) default: } return nil, engine.ErrUnknownTarget diff --git a/pkg/engine/interface.go b/pkg/engine/interface.go index fe0b4a7..0b12e32 100644 --- a/pkg/engine/interface.go +++ b/pkg/engine/interface.go @@ -14,13 +14,13 @@ type ChainRouter interface { // LocalOverrideStorage is the interface to manage local overrides defined // for a node. Local overrides have a higher priority than chains got from morph storage. type LocalOverrideStorage interface { - AddOverride(name chain.Name, resource string, c *chain.Chain) (chain.ID, error) + AddOverride(name chain.Name, target Target, c *chain.Chain) (chain.ID, error) - GetOverride(name chain.Name, resource string, chainID chain.ID) (*chain.Chain, error) + GetOverride(name chain.Name, target Target, chainID chain.ID) (*chain.Chain, error) - RemoveOverride(name chain.Name, resource string, chainID chain.ID) error + RemoveOverride(name chain.Name, target Target, chainID chain.ID) error - ListOverrides(name chain.Name, resource string) ([]*chain.Chain, error) + ListOverrides(name chain.Name, target Target) ([]*chain.Chain, error) DropAllOverrides(name chain.Name) error }