GCP config: gke-exec-auth-plugin for ValidatingAdmissionWebhook #79553

ahmedtd · 2019-06-29T01:30:44Z

This commit adds support for using gke-exec-auth-plugin for webhooks when calling endpoints matching *.googleapis.com, and integrates this support with ValidatingAdmissionWebhook.

To enable it, request ValidatingAdmissionWebhook with ADMISSION_CONTROL=...,ValidatingAdmissionWebhook,... (default) and opt in to gke-exec-auth-plugin using WEBHOOK_GKE_EXEC_AUTH=true during the configuration process.

If you don't opt-in, ValidatingAdmissionWebhook will be deployed as before.

Requesting WEBHOOK_GKE_EXEC_AUTH=true will fail if you have not provided other configuration variables:

EXEC_AUTH_PLUGIN_URL: controls whether gke-exec-auth-plugin is downloaded during the installation step. A prerequisite for actually using the plugin.
TOKEN_URL, TOKEN_BODY, and TOKEN_BODY_UNQUOTED: configuration values used when calling the plugin. TOKEN_URL and TOKEN_BODY have existing usage.
TOKEN_BODY_UNQUOTED is a new variable that is meant to sidestep the problem of inverting strconv.Quote in Bash.

The existing configuration process for ImagePolicyWebhook has been reworked to make it play nicely with ValidatingAdmissionWebhook under WEBHOOK_GKE_EXEC_AUTH=true:

It originally placed the ImagePolicyWebhook configuration object at the top-level of the file specified by --admission-control-config-file. I can't see why this worked; it must have been hitting some sort of lucky path through the various config file loading mechanisms. Now, it places its configuration in a sub-field of that file, which is shared among all admission control plugins.
It mounted its various config files read-write. I reviewed the code and couldn't see why it was necessary, so I moved the config files into the existing read-only mount at /etc/srv/kubernetes.
It now checks that all the configuration values it requires have been provided, rather than silently writing out a broken config.

/kind feature

NONE

k8s-ci-robot · 2019-06-29T01:30:51Z

Hi @ahmedtd. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ahmedtd · 2019-07-02T02:19:49Z

/assign @MrHohn @yguo0905

ahmedtd · 2019-07-02T05:39:27Z

/retest

cluster/gce/gci/configure-helper.sh

MrHohn · 2019-07-03T00:16:39Z

cluster/gce/gci/configure-helper.sh

+plugins:
+EOF
+
+    if [[ ${ADMISSION_CONTROL:-} == *"ImagePolicyWebhook"* ]]; then


Maybe worth to log when a admission control config is enabled?

I added some log messages, but these blocks aren't enabling the admission control plugins. The admission control plugins are enabled by inclusion in ADMISSION_CONTROL, which get included in the apiserver command line around line 1801 of configure-helper.sh.

These blocks are just "noticing" that the caller requested admission control plugins that might require special setup.

Right, what you describe is more accurate. Thanks for the logs.

MrHohn · 2019-07-03T00:27:42Z

cluster/gce/gci/configure-helper.sh

+      # kubeconfig to be used by webhooks with GKE exec auth support.  Note that
+      # the path to gke-exec-auth-plugin is the path when mounted inside the
+      # kube-apiserver pod.
+      cat <<EOF >/etc/srv/kubernetes/webhook_kubeconfig.yaml


nit: make this end with .kubeconfig to be consistent?

MrHohn · 2019-07-03T00:30:28Z

cluster/gce/gci/configure-helper.sh

-if [[ -n "${GCP_IMAGE_VERIFICATION_URL:-}" ]]; then
-    # This is the config file for the image review webhook.
-    cat <<EOF >/etc/gcp_image_review.config
+   if [[ ${WEBHOOK_GKE_EXEC_AUTH:-} ]]; then


Indent mismatch?

MrHohn · 2019-07-03T00:45:21Z

cluster/gce/gci/configure-helper.sh

+      # kubeconfig to be used by webhooks with GKE exec auth support.  Note that
+      # the path to gke-exec-auth-plugin is the path when mounted inside the
+      # kube-apiserver pod.
+      cat <<EOF >/etc/srv/kubernetes/webhook_kubeconfig.yaml


Would this kubeconfig be ever needed by something other than the exec auth admission plugin? If not, should it fall under the same if [[ ${ADMISSION_CONTROL:-} == *"ImagePolicyWebhook"* ]] bucket?

webhook.kubeconfig should be usable for any webhook that needs to call to *.googleapis.com. For example, MutatingAdmissionWebhook.

I didn't want to go through and add the kubeconfig to a bunch of other webhooks that we're not planning on exercising regularly --- ValidatingAdmissionWebhook will be using this kubeconfig as part of our binary authorization feature.

MrHohn · 2019-07-03T01:20:29Z

cluster/gce/gci/configure-helper.sh

+  # gke-exec-auth-plugin needs to be mounted into the kube-apiserver container.
+  local webhook_exec_auth_plugin_mount=""
+  local webhook_exec_auth_plugin_volume=""
+  if [[ ${WEBHOOK_GKE_EXEC_AUTH:-} ]]; then


