Skip to content

Conversation

@mociarain
Copy link
Contributor

This PR replaces: #2245

@openshift-ci
Copy link

openshift-ci bot commented Aug 8, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@mociarain mociarain mentioned this pull request Aug 8, 2025
}
},
),
expectedCSExternalAuth: getBaseCSExternalAuthBuilder().Clients(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JameelB These expectedCSExternalAuth represent what we will send to CS

Copy link
Collaborator

@mbarnes mbarnes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for the most part. In the RP-to-CS conversion, I suggest we skip setting builder fields to empty strings.

@mociarain mociarain requested a review from mbarnes August 8, 2025 17:13
@mfreeraro
Copy link

mfreeraro commented Aug 12, 2025

a part from the changes above LGTM

@mociarain mociarain requested review from mbarnes and mfreeraro August 12, 2025 12:49
@mbarnes mbarnes requested a review from sclarkso as a code owner August 13, 2025 17:02
@mbarnes mbarnes force-pushed the mociarain/external-auth-fixes branch from 2050353 to c69f83f Compare August 14, 2025 03:10
@mbarnes mbarnes force-pushed the mociarain/external-auth-fixes branch from c96336c to d91082a Compare August 14, 2025 03:37
@mbarnes mbarnes dismissed their stale review August 14, 2025 03:49

Fixed my own requests.

@mfreeraro
Copy link

@mbarnes and @SudoBrendan or @bennerv can this be merged now or is there additional work needed?

@mbarnes mbarnes merged commit 9e68480 into main Aug 14, 2025
21 checks passed
@mbarnes mbarnes deleted the mociarain/external-auth-fixes branch August 14, 2025 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants