Fix admission-controllers section #4751

tengqm · 2017-08-11T03:18:23Z

This patch refactors the admin/admission-controllers section:

Reorder the built-in controllers based on their names

Added controllers that were not documented:

* GenericAdmissionWebhook
* Initializers
* LimitPodHardAntiAffinity
* NamespaceAutoProvision
* NamespaceExists
* OwnerReferencesPermissionEnforcement
* PersistenVolumeLabel
* PodPreset
* PodTolerationRestrictio

This change is

k8sio-netlify-preview-bot · 2017-08-11T03:20:03Z

Deploy preview ready!

Built with commit d4b9363

https://deploy-preview-4751--kubernetes-io-master-staging.netlify.com

tengqm · 2017-08-11T03:21:07Z

To reviewers: this git diff is showing the patch in a funny way ... however, as summarized in the commit message, this patch did NOT change contents about documented controllers besides reordering them.
It also adds documentation about three controllers that were missing.

chenopis

A few minor corrections requested.

chenopis · 2017-08-21T18:43:21Z

docs/admin/admission-controllers.md


-This plug-in sets the default forgiveness toleration for pods, which have no forgiveness tolerations, to tolerate
-the taints `notready:NoExecute` and `unreachable:NoExecute` for 5 minutes.
+This plug-in automatically attach region or zone labels to PersistentVolumes


"attach" => "attaches"

chenopis · 2017-08-21T18:45:42Z

docs/admin/admission-controllers.md

+### PodPreset
+
+This plug-in injects a pod with the fields specified in a matching PodPreset.
+See also [Inject Information into Pods Using a PodPreset](/docs/tasks/inject-data-application/podpreset.md)


The URL should be /docs/tasks/inject-data-application/podpreset/.

chenopis · 2017-08-21T18:46:41Z

docs/admin/admission-controllers.md

-Future versions may add additional restrictions to ensure kubelets have the minimal set of permissions required to operate correctly.
+This plug-in first verifies any conflict between a pod's tolerations and its
+namespace's tolerations, and rejects the pod request if there is a conflict.
+It then merge the namespace's tolerations into the pod's tolerations.


"merge" => "merges"

chenopis · 2017-08-21T18:47:36Z

docs/admin/admission-controllers.md

+namespace's tolerations, and rejects the pod request if there is a conflict.
+It then merge the namespace's tolerations into the pod's tolerations.
+The resulting tolerations are checked against the namespace's whitelist of
+tolerations. If the checking succeeds, the pod request is admitted otherwise


"checking" => "check"

chenopis · 2017-08-21T18:49:26Z

docs/admin/admission-controllers.md

+This plug-in does not do anything when no default storage class is configured. When more than one storage
+class is marked as default, it rejects any creation of `PersistentVolumeClaim` with an error and administrator
+must revisit `StorageClass` objects and mark only one as default.
+This plugin ignores any `PersistentVolumeClaim` updates, it acts only on creation.


caesarxuchao · 2017-08-21T21:59:06Z

docs/admin/admission-controllers.md


-It is strongly encouraged that this plug-in is configured last in the sequence of admission control plug-ins.  This is
-so that quota is not prematurely incremented only for the request to be rejected later in admission control.
+### InitialResources (experimental)


yes, will fix.

caesarxuchao · 2017-08-21T22:07:10Z

docs/admin/admission-controllers.md

-See [persistent volume](/docs/user-guide/persistent-volumes) documentation about persistent volume claims and
-storage classes and how to mark a storage class as default.
+This plug-in protects the access to the `blockOwnerDeletion` property of an object
+so that only users with "delete" permission to the object can change it.


This plug-in protects the access to metadata.ownerReferences of an object so that only users with "delete" permission to the object can change it.
This plug-in also protects the access to metadata.ownerReferences[x].blockOwnerDeletion of an object, so that only users with "delete" permission to the owner, i.e., the object referred by the ownerReference[x], can change it.

thanks, will revise

