Adding support for kubectl ExecCredential output #72

christopherhein · 2018-04-07T02:23:29Z

Why:

Allows you to use the heptio-authenticator-aws binary in your ~/.kube/config as an external ExecCredential

Usage:

users:
- name: user
  user:
    exec:
      apiVersion: client.authentication.k8s.io/v1alpha1
      command: heptio-authenticator-aws
      args:
        - "token"
        - "--cluster-id"
        - "cluster-id"

This change addresses the need by:

closes Support kubectl external credential provider #64

Todos:

Documentation for usage on the readme.md
Add error handling to the token response (not sure if this is possible or needed)

Questions:

Do we need to implement a check for KUBERNETES_EXEC_INFO?
Do we need the expirationTimestamp it's optional per the documentation, if so could you provide me a little guidance on where to make or get that from?

https://kubernetes.io/docs/admin/authentication/#client-go-credential-plugins

Signed-off-by: Christopher Hein [email protected]

nckturner · 2018-04-08T22:08:18Z

Can you separate vendor into a separate commit to make this more reviewable?

**Why:** * Allows you to use the `heptio-authenticator-aws` binary in your `~/.kube/config` as an external authProvider **Usage:** ```yaml users: - name: user user: exec: apiVersion: client.authentication.k8s.io/v1alpha1 command: heptio-authenticator-aws args: - "token" - "--cluster-id" - "cluster-id" ``` **This change addresses the need by:** * closes #64 Signed-off-by: Christopher Hein <[email protected]>

**Why:** * Separated this commit to make reviewing easier Signed-off-by: Christopher Hein <[email protected]>

christopherhein · 2018-04-09T23:30:36Z

@nckturner Updated with commits separated, I should have error handling done soon to review that as well.

Also rebased master so it has the proper build mechanisms.

christopherhein · 2018-04-10T01:02:55Z

I was mistaken about the flow for the authenticator, I thought it passed a token to the API server that it validated after learning a little more it's passing a signed URL that the server uses to in essence callsts:getCallerIdentity with that being the flow I'm not sure how I can do error handling like defined - https://kubernetes.io/docs/admin/authentication/#input-and-output-formats

Any thoughts or suggestions?

christopherhein · 2018-04-10T01:25:29Z

cmd/heptio-authenticator-aws/token.go

@@ -35,7 +35,7 @@ var tokenCmd = &cobra.Command{
 		clusterID := viper.GetString("clusterID")

 		if clusterID == "" {
-			fmt.Fprintf(os.Stderr, "error: cluster ID not specified\n")
+			fmt.Fprintf(os.Stderr, "Error: cluster ID not specified\n")


Updated to be cohesive with the default Cobra error responses. heptio-authenticator-aws token -i for example returns:

Error: flag needs an argument: 'i' in -i

**Why:** * Documentes how to configured your `kubeconfig` to use `ExecCredential` `client-go` mechanisms. * Changes some of the documented comments to reflex `clientauth` vs 1.10 release Signed-off-by: Christopher Hein <[email protected]>

christopherhein · 2018-04-10T01:28:01Z

pkg/token/token.go

@@ -180,6 +184,21 @@ func (g generator) GetWithRole(clusterID string, roleARN string) (string, error)
 	return v1Prefix + base64.RawURLEncoding.EncodeToString([]byte(presignedURLString)), nil
 }

+// FormatJSON formats the json to support 1.10 external authProvider


Updated in a later commit.

I actually think this version is slightly more readable. The docs and implementation use different names in different places, so I'll leave it to you, but something like 1.10 client-go exec-based auth provider would be explicit enough.

Signed-off-by: Christopher Hein <[email protected]>

nckturner

Looking pretty good, a few comments. Also, you might need to consume KUBERNETES_EXEC_INFO in order to determine if the caller is interactive, or perhaps we can detect it, and we need to make sure interactive mode works with MFA.

nckturner · 2018-04-10T06:27:50Z

README.md

@@ -82,13 +82,26 @@ systemctl restart kubelet.service
 ```

 ### 4. Set up kubectl to use Heptio Authenticator for AWS tokens
+
+> This requires a 1.10+ `kubectl` binary to work. If you receive `Please enter Username:` when trying to use `kubectl` you need to update to the latest `kubectl`
+
 Finally, once the server is set up you'll want to authenticate!
 You will still need a `kubeconfig` that has the public data about your cluster (cluster CA certificate, endpoint address).
 The `users` section of your configuration, however, can be mostly blank:


Instead of:

can be mostly blank:

say:

should include an exec section (refer to the v1.10 docs):

nckturner · 2018-04-10T06:33:26Z

README.md

-To authenticate, run `kubectl --kubeconfig /path/to/kubeconfig --token "$(heptio-authenticator-aws token -i CLUSTER_ID -r ROLE_ARN)" [...]`.
-You can simplify this with an alias or shell wrapper.
+To authenticate, run `kubectl --kubeconfig /path/to/kubeconfig" [...]`.
+kubectl will handle `exec`'ing the `heptio-authenticator-aws` binary with the supplied params in your kubeconfig which will generate a token and pass it to the apiserver.


nit: kubectl will exec the ...

nckturner · 2018-04-10T06:38:40Z

pkg/token/token.go

@@ -180,6 +184,21 @@ func (g generator) GetWithRole(clusterID string, roleARN string) (string, error)
 	return v1Prefix + base64.RawURLEncoding.EncodeToString([]byte(presignedURLString)), nil
 }

