enable kubelet csr bootstrap in GCE/GKE #40760

mikedanese · 2017-01-31T21:13:36Z

Enable kubelet csr bootstrap in GCE/GKE

k8s-reviewable · 2017-01-31T21:14:42Z

This change is

mikedanese · 2017-01-31T21:15:19Z

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go

+			ObjectMeta: metav1.ObjectMeta{Name: "system:kube-csr"},
+			Rules: []rbac.PolicyRule{
+				eventsRule(),
+				rbac.NewRule("get", "create", "list", "watch").Groups(certificatesGroup).Resources("certificatesigningrequests").RuleOrDie(),


Not sure what do about the RBAC permissions for creating and reading CSRs. Is it ok to create a system role for this? @liggitt @cjcullen

We could alternatively give this permission to user.AllAuthenticated instead of creating a new group.

there's already a role: system:node-bootstrapper (added in https://github.com/kubernetes/kubernetes/pull/40034/files)

it is not bound to any users by default.

I'd expect the thing starting the cluster and setting up the bootstrap credential to grant this role to that user or group

node-bootstrapper is a pretty bad name for a permission that I hope will be used more broadley than node-bootstrapping. Can we change it to something like csrequestor and bind it to all authenticated users or a csrquestor group?

node-bootstrapper is a pretty bad name for a permission that I hope will be used more broadley than node-bootstrapping.

I anticipate that there might be additional permissions in the future we'd want to add to the node bootstrapper role (like being able to get a node to see if there is already a record for itself, etc), so I'd rather give the bootstrapper role to the bootstrap credential for now.

Can we change it to something like csrequestor

If we want an additional separate role that is limited in purpose to requesting a certificate, that could make sense as well

bind it to all authenticated users or a csrquestor group?

I wouldn't expose cert requests to all authenticated users or a fixed group yet...

Ok, I see that node-bootstrapper also has node get permissions...

I will use node-bootstrapper role for now then.

In the longer term we want this thing to not be able to get nodes, right?

mikedanese · 2017-01-31T21:16:43Z

cluster/gce/config-default.sh

@@ -38,7 +38,7 @@ KUBE_DELETE_NODES=${KUBE_DELETE_NODES:-true}
 KUBE_DELETE_NETWORK=${KUBE_DELETE_NETWORK:-false}

 MASTER_OS_DISTRIBUTION=${KUBE_MASTER_OS_DISTRIBUTION:-${KUBE_OS_DISTRIBUTION:-gci}}
-NODE_OS_DISTRIBUTION=${KUBE_NODE_OS_DISTRIBUTION:-${KUBE_OS_DISTRIBUTION:-debian}}
+NODE_OS_DISTRIBUTION=${KUBE_NODE_OS_DISTRIBUTION:-${KUBE_OS_DISTRIBUTION:-gci}}


Will revert.

deads2k · 2017-01-31T21:33:43Z

@liggitt I suspect you're closer to this than I am. If you don't get to it, I'll look tomorrow AM.

liggitt · 2017-02-02T00:47:51Z

cluster/addons/rbac/node-bootstrap-binding.yaml

+  name: system:node-bootstrapper
+- apiVersion: rbac/v1beta1
+  kind: Group
+  name: system:node


Was this supposed to be system:nodes? Once a node has a credential in this group, isn't it done bootstrapping?

@jcbsmpsn is working on implementing rotation that will use the kubelet's certificate as credentials. I can omit this until it is impelmented.

if we expect nodes to request certs as themselves (for rotation, or for requesting serving certs), let's add the csr permissions to the system:node role as well

liggitt · 2017-02-02T00:48:18Z

cluster/addons/rbac/node-bootstrap-binding.yaml

+subjects:
+- apiVersion: rbac/v1beta1
+  kind: Group
+  name: system:node-bootstrapper


What creates this group? Under what circumstances are the RBAC addons created?

@liggitt the addon manager creates this and it will be enabled in GCE kube-up.sh deployments only. It will be a enabled in all GCE clusters

sounds reasonable for GCE, but I would avoid the system: prefix on the bootstrap user/group name if it's not something we intend to standardize

liggitt · 2017-02-02T00:49:19Z

cluster/gce/gci/configure-helper.sh

@@ -1099,9 +1130,13 @@ function start-kube-addons {
  local -r src_dir="${KUBE_HOME}/kube-manifests/kubernetes/gci-trusty"
  local -r dst_dir="/etc/kubernetes/addons"

+  # TODO(mikedanese): only enable these in e2e
  # prep the additional bindings that are particular to e2e users and groups
  setup-addon-manifests "addons" "e2e-rbac-bindings"


It's possible some of these are no longer needed, actually

liggitt · 2017-02-02T00:54:25Z

@cjcullen: In the longer term we want this thing to not be able to get nodes, right?

I think so, yes. Originally it was just CSR permissions. kubeadm does an early get of the anticipated node during bootstrap, but on reflection I think that is incorrect. @luxas, I'll open a PR to remove that usage and tighten this role back to CSR only.

I do anticipate using the authorizer to replace the hard coded autoapprove group (do an authorization check on the subject that requested a node client cert to see if they are allowed to join nodes)

liggitt · 2017-02-02T18:41:33Z

cluster/gce/gci/configure-helper.sh

@@ -239,6 +240,10 @@ function create-master-auth {
  if [[ -n "${KUBE_PROXY_TOKEN:-}" ]]; then
    replace_prefixed_line "${known_tokens_csv}" "${KUBE_PROXY_TOKEN},"              "system:kube-proxy,uid:kube_proxy"
  fi
+  if [[ -n "${KUBE_CSR_TOKEN:-}" ]]; then
+    replace_prefixed_line "${known_tokens_csv}" "${KUBE_CSR_TOKEN},"                "system:node-bootstrap-secret,uid:node-bootstrap-secret,system:node-bootstrapper"


if this isn't a username or group we intend to standardize (and I don't think it is), then omit the system: prefix... it's GCE-specific, like the admin username above.

liggitt · 2017-02-02T18:51:32Z

cluster/addons/rbac/node-bootstrap-binding.yaml

+subjects:
+- apiVersion: rbac/v1beta1
+  kind: Group
+  name: system:node-bootstrapper


heh, other way around. role is standard (system:...), particular group used by kube-up.sh is not.

Gaah, thanks.

liggitt · 2017-02-02T18:55:12Z

cluster/gce/configure-vm.sh

@@ -698,7 +698,8 @@ function create-salt-master-auth() {
    (umask 077;
      echo "${KUBE_BEARER_TOKEN},admin,admin" > "${KNOWN_TOKENS_FILE}";
      echo "${KUBELET_TOKEN},kubelet,kubelet" >> "${KNOWN_TOKENS_FILE}";
-      echo "${KUBE_PROXY_TOKEN},kube_proxy,kube_proxy" >> "${KNOWN_TOKENS_FILE}")
+      echo "${KUBE_PROXY_TOKEN},kube_proxy,kube_proxy" >> "${KNOWN_TOKENS_FILE}";
+      echo "${KUBE_CSR_TOKEN},kube-csr,kube-csr" >> "${KNOWN_TOKENS_FILE}")


this will make the username kube-csr... is that right?

Nope. This change is not necessary, removed.

liggitt · 2017-05-31T16:16:57Z

cluster/addons/rbac/kubelet-certificate-management.yaml

+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: beta:gce:kubelet-certificate-rotation


this role isn't defined, right?

liggitt · 2017-05-31T17:18:43Z

this LGTM, just the one question about managed files in salt, and the note that letting nodes renew their own credentials automatically makes revoking a node's credentials a race (though without CRL support, revoking cert credentials is largely theoretical at the moment). I'm ok iterating on this.

mikedanese · 2017-06-01T17:15:23Z

@liggitt can you review and approve the change to bootstrap controller policy I needed to add to allow the SAR approver to create subject access reviews?

liggitt · 2017-06-01T17:37:49Z

policy change LGTM

liggitt · 2017-06-01T17:38:51Z

/approve

cjcullen · 2017-06-01T17:39:32Z

/lgtm

k8s-github-robot · 2017-06-01T17:45:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cjcullen, falenn, liggitt, mikedanese

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

Needs approval from an approver in each of these OWNERS Files:

~~cluster/OWNERS~~ [mikedanese]
~~plugin/pkg/auth/OWNERS~~ [liggitt]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

mikedanese · 2017-06-03T21:30:46Z

@k8s-bot pull-kubernetes-e2e-gce-etcd3 test this

k8s-github-robot · 2017-06-03T23:52:45Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

liggitt · 2017-06-04T00:54:27Z

@k8s-bot pull-kubernetes-federation-e2e-gce test this
@k8s-bot pull-kubernetes-kubemark-e2e-gce test this

k8s-github-robot · 2017-06-04T01:30:38Z

Automatic merge from submit-queue (batch tested with PRs 40760, 46706, 46783, 46742, 46751)

k8s-ci-robot · 2017-06-04T01:39:40Z

@mikedanese: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-federation-e2e-gce	`f533bf7`	link	`@k8s-bot pull-kubernetes-federation-e2e-gce test this`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 31, 2017

k8s-github-robot assigned zmerlynn Jan 31, 2017

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Jan 31, 2017

mikedanese added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Jan 31, 2017

mikedanese changed the title ~~enable kubelet csr bootstrap in GCE.~~ enable kubelet csr bootstrap in GCE GCI. Jan 31, 2017

mikedanese commented Jan 31, 2017

View reviewed changes

mikedanese force-pushed the gce branch from 094803c to f1cc907 Compare February 1, 2017 22:30

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 1, 2017

mikedanese force-pushed the gce branch 2 times, most recently from 6aa76e1 to d7fc8de Compare February 1, 2017 23:43

k8s-github-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 1, 2017

mikedanese force-pushed the gce branch from d7fc8de to 9c18941 Compare February 2, 2017 00:14

liggitt reviewed Feb 2, 2017

View reviewed changes

liggitt mentioned this pull request Feb 2, 2017

Remove 'get node' call during bootstrapping #40873

Merged

liggitt reviewed Feb 2, 2017

View reviewed changes

mikedanese force-pushed the gce branch 4 times, most recently from fca9ccc to 1344778 Compare February 2, 2017 18:46

liggitt reviewed Feb 2, 2017

View reviewed changes

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 2, 2017

mikedanese force-pushed the gce branch 2 times, most recently from b393102 to f76630a Compare February 2, 2017 18:57

liggitt reviewed May 31, 2017

View reviewed changes

mikedanese force-pushed the gce branch 3 times, most recently from f123d3c to 97d7c37 Compare May 31, 2017 17:15

mikedanese changed the title ~~enable kubelet csr bootstrap in GCE GCI.~~ enable kubelet csr bootstrap in GCE/GKE May 31, 2017

mikedanese assigned cjcullen May 31, 2017

enable tls bootstrap in GCE/GKE

ae91ecb

mikedanese force-pushed the gce branch from 97d7c37 to 86ceaf5 Compare June 1, 2017 16:18

k8s-github-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 1, 2017

allow sarapprover to create subject access reviews

f533bf7

mikedanese force-pushed the gce branch from 86ceaf5 to f533bf7 Compare June 1, 2017 16:43

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 1, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 1, 2017

liggitt mentioned this pull request Jun 2, 2017

promote tls-bootstrap to beta #46799

Merged

mikedanese added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jun 3, 2017

k8s-github-robot merged commit 4c7e159 into kubernetes:master Jun 4, 2017

mikedanese deleted the gce branch June 4, 2017 05:11

foxish mentioned this pull request Jun 21, 2017

Upgrade test cluster inconsistency #47882

Closed

enable kubelet csr bootstrap in GCE/GKE #40760

enable kubelet csr bootstrap in GCE/GKE #40760

Conversation

mikedanese commented Jan 31, 2017 • edited Loading

k8s-reviewable commented Jan 31, 2017

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

deads2k commented Jan 31, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt Feb 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikedanese Feb 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt commented Feb 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt Feb 2, 2017 • edited Loading

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

liggitt commented May 31, 2017 • edited Loading

mikedanese commented Jun 1, 2017 • edited Loading

liggitt commented Jun 1, 2017

liggitt commented Jun 1, 2017

cjcullen commented Jun 1, 2017

k8s-github-robot commented Jun 1, 2017

mikedanese commented Jun 3, 2017

k8s-github-robot commented Jun 3, 2017

liggitt commented Jun 4, 2017

k8s-github-robot commented Jun 4, 2017

k8s-ci-robot commented Jun 4, 2017

mikedanese commented Jan 31, 2017 •

edited

Loading

liggitt Feb 2, 2017 •

edited

Loading

mikedanese Feb 2, 2017 •

edited

Loading

liggitt Feb 2, 2017 •

edited

Loading

liggitt commented May 31, 2017 •

edited

Loading

mikedanese commented Jun 1, 2017 •

edited

Loading