caesarxuchao · 2017-08-21T22:07:49Z

Also these sections lgtm:

GenericAdmissionWebhook
Initializers
NamespaceAutoProvision
NamespaceExists

chenopis · 2017-08-22T22:48:49Z

@caesarxuchao Has everything been addressed?

tengqm · 2017-09-05T01:13:45Z

@caesarxuchao PTAL, thanks.

chenopis · 2017-09-05T22:37:32Z

@caesarxuchao Ping.

caesarxuchao · 2017-09-06T01:15:20Z

docs/admin/admission-controllers.md

+This plug-in protects the access to the `metadata.ownerReferences` of an object
+so that only users with "delete" permission to the object can change it.
+This plug-in also protects the access to `metadata.ownerReferences[x].blockOwnerDeletion`
+of an object, so that only users with "delete" permission to the *owner* (i.e,


This one needs update after kubernetes/kubernetes#49133 gets merged.

so that only users with "update" permission to the `finalizers` subresource of the referenced *owner* can change it.

caesarxuchao · 2017-09-06T01:27:45Z

docs/admin/admission-controllers.md

+
+This plug-in observes creation of `PersistentVolumeClaim` objects that do not request any specific storage class
+and automatically adds a default storage class to them.
+This way, users that do not request any special storage class do no need to care about them at all and they


oops~ done.

ahmetb · 2017-09-06T01:55:01Z

docs/admin/admission-controllers.md


-### SecurityContextDeny
+This plug-in is related to the [Dynamic Admission Control](/docs/admin/extensible-admission-controllers)
+introduced in v1.7.


I think you can explain further:

It determines initializers of a resource based on existing InitializerConfigurations , sets the pending initializers on the resource metadata and blocks the Create request until all pending initializers complete.

this is my understanding, @caesarxuchao to confirm

Thanks for the suggestion. Description revised.

caesarxuchao

I left comments for the other plugins. I only learnt them recently so I could be wrong. Otherwise LGTM.

caesarxuchao · 2017-09-06T04:07:37Z

docs/admin/admission-controllers.md

+### DefaultTolerationSeconds
+
+This plug-in sets the default forgiveness toleration for pods, which have no forgiveness tolerations, to tolerate
+the taints `notready:NoExecute` and `unreachable:NoExecute` for 5 minutes.


This seems to be a little more accurate: "This plug-in sets the default forgiveness toleration for pods to tolerate the taints notready:NoExecute and unreachable:NoExecute for 5 minutes, if the pods don't already have toleration for taints notready:NoExecute or unreachable:NoExecute."

caesarxuchao · 2017-09-06T04:15:57Z

docs/admin/admission-controllers.md

@@ -78,6 +98,11 @@ If your cluster supports containers that run with escalated privileges, and you
 restrict the ability of end-users to exec commands in those containers, we strongly encourage
 enabling this plug-in.

+### GenericAdmissionWebhook (alpha)
+
+This plug-in is related to the [Dynamic Admission Control](/docs/admin/extensible-admission-controllers)


This plug-in calls the webhooks configured via ExternalAdmissionHookConfiguration, and only admits the operation if all the webhooks admit it. Currently the plugin always fails open, i.e., ignore the failed call to a webhook.

caesarxuchao · 2017-09-06T04:30:17Z

docs/admin/admission-controllers.md

+introduced in v1.7.
+The plug-in determines the initializers of a resource based on the existing
+`InitializerConfiguration`s. It sets the pending initializers by modifying the
+metadata of the resource to be created and and then blocks the resource create


and and then blocks the resource create request until all pending initializers have completed is not very accurate. The apiserver blocks the CREATE call for 30s and then returns a 504. I think it's fine if we end the description at by modifying the metadata of the resource. Readers can refer to the linked doc for details.

caesarxuchao · 2017-09-06T04:40:19Z

docs/admin/admission-controllers.md


-This plug-in will deny any pod that attempts to set certain escalating [SecurityContext](/docs/user-guide/security-context) fields. This should be enabled if a cluster doesn't utilize [pod security policies](/docs/user-guide/pod-security-policy) to restrict the set of values a security context can take.
+This plug-in observes pod creation requests. If a container omits compute resource requests and limits,
+then the plug-in auto-populates a compute resource request based on historical usage of containers running the same image.


I think it's worth mentioning the estimated resource requests are set in annotations, rather than the actual container.resources fields.

the description already uses "annotates" as the verb, but I'm okay with more clarifications.

This patch refactors the admin/admission-controllers section: - Reorder the built-in controllers based on their names - Added controllers that were not documented: * GenericAdmissionWebhook * Initializers * LimitPodHardAntiAffinity * NamespaceAutoProvision * NamespaceExists * OwnerReferencesPermissionEnforcement * PersistenVolumeLabel * PodPreset * PodTolerationRestriction

k8s-github-robot assigned chenopis Aug 11, 2017

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

chenopis added the Needs Docs Review label Aug 11, 2017

This was referenced Aug 17, 2017

PersistentVolumeLabel admission controller is undocumented kubernetes/kubernetes#50699

Closed

Initializer admission control plugin not documented #5019

Closed

tengqm force-pushed the admission-controller-fix branch from dae39e6 to 87f0acb Compare August 18, 2017 01:38

chenopis suggested changes Aug 21, 2017

View reviewed changes

chenopis added Docs Review: Open Issues and removed Needs Docs Review labels Aug 21, 2017

chenopis requested a review from janetkuo August 21, 2017 18:50

caesarxuchao reviewed Aug 21, 2017

View reviewed changes

chenopis added Tech Review: Open Issues and removed Needs Tech Review labels Aug 22, 2017

tengqm force-pushed the admission-controller-fix branch 2 times, most recently from 4e14448 to 97f72f7 Compare August 22, 2017 01:43

chenopis approved these changes Aug 22, 2017

View reviewed changes

chenopis added Docs LGTM and removed Docs Review: Open Issues labels Aug 22, 2017

chenopis removed the request for review from janetkuo August 22, 2017 22:47

caesarxuchao reviewed Sep 6, 2017

View reviewed changes

caesarxuchao mentioned this pull request Sep 6, 2017

Issue with /docs/admin/admission-controllers/ #3588

Closed

2 tasks

caesarxuchao reviewed Sep 6, 2017

View reviewed changes

ahmetb reviewed Sep 6, 2017

View reviewed changes

tengqm force-pushed the admission-controller-fix branch from 97f72f7 to 0f50a9e Compare September 6, 2017 03:09

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 6, 2017

caesarxuchao reviewed Sep 6, 2017

View reviewed changes

tengqm force-pushed the admission-controller-fix branch from 0f50a9e to d4b9363 Compare September 6, 2017 05:29

chenopis added Tech Review LGTM and removed Tech Review: Open Issues labels Sep 6, 2017

chenopis merged commit ee64185 into kubernetes:master Sep 6, 2017

tengqm deleted the admission-controller-fix branch September 28, 2017 04:44

Fix admission-controllers section #4751

Fix admission-controllers section #4751

Conversation

tengqm commented Aug 11, 2017 • edited Loading

k8sio-netlify-preview-bot commented Aug 11, 2017 • edited Loading

tengqm commented Aug 11, 2017

chenopis left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caesarxuchao Aug 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caesarxuchao commented Aug 21, 2017

chenopis commented Aug 22, 2017

tengqm commented Sep 5, 2017

chenopis commented Sep 5, 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

caesarxuchao left a 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

tengqm Sep 6, 2017 • edited Loading

Choose a reason for hiding this comment

tengqm commented Aug 11, 2017 •

edited

Loading

k8sio-netlify-preview-bot commented Aug 11, 2017 •

edited

Loading

caesarxuchao Aug 21, 2017 •

edited

Loading

tengqm Sep 6, 2017 •

edited

Loading