Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: ability to disable pod-security labels added by tigera/operator #2872

Open
uwebartels opened this issue Sep 14, 2023 · 11 comments

Comments

@uwebartels
Copy link

tigera is reverting custom labels on namespace calico-system

Expected Behavior

custom labels for namespaces should be deletable and not be reverted to its former state.

Current Behavior

Currently I need to delete a custom label on the namespace calico-system. Technically I can do this, but tigera-operator reverts my change within some seconds.

Possible Solution

Steps to Reproduce (for bugs)

  1. add a custom label to the namespace, I don't really know how this was done.
  2. remove the label
  3. check tigera log output for {"level":"info","ts":1694683553.298883,"logger":"controller_apiserver","msg":"Reconciling APIServer","Request.Namespace":"","Request.Name":"default"}
  4. kubectl describe ns calico-system and see that the former deleted label is configured again

Context

label for the pod-security-webhook are set. they prevent the creation of new nodes - or better to setup new node for joing the cluster. Especially when there is no node at all, I cannot start the calico pods due to the non-reachability of the webhook.

Your Environment

  • aws eks 1.25
  • Operating System and version: Amazon Linux release 2 (Karoo)
  • helm chart: tigera-operator 3.25
@tmjd
Copy link
Member

tmjd commented Sep 14, 2023

The operator is doing as expected and making sure configuration stays as it expects.

Is that webhook running as a pod? If so you would need to have at least one pod running so the webhook is functional. Without at least one node running then your cluster is effectively broken since it is configured with a webhook which is served by a pod but that pod can't exist without a node.

Does this issue happen for you only when you've got a cluster which had nodes, scaled to zero nodes, and then when it tries to scale back up it cannot?
Or does this issue still happen if you have one node (or more) and add another node?

@uwebartels
Copy link
Author

sure - my cluster was broken. It happened during an update.
There was no node available, so neither any pod answering that webhook.

I can scale down the webhook deployment and see the same behavior.
That is the reason I want to delete those labels.

When the webhook is up and running, new nodes can join the cluster.

@tmjd
Copy link
Member

tmjd commented Sep 14, 2023

It seem to me like the correct fix would be to either:

  1. Remove that webhook if you're going to scale to zero nodes. If the webhook is removed from the cluster then the labels should not cause a problem.
  2. Always keep at least one node so your cluster's webhook remains functional.

With zero nodes and the webhook you effectively have a broken cluster because a service that you have configured which the cluster needs to function is not available.

@uwebartels
Copy link
Author

Thanks. This is a workaround I am already aware of.

I want to remove these labels.

@uwebartels
Copy link
Author

Nobody has an idea how to permanently remove custom labels on the calico* namespaces?

@tmjd
Copy link
Member

tmjd commented Sep 25, 2023

I do not think we would change the operator to support what you are asking. The only reason this is a problem is because a webhook you have configured is not functional in your cluster effectively meaning your cluster is in a broken state without the webhook functioning. I would equate this to be like if you did not have components (like kube-proxy or another necessary kubernetes component needed in a functional cluster), the operator has dependencies it expects and if a cluster has pod-security functionality configured then the operator expects it to be functional.

A few possible options:

  1. You mention "they prevent the creation of new nodes", I don't see how the state you're suggesting where a node cannot join would make sense, I would expect a node can join but doesn't become healthy/ready/schedulable because calico-node can't run because of the webhook dependency in the cluster. You could make the webhook pod hostNetworked and have the necessary tolerations so the webhook can become functional without pod networking which then would allow calico to become healthy and the node become healthy.

  2. You could modify the operator code and build your own custom build that doesn't add those labels.

  3. You could scale the operator to 0 replicas and then it won't be there to re-add the labels, you'd need to do this after any upgrade after everything has been upgraded, the operator in general isn't necessary during normal operation. Though the operator does scale typha based on the number of nodes in a cluster so you would be missing that functionality.

@caseydavenport
Copy link
Member

I am bit confused here, I don't think I have a clear understanding of the problem statement. But here are the relevant cases, I think:

  • tigera/operator manages a set of labels on all namespaces it creates. Users cannot modify these, as doing so would break Calico.
  • Users can apply their own labels to any namespaces. tigera/operator will not touch these. However, per the above bullet point, users cannot modify labels that the tigera/operator itself creates, as that would break Calico.

I don't really understand where the webhook comes into play here based on the discussion above. Could you share:

  • The source code or documentation of the webhook that you are attempting to run in your cluster?
  • The exact labels (key and value) that you want to remove from the calico-system namespace

@uwebartels
Copy link
Author

Hi,

Thanks for your response.
First I did not modify labels on the calico-system namespace created by tigera and never wanted or tried.

I added my own labels to the calico-system namespace, for example
pod-security.kubernetes.io/enforce: privileged
pod-security.kubernetes.io/warn: privileged
pod-security.kubernetes.io/audit: privileged

My intention was to "show" that the namespace is running with full privileges in terms of pod security admission.
It has actually no effect, as pod security admission rather removes privileges than giving these.

