vmware-tanzu / pinniped

Pinniped is the easy, secure way to log in to your Kubernetes clusters.
https://pinniped.dev
Apache License 2.0
541 stars 65 forks source link

Add `pinniped_supported_identity_provider_types` to the IDP discovery endpoint #1928

Closed joshuatcasey closed 3 months ago

joshuatcasey commented 4 months ago

Add pinniped_supported_identity_provider_types to the IDP discovery endpoint

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 88.29114% with 37 lines in your changes are missing coverage. Please review.

Project coverage is 29.62%. Comparing base (6b3f175) to head (7787885).

Files Patch % Lines
cmd/pinniped/cmd/oidc_client_options.go 0.00% 24 Missing :warning:
pkg/oidcclient/login.go 93.50% 7 Missing and 3 partials :warning:
.../controller/kubecertagent/mocks/mockdynamiccert.go 25.00% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1928 +/- ## ========================================== + Coverage 29.40% 29.62% +0.22% ========================================== Files 348 350 +2 Lines 58499 58730 +231 ========================================== + Hits 17200 17400 +200 - Misses 40785 40813 +28 - Partials 514 517 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cfryanr commented 4 months ago

lgtm. Should we do the CLI changes in the same PR, or a new PR?

joshuatcasey commented 4 months ago

lgtm. Should we do the CLI changes in the same PR, or a new PR?

I'm not really sure exactly how we should translate this into CLI changes at the moment. Right now the best place to use it seems to be in pinniped get kubeconfig help text, but calling this endpoint in order to generate help text seems a little awkward. That command at the moment only conditionally calls the endpoint at all.

joshuatcasey commented 4 months ago

I'm actually not super clear how the CLI could best make use of this information for help text purposes.

The pinniped get kubeconfig command has a flag --oidc-issuer, which can be used to look up discovery information. If that flag is not set, it will inspect the JWTAuthenticator for the issuer. Even then it will only perform discovery if it needs additional information such as which IDPs are available.

Perhaps another way we could make use of the supported IDP types is by checking the --upstream-identity-provider-type against pinniped_supported_identity_provider_types? If a user calls pinniped get kubeconfig --upstream-identity-provider-name=<> --upstream-identity-provider-type=<> --upstream-identity-provider-flow=<> all specified, current logic will not perform upstream discovery at all. We'd likely want to rethink how this works.

cfryanr commented 3 months ago

Cases that we should make sure are covered:

What else should we consider?