feat(exceptions): add cluster-matching and control-ID helpers for external callers#165
Conversation
📝 WalkthroughWalkthroughCentralized exception-matching logic: Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant Processor as Processor
participant Cache as AttributeCache
participant Comparator as ClusterComparator
Caller->>Processor: MatchesCluster(designator, clusterName)
Processor->>Cache: getAttributes(designator) (cache-aware)
Cache-->>Processor: attributes (digested)
Processor->>Comparator: matchesCluster(attributes, clusterName)
Comparator-->>Processor: matchResult (empty constraint => match any / else compareCluster)
Processor-->>Caller: bool (matchResult)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
… exception matching Extract cluster matching logic into an exported MatchesCluster method on Processor. This allows callers (e.g. manual-control exception path in kubescape) to reuse the same cluster matching logic — including regex support and caching — without duplicating the inline regexp check. hasException now calls MatchesCluster instead of inlining the cluster check, removing the TODO comment about cluster name origin. Signed-off-by: rohankaran <rohankaran001@gmail.com>
- Add nil guard to MatchesCluster to prevent panic on nil designator - Extract getAttributes helper to centralise cache lookup logic - Extract private matchesCluster(attributes) so hasException reuses already-digested attributes without a redundant cache lookup Signed-off-by: rohankaran <rohankaran001@gmail.com>
4803a61 to
86a0b9a
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
exceptions/exceptionprocessor.go (1)
183-185: Add a defensive nil guard inhasException.Line 184 assumes
designatoris non-nil. Current call paths are safe, but guarding here prevents future panic risk if this helper is reused with nil input.Patch suggestion
func (p *Processor) hasException(clusterName string, designator *identifiers.PortalDesignator, workload workloadinterface.IMetadata) bool { + if designator == nil { + return false + } attributes := p.getAttributes(designator)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@exceptions/exceptionprocessor.go` around lines 183 - 185, The hasException helper assumes designator is non-nil and calls p.getAttributes(designator); add a defensive nil-check at the start of hasException to return false (no exception) when designator == nil to avoid panics if it is ever called with nil; ensure the nil guard is placed before calling p.getAttributes and mentions identifiers.PortalDesignator and the hasException method name so reviewers can find the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@exceptions/exceptionprocessor.go`:
- Around line 183-185: The hasException helper assumes designator is non-nil and
calls p.getAttributes(designator); add a defensive nil-check at the start of
hasException to return false (no exception) when designator == nil to avoid
panics if it is ever called with nil; ensure the nil guard is placed before
calling p.getAttributes and mentions identifiers.PortalDesignator and the
hasException method name so reviewers can find the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea5a363c-a6be-4e78-86cb-439755e97fd7
📒 Files selected for processing (1)
exceptions/exceptionprocessor.go
…olID method - Add RegexCompareControlID public method for case-insensitive control ID pattern matching - Clarify MatchesCluster documentation to explain nil designator and empty cluster field behavior - Improve getAttributes comment to be more concise - Update matchesCluster comment to use "pre-digested" for clarity - Enhance code documentation for better maintainability and API clarity Signed-off-by: rohankaran <rohankaran001@gmail.com>
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Overview
Add three exported helpers on
Processorto support the manual-control exception path in kubescape:MatchesCluster— nil-safe cluster designator check; delegates to privatematchesClusterRegexCompareControlID— case-insensitive regex match for control IDsgetAttributes/matchesCluster— private helpers that centralise the designator cache lookup and cluster check, removing the redundant lookup inhasExceptionand the existing TODO commentHow to Test
No behaviour change — refactor only.
Checklist before requesting a review