OCPBUGS-56908: validate identity provider names as annotation keys - #3025
OCPBUGS-56908: validate identity provider names as annotation keys#3025agawai wants to merge 1 commit into
Conversation
Reject identity provider names that cannot be used in the oauth.openshift.io/idp.<name> Group annotation so OpenID group sync cannot break login. Existing names are grandfathered until changed. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@agawai: This pull request references Jira Issue OCPBUGS-56908, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
[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 |
|
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: Team Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (3)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe OAuth API now validates identity provider names against the Kubernetes annotation name-part pattern. Existing unchanged invalid names remain allowed through the grandfather clause. The generated CRD includes the same validation and documentation. Tests cover valid names, names containing spaces, provider field updates, invalid provider additions, and valid provider renames. Merge Risk: ⚪ Minimal · up to OAuth identity provider names now reject annotation-incompatible new values while allowing unchanged legacy names to be updated. The validation, published schema, and covered update behavior are aligned, with no remaining merge-blocking risk identified. 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 golangci-lint (2.13.2)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
|
Hello @agawai! Some important instructions when contributing to openshift/api: |
|
Hi @agawai. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
Summary
spec.identityProviderssonamematches the annotation name-part used by oauth-server (oauth.openshift.io/idp.<name>): alphanumeric,-,_,., start and end alphanumeric, at most 59 characters. This is the OCPBUGS-56908 failure (Microsoft Entra ID,AIF - Keycloak).optionalOldSelfso existing clusters that already have those names can still update the OAuth object until the name is changed. New names with spaces are rejected at admission.Test plan
config/v1/tests/oauths.config.openshift.io/AAA_ungated.yamlcover valid names, names with spaces, adding a new invalid IdP, and renaming to a valid nameverify/ integration tests foroauths.config.openshift.ioMade with Cursor