OLS-1894 Add ROSA-aware OKP retrieval spec#1790
Conversation
Add spec documentation for ROSA cluster detection and ROSA-specific OKP product filtering. The operator detects ROSA via Console brand and Infrastructure controlPlaneTopology, passing the product identifier as OLS_ROSA_PRODUCT env var to the service. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughDocumentation-only changes to three spec files describing planned (OLS-1894) app-server environment variables: ChangesROSA/Cluster Version Documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.ai/spec/what/app-server.md:
- Around line 26-32: Add the missing RBAC permission for the ROSA variant
lookup: the lookup role currently covers `clusterversions` and secret access,
but `Infrastructure/cluster` requires `get` on
`config.openshift.io/infrastructures`. Update the role used by the app-server
lookup path so the `controlPlaneTopology` read in the ROSA detection flow is
allowed, and mirror the same permission in the generated manifest/CSV. Ensure
the change stays aligned with the `OLS_ROSA_PRODUCT` detection logic that
depends on this lookup.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 81584dbc-a243-44cb-a026-5c91e3ab2a9d
📒 Files selected for processing (3)
.ai/spec/how/config-generation.md.ai/spec/how/deployment-generation.md.ai/spec/what/app-server.md
| ### ROSA-Aware OKP Retrieval [PLANNED: OLS-1894] | ||
| 15b. Unless `byokRAGOnly` is true, the operator detects whether the cluster is ROSA and, if so, which variant (Classic vs HCP). Detection uses two standard OpenShift API resources, following the same pattern as OCP version detection — determined once and passed to the service as an environment variable: | ||
| - **ROSA detection:** Read `console.operator.openshift.io/v1` Console `cluster` resource, field `.spec.customization.brand`. Value `ROSA` indicates a ROSA cluster (reliable on OCP 4.16+). | ||
| - **Variant detection:** Read `infrastructure.config.openshift.io/v1` Infrastructure `cluster` resource, field `.status.controlPlaneTopology`. `External` = HCP, `HighlyAvailable` = Classic. | ||
| - When ROSA is detected, the operator sets `OLS_ROSA_PRODUCT` on the app-server container: `red_hat_openshift_service_on_aws` for HCP, `red_hat_openshift_service_on_aws_classic_architecture` for Classic. | ||
| - On non-ROSA clusters the env var is absent and the service uses OCP-only retrieval. | ||
| - RBAC: requires `get` on `consoles` in the `operator.openshift.io` API group (Infrastructure is already covered by existing cluster-version permissions). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repo files of interest:\n'
git ls-files '.ai/spec/what/app-server.md' | sed 's/^/ - /'
printf '\nSearch for infrastructure permission and ROSA-related references:\n'
rg -n --hidden --glob '!.git' -e 'infrastructure\.config\.openshift\.io' -e 'infrastructures/cluster' -e 'OLS_ROSA_PRODUCT' -e 'customization\.brand' -e 'controlPlaneTopology' .Repository: openshift/lightspeed-operator
Length of output: 1519
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'RBAC and deployment files mentioning app-server / permissions:\n'
git ls-files | rg '(^|/)(rbac|role|clusterrole|deployment|app-server|serviceaccount|manifest|yaml|yml)' || true
printf '\nSearch for relevant permission rules and app-server references:\n'
rg -n --hidden --glob '!.git' \
-e 'clusterversions' \
-e 'infrastructures' \
-e 'operator\.openshift\.io' \
-e 'console\.operator\.openshift\.io' \
-e 'app-server' \
-e 'OLS_ROSA_PRODUCT' \
-e 'get:\s*$' \
.Repository: openshift/lightspeed-operator
Length of output: 11747
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'appserver RBAC generation:\n'
ast-grep outline internal/controller/appserver/assets.go --view expanded || true
printf '\nRelevant slice of internal/controller/appserver/assets.go:\n'
sed -n '1,220p' internal/controller/appserver/assets.go
printf '\nRendered role manifest slice:\n'
sed -n '1,240p' config/rbac/role.yaml
printf '\nCSV permissions slice:\n'
sed -n '700,860p' bundle/manifests/lightspeed-operator.clusterserviceversion.yamlRepository: openshift/lightspeed-operator
Length of output: 17524
Add get on config.openshift.io/infrastructures to the lookup role. The ROSA variant check reads Infrastructure/cluster, but the current RBAC only grants clusterversions and secret access, so that lookup will be forbidden on ROSA clusters. Mirror the change in the generated manifest/CSV.
🤖 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 @.ai/spec/what/app-server.md around lines 26 - 32, Add the missing RBAC
permission for the ROSA variant lookup: the lookup role currently covers
`clusterversions` and secret access, but `Infrastructure/cluster` requires `get`
on `config.openshift.io/infrastructures`. Update the role used by the app-server
lookup path so the `controlPlaneTopology` read in the ROSA detection flow is
allowed, and mirror the same permission in the generated manifest/CSV. Ensure
the change stays aligned with the `OLS_ROSA_PRODUCT` detection logic that
depends on this lookup.
|
@blublinsky: 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. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: blublinsky 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
1 participant
Summary
brandfield + InfrastructurecontrolPlaneTopologyOLS_ROSA_PRODUCTenv var to the serviceFiles changed
.ai/spec/what/app-server.md— New rule 15b for ROSA detection.ai/spec/how/deployment-generation.md—OLS_ROSA_PRODUCTenv var in deployment flow.ai/spec/how/config-generation.md— ROSA product in integration points tableRelated
docs/superpowers/specs/2026-07-06-rosa-aware-answering.md(ols workspace)Test plan
Made with Cursor
Summary by CodeRabbit