patches should accept a patch file with multiple patches

[iamnoah]

Eschewed features

• 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?

patchesStrategicMerge is deprecated but provides a feature that is not currently possible with patches in that multiple strategic merge patches can be combined into a single file.

I should be able to specify that my targets are determined by the GVKNN of the strategic merge patch itself:

patches:
- path: sm-patches-for-multiple-objects.yaml
   target:
     strategicMerge: byName

The exact syntax is unimportant.

Why is this needed?

This is valuable when patch changes are correlated and future changes need to consider multiple targets. Splitting such patches across multiple files is incoherent and error prone.

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

My first preference would be to simply recognize this is a valuable use case and not deprecated or remove it.

What other solutions have you considered?

Helm? This ability was central to why I started using kustomize years ago, so I am frankly shocked that it is on the chopping block.

Anything else we should know?

Relates to #5040

Feature ownership

• I am interested in contributing this feature myself! 🎉

[natasha41575]

I should be able to specify that my targets are determined by the GVKNN of the strategic merge patch itself

Maybe this is not documented properly, but you can already do this by not providing a target and it will use the GVKN of the patch to find the target.

Here is an example I tried:

# kustomization.yaml
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- pod.yaml
patches:
- path: patch.yaml

# pod.yaml
apiVersion: v1
kind: Pod
metadata:
  name: bar

# patch.yaml
apiVersion: v1
kind: Pod
metadata:
  annotations:
    foo: bar
  name: bar

Running kustomize build shows that the proper target gets patched, so I don't think we need to add additional syntax to targets for this.

Your second point, that there should be able to be multiple patches in a single file, is valid IMO, and I don't see a strong reason why we shouldn't allow that. @KnVerey do you have any concerns with that?

/triage under-consideration
/retitle patches should accept a patch file with multiple patches

Editing to add a small response:

I am frankly shocked that it is on the chopping block.

Part of the reason for the lengthy deprecation process is so that we can address user concerns like these before removing large features, and I'm optimistic that we can get patches to a place that minimizes user friction when migrating.

[lblazewski]

The feature of supporting multiple patches used the same way as @natasha41575 showed is highly needed for all of the folks relying currently on patchesStrategicMerge since otherwise the migration would be quite a hassle.

[natasha41575]

#3481 is a related issue, but the decision there was made prior to the deprecation of patchesStrategicMerge, so we should reconsider it. We will send an update here once we have decided how we want to handle it.

[natasha41575]

After discussion, we have decided to accept the feature with the following restrictions:

• Multiple SMP patches in one file should be allowed. But you cannot specify an explicit patches.target field in this case, and instead the implicit target from each SMP will be used.
• Multiple JSON patch documents are should not be allowed (currently, only the first one is used and the rest are ignored. This should be an error instead.)
• kustomize edit fix will still split patchesStrategicMerge patches into multiple files, as per Change kustomize edit fix to split patchesStrategicMerge entries before converting them to patch entries. #5059.

/triage accepted
/help

[k8s-ci-robot]

@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:

After discussion, we have decided to accept the feature with the following restrictions:

• Multiple SMP patches in one file are allowed. But you cannot specify an explicit patches.target field in this case, and instead the implicit target from each SMP will be used.
• Multiple JSON patch documents are should not be allowed (currently, only the first one is used and the rest are ignored. This should be an error instead.)
• kustomize edit fix will still split patchesStrategicMerge patches into multiple files, as per Change kustomize edit fix to split patchesStrategicMerge entries before converting them to patch entries. #5059.

/triage accepted
/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.

[natasha41575]

@koba1t Are you interested in working on this?

[koba1t]

/assign

[pre]

It would be great to get #5059 merged!

[annasong20]

• Multiple JSON patch documents are should not be allowed (currently, only the first one is used and the rest are ignored. This should be an error instead.)

To clarify, this comment refers to json6902 patches expressed in yaml documents, not json.

[ashutosh887]

@natasha41575 Is it resolved?

/assign

[koba1t]

@ashutosh887
Thanks for worrying about this issue.
Please check #5194

/unassign @ashutosh887