KEP-4742: Expose Node Labels via Downward API

[docandrew]

• Adding new KEP 4742: Expose Node Labels via Downward API

• Issue link: Expose Node labels via downward API #4742

• Other comments: In response to discussion here: Consider exposing node labels via downward api kubernetes#40610

[k8s-ci-robot]

Welcome @docandrew!

It looks like this is your first PR to kubernetes/enhancements 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/enhancements has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @docandrew. 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-sigs/prow repository.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: docandrew
Once this PR has been reviewed and has the lgtm label, please assign mrunalp for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• keps/sig-node/OWNERS

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

[pacoxu]

/cc @kerthcet

[ArangoGutierrez]

As discussed at kubernetes/kubernetes#40610
this is open a lot of concerns around security

[cmwylie19]

As discussed at kubernetes/kubernetes#40610 this is open a lot of concerns around security

I think we can do this in a safe way. Let me know if the reply makes sense. Happy to work through this and give examples.

[ArangoGutierrez]

As discussed at kubernetes/kubernetes#40610 this is open a lot of concerns around security

I think we can do this in a safe way. Let me know if the reply makes sense. Happy to work through this and give examples.

After reading the entire conversation on the KEP issue and related links, I am ok with this now. As NFD maintainer I am eager to help here, as NFD creates labels for the underlying infra

[SergeyKanzhelev]

/ok-to-test

[SergeyKanzhelev]

/sig auth

need opinion on security and potential abuse

[SergeyKanzhelev]

some concerns to address: kubernetes/kubernetes#40610 (comment)

[mrueg]

Is this overloading the namespace of topology.k8s.io? Are we encouraging users to overload it with their own labels (toplogy.k8s.io/hostuefipassword=foo)? Will users accidentally see side effects because they use the same fields inside the topology.k8s.io namespace for another purpose?

Maybe it would be better to either use a config-controlled allow-list within the kubelet(?) config or a limited allow-list of well-known labels plus a new namespace like alpha.exposednodelabels.k8s.io/* (feel free to come up with a better name).

[ArangoGutierrez]

Maybe we could follow a similar logic to https://kubernetes-sigs.github.io/node-feature-discovery/v0.16/usage/features.html#built-in-labels

[thockin]

If we think "topology.k8s.io" in an extensibility mechanism, we need to spec that very clearly.

e.g. x.topology.k8s.io/<anything> is for extensions, while topology.k8s.io/<anything> is reserved for k8s use.

And even then we need to define "local". Can vendors define topology extensions?

[cmwylie19]

I'm looking to update this a KEP a bit, but i think I am a little lost. So we need to strictly document and define through code what the <anything> flag can be? Is this the ask?

[cmwylie19]

flag as in part of the label that would be applied to the pod

[thockin]

I'm looking for the KEP to say SOMETHING LIKE the below. Note that this is a suggested approach, we should consider alternatives:

"""
In KEP 1659, the following labels are defined:
* topology.kubernetes.io/region
* topology.kubernetes.io/zone

In addition to those, KEP 1659 declares the entire topology.kubernetes.io prefix space as reserved for use by the Kubernetes project.

This KEP expands upon KEP1659 in the following ways:

1. The x.topology.kubernetes.io prefix is allocated for use by end users. The kubernetes project itself will not define any standard labels with that prefix.
2. The <domain>.x.topology.kubernetes.io prefix is likewise allocated for use by end users or third-parties. The <domain> portion is treated the same as a "normal" label prefix. For example, example.com.x.topology.k8s.io/label-name.
3. All labels using the topology.kubernetes.io or *.topology.kubernetes.io prefix spaces are considered "safe" for workloads. A workload may be exposed to the values of these labels which directly apply to the workload. For example, a pod may learn the topology of the node on which it is running.
   """

Then this KEP can describe how it will expose those labels from nodes to pods (is it a literal copy or a virtual one, who wins in case of conflict, etc).

If we think there is a need for ANOTHER prefix which is "safe" but isn't "topology", we can discuss doing a similar carve-out for another prefix. But I'd argue it stops at 2.

[thockin]

I want to be really clear about my position on this:

• DownwardAPI for anything inside the Pod object seems OK (we opened that door a long time ago).
• DownwardAPI for things outside of the Pod object is not OK unless it follows authz rules (pod's SA).
• The fact that workarounds exist means we should NOT consider this an extraoridnary need and should NOT be bending rules.
• Arguments based on reducing apiserver load need to show proof.

[thockin]

If we think "topology.k8s.io" in an extensibility mechanism, we need to spec that very clearly.

e.g. x.topology.k8s.io/<anything> is for extensions, while topology.k8s.io/<anything> is reserved for k8s use.

And even then we need to define "local". Can vendors define topology extensions?

[thockin]

fieldpaths are supposed to be paths to fields, not magic words.

What I proposed in kubernetes/kubernetes#40610 (comment) was to copy (literally copy the data) from node to pod, and define the behavior of that.

Then we don't need a new fieldpath at all. We just need to decide WHO copies that data, and whether it is literally (and pushed back to apiserver) or virtually copied by kubelet (and kept local, like service link env vars).

I think this will be net simpler than having new magic words.

[thockin]

feature gates are not long-term flags -- if you need a mechanism for admisn to disable this entirely, that's not a feature gate (and I would argue it indicates a deeper problem)

[thockin]

For very specific cases, we have workarounds like sidecars. If these DaemonSets need more info, that's their escape path.

[cmwylie19]

Notes I got from the meeting so they can be fixed and address later when there is time:

• Document behavior around restarts as it relates to fileMounts and env (values may change)
• Elaborate/Fix use-cases (can't use vpn for example based on fixed values)

updated since we are not considering using kubelet flags

[thockin]

I am -2 on kubelet flags. In most managed providers, those flags are not accessible to users or admins, and differeing opinions between providers will make portability WORSE, not better.

We'd do better to define a small set of extension mechanisms and leave it fixed at that.

[thockin]

topology.k8s.io/vpn is NOT a standard label. If you (abstract you) make up labels with "k8s.io" or "kubernetes.io" keys, you are asking to get broken. To the best of my knowledge, we have not declared that "topology.k8s.io" in an extensibility mechanism -- am I misremembering?

[docandrew]

We used this based off the suggestion made in kubernetes/kubernetes#40610 (comment) that these labels could be copied - but I think the comment there was addressing the very specific use case of things like being AZ-aware, and not the broader need to make certain node labels visible in pods.

Whatever the scheme, I think I'd like to stick with a convention for labels (e.g. downwardapi.k8s.io/* or shared.k8s.io/*) that get copied rather than specify which labels get copied via something like kubelet args or CRDs.

[thockin]

When we started this conversation, I assumed (I know, my mistake) that we would only be copying topology labels, and those topology labels are few in number and well defined by the project.

I am okay if we want to define a way for admins to extend that using our topology label space, as long as we make it clear what's an extension and what is standard. But we have to define that somewhere.

I am also okay, I guess, to define a different label space, which explicitly means that "these labels are safe to copy to pods". But again we have to define that somewhere.

I really really really do not want this to be something that is configurable, either through flags or crds or something else. Until and unless we can show that something simple and well defined absolutely cannot work, let's just do the simple thing.

[ArangoGutierrez]

Just as an after thought kubernetes-sigs/node-feature-discovery#1779

[k8s-ci-robot]

@docandrew: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-enhancements-verify	d3ce8be	link	true	/test pull-enhancements-verify

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

[kannon92]

Reading @thockin comments about kubelet-sidecar. Maybe you can comment here about some alternatives that have been purusued or discussed?