Skip to content

fix(admission): take the cluster-object opt-out from the binding, not the object - #494

Merged
matthyx merged 1 commit into
kubescape:mainfrom
itsvishalyadav:fix/cluster-object-opt-out-from-binding
Nov 12, 2026
Merged

matthyx merged 1 commit into
kubescape:mainfrom
itsvishalyadav:fix/cluster-object-opt-out-from-binding

Conversation

@itsvishalyadav

@itsvishalyadav itsvishalyadav commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Overview

This PR fixes #493

Signed Commits

  • Yes, I signed my commits.

ListRulesForObject no longer reads includeClusterObjects from the object being admitted. The opt-out now comes from configuration:

  • With rule bindings: the label is read from the RuntimeRuleAlertBinding. A binding labelled includeClusterObjects: "false" applies to namespaced objects only; without the label it applies to cluster-scoped objects too, as today. The helper is bindingIncludesClusterObjects(rb) in helpers.go. The binding is cluster-scoped, so only whoever can manage bindings can set it.
  • With rule bindings ignored (rulesUpdateConfig.enabled): a new operator setting decides, rulesUpdateConfig.excludeClusterObjects (RulesUpdaterConfig.ExcludeClusterObjects). It defaults to false, today's default behaviour. NewCache takes it as a fourth argument, wired from main.go.
  • Docs: docs/features/cel-admission-rules.md gets a "Cluster-Scoped Objects" section.

Design notes:

  • Why a label on the binding: it needs no CRD schema change or node-agent type change.
  • Why exclude… defaulting to false: the zero value keeps today's behaviour, so a struct literal can't silently switch evaluation off.
  • Compatibility: an object's own label is no longer honoured at all. Since evaluation only raises alerts, users who labelled objects get more alerts, never denied requests, and they can move the label to the binding.
  • Not in this repo: to expose the setting through Helm, kubescape/helm-charts would need excludeClusterObjects in templates/operator/configmap.yaml. I can send that once you're happy with the shape here.

Questions where I'd value your preference:

  • a label on the binding, or an annotation or spec field instead;
  • whether the setting belongs in rulesUpdateConfig or should be admission-wide;
  • whether to log a deprecation warning when an admitted object still carries the label.

How to Test

GOOS=linux go test -c -o cache.test ./admission/rulebinding/cache/   # the package imports Linux-only code
./cache.test -test.run 'TestListRulesForObjectIgnoreBindings|TestListRulesForClusterObject' -test.v

TestListRulesForObjectIgnoreBindings (rewritten) covers:

  • a namespaced object gets all rules;
  • an object's own label doesn't opt it out;
  • the operator config excludes cluster objects (built through NewCache, so the wiring is covered too).

New TestListRulesForClusterObject covers:

  • a binding applies to cluster objects by default;
  • an object's own label doesn't opt it out;
  • a binding label opts that binding out;
  • a binding label set to true keeps cluster objects;
  • an opted-out binding still applies to namespaced objects.

I ran the package tests for Linux on WSL: with the fix all pass; with main's cache.go the new tests fail, including both "object label doesn't opt out" cases. GOOS=linux go vet is clean for the touched packages, and gofmt is clean on the touched files.

Checklist before requesting a review

  • My code follows the style guidelines of this project
  • I have commented on my code, particularly in hard-to-understand areas
  • I have performed a self-review of my code
  • If it is a core feature, I have added thorough tests.
  • New and existing unit tests pass locally with my changes

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Cluster-scoped objects are included in admission-rule evaluation by default. Individual bindings can opt out of applying to these objects.
    • When rule bindings are ignored, a new configuration option controls whether cluster-scoped objects are excluded from evaluation. This option defaults to off.
  • Documentation
    • Added guidance on how cluster-scoped objects are handled, including the binding label and operator setting that control their evaluation.

… the object

