enable Node authorizer and NodeRestriction admission controller #46796

mikedanese · 2017-06-01T19:59:37Z

gce kube-up: The `Node` authorization mode and `NodeRestriction` admission controller are now enabled

luxas · 2017-06-01T20:10:14Z

@mikedanese BTW, have we enabled the Node Authorizer in kubeadm?

liggitt · 2017-06-01T20:19:10Z

pkg/auth/nodeidentifier/default.go

-	nodeName := ""
-	if strings.HasPrefix(userName, nodeUserNamePrefix) {
-		nodeName = strings.TrimPrefix(userName, nodeUserNamePrefix)
+	if !strings.HasPrefix(userName, nodeUserNamePrefix) {


update godoc to match and move the username check before the loop over the groups

also, this'll require updating unit tests

liggitt · 2017-06-01T20:20:45Z

cluster/gce/gci/configure-helper.sh

@@ -1123,7 +1123,7 @@ function start-kube-apiserver {
  fi


-  local authorization_mode="RBAC"
+  local authorization_mode="RBAC,Node"


I would probably put the Node authorizer first... it does almost zero work for requests that are not from nodes, and is faster for requests that are

liggitt · 2017-06-01T20:28:21Z

cluster/gce/config-default.sh

@@ -217,7 +217,7 @@ fi

 # Admission Controllers to invoke prior to persisting objects in cluster
 # If we included ResourceQuota, we should keep it at the end of the list to prevent incrementing quota usage prematurely.
-ADMISSION_CONTROL=NamespaceLifecycle,LimitRanger,ServiceAccount,PersistentVolumeLabel,DefaultStorageClass,DefaultTolerationSeconds,ResourceQuota
+ADMISSION_CONTROL=NamespaceLifecycle,LimitRanger,ServiceAccount,PersistentVolumeLabel,DefaultStorageClass,DefaultTolerationSeconds,NodeRestriction,ResourceQuota


does this apply to all the gce clusters? what about ubuntu, container-linux, etc?

I guess with the change to treat things without a system:node:... formatted name as non-nodes, this is safe

liggitt · 2017-06-01T21:03:20Z

/release-note-required

liggitt · 2017-06-01T21:05:27Z

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

@@ -277,6 +277,7 @@ func init() {
 		Rules: []rbac.PolicyRule{
 			rbac.NewRule("get", "list", "watch").Groups(certificatesGroup).Resources("certificatesigningrequests").RuleOrDie(),
 			rbac.NewRule("update").Groups(certificatesGroup).Resources("certificatesigningrequests/status", "certificatesigningrequests/approval").RuleOrDie(),
+			rbac.NewRule("create").Groups(authorizationGroup).Resources("subjectaccessreviews").RuleOrDie(),


needs bootstrap yaml updated (was correct in f533bf7)

luxas · 2017-06-12T20:24:18Z

Quota 'SUBNETWORKS' exceeded. Limit: 150.0 was the failure on gce-etcd3...

mikedanese · 2017-06-12T21:01:07Z

/retest

mikedanese · 2017-06-12T23:30:12Z

/retest

mikedanese · 2017-06-13T00:00:45Z

/retest

liggitt · 2017-06-13T01:37:06Z

cc @deads2k @ncdc @derekwaynecarr @wojtek-t on the namespace deletion worker bump (c.f. #46437). I'm in favor of it... experimentally, it cut cleanup time in half on the e2e, and we saw peak times to sync individual namespaces cut in half as well, indicating the controllers had QPS (c.f. #45304) and CPU headroom and were blocking on something else.

controller time to perform a single namespace sync

Before (max=215.29, median=20.92, average=26.82):

After (max=98.93, median=19.43, average=19.68):

e2e time to observe a namespace deletion completing

Before (max=305, median=100, average=120):

After (max=156, median=44, average=45):

derekwaynecarr · 2017-06-13T02:13:57Z

+1 the worker bump to 10

liggitt · 2017-06-13T02:35:55Z

cluster/gce/gci/configure-helper.sh

@@ -1288,7 +1288,7 @@ function start-kube-apiserver {
  fi


-  local authorization_mode="RBAC"
+  local authorization_mode="Node,RBAC"


I haven't kept up with the cluster/gce/... deployments...

this line also appears in gce/container-linux, gce/cos, gce/ubuntu as well... do any of those need to be updated as well?

gce/cos is a symlink to gce/gci. I'm not sure if gce/container-linux is tested anywhere. A quick search of testgrid didn't turn anything up. I'm not inclined to change anywhere that is not being tested.

@roberthbailey who maintains container-linux?

According to the OWNERS file, it's @euank, @yifan-gu, and @ethernetdan.

mikedanese · 2017-06-13T02:56:24Z

/retest

liggitt · 2017-06-13T03:50:39Z

/lgtm

k8s-github-robot · 2017-06-13T03:51:04Z

[APPROVALNOTIFIER] This PR is APPROVED

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

Associated issue: 46999

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/gce/OWNERS~~ [mikedanese]
~~cmd/kube-controller-manager/OWNERS~~ [mikedanese]

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

roberthbailey · 2017-06-13T06:06:41Z

cmd/kube-controller-manager/app/options/options.go

@@ -67,7 +67,7 @@ func NewCMServer() *CMServer {
 			ConcurrentJobSyncs:                              5,
 			ConcurrentResourceQuotaSyncs:                    5,
 			ConcurrentDeploymentSyncs:                       5,
-			ConcurrentNamespaceSyncs:                        5,
+			ConcurrentNamespaceSyncs:                        10,


Is 10 enough?

mikedanese · 2017-06-13T06:09:37Z

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

mikedanese · 2017-06-13T08:08:38Z

/retest

k8s-github-robot · 2017-06-13T09:03:14Z

Automatic merge from submit-queue

mikedanese self-assigned this Jun 1, 2017

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

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Jun 1, 2017

mikedanese force-pushed the gce-2 branch from 2dfa199 to 9d43cbb Compare June 1, 2017 20:11

liggitt reviewed Jun 1, 2017

View reviewed changes

mikedanese added this to the v1.7 milestone Jun 1, 2017

liggitt mentioned this pull request Jun 1, 2017

Update node authorization proposal kubernetes/community#672

Merged

liggitt reviewed Jun 1, 2017

View reviewed changes

liggitt added area/platform/gce area/provider/gcp Issues or PRs related to gcp provider sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. labels Jun 1, 2017

mikedanese force-pushed the gce-2 branch 4 times, most recently from e04c9a8 to 9b2540d Compare June 1, 2017 21:46

mikedanese changed the title ~~enable Node authorizer and NodeRestriction admission controller in GCE/GKE~~ enable Node authorizer and NodeRestriction admission controller Jun 1, 2017

mikedanese assigned liggitt and luxas Jun 1, 2017

mikedanese added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Jun 1, 2017

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 1, 2017

mikedanese force-pushed the gce-2 branch from 9b2540d to 811dfae Compare June 1, 2017 22:55

mikedanese unassigned liggitt Jun 1, 2017

mikedanese force-pushed the gce-2 branch from d3d6bd6 to b633289 Compare June 12, 2017 18:00

k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 12, 2017

mikedanese force-pushed the gce-2 branch from eb06e4a to b633289 Compare June 12, 2017 19:45

kubernetes deleted a comment from k8s-ci-robot Jun 12, 2017

mikedanese force-pushed the gce-2 branch 3 times, most recently from 85059ce to d6202e6 Compare June 12, 2017 22:11

set max concurrent namespace syncs to 10

76493fc

mikedanese force-pushed the gce-2 branch from d6202e6 to 76493fc Compare June 13, 2017 01:50

liggitt mentioned this pull request Jun 13, 2017

Bump the concurrent syncs of the namespace controller kubernetes/test-infra#3029

Closed

liggitt reviewed Jun 13, 2017

View reviewed changes

liggitt mentioned this pull request Jun 13, 2017

Add feature gating to REST Compression #46966

Merged

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

roberthbailey reviewed Jun 13, 2017

View reviewed changes

k8s-github-robot merged commit 5d2dbb5 into kubernetes:master Jun 13, 2017

mikedanese deleted the gce-2 branch June 13, 2017 13:18

liggitt mentioned this pull request Jun 13, 2017

cache mutation detector causes memory/cpu pressure at the end of long e2e runs (like pull-kubernetes-e2e-gce-etcd3) #47135

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enable Node authorizer and NodeRestriction admission controller #46796

enable Node authorizer and NodeRestriction admission controller #46796

mikedanese commented Jun 1, 2017 •

edited

Loading

luxas commented Jun 1, 2017

liggitt Jun 1, 2017

liggitt Jun 1, 2017

liggitt Jun 1, 2017 •

edited

Loading

liggitt Jun 1, 2017

liggitt Jun 1, 2017

liggitt commented Jun 1, 2017

liggitt Jun 1, 2017 •

edited

Loading

luxas commented Jun 12, 2017

mikedanese commented Jun 12, 2017

mikedanese commented Jun 12, 2017

mikedanese commented Jun 13, 2017

liggitt commented Jun 13, 2017 •

edited

Loading

derekwaynecarr commented Jun 13, 2017

liggitt Jun 13, 2017

mikedanese Jun 13, 2017

liggitt Jun 13, 2017

mikedanese Jun 13, 2017

roberthbailey Jun 13, 2017

mikedanese commented Jun 13, 2017

liggitt commented Jun 13, 2017

k8s-github-robot commented Jun 13, 2017

roberthbailey Jun 13, 2017

mikedanese commented Jun 13, 2017

mikedanese commented Jun 13, 2017

k8s-github-robot commented Jun 13, 2017

enable Node authorizer and NodeRestriction admission controller #46796

enable Node authorizer and NodeRestriction admission controller #46796

Conversation

mikedanese commented Jun 1, 2017 • edited Loading

luxas commented Jun 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt Jun 1, 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 Jun 1, 2017

liggitt Jun 1, 2017 • edited Loading

Choose a reason for hiding this comment

luxas commented Jun 12, 2017

mikedanese commented Jun 12, 2017

mikedanese commented Jun 12, 2017

mikedanese commented Jun 13, 2017

liggitt commented Jun 13, 2017 • edited Loading

controller time to perform a single namespace sync

e2e time to observe a namespace deletion completing

derekwaynecarr commented Jun 13, 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

mikedanese commented Jun 13, 2017

liggitt commented Jun 13, 2017

k8s-github-robot commented Jun 13, 2017

Choose a reason for hiding this comment

mikedanese commented Jun 13, 2017

mikedanese commented Jun 13, 2017

k8s-github-robot commented Jun 13, 2017

mikedanese commented Jun 1, 2017 •

edited

Loading

liggitt Jun 1, 2017 •

edited

Loading

liggitt Jun 1, 2017 •

edited

Loading

liggitt commented Jun 13, 2017 •

edited

Loading