OPCT-353: fix UserAPI test to accept SA-based admin identities - #31646
redhat-chai-bot wants to merge 1 commit into
Conversation
The "[sig-auth][Feature:UserAPI] users can manipulate groups" test hardcoded allowedGroups to only accept system:masters or system:cluster-admins. When tests run using a ServiceAccount (e.g. OPCT), the user/~ API correctly returns SA-specific groups like system:serviceaccounts and system:serviceaccounts:<ns>, but the assertion rejected them. Replace the hardcoded list with a dynamic check that validates system:authenticated is present for all callers, then checks identity-specific groups based on the caller name: - ServiceAccount callers: system:serviceaccounts + namespace group - Cert-based admins: system:masters or system:cluster-admins Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
Bot
added
the
ready-for-human-review
|
@redhat-chai-bot: This pull request references OPCT-353 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.1.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. WalkthroughChangesUser group assertions
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The updated test assertions accommodate ServiceAccount admin identities without an identified regression. 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: redhat-chai-bot 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
/ok-to-test |
openshift-ci
Bot
added
the
ok-to-test
|
Scheduling tests matching the |
|
/override-sticky ci/prow/e2e-aws-ovn-microshift-serial Automated triage: This failure appears unrelated to the PR changes. Job classification: Eligible long-running AWS MicroShift e2e/integration job. The Prow run completed after 1h35m57s, and the job definition resolves to the Revision check: run Execution status: Tests executed. The run produced test artifacts with 560 tests and failed three unique blocking tests, each retried. The failure is Failing tests:
Completed supporting jobs: None of the completed checks are supporting e2e jobs; Fleet-wide failure rate: Outside this PR, each of the three exact tests had 28 failures and 65 passes in 93 runs, a 30.1% failure rate. The job had 18 successes and 16 failures in the last 14 days, a 52.9% pass rate. The same three failures were reproduced on unrelated PR #31638 with the identical error. Overlap assessment: The PR changes only Missing-coverage risk: Low for this PR. The job executed the suite and the failures are isolated to a known MicroShift storage/environment mismatch unrelated to UserAPI behavior. The override does not replace the still-pending e2e coverage. Rationale: The exact failures are fleet-wide and MicroShift-specific, reproduce on an unrelated PR, and do not exercise the code changed here. The run is current and test execution is proven, so If you disagree with this assessment, rerun the current job with AI-generated. Review for accuracy. |
|
@redhat-chai-bot: Overrode contexts on behalf of redhat-chai-bot: ci/prow/e2e-aws-ovn-microshift-serial These overrides will persist across retests on the current HEAD SHA. Pushing a new commit will clear them. Use DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@redhat-chai-bot: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
CI triage — This failure is unrelated to the PR changes. All 6 blocking failures are CSI VolumeGroupSnapshot tests:
This is the same fleet-wide CSI VolumeGroupSnapshotClass issue that caused the A reviewer with the required Prow permissions can apply a sticky override for AI-generated. Review for accuracy. |
|
/payload-job periodic-ci-openshift-release-main-nightly-5.0-opct-platform-external-aws-ccm |
|
@bshaw7: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/91f7ab40-b19f-11f1-8c91-c34b36cc44d2-0 |
|
/retest ci/prow/e2e-aws-ovn-microshift AI-generated. Review for accuracy. |
|
/retest |
|
/override-sticky ci/prow/e2e-aws-ovn-microshift Automated triage: This failure appears unrelated to the PR changes. Job classification: Eligible long-running AWS MicroShift e2e/integration presubmit. The Prow run completed after 2h0m1s and used the Revision check: run Execution status: Tests executed. The conformance suite ran for 1h2m50s and reported 6 blocking failures, 1115 passes, 2006 skips, and 0 flaky results. The blocking failures included these exact tests:
All three failed with Completed supporting jobs: No completed e2e job is needed for this executed-test override path. Other completed PR checks included Fleet-wide failure rate: CI fleet analysis found each exact failing test at 0% pass across 11 runs of this presubmit job outside this PR. The same tests also failed 33/33 runs in the periodic MicroShift 5.1 conformance job. This is a deterministic, pre-existing MicroShift VolumeGroupSnapshot API/CRD availability failure, not a PR-specific test flake. Overlap assessment: The PR changes only Missing-coverage risk: Low for this decision. The failed coverage is the MicroShift VolumeGroupSnapshot surface, which is unavailable independently of this UserAPI-only test assertion change. The PR's other completed checks passed, including unit, lint, verify, image, and multiple other e2e checks. The override does not claim the storage coverage passed; it records that this job's failure is unrelated. Prior bot activity on this SHA: A targeted retest for Rationale: Override this required long-running job because the tests actually executed, the exact failing storage tests have a 0% fleet-wide pass rate in this job and fail identically in periodic MicroShift runs, and the PR has no plausible overlap with the failing tested surface. If you disagree with this assessment, rerun the current job with AI-generated. Review for accuracy. |
|
@redhat-chai-bot: Overrode contexts on behalf of redhat-chai-bot: ci/prow/e2e-aws-ovn-microshift These overrides will persist across retests on the current HEAD SHA. Pushing a new commit will clear them. Use DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
CI test openshift-origin-31646-nightly-5.0-opct-platform-external-aws-ccm shows this time test 
|
|
/verified by CI test |
openshift-ci-robot
added
the
verified
|
@bshaw7: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
3 participants
Summary
The
[sig-auth][Feature:UserAPI] users can manipulate groupstest hardcodesallowedGroupsto only acceptsystem:mastersorsystem:cluster-adminswhen checking theuser/~response for unbacked users. This causes a false-positive failure when tests are run using a ServiceAccount identity (e.g. by OPCT), because SAs belong tosystem:serviceaccountsandsystem:serviceaccounts:<namespace>groups instead.The
user/~API works correctly in all cases — it faithfully returns the caller's authenticator-assigned groups. The test assertion is too strict.Changes
Replace the hardcoded
allowedGroupsexact-match check with dynamic validation based on the caller's identity type:system:authenticatedmust always be present — this is true for all authenticated callers regardless of identity typesystem:serviceaccount:*) — validate thatsystem:serviceaccountsandsystem:serviceaccounts:<namespace>are presentsystem:mastersorsystem:cluster-adminsis present (preserves existing behavior)Adds a
containsGrouphelper function for membership checks.Why This Matters
The
openshift/conformance/parallelsuite is used by downstream tools (OPCT, partner testing frameworks) that authenticate via ServiceAccount tokens rather than client certificates. These tools have cluster-admin RBAC permissions but different group identities. The test should validate thatuser/~returns correct authenticator-assigned groups for any caller type, not justsystem:masters.Testing
go vet ./test/extended/user/passesJira: OPCT-353
AI-generated. Review for accuracy.
@bshaw7 requested via Chai Bot
Summary by CodeRabbit