[WIP] - keystone token authentication #25536
Conversation
|
Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test". This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry. Otherwise, if this message is too spammy, please complain to ixdy. |
2 similar comments
|
Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test". This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry. Otherwise, if this message is too spammy, please complain to ixdy. |
|
Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test". This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry. Otherwise, if this message is too spammy, please complain to ixdy. |
k8s-github-robot
added
size/XXL
|
@zhouhaibing089 Thanks for posting this. This code, and the one in the other review seem pretty complimentary. #25391 has a bit less code in the getting of the Keystone Auth user credentials part, reusing a lot of existing code in k8s. I think that part's cleaner over there. This code seems to be mostly about adding pki token validation support. Thats awesome work. I hadn't attempted that since we're using Fernet tokens on all of our clouds now. The tests here are very nice too. The rest of #25391 is related to returning data from the token over to authz plugins, and I have one almost completed I think. Once we can figure out the Extra data accessibility thing. I think its only a day or two away before I can post that. So, how would you like to proceed? I think the other patch is maybe a good place to start from, and then we can rebase this one on top of the other one to add the pki support to it? That way, we don't have one gigantic patch. I'm much more use to using Gerrit so I don't quite know how the workflow for having one PR depend on another PR works, if that's even possible. Anyone know? Thanks, |
if you mean reusing most of the
It is also not true, it is a special case that the token which starts with
I admire your work, which I would like to pick, especially thank you for reminds me of adding
I do not want to be selfishly, so let's discuss, I personally think porting your feature here would be much easier though. To address your concern, I will do some refactoring on |
|
@kfox1111 we could do some offline discussion, however I could not find your email in your profile, so would you mind sending message with me in slack |
zhouhaibing089
force-pushed
the
ks-token
branch
3 times, most recently
from
4065f12
to
316318c
Compare
|
Its unfortunate you waited so long to post this. I've been working on the other set for a while and I've gotten the whole thing working in the other review. I was able to test it with PKI tokens with the remote authentication does work with it, so I believe the majority of the code here is an optimization for local authentication of PKI tokens. (All of the vendor/github.com/fullsailor/pkcs7/* stuff) I think all the other features that this patch does, is already covered in the other version? Are there any gaps? If that's the case, I still think its probably a good idea to layer these changes on top of the other patch set, rather the selectively copy features from the other patch set into this one. Then this set of optimizations can happen as a separate review, allowing for an easier set of reviews. To merge, I think all we need to do is merge one file that exists in both: And remove the following lines from this patchset as they are no longer needed: |
k8s-github-robot
added
kind/old-docs
needs-rebase
|
rebased. |
k8s-github-robot
removed
the
needs-rebase
|
@zhouhaibing089 PR needs rebase |
k8s-github-robot
added
the
needs-rebase
k8s-github-robot
added
size/XL
|
Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test". This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry. Otherwise, if this message is too spammy, please complain to ixdy. |
1 similar comment
|
Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test". This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry. Otherwise, if this message is too spammy, please complain to ixdy. |
|
keep-open |
|
Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test". This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry. Otherwise, if this message is too spammy, please complain to ixdy. |
2 similar comments
|
Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test". This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry. Otherwise, if this message is too spammy, please complain to ixdy. |
|
Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test". This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry. Otherwise, if this message is too spammy, please complain to ixdy. |
|
Close it as it is not my focus for now, also it causes many spam messages. |
|
@zhouhaibing089 I think I would like to pick this up - would you be willing to chat on kube-slack if we have questions? |
|
@stevemcquaid : I've got a version here that I think is pretty close: #25624 |
|
@stevemcquaid if you have interest in openstack token auth, I'd recommend working with @kfox1111 instead on #25391 |
|
@kfox1111 @liggitt @stevemcquaid Basically I am suggesting to move to a webhook implementation, it causes some maintainance pain, as openstack is too diversity, we have v2 token, and now we have v3 toke which adds the domain parameter, and actually, the token itself has four types as far as I know. If necessary, I will pick something up(like local validator, cache, etc) when @kfox1111 has done all the things right. |
|
Is this dead? Is it possible to make this use all of the Keystone auth methods instead of hardcoding it to Keystone Password? |
Automatic merge from submit-queue (batch tested with PRs 50087, 39587, 50042, 50241, 49914) plugin/pkg/client/auth: add openstack auth provider This is an implementation of auth provider for OpenStack world, just like python-openstackclient, we read the environment variables of a list `OS_*`, and client will cache a token to interact with each components, we can do the same here, the client side can cache a token locally at the first time, and rotate automatically when it expires. This requires an implementation of token authenticator at server side, refer: 1. [made by me] #25536, I can carry this on when it is fine to go. 2. [made by @kfox1111] #25391 The reason why I want to add this is due to the `client-side` nature, it will be confusing to implement it downstream, we would like to add this support here, and customers can get `kubectl` like they usually do(`brew install kubernetes-cli`), and it will just work. When this is done, we can deprecate the password keystone authenticator as the following reasons: 1. as mentioned at some other places, the `domain` is another parameters which should be provided. 2. in case the user supplies `apikey` and `secrets`, we might want to fill the `UserInfo` with the real name which is not implemented for now. cc @erictune @liggitt ``` add openstack auth provider ```
Automatic merge from submit-queue (batch tested with PRs 50087, 39587, 50042, 50241, 49914) plugin/pkg/client/auth: add openstack auth provider This is an implementation of auth provider for OpenStack world, just like python-openstackclient, we read the environment variables of a list `OS_*`, and client will cache a token to interact with each components, we can do the same here, the client side can cache a token locally at the first time, and rotate automatically when it expires. This requires an implementation of token authenticator at server side, refer: 1. [made by me] kubernetes/kubernetes#25536, I can carry this on when it is fine to go. 2. [made by @kfox1111] kubernetes/kubernetes#25391 The reason why I want to add this is due to the `client-side` nature, it will be confusing to implement it downstream, we would like to add this support here, and customers can get `kubectl` like they usually do(`brew install kubernetes-cli`), and it will just work. When this is done, we can deprecate the password keystone authenticator as the following reasons: 1. as mentioned at some other places, the `domain` is another parameters which should be provided. 2. in case the user supplies `apikey` and `secrets`, we might want to fill the `UserInfo` with the real name which is not implemented for now. cc @erictune @liggitt ``` add openstack auth provider ```
Automatic merge from submit-queue (batch tested with PRs 50087, 39587, 50042, 50241, 49914) plugin/pkg/client/auth: add openstack auth provider This is an implementation of auth provider for OpenStack world, just like python-openstackclient, we read the environment variables of a list `OS_*`, and client will cache a token to interact with each components, we can do the same here, the client side can cache a token locally at the first time, and rotate automatically when it expires. This requires an implementation of token authenticator at server side, refer: 1. [made by me] kubernetes/kubernetes#25536, I can carry this on when it is fine to go. 2. [made by @kfox1111] kubernetes/kubernetes#25391 The reason why I want to add this is due to the `client-side` nature, it will be confusing to implement it downstream, we would like to add this support here, and customers can get `kubectl` like they usually do(`brew install kubernetes-cli`), and it will just work. When this is done, we can deprecate the password keystone authenticator as the following reasons: 1. as mentioned at some other places, the `domain` is another parameters which should be provided. 2. in case the user supplies `apikey` and `secrets`, we might want to fill the `UserInfo` with the real name which is not implemented for now. cc @erictune @liggitt ``` add openstack auth provider ```
Automatic merge from submit-queue (batch tested with PRs 50087, 39587, 50042, 50241, 49914) plugin/pkg/client/auth: add openstack auth provider This is an implementation of auth provider for OpenStack world, just like python-openstackclient, we read the environment variables of a list `OS_*`, and client will cache a token to interact with each components, we can do the same here, the client side can cache a token locally at the first time, and rotate automatically when it expires. This requires an implementation of token authenticator at server side, refer: 1. [made by me] kubernetes/kubernetes#25536, I can carry this on when it is fine to go. 2. [made by @kfox1111] kubernetes/kubernetes#25391 The reason why I want to add this is due to the `client-side` nature, it will be confusing to implement it downstream, we would like to add this support here, and customers can get `kubectl` like they usually do(`brew install kubernetes-cli`), and it will just work. When this is done, we can deprecate the password keystone authenticator as the following reasons: 1. as mentioned at some other places, the `domain` is another parameters which should be provided. 2. in case the user supplies `apikey` and `secrets`, we might want to fill the `UserInfo` with the real name which is not implemented for now. cc @erictune @liggitt ``` add openstack auth provider ``` Kubernetes-commit: 59b8fa32f129be29f146bfd4888a5d1ab7e71ca5
…ceful-4.5 [release-4.5] Bug 1881351: KCM and KS graceful termination Origin-commit: 3f0e0566c5abf9bbbfa5af0352e2796cd0f9f061
Automatic merge from submit-queue (batch tested with PRs 50087, 39587, 50042, 50241, 49914) plugin/pkg/client/auth: add openstack auth provider This is an implementation of auth provider for OpenStack world, just like python-openstackclient, we read the environment variables of a list `OS_*`, and client will cache a token to interact with each components, we can do the same here, the client side can cache a token locally at the first time, and rotate automatically when it expires. This requires an implementation of token authenticator at server side, refer: 1. [made by me] kubernetes/kubernetes#25536, I can carry this on when it is fine to go. 2. [made by @kfox1111] kubernetes/kubernetes#25391 The reason why I want to add this is due to the `client-side` nature, it will be confusing to implement it downstream, we would like to add this support here, and customers can get `kubectl` like they usually do(`brew install kubernetes-cli`), and it will just work. When this is done, we can deprecate the password keystone authenticator as the following reasons: 1. as mentioned at some other places, the `domain` is another parameters which should be provided. 2. in case the user supplies `apikey` and `secrets`, we might want to fill the `UserInfo` with the real name which is not implemented for now. cc @erictune @liggitt ``` add openstack auth provider ``` Kubernetes-commit: 59b8fa32f129be29f146bfd4888a5d1ab7e71ca5
Automatic merge from submit-queue (batch tested with PRs 50087, 39587, 50042, 50241, 49914) plugin/pkg/client/auth: add openstack auth provider This is an implementation of auth provider for OpenStack world, just like python-openstackclient, we read the environment variables of a list `OS_*`, and client will cache a token to interact with each components, we can do the same here, the client side can cache a token locally at the first time, and rotate automatically when it expires. This requires an implementation of token authenticator at server side, refer: 1. [made by me] kubernetes/kubernetes#25536, I can carry this on when it is fine to go. 2. [made by @kfox1111] kubernetes/kubernetes#25391 The reason why I want to add this is due to the `client-side` nature, it will be confusing to implement it downstream, we would like to add this support here, and customers can get `kubectl` like they usually do(`brew install kubernetes-cli`), and it will just work. When this is done, we can deprecate the password keystone authenticator as the following reasons: 1. as mentioned at some other places, the `domain` is another parameters which should be provided. 2. in case the user supplies `apikey` and `secrets`, we might want to fill the `UserInfo` with the real name which is not implemented for now. cc @erictune @liggitt ``` add openstack auth provider ``` Kubernetes-commit: 59b8fa32f129be29f146bfd4888a5d1ab7e71ca5
This is the keystone token authentication used as
authenticator.Tokenwhich provides another authentication options.features include:
referring #25391 as there will be todo list based on the present discussion there, which includes:
@kfox1111 @liggitt @mkumatag please comment if you get any concern about this, thanks! since there will be more fix accordingly, so I will mark this as a WIP, but u know, it is ready for comments.
Also /cc @ashw7n @uruddarraju and @arkxu who implements the token local validation.
This change is