However it turned out that my custom labels "tell" kubernetes to check pods to be created against pod security admission.
The pod security webhook contains a ValidatingWebhookConfiguration.
I'm runnig kubernetes 1.24 on aws eks. Here pod security admission is only usable via the pod security webhook.
https://artifacthub.io/packages/helm/av1o-charts/pod-security-webhook version 0.3.2

So during a difficult upgrade all nodes were removed and we had to create a new nodegroup.
Here it turned out that no calico pod could be created as kubernetes checks first the pod-security-webhook if this allowed. So the nodes never became ready as it wasn't running of course.
My workaround was to delete the pod-security-webhook then wait until the calico pods are ready and install the pod-security webhook again.

So I want to remove my custom labels e.g. pod-security.kubernetes.io/enforce: privileged to be safer in critical situations.

Hops this answers your questions.

Best...
Uwe

@tmjd
Copy link
Member

tmjd commented Sep 29, 2023

Excellent questions Casey.
@uwebartels Thanks for explaining the details more specifically. I was completely misunderstanding what you were saying. The details you've provided really help.

But just to make sure I understand, I'll put what I understand in my own words.
You've added several labels and you now want to remove the labels but it seems like the operator is adding back those labels when you try to remove them.
I don't think the pod-security-webhook factors into this issue at all except that it prevented calico pods from deploying because of the labels you added previously to the calico-system namespace and the labels you added were for the pod-security-webhook.

One thing to clarify, from your original steps

  1. check tigera log output for {"level":"info","ts":1694683553.298883,"logger":"controller_apiserver","msg":"Reconciling APIServer","Request.Namespace":"","Request.Name":"default"}

The controller that this message comes from does nothing with the calico-system namespace. This does not suggest that the tigera/operator put your label back on the namespace.

I would not expect the tigera/operator to re-add a label that you previously added and then removed. I've quickly tested on a cluster I have and I was able to add a label to the calico-system namespace and then was able to remove it. (This was not using the exact same
There is the possibility that there is a race condition where the operator is updating the namespace at the same time that you are editing the namespace and it overwrites your change. I believe the code shouldn't overwrite and should receive an error in that case which should result in trying again but when it reads the namespace again the label should be removed.
Have you tried removing the label more than once and this behavior is persistent for you?
If the answer is yes, then I'd suggest you scale the operator to zero replicas. Then if it is the operator you should definitely be able to remove the label as the operator wouldn't be running. If the label stays, with the operator at zero replicas, and you still can't remove the label then it definitely isn't the operator.

@caseydavenport
Copy link
Member

One thing to add - the tigera/operator does set some pod-security labels here:

ns.Labels["pod-security.kubernetes.io/enforce"] = string(pss)
ns.Labels["pod-security.kubernetes.io/enforce-version"] = "latest"

Namely, it always sets the pod-security.kubernetes.io/enforce: privileged and pod-security.kubernetes.io/enforce-version: latest labels on the calico-system Namespace. This is why I was a bit confused when you mentioned they were custom labels, because the tigera/operator already sets some of these itself. That means the operator will put that label back on if you remove it (It doesn't set the audit or warn labels, though. So I'd expect you to be able to remove those without a problem).

The interesting part that I hadn't realized is that these pod-security annotations aren't implemented natively within the apiserver for EKS, rather they use a ValidatingWebhookConfiguration to outsource the enforcement to a controller running on the cluster. This of course will prove problematic when all the nodes are destroyed, because:

  • Kubernetes won't allow the webhook controller pod to start until pod networking is available
  • Kubernetes won't launch any Calico pods until they pass the validating webhook (which obviously is broken in this state)

So if I understand correctly, it sounds like there is a deadlock sitaution here. The possible resolutions would be:

  • Remove the pod-security.kubernetes.io/enforce label from calico-system
  • Remove the pod-security ValidatingWebhookConfiguration until Calico is running, then re-install it.

Removing the pod-security.kubernetes.io/enforce label from calico-system won't work unless the operator is stopped, because the operator thinks that it owns that label (which it does, normally). However, stopping the operator will also stop it from installing Calico. So I am not sure that's really an option.

@uwebartels
Copy link
Author

oh yes, we come closer!
I was pretty sure that we set these labels, cause we introduced pod security admission short time ago.
So tigera set these labels!!

And yes, when for example I use aws eks 1.24 there is no pod security admission in the cluster enabled. the aws support mentioned that they do this when it is out of beta. So I had to implement the pod security webhook.

And yes you see now the deadlock situation.
I am aware of the workaround for this deadlock situation to delete the webhook when there is no node at all in the cluster.

But for good reasons I am not happy with workarounds. You have to document them and in an emergency we had nobody will find that piece of documentation of course.

So for me a possible solution would be that these pod-security labels are optional. They can be enabled by default of course.

@caseydavenport caseydavenport changed the title tigera is reverting custom labels on namespace calico-system Feature request: ability to disable pod-security labels added by tigera/operator Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants