manifests
manifests copied to clipboard
feat: upgrade oidc-authservice
Which issue is resolved by this Pull Request: Upgrade oidc-authservice to gcr.io/arrikto/kubeflow/oidc-authservice:e236439
- Support OIDC ID Token authentication
- Support ServiceAccount Token authentication
Description of your changes:
- Upgrade oidc-authservice to new docker image
- Add rbac to allow calling tokenreviews API
Additional information
- oidc-authservice changelogs https://github.com/arrikto/oidc-authservice/commits/master
- oidc-authservice registry https://console.cloud.google.com/gcr/images/arrikto/global/kubeflow/oidc-authservice
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
For more information, open the CLA check for this pull request.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: ittus
To complete the pull request process, please assign yanniszark after the PR has been reviewed.
You can assign the PR to them by writing /assign @yanniszark
in a comment when ready.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve
in a comment
Approvers can cancel approval by writing /approve cancel
in a comment
Hi @ittus. Thanks for your PR.
I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test
on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test
label.
I understand the commands that are listed here.
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/test-infra repository.
@ittus could you elaborate a bit more on the changes?
"Support OIDC ID Token authentication" Does that mean we can use serviceaccounts not just for the pipelines API, but for all istio secured stuff, e.g. kfserving, seldon, Kserve etc?
"Support ServiceAccount Token authentication" How exactly does that work? Is it enough to have the default-editor serviceaccount in a Jupyterlab?
Hi @juliusvonkohout , The purpose is that we can use OIDC IdToken or serviceAccount token to authenticate in oidc-authservice. The upstream action can be calling KFServing API.
Currently, it's only available with session authentication
In the new version of oidc-authservice, it has 3 authentications: sessions, idtoken, and k8sauthentication. The authenticate method will check one by one until it passed.
I checked with KFServing, ServiceAccount token and IdToken can be used to call serving API along with session
Can we use it then to authenticate from outside AND inside of the cluster with an extracted serviceaccounttoken from default-editor? I need something like this to trigger pipelines automatically without manual login.
Currently, I'm using it to invoke KFServing outside of cluster. Not test with pipeline yet.
Currently, I'm using it to invoke KFServing outside of cluster. Not test with pipeline yet.
@ittus Thank you very much, then this is a very important feature.
@kimwnasptd is this possible for 1.5 ?
@DavidSpek this feature might also be interesting for you.
@ittus Do you have an example implementation of how you are using either the ID token or service account to call KFserving API ?
@snmohan83 I implemented this in a private project. Do you have a suggestion on how to create an example for this easily for you to check?
@snmohan83 I implemented this in a private project. Do you have a suggestion on how to create an example for this easily for you to check?
@ittus I would like to use API calls similar to the example used here: https://github.com/kserve/kserve/tree/master/docs/samples/istio-dex#run-a-prediction where a session_token can be obtained that can be used in the application.
@snmohan83 I implemented this in a private project. Do you have a suggestion on how to create an example for this easily for you to check?
@ittus I would like to use API calls similar to the example used here: https://github.com/kserve/kserve/tree/master/docs/samples/istio-dex#run-a-prediction where a session_token can be obtained that can be used in the application.
@ittus maybe you have a similar example to share. Are you also using it with other dex authentication providers like keycloak and gitlab?
@kimwnasptd Can we get this into Kubeflow 1.6?
Updating the oidc-authservice was also discussed for the release of Kubeflow 1.3 but could not be included because it would break the logout button of the centraldashboard. Is this still an issue? https://github.com/kubeflow/manifests/pull/1714#issuecomment-803106340
It will also be interesting to see how it works with the new DEX version 2.31.2 https://github.com/kubeflow/manifests/pull/2243#issuecomment-1176388211
Updating the oidc-authservice was also discussed for the release of Kubeflow 1.3 but could not be included because it would break the logout button of the centraldashboard. Is this still an issue? #1714 (comment)
Logout button is still an issue.
Updating the oidc-authservice was also discussed for the release of Kubeflow 1.3 but could not be included because it would break the logout button of the centraldashboard. Is this still an issue? #1714 (comment)
Logout button is still an issue.
Is that also the case for the new DEX version 2.31.2 in Kubeflow 1.6 https://github.com/kubeflow/manifests/pull/2243#issuecomment-1176388211 ?
Hi Team, did you have a chance to test this upgrade?
We have tested the new oidc image with dex 2.31.2 in charmed kubeflow, but we're getting troubles with accessing the dashboard (403 authorization error). Are you aware of sth in the image impacting istio?
The previous image gcr.io/arrikto/kubeflow/oidc-authservice:fef11c3
works correctly with dex 2.31.2, but the logout button is still an issue @juliusvonkohout.
I'd appreciate your feedback, thanks!
I opened a PR to make the logout button work with the updated version of the authservice, feel free to review and provide feedback
@kimwnasptd any hope that it goes into 1.6 with the fixes from @alembiewski ?
We also had to diverge from the upstream manifests since they're using an outdated version of oidc-authservice that doesn't support opaque tokens. The opaque token pattern lets us use the KFP SDK from outside of the cluster by consuming the auth tokens in our kubeconfig.
+1 I would also like this to be implemented in 1.6
/ok-to-test
@manolis-andr @athamark could you help with this review to update the AuthService manifests? Let's also have a follow up PR afterwards to have an OWNERS file in that component.
Hello everyone, sorry for the delay. I am picking this up. In the comments below I will try address the following topics:
- What does the new
e236439
image ofoidc-authservice
offer to the Kubeflow users? - Review the proposed changes of this PR.
- What else do we need for this effort to conclude?
- A brief overview of new features that AuthService offers but this image does not include.
1. What does the proposed e236439
image of oidc-authservice
offer to the Kubeflow users?
The transition proposed with this PR corresponds to ~50 commits. I will try to highlight the most important KF-visible aspects of these changes:
- we have introduced two new authentication methods (
kubernetes
andID token
) and we have factored out thesession
authentication method. This will allow authenticating Kubeflow users to perform requests with one of the three supported authentication methods. Note that for the two new methods AuthService examines theBearer
token included in theAuthorization
header of the request. - we have iterated over the
oidc-authservice
code to resolve some known bugs. For example,oidc-authservice
failed to accept an external Identity Provider that does not support the revocation of the access token. As stated in the RFC 7009 (https://www.rfc-editor.org/rfc/rfc7009#section-2):Implementations MUST support the revocation of refresh tokens and SHOULD support the revocation of access tokens We have made sure that
oidc-authservice
is compliant on this front.
2. Review the proposed changes of this PR.
All the changes proposed for the:
-
common/dex/base/config-map.yaml
-
common/oidc-authservice/base/params.env
-
common/oidc-authservice/base/kustomization.yaml
-
common/oidc-authservice/base/rbac.yaml
-
common/oidc-authservice/base/statefulset.yaml
make sense. I do not see anything that needs to be corrected. Good job @ittus!
Just one minor comment regarding the REDIRECT_URL
:
Keep in mind that the e236439
image does not handle gracefully the REDIRECT_URL
configuration. Admins may initialize it but it won't affect the behavior of oidc-authservice
as intended. We resolved this issue with this PR: https://github.com/arrikto/oidc-authservice/pull/99.
Since this behavior was part of the 6ac9400
image of oidc-authservice
which we currently support for Kubeflow, we should not block on this. But let's expose this so that the community is aware of it.
3. What else do we need for this effort to conclude?
As mentioned above in this conversation, we can not merge this effort without updating the CentralDashboard. See here:
- https://github.com/kubeflow/manifests/pull/2150#issuecomment-1179593861
- a PR for fixing the logout button of the CentralDashboard: https://github.com/kubeflow/kubeflow/pull/6609
I tested the e236439
image with the current CentralDashboard to verify the above claims. Indeed we need to make some modifications on the CentralDashboard to avoid the problem with the logout button.
For this, I am in sync with @kimwnasptd and we will comment on the respective effort to highlight minor fixes that we should make. In short, we should first take care of the logout button and then we should proceed with merging this PR. @kimwnasptd I suggest that we start reviewing the https://github.com/kubeflow/kubeflow/pull/6609.
4. A brief overview of new features that AuthService offers but this image does not include.
Later oidc-authservice
image will:
- include slight improvements regarding the log messages.
- support both
bool
andstring
values for theemail_verified
standard claim. Case is that some Identity Providers (e.g. AWS Cognito) deviate from this spec (https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims) and returnstring
values instead (https://github.com/amazon-archives/amazon-cognito-identity-js/issues/436, https://github.com/arrikto/oidc-authservice/issues/63). - include a caching mechanism and other small changes for boosting the authentication performance.
- include two new authentication methods for both JWT access tokens, and opaque access tokens.
- give the option of integrating with an external authorizer (right now the only authorization check that
oidc-authservice
offers is group-based). - support
redis
session store, admins will be able to select between the currently supportedboltDB
session store and theredis
session store.
@yuzisun @yubozhao this might be interesting to inference systems that integrate with Kubeflow
Thanks for the thorough review and context @athamark! I'll move on and merge this PR.
The follow up step is to work on https://github.com/kubeflow/kubeflow/pull/6609 and ensure it can use a dynamic link for the logout URL. So the first RC will have a bug with the logout but we'll fix it during feature freeze. I'll open an issue so that we can track this.
Thank you for the work @ittus!
/lgtm /approve
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: ittus, kimwnasptd
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [kimwnasptd]
Approvers can indicate their approval by writing /approve
in a comment
Approvers can cancel approval by writing /approve cancel
in a comment