OLS-0000 Scope secrets RBAC from ClusterRole to namespace-scoped Role - #1944
Open
xrajesh wants to merge 1 commit into
Open
OLS-0000 Scope secrets RBAC from ClusterRole to namespace-scoped Role#1944xrajesh wants to merge 1 commit into
xrajesh wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe operator RBAC rules now limit cluster-level Secret access to list operations. Secret and Role management permissions are scoped to the ChangesRBAC scope updates
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
xrajesh
force-pushed
the
OLS-0000/scope-secrets-rbac
branch
from
August 11, 2026 20:11
ea7ae4b to
3584953
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bundle/manifests/lightspeed-operator.clusterserviceversion.yaml (1)
803-816: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftRemove or justify the cluster-wide Secret informer permission.
The rule at Line 803 through Line 806 authorizes
listandwatchfor every Secret in the cluster. Secret list/watch responses include Secret data. Predicates inSetupWithManager()filter events only after the informer receives them, so they do not reduce this RBAC exposure. Replace the broad rule with a namespace-scoped watch and a targeted read path forpull-secret, or document and verify the required cluster-wide informer exception.🤖 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 `@bundle/manifests/lightspeed-operator.clusterserviceversion.yaml` around lines 803 - 816, Update the RBAC rules associated with the operator’s Secret informer to remove cluster-wide list/watch access to all Secrets; use a namespace-scoped watch plus targeted get/list access for the pull-secret resource, or explicitly document and verify why the cluster-wide exception is required.
🤖 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 `@config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml`:
- Around line 339-345: Update the replica description associated with
ols.deployment.rhokp.replicas to state that replicas are configurable for RHOKP
in addition to APIContainer and mcpServer. Apply the same wording to the
repeated generated descriptor entry in the bundle manifest.
- Around line 336-347: Update the RHOKP entries in the CSV to describe RHOKP as
a standalone deployment rather than a sidecar, and remove the replicas setting
entry identified by path ols.deployment.rhokp.replicas because RHOKP replicas
are always forced to 1 and must not appear configurable.
In `@internal/controller/olsconfig_controller.go`:
- Around line 126-127: The Secret RBAC marker near the controller’s RBAC
declarations hardcodes openshift-lightspeed, preventing Secret CRUD in supported
installation namespaces. Remove the fixed namespace scope and generate
permissions for the operator’s installation namespace using the repository’s
OwnNamespace/CSV RBAC configuration mechanism, while preserving the existing
Secret CRUD verbs.
---
Outside diff comments:
In `@bundle/manifests/lightspeed-operator.clusterserviceversion.yaml`:
- Around line 803-816: Update the RBAC rules associated with the operator’s
Secret informer to remove cluster-wide list/watch access to all Secrets; use a
namespace-scoped watch plus targeted get/list access for the pull-secret
resource, or explicitly document and verify why the cluster-wide exception is
required.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b10758f8-541e-4a95-91e7-2c8a9d8efa7a
⛔ Files ignored due to path filters (1)
config/rbac/role.yamlis excluded by!config/rbac/role.yaml
📒 Files selected for processing (3)
bundle/manifests/lightspeed-operator.clusterserviceversion.yamlconfig/manifests/bases/lightspeed-operator.clusterserviceversion.yamlinternal/controller/olsconfig_controller.go
Comment on lines
+336
to
+347
| - description: RHOKP standalone deployment settings (Solr / OKP). | ||
| displayName: RHOKP Container | ||
| path: ols.deployment.rhokp | ||
| - description: |- | ||
| Defines the number of desired pods. Default: "1" | ||
| Note: Replicas are configurable for APIContainer and MCP server (mcpServer). | ||
| For PostgreSQL, Console, Agentic Console, Alerts Adapter, OTEL Collector, and | ||
| Agentic Sandbox (spec.agenticOLS.agenticSandboxConfig), the number of replicas is always set to 1. | ||
| displayName: Number of replicas | ||
| path: ols.deployment.rhokp.replicas | ||
| x-descriptors: | ||
| - urn:alm:descriptor:com.tectonic.ui:podCount |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -i -C 6 'rhokp|ols\.deployment\.rhokp|replicas' .Repository: openshift/lightspeed-operator
Length of output: 50386
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- spec files ---'
git ls-files '.ai/spec/*' | head -50
printf '%s\n' '--- spec README ---'
if [ -f .ai/spec/README.md ]; then
cat -n .ai/spec/README.md
else
printf '%s\n' 'missing .ai/spec/README.md'
fi
printf '%s\n' '--- candidate API and RHOKP files ---'
git ls-files | rg '(^api/|rhokp|config/manifests/bases/lightspeed-operator\.clusterserviceversion\.yaml|config/crd|bundle/manifests/.*crd|test.*rhokp)' | head -200Repository: openshift/lightspeed-operator
Length of output: 7842
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- RHOKP specification ---'
cat -n .ai/spec/what/rhokp.md
printf '%s\n' '--- CRD API specification ---'
cat -n .ai/spec/what/crd-api.md
printf '%s\n' '--- API outline ---'
ast-grep outline api/v1alpha1/olsconfig_types.go --view expanded
printf '%s\n' '--- RHOKP implementation outline ---'
ast-grep outline internal/controller/rhokp/deployment.go --view expanded
ast-grep outline internal/controller/rhokp/assets.go --view expanded
ast-grep outline internal/controller/rhokp/reconciler.go --view expanded
printf '%s\n' '--- RHOKP test outline ---'
ast-grep outline internal/controller/rhokp/reconciler_test.go --view expandedRepository: openshift/lightspeed-operator
Length of output: 50387
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- Config and RHOKP symbol locations ---'
rg -n 'type (Config|DeploymentConfig|OLSSpec)|Deployment.*Rhokp|Rhokp|RHOKP|Replicas|UpdateDeployment|GenerateDeployment' \
api/v1alpha1/olsconfig_types.go \
internal/controller/rhokp \
config/crd/bases/ols.openshift.io_olsconfigs.yaml
printf '%s\n' '--- API focused ranges ---'
sed -n '300,470p' api/v1alpha1/olsconfig_types.go
printf '%s\n' '--- RHOKP deployment focused ranges ---'
sed -n '1,280p' internal/controller/rhokp/deployment.go
sed -n '1,220p' internal/controller/rhokp/assets.go
printf '%s\n' '--- RHOKP reconciler focused ranges ---'
sed -n '88,118p' internal/controller/rhokp/reconciler.go
sed -n '1,260p' internal/controller/rhokp/reconciler_test.go
printf '%s\n' '--- CRD RHOKP schema range ---'
rg -n -C 25 'rhokp:' config/crd/bases/ols.openshift.io_olsconfigs.yamlRepository: openshift/lightspeed-operator
Length of output: 50385
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- ApplyPodDeploymentConfig definition and callers ---'
rg -n -C 18 'func ApplyPodDeploymentConfig|ApplyPodDeploymentConfig\(' internal api
printf '%s\n' '--- RHOKP test replica and deployment assertions ---'
rg -n -C 12 'Replicas|replica|Spec\.Replicas|Deployment update|resources|toleration|nodeSelector' \
internal/controller/rhokp/reconciler_test.go \
internal/controller/rhokp/*_test.go
printf '%s\n' '--- CRD RHOKP schema ---'
sed -n '1590,1645p' config/crd/bases/ols.openshift.io_olsconfigs.yaml
printf '%s\n' '--- Relevant defaults and comparison helpers ---'
rg -n -C 18 'func SetDefaults_Deployment|func DeploymentSpecEqual|SetDefaults_Deployment\(' internal/controllerRepository: openshift/lightspeed-operator
Length of output: 50385
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
api = Path("api/v1alpha1/olsconfig_types.go").read_text()
utils = Path("internal/controller/utils/utils.go").read_text()
rhokp = Path("internal/controller/rhokp/deployment.go").read_text()
crd = Path("config/crd/bases/ols.openshift.io_olsconfigs.yaml").read_text()
csv = Path("config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml").read_text()
spec = Path(".ai/spec/what/rhokp.md").read_text()
checks = {
"API exposes rhokp as Config": '"rhokp,omitempty"' in api and "RHOKPContainer Config" in api,
"Config exposes replicas": 'Replicas *int32 `json:"replicas,omitempty"`' in api,
"RHOKP call disables replica override": "ApplyPodDeploymentConfig(deployment, cr.Spec.OLSConfig.DeploymentConfig.RHOKPContainer, false)" in rhokp,
"helper forces one when override disabled": bool(re.search(
r"if applyReplicas && config\.Replicas != nil.*?else \{\s*"
r"deployment\.Spec\.Replicas = &\[\]int32\{1\}\[0\]",
utils, re.S)),
"CRD contains RHOKP replicas": bool(re.search(
r"rhokp:.*?replicas:\s*\n\s+default: 1.*?minimum: 0",
crd, re.S)),
"CSV exposes RHOKP replica descriptor": "path: ols.deployment.rhokp.replicas" in csv,
"spec requires one RHOKP replica": "operator forces replicas to 1" in spec,
}
for name, passed in checks.items():
print(f"{'PASS' if passed else 'FAIL'}: {name}")
if not all(checks.values()):
raise SystemExit(1)
PYRepository: openshift/lightspeed-operator
Length of output: 436
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- CSV descriptor ---'
rg -n -F -C 4 'ols.deployment.rhokp.replicas' config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml || true
printf '%s\n' '--- RHOKP replica specification ---'
rg -n -i -C 3 'single replica|replicas.*1|1.*replica|forces' .ai/spec/what/rhokp.md
printf '%s\n' '--- CSV file status and matching paths ---'
git ls-files | rg 'clusterserviceversion\.yaml$'Repository: openshift/lightspeed-operator
Length of output: 1642
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- config CSV RHOKP range ---'
sed -n '325,355p' config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml
printf '%s\n' '--- bundle CSV RHOKP range ---'
sed -n '355,380p' bundle/manifests/lightspeed-operator.clusterserviceversion.yaml
printf '%s\n' '--- all RHOKP CSV paths ---'
rg -n -F -C 2 'ols.deployment.rhokp' \
config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml \
bundle/manifests/lightspeed-operator.clusterserviceversion.yamlRepository: openshift/lightspeed-operator
Length of output: 5255
Update the RHOKP CSV entry.
config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml still describes RHOKP as a sidecar. Describe it as a standalone deployment. RHOKP replicas are always forced to 1; do not present them as configurable.
🤖 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 `@config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml` around
lines 336 - 347, Update the RHOKP entries in the CSV to describe RHOKP as a
standalone deployment rather than a sidecar, and remove the replicas setting
entry identified by path ols.deployment.rhokp.replicas because RHOKP replicas
are always forced to 1 and must not appear configurable.
Comment on lines
+339
to
+345
| - description: |- | ||
| Defines the number of desired pods. Default: "1" | ||
| Note: Replicas are configurable for APIContainer and MCP server (mcpServer). | ||
| For PostgreSQL, Console, Agentic Console, Alerts Adapter, OTEL Collector, and | ||
| Agentic Sandbox (spec.agenticOLS.agenticSandboxConfig), the number of replicas is always set to 1. | ||
| displayName: Number of replicas | ||
| path: ols.deployment.rhokp.replicas |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Include RHOKP in the replica description.
The descriptor adds ols.deployment.rhokp.replicas, but the note still says that replicas are configurable only for APIContainer and mcpServer. Update the note to include RHOKP. The generated descriptor in bundle/manifests/lightspeed-operator.clusterserviceversion.yaml repeats the same text.
Proposed wording
- Note: Replicas are configurable for APIContainer and MCP server (mcpServer).
+ Note: Replicas are configurable for APIContainer, RHOKP, and MCP server (mcpServer).📝 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.
Suggested change
| - description: |- | |
| Defines the number of desired pods. Default: "1" | |
| Note: Replicas are configurable for APIContainer and MCP server (mcpServer). | |
| For PostgreSQL, Console, Agentic Console, Alerts Adapter, OTEL Collector, and | |
| Agentic Sandbox (spec.agenticOLS.agenticSandboxConfig), the number of replicas is always set to 1. | |
| displayName: Number of replicas | |
| path: ols.deployment.rhokp.replicas | |
| - description: |- | |
| Defines the number of desired pods. Default: "1" | |
| Note: Replicas are configurable for APIContainer, RHOKP, and MCP server (mcpServer). | |
| For PostgreSQL, Console, Agentic Console, Alerts Adapter, OTEL Collector, and | |
| Agentic Sandbox (spec.agenticOLS.agenticSandboxConfig), the number of replicas is always set to 1. | |
| displayName: Number of replicas | |
| path: ols.deployment.rhokp.replicas |
🤖 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 `@config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml` around
lines 339 - 345, Update the replica description associated with
ols.deployment.rhokp.replicas to state that replicas are configurable for RHOKP
in addition to APIContainer and mcpServer. Apply the same wording to the
repeated generated descriptor entry in the bundle manifest.
Comment on lines
+126
to
+127
| // Secret CRUD scoped to the operator namespace (namespace-scoped Role, not ClusterRole) | ||
| // +kubebuilder:rbac:groups="",resources=secrets,namespace=openshift-lightspeed,verbs=get;list;watch;create;update;delete;deletecollection |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 4 \
'namespace=openshift-lightspeed|r\.Options\.Namespace|olm\.targetNamespaces|OwnNamespace|RoleBinding' \
internal/controller config bundle cmdRepository: openshift/lightspeed-operator
Length of output: 50385
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- project specification overview ---'
if [ -f .ai/spec/README.md ]; then
sed -n '1,220p' .ai/spec/README.md
fi
printf '%s\n' '--- RBAC marker and generated RBAC resources ---'
sed -n '115,145p' internal/controller/olsconfig_controller.go
rg -n -C 8 \
'resources: *secrets|namespace: *openshift-lightspeed|name:.*secret|kind: Role|kind: ClusterRole|installModes:|OwnNamespace|WATCH_NAMESPACE|olm.targetNamespaces' \
config/rbac bundle/manifests/lightspeed-operator.clusterserviceversion.yaml \
--glob '*.yaml' --glob '*.yml' | head -n 240
printf '%s\n' '--- namespace defaults and manager configuration ---'
rg -n -C 5 \
'OLSNamespaceDefault|WATCH_NAMESPACE|Options\.Namespace|Namespace:' \
internal/controller cmd config bundle/manifests/lightspeed-operator.clusterserviceversion.yaml \
--glob '*.go' --glob '*.yaml' --glob '*.yml' | head -n 260Repository: openshift/lightspeed-operator
Length of output: 42046
Generate Secret CRUD RBAC for the installation namespace. The CSV supports OwnNamespace, and WATCH_NAMESPACE can differ from openshift-lightspeed; no repository setting enforces that namespace. A fixed Role namespace causes Secret create, update, and delete requests to fail in another supported installation namespace.
🤖 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 `@internal/controller/olsconfig_controller.go` around lines 126 - 127, The
Secret RBAC marker near the controller’s RBAC declarations hardcodes
openshift-lightspeed, preventing Secret CRUD in supported installation
namespaces. Remove the fixed namespace scope and generate permissions for the
operator’s installation namespace using the repository’s OwnNamespace/CSV RBAC
configuration mechanism, while preserving the existing Secret CRUD verbs.
The operator's manager-role ClusterRole granted full CRUD on secrets cluster-wide, but all write operations only target the operator namespace. Move secrets CRUD to a namespace-scoped Role (OLM permissions) and remove the unused patch verb. The cluster-scoped pull-secret read and informer list/watch are unchanged. Resolves: Glasswing AI-SAST FIND-001 (CWE-269, CWE-732) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
xrajesh
force-pushed
the
OLS-0000/scope-secrets-rbac
branch
from
August 11, 2026 21:32
3584953 to
c70b0d5
Compare
|
@xrajesh: all tests passed! 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. |
Contributor
Author
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
Contributor
|
/lgtm |
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
clusterPermissions(ClusterRole) to namespace-scopedpermissions(Role) in the OLM CSV. All secret write operations only target the operator namespace — the ClusterRole was granting unnecessary cluster-wide access.patchverb on secrets (noPatch()calls on secrets exist in the codebase).pull-secretread (resourceNames-scoped) and informerlist;watchunchanged.Changes
internal/controller/olsconfig_controller.gonamespace=openshift-lightspeed, removedpatchconfig/rbac/role.yamlbundle/manifests/...csv.yamlclusterPermissionstopermissionsconfig/manifests/bases/...csv.yamlTest plan
make manifests— regenerated RBAC from markersmake bundle— bundle validated successfullymake test— all 13 test suites pass (pre-existingcovdatatool error ininternal/relatedimagesis unrelated)pull-secretinopenshift-config)🤖 Generated with Claude Code
Summary by CodeRabbit