kustomize icon indicating copy to clipboard operation
kustomize copied to clipboard

Remove duplicate label behavior: commonLabels vs labels.includeSelectors

Open ncapps opened this issue 2 years ago • 10 comments

Eschewed features

  • [X] This issue is not requesting templating, unstuctured edits, build-time side-effects from args or env vars, or any other eschewed feature.

What would you like to have added?

The commonLabels and labels fields with includeSelectors enabled produce the same output. It is preferable to have one method to add labels that are propagated to selectors and templates.

The sample kustomization files below would produce the same output.

# Add labels with labels and includeSelectors enabled
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

labels:
  - pairs:
      someName: someValue
      owner: alice
      app: bingo
    includeSelectors: true

resources:
- deploy.yaml
- service.yaml
# Add labels with commonLables
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

commonLabels:
  someName: someValue
  owner: alice
  app: bingo

resources:
- deploy.yaml
- service.yaml

Why is this needed?

We should remove duplicate behavior where possible to potentially simplify our kustomization API.

Can you accomplish the motivating task without this feature, and if so, how?

We can produce the same result with either commonLabels and labels fields.

What other solutions have you considered?

We can remove duplicate behavior with multiple approaches.

Option 1: Deprecate commonLabels

  • Deprecate the commonLabels field and exclusively use the labels field.
  • Suggest adding the annotations field with an includeTemplates option. This would mirror the labels field spec.

Option 2: Deprecate labels

  • Deprecate the labels field and add includeSelectors and includeTemplates options to the commonLabels field.
  • Suggest adding the includeTemplates option to the commonAnnotations field.

Anything else we should know?

This was identified when documenting labels and annotations behavior here: https://github.com/kubernetes-sigs/kustomize/pull/5383

Feature ownership

  • [X] I am interested in contributing this feature myself! 🎉

ncapps avatar Nov 10 '23 23:11 ncapps

I agree that we should remove the duplicate behavior, but we need to be careful and adhere to the deprecation policy. Between the two options that you listed, I think it probably makes more sense to deprecate commonLabels as it would be fewer steps (just the deprecation), rather than the multiple steps of deprecating labels and then adding new functionality to commonLabels.

I think this has been discussed in the past, and rather unfortunately we cannot just remove a field from the kustomization definition as it is defined in the current v1beta1 version. We would have to include the removal as part of the definition of kustomization v1; then we would have to deprecate kustomization v1beta1 entirely. During the deprecation cycle (which I believe lasts at least 2 releases), we would have to support both kustomization v1 and kustomization v1beta1 before the eventual removal of kustomization v1beta1.

If you don't mind, I'd like to discuss this in the monthly kustomize bug scrub (there's one next week) so that we can make sure everyone is on the same page before we triage it.

natasha41575 avatar Nov 13 '23 15:11 natasha41575

/kind deprecation

annasong20 avatar Nov 17 '23 22:11 annasong20

/remove-kind feature

annasong20 avatar Nov 17 '23 22:11 annasong20

/triage accepted

natasha41575 avatar Nov 22 '23 17:11 natasha41575

For now, we can add a deprecation warning to commonLabels letting users know that this will not be available in kustomization v1. We have a few other fields that we've deprecated this way, so we can follow the same model.

For long-term removal, we will have to move the kustomization v1 project forward, which has some nontrivial design & feature work.

/label good-first-issue /help

natasha41575 avatar Nov 22 '23 17:11 natasha41575

@natasha41575: This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

  • Why are we solving this issue?
  • To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
  • Does this issue have zero to low barrier of entry?
  • How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to this:

For now, we can add a deprecation warning to commonLabels letting users know that this will not be available in kustomization v1. We have a few other fields that we've deprecated this way, so we can follow the same model.

For long-term removal, we will have to move the kustomization v1 project forward, which has some nontrivial design & feature work.

/label good-first-issue /help

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.

k8s-ci-robot avatar Nov 22 '23 17:11 k8s-ci-robot

@natasha41575: The label(s) /label good-first-issue cannot be applied. These labels are supported: api-review, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, team/katacoda, refactor. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to this:

For now, we can add a deprecation warning to commonLabels letting users know that this will not be available in kustomization v1. We have a few other fields that we've deprecated this way, so we can follow the same model.

For long-term removal, we will have to move the kustomization v1 project forward, which has some nontrivial design & feature work.

/label good-first-issue /help

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.

k8s-ci-robot avatar Nov 22 '23 17:11 k8s-ci-robot

As noted in https://github.com/kubernetes-sigs/kustomize/issues/5527#issuecomment-1953171781, while labels offers feature parity with commonLabels, it removes some reusability aspects. commonLabels allows per-field transformer configuration to be added to a separate configuration file, which is helpful to avoid configuration duplication for situations when different kustomizations are in use. It might be worth considering an alternative mechanism for doing so before commonLabels gets entirely removed.

stormqueen1990 avatar Feb 19 '24 21:02 stormqueen1990

The new labels field is very opaque. I find the deprecation notice annoying when there does not appear to be any documentation on how to properly use labels (which I had to divine by trial and error).

Examples of it not being documented:

  • https://kubectl.docs.kubernetes.io/references/kustomize/builtins/#field-name-commonlabels
  • https://github.com/kubernetes-sigs/kustomize/blob/master/examples/transformerconfigs/README.md#labels-transformer

I'd offer to fix the documentation, but I have PRs with documentation open that no one will merge in other kubernetes-sigs projects so probably not worth my time to write up something that can't get merged.

To the maintainers of this and other kubernetes-sigs projects, please:

  • Stop adding features without updating the docs
  • Accept PRs to edit the docs in a timely manner.
  • Fix your unit tests. If I make a markdown only change and unit testing fails, then your unit testing needs to be fixed.

kingledion avatar Oct 28 '24 16:10 kingledion

Hi folks, From a QA/DevOps perspective I’m curious whether this duplication issue has observable effects in CI/CD pipelines (e.g., duplicate resources being applied, drift, or failing tests). Could you share any failure cases or test scenarios where commonLabels vs includeSelectors produced unintended results? I’d be happy to help reproduce in a test cluster or write a small test case.

krishnaharshap avatar Oct 18 '25 16:10 krishnaharshap