Skip to content

fix a patch files accept multiple patches#5194

Merged
k8s-ci-robot merged 11 commits intokubernetes-sigs:masterfrom
koba1t:fix/patch_files_accept_multple_patches
Sep 15, 2023
Merged

fix a patch files accept multiple patches#5194
k8s-ci-robot merged 11 commits intokubernetes-sigs:masterfrom
koba1t:fix/patch_files_accept_multple_patches

Conversation

@koba1t
Copy link
Member

@koba1t koba1t commented Jun 7, 2023

fix: #5049

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Jun 7, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 7, 2023
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 7, 2023
@koba1t koba1t force-pushed the fix/patch_files_accept_multple_patches branch 4 times, most recently from 9962a9e to 132b558 Compare June 14, 2023 18:56
@koba1t koba1t marked this pull request as ready for review June 14, 2023 19:27
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 14, 2023
@k8s-ci-robot k8s-ci-robot requested a review from annasong20 June 14, 2023 19:27
@koba1t koba1t force-pushed the fix/patch_files_accept_multple_patches branch from 132b558 to a761588 Compare June 14, 2023 19:57
@koba1t
Copy link
Member Author

koba1t commented Jun 14, 2023

I test this function in this scenario.
If you think we need more tests, please tell me your ideas.

@natasha41575
Copy link
Contributor

natasha41575 commented Jun 28, 2023

/assign

@annasong20
Copy link
Contributor

annasong20 commented Jun 28, 2023

/assign

annasong20
annasong20 suggested changes Jul 1, 2023
View reviewed changes
Copy link
Contributor

@annasong20 annasong20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're almost there! Feedback mostly to address #5049 (comment)

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Jul 3, 2023

This PR has multiple commits, and the default merge method is: merge.
You can request commits to be squashed using the label: tide/merge-method-squash

Details

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.

@koba1t koba1t force-pushed the fix/patch_files_accept_multple_patches branch from 2768237 to 0bb40bc Compare July 3, 2023 18:48
@koba1t koba1t force-pushed the fix/patch_files_accept_multple_patches branch from 0bb40bc to 692477a Compare July 3, 2023 19:07
annasong20
annasong20 reviewed Jul 8, 2023
View reviewed changes
Copy link
Contributor

@annasong20 annasong20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes and additional test, @koba1t! Apart from the document discussion, I think we're almost ready!

@annasong20
Copy link
Contributor

annasong20 commented Jul 8, 2023

/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jul 8, 2023
@koba1t koba1t force-pushed the fix/patch_files_accept_multple_patches branch from 7595196 to 6aac386 Compare August 23, 2023 21:21
@koba1t
Copy link
Member Author

koba1t commented Aug 23, 2023

@annasong20
Thanks for your reviews!
I completely fix every problem!

Could you recheck this PR?

@koba1t koba1t mentioned this pull request Sep 1, 2023
Closed
2 tasks
annasong20
annasong20 approved these changes Sep 9, 2023
View reviewed changes
Copy link
Contributor

@annasong20 annasong20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR looks great! Thanks for iterating on this, @koba1t!!

Other than the error message that includes "target", the rest of the comments are minor. Feel free to lgtm yourself after removing "target" or I'd be happy to take another look. Also sorry for the delay in PR review.

/approve

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Sep 9, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: annasong20, koba1t

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 9, 2023
@koba1t
Copy link
Member Author

koba1t commented Sep 11, 2023

Hi @annasong20
Thanks for your review!!
I'm done fixing everything from your comment.

I'll merge this PR without your additional check.
If you find any problem, Please feel free to revert or send me a DM.

/lgtm

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Sep 11, 2023

@koba1t: you cannot LGTM your own PR.

Details

In response to this:

Hi @annasong20
Thanks for your review!!
I'm done fixing everything from your comment.

I'll merge this PR without your additional check.
If you find any problem, Please feel free to revert or send me a DM.

/lgtm

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.

@koba1t
Copy link
Member Author

koba1t commented Sep 11, 2023 •
edited
Loading

Sorry, @annasong20
I can't LGTM for my PR.
I'm afraid to ask, but could you check and add an LGTM to this PR?

@scila1996
Copy link

scila1996 commented Sep 13, 2023

I"m so expecting for this PR will be merged !!

@oaxiento
Copy link

oaxiento commented Sep 14, 2023

It'd be great to see this get merged. We absolutely need that feature. Thanks.

@annasong20
Copy link
Contributor

annasong20 commented Sep 15, 2023

