-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Using stackrox/storage/ResourceCollection #1622
Conversation
as filter object for runtime config
…d some refactoring to use more functions and less nesting
} | ||
} | ||
return false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible improvements to this function include checks for cycles and stopping at a maximum depth.
return false; | ||
} | ||
|
||
bool ResourceSelector::AreClusterAndNamespaceSelected(const storage::ResourceCollection& rc, const std::string& cluster, const std::string& ns) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One alternative could be to have a function that strips out all of the ResourceSelectors where the cluster does not match collector's cluster.
…nit test before embedded collections are added
…ng fields and empty arrays
Co-authored-by: Mauro Ezequiel Moltrasio <[email protected]>
|
||
namespace collector { | ||
|
||
class ResourceSelector { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of a class here feels a little redundant since everything is static anyway. A namespace might be a better fit
static bool IsRuleFollowed(const storage::SelectorRule& rule, const std::string& ns); | ||
static bool IsRuleValueFollowed(const storage::RuleValue& value, const std::string& ns); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could also use overloading here and call both functions IsRuleFollowed
(or IsFollowed
to be type-agnostic)
for (const auto& embeddedCollection : rc.embedded_collections()) { | ||
auto embeddedRc = rcMap.find(embeddedCollection.id()); | ||
if (embeddedRc != rcMap.end()) { | ||
bool inEmbeddedRc = AreClusterAndNamespaceSelected(embeddedRc->second, rcMap, cluster, ns); | ||
if (inEmbeddedRc) { | ||
return true; | ||
} | ||
} | ||
} | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be std::any_of
as well?
bool inEmbeddedRc = AreClusterAndNamespaceSelected(embeddedRc->second, rcMap, cluster, ns); | ||
if (inEmbeddedRc) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bool inEmbeddedRc = AreClusterAndNamespaceSelected(embeddedRc->second, rcMap, cluster, ns); | |
if (inEmbeddedRc) { | |
if (AreClusterAndNamespaceSelected(embeddedRc->second, rcMap, cluster, ns)) { |
static bool IsResourceInResourceSelector(const storage::ResourceSelector& rs, const std::string& resource_type, const std::string& resource_name); | ||
static bool IsNamespaceInResourceSelector(const storage::ResourceSelector& rs, const std::string& ns); | ||
static bool IsClusterInResourceSelector(const storage::ResourceSelector& rs, const std::string& cluster); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] these are pretty verbose and the type name suffix is unnecessary - perhaps ContainsResource
ContainsNamespace
ContainsCluster
?
|
||
for (auto filteringRule : filteringRules) { | ||
auto rc = rcMap.find(filteringRule.collectionId()); | ||
if (rc != rcMap.end()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to decide what to do if the key is not in the map.
Description
Uses ResourceCollection storage object for filtering at the cluster and namespace levels.
Builds on #1620 by adding the use of the AND operator, checking cluster level rules, checking embedded collections, and more unit tests.
Checklist
Automated testing
- [ ] Added integration tests- [ ] Added regression testsThe changes are not currently wired to anything that would impact integration tests.
If any of these don't apply, please comment below.
Testing Performed
CI is sufficient