feat: import kubelogin library mode #207
Conversation
There was a problem hiding this comment.
@erhancagirici kubelogin has merged the library mode change, this is the draft pr for showing how to use the library mode. A few things needs to confirm with your side's usage. Please see the comments.
| func WrapRESTConfig(_ context.Context, rc *rest.Config, credentials []byte, _ ...string) error { | ||
| m := map[string]string{} | ||
| if err := json.Unmarshal(credentials, &m); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| fs := pflag.NewFlagSet("kubelogin", pflag.ContinueOnError) |
There was a problem hiding this comment.
@erhancagirici I am less sure how crossplane consumes this rest config: are we expecting the input config is returned from an AAD enabled AKS cluster? Because using kubelogin's SP login method, it's required to have server-id set.
There was a problem hiding this comment.
@bcho
yes, this codepath here is executed when the user specifies an identity section in the provider configuration, alongside the cluster credentials (kubeconfig). Specifying the identity section (credentials for the SP) assumes user's intention that the kubeconfig cannot work standalone and this is an Azure AAD enabled AKS cluster kubeconfig with execProvider section. If user does not specify any identity block in the provider configuration, then we assume that the kubeconfig can work standalone, therefore do not visit this codepath at all, and do not wrap the rest config i.e no kubelogin invocation.
So, it is OK to assume that we have a AAD-enabled AKS kubeconfig, hence with execProvider section and a --server-id present here.
Regarding the general intention here: we aim to non-interactively authenticate to the k8s cluster, given a kubeconfig with interactive login. Therefore we would like to parse execProvider section that has kubelogin command and arguments, and build the necessary list of options for SP auth (we currently support only SP auth ). This I think, in a way, corresponds to manually handling the kubelogin convert. Then obtain a token to authenticate and remove the execProvider altogether.
Actually, my initial intention was to offload the argument parsing to kubelogin itself somehow, so that we don't maintain kubelogin flags here and we obtain CLI options directly. Then extract/reuse some like server-id, environment to build new options for SP auth, combining them with the externally-provided SP credentials.
AFAIU, kubelogin library mode changes does not expose CLI flags, so we need to still handle the CLI options -> library options parsing + conversions (which is fine btw).
So this is kind of an hybrid use case, where we would like to still kubelogin with CLI params but as a library. It is a matter of at which level we integrate to kubelogin.
Thanks a ton for working on this, the kubelogin changes are very much appreciated!
There was a problem hiding this comment.
thanks for your review, I agree with you on the flags parsing pain here. I created an issue to track it: Azure/kubelogin#394 and will see how to expose it in library mode. We can follow up after the change in kubelogin side.
bcho
changed the title
There was a problem hiding this comment.
@bcho thanks a ton for this pull request and upstream kubelogin changes, much appreciated!
Left a comment regarding the flag parsing and code change suggestion. With the changed suggestion I was able to test and validate that it is working.
Signed-off-by: hbc <[email protected]>
Signed-off-by: hbc <[email protected]>
Co-authored-by: Erhan Cagirici <[email protected]> Signed-off-by: hbc <[email protected]>
Description of your changes
This pull request bumps the
azure/kubeloginversion to v0.1.0, which has direct support for using as a library.This is a follow pr for: #205
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested