Propagation of labels and/or annotations

[akazs]

Hello,

We are adopting HNC to our clusters.
However, we usually use labels to manage some information about our namespaces.
Is there any way we can propagate labels and/or annotations to subnamespaces?
If not, do you plan on implementing such a feature?

[juv]

We were also looking for this feature while considering HNC vs. Kiosk for our multi-tenancy approach. Unfortunately there is no such option in HNC right now.

For me, this one of those features that makes certain usecases, e.g. aggregate costs of tenants with some kind of cost-aggregation-application, monitoring and alerting or even scraping the API server for all resources of a certain tenant, a lot easier.

[adrianludwin]

We've considered it, but we've been lacking:

1. A solid set of use cases
2. A concrete technical proposal
3. Enough people willing to help implement the feature

If both of you (@akazs and @juv) could describe your use cases, that would be very helpful. This may not be terribly technically difficult but it would be easy to implement the "wrong" API by mistake and have to deprecate it and start over. I'd rather not do that if possible.

The big problem is that we don't know which labels (and annotations) should be propagated, and which should not. Possibly worse, if you remove a label from a parent (or move a child to a different parent), we don't know which labels should be removed from the child, because any one of them could have been put there manually by a human, and not HNC. We don't have this problem with regular objects, because all objects propagated by HNC have a label that (if present) tells us that we can modify or delete it. But you can't put a label on a label.

My best guess is to start by adding a list of propagated labels to the cluster-wide HNCConfiguration object. E.g., we could say that foo.com/my-label is handled by HNC, so you would no longer be able to add it yourself to a namespace, and if a namespace contained that label but its parent did not, HNC would remove it. The only way to set this label would then be in the HierarchyConfiguration in the namespace. This would allow us to robustly remove labels (and annotations) from namespaces when HNC no longer thinks it belongs there.

I'd be willing to do at least some of the implementation work (e.g. the basic propagation and validation) but I'd want some help finishing it up. And I'd like to know if this proposal meets any use cases as well.

Thoughts?

[akazs]

We label our namespaces, and use these labels when generating metrics using kubernetes/kube-state-metrics.
This allows us to filter metrics of, say a team, to send alerts or so.
When creating subnamespaces, we would like the same labels to be applied to the newly created subnamespaces.

We don't have a concrete proposal yet, but we consider that managedFields may help us achieve this.
Specifically, by using managedFields, we could tell that who created or updated those labels, to determine that which labels should be propagated and which may be deleted.

We are a relatively small team, so we cannot guarantee.
However if you decide to implement this feature, we will try our best to help.

[zoetrope]

I have tried to implement it with Server Side Apply. #48
This will ensure that if you remove a label from a parent, the child labels will be removed properly.

The big problem is that we don't know which labels (and annotations) should be propagated, and which should not.

The current implementation does not solve this problem. All labels are propagated.

[akazs]

After some discussion, we figured out that in our use case, we don't necessarily need those labels to be propagated.
If we can put labels onto subnamespaceanchors, and make those labels be on the corresponding namespaces, that would be enough.
This also doesn't require changes to crd definitions.

[adrianludwin]

I have tried to implement it with Server Side Apply. #48

Thanks for the contribution! But it's quite hard for me to tell what the behaviour of this change is supposed to be, and there aren't any tests that I can use to determine whether I agree with the behaviour (also, tests are critical to ensure that the feature continues to work).

Can you please write a short document that explains exactly what your change is trying to do, as well as what use cases it solves? A Google doc would be a good choice, since then other people can comment on it.

This will ensure that if you remove a label from a parent, the child labels will be removed properly.

