Replace excluded-namespace label with included-namespace

[yiqigao217]

Issue by yiqigao217
Friday Apr 30, 2021 at 16:22 GMT
Originally opened as https://github.com/kubernetes-sigs/multi-tenancy/issues/1503

---

Consider excluding namespaces with K8s 1.21+ namespace metadata.name label -
See kubernetes/kubernetes#92157 (comment)

[yiqigao217]

Comment by yiqigao217
Friday Apr 30, 2021 at 20:07 GMT

---

Another thought is that instead of using excluded, maybe use included? Considering the damage it could cause if the system namespaces are not labeled as excluded.

HNC can add the included label to all namespaces except those flagged as excluded-namespaces in HNC container.args. Then our object webhook can remain fail CLOSE and use that label instead.

[yiqigao217]

(The app.kubernetes.io/managed-by label may be a good option, but ACM already uses it on namespaces created from the repo. In that case, when HNC works with ACM in the same cluster, HNC actually "manages" the namespace but cannot have the app.kubernetes.io/managed-by: hnc.x-k8s.io on that namespace.)

Since this label is mainly for the object webhook to decide whether to enforce policy on the namespaces, we may name it as hnc.x-k8s.io/webhooks-enforced?

The excluded-namespace container args in the HNC deployment manifest is the only source-of-truth. HNC won't reconcile these namespaces or objects in these namespaces.

As for the currently used hnc.x-k8s.io/excluded-namespace label introduced in v0.8, we will depreciate it in v0.9 since this is a non-breaking change.

WDYT @rjbez17 @adrianludwin

[adrianludwin]

I like this idea. Maybe webhooks-enabled is a little more typical. @rjbez17 wdty?

[rjbez17]

Sorry, I never hit submit last week. Yes, I do agree with this approach. Would we use a mutating WH on the NS object to add the label? Do it in reconciliation later? I assume it will also disallow removing the label/re-add it?

[yiqigao217]

Would we use a mutating WH on the NS object to add the label? Do it in reconciliation later? I assume it will also disallow removing the label/re-add it?

No, we can do it in the HC reconciler, since we can still get the source-of-truth excluded namespaces from the HNC container args.

[adrianludwin]

... but we should add a webhook to prevent users from modifying these labels as well (I think there's another bug about stopping people from modifying tree labels too). Ryan, any thoughts on webhooks-enforced vs webhooks-enabled? I'm also thinking that it's a bit weird that this label only controls _one_ webhook (the object webhook) so maybe we need a more specific name? Maybe hnc.x-k8s.io/included-namespace (opposite of the --excluded-namespace command-line flag)?

…

On Mon, May 10, 2021 at 12:29 PM Yiqi Gao ***@***.***> wrote: Would we use a mutating WH on the NS object to add the label? Do it in reconciliation later? I assume it will also disallow removing the label/re-add it? No, we can do it in the HC reconciler, since we can still get the source-of-truth excluded namespaces from the HNC container args. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE43PZFCR7J3CX5VVKCLQ7TTNACXRANCNFSM44FQEKEA> .

[rjbez17]

Yea I was just going to suggest something like hnc.x-k8s.io/is-excluded or hnc.x-k8s.io/hnc-managed but hnc.x-k8s.io/included-namespace works as well. It would also make it more clear to admins what the label is for without having to dive into docs.

[yiqigao217]

hnc.x-k8s.io/included-namespace as opposite of the --excluded-namespace command-line flag sgtm! Will update the PR #36