+// FormatJSON formats the json to support 1.10 external authProvider


I actually think this version is slightly more readable. The docs and implementation use different names in different places, so I'll leave it to you, but something like 1.10 client-go exec-based auth provider would be explicit enough.

nckturner · 2018-04-10T06:41:07Z

cmd/heptio-authenticator-aws/token.go

@@ -58,7 +58,9 @@ var tokenCmd = &cobra.Command{
 			fmt.Fprintf(os.Stderr, "could not get token: %v\n", err)
 			os.Exit(1)
 		}
-		fmt.Println(tok)
+
+		enc := gen.FormatJSON(tok)


nit:

fmt.Println(gen.FormatJSON(tok))

Signed-off-by: Christopher Hein <[email protected]>

christopherhein · 2018-04-10T07:58:58Z

@nckturner updated based on your review. For the MFA, assuming we can track that separately through #42 right?

nckturner · 2018-04-12T06:59:23Z

I'd like to merge this after #42, I've proposed how we do that on that PR.

christopherhein changed the title ~~Adding support for kubectl execcommand output~~ Adding support for kubectl ExecCredential output Apr 7, 2018

christopherhein added 2 commits April 9, 2018 16:24

Adding Vendored Dependencies for ExecCredentials Flow

73dc4ff

**Why:** * Separated this commit to make reviewing easier Signed-off-by: Christopher Hein <[email protected]>

christopherhein commented Apr 10, 2018

View reviewed changes

Adding README.md Updates With the New Auth Flow

c66d898

**Why:** * Documentes how to configured your `kubeconfig` to use `ExecCredential` `client-go` mechanisms. * Changes some of the documented comments to reflex `clientauth` vs 1.10 release Signed-off-by: Christopher Hein <[email protected]>

christopherhein commented Apr 10, 2018

View reviewed changes

Updated dep format from newer version

83c4444

Signed-off-by: Christopher Hein <[email protected]>

nckturner requested changes Apr 10, 2018

View reviewed changes

Updates based on feedback

562cc3e

Signed-off-by: Christopher Hein <[email protected]>

nckturner approved these changes Apr 12, 2018

View reviewed changes

nckturner merged commit 3b5322c into kubernetes-sigs:master Apr 12, 2018

christopherhein deleted the feature/add-kubectl-output branch April 12, 2018 19:14

christopherhein mentioned this pull request Aug 21, 2018

REQUEST: New membership for @christopherhein kubernetes/org#24

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for kubectl ExecCredential output #72

Adding support for kubectl ExecCredential output #72

christopherhein commented Apr 7, 2018 •

edited

Loading

nckturner commented Apr 8, 2018

christopherhein commented Apr 9, 2018 •

edited

Loading

christopherhein commented Apr 10, 2018

christopherhein Apr 10, 2018

christopherhein Apr 10, 2018

nckturner Apr 10, 2018

nckturner left a comment

nckturner Apr 10, 2018

nckturner Apr 10, 2018

nckturner Apr 10, 2018

nckturner Apr 10, 2018

christopherhein commented Apr 10, 2018

nckturner commented Apr 12, 2018

Adding support for kubectl ExecCredential output #72

Adding support for kubectl ExecCredential output #72

Conversation

christopherhein commented Apr 7, 2018 • edited Loading

nckturner commented Apr 8, 2018

christopherhein commented Apr 9, 2018 • edited Loading

christopherhein commented Apr 10, 2018

christopherhein Apr 10, 2018

Choose a reason for hiding this comment

christopherhein Apr 10, 2018

Choose a reason for hiding this comment

nckturner Apr 10, 2018

Choose a reason for hiding this comment

nckturner left a comment

Choose a reason for hiding this comment

nckturner Apr 10, 2018

Choose a reason for hiding this comment

nckturner Apr 10, 2018

Choose a reason for hiding this comment

nckturner Apr 10, 2018

Choose a reason for hiding this comment

nckturner Apr 10, 2018

Choose a reason for hiding this comment

christopherhein commented Apr 10, 2018

nckturner commented Apr 12, 2018

christopherhein commented Apr 7, 2018 •

edited

Loading

christopherhein commented Apr 9, 2018 •

edited

Loading