Note that this can result in a destruction of user data. If you move a namespace (that already has a label) into a hierarchy, and then back out again, it could lose its label. This may be ok, but a) we should warn the user about it (just like we warn the user if we're about to overwrite one of their objects), b) we should give people the chance to think about whether this will affect them, and c) we should document it clearly.

The big problem is that we don't know which labels (and annotations) should be propagated, and which should not.

The current implementation does not solve this problem. All labels are propagated.

There are two problems with this - one critical, and one important. The critical problem is that HNC already puts different labels (the tree labels) on parents and children, so we cannot just blindly overwrite all labels on the children since this will cause the labels to be incorrect.

The important problem is basically a generalization of this problem. Labels can be added to an object for many reasons, and it's far from clear that they should all be propagated all the time, especially without a design doc that lists all the reasons we might want to propagate them in the first place. If we introduce the behaviour that all labels get propagated, we can't (easily) go back on that and make the feature opt-in since it will break existing users. Therefore, we need to be very sure about this decision before we make it.

Again, thanks for the PR! It's not too far away from my suggestion so we should be able to iterate on it to the point where we could accept it, but we need to agree on the approach.

[adrianludwin]

The current implementation does not solve this problem. All labels are propagated.

There are two problems with this - one critical, and one important. The critical problem is that HNC already puts different labels (the tree labels) on parents and children, so we cannot just blindly overwrite all labels on the children since this will cause the labels to be incorrect.

Ah, I just read the PR and I can see that you're not overwriting any labels with the hnc.x-k8s.io prefix, so that will solve this problem. But it's unclear if this rule should be applied to all such labels, and the important problem about how we treat all the non-HNC labels still stands.

[adrianludwin]

Hey @akazs , I was thinking about this and have two quick comments:

After some discussion, we figured out that in our use case, we don't necessarily need those labels to be propagated.
If we can put labels onto subnamespaceanchors, and make those labels be on the corresponding namespaces, that would be enough.
This also doesn't require changes to crd definitions.

If we were to put the labels on the anchors, I think I'd rather modify the CRD (it's pretty easy). The reason is that I like to distinguish between the labels on the anchors (e.g. "this anchor was created by controller XYZ") and the label on the subns (e.g. "this subns can have network connectivity to the Ingress"). Those labels are not necessarily the same, so we should have a way to represent them differently. Plus it's only slightly harder to modify the CRD than it is to modify the controller :)

A slightly more serious concern I have is related to the use cases. Anchors are supposed to be given to relatively unprivileged users so that they can create subnamespaces freely, but namespace labels are often used to represent policies that should only be set by privileged users. For example, "this namespace is in prod," "this namespace has workloads that handle user data," etc. I wouldn't want an unprivileged user to be able to create a namespace that says, for example, "this namespace is highly trusted and can exchange network traffic with all other namespaces."

This is why I suggested that at the cluster level, you tell HNC which labels it's going to control, and which ones it's not, and then the only way to set those controlled labels is by modifying a HierarchyConfiguration, which is a highly privileged action. Is there a reason why this approach wouldn't work for your usecase, or is it just slightly overkill (which I'm ok with as long as it's not unusable for you)? Do you need unprivileged users to be able to set labels?

[zoetrope]

Thank you for comments.

A slightly more serious concern I have is related to the use cases. Anchors are supposed to be given to relatively unprivileged users so that they can create subnamespaces freely, but namespace labels are often used to represent policies that should only be set by privileged users.

I agree it.
For example, new PodSecurity Admission will control policies through namespace labels.

For this reason, I think we should propagate root namespace labels to subnamespaces to apply the policy.

Can you please write a short document that explains exactly what your change is trying to do, as well as what use cases it solves? A Google doc would be a good choice, since then other people can comment on it.

I will try it.

[adrianludwin]

Thanks @zoetrope !

[mutescent]

The big problem is that we don't know which labels (and annotations) should be propagated, and which should not. Possibly worse, if you remove a label from a parent (or move a child to a different parent), we don't know which labels should be removed from the child, because any one of them could have been put there manually by a human, and not HNC. We don't have this problem with regular objects, because all objects propagated by HNC have a label that (if present) tells us that we can modify or delete it. But you can't put a label on a label.

I agree with this comment. However I sort of think that there would be some needs of propagation of annotations as well. For instance, my team is adopting PodNodeSelector Admission Controller to make some resources be bound to specific nodes which are labeled. And this requires an annotation on namespaces.

