kustomize-controller icon indicating copy to clipboard operation
kustomize-controller copied to clipboard

Allow Common Labels and Common Annotations Transformers on Kustomization Apply

Open jonathan-innis opened this issue 2 years ago • 9 comments

Currently, there doesn't seem to be a way to append a given label to all Kubernetes objects that my Kustomization applies. If I want to leverage the .spec.patches property in Kustomization to achieve this, I either have to be willing to completely overwrite my /metadata/labels field or I have to already have the label field available in each metadata section of all objects in my git repo as described here

Proposal

Enable a spec.commonLabels and spec.commonAnnotations similar to the CommonLabel and CommonAnnotation transformers available natively with Kustomize

jonathan-innis avatar Dec 04 '21 23:12 jonathan-innis

The major issue with the common labels and annotations is that they are applied to pod selectors which are immutable thus breaking the sync.

stefanprodan avatar Dec 05 '21 09:12 stefanprodan

@stefanprodan This is not an issue with spec.commonAnnotations since it only updates the metadata and not the selectors for any resources, right? Feasibly, we could alter the behavior of the CommonLabel transformer slightly so we don't update the selectors of resources, though this obviously deviates slightly from Kustomize

jonathan-innis avatar Dec 06 '21 19:12 jonathan-innis

@stefanprodan I'm a little confused about the explanation above. In what way does it break the sync? Do you mean that we would have to destroy the pods to apply new common labels if they are added to the flux kustomization?

JamWils avatar Jan 19 '22 14:01 JamWils

@JamWils that's how Kubernetes works, pod label selectors are immutable, so you need to delete the Deployments/StatefulSets/DaemonSets to change them. See the docs here https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#label-selector-updates

stefanprodan avatar Jan 19 '22 16:01 stefanprodan

I 100% agree with that. However, if I change the label on a pod manually and then push then the kustomization controller will replace the pod with the new label. If we had a commonLabels on the flux kustomization wouldn't we expect the same behavior? I.e. I add a common label test: this and then I would see that common label applied to the resources kustomize is working with.

JamWils avatar Jan 24 '22 23:01 JamWils

@stefanprodan https://github.com/kubernetes-sigs/kustomize/issues/1009#issuecomment-801460003 shows how you would be able to work around the problem, but more importantly I think, is that in later versions of Kustomize, https://github.com/kubernetes-sigs/kustomize/pull/3743 has been merged into the project, allowing to set a labels field. It seems that the documentation hasn't been updated yet (https://github.com/kubernetes-sigs/kustomize/issues/3756), but something like this should work (pulled from the tests):

resources:
- deployment.yaml
labels:
- pairs:
    foo: bar
- pairs:
    a: b
  includeSelectors: true
- pairs:
    c: d
  fields:
  - path: spec/selector
    group: example.dev
    version: v1
    kind: MyCRD
    create: true

From go.mod#L188 it seems that the Kustomize-controller uses Kustomize v4.4.1, and already from v4.1.0 you get the labels field, so theoretically, it should be possible to solve this issue :smile:

AndersBennedsgaard avatar Jan 26 '22 07:01 AndersBennedsgaard

@AndersBennedsgaard yes I'm aware of the labels field but this issue is about the CommonLabels transform which includes selectors. I'm not for adding a CommonLabels field to Flux that diverges in behaviour from kustomize, this will confuse users.

@jonathan-innis Flux runs all transforms found in a kustomization.yaml, why do we need to add these fields to Flux CRD? Users can add labels and annotations in the various ways that kustomize supports inside their kustomization.yaml.

stefanprodan avatar Jan 26 '22 08:01 stefanprodan

I agree, creating a commonLabels field with a different behavior from the one with Kustomize would get confusing for people who are actually aware of the behavior of Kustomize, but, as stated several times in the original issue, the commonLabels field are likely to be removed or changed in future versions of Kustomize due to the confusing name.

My guess is that creating some field in Kustomization that adds labels to all the resources, could be done by adding to https://github.com/fluxcd/kustomize-controller/blob/main/controllers/kustomization_generator.go#L110:

if kg.kustomization.Spec.CommonUnpropagatedLabels != "" {
    kus.Labels[0].Pairs = kg.kustomization.Spec.CommonUnpropagatedLabels
}

(I am not a Go developer, if that wasn't clear :wink: ) Obviously, CommonUnpropagatedLabels, is not a name that is set in stone, but what I could come up with on the spot. This could also be made more generic, allowing users to specifically input pairs.includeSelectors and pairs.fields[].

AndersBennedsgaard avatar Jan 26 '22 08:01 AndersBennedsgaard

Just to expand on my reason for adding the fields to the Kustomization: I help run a multi-tenant setup, where some of the tenants does not care much for Kubernetes. They just want to write as little YAML as possible, and using the very simple Helm chart we have written for them is enough. We therefore make much use of the autodetection and recursiveness of the Kustomization, in order to automate the deployments. This means that placing a kustomization.yaml with the transformers at the root of the project, we would require our tenants to write and manage one for each of their subdirectories - removing the nice autodetection of Flux.

AndersBennedsgaard avatar Jan 26 '22 11:01 AndersBennedsgaard

I've got the same multi-tenant use-case, where it would be nice to apply labels to all resources for a tenant for them instead of relying on the tenants to add their own labels or add a LabelTransformer in their own kustomization.yaml.

Supporting LabelTransformation would also work, without breaking changes since you specify the labels, object types, and full paths to apply the labels. It would also not cause confusion like having unique commonLabels for flux with a different operation than kustomize.

alphabet5 avatar Mar 07 '23 22:03 alphabet5