Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Select spec fields specific to Upbound OIDC #1265

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

jastang
Copy link
Contributor

@jastang jastang commented Apr 16, 2024

Description of your changes

Update auth.yaml to select specific fields in the spec to support Upbound as an identity provider.

Screenshot of a deploy with a new xpkg:
Screen Shot 2024-04-16 at 9 43 19 AM

Fixes https://github.com/upbound/upbound-frontend/issues/3751

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Signed-off-by: Jason Tang <jason@upbound.io>
Copy link
Collaborator

@ulucinar ulucinar left a comment

Choose a reason for hiding this comment

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

Thank you @jastang. Discussed this issue with @erhancagirici and we will remove the TokenConfig API from the Upbound authentication scheme once we have the ProviderConfig e2e tests in place.

If, in the future, we would like to be able to configure the source of the OIDC ID token (e.g., making the location of the token configurable so that we may either read it from a local file (as we do right now), or read it from a secret or similar, when the auth method is Upbound), then we can consider re-adding it to the API and implement the retrieval mechanism on top of that API. Currently, the API is there but the retrieval implementation is missing. And in the managed control planes, we always provision the OIDC token on the filesystem at a well-known path, so a configuration for the token source is currently unnecessary.

The alternative would be just to remove the TokenConfig from Upbound auth configuration as the API is dummy and I assume the marketplace does not generate a ProviderConfig with the configuration provided by the TokenConfig API (as it's currently useless). But this is technically a breaking change. And once we have the ProviderConfig e2e tests in place, it will be a better time to make this change in the ProviderConfig.

@ulucinar ulucinar merged commit ae4e4d3 into crossplane-contrib:main Apr 17, 2024
11 checks passed
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.

2 participants