fix: stop emitting false-positive deprecated-API warnings for RBAC rules#496
fix: stop emitting false-positive deprecated-API warnings for RBAC rules#496SebTardif wants to merge 2 commits into
Conversation
RBAC PolicyRules only specify apiGroups and resources, not versions. The deprecatedGroupResource map was flagging any RBAC rule that referenced a group+resource combination where a beta version had been removed, even when a stable version still exists in the same group. This produced false-positive warnings for operators that use stable APIs like batch/v1 CronJob, autoscaling/v2 HPA, events.k8s.io/v1 Event, policy/v1 PDB, discovery.k8s.io/v1 EndpointSlice, and node.k8s.io/v1 RuntimeClass. Remove these entries from the deprecatedGroupResource map since RBAC rules for them are valid. Keep only PodSecurityPolicy, which was entirely removed in v1.25 with no stable replacement in the policy group. The deprecatedGvk check (for actual versioned manifests in the bundle) is unaffected and continues to correctly flag deprecated objects. Fixes operator-framework#378 Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR refines Kubernetes v1.25 “removed API” validation to avoid emitting RBAC deprecation warnings for resources that still exist under stable versions, and keeps RBAC warnings focused on resources truly removed from the API surface.
Changes:
- Limit RBAC warnings in v1.25 checks to
policy/podsecuritypolicies(fully removed), excluding resources that still exist in stable versions. - Update unit test expectations to match the reduced RBAC warning set.
- Add clarifying comments explaining why RBAC rules cannot be version-disambiguated.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/validation/internal/removed_apis.go | Narrows the “deprecated” RBAC group/resource set to only PodSecurityPolicy and documents the rationale. |
| pkg/validation/internal/removed_apis_test.go | Updates expected RBAC warnings to only include PodSecurityPolicy and adds explanatory test comments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #496 +/- ##
==========================================
- Coverage 22.13% 22.07% -0.06%
==========================================
Files 60 60
Lines 7869 7863 -6
==========================================
- Hits 1742 1736 -6
Misses 5969 5969
Partials 158 158 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Rename identifiers to match actual semantics: this map tracks resources entirely removed from their API group, not merely deprecated. Renames deprecatedGroupResource -> removedGroupResource and warnIfDeprecated -> warnIfRemoved. Suggested-by: Copilot Requested-by: perdasilva Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: perdasilva The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
Problem
The
deprecatedGroupResourcemap ingetRemovedAPIsOn1_25From()flags RBAC PolicyRules that reference API groups where a beta version was removed in Kubernetes v1.25. However, RBACPolicyRuleobjects only specifyapiGroupsandresources, they do not carry aversionfield. This means the validator cannot distinguish between a rule granting access tobatch/v1 CronJob(stable) andbatch/v1beta1 CronJob(removed).This produces false-positive warnings for every operator that has RBAC rules for common resources like CronJobs, HPAs, Events, PDBs, EndpointSlices, or RuntimeClasses, even when the operator exclusively uses stable API versions.
Impact: This warning appears on the community-operators-prod CI pipeline for every operator that needs RBAC access to these resources (see community-operators-prod#9875 for an example). As noted in the original issue, the misleading warning has led developers to incorrectly modify their RBAC rules.
Fix
Remove entries from
deprecatedGroupResourcewhere the resource still exists under a stable API version in the same group:batch/cronjobsbatch/v1(stable since v1.21)discovery.k8s.io/endpointslicesdiscovery.k8s.io/v1(stable since v1.21)events.k8s.io/eventsevents.k8s.io/v1(stable since v1.19)autoscaling/horizontalpodautoscalersautoscaling/v2(stable since v1.23)policy/poddisruptionbudgetspolicy/v1(stable since v1.21)node.k8s.io/runtimeclassesnode.k8s.io/v1(stable since v1.20)Keep
policy/podsecuritypoliciessince PodSecurityPolicy was entirely removed in v1.25 with no stable replacement in the same API group.The
deprecatedGvkcheck (for actual versioned manifests in the bundle) is unaffected and continues to correctly flag deprecated objects likeautoscaling/v2beta1 HorizontalPodAutoscalerorpolicy/v1beta1 PodDisruptionBudgetwhen they appear as bundle manifests.Testing
Test_GetRemovedAPIsOn1_25Fromto expect warnings only forpodsecuritypoliciesgo test ./...Fixes #378