feat(exceptions): support container-level exceptions via containerName attribute#169
Conversation
…e attribute When an exception designator includes a containerName attribute, it now matches only workloads that have a container (or init container) with that name, rather than treating it as a Kubernetes label. Previously containerName fell into the labels bucket in DigestAttributesDesignator and was compared against pod labels, which always failed. Now metadataHasException strips containerName from the labels map before the label comparison and runs compareContainerName against the pod's containers and initContainers instead. compareContainerName supports regex patterns, consistent with how namespace, name, and kind matching already work. Signed-off-by: Sarang Bodhe <221710715+workwithsarang@users.noreply.github.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds container-name-aware exception matching: comparator.compareContainerName performs regex matching (with optional failing-name filter). The exception processor extracts failing container names from FailedPaths, propagates them through exception evaluation, and updates metadata/hasException flows and tests accordingly. ChangesContainer-level exception filtering
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
matthyx
left a comment
matthyx
left a comment
There was a problem hiding this comment.
I found two blocker-level issues with the new containerName matching.
| if isTypeWorkload(workload) && len(attributes.GetLabels()) > 0 { | ||
| if !p.compareLabels(workload, attributes.GetLabels()) && !p.compareAnnotations(workload, attributes.GetLabels()) { | ||
| return false // labels nor annotations do not match | ||
| if isTypeWorkload(workload) { |
There was a problem hiding this comment.
This only enforces containerName for workload objects. hasException() still falls back from a RegoResponseVector to metadataHasException() on the base vector object, so a designator that includes containerName plus a base-object field like name can still match after the related-object scan misses because the container constraint is silently skipped in that fallback path. I reproduced this with a small test: a vector named base-subject with only an app pod in relatedObjects still returns true for {name: base-subject, containerName: missing}.
| return designatorCluster != "" && c.regexCompare(designatorCluster, clusterName) | ||
| } | ||
|
|
||
| func (c *comparator) compareContainerName(workload workloadinterface.IMetadata, containerName string) bool { |
There was a problem hiding this comment.
This only proves that the workload contains some matching container. The exception is still attached to the whole RuleResponse, so on a pod with app + sidecar, a containerName: sidecar exception will also suppress a finding that was raised for app. I reproduced that with SetRuleResponsExceptions: a response whose alert message points at container 'app' still gets excepted as soon as the pod also contains sidecar. We need a way to match against the failing container identity, not just container membership in the pod.
… only
Two bugs were found in the original containerName matching:
1. RegoResponseVector fallback bypass: when iterateRegoResponseVector
found no match, hasException fell back to checking the base vector
object which is not a workload. Because isTypeWorkload returns false
for RegoResponseVector, the containerName check was silently skipped,
allowing a designator with {name: X, containerName: missing} to match
just because the vector's name was X. Fix: return false immediately
when containerName is in the designator and the related-object scan
found no match.
2. Exception applied to wrong container: compareContainerName checked
whether the pod *contained* a container with the given name, not
whether that container *caused* the failing finding. A pod with
containers [app, sidecar] and a containerName:sidecar exception would
suppress a finding for app as long as sidecar was present anywhere in
the pod. Fix: SetRuleResponsExceptions now parses FailedPaths to
resolve which container indices produced the finding, maps them to
names, and passes those names to compareContainerName. If failingNames
is provided, only those containers are candidates for the match.
Signed-off-by: Sarang Bodhe <221710715+workwithsarang@users.noreply.github.com>
|
Thanks for the detailed analysis — both issues are now fixed. Issue 1 (RegoResponseVector fallback bypass): When Issue 2 (wrong container excepted): |
matthyx
left a comment
matthyx
left a comment
There was a problem hiding this comment.
Rechecked on the latest commit. The earlier base-object fallback bug is fixed, but one blocker remains in the RegoResponseVector path.
| // Resolve which containers actually produced the finding so that a | ||
| // containerName exception is only applied when the excepted container | ||
| // is the one that failed, not just any container in the pod. | ||
| failingContainerNames := extractFailingContainerNames(results[i].FailedPaths, workloads[w]) |
There was a problem hiding this comment.
This still loses container precision for RegoResponseVector alerts. Here workloads[w] is the vector object, so extractFailingContainerNames() sees no containers and returns nil. That nil then flows through iterateRegoResponseVector() into metadataHasException(), so compareContainerName() falls back to scanning every container in the related pod. I reproduced it locally: a vector wrapping a pod with containers [app, sidecar], FailedPaths = ["spec.containers[0].securityContext.privileged"], and an exception {containerName: sidecar} still sets the exception. We need to derive failing container names from the related workload in the vector path (and add a regression test for AlertObject.ExternalObjects / RegoResponseVector).
…ted objects extractFailingContainerNames called GetContainers() on the vector object itself, which has no containers at the top level. This returned nil, causing compareContainerName to fall back to scanning every container in the related pod and incorrectly applying a containerName exception to the wrong container. Fix: when the workload is a RegoResponseVector, recurse into its related objects and collect failing container names from each one instead. Add TestSetRuleResponsExceptions_RegoResponseVector_ContainerNamePrecision to cover the exact scenario: vector wrapping a pod with [app, sidecar], FailedPaths pointing at containers[0]=app, exception targeting sidecar — the exception must not be applied. Also fix the RegoResponseVector construction in both new tests: the type check requires top-level "name" and "relatedObjects" keys, not the nested metadata.name used by regular Kubernetes objects. Signed-off-by: Sarang Bodhe <221710715+workwithsarang@users.noreply.github.com>
Covers the AlertObject.ExternalObjects path — a RegoResponseVector delivered via ExternalObjects (single map, not list) must also respect container-name precision when matching exceptions. Signed-off-by: Sarang Bodhe <221710715+workwithsarang@users.noreply.github.com>
matthyx
left a comment
matthyx
left a comment
There was a problem hiding this comment.
Rechecked the latest head. The RegoResponseVector containerName precision issue is fixed, and I do not see any blocker-level issues remaining.
2 participants
Overview
Exceptions in Kubescape currently work at the pod/workload level. If a pod has multiple containers and only one of them needs to be excluded from a check, there's no way to do that.
The
containerNameattribute key already exists inarmoapi-go(identifiers.AttributeContainerName), but it was falling into thelabelsbucket inDigestAttributesDesignatorand being compared against pod labels — which always fails.This PR wires it up properly.
What changed:
exceptions/comparator.go— addedcompareContainerNamethat checks all containers and init containers in a workload using regex (same as namespace/name/kind matching)exceptions/exceptionprocessor.go—metadataHasExceptionnow stripscontainerNamefrom the labels map before label comparison and runscompareContainerNameseparatelyExample exception that now works:
{ "designators": [{ "designatorType": "Attributes", "attributes": { "namespace": "kube-system", "name": "kube-proxy", "containerName": "kube-proxy" } }] }Related issues/PRs
Closes kubescape/kubescape#1684
Summary by CodeRabbit
New Features
Improvements
Tests