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

New plugin: gke-exec-credential #118

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@jglick
Copy link

commented Mar 21, 2019

https://github.com/jglick/gke-exec-credential/blob/master/README.md#gke-exec-credential and see kubernetes/kubernetes#62185


Checklist for plugin developers:

  • Read the Plugin Naming Guide (for new plugins)
  • Verify the installation from URL or a local archive works (kubectl krew install --manifest=[...] --archive=[...])
@googlebot

This comment has been minimized.

Copy link

commented Mar 21, 2019

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

1 similar comment
@googlebot

This comment has been minimized.

Copy link

commented Mar 21, 2019

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@ahmetb

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2019

Thanks for the PR. I'll currently put this on hold, as we're considering allowing only official vendors to allow submitting plugins with the vendor prefix (in this case gke-) to the centralized plugin index.

We haven't yet come to a decision about the plugin acceptance criteria, but will do soon.


Aside from that, I think (since I work on GKE) we should probably distribute a credential retriever binary ourselves as part of gcloud SDK that's automatically installed (just like docker credentials helper for GCR).

@ahmetb ahmetb added the hold label Mar 21, 2019

@ahmetb

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2019

I've found several other reasons why this might not be suitable for a distribution via krew's centralized index:

  • users don't directly call this tool, plugins are mostly intended to be visible to the users and enhance kubectl's behavior.
  • I think most users won't be editing their GKE kubeconfigs to reference this tool.

I don't see a clear benefit to this tool than GKE's current model where we create per-context "user" entry (sure, it's wasteful, but that's something GKE command can probably fix).

I'm curious about why you’re developing this plugin and why you think it's beneficial. Feel free to drop me a line at {my-username}@google.com.

@jglick

This comment has been minimized.

Copy link
Author

commented Mar 22, 2019

we're considering allowing only official vendors to allow submitting plugins with the vendor prefix […] to the centralized plugin index

Ah, something to mention in the naming guide?

(since I work on GKE) we should probably distribute a credential retriever binary ourselves as part of gcloud SDK

Of course I would be happy to close this if that is in the works! Since my script merely massages the JSON output a bit from an existing gcloud subcommand, presumably it would not be much work to do that upstream.

I don't see a clear benefit to this tool than GKE's current model where we create per-context "user" entry

The redundancy (when you have multiple clusters registered) is an annoyance, but independent of this tool since you could of course manually deduplicate.

why you’re developing this plugin and why you think it's beneficial

Perhaps the README was not sufficiently clear. The purpose of this tool is to help migrate towards the newer and more generic authentication method, to reduce the pressure on clients to perfect support for older vendor-specific methods such as gcp. See kubernetes-client/java#290 (comment) for an example.

@ahmetb

This comment has been minimized.

Copy link
Contributor

commented Mar 22, 2019

The purpose of this tool is to help migrate towards the newer and more generic authentication method

But when users run gcloud [...] get-credentials again, that benefit will be reverted, as the new context entries won't be using the auth plugin you developed?

Also, right now this plugin is just 1-line, I'm not sure if it's something we should be accepting to the main repo as it is.

gcloud config config-helper --format=json "$@" | jq '{"apiVersion": "client.authentication.k8s.io/v1beta1", "kind": "ExecCredential", "status": {"token": .credential.access_token, "expirationTimestamp": .credential.token_expiry}}'

Ideally this should be distributed as part of gcloud ––if there's a need. Right now our team doesn't see a clear benefit for it. You will still be depending on gcloud to be in $PATH.

P.S. If you would like to authenticate to GKE in headless environments, you can just use the GOOGLE_APPLICATION_CREDENTIALS env variable to supply credentials and delete the part of the users entry that calls out to gcloud, this way, you actually don't need gcloud to be installed.

@jglick

This comment has been minimized.

Copy link
Author

commented Mar 27, 2019

But when users run gcloud [...] get-credentials again, that benefit will be reverted

That is why when running this command I back up my ~/.kube/config, run it, copy out the server URL & credentials, revert to backup, and then copy in the context entries. Even before switching to this mode I did the same workflow, since get-credentials gratuitously reformats everything and I want this file versioned.

Ideally this should be distributed as part of gcloud

Yes that would be nice.

You will still be depending on gcloud to be in $PATH.

Correct, just using a more general authentication method than gcp.

@ahmetb

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

/hold

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.