Add support of client-go credential plugins in auth #396

dh-harald · 2019-04-12T09:18:48Z

Closes #161

bflad · 2019-04-24T11:20:35Z

This looks like this would close #161 so I have edited the PR description to automatically close it. I cannot provide a full review of this enhancement, but can provide some review of the Go/Terraform Provider SDK code. 👍

alexsomesan

Thanks for taking care of this issue. Change mostly looks good.
I've one suggestion about the type of structure used to model it. See below.

@bflad Happy to have your eyes on it too and please add any suggestions you might have.

alexsomesan · 2019-04-24T11:31:58Z

kubernetes/provider.go

@@ -101,6 +101,33 @@ func Provider() terraform.ResourceProvider {
 				DefaultFunc: schema.EnvDefaultFunc("KUBE_LOAD_CONFIG_FILE", true),
 				Description: "Load local kubeconfig.",
 			},
+			"exec": {
+				Type:     schema.TypeSet,


Can you please make this a TypeList instead?
I've found that in Terraform 0.12+ a TypeList works best for modeling a complex block, such as this.

bflad · 2019-04-24T11:21:44Z

kubernetes/provider.go

@@ -101,6 +101,33 @@ func Provider() terraform.ResourceProvider {
 				DefaultFunc: schema.EnvDefaultFunc("KUBE_LOAD_CONFIG_FILE", true),
 				Description: "Load local kubeconfig.",
 			},
+			"exec": {
+				Type:     schema.TypeSet,


With MaxItems: 1 configurations blocks, it is generally preferred to use Type: schema.TypeList to simplify handling. 👍

kubernetes/provider.go

bflad · 2019-04-24T11:35:25Z

website/docs/index.html.markdown

@@ -109,4 +109,4 @@ The following arguments are supported:
 * `config_context_cluster` - (Optional) Cluster context of the kube config (name of the kubeconfig cluster, `--cluster` flag in `kubectl`). Can be sourced from `KUBE_CTX_CLUSTER`.
 * `token` - (Optional) Token of your service account.  Can be sourced from `KUBE_TOKEN`.
 * `load_config_file` - (Optional) By default the local config (~/.kube/config) is loaded when you use this provider. This option at false disable this behaviour. Can be sourced from `KUBE_LOAD_CONFIG_FILE`.
-
+* `exec` - (Optional) Exec-based client auth provider (https://kubernetes.io/docs/reference/access-authn-authz/authentication/#client-go-credential-plugins)


Since we are adding a new configuration block with nested arguments, we should appropriately document everything here. 👍

Suggested change

* `exec` - (Optional) Exec-based client auth provider (https://kubernetes.io/docs/reference/access-authn-authz/authentication/#client-go-credential-plugins)

* `exec` - (Optional) Configuration block to use an [exec-based credential plugin](https://kubernetes.io/docs/reference/access-authn-authz/authentication/#client-go-credential-plugins), e.g. call an external command to receive user credentials.

* `api_version` - (Required) API version to use when decoding the ExecCredentials resource, e.g. `client.authentication.k8s.io/v1beta1`.

* `command` - (Required) Command to execute.

* `args` - (Optional) List of arguments to pass when executing the plugin.

* `env` - (Optional) Map of environment variables to set when executing the plugin.

alexsomesan · 2019-04-24T11:42:37Z

kubernetes/provider.go

@@ -181,6 +208,18 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) {
 		cfg.BearerToken = v.(string)
 	}

+	if v, ok := d.GetOk("exec"); ok {
+		exec := &clientcmdapi.ExecConfig{}
+		spec := v.(*schema.Set).List()[0].(map[string]interface{})


Also, would you mind breaking down this line into multiple steps and check for errors and nils on the type assertions. This looks like it has the potential to panic.

In theory, it should pass by the schema checker, but I've added an extra check to avoid the panic

alexsomesan

LGTM

chancez · 2020-02-21T01:58:41Z

kubernetes/provider.go

@@ -101,6 +101,34 @@ func Provider() terraform.ResourceProvider {
 				DefaultFunc: schema.EnvDefaultFunc("KUBE_LOAD_CONFIG_FILE", true),
 				Description: "Load local kubeconfig.",
 			},
+			"exec": {
+				Type:     schema.TypeList,


Why wasn't this done as TypeMap?

chancez · 2020-02-21T01:59:36Z

Is this in a release? The code is in available at least in 1.10.0, but I'm getting:

Error: Unsupported argument

  on ../../modules/eks-cluster/main.tf line 70, in provider "kubernetes":
  70:   exec = [{

An argument named "exec" is not expected here. Did you mean to define a block
of type "exec"?

chancez · 2020-02-21T20:43:37Z

Ah, it's a block. I see.

  exec {
    api_version = "client.authentication.k8s.io/v1alpha1"
    command     = "aws-iam-authenticator"
    args        = concat(["token", "-i", data.aws_eks_cluster.cluster.name], local.authenticator_additional_args)
    env         = local.authenticator_env_vars
  }

ballad89 · 2020-04-13T15:23:48Z

@chancez did the block above work for you ? what versions are you using ?

$ terraform version                                                                                         ✔  5506  17:22:08
Terraform v0.12.24
+ provider.aws v2.57.0
+ provider.external v1.2.0
+ provider.helm v1.1.1
+ provider.kubernetes v1.11.1
+ provider.local v1.4.0
+ provider.null v2.1.2
+ provider.random v2.2.1
+ provider.template v2.1.2

chancez · 2020-04-13T18:29:22Z

It did.

$ terraform version
Terraform v0.12.24
+ provider.aws v2.54.0
+ provider.kubernetes v1.10.0
+ provider.local v1.4.0
+ provider.null v2.1.2
+ provider.random v2.2.1
+ provider.template v2.1.2

ghost · 2020-04-21T05:10:09Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

ghost added size/S documentation labels Apr 12, 2019

bflad added the enhancement label Apr 24, 2019

alexsomesan reviewed Apr 24, 2019

View reviewed changes

bflad reviewed Apr 24, 2019

View reviewed changes

alexsomesan reviewed Apr 24, 2019

View reviewed changes

bflad mentioned this pull request Apr 24, 2019

Feature Request: Support exec Authentication (client-go Credential Plugins) #161

Closed

dh-harald force-pushed the master branch from 85b0b37 to 017aea8 Compare April 25, 2019 12:26

Add support of client-go credential plugins in auth

bbef51f

dh-harald force-pushed the master branch from 017aea8 to bbef51f Compare April 25, 2019 12:49

alexsomesan approved these changes May 9, 2019

View reviewed changes

alexsomesan merged commit 64973a4 into hashicorp:master May 9, 2019

alexsomesan pushed a commit to chanzuckerberg/terraform-provider-kubernetes that referenced this pull request May 10, 2019

Add support of client-go credential plugins in auth (hashicorp#396)

9b66bec

chancez reviewed Feb 21, 2020

View reviewed changes

ghost locked and limited conversation to collaborators Apr 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support of client-go credential plugins in auth #396

Add support of client-go credential plugins in auth #396

dh-harald commented Apr 12, 2019 •

edited by bflad

Loading

bflad commented Apr 24, 2019

alexsomesan left a comment

alexsomesan Apr 24, 2019

bflad Apr 24, 2019

bflad Apr 24, 2019

alexsomesan Apr 24, 2019

dh-harald Apr 25, 2019 •

edited

Loading

alexsomesan left a comment

chancez Feb 21, 2020

chancez commented Feb 21, 2020

chancez commented Feb 21, 2020

ballad89 commented Apr 13, 2020

chancez commented Apr 13, 2020

ghost commented Apr 21, 2020

-* `exec` - (Optional) Exec-based client auth provider (https://kubernetes.io/docs/reference/access-authn-authz/authentication/#client-go-credential-plugins)
+* `exec` - (Optional) Configuration block to use an [exec-based credential plugin](https://kubernetes.io/docs/reference/access-authn-authz/authentication/#client-go-credential-plugins), e.g. call an external command to receive user credentials.
+  * `api_version` - (Required) API version to use when decoding the ExecCredentials resource, e.g. `client.authentication.k8s.io/v1beta1`.
+  * `command` - (Required) Command to execute.
+  * `args` - (Optional) List of arguments to pass when executing the plugin.
+  * `env` - (Optional) Map of environment variables to set when executing the plugin.

Add support of client-go credential plugins in auth #396

Add support of client-go credential plugins in auth #396

Conversation

dh-harald commented Apr 12, 2019 • edited by bflad Loading

bflad commented Apr 24, 2019

alexsomesan left a comment

Choose a reason for hiding this comment

alexsomesan Apr 24, 2019

Choose a reason for hiding this comment

bflad Apr 24, 2019

Choose a reason for hiding this comment

bflad Apr 24, 2019

Choose a reason for hiding this comment

alexsomesan Apr 24, 2019

Choose a reason for hiding this comment

dh-harald Apr 25, 2019 • edited Loading

Choose a reason for hiding this comment

alexsomesan left a comment

Choose a reason for hiding this comment

chancez Feb 21, 2020

Choose a reason for hiding this comment

chancez commented Feb 21, 2020

chancez commented Feb 21, 2020

ballad89 commented Apr 13, 2020

chancez commented Apr 13, 2020

ghost commented Apr 21, 2020

dh-harald commented Apr 12, 2019 •

edited by bflad

Loading

dh-harald Apr 25, 2019 •

edited

Loading