WIP: CNTRLPLANE-334: Update KAS structured authentication configuration generation logic to use new `uid` and `extra` field #5840

everettraven · 2025-03-14T19:22:30Z

What this PR does / why we need it:

Updates the KAS structured authentication generation logic to use the new uid and extra claim mapping fields introduced to the authentications.config.openshift.io CRD in CNTRLPLANE-332: Add uid and extra claim mappings for external OIDC configuration api#2234

Which issue(s) this PR fixes (optional, use fixes #<issue_number>(, fixes #<issue_number>, ...) format, where issue_number might be a GitHub issue, or a Jira story:
Fixes #

Checklist

Subject and description added to both, commit and PR.
Relevant issues have been referenced.
This change includes docs.
This change includes unit tests.

openshift-ci · 2025-03-14T19:23:54Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: everettraven
Once this PR has been reviewed and has the lgtm label, please assign bryan-cox for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

everettraven · 2025-03-14T19:25:45Z

It is expected that all tests will fail as this is consuming unmerged changes to the openshift/api dependency.

rtheis · 2025-03-15T09:59:22Z

@jonesbr17 ptal

openshift-ci-robot · 2025-03-18T13:47:10Z

@everettraven: This pull request references CNTRLPLANE-334 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 story to target the "4.19.0" version, but no target version was set.

In response to this:

What this PR does / why we need it:

Updates the KAS structured authentication generation logic to use the new uid and extra claim mapping fields introduced to the authentications.config.openshift.io CRD in CNTRLPLANE-332: Add uid and extra claim mappings for external OIDC configuration api#2234

Which issue(s) this PR fixes (optional, use fixes #<issue_number>(, fixes #<issue_number>, ...) format, where issue_number might be a GitHub issue, or a Jira story:
Fixes #

Checklist

Subject and description added to both, commit and PR.

Relevant issues have been referenced.

This change includes docs.

This change includes unit tests.

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.

everettraven · 2025-03-25T12:23:16Z

/retest

everettraven · 2025-03-25T12:32:52Z

/testwith openshift/hypershift/main/e2e-aws openshift/api#2234

everettraven · 2025-03-25T15:48:50Z

/test e2e-aws

enxebre · 2025-03-25T16:09:12Z

/label tide/merge-method-squash

everettraven · 2025-03-25T18:24:27Z

/test e2e-aws

everettraven · 2025-03-25T19:33:57Z

/test e2e-aws

everettraven · 2025-03-26T11:16:40Z

/payload-with-prs 4.19 ci blocking openshift/api#2234

openshift-ci · 2025-03-26T11:16:52Z

@everettraven: trigger 4 job(s) of type blocking for the ci release of OCP 4.19

periodic-ci-openshift-release-master-ci-4.19-upgrade-from-stable-4.18-e2e-aws-ovn-upgrade
periodic-ci-openshift-release-master-ci-4.19-upgrade-from-stable-4.18-e2e-azure-ovn-upgrade
periodic-ci-openshift-release-master-ci-4.19-e2e-gcp-ovn-upgrade
periodic-ci-openshift-hypershift-release-4.19-periodics-e2e-aws-ovn

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/cb5c9ea0-0a33-11f0-8c09-1b0434e6e3a5-0

everettraven · 2025-03-26T13:40:50Z

/payload-abort

openshift-ci · 2025-03-26T13:40:55Z

@everettraven: aborted active payload jobs for pull request #5840

openshift-ci · 2025-03-28T17:45:29Z

@everettraven: trigger 4 job(s) of type blocking for the ci release of OCP 4.19

periodic-ci-openshift-release-master-ci-4.19-upgrade-from-stable-4.18-e2e-aws-ovn-upgrade
periodic-ci-openshift-release-master-ci-4.19-upgrade-from-stable-4.18-e2e-azure-ovn-upgrade
periodic-ci-openshift-release-master-ci-4.19-e2e-gcp-ovn-upgrade
periodic-ci-openshift-hypershift-release-4.19-periodics-e2e-aws-ovn

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/6f211ab0-0bfc-11f0-87b2-e5e70c1f99a6-0

everettraven · 2025-03-31T20:18:15Z

...nstall/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Default.crd.yaml

                        required:
                        - name
                        type: object
-                      oidcProviders:


This seems wrong. Digging.

everettraven · 2025-04-07T12:19:42Z

control-plane-operator/controllers/hostedcontrolplane/kas/auth.go

+	out.Username = username
+	out.Groups = groups
+
+	if featuregate.Gates.Enabled(featuregate.ExternalOIDCWithUIDAndExtraClaimMappings) {


TODO: pending #5976 merging, will need to update how the feature gates are being used here

Signed-off-by: Bryce Palmer <[email protected]>

everettraven · 2025-04-15T20:02:06Z

control-plane-operator/controllers/hostedcontrolplane/kas/auth.go

+	// TODO: using the default compiler means that we are allowing CEL library usage for the Kube version that maps to our
+	// dependency import. This should align with the version of Kubernetes that will be running for a guest cluster instead
+	// since that is what will actually load and validate the configuration.


2 lines of thought here:

We leave this as-is. This means we validate against progressively newer validations every time we bump our kube dependencies. Should generally be safe as kube tries to avoid breaking changes and is heavily concerned with backwards compatibility. However, we may end up with false-positives where a CEL library is available in the version we are using for our dependency, but not available on the KAS being deployed. Seems like a non-starter, but sharing my train of thought here for others.

We implement a system to map an OCP version --> Kubernetes version. This seems like the only reasonable way to really approach this. In practice, I'm not sure how one would extract this information from the HostedControlPlane resource as it doesn't seem to include the OCP release version it is using in the spec anywhere. Doing some digging on this.

Signed-off-by: Bryce Palmer <[email protected]>

…on for CEL compilation validation Signed-off-by: Bryce Palmer <[email protected]>

everettraven · 2025-04-16T19:48:25Z

/retest

openshift-ci · 2025-04-16T23:31:45Z

@everettraven: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws	`400c710`	link	true	`/test e2e-aws`
ci/prow/verify	`400c710`	link	true	`/test verify`
ci/prow/okd-scos-e2e-aws-ovn	`400c710`	link	false	`/test okd-scos-e2e-aws-ovn`

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

everettraven · 2025-05-01T11:27:46Z

Closing in favor of #6073

openshift-merge-robot · 2025-05-01T11:27:55Z

PR needs rebase.

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.

openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/needs-area labels Mar 14, 2025

openshift-ci bot requested review from csrwng and sjenning March 14, 2025 19:23

openshift-ci bot added the area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release label Mar 14, 2025

openshift-ci bot removed the do-not-merge/needs-area label Mar 14, 2025

everettraven changed the title ~~WIP: Update KAS structured authentication configuration generation logic to use new uid and extra field~~ WIP: CNTRLPLANE-334: Update KAS structured authentication configuration generation logic to use new uid and extra field Mar 18, 2025

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Mar 18, 2025

everettraven mentioned this pull request Mar 21, 2025

CNTRLPLANE-332: Add uid and extra claim mappings for external OIDC configuration openshift/api#2234

Merged

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 24, 2025

everettraven force-pushed the feature/oidc-uid-extra branch from a0fc23a to c3f2ffa Compare March 24, 2025 14:24

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 24, 2025

openshift-ci bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Mar 25, 2025

openshift-ci bot added area/api Indicates the PR includes changes for the API area/cli Indicates the PR includes changes for CLI labels Mar 26, 2025

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 26, 2025

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 31, 2025

everettraven commented Mar 31, 2025

View reviewed changes

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 1, 2025

openshift-ci bot added the area/ci-tooling Indicates the PR includes changes for CI or tooling label Apr 1, 2025

everettraven mentioned this pull request Apr 3, 2025

CNTRLPLANE-334: openshift/api dependency bumps #5971

Merged

4 tasks

everettraven force-pushed the feature/oidc-uid-extra branch 3 times, most recently from e83d621 to ab7df82 Compare April 7, 2025 12:17

everettraven commented Apr 7, 2025

View reviewed changes

everettraven mentioned this pull request Apr 7, 2025

CNTRLPLANE-334: add feature set aware feature gates to HO and CPO #5976

Merged

4 tasks

everettraven force-pushed the feature/oidc-uid-extra branch 2 times, most recently from bbf1630 to a9e70f1 Compare April 9, 2025 17:41

everettraven mentioned this pull request Apr 11, 2025

OCPBUGS-56444: feat: Promote ExternalOIDCWithUIDAndExtraClaimMappings feature to GA for Hypershift #6025

Merged

4 tasks

everettraven added 3 commits April 15, 2025 13:33

byooidc: add support for specifying uid and extra claim mappings

ea0912b

Signed-off-by: Bryce Palmer <[email protected]>

fixup! add back featuregate post-rebase

fac7945

Signed-off-by: Bryce Palmer <[email protected]>

fixup! remove cpo feature-gate flags setup, fix some formatting

8d7c47f

Signed-off-by: Bryce Palmer <[email protected]>

everettraven force-pushed the feature/oidc-uid-extra branch from 1196519 to 8d7c47f Compare April 15, 2025 17:33

improve validation logic for OIDC authentication mode

5f1294f

Signed-off-by: Bryce Palmer <[email protected]>

openshift-ci bot added the area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release label Apr 15, 2025

everettraven commented Apr 15, 2025

View reviewed changes

everettraven added 2 commits April 16, 2025 08:51

supportedversion: add ocp-to-kube version mapping utilities

7925cad

Signed-off-by: Bryce Palmer <[email protected]>

update structured auth validation to use minimum supported kube versi…

400c710

…on for CEL compilation validation Signed-off-by: Bryce Palmer <[email protected]>

kevinrizza mentioned this pull request Apr 23, 2025

CNTRLPLANE-334: Update KAS structured authentication configuration generation logic to use new uid and extra field #6073

Merged

4 tasks

everettraven closed this May 1, 2025

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 1, 2025

WIP: CNTRLPLANE-334: Update KAS structured authentication configuration generation logic to use new uid and extra field #5840

WIP: CNTRLPLANE-334: Update KAS structured authentication configuration generation logic to use new uid and extra field #5840

Uh oh!

Conversation

everettraven commented Mar 14, 2025

Uh oh!

openshift-ci bot commented Mar 14, 2025

Uh oh!

everettraven commented Mar 14, 2025

Uh oh!

rtheis commented Mar 15, 2025

Uh oh!

openshift-ci-robot commented Mar 18, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

everettraven commented Mar 25, 2025

Uh oh!

everettraven commented Mar 25, 2025

Uh oh!

everettraven commented Mar 25, 2025

Uh oh!

enxebre commented Mar 25, 2025

Uh oh!

everettraven commented Mar 25, 2025

Uh oh!

everettraven commented Mar 25, 2025

Uh oh!

everettraven commented Mar 26, 2025

Uh oh!

openshift-ci bot commented Mar 26, 2025

Uh oh!

everettraven commented Mar 26, 2025

Uh oh!

openshift-ci bot commented Mar 26, 2025

Uh oh!

openshift-ci bot commented Mar 28, 2025

Uh oh!

everettraven Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

everettraven Apr 7, 2025

Choose a reason for hiding this comment

Uh oh!

everettraven Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

everettraven commented Apr 16, 2025

Uh oh!

openshift-ci bot commented Apr 16, 2025

Uh oh!

everettraven commented May 1, 2025

Uh oh!

openshift-merge-robot commented May 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

WIP: CNTRLPLANE-334: Update KAS structured authentication configuration generation logic to use new `uid` and `extra` field #5840

WIP: CNTRLPLANE-334: Update KAS structured authentication configuration generation logic to use new `uid` and `extra` field #5840

openshift-ci-robot commented Mar 18, 2025 •

edited by openshift-ci bot

Loading