It is a bit concerning that the logic for exec webhook are scattered among a couple places. Is there a specific reason the mount/volume setup has to be placed here?

Here I was mostly following the existing organization of the file.

Note that this block isn't doing mounts for use by gke-exec-auth-plugin, rather it's mounting gke-exec-auth-plugin into the kube-apiserver container (so that the various webhooks can execute it). To do so, it needs to modify the kube-apiserver's static pod manifest, which is what this function is all about.

Makes sense, thanks for the explanation :)

ahmedtd · 2019-07-03T20:08:08Z

/retest

MrHohn

/lgtm

MrHohn · 2019-07-03T22:01:13Z

cluster/gce/gci/configure-helper.sh

+plugins:
+EOF
+
+    if [[ ${ADMISSION_CONTROL:-} == *"ImagePolicyWebhook"* ]]; then


Right, what you describe is more accurate. Thanks for the logs.

MrHohn · 2019-07-03T23:19:49Z

cluster/gce/gci/configure-helper.sh

+  # gke-exec-auth-plugin needs to be mounted into the kube-apiserver container.
+  local webhook_exec_auth_plugin_mount=""
+  local webhook_exec_auth_plugin_volume=""
+  if [[ ${WEBHOOK_GKE_EXEC_AUTH:-} ]]; then


Makes sense, thanks for the explanation :)

ahmedtd · 2019-07-05T01:11:41Z

/test pull-kubernetes-kubemark-e2e-gce-big

ahmedtd · 2019-07-05T06:27:31Z

/test pull-kubernetes-node-e2e-containerd

ahmedtd · 2019-07-08T21:07:18Z

/unassign @yguo0905
/assign @mikedanese

mikedanese · 2019-07-11T20:36:05Z

cluster/gce/gci/configure-helper.sh

+    fi
+
+    if [[ -z ${TOKEN_URL:-} || -z ${TOKEN_BODY:-} || -z ${TOKEN_BODY_UNQUOTED:-} ]]; then
+      echo "You requested GKE exec auth support for webhooks, but TOKEN_URL, TOKEN_BODY, and TOKEN_BODY_UNQUOTED were not provided.  gke-exec-auth-plugin requires these values for its configuration."


These should be 1>&2 echo "....", but this file isn't consistent.

mikedanese · 2019-07-11T20:37:27Z

cluster/gce/gci/configure-helper.sh

+      args:
+      - --mode=alt-token
+      - --alt-token-url=${TOKEN_URL?}
+      - --alt-token-body=${TOKEN_BODY_UNQUOTED?}


Side note: we set -o nounset so the ? is the default behavior for parameter expansion, although no problem with being defensive.

mikedanese · 2019-07-11T20:37:29Z

cluster/gce/gci/configure-helper.sh

-if [[ -n "${GCP_IMAGE_VERIFICATION_URL:-}" ]]; then
-    # This is the config file for the image review webhook.
-    cat <<EOF >/etc/gcp_image_review.config
+  if [[ ${WEBHOOK_GKE_EXEC_AUTH:-} ]]; then


Wrap parameter expansions in quotes here and below. Be explicit about what the condition is checking a -n.

mikedanese · 2019-07-11T20:45:02Z

Removed lgtm for you to get a chance to look at the comments.
/approve

k8s-ci-robot · 2019-07-11T20:45:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahmedtd, mikedanese

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~cluster/gce/gci/OWNERS~~ [mikedanese]
~~cluster/gce/manifests/OWNERS~~ [mikedanese]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

This commit adds support for using `gke-exec-auth-plugin` (vTPM-based certificates for mTLS) for webhooks when calling endpoints matching `*.googleapis.com`, and integrates this support with ValidatingAdmissionWebhook. To enable it, request ValidatingAdmissionWebhook with `ADMISSION_CONTROL=...,ValidatingAdmissionWebhook,...` (default) and opt in to `gke-exec-auth-plugin` using `WEBHOOK_GKE_EXEC_AUTH=true` during the configuration process. If you don't opt-in, ValidatingAdmissionWebhook will be deployed as before. Requesting `WEBHOOK_GKE_EXEC_AUTH=true` will fail if you have not provided other configuration variables: * `EXEC_AUTH_PLUGIN_URL`: controls whether `gke-exec-auth-plugin` is downloaded during the installation step. A prerequisite for actually using the plugin. * `TOKEN_URL`, `TOKEN_BODY`, and `TOKEN_BODY_UNQUOTED`: configuration values used when calling the plugin. `TOKEN_URL` and `TOKEN_BODY` have existing usage. `TOKEN_BODY_UNQUOTED` is a new variable that is meant to sidestep the problem of inverting `strconv.Quote` in Bash. The existing configuration process for ImagePolicyWebhook has been reworked to make it play nicely with ValidatingAdmissionWebhook under `WEBHOOK_GKE_EXEC_AUTH=true`. * It originally placed the ImagePolicyWebhook configuration object at the top-level of the file specified by `--admission-control-config-file`. I can't see why this worked; it must have been hitting some sort of lucky path through the various config file loading mechanisms. Now, it places its configuration in a sub-field of that file, which is shared among all admission control plugins. * It mounted its various config files read-write. I reviewed the code and couldn't see why it was necessary, so I moved the config files into the existing read-only mount at `/etc/srv/kubernetes`. * It now checks that all the configuration values it requires have been provided. Co-authored-by: Mike Danese <mikedanese@google.com> Co-authored-by: Taahir Ahmed <taahm@google.com>

