[CNV-63822] cont. Test plan out-out of role aggregation#73
Open
albarker-rh wants to merge 8 commits intoRedHatQE:mainfrom
Open
[CNV-63822] cont. Test plan out-out of role aggregation#73albarker-rh wants to merge 8 commits intoRedHatQE:mainfrom
albarker-rh wants to merge 8 commits intoRedHatQE:mainfrom
Conversation
|
Clean rebase detected — no code changes compared to previous head ( |
📝 WalkthroughWalkthroughAdds a new OpenShift Virtualization Software Test Plan (CNV-63822) for Role Aggregation Opt-Out, detailing feature metadata, QE ownership, a structured review checklist, prioritized P0/P1 test goals (RBAC enforcement, config persistence, upgrade behavior), out-of-scope items, environment requirements, and traceability to test scenarios. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
albarker-rh
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md`:
- Line 55: Replace the placeholder "[TBD — meeting takeaways must be filled in
after kickoff with Dev/Arch]" in the table cell under "Developer Handoff/QE
Kickoff" with concrete meeting takeaways from the developer handoff (e.g.,
confirmed RBAC enforcement path, reviewed cluster config field behavior,
HTPasswd test setup requirements, date and attendees); update the text in the
CNV-63822-role-aggregation-opt-out.md row so it contains specific, dated meeting
notes rather than the template instruction.
- Around line 48-49: Replace the placeholder "[Name/Date — TBD]" in the STP
Section I.2 Known Limitations paragraph with an actual sign-off in the required
format (e.g., "Ramon Lobillo / 2026-04-10"); locate the placeholder string in
the document CNV-63822-role-aggregation-opt-out.md (Section I.2 Known
Limitations) and update it to "None — reviewed and confirmed with [Name/Date]"
using the real reviewer name and date so the Known Limitations entry contains a
proper sign-off.
- Around line 82-87: Replace every "[Name/Date — TBD]" placeholder in the "PM/
Lead Agreement" column of the out-of-scope table entries (rows for "Testing
OpenShift RBAC infrastructure", "Testing all individual permission rules within
virtualization roles", "External IdP compatibility (LDAP, Active Directory)",
and "Multi-tenant cluster scale testing (100+ users)") with the actual PM/Lead
name and date in the required format (e.g., "Ronen Sde-Or / 2026-04-10" or
"Ramon Lobillo / 2026-04-08"), ensuring each cell uses the exact format "-
**Item** / *Rationale:* / *PM/Lead Agreement:*" as per STP Section II.1.
- Line 96: Replace the placeholder line "*Sign-off:* [Name/Date — TBD]" in the
"Test Limitations" section with an actual reviewer sign-off in the required
format (e.g., "*Sign-off:* Ramon Lobillo / 2026-04-10"); update the text exactly
where the placeholder appears so the STP Section II.1 requirement is satisfied
and the document shows a real name and date instead of "TBD".
- Around line 150-158: Replace the seven "[Name/Date — TBD]" placeholders in the
Sign-off column of the risks table with actual reviewer names and ISO dates for
each risk row: Timeline/Schedule, Test Coverage, Test Environment, Untestable
Aspects, Resource Constraints, Dependencies, and Other; update the Sign-off
cells so they contain specific signatory names and dates (e.g., "Ramon Lobillo /
2026-04-10") and ensure each mitigation entry remains paired with a concrete
sign-off in the corresponding Sign-off column.
- Line 58: Update the API Extensions entry that currently reads "New
cluster-level configuration field to control role aggregation behavior" to a
user-facing description such as "Cluster configuration option to enable/disable
automatic virtualization role aggregation" (or similar), ensuring it refers to
the exposed user API (e.g., HCO CR) rather than internal implementation; keep
the test requirement about validating config changes and downstream RBAC effects
but remove or avoid internal phrasing like "field" or implementation details.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 703bca08-5070-405f-af0a-79d6dff2c557
📒 Files selected for processing (1)
stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md
Comment on lines
+48
to
+49
| None — reviewed and confirmed with [Name/Date — TBD] that no feature limitations apply for | ||
| this release. |
There was a problem hiding this comment.
Replace placeholder with actual sign-off.
Line 48-49 contains "[Name/Date — TBD]" placeholder. As per coding guidelines, Known Limitations must have actual sign-offs, not placeholders.
CRITICAL SEVERITY — this blocks approval. Replace with actual reviewer name and date (e.g., "Ramon Lobillo / 2026-04-10").
As per coding guidelines: "STP Section I.2 Known Limitations: Each limitation must have a sign-off in format 'Sign-off: [Name/Date]' — if none exist, must state 'None — reviewed and confirmed with [Name/Date]'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md` around lines 48 - 49,
Replace the placeholder "[Name/Date — TBD]" in the STP Section I.2 Known
Limitations paragraph with an actual sign-off in the required format (e.g.,
"Ramon Lobillo / 2026-04-10"); locate the placeholder string in the document
CNV-63822-role-aggregation-opt-out.md (Section I.2 Known Limitations) and update
it to "None — reviewed and confirmed with [Name/Date]" using the real reviewer
name and date so the Known Limitations entry contains a proper sign-off.
There was a problem hiding this comment.
@albarker-rh, thanks for acknowledging! Just a reminder — this is CRITICAL severity and will block approval. Please replace the [Name/Date — TBD] placeholder with an actual reviewer name and date before merging. The format should be:
None — reviewed and confirmed with Ramon Lobillo / 2026-04-XX
🧠 Learnings used
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 65
File: stps/sig-virt/heterogeneous-rhcos9-rhcos10/network.md:7-7
Timestamp: 2026-04-13T10:17:47.455Z
Learning: In the openshift-virtualization-tests-design-docs repository (stps/sig-virt/heterogeneous-rhcos9-rhcos10/network.md, PR `#65`), when a feature has neither a VEP nor an HLD, it is acceptable to reference the Jira epic (e.g., CNV-77027) in the Enhancement(s) field, with an explicit note that no VEP or HLD exists. This serves as the closest available design artifact for traceability.
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 15
File: stps/sig-network/ipv6-single-stack-stp.md:169-175
Timestamp: 2026-01-26T12:49:09.313Z
Learning: In documentation files under stps/, when documenting test scenarios in the 'Test Scenarios & Traceability' table, it's acceptable to reuse a single epic ID (e.g., CNV-28924) for all requirement rows that fall under that epic instead of creating unique sub-requirement IDs for each scenario. This applies only to this repository's documentation guidelines; confirm with project governance if this affects traceability or tooling used for test execution mapping.
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 39
File: stps/sig-network/hotpluggable-nad-ref-stp.md:66-66
Timestamp: 2026-03-11T17:53:51.381Z
Learning: In the openshift-virtualization-tests-design-docs repository (PR `#39`, hotpluggable-nad-ref-stp.md), the agreed outcome from the Anatw review discussion about "Verify behavior when LiveUpdateNADRef feature-gate is disabled" was to lower the priority of the test goal (from P1 to P2/P3), not to remove the goal entirely. Anatw stated: if the scenario is included, it should be lower priority (P3). The author lowered it to P2, which satisfies the agreement.
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 74
File: stps/sig-network/hotpluggable-nad-ref.md:100-102
Timestamp: 2026-04-14T11:30:34.673Z
Learning: In the openshift-virtualization-tests-design-docs repository (stps/sig-network/hotpluggable-nad-ref.md, PR `#74`), the "Topology Considerations" subsection of Section I.3 does not need to repeat cluster requirements (e.g., multi-node required, SNO excluded) if those constraints are already explicitly documented in the "Known Limitations" section (I.2) and the "Test Environment" section (II.3). Cross-referencing across sections is acceptable; duplication is not required.
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 15
File: stps/sig-network/ipv6-single-stack-stp.md:175-179
Timestamp: 2026-01-29T20:07:55.187Z
Learning: In the openshift-virtualization-tests-design-docs repository, when documenting test scenarios in the "Test Scenarios & Traceability" table where multiple test scenarios fall under the same epic, it's acceptable and preferred to leave the Requirement ID cells empty for subsequent rows after the first row which contains the epic ID (e.g., CNV-28924 (epic)). Repeating the epic ID in each row is considered redundant.
Learnt from: Anatw
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 37
File: stps/sig-network/stuntime_measurement.md:123-124
Timestamp: 2026-02-24T16:42:14.161Z
Learning: In the openshift-virtualization-tests-design-docs repository, when documenting "OCP & OpenShift Virtualization Version(s)" in STP test environment tables, features may specify "v4.X onward" while also noting "Can be backported to all versions." This indicates the feature debuts in version X and continues forward, but tests and functionality can also be backported to earlier releases.
<!-- <review_comment_addressed>
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 7
File: stps/sig-network/ip-request-stp.md:0-0
Timestamp: 2026-01-19T08:27:03.411Z
Learning: In the openshift-virtualization-tests-design-docs repository, not all features have OpenShift enhancement PRs. When no enhancement PR exists, it's acceptable to reference only the HLD (High-Level Design) document in the Enhancement(s) field of the STP metadata table.
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 65
File: stps/sig-virt/heterogeneous-rhcos9-rhcos10/network.md:187-187
Timestamp: 2026-04-13T10:27:58.826Z
Learning: When reviewing STP markdown docs in this repo (e.g., under `stps/`), do not flag the template default in Section II.3.1 “Testing Tools & Frameworks” where `- **Test Framework:** Standard` is used. This is intentional and indicates no new or non-standard tools are required. Only suggest changes if the feature introduces a new/modified test framework, a custom test harness, or significant test infrastructure changes beyond the standard baseline.
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section I.2 Known Limitations: Each limitation must have a sign-off in format '*Sign-off:* [Name/Date]' — if none exist, must state 'None — reviewed and confirmed with [Name/Date]'
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section II.1 Test Limitations: Each limitation must have sign-off in format '*Sign-off:* [Name/Date]' and items must be constraints imposed on QE (not decisions QE made)
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section IV Sign-off and Approval: Reviewers and Approvers must be listed with names and GitHub handles (QE Lead, PM, Dev Lead at minimum) with no placeholder text remaining
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section I.2 Known Limitations: Limitations are constraints (not scope decisions) and open bugs affecting the feature must be listed with Jira links
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T10:45:10.935Z
Learning: Refer to AGENTS.md for AI review standards for STP pull requests
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section I.3 Technology and Design Review: All items must use [x] checkboxes — Developer Handoff must describe actual meeting takeaways, not just 'meeting conducted'
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: When reviewing STPs, distinguish review severity levels: CRITICAL (missing traceability, empty sign-offs, contradictions — blocks approval), HIGH (template non-compliance, incomplete sections — must fix), MEDIUM (formatting issues, minor improvements — should fix), LOW (style suggestions — nice to have)
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section II.5 Risks: Each risk must have Risk description, Mitigation strategy, and Sign-off — mitigations must be specific and actionable (not vague)
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : Every claim in STPs needs evidence — sign-offs, Jira links, dates. No empty placeholders in approved STPs
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section II.1 Out of Scope: Each item must have Rationale and PM/Lead Agreement with name and date in format '- **Item** / *Rationale:* / *PM/Lead Agreement:*'
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 52
File: stps/stp-template/stp.md:11-11
Timestamp: 2026-03-12T12:39:22.684Z
Learning: In the openshift-virtualization-tests-design-docs repository, already-merged STP documents should NOT be checked for formatting consistency with the current template under review. The template may evolve independently, and existing merged STPs are not expected to retroactively conform to new template conventions (e.g., field renaming such as "Jira Tracking" → "Epic in Jira").
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 65
File: stps/sig-virt/heterogeneous-rhcos9-rhcos10/network.md:174-174
Timestamp: 2026-04-13T10:20:13.766Z
Learning: In the openshift-virtualization-tests-design-docs repository, when a feature's tests are genuinely storage-agnostic (e.g., network-focused STPs under stps/sig-virt/), it is acceptable to write "Any available StorageClass (network-focused tests; storage type does not affect test outcomes)" in the Storage field of Section II.3 Test Environment rather than leaving it as the bare word "Agnostic". The Platform field must also always be populated explicitly (e.g., "Bare metal").
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section II.3 Test Environment: All fields must be filled or marked N/A (no empty fields) — OCP and OpenShift Virtualization versions must be explicit
Comment on lines
+82
to
+87
| | Out-of-Scope Item | Rationale | PM/ Lead Agreement | | ||
| |:----------------------------------------------------------|:------------------------------------------------------------------------------|:-------------------| | ||
| | Testing OpenShift RBAC infrastructure itself | Core RBAC evaluation is the responsibility of the OCP platform team; no duplication of their test effort | [Name/Date — TBD] | | ||
| | Testing all individual permission rules within virtualization roles | Individual role rules are not affected by this feature; this feature controls whether roles are aggregated, not the content of the roles themselves | [Name/Date — TBD] | | ||
| | External IdP compatibility (LDAP, Active Directory) | RBAC logic is IdP-agnostic; HTPasswd testing validates the core permission logic | [Name/Date — TBD] | | ||
| | Multi-tenant cluster scale testing (100+ users) | RBAC evaluation overhead is negligible; functional correctness at smaller scale is sufficient | [Name/Date — TBD] | |
There was a problem hiding this comment.
Replace all PM/Lead Agreement placeholders with actual names and dates.
Lines 84, 85, 86, 87 all contain "[Name/Date — TBD]" placeholders in the "PM/ Lead Agreement" column. As per coding guidelines, each out-of-scope item must have actual PM/Lead Agreement with name and date.
CRITICAL SEVERITY — this blocks approval. Replace with actual agreement details (e.g., "Ronen Sde-Or / 2026-04-10" or "Ramon Lobillo / 2026-04-08").
As per coding guidelines: "STP Section II.1 Out of Scope: Each item must have Rationale and PM/Lead Agreement with name and date in format '- Item / Rationale: / PM/Lead Agreement:'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md` around lines 82 - 87,
Replace every "[Name/Date — TBD]" placeholder in the "PM/ Lead Agreement" column
of the out-of-scope table entries (rows for "Testing OpenShift RBAC
infrastructure", "Testing all individual permission rules within virtualization
roles", "External IdP compatibility (LDAP, Active Directory)", and "Multi-tenant
cluster scale testing (100+ users)") with the actual PM/Lead name and date in
the required format (e.g., "Ronen Sde-Or / 2026-04-10" or "Ramon Lobillo /
2026-04-08"), ensuring each cell uses the exact format "- **Item** /
*Rationale:* / *PM/Lead Agreement:*" as per STP Section II.1.
| **Test Limitations** | ||
|
|
||
| None — reviewed and confirmed that no test limitations apply for this release. | ||
| *Sign-off:* [Name/Date — TBD] |
There was a problem hiding this comment.
Replace Test Limitations sign-off placeholder with actual name and date.
Line 96 contains "Sign-off: [Name/Date — TBD]" placeholder. As per coding guidelines, test limitations must have actual sign-offs, not placeholders.
CRITICAL SEVERITY — this blocks approval. Replace with actual reviewer name and date (e.g., "Sign-off: Ramon Lobillo / 2026-04-10").
As per coding guidelines: "STP Section II.1 Test Limitations: Each limitation must have sign-off in format 'Sign-off: [Name/Date]'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md` at line 96, Replace the
placeholder line "*Sign-off:* [Name/Date — TBD]" in the "Test Limitations"
section with an actual reviewer sign-off in the required format (e.g.,
"*Sign-off:* Ramon Lobillo / 2026-04-10"); update the text exactly where the
placeholder appears so the STP Section II.1 requirement is satisfied and the
document shows a real name and date instead of "TBD".
There was a problem hiding this comment.
@albarker-rh, understood — the comment will remain open until the actual sign-off is in place. Once you have the reviewer name and date, simply replace [Name/Date — TBD] with the real value (e.g., *Sign-off:* Ramon Lobillo / 2026-04-XX) and this can be resolved. No rush on my end; just a blocker for final approval.
🧠 Learnings used
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 39
File: stps/sig-network/hotpluggable-nad-ref-stp.md:66-66
Timestamp: 2026-03-11T17:53:51.381Z
Learning: In the openshift-virtualization-tests-design-docs repository (PR `#39`, hotpluggable-nad-ref-stp.md), the agreed outcome from the Anatw review discussion about "Verify behavior when LiveUpdateNADRef feature-gate is disabled" was to lower the priority of the test goal (from P1 to P2/P3), not to remove the goal entirely. Anatw stated: if the scenario is included, it should be lower priority (P3). The author lowered it to P2, which satisfies the agreement.
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 65
File: stps/sig-virt/heterogeneous-rhcos9-rhcos10/network.md:187-187
Timestamp: 2026-04-13T10:27:58.826Z
Learning: When reviewing STP markdown docs in this repo (e.g., under `stps/`), do not flag the template default in Section II.3.1 “Testing Tools & Frameworks” where `- **Test Framework:** Standard` is used. This is intentional and indicates no new or non-standard tools are required. Only suggest changes if the feature introduces a new/modified test framework, a custom test harness, or significant test infrastructure changes beyond the standard baseline.
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 7
File: stps/sig-network/ip-request-stp.md:0-0
Timestamp: 2026-01-19T08:27:03.411Z
Learning: In the openshift-virtualization-tests-design-docs repository, not all features have OpenShift enhancement PRs. When no enhancement PR exists, it's acceptable to reference only the HLD (High-Level Design) document in the Enhancement(s) field of the STP metadata table.
Learnt from: Anatw
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 37
File: stps/sig-network/stuntime_measurement.md:123-124
Timestamp: 2026-02-24T16:42:14.161Z
Learning: In the openshift-virtualization-tests-design-docs repository, when documenting "OCP & OpenShift Virtualization Version(s)" in STP test environment tables, features may specify "v4.X onward" while also noting "Can be backported to all versions." This indicates the feature debuts in version X and continues forward, but tests and functionality can also be backported to earlier releases.
<!-- <review_comment_addressed>
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 52
File: stps/stp-template/stp.md:11-11
Timestamp: 2026-03-12T12:39:22.684Z
Learning: In the openshift-virtualization-tests-design-docs repository, already-merged STP documents should NOT be checked for formatting consistency with the current template under review. The template may evolve independently, and existing merged STPs are not expected to retroactively conform to new template conventions (e.g., field renaming such as "Jira Tracking" → "Epic in Jira").
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section II.3 Test Environment: All fields must be filled or marked N/A (no empty fields) — OCP and OpenShift Virtualization versions must be explicit
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 65
File: stps/sig-virt/heterogeneous-rhcos9-rhcos10/network.md:174-174
Timestamp: 2026-04-13T10:20:13.766Z
Learning: In the openshift-virtualization-tests-design-docs repository, when a feature's tests are genuinely storage-agnostic (e.g., network-focused STPs under stps/sig-virt/), it is acceptable to write "Any available StorageClass (network-focused tests; storage type does not affect test outcomes)" in the Storage field of Section II.3 Test Environment rather than leaving it as the bare word "Agnostic". The Platform field must also always be populated explicitly (e.g., "Bare metal").
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 74
File: stps/sig-network/hotpluggable-nad-ref.md:100-102
Timestamp: 2026-04-14T11:30:34.673Z
Learning: In the openshift-virtualization-tests-design-docs repository (stps/sig-network/hotpluggable-nad-ref.md, PR `#74`), the "Topology Considerations" subsection of Section I.3 does not need to repeat cluster requirements (e.g., multi-node required, SNO excluded) if those constraints are already explicitly documented in the "Known Limitations" section (I.2) and the "Test Environment" section (II.3). Cross-referencing across sections is acceptable; duplication is not required.
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 15
File: stps/sig-network/ipv6-single-stack-stp.md:169-175
Timestamp: 2026-01-26T12:49:09.313Z
Learning: In documentation files under stps/, when documenting test scenarios in the 'Test Scenarios & Traceability' table, it's acceptable to reuse a single epic ID (e.g., CNV-28924) for all requirement rows that fall under that epic instead of creating unique sub-requirement IDs for each scenario. This applies only to this repository's documentation guidelines; confirm with project governance if this affects traceability or tooling used for test execution mapping.
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 65
File: stps/sig-virt/heterogeneous-rhcos9-rhcos10/network.md:7-7
Timestamp: 2026-04-13T10:17:47.455Z
Learning: In the openshift-virtualization-tests-design-docs repository (stps/sig-virt/heterogeneous-rhcos9-rhcos10/network.md, PR `#65`), when a feature has neither a VEP nor an HLD, it is acceptable to reference the Jira epic (e.g., CNV-77027) in the Enhancement(s) field, with an explicit note that no VEP or HLD exists. This serves as the closest available design artifact for traceability.
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section I.2 Known Limitations: Each limitation must have a sign-off in format '*Sign-off:* [Name/Date]' — if none exist, must state 'None — reviewed and confirmed with [Name/Date]'
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section II.1 Test Limitations: Each limitation must have sign-off in format '*Sign-off:* [Name/Date]' and items must be constraints imposed on QE (not decisions QE made)
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section IV Sign-off and Approval: Reviewers and Approvers must be listed with names and GitHub handles (QE Lead, PM, Dev Lead at minimum) with no placeholder text remaining
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T10:45:10.935Z
Learning: Refer to AGENTS.md for AI review standards for STP pull requests
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : Multi-SIG STPs must list all participating SIGs with confirmed test scope, prevent child STP duplication of parent STP content, explicitly document each SIG's regression responsibility, and assign clear ownership for cross-SIG test scenarios
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section I.2 Known Limitations: Limitations are constraints (not scope decisions) and open bugs affecting the feature must be listed with Jira links
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : Every claim in STPs needs evidence — sign-offs, Jira links, dates. No empty placeholders in approved STPs
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: When reviewing STPs, distinguish review severity levels: CRITICAL (missing traceability, empty sign-offs, contradictions — blocks approval), HIGH (template non-compliance, incomplete sections — must fix), MEDIUM (formatting issues, minor improvements — should fix), LOW (style suggestions — nice to have)
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section II.5 Risks: Each risk must have Risk description, Mitigation strategy, and Sign-off — mitigations must be specific and actionable (not vague)
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 15
File: stps/sig-network/ipv6-single-stack-stp.md:175-179
Timestamp: 2026-01-29T20:07:55.187Z
Learning: In the openshift-virtualization-tests-design-docs repository, when documenting test scenarios in the "Test Scenarios & Traceability" table where multiple test scenarios fall under the same epic, it's acceptable and preferred to leave the Requirement ID cells empty for subsequent rows after the first row which contains the epic ID (e.g., CNV-28924 (epic)). Repeating the epic ID in each row is considered redundant.
Comment on lines
+150
to
+158
| | Risk Category | Specific Risk for This Feature | Mitigation Strategy | Sign-off | | ||
| |:---------------------|:-------------------------------------------------------------------------------------|:----------------------------------------------------------------------------------------|:-------------------| | ||
| | Timeline/Schedule | N/A — feature implementation is complete (upstream and downstream) | N/A | [Name/Date — TBD] | | ||
| | Test Coverage | Cannot exhaustively test all RBAC role combinations and permission permutations | Focus on the 3 critical role levels (admin, edit, view) covering acceptance criteria; individual permission rules within roles are unaffected by this feature | [Name/Date — TBD] | | ||
| | Test Environment | N/A — standard OCP cluster with HTPasswd IdP is sufficient; no special hardware required | N/A | [Name/Date — TBD] | | ||
| | Untestable Aspects | Cannot test with production identity providers (LDAP, Active Directory, OAuth) in the lab | RBAC logic is IdP-agnostic; HTPasswd validation covers the enforcement path regardless of IdP | [Name/Date — TBD] | | ||
| | Resource Constraints | N/A — no staffing or capacity constraints; feature testing scope is manageable with assigned QE resources | N/A | [Name/Date — TBD] | | ||
| | Dependencies | UI changes ([CNV-80935](https://issues.redhat.com/browse/CNV-80935)) are pending; console configuration interface may not be ready for testing | Track progress with UI team (sig-ui); API-level testing can proceed independently of UI | [Name/Date — TBD] | | ||
| | Other | N/A — no additional risks identified | N/A | [Name/Date — TBD] | |
There was a problem hiding this comment.
Replace all Risk sign-off placeholders with actual names and dates.
Lines 152, 153, 154, 155, 156, 157, 158 all contain "[Name/Date — TBD]" placeholders in the "Sign-off" column. As per coding guidelines, each risk must have actual sign-offs, not placeholders.
CRITICAL SEVERITY — this blocks approval. Replace with actual reviewer names and dates for all 7 risk categories (e.g., "Ramon Lobillo / 2026-04-10").
As per coding guidelines: "STP Section II.5 Risks: Each risk must have Risk description, Mitigation strategy, and Sign-off — mitigations must be specific and actionable (not vague)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md` around lines 150 - 158,
Replace the seven "[Name/Date — TBD]" placeholders in the Sign-off column of the
risks table with actual reviewer names and ISO dates for each risk row:
Timeline/Schedule, Test Coverage, Test Environment, Untestable Aspects, Resource
Constraints, Dependencies, and Other; update the Sign-off cells so they contain
specific signatory names and dates (e.g., "Ramon Lobillo / 2026-04-10") and
ensure each mitigation entry remains paired with a concrete sign-off in the
corresponding Sign-off column.
There was a problem hiding this comment.
@albarker-rh, understood — the placeholder sign-offs across the document are a CRITICAL blocker for approval, so please ensure the following are all resolved before requesting final review:
| Location | Placeholder(s) to replace |
|---|---|
| I.2 Known Limitations | [Name/Date — TBD] |
| I.3 Developer Handoff | [TBD — meeting takeaways must be filled in after kickoff] |
| II.1 Out of Scope (4 rows) | [Name/Date — TBD] |
| II.1 Test Limitations | [Name/Date — TBD] |
| II.5 Risks (7 rows) | [Name/Date — TBD] |
Once all stakeholders have signed off and you've updated the document, this comment can be resolved. No action needed from your side right now — just flagging the full scope so nothing is missed when the sign-offs come in. 👍
🧠 Learnings used
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 39
File: stps/sig-network/hotpluggable-nad-ref-stp.md:66-66
Timestamp: 2026-03-11T17:53:51.381Z
Learning: In the openshift-virtualization-tests-design-docs repository (PR `#39`, hotpluggable-nad-ref-stp.md), the agreed outcome from the Anatw review discussion about "Verify behavior when LiveUpdateNADRef feature-gate is disabled" was to lower the priority of the test goal (from P1 to P2/P3), not to remove the goal entirely. Anatw stated: if the scenario is included, it should be lower priority (P3). The author lowered it to P2, which satisfies the agreement.
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 7
File: stps/sig-network/ip-request-stp.md:0-0
Timestamp: 2026-01-19T08:27:03.411Z
Learning: In the openshift-virtualization-tests-design-docs repository, not all features have OpenShift enhancement PRs. When no enhancement PR exists, it's acceptable to reference only the HLD (High-Level Design) document in the Enhancement(s) field of the STP metadata table.
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 65
File: stps/sig-virt/heterogeneous-rhcos9-rhcos10/network.md:187-187
Timestamp: 2026-04-13T10:27:58.826Z
Learning: When reviewing STP markdown docs in this repo (e.g., under `stps/`), do not flag the template default in Section II.3.1 “Testing Tools & Frameworks” where `- **Test Framework:** Standard` is used. This is intentional and indicates no new or non-standard tools are required. Only suggest changes if the feature introduces a new/modified test framework, a custom test harness, or significant test infrastructure changes beyond the standard baseline.
Learnt from: Anatw
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 37
File: stps/sig-network/stuntime_measurement.md:123-124
Timestamp: 2026-02-24T16:42:14.161Z
Learning: In the openshift-virtualization-tests-design-docs repository, when documenting "OCP & OpenShift Virtualization Version(s)" in STP test environment tables, features may specify "v4.X onward" while also noting "Can be backported to all versions." This indicates the feature debuts in version X and continues forward, but tests and functionality can also be backported to earlier releases.
<!-- <review_comment_addressed>
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 65
File: stps/sig-virt/heterogeneous-rhcos9-rhcos10/network.md:7-7
Timestamp: 2026-04-13T10:17:47.455Z
Learning: In the openshift-virtualization-tests-design-docs repository (stps/sig-virt/heterogeneous-rhcos9-rhcos10/network.md, PR `#65`), when a feature has neither a VEP nor an HLD, it is acceptable to reference the Jira epic (e.g., CNV-77027) in the Enhancement(s) field, with an explicit note that no VEP or HLD exists. This serves as the closest available design artifact for traceability.
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 15
File: stps/sig-network/ipv6-single-stack-stp.md:169-175
Timestamp: 2026-01-26T12:49:09.313Z
Learning: In documentation files under stps/, when documenting test scenarios in the 'Test Scenarios & Traceability' table, it's acceptable to reuse a single epic ID (e.g., CNV-28924) for all requirement rows that fall under that epic instead of creating unique sub-requirement IDs for each scenario. This applies only to this repository's documentation guidelines; confirm with project governance if this affects traceability or tooling used for test execution mapping.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 52
File: stps/stp-template/stp.md:11-11
Timestamp: 2026-03-12T12:39:22.684Z
Learning: In the openshift-virtualization-tests-design-docs repository, already-merged STP documents should NOT be checked for formatting consistency with the current template under review. The template may evolve independently, and existing merged STPs are not expected to retroactively conform to new template conventions (e.g., field renaming such as "Jira Tracking" → "Epic in Jira").
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section II.3 Test Environment: All fields must be filled or marked N/A (no empty fields) — OCP and OpenShift Virtualization versions must be explicit
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 65
File: stps/sig-virt/heterogeneous-rhcos9-rhcos10/network.md:174-174
Timestamp: 2026-04-13T10:20:13.766Z
Learning: In the openshift-virtualization-tests-design-docs repository, when a feature's tests are genuinely storage-agnostic (e.g., network-focused STPs under stps/sig-virt/), it is acceptable to write "Any available StorageClass (network-focused tests; storage type does not affect test outcomes)" in the Storage field of Section II.3 Test Environment rather than leaving it as the bare word "Agnostic". The Platform field must also always be populated explicitly (e.g., "Bare metal").
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 74
File: stps/sig-network/hotpluggable-nad-ref.md:100-102
Timestamp: 2026-04-14T11:30:34.673Z
Learning: In the openshift-virtualization-tests-design-docs repository (stps/sig-network/hotpluggable-nad-ref.md, PR `#74`), the "Topology Considerations" subsection of Section I.3 does not need to repeat cluster requirements (e.g., multi-node required, SNO excluded) if those constraints are already explicitly documented in the "Known Limitations" section (I.2) and the "Test Environment" section (II.3). Cross-referencing across sections is acceptable; duplication is not required.
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section II.5 Risks: Each risk must have Risk description, Mitigation strategy, and Sign-off — mitigations must be specific and actionable (not vague)
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section IV Sign-off and Approval: Reviewers and Approvers must be listed with names and GitHub handles (QE Lead, PM, Dev Lead at minimum) with no placeholder text remaining
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section I.2 Known Limitations: Each limitation must have a sign-off in format '*Sign-off:* [Name/Date]' — if none exist, must state 'None — reviewed and confirmed with [Name/Date]'
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section II.5 Risks: ALL 7 risk categories must be addressed (Timeline/Schedule, Test Coverage, Test Environment, Untestable Aspects, Resource Constraints, Dependencies, Other) even if N/A with justification
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section II.1 Test Limitations: Each limitation must have sign-off in format '*Sign-off:* [Name/Date]' and items must be constraints imposed on QE (not decisions QE made)
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T10:45:10.935Z
Learning: Refer to AGENTS.md for AI review standards for STP pull requests
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: When reviewing STPs, distinguish review severity levels: CRITICAL (missing traceability, empty sign-offs, contradictions — blocks approval), HIGH (template non-compliance, incomplete sections — must fix), MEDIUM (formatting issues, minor improvements — should fix), LOW (style suggestions — nice to have)
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section II.1 Out of Scope: Each item must have Rationale and PM/Lead Agreement with name and date in format '- **Item** / *Rationale:* / *PM/Lead Agreement:*'
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : Every claim in STPs needs evidence — sign-offs, Jira links, dates. No empty placeholders in approved STPs
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-12T10:45:57.620Z
Learning: Applies to **/stps/**/*.md : STP Section I.3 Technology and Design Review: All items must use [x] checkboxes — Developer Handoff must describe actual meeting takeaways, not just 'meeting conducted'
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 15
File: stps/sig-network/ipv6-single-stack-stp.md:175-179
Timestamp: 2026-01-29T20:07:55.187Z
Learning: In the openshift-virtualization-tests-design-docs repository, when documenting test scenarios in the "Test Scenarios & Traceability" table where multiple test scenarios fall under the same epic, it's acceptable and preferred to leave the Requirement ID cells empty for subsequent rows after the first row which contains the epic ID (e.g., CNV-28924 (epic)). Repeating the epic ID in each row is considered redundant.
openshift-virtualization-qe-bot-4
requested review from
ema-aka-young and
jpeimer
openshift-virtualization-qe-bot-2
added
size/L
and removed
changes-requested-coderabbitai[bot]
size/XXL
needs-rebase
labels
…es comments Signed-off-by: Alex <albarker@redhat.com>
Author
|
/wip cancel |
openshift-virtualization-qe-bot-2
changed the title
Signed-off-by: Alex <albarker@redhat.com>
openshift-virtualization-qe-bot
added
size/XL
and removed
commented-coderabbitai[bot]
commented-albarker-rh
size/L
labels
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md (3)
358-360:⚠️ Potential issue | 🔴 CriticalCRITICAL: Section IV approver set is incomplete (Dev Lead missing) and PM handle is not provided.
Approvers currently list QE Manager and Product Manager only; Dev Lead is required at minimum. Also, Product Manager should include a GitHub handle for consistency with Section IV requirements.
As per coding guidelines: "STP Section IV Sign-off and Approval: Reviewers and Approvers must be listed with names and GitHub handles (QE Lead, PM, Dev Lead at minimum) with no placeholder text remaining".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md` around lines 358 - 360, The Approvers list under "Approvers" (Section IV) is incomplete—add the required Dev Lead entry and include the Product Manager's GitHub handle instead of plain name; update the existing list that currently shows "QE Manager / `@kmajcher-rh` `@fabiand`" and "Product Manager / Ronen Sde-Or" to include a "Dev Lead / <Name> / `@github-handle`" line and change the PM line to "Product Manager / Ronen Sde-Or / `@pm-github-handle`"; ensure no placeholder text remains and that each approver (QE Lead, PM, Dev Lead) has both a name and GitHub handle.
83-84:⚠️ Potential issue | 🔴 CriticalCRITICAL: unresolved sign-off placeholders still block approval.
[Name/Date — TBD]is still present in Known Limitations (Line 83), Out of Scope PM/Lead Agreements (Line 138, Line 142, Line 146, Line 150), Test Limitations (Line 155), and Risks (Line 253, Line 260, Line 267, Line 274, Line 281, Line 288, Line 294). These must be replaced with actual names/dates before merge.As per coding guidelines: "STP Section I.2 Known Limitations: Each limitation must have a sign-off in format 'Sign-off: [Name/Date]' — if none exist, must state 'None — reviewed and confirmed with [Name/Date]'", "STP Section II.1 Out of Scope: Each item must have Rationale and PM/Lead Agreement with name and date", "STP Section II.1 Test Limitations: Each limitation must have sign-off", and "STP Section II.5 Risks: Each risk must have ... Sign-off".
Also applies to: 138-150, 155-155, 253-294
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md` around lines 83 - 84, Replace every remaining placeholder "[Name/Date — TBD]" in the document sections Known Limitations, Out of Scope PM/Lead Agreements, Test Limitations, and Risks with actual sign-off entries in the required format (e.g., "*Sign-off:* [Full Name/YYYY-MM-DD]") or, if no limitation exists, replace the placeholder with the exact required confirmation string ("None — reviewed and confirmed with [Full Name/YYYY-MM-DD]"). Ensure you update each occurrence referenced in the review (Known Limitations; Out of Scope PM/Lead Agreements; Test Limitations; Risks) so all sign-offs conform to the STP Section formats.
98-99:⚠️ Potential issue | 🟠 MajorHIGH: internal implementation terminology appears in user-facing STP sections.
Phrases like “configuration field” and “KubeVirt CR level” are implementation-oriented. Rephrase to user-visible behavior/capability only.
As per coding guidelines: "STPs must describe what users experience from a user perspective only — no API field names, CRD names, internal component references, or implementation mechanisms".
Also applies to: 106-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md` around lines 98 - 99, Rewrite the STP text to remove implementation terms like "configuration field" and "KubeVirt CR level" and instead describe only the user-facing capability: state that there is a new cluster-wide option to control role aggregation, what the default behavior is (aggregation enabled) and how users can change behavior (opt-out is possible and must be done manually), and update the same wording at the other occurrence of "configuration field" / "KubeVirt CR level" on lines 106-107 to the same user-centric phrasing so the document uses no API/CRD/internal terminology.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md`:
- Around line 46-49: Rewrite the two customer-use bullets into explicit user
stories in the required "As a [role], I want to [action]" format: replace
"Multi-tenant clusters — different namespaces are used to allow different
workloads and prevent unallowed usage of workload that the tenant is not
eligible to use" with a user story like "As a cluster administrator, I want
namespaces per tenant so that workloads are isolated and tenants cannot use
workloads they are not eligible for", and replace "Resources usage control —
cluster admin wants to get a request to allow a specific user to create VMs and
Storage" with a user story like "As a cluster administrator, I want to receive
and review requests to grant a specific user permission to create VMs and
storage so I can control resource usage"; ensure each bullet begins with "As a
..." and includes role, action, and intent.
---
Duplicate comments:
In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md`:
- Around line 358-360: The Approvers list under "Approvers" (Section IV) is
incomplete—add the required Dev Lead entry and include the Product Manager's
GitHub handle instead of plain name; update the existing list that currently
shows "QE Manager / `@kmajcher-rh` `@fabiand`" and "Product Manager / Ronen Sde-Or"
to include a "Dev Lead / <Name> / `@github-handle`" line and change the PM line to
"Product Manager / Ronen Sde-Or / `@pm-github-handle`"; ensure no placeholder text
remains and that each approver (QE Lead, PM, Dev Lead) has both a name and
GitHub handle.
- Around line 83-84: Replace every remaining placeholder "[Name/Date — TBD]" in
the document sections Known Limitations, Out of Scope PM/Lead Agreements, Test
Limitations, and Risks with actual sign-off entries in the required format
(e.g., "*Sign-off:* [Full Name/YYYY-MM-DD]") or, if no limitation exists,
replace the placeholder with the exact required confirmation string ("None —
reviewed and confirmed with [Full Name/YYYY-MM-DD]"). Ensure you update each
occurrence referenced in the review (Known Limitations; Out of Scope PM/Lead
Agreements; Test Limitations; Risks) so all sign-offs conform to the STP Section
formats.
- Around line 98-99: Rewrite the STP text to remove implementation terms like
"configuration field" and "KubeVirt CR level" and instead describe only the
user-facing capability: state that there is a new cluster-wide option to control
role aggregation, what the default behavior is (aggregation enabled) and how
users can change behavior (opt-out is possible and must be done manually), and
update the same wording at the other occurrence of "configuration field" /
"KubeVirt CR level" on lines 106-107 to the same user-centric phrasing so the
document uses no API/CRD/internal terminology.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: d071cad0-0337-417b-8485-ce7bc3881c00
📒 Files selected for processing (1)
stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md
Signed-off-by: Alex <albarker@redhat.com>
openshift-virtualization-qe-bot-4
removed
the
changes-requested-coderabbitai[bot]
label
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md (5)
155-156:⚠️ Potential issue | 🔴 CriticalCRITICAL: Test Limitations sign-off is still a placeholder.
Line 156 still has
*Sign-off:* [Name/Date — TBD]. Replace with an actual reviewer name and date.As per coding guidelines: "STP Section II.1 Test Limitations: Each limitation must have sign-off in format 'Sign-off: [Name/Date]'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md` around lines 155 - 156, Replace the placeholder sign-off in the STP Section II.1 "Test Limitations" where the line reads "*Sign-off:* [Name/Date — TBD]" with a real reviewer name and date (e.g., "*Sign-off:* [Reviewer Name/YYYY-MM-DD]"); locate the string in CNV-63822-role-aggregation-opt-out.md (the Test Limitations block) and update the bracketed placeholder to the actual signer and ISO-style date so the document meets the required sign-off format.
84-85:⚠️ Potential issue | 🔴 CriticalCRITICAL: Known Limitations still contains placeholder sign-off.
Line 84 still has
[Name/Date — TBD]. This remains a release-blocking placeholder and must be replaced with an actual signer and date.As per coding guidelines: "STP Section I.2 Known Limitations: Each limitation must have a sign-off in format 'Sign-off: [Name/Date]' — if none exist, must state 'None — reviewed and confirmed with [Name/Date]'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md` around lines 84 - 85, Replace the placeholder "[Name/Date — TBD]" in the "Known Limitations" section (the line currently reading "None — reviewed and confirmed with [Name/Date — TBD]") with an actual signer and date using the required sign-off format; update that line to either "None — reviewed and confirmed with [Signer Name/YYYY-MM-DD]" or change to a proper "*Sign-off:* [Signer Name/YYYY-MM-DD]" entry for the limitation so it conforms to the STP Section I.2 sign-off requirement.
359-361:⚠️ Potential issue | 🔴 CriticalCRITICAL: Approvers list is missing required minimum coverage and handle format.
Approvers still do not explicitly include Dev Lead, and PM is listed without a GitHub handle. Section IV must include QE Lead, PM, and Dev Lead with names and handles.
As per coding guidelines: "STP Section IV Sign-off and Approval: Reviewers and Approvers must be listed with names and GitHub handles (QE Lead, PM, Dev Lead at minimum) with no placeholder text remaining".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md` around lines 359 - 361, Update Section IV "Approvers" to include the required minimum roles (QE Lead, Product Manager, and Dev Lead) with full names and GitHub handles; replace the current list that only shows "QE Manager" and a PM name without handle. Specifically edit the Approvers bullet list (the lines containing "Approvers:", "QE Manager / `@kmajcher-rh` `@fabiand`", and "Product Manager / Ronen Sde-Or") to: (1) explicitly include a QE Lead entry with name and `@handle`, (2) include the PM entry with the PM's GitHub handle (e.g., replace "Ronen Sde-Or" with "Ronen Sde-Or / `@pm-handle`"), and (3) add a Dev Lead entry with name and `@handle`; ensure there are no placeholder texts and all three required roles are present.
249-295:⚠️ Potential issue | 🔴 CriticalCRITICAL: Risk section sign-offs remain placeholders across all categories.
Lines 254, 261, 268, 275, 282, 289, and 295 still contain
[Name/Date — TBD]. These sign-offs are required for all risk categories and block approval until replaced.As per coding guidelines: "STP Section II.5 Risks: Each risk must have Risk description, Mitigation strategy, and Sign-off — mitigations must be specific and actionable (not vague)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md` around lines 249 - 295, The Risk sections across the document still have placeholder sign-offs and non-actionable mitigations; replace every "[Name/Date — TBD]" (present under the Risk/ Mitigation/Sign-off entries for Timeline/Schedule, Test Coverage, Test Environment, Untestable Aspects, Resource Constraints, Dependencies, Other) with actual approver names and dates, and update each Mitigation entry to a specific, actionable step (e.g., who will perform it, timeline, and success criteria) rather than "N/A" or vague text so every risk entry meets STP Section II.5 requirements.
137-151:⚠️ Potential issue | 🔴 CriticalCRITICAL: Out-of-scope PM/Lead agreements are still unresolved placeholders.
Lines 139, 143, 147, and 151 still use
[Name/Date — TBD]. Each out-of-scope item needs real PM/Lead agreement details before approval.As per coding guidelines: "STP Section II.1 Out of Scope: Each item must have Rationale and PM/Lead Agreement with name and date in format '- Item / Rationale: / PM/Lead Agreement:'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md` around lines 137 - 151, Replace the placeholder PM/Lead Agreement entries for the four out-of-scope items — "Testing OpenShift RBAC infrastructure itself", "Testing all individual permission rules within virtualization roles", "External IdP compatibility (LDAP, Active Directory)", and "Multi-tenant cluster scale testing (100+ users)" — with actual approver name and date values in the required format; update each "- **Item**" bullet so it follows the specified pattern ('- **Item** / *Rationale:* / *PM/Lead Agreement:*') and substitute each "[Name/Date — TBD]" with a concrete "Name — YYYY-MM-DD" (or equivalent full date) to satisfy the STP Section II.1 requirement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md`:
- Around line 120-130: Add a dedicated "Regression" goal under Section II.1
Testing Goals that explicitly distinguishes regression from new functional tests
and lists which SIG suites will run on the feature cluster (e.g., "Regression:
run SIG-iuo regression suite" and any other relevant SIG suites); update the
Testing Goals list near the existing P0/P1 functional bullets so it clearly
states "Functional: ..." and "Regression: [SIG suites]" to satisfy the
requirement that regression-suite execution be identified.
---
Duplicate comments:
In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md`:
- Around line 155-156: Replace the placeholder sign-off in the STP Section II.1
"Test Limitations" where the line reads "*Sign-off:* [Name/Date — TBD]" with a
real reviewer name and date (e.g., "*Sign-off:* [Reviewer Name/YYYY-MM-DD]");
locate the string in CNV-63822-role-aggregation-opt-out.md (the Test Limitations
block) and update the bracketed placeholder to the actual signer and ISO-style
date so the document meets the required sign-off format.
- Around line 84-85: Replace the placeholder "[Name/Date — TBD]" in the "Known
Limitations" section (the line currently reading "None — reviewed and confirmed
with [Name/Date — TBD]") with an actual signer and date using the required
sign-off format; update that line to either "None — reviewed and confirmed with
[Signer Name/YYYY-MM-DD]" or change to a proper "*Sign-off:* [Signer
Name/YYYY-MM-DD]" entry for the limitation so it conforms to the STP Section I.2
sign-off requirement.
- Around line 359-361: Update Section IV "Approvers" to include the required
minimum roles (QE Lead, Product Manager, and Dev Lead) with full names and
GitHub handles; replace the current list that only shows "QE Manager" and a PM
name without handle. Specifically edit the Approvers bullet list (the lines
containing "Approvers:", "QE Manager / `@kmajcher-rh` `@fabiand`", and "Product
Manager / Ronen Sde-Or") to: (1) explicitly include a QE Lead entry with name
and `@handle`, (2) include the PM entry with the PM's GitHub handle (e.g., replace
"Ronen Sde-Or" with "Ronen Sde-Or / `@pm-handle`"), and (3) add a Dev Lead entry
with name and `@handle`; ensure there are no placeholder texts and all three
required roles are present.
- Around line 249-295: The Risk sections across the document still have
placeholder sign-offs and non-actionable mitigations; replace every "[Name/Date
— TBD]" (present under the Risk/ Mitigation/Sign-off entries for
Timeline/Schedule, Test Coverage, Test Environment, Untestable Aspects, Resource
Constraints, Dependencies, Other) with actual approver names and dates, and
update each Mitigation entry to a specific, actionable step (e.g., who will
perform it, timeline, and success criteria) rather than "N/A" or vague text so
every risk entry meets STP Section II.5 requirements.
- Around line 137-151: Replace the placeholder PM/Lead Agreement entries for the
four out-of-scope items — "Testing OpenShift RBAC infrastructure itself",
"Testing all individual permission rules within virtualization roles", "External
IdP compatibility (LDAP, Active Directory)", and "Multi-tenant cluster scale
testing (100+ users)" — with actual approver name and date values in the
required format; update each "- **Item**" bullet so it follows the specified
pattern ('- **Item** / *Rationale:* / *PM/Lead Agreement:*') and substitute each
"[Name/Date — TBD]" with a concrete "Name — YYYY-MM-DD" (or equivalent full
date) to satisfy the STP Section II.1 requirement.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: b9d52cd3-054c-41f2-a94a-4444efb0fd10
📒 Files selected for processing (1)
stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md
Comment on lines
+120
to
+130
| - **[P0]** Verify a cluster administrator can enable role aggregation opt-out through the cluster configuration and the setting persists | ||
| - **[P0]** Verify that when opt-out is enabled, an unprivileged user with a project admin role cannot perform virtualization admin actions (receives Forbidden error) | ||
| - **[P0]** Verify that when opt-out is enabled, an unprivileged user with an edit role cannot perform virtualization edit actions (receives Forbidden error) | ||
| - **[P0]** Verify that when opt-out is enabled, an unprivileged user with a view role cannot perform virtualization view actions (receives Forbidden error) | ||
| - **[P0]** Verify that a cluster administrator can explicitly grant virtualization admin, edit, and view permissions to a user when opt-out is enabled, and the user can perform the corresponding actions | ||
| - **[P0]** Verify a cluster administrator can disable role aggregation opt-out and default automatic access is restored | ||
| - **[P0]** Verify default behavior (role aggregation enabled) remains unchanged when the feature is not configured | ||
| - **[P0]** Verify default behavior is preserved across OpenShift Virtualization z-stream upgrades | ||
| - **[P1]** Verify that toggling opt-out off after it was enabled restores automatic access for users who previously lost it | ||
| - **[P1]** Verify that removing an explicit role binding while opt-out is enabled immediately revokes the user's virtualization access | ||
|
|
There was a problem hiding this comment.
HIGH: Testing Goals section does not explicitly distinguish regression-suite execution.
The goals list is functional-focused, but Section II.1 also requires explicit regression-goal identification (which SIG suites run on the feature cluster). Please add a dedicated regression goal entry in this section.
As per coding guidelines: "STP Section II.1 Testing Goals: Must distinguish between new functional tests and regression tests — regression tests in Testing Goals identify which SIG test suites run on the feature cluster".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stps/sig-iuo/CNV-63822-role-aggregation-opt-out.md` around lines 120 - 130,
Add a dedicated "Regression" goal under Section II.1 Testing Goals that
explicitly distinguishes regression from new functional tests and lists which
SIG suites will run on the feature cluster (e.g., "Regression: run SIG-iuo
regression suite" and any other relevant SIG suites); update the Testing Goals
list near the existing P0/P1 functional bullets so it clearly states
"Functional: ..." and "Regression: [SIG suites]" to satisfy the requirement that
regression-suite execution be identified.
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
6 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.
STP Metadata
Continuation of #28 while rlobillo is away
VEP issue: kubevirt/enhancements#160
What this PR does
STP ready for QE team review
Special notes for your reviewer
Summary by CodeRabbit