Looks amazing!!! Thank you SO much for carrying this PR to fruition!
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 15, 2023
@k8s-ci-robot k8s-ci-robot merged commit 59696d1 into kubernetes-sigs:master Sep 15, 2023
@koba1t koba1t deleted the fix/patch_files_accept_multple_patches branch September 16, 2023 05:14
@macieklewkowicz macieklewkowicz mentioned this pull request Sep 28, 2023
Draft
@MaGaroo MaGaroo mentioned this pull request Oct 19, 2023
Merged
@stormqueen1990 stormqueen1990 mentioned this pull request Oct 21, 2023
Closed
@Stringls Stringls mentioned this pull request Nov 2, 2023
Merged
AndiDog added a commit to giantswarm/management-cluster-bases that referenced this pull request Nov 20, 2023
@AndiDog
Upgrade kustomize to fix support for multiple patches in one file
2fa7c56 
See kubernetes-sigs/kustomize#5194 which was released in https://github.com/kubernetes-sigs/kustomize/releases/tag/kustomize%2Fv5.2.1
@AndiDog AndiDog mentioned this pull request Nov 20, 2023
Closed
@jfavellar90 jfavellar90 mentioned this pull request Nov 24, 2023
Closed
@juliusvonkohout juliusvonkohout mentioned this pull request Mar 13, 2024
Merged
1 task
bartoszmajsak added a commit to bartoszmajsak/opendatahub-operator that referenced this pull request Aug 6, 2025
@bartoszmajsak
chore(deps): bumps kustomize libraries
21615cd 
Without updating kyaml and kustomize api it's not possible to use
patches containing several targets.

The issue is reported as component's condition:

```
trouble configuring builtin PatchTransformer with config: `                                                                                                                           │
path: remove-cert-manager.yaml                                                                                                                                                        │
`: unable to parse SM or JSON patch from [$patch: delete                                                                                                                              │
apiVersion: cert-manager.io/v1                                                                                                                                                        │
kind: Issuer                                                                                                                                                                          │
metadata:                                                                                                                                                                             │
  name: selfsigned-issuer                                                                                                                                                             │
  namespace: kserve                                                                                                                                                                   │
---                                                                                                                                                                                   │
$patch: delete
```

Related issue: kubernetes-sigs/kustomize#5194

Signed-off-by: Bartosz Majsak <bartosz.majsak@gmail.com>
@bartoszmajsak bartoszmajsak mentioned this pull request Aug 6, 2025
Merged
5 tasks
zdtsw pushed a commit to opendatahub-io/opendatahub-operator that referenced this pull request Aug 8, 2025
@bartoszmajsak
chore(deps): bumps kustomize libraries (#2259)
e5e63a0 
Without updating kyaml and kustomize api it's not possible to use
patches containing several targets.

The issue is reported as component's condition:

```
trouble configuring builtin PatchTransformer with config: `                                                                                                                           │
path: remove-cert-manager.yaml                                                                                                                                                        │
`: unable to parse SM or JSON patch from [$patch: delete                                                                                                                              │
apiVersion: cert-manager.io/v1                                                                                                                                                        │
kind: Issuer                                                                                                                                                                          │
metadata:                                                                                                                                                                             │
  name: selfsigned-issuer                                                                                                                                                             │
  namespace: kserve                                                                                                                                                                   │
---                                                                                                                                                                                   │
$patch: delete
```

Related issue: kubernetes-sigs/kustomize#5194

Signed-off-by: Bartosz Majsak <bartosz.majsak@gmail.com>
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/opendatahub-operator that referenced this pull request Aug 20, 2025
@bartoszmajsak
chore(deps): bumps kustomize libraries
7e2823c 
Without updating kyaml and kustomize api it's not possible to use
patches containing several targets.

The issue is reported as component's condition:

```
trouble configuring builtin PatchTransformer with config: `                                                                                                                           │
path: remove-cert-manager.yaml                                                                                                                                                        │
`: unable to parse SM or JSON patch from [$patch: delete                                                                                                                              │
apiVersion: cert-manager.io/v1                                                                                                                                                        │
kind: Issuer                                                                                                                                                                          │
metadata:                                                                                                                                                                             │
  name: selfsigned-issuer                                                                                                                                                             │
  namespace: kserve                                                                                                                                                                   │
---                                                                                                                                                                                   │
$patch: delete
```

Related issue: kubernetes-sigs/kustomize#5194

Signed-off-by: Bartosz Majsak <bartosz.majsak@gmail.com>
@zdtsw zdtsw mentioned this pull request Aug 21, 2025
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@natasha41575 natasha41575 Awaiting requested review from natasha41575

@yuwenma yuwenma Awaiting requested review from yuwenma

2 more reviewers

@pascalgn pascalgn pascalgn left review comments

@annasong20 annasong20 annasong20 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@annasong20 annasong20

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.

Projects

Kustomize Core Usability Bugs
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

patches should accept a patch file with multiple patches

7 participants

@koba1t @k8s-ci-robot @natasha41575 @annasong20 @scila1996 @oaxiento @pascalgn