ListRulesForObject decided whether admission rules apply to a
cluster-scoped object from the includeClusterObjects label on the object
being admitted, so whoever created or updated a ClusterRoleBinding,
ClusterRole, webhook configuration or Namespace could switch off its
evaluation by adding that label. Read it from the RuntimeRuleAlertBinding
instead (a binding labelled includeClusterObjects: "false" applies to
namespaced objects only), and, when rule bindings are ignored, from a new
operator setting rulesUpdateConfig.excludeClusterObjects (default false,
today's behaviour). The admitted object's label no longer has any effect.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: itsvishalyadav <vishalyadav87695@gmail.com>
@coderabbitai

coderabbitai Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The cache now reads includeClusterObjects from each rule binding. When bindings are ignored, the excludeClusterObjects operator setting controls whether cluster-scoped objects receive rules. The setting has a false default and is passed to the cache during initialization.

Changes: Cluster-scoped admission rule selection

Layer / File(s) Summary
Operator exclusion configuration and cache path
admission/rulesupdate/updater.go, config/config.go, admission/rulebinding/cache/cache.go, main.go, admission/rulebinding/cache/cache_test.go
Adds the excludeClusterObjects configuration field with a false default, passes it to the cache, and tests exclusion when bindings are ignored.
Binding-based cluster-object selection
admission/rulebinding/cache/cache.go, admission/rulebinding/cache/helpers.go, admission/rulebinding/cache/cache_test.go, docs/features/cel-admission-rules.md
Uses the binding’s includeClusterObjects label for cluster-scoped objects. The tests and documentation cover the label behavior and clarify that admitted-object labels do not control selection.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested labels: ai-assisted

Suggested reviewers: matthyx, slashben

Merge Risk: Minimal · up to a4a26

This change moves the cluster-object opt-out from a label on the admitted object to the binding or the operator configuration, which matches the linked issue. No actionable merge-blocking risk is visible.

Pre-merge checks: Passed 4

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title names the main change: the cluster-object opt-out is now read from the RuntimeRuleAlertBinding instead of the admitted object. The raw summary and objectives confirm this. The conventional-c…
Linked Issues check Passed Issue #493 requires that the cluster-object opt-out come from Kubescape configuration, not from the admitted object. At head, admission/rulebinding/cache/cache.go no longer reads the includeClusterObj…
Out of Scope Changes check Passed The changes stay within the issue's scope. cache.go, helpers.go, and cache_test.go change the opt-out logic. admission/rulesupdate/updater.go and config/config.go add the excludeClusterObjects setting…
Full details: Title check

Explanation

The title names the main change: the cluster-object opt-out is now read from the RuntimeRuleAlertBinding instead of the admitted object. The raw summary and objectives confirm this. The conventional-commit prefix 'fix(admission):' is accurate for a behavior correction. The title is specific and concise, and it is 80 characters, so it is within a typical length limit.

Full details: Linked Issues check

Explanation

Issue #493 requires that the cluster-object opt-out come from Kubescape configuration, not from the admitted object. At head, admission/rulebinding/cache/cache.go no longer reads the includeClusterObjects label from the object. Two paths implement the requirement. (1) With rule bindings ignored (ignoreRuleBindings), ListRulesForObject returns no rules for cluster-scoped objects only when the operator-level excludeClusterObjects flag is set. That flag comes from NewCache, and the summary reports it is wired from rulesUpdateConfig.excludeClusterObjects in main.go. (2) With bindings, the label is read from the RuntimeRuleAlertBinding via bindingIncludesClusterObjects(rb) in helpers.go, which returns false only for the exact value "false". The object's own label is no longer consulted. The issue's expected behaviour is met. The summary reports new and rewritten tests for these cases; I did not run them.

Full details: Out of Scope Changes check

Explanation

The changes stay within the issue's scope. cache.go, helpers.go, and cache_test.go change the opt-out logic. admission/rulesupdate/updater.go and config/config.go add the excludeClusterObjects setting, which the issue's proposed fix requires. main.go passes that setting into NewCache. docs/features/cel-admission-rules.md documents the new behaviour. The tests cover the changed logic. No unrelated functional change appears in the summary. The validator's alert-only behaviour is unchanged, as the issue requests.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fabe28ec-6bf3-46a4-8e48-ce7920fa6820
Commits

Reviewing files that changed from the base of the PR and between a9fc09a and a4a26e3.

Files selected for processing: 7

  • admission/rulebinding/cache/cache.go
  • admission/rulebinding/cache/cache_test.go
  • admission/rulebinding/cache/helpers.go
  • admission/rulesupdate/updater.go
  • config/config.go
  • docs/features/cel-admission-rules.md
  • main.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

@matthyx matthyx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed a4a26e37635535eb8437d50834b73b1a995e9be8 against main at a9fc09a55f434dbdf2f6b56e86a60952705a1a00.

The issue in #493 is real: the old code let the object under admission suppress its own cluster-scoped evaluation. This change moves the decision to the binding label or operator configuration, keeps the default behavior, and adds regression tests for both paths. I found no code-level blocker. Related PRs #346 (ignore-bindings mode) and #402 (namespace selector matching) were merged; neither rejected this approach. Broader closed-PR/issue searches were limited by GitHub search API rate limiting; a title scan found no superseding fix.

CI passed for Basic-Test (including admission/rulebinding/cache), go test -tags=integration ./..., cross-platform build, CodeQL, DCO, GitGuardian, and Dependency Quality. License Compliance is failing with FOSSA's “1 issues found.” The same failure appears on merged PRs #492 and #346, but the FOSSA report does not expose the finding details here, so I cannot verify it is an unrelated infrastructure failure. I did not run tests locally because this shared workspace is not an isolated environment for executing untrusted PR code.

Request changes for the failing CI check only; no source-code blocker found. Please get License Compliance green or have the failure verified as unrelated before approval. A Helm follow-up was opened: kubescape/helm-charts#1023 (the chart does not yet expose the new setting).

@itsvishalyadav

Copy link
Copy Markdown
Contributor Author

Thanks for the review. On License Compliance: this PR doesn't touch go.mod or go.sum, so its dependency tree is the same as main at a9fc09a. The FOSSA failure first shows up on #492, which changed go.mod/go.sum (syft v1.55.0, the AWS SDK v2 modules, cyclonedx-go and others) and has the same "1 issues found". The PRs merged just before it, #489 and #491, pass. So the failing check comes from what's already on main, not from this change. I can't see the finding details in the FOSSA report either. If you'd like it green before merging, re-running FOSSA once the #492 finding is triaged should clear it here too.

@matthyx matthyx moved this to Needs Reviewer in KS PRs tracking Nov 12, 2026
@matthyx
matthyx merged commit 1c4c226 into kubescape:main Nov 12, 2026
9 of 10 checks passed
@matthyx matthyx moved this from Needs Reviewer to To Archive in KS PRs tracking Nov 12, 2026
itsvishalyadav added a commit to itsvishalyadav/helm-charts that referenced this pull request Nov 12, 2026
No operator release has kubescape/operator#494 yet: v0.2.220 and earlier,
including the chart's default image tag, ignore the setting. Say so in
values.yaml and the README, and point to operator.image.tag until the
default moves to a release that includes it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: itsvishalyadav <vishalyadav87695@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: To Archive

Development

Successfully merging this pull request may close these issues.

The includeClusterObjects opt-out is read from the admitted object, so its creator can switch off evaluation

2 participants