mikedanese · 2019-07-23T04:00:13Z

/lgtm

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 29, 2019

k8s-ci-robot requested review from MaciekPytel and MrHohn June 29, 2019 01:31

k8s-ci-robot added sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/gcp and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jun 29, 2019

ahmedtd force-pushed the binauthz branch 8 times, most recently from 3977fab to 7587265 Compare June 30, 2019 08:34

ahmedtd changed the title ~~GCP config: Support AdmissionWebhook plugin~~ GCP config: gke-exec-auth-plugin for ValidatingAdmissionWebhook Jun 30, 2019

ahmedtd force-pushed the binauthz branch 4 times, most recently from da55df9 to f6e47b5 Compare July 2, 2019 01:48

k8s-ci-robot assigned MrHohn and yguo0905 Jul 2, 2019

ahmedtd force-pushed the binauthz branch from f6e47b5 to c0172ed Compare July 2, 2019 05:38

ahmedtd force-pushed the binauthz branch from c0172ed to 6b6b42f Compare July 2, 2019 05:56

MrHohn reviewed Jul 3, 2019

View reviewed changes

ahmedtd force-pushed the binauthz branch from 6b6b42f to edaf129 Compare July 3, 2019 20:07

MrHohn approved these changes Jul 3, 2019

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 3, 2019

k8s-ci-robot assigned mikedanese and unassigned yguo0905 Jul 8, 2019

mikedanese reviewed Jul 11, 2019

View reviewed changes

mikedanese added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Jul 11, 2019

k8s-ci-robot removed the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Jul 11, 2019

mikedanese added needs-priority Indicates a PR lacks a `priority/foo` label and requires one. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jul 11, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 11, 2019

ahmedtd force-pushed the binauthz branch from edaf129 to 8d1b9a6 Compare July 22, 2019 22:50

ahmedtd force-pushed the binauthz branch from 8d1b9a6 to 9702c6e Compare July 22, 2019 23:01

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 23, 2019

k8s-ci-robot merged commit 5381bf5 into kubernetes:master Jul 23, 2019

ahmedtd deleted the binauthz branch August 28, 2019 21:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GCP config: gke-exec-auth-plugin for ValidatingAdmissionWebhook #79553

GCP config: gke-exec-auth-plugin for ValidatingAdmissionWebhook #79553

ahmedtd commented Jun 29, 2019 •

edited

k8s-ci-robot commented Jun 29, 2019

ahmedtd commented Jul 2, 2019

ahmedtd commented Jul 2, 2019

MrHohn Jul 3, 2019

ahmedtd Jul 3, 2019

MrHohn Jul 3, 2019

MrHohn Jul 3, 2019

ahmedtd Jul 3, 2019

MrHohn Jul 3, 2019

ahmedtd Jul 3, 2019

MrHohn Jul 3, 2019

ahmedtd Jul 3, 2019

MrHohn Jul 3, 2019

ahmedtd Jul 3, 2019

MrHohn Jul 3, 2019

ahmedtd commented Jul 3, 2019

MrHohn left a comment

MrHohn Jul 3, 2019

MrHohn Jul 3, 2019

ahmedtd commented Jul 5, 2019

ahmedtd commented Jul 5, 2019

ahmedtd commented Jul 8, 2019

mikedanese Jul 11, 2019

ahmedtd Jul 22, 2019

mikedanese Jul 11, 2019

ahmedtd Jul 22, 2019

mikedanese Jul 11, 2019

ahmedtd Jul 22, 2019

mikedanese commented Jul 11, 2019

k8s-ci-robot commented Jul 11, 2019

mikedanese commented Jul 23, 2019

GCP config: gke-exec-auth-plugin for ValidatingAdmissionWebhook #79553

GCP config: gke-exec-auth-plugin for ValidatingAdmissionWebhook #79553

Conversation

ahmedtd commented Jun 29, 2019 • edited

k8s-ci-robot commented Jun 29, 2019

ahmedtd commented Jul 2, 2019

ahmedtd commented Jul 2, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahmedtd commented Jul 3, 2019

MrHohn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahmedtd commented Jul 5, 2019

ahmedtd commented Jul 5, 2019

ahmedtd commented Jul 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikedanese commented Jul 11, 2019

k8s-ci-robot commented Jul 11, 2019

mikedanese commented Jul 23, 2019

ahmedtd commented Jun 29, 2019 •

edited