kubernetes / website

Kubernetes website and documentation repo:
https://kubernetes.io
Creative Commons Attribution 4.0 International
4.49k stars 14.43k forks source link

Clarify docs around node self-labelling restriction #31992

Open sftim opened 2 years ago

sftim commented 2 years ago

This is a Bug Report

Problem: Issue https://github.com/kubernetes/website/issues/31972 and the related PR documented that setting the label node-role.kubernetes.io/control-plane is a privileged operation, and that nodes cannot set that label themselves.

I'm concerned about the sentence:

The admission controller documentation covers what labels are permitted to be used with the kubelet --node-labels option.

in https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/create-cluster-kubeadm/

  1. Rather than refer to documentation in plain text, it's better to either hyperlink to that documentation or to summarize it. I would use an inline hyperlink here; it may not otherwise be obvious to all readers where they can find that documentation.
  2. The documentation for NodeRestriction may accurate but even if so, it is hard to follow. It could use an update, and especially so now that we're referring readers there.
  3. There's ambiguity with the language about whether we're calling labels “restricted” or “reserved”. According to the documentation for NodeRestriction (the admission controller), kubelets can label themselves as node-role.kubernetes.io/control-plane on themselves, but that a future version of Kubernetes may disallow this. However, https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/create-cluster-kubeadm/ implies that a kubelet is forbidden from setting that label on its Node object.

Proposed Solution: Update those pages to be clear about the behavior and about which labels the kubelet can / can't / might be allowed to set.

Pages to Update: https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/ https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/create-cluster-kubeadm/

Kubernetes Version: v1.23 (at the time of filing)

Additional Information:

/language en /kind bug

sftim commented 2 years ago

@neolit123 FYI - I hope you can see why this needs further work

neolit123 commented 2 years ago

the summary that we just added in the kubeadm page is what we have been explaining to users in too many instances on github/slack/etc and i have not seen instances of "i still do not understand after you explained it". but sure, i don't object to improving these areas further.

Rather than refer to documentation in plain text, it's better to either hyperlink to that documentation or to summarize it. I would use an inline hyperlink here; it may not otherwise be obvious to all readers where they can find that documentation.

well there is a prior sentence that has the link to the same documentation, so i guess you mean including the link in both sentences:

By default, kubeadm enables the NodeRestriction admission controller that restricts what labels can be self-applied by kubelets on node registration. The admission controller documentation covers what labels are permitted to be used with the kubelet --node-labels option.

The documentation for NodeRestriction may accurate but even if so, it is hard to follow. It could use an update, and especially so now that we're referring readers there.

i think i agree that it might need some more detail around the part that *.kubernetes.io labels that are not in the allowed list are restricted and reserved.

There's ambiguity with the language about whether we're calling labels “restricted” or “reserved”.

i can see the confusion with this one. they are separate properties. reserved means that the kubelet may decide in the future to reserve foo.kubernetes.io for its own use, which will clash with the user label foo. restriction has two purposes - security to disallow kubelet clients from self-applying kubernetes.io labels that are not desired (node-role labels are a security topic here) and also to prevent kublets from self-allocating kubernetes.io labels that may end up being reserved in the future (results in a clash; admins might know better to not do that).

According to the documentation for NodeRestriction (the admission controller), kubelets can label themselves as node-role.kubernetes.io/control-plane on themselves, but that a future version of Kubernetes may disallow this. However, https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/create-cluster-kubeadm/ implies that a kubelet is forbidden from setting that label on its Node object.

there are no "node-role..." labels in the allowed list, only "node..", thus they fall under "restricted" as a regular *.kubernetes.io label.

as mentioned on the PR, "node-role" is a legacy concept that still exists in kubectl and kubeadm and a number of other places. the KEP https://github.com/kubernetes/enhancements/blob/926a1238304eba922be389a99d51c36cda3510ed/keps/sig-architecture/1143-node-role-labels/README.md was written specifically to externalize the node-role labels from k8s core. given the kubelet and NodeRestriction AC are core components, and if the section https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#noderestriction is considered to talk strictly about k8s core, then it might not make sense to even mention the "node-role" label in there...just cover that anything kubernetes.io in the not allowed list is restricted.

neolit123 commented 2 years ago

/sig auth cluster-lifecycle

sftim commented 2 years ago

I agree about not mentioning node-role.kubernetes.io/* labels on the admission controllers page.

We should make it very clear how the NodeRestriction admission controller treats requests by the kubelet to set what's currently documented as a “reserved” label. At the moment that documentation lists only node-restriction.kubernetes.io/* labels as those that the kubelet is outright forbidden from setting. My guess is that some time between Kubernetes v1.13 and now we started to enforce some extra restrictions but we didn't update the docs to match that at the time.

sftim commented 2 years ago

/triage accepted /priority backlog /lifecycle frozen

k8s-triage-robot commented 1 year ago

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

natalisucks commented 10 months ago

This issue is still listed on SIG Auth's backlog as relevant /triage accepted