(You could say that users can just set NodeSelector in every resource's spec, but it is actually not guaranteed since NodeSelector is optional. This is why we are looking for controllers like HNC.)

Then my team is adopting HNC for normal cluster users so that they can create multiple sub namespaces under their own namespace(having the annotation) which is provided by us. The problem is that PodNodeSelector Admission Controller won't work in sub namespaces since they don't have the specific annotation that the root namespace has.
As the result, the resources on sub namespaces are scheduled to other users's nodes .

@zoetrope I wonder if you have plan to implement of propagation of annotations as well.

[adrianludwin]

Hey @mutescent , what do you think of the proposal in this comment? This is my preferred approach, but because we have so few use cases that have been written down it's very hard to know what's the right thing to do. Note that while I often say "labels," I always mean "labels and annotations" - they're equally important.

When I say "use cases," I mean things like "who should be allowed to modify the propagated annotations," "how often do they change," "can permissions to modify them be delegated," "can a child namespace override a parent namespace," etc. My proposal has some implicit answers to these questions but it would be better to be explicit as well.

@mutescent , are you available to help on the implementation of this feature?

[mutescent]

@adrianludwin The mentioned comment is reasonable, because it looks simple for users and also it provides the same way to other resources.

are you available to help on the implementation of this feature?

I will give it a go. But it could take longer time than you are expecting, since I need to look into codes thoroughly 😢

[adrianludwin]

@adrianludwin The mentioned comment is reasonable, because it looks simple for users and also it provides the same way to other resources.

Thanks! It would be really helpful if you could tell us more about your usecase though, e.g. answer some of the questions I asked in my last comment. You could answer them here or maybe in a Google Doc, whichever works for you.

I will give it a go. But it could take longer time than you are expecting, since I need to look into codes thoroughly 😢

No rush :) And Ryan and I can help you out. Ping me on slack.k8s.io if you want to talk (I'm @Adrian Ludwin, you can find me on #wg-multitenancy).

[mutescent]

Hi all. Here is the Google doc link: https://bit.ly/2Vpy136 . I am not sure if this is enough or not. Thanks for helping me in advance!

[zoetrope]

@mutescent
Thanks for the great documentation.
These use cases match mine perfectly.

@adrianludwin
My colleague is developing another namespace controller.
https://github.com/cybozu-go/innu/blob/main/docs/design.md

This controller is able to propagate labels and annotations.
You can refer to it.

[adrianludwin]

Thanks all. I'm not sure when we'll get time to implement but I'm feeling like we have a fairly good direction now at least.

[bgrant0607]

Two example use cases:

1. Gatekeeper namespaceSelector labels: https://open-policy-agent.github.io/gatekeeper/website/docs/howto#constraints
2. Istio sidecar injection label: https://istio.io/latest/docs/setup/additional-setup/sidecar-injection/#controlling-the-injection-policy

[marandalucas]

Hello for everyone! We would like to get this funcionality in the future. We are in the same situation like @bgrant0607
Thanks!

[adrianludwin]

I decided to bite the bullet and start implementing this last night. Hopefully I'll have an update within a week or so.

[adrianludwin]

I'm testing an implementation that follows this: https://docs.google.com/document/d/1BaQjAQ9Rmf4LOuzUtBBzyIKk3TMkOnD35ZUzHSC9OWs/edit?resourcekey=0-vfnZVttubog-m2IjiGtATw#. Please request access and review to see if it will meet your needs. Thanks!

[adrianludwin]

See adrianludwin@89d7af1 for my initial implementation of this feature, I'll submit a PR once its predecessors have been submitted.

[adrianludwin]

The last sub-issue was #130 and this is now fixed. Closing for v1.0, we can file individual followups later.

Thanks everyone for your help in scoping this and getting it in! v1.0 should be out this week.

/close

[k8s-ci-robot]

@adrianludwin: Closing this issue.

In response to this:

The last sub-issue was #130 and this is now fixed. Closing for v1.0, we can file individual followups later.

Thanks everyone for your help in scoping this and getting it in! v1.0 should be out this week.

/close

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.