Avoid removing annotations or labels set to resources by someone other than Strimzi

[diranged]

Describe the bug

The changes in Strimzi 0.24.0, 0.27 and upcoming in 0.30.0 are causing us some pain around the ServiceAccountPatching FeatureGate. The problem is that we use AWS’s IAM system to grant our ServiceAccount resources permissions within AWS to do things … but in order to do that, we use the Keiko iam-manager controller (a modified version of it) which is able to find any ServiceAccount and automatically annotate it with .metadata.annotations."[eks.amazonaws.com/role-arn](http://eks.amazonaws.com/role-arn)": $dynamically_generated_role_arn. We just discovered that Strimzi is stripping ALL annotations out of the ServiceAccount during its reconciliation loop.

I believe this behavior is “wrong” .. and what I mean by that is that generally speaking the Kubernetes communities have allowed for controllers to completely own the “spec” of a resource, its “name”, “namespace” and even “metadata.labels” keys. However, there’s been an implicit understanding that you don’t mess with “annotations” because they tend to mean lots of different things to lots of different controllers. Having an outside-controller come in and annotate a ServiceAccount with an annotation that makes no difference to Strimzi should not result in Strimzi deleting that annotation.

To Reproduce

1. Create a KafkaConnect resource
2. Apply your own annotation of foo: bar to the ServiceAccount that was created.
3. Watch that annotation get deleted.

Expected behavior
The controller should ignore annotations that it does not manage - it has no idea whether those annotations are critical to us or not.

Environment (please complete the following information):

• Strimzi version: 0.29.0
• Installation method: Helm Chart
• Kubernetes cluster: Kubernetes 1.22
• Infrastructure: Amazon EKS

[scholzj]

The ServiceAccountPatching starts treating the service accounts as any other resource. Acou can use the template field to set the annotations on the service account. This is something what is in many cases desired in order to be able to set the annotations declaratively.

[scholzj]

Triaged on 18.8.2022: There are some possible ways how to implement this:

• Have a 3 way diff to identify which annotations/labels are managed by Strimzi and update only those set by us.
• Use an approach similar to what we use in StrimziPodSets -> generate a revision of the object and update it only when it change. Such update would rewrite the annotations, but it would do it less often.

In any case, this:

• Does not apply only to service accounts but to most resources
• Does apply for both labels and annotations

The solution is non-trivial, so this requires a proposal.

[mikebryant]

Might switching to using server-side apply be an option here? This would let the k8s apiserver handle the three-way diff / updates etc, and allow other controllers/webhooks to touch fields / labels / annotations the operator doesn't care about.

We've just hit this issue where our cluster-wide mutatingwebhookconfiguration has added labels to the StatefulSet and the strimzi-cluster-operator has gone into a loop repeatedly trying to remove the labels

[scholzj]

But Kubernetes does not know all the 3 sides. So it cannot be handled there. It knows only two sides of the diff.

[mikebryant]

But it does? This is a direct replacement for kubectl client-side apply, which does a three way diff.

The live object (A) is stored in the .metadata, .spec etc. The knowledge of which fields the client previously applied, and indeed all clients (B) is stored in the .metadata.managedFields. And the incoming object (C) is supplied by the client. So it knows which fields were previously set by the client (in the case that the new object no longer specifies some, and they can be removed), and it knows about which client set all the other fields, and will keep them around instead of wiping out all the other labels etc.

All the details are covered in https://kubernetes.io/docs/reference/using-api/server-side-apply/#merge-strategy

[scholzj]

Strimzi lets you declaratively manage labels and annotations through the custom resources. That does not include only adding but also removing. So you need to compare 3 things:

• The labels and annotations previously configured in the custom resource
• The currently configured labels and annotations in the custom resource
• The current labels on the resource it self

And based on that, you will see that the resource has for example 10 different annotations which are not anymore in the custom resource. And for each of them, you need to decide if they previously were in the Strimzi custom resource => in that case they were added by Strimzi and now should be removed by Strimzi. Or if they were not in the Strimzi custom resource before => in which case someone else added them to the resource and Strimzi should not remove them.

I'm not sure I follow how the server-side apply helps with this. (I have also no idea if the Kubernetes client supports it, but that is another story)

[mikebryant]

This is exactly what server-side apply will do for you

The labels and annotations previously configured in the custom resource

This information is stored in metadata.managedFields. For each field set the ID of the manager (i.e. strimzi-cluster-operator) is stored

The currently configured labels and annotations in the custom resource

This is in the request to update, when the operator tries to update the object in the apiserver

The current labels on the resource it self

This is, well, in the object

Service-side apply will do what you expect here - from the docs page:

If you remove a field from a configuration and apply the configuration, Server-Side Apply checks if there are any other field managers that also own the field. If the field is not owned by any other field managers, it is either deleted from the live object or reset to its default value, if it has one. The same rule applies to associative list or map items.

Any of those 10 annotations which were previously set by the operator will be removed if you're not setting them in the latest version, any that were set by other managers will be left alone and not removed.

The whole idea is for a standard controller you don't need to keep track of any of this - just send your desired configuration and the apiserver will handle the removal of things no longer set, updating what you've asked to change, and leaving other managers' config alone.

[scholzj]

Ok, I guess I need to look into it a bit more closely - I never used it TBH. Thanks for the explanation and pointing us in this direction.

[akamac]

This is exactly the same issue we are facing with deploying Strimzi on EKS. I hope the server-side apply is on the way.

[pupseba]

This also brings problems to us when used in combination with MetalLB, as metalLB needs to add one annotation (metallb.universe.tf/ip-allocated-from-pool) once it announces the IP.

Hardcoding that annotation in the kafka CR turns out to be a not very optimal solution for MetalLB as it exepcts to manage that annotation dynamically.

[pupseba]

Ok, I guess I need to look into it a bit more closely - I never used it TBH. Thanks for the explanation and pointing us in this direction.

Maybe adding an option so the operator comparisson ignores certain annotations/labels could be a practical alternative while the implementation of the apparently more complex "server-side apply" is considered? Having such mechanism (if possible) would be really helpful.

[jpuskar]

I'm trying to use kopf to watch the kafka-generated secret, and trigger other processes when the secret changes. Kopf writes an annotation. This causes Strimzi to trigger a reconcile and remove kopf's annotation. Kopf doesn't like this, and tries to add it back.

Being able to specify an array of ignorable annotation keys to strimzi-operator would be amazing. I'll go out on a limb and say that for most use-cases, it doesn't need to be dynamic. The MVP here does not need to read the list of existing annotations, and compare against what Strimzi manages. A static list of ignorable annotations would cover many use cases.

[AntoineDao]

For future reference, we are working on a proposal to try to resolve this issue. Feel free to weigh in with thoughts/comments or a 👍 if you think the proposal would be beneficial to your use case: strimzi/proposals#80

[davidballano]

Hi, I ran into this problem too, we do add some custom annotations to the Loadbalancer services the Strimzi operator creates so we can access the cluster externally from the Kubernetes network. We ran this ipvs/balanced setup that needs those so it routes the traffic. clients were disconnecting on and off against Kafka until I figured the operator was deleting the annotations and another process was adding them back every 2 mins. hopefully, this proposal will help on this front by leaving custom annotations set externally alone.

thank you.

[pantaoran]

Does anyone know that status of the K8s Server Side Apply feature? Was it ever released?
I also need a solution to similar problems...

[ppatierno]

This is the PR which some contributors started but it has been inactive since long time now.
#9306