WIP: STOR-2962: Save SELinuxWarningController upgradeability to a ConfigMap#2671
WIP: STOR-2962: Save SELinuxWarningController upgradeability to a ConfigMap#2671jsafrane wants to merge 2 commits into
Conversation
openshift-ci-robot
added
jira/valid-reference
|
@jsafrane: This pull request references STOR-2962 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
@jsafrane: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
WalkthroughRefactors the SELinux volume conflict cache to a query-based API with parsed-label detection and reverse indexing, exposes a conflict-count interface, updates metrics and tests, and adds a periodic OpenShift reporter that patches/creates a ConfigMap indicating whether conflicts are present. ChangesSELinux Cache Refactoring and OpenShift Reporter
🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jsafrane The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@jsafrane: This pull request references STOR-2962 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/controller/volume/selinuxwarning/openshift_upgrade_controller.go`:
- Around line 23-25: The package-global variable previousUnogradeable should be
moved onto the Controller type to avoid shared state across controller instances
and tests: remove the package-level previousUnogradeable declaration and add a
field (e.g., previousUnogradeable metav1.ConditionStatus) to the Controller
struct, initialize it in the controller constructor/New function, and update all
references in openshift_upgrade_controller.go (including the usages around lines
43-55) to use c.previousUnogradeable instead of the global name; ensure any
concurrent access follows the Controller's existing synchronization model if
needed.
- Around line 57-59: In getUpgradeability, avoid the unchecked assertion
c.labelCache.(cache.ConflictCounter); instead use a safe type assertion (e.g.,
cc, ok := c.labelCache.(cache.ConflictCounter)) and handle the !ok case by
logging via the provided logger and returning a safe failure status (for example
metav1.ConditionFalse) so the controller won't panic; when ok is true continue
using cc.GetConflictCount() as before.
- Around line 68-81: The current JSON patch built with json.Marshal uses an op
"replace" on path "/data" which fails if the ConfigMap exists without a data
field; change the patch to perform a merge-patch or use a JSON Patch "add" op
for the specific key instead of replacing /data. Update the code that builds
patch (the json.Marshal call that creates patch) and the subsequent call to
c.kubeClient.CoreV1().ConfigMaps(configMapNamespace).Patch(...) (currently using
types.JSONPatchType) so it either constructs a strategic/merge JSON object that
upserts the "upgradeable" key under data, or uses a JSON Patch with op "add" and
path "/data/upgradeable"; ensure configMapName and configMapNamespace are left
intact and error handling remains the same.
In `@plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go`:
- Line 567: The rule created by rbacv1helpers.NewRule("create",
"patch").Groups(legacyGroup).Resources("configmaps").RuleOrDie() is too broad;
restrict it to the specific assessment ConfigMap by adding
ResourceNames("selinux-warning-assessment") (i.e., change the chain to include
.ResourceNames("selinux-warning-assessment")) so the generated role matches the
golden least-privilege scope.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8e9778de-4b6b-4bc0-b002-00c6cdb7e155
📒 Files selected for processing (10)
pkg/controller/volume/selinuxwarning/cache/openshift_patch.gopkg/controller/volume/selinuxwarning/cache/volumecache.gopkg/controller/volume/selinuxwarning/cache/volumecache_test.gopkg/controller/volume/selinuxwarning/metrics.gopkg/controller/volume/selinuxwarning/openshift_upgrade_controller.gopkg/controller/volume/selinuxwarning/selinux_warning_controller.gopkg/controller/volume/selinuxwarning/selinux_warning_controller_test.gopkg/controller/volume/selinuxwarning/translator/selinux_translator.goplugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.goplugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/controller-roles.yaml
| var ( | ||
| previousUnogradeable metav1.ConditionStatus = metav1.ConditionUnknown | ||
| ) |
There was a problem hiding this comment.
Move upgradeability state off package-global storage.
previousUnogradeable being package-global makes state shared across controller instances and test cases. Keep this on Controller to avoid cross-instance leakage and concurrency hazards.
Suggested fix
- var (
- previousUnogradeable metav1.ConditionStatus = metav1.ConditionUnknown
- )
+ // in Controller struct (pkg/controller/volume/selinuxwarning/selinux_warning_controller.go):
+ // previousUpgradeable metav1.ConditionStatus
func (c *Controller) checkForOpenShiftUpgrade(ctx context.Context) {
@@
- if currentUpgradeable == previousUnogradeable {
+ if currentUpgradeable == c.previousUpgradeable {
@@
- previousUnogradeable = currentUpgradeable
+ c.previousUpgradeable = currentUpgradeable
}Also applies to: 43-55
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/controller/volume/selinuxwarning/openshift_upgrade_controller.go` around
lines 23 - 25, The package-global variable previousUnogradeable should be moved
onto the Controller type to avoid shared state across controller instances and
tests: remove the package-level previousUnogradeable declaration and add a field
(e.g., previousUnogradeable metav1.ConditionStatus) to the Controller struct,
initialize it in the controller constructor/New function, and update all
references in openshift_upgrade_controller.go (including the usages around lines
43-55) to use c.previousUnogradeable instead of the global name; ensure any
concurrent access follows the Controller's existing synchronization model if
needed.
| func (c *Controller) getUpgradeability(logger klog.Logger) metav1.ConditionStatus { | ||
| conflictsCount := c.labelCache.(cache.ConflictCounter).GetConflictCount() | ||
| if conflictsCount > 0 { |
There was a problem hiding this comment.
Avoid unchecked type assertion on labelCache.
c.labelCache.(cache.ConflictCounter) can panic if labelCache is swapped with an implementation that doesn’t satisfy ConflictCounter. Guard the assertion and fail safely.
Suggested fix
func (c *Controller) getUpgradeability(logger klog.Logger) metav1.ConditionStatus {
- conflictsCount := c.labelCache.(cache.ConflictCounter).GetConflictCount()
+ conflictCounter, ok := c.labelCache.(cache.ConflictCounter)
+ if !ok {
+ logger.Error(nil, "labelCache does not implement ConflictCounter")
+ return metav1.ConditionUnknown
+ }
+ conflictsCount := conflictCounter.GetConflictCount()
if conflictsCount > 0 {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (c *Controller) getUpgradeability(logger klog.Logger) metav1.ConditionStatus { | |
| conflictsCount := c.labelCache.(cache.ConflictCounter).GetConflictCount() | |
| if conflictsCount > 0 { | |
| func (c *Controller) getUpgradeability(logger klog.Logger) metav1.ConditionStatus { | |
| conflictCounter, ok := c.labelCache.(cache.ConflictCounter) | |
| if !ok { | |
| logger.Error(nil, "labelCache does not implement ConflictCounter") | |
| return metav1.ConditionUnknown | |
| } | |
| conflictsCount := conflictCounter.GetConflictCount() | |
| if conflictsCount > 0 { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/controller/volume/selinuxwarning/openshift_upgrade_controller.go` around
lines 57 - 59, In getUpgradeability, avoid the unchecked assertion
c.labelCache.(cache.ConflictCounter); instead use a safe type assertion (e.g.,
cc, ok := c.labelCache.(cache.ConflictCounter)) and handle the !ok case by
logging via the provided logger and returning a safe failure status (for example
metav1.ConditionFalse) so the controller won't panic; when ok is true continue
using cc.GetConflictCount() as before.
| patch, err := json.Marshal([]map[string]any{ | ||
| { | ||
| "op": "replace", | ||
| "path": "/data", | ||
| "value": map[string]string{ | ||
| "upgradeable": string(upgradeable), | ||
| }, | ||
| }, | ||
| }) | ||
| if err != nil { | ||
| return fmt.Errorf("error building config map %s patch: %w", configMapName, err) | ||
| } | ||
| _, err = c.kubeClient.CoreV1().ConfigMaps(configMapNamespace).Patch(ctx, configMapName, types.JSONPatchType, patch, metav1.PatchOptions{}) | ||
| if err != nil { |
There was a problem hiding this comment.
Use merge-patch (or add op) instead of replace /data.
replace on /data fails when the target ConfigMap exists without data. This makes updates unnecessarily brittle. Use merge-patch for idempotent upsert behavior of the key.
Suggested fix
- patch, err := json.Marshal([]map[string]any{
- {
- "op": "replace",
- "path": "/data",
- "value": map[string]string{
- "upgradeable": string(upgradeable),
- },
- },
- })
+ patch, err := json.Marshal(map[string]any{
+ "data": map[string]string{
+ "upgradeable": string(upgradeable),
+ },
+ })
@@
- _, err = c.kubeClient.CoreV1().ConfigMaps(configMapNamespace).Patch(ctx, configMapName, types.JSONPatchType, patch, metav1.PatchOptions{})
+ _, err = c.kubeClient.CoreV1().ConfigMaps(configMapNamespace).Patch(ctx, configMapName, types.MergePatchType, patch, metav1.PatchOptions{})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| patch, err := json.Marshal([]map[string]any{ | |
| { | |
| "op": "replace", | |
| "path": "/data", | |
| "value": map[string]string{ | |
| "upgradeable": string(upgradeable), | |
| }, | |
| }, | |
| }) | |
| if err != nil { | |
| return fmt.Errorf("error building config map %s patch: %w", configMapName, err) | |
| } | |
| _, err = c.kubeClient.CoreV1().ConfigMaps(configMapNamespace).Patch(ctx, configMapName, types.JSONPatchType, patch, metav1.PatchOptions{}) | |
| if err != nil { | |
| patch, err := json.Marshal(map[string]any{ | |
| "data": map[string]string{ | |
| "upgradeable": string(upgradeable), | |
| }, | |
| }) | |
| if err != nil { | |
| return fmt.Errorf("error building config map %s patch: %w", configMapName, err) | |
| } | |
| _, err = c.kubeClient.CoreV1().ConfigMaps(configMapNamespace).Patch(ctx, configMapName, types.MergePatchType, patch, metav1.PatchOptions{}) | |
| if err != nil { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/controller/volume/selinuxwarning/openshift_upgrade_controller.go` around
lines 68 - 81, The current JSON patch built with json.Marshal uses an op
"replace" on path "/data" which fails if the ConfigMap exists without a data
field; change the patch to perform a merge-patch or use a JSON Patch "add" op
for the specific key instead of replacing /data. Update the code that builds
patch (the json.Marshal call that creates patch) and the subsequent call to
c.kubeClient.CoreV1().ConfigMaps(configMapNamespace).Patch(...) (currently using
types.JSONPatchType) so it either constructs a strategic/merge JSON object that
upserts the "upgradeable" key under data, or uses a JSON Patch with op "add" and
path "/data/upgradeable"; ensure configMapName and configMapNamespace are left
intact and error handling remains the same.
| rbacv1helpers.NewRule("get", "list", "watch").Groups(legacyGroup).Resources("persistentvolumeclaims").RuleOrDie(), | ||
| rbacv1helpers.NewRule("get", "list", "watch").Groups(legacyGroup).Resources("pods").RuleOrDie(), | ||
| rbacv1helpers.NewRule("get", "list", "watch").Groups(storageGroup).Resources("csidrivers").RuleOrDie(), | ||
| rbacv1helpers.NewRule("create", "patch").Groups(legacyGroup).Resources("configmaps").RuleOrDie(), |
There was a problem hiding this comment.
Scope ConfigMap permissions to the assessment object (and align with golden).
This rule grants create/patch on all ConfigMaps, while the paired golden role scopes to selinux-warning-assessment. Please align policy generation with the intended least-privilege scope.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go` at line
567, The rule created by rbacv1helpers.NewRule("create",
"patch").Groups(legacyGroup).Resources("configmaps").RuleOrDie() is too broad;
restrict it to the specific assessment ConfigMap by adding
ResourceNames("selinux-warning-assessment") (i.e., change the chain to include
.ResourceNames("selinux-warning-assessment")) so the generated role matches the
golden least-privilege scope.
|
@jsafrane: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
…ConfigMap To upgrade OCP to Kubernetes 1.37 with SELinuxMount enabled, we need to ensure there are no user workloads that could get broken by the feature gate. SELinuxWarningController in KCP has the information and emits it as a metric. To mark the cluster un-upgradeable easily using API objects, store the information as a ConfigMap too. Reading metrics in an operator is too complicated.
jsafrane
force-pushed
the
kcm-selinux-configmap
branch
from
cde26e2 to
1432d8b
Compare
|
@jsafrane: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
|
@jsafrane: This pull request references STOR-2962 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go (1)
567-567:⚠️ Potential issue | 🟠 Major | ⚡ Quick winScope ConfigMap permissions to the single assessment object.
This grants
create/patchon allconfigmaps. Restrict with.ResourceNames("selinux-warning-assessment")to enforce least privilege and align policy output.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go` at line 567, The rule currently grants create/patch on all configmaps via rbacv1helpers.NewRule("create", "patch").Groups(legacyGroup).Resources("configmaps").RuleOrDie(); restrict it to only the single assessment object by adding .ResourceNames("selinux-warning-assessment") to that rule so it becomes rbacv1helpers.NewRule(...).Groups(legacyGroup).Resources("configmaps").ResourceNames("selinux-warning-assessment").RuleOrDie(), thereby limiting permissions to the specific ConfigMap.pkg/controller/volume/selinuxwarning/openshift_upgrade_controller.go (3)
23-25:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid package-global status state for controller instance logic.
previousConflictsas package-global shares state across controller instances/tests. Keep it onControllerinstead.Also applies to: 43-55
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/controller/volume/selinuxwarning/openshift_upgrade_controller.go` around lines 23 - 25, The package-global variable previousConflicts (metav1.ConditionStatus) creates shared state; move it into the Controller struct as a field (e.g., Controller.previousConflicts) and remove the global declaration; update all references (previousConflicts -> c.previousConflicts) inside methods such as those changed around lines 43-55 and initialize it via the Controller constructor (NewController) or rely on the zero-value metav1.ConditionUnknown; also update any tests to construct Controller with the desired initial previousConflicts value instead of relying on package-global state.
82-95:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPatch strategy is brittle for ConfigMaps missing
.data.
replaceon/datacan fail when the field is absent. Use merge-patch (or JSON Patchaddon the key) for idempotent updates.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/controller/volume/selinuxwarning/openshift_upgrade_controller.go` around lines 82 - 95, The JSON Patch uses a "replace" on "/data" which fails if .data is missing; instead build a merge patch that sets data.conflictsPresent (e.g. marshal map[string]any{"data": map[string]string{"conflictsPresent": string(conflictsPresent)}}) and call Patch with types.MergePatchType, or switch to a JSON Patch "add" op targeting "/data/conflictsPresent" so the update is idempotent; update the code around the json.Marshal call and the Patch invocation that uses c.kubeClient.CoreV1().ConfigMaps(configMapNamespace).Patch to use the merge patch payload and types.MergePatchType (or use the "add" JSON Patch op) referencing configMapName, configMapNamespace and conflictsPresent.
57-59:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUnchecked type assertion can panic.
Use a safe assertion before calling
GetConflictCount()to avoid runtime panic whenlabelCacheis swapped with a non-ConflictCounterimplementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/controller/volume/selinuxwarning/openshift_upgrade_controller.go` around lines 57 - 59, In getConflicts, replace the unchecked type assertion on c.labelCache to cache.ConflictCounter with a safe comma-ok assertion (e.g., cc, ok := c.labelCache.(cache.ConflictCounter)); if ok use cc.GetConflictCount(), otherwise log the mismatch via the provided logger and treat conflictsCount as 0 (returning metav1.ConditionFalse) to avoid a panic; reference getConflicts, c.labelCache, cache.ConflictCounter, and GetConflictCount when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/controller/volume/selinuxwarning/openshift_upgrade_controller.go`:
- Around line 19-21: The ConfigMap name constant configMapName is set to
"selinux-conflicts" but the RBAC resourceNames scope targets
"selinux-warning-assessment", causing create/patch calls to be denied; update
the constant configMapName to "selinux-warning-assessment" (or alternatively
adjust the RBAC resourceNames to match the current constant) so the value used
in the controller (configMapName and any code that references it) matches the
RBAC-scoped object name.
---
Duplicate comments:
In `@pkg/controller/volume/selinuxwarning/openshift_upgrade_controller.go`:
- Around line 23-25: The package-global variable previousConflicts
(metav1.ConditionStatus) creates shared state; move it into the Controller
struct as a field (e.g., Controller.previousConflicts) and remove the global
declaration; update all references (previousConflicts -> c.previousConflicts)
inside methods such as those changed around lines 43-55 and initialize it via
the Controller constructor (NewController) or rely on the zero-value
metav1.ConditionUnknown; also update any tests to construct Controller with the
desired initial previousConflicts value instead of relying on package-global
state.
- Around line 82-95: The JSON Patch uses a "replace" on "/data" which fails if
.data is missing; instead build a merge patch that sets data.conflictsPresent
(e.g. marshal map[string]any{"data": map[string]string{"conflictsPresent":
string(conflictsPresent)}}) and call Patch with types.MergePatchType, or switch
to a JSON Patch "add" op targeting "/data/conflictsPresent" so the update is
idempotent; update the code around the json.Marshal call and the Patch
invocation that uses c.kubeClient.CoreV1().ConfigMaps(configMapNamespace).Patch
to use the merge patch payload and types.MergePatchType (or use the "add" JSON
Patch op) referencing configMapName, configMapNamespace and conflictsPresent.
- Around line 57-59: In getConflicts, replace the unchecked type assertion on
c.labelCache to cache.ConflictCounter with a safe comma-ok assertion (e.g., cc,
ok := c.labelCache.(cache.ConflictCounter)); if ok use cc.GetConflictCount(),
otherwise log the mismatch via the provided logger and treat conflictsCount as 0
(returning metav1.ConditionFalse) to avoid a panic; reference getConflicts,
c.labelCache, cache.ConflictCounter, and GetConflictCount when making the
change.
In `@plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go`:
- Line 567: The rule currently grants create/patch on all configmaps via
rbacv1helpers.NewRule("create",
"patch").Groups(legacyGroup).Resources("configmaps").RuleOrDie(); restrict it to
only the single assessment object by adding
.ResourceNames("selinux-warning-assessment") to that rule so it becomes
rbacv1helpers.NewRule(...).Groups(legacyGroup).Resources("configmaps").ResourceNames("selinux-warning-assessment").RuleOrDie(),
thereby limiting permissions to the specific ConfigMap.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3b898633-0b8e-48a5-a9cc-d00e6e0c9b42
📒 Files selected for processing (5)
pkg/controller/volume/selinuxwarning/cache/openshift_patch.gopkg/controller/volume/selinuxwarning/openshift_upgrade_controller.gopkg/controller/volume/selinuxwarning/selinux_warning_controller.goplugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.goplugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/controller-roles.yaml
| configMapNamespace = "openshift-config" | ||
| configMapName = "selinux-conflicts" | ||
| ) |
There was a problem hiding this comment.
ConfigMap name does not match the RBAC-scoped assessment object.
configMapName is selinux-conflicts, but this PR’s RBAC scope targets selinux-warning-assessment. With resourceNames scoping, patch/create will fail for the current name.
Suggested fix
const (
checkInterval = 30 * time.Second
configMapNamespace = "openshift-config"
- configMapName = "selinux-conflicts"
+ configMapName = "selinux-warning-assessment"
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| configMapNamespace = "openshift-config" | |
| configMapName = "selinux-conflicts" | |
| ) | |
| const ( | |
| checkInterval = 30 * time.Second | |
| configMapNamespace = "openshift-config" | |
| configMapName = "selinux-warning-assessment" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/controller/volume/selinuxwarning/openshift_upgrade_controller.go` around
lines 19 - 21, The ConfigMap name constant configMapName is set to
"selinux-conflicts" but the RBAC resourceNames scope targets
"selinux-warning-assessment", causing create/patch calls to be denied; update
the constant configMapName to "selinux-warning-assessment" (or alternatively
adjust the RBAC resourceNames to match the current constant) so the value used
in the controller (configMapName and any code that references it) matches the
RBAC-scoped object name.
2 participants
KCM's SELinuxWarningController knows how many Pods could get broken by upgrade to Kubernetes 1.37 / a version where
SELinuxMountfeature gate is enabled.Add a carry patch to KCM to store the information into a ConfigMap
openshift-config/selinux-conflicts.cluster-storage-operator can read it from there and mark itself
Upgradeable: false.See openshift/enhancements#2010 for details.
<carry>. Written in a way that minimizes a possibility of a conflict with future Kubernetes patches.Summary by CodeRabbit
New Features
Improvements
Tests
Chores