Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kustomize edit fix needs to split multi-doc SMP patches into multiple files #5040

Closed
IvanovOleg opened this issue Feb 8, 2023 · 15 comments
Closed
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@IvanovOleg
Copy link

What happened?

I've tried my existing kustomize manifests with kustomize 5.0.0 and found a problem. I applied kustomize edit fix, but kustomize build command returns error:

Error: trouble configuring builtin PatchTransformer with config: `
path: secret.patch.yaml
`: unable to parse SM or JSON patch from [
manifest content
]

What did you expect to happen?

Kustomize build returns:

apiVersion: v1
kind: Secret
metadata:
  name: secret1
type: Opaque
stringData:
  admin.password: newpassword

---
apiVersion: v1
kind: Secret
metadata:
  name: secret2
type: Opaque
stringData:
  admin.user: newuser

How can we reproduce it (as minimally and precisely as possible)?

# kustomization.yaml
resources:
  - secret.yaml

patches:
  - path: secret.patch.yaml
# secret.yaml
apiVersion: v1
kind: Secret
metadata:
  name: secret1
type: Opaque
stringData:
  admin.password: password

---
apiVersion: v1
kind: Secret
metadata:
  name: secret2
type: Opaque
stringData:
  admin.user: user
# secret.patch.yaml
apiVersion: v1
kind: Secret
metadata:
  name: secret1
type: Opaque
stringData:
  admin.password: newpassword

---
apiVersion: v1
kind: Secret
metadata:
  name: secret2
type: Opaque
stringData:
  admin.user: newuser

Expected output

apiVersion: v1
kind: Secret
metadata:
  name: secret1
type: Opaque
stringData:
  admin.password: newpassword

---
apiVersion: v1
kind: Secret
metadata:
  name: secret2
type: Opaque
stringData:
  admin.user: newuser

Actual output

Error: trouble configuring builtin PatchTransformer with config: `
path: secret.patch.yaml
`: unable to parse SM or JSON patch from [apiVersion: v1
kind: Secret
metadata:
  name: secret1
type: Opaque
stringData:
  admin.password: newpassword

---
apiVersion: v1
kind: Secret
metadata:
  name: secret2
type: Opaque
stringData:
  admin.user: newuser
]

Kustomize version

5.0.0

Operating system

Linux

@IvanovOleg IvanovOleg added the kind/bug Categorizes issue or PR as related to a bug. label Feb 8, 2023
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Feb 8, 2023
@iamnoah
Copy link

iamnoah commented Feb 15, 2023

Also hit by this. Only workaround I have found is to explicitly specify patch targets in kustomization.yaml, but that a ton of refactoring for patches targeting multiple resources by name.

@KnVerey
Copy link
Contributor

KnVerey commented Feb 15, 2023

/retitle kustomize edit fix needs to split multi-doc SMP patches into multiple files
/triage accepted

The problem here is that kustomize edit fix is not correctly transforming the Kustomization--for older style patch file that contain multiple documents, we need to split them into separate files and list those files in the 'fixed' kustomization.

/assign @brianpursley

@k8s-ci-robot k8s-ci-robot changed the title Patching fail if a patch file contains several resources kustomize edit fix needs to split multi-doc SMP patches into multiple files Feb 15, 2023
@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 15, 2023
@iamnoah
Copy link

iamnoah commented Feb 16, 2023

IMO, splitting into multiple files is worse, even if they are kept in a subdirectory. I have to create multiple patch lines, I have to open multiple files when changes are correlated, etc. Nothing about this is an improvement over the existing behavior of patchStrategicMerge.

It does not seem terribly difficult to transform an existing SM patch, so why can't we keep the functionality? What if it was something like:

patches:
- path: my-multi-sm-patch.yaml
  target: { byGVKN: true }

@KnVerey
Copy link
Contributor

KnVerey commented Feb 16, 2023

That's an interesting idea; please open a separate issue for the feature request. One thing to address is the fact that the patch file could contain a JSON patch, which does not include GVKNN information.

@lindhe
Copy link

lindhe commented Jul 17, 2023

GI'm having a very similar problem with an even more minimal example. Do you think it's the same bug?

I'm running kustomize v5.1.0, but it seems like this bug exists in v4.5.7 too!

# kustomization.yaml
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
- cm.yaml

patches:
- patch: cm.patch.yaml
# cm.yaml
apiVersion: v1
kind: ConfigMap
metadata:
  name: cm
# cm.patch.yaml
apiVersion: v1
kind: ConfigMap
metadata:
  name: cm
data:
  foo: bar
$ kustomize build .
Error: trouble configuring builtin PatchTransformer with config: `
patch: cm.patch.yaml
`: unable to parse SM or JSON patch from [cm.patch.yaml]

It works fine if I change the kustomization.yaml to the following:

# kustomization.yaml
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
- cm.yaml

patches:
- patch: |-
    apiVersion: v1
    kind: ConfigMap
    metadata:
      name: cm
    data:
      foo: bar

@lindhe
Copy link

lindhe commented Jul 17, 2023

If you agree that this is the same bug as I experience, then this has nothing to do with multi-doc SMP patches. In that case, we should rename the Issue to something like "patch cannot take file path as argument".

@brianpursley
Copy link
Member

@lindhe Is patch supposed to take a path as an argument, or is that supposed to be only for an inline patch?

Can you change your kustomization.yaml to use path instead like this?

# kustomization.yaml
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
- cm.yaml

patches:
- path: cm.patch.yaml

@lindhe
Copy link

lindhe commented Jul 17, 2023

Yep, that works! Thanks a lot, I was pulling my hair not understanding why it didn't work. And as per usual it was a typo. :D

@gtirloni
Copy link

I just found this issue after running kustomize edit fix to fix some sort order warning.

Turned out I needed to split each resource patch in its own file and things started to work.

This is a very cryptic error message: unable to parse SM or JSON patch from

@husseinmimiHarri
Copy link

any updates for this ?

@kasvith
Copy link

kasvith commented Oct 7, 2023

Also hit by this, i wanted to set replica count of a deployment using patches and its giving same error

@natasha41575
Copy link
Contributor

For reasons described in #5059 (comment), I don't think this is needed anymore, as multi-doc SMP patches are now allowed under patchesStrategicMerge.

@mo-dayyeh
Copy link

mo-dayyeh commented Apr 5, 2024

I have a similar issue and only managed to solve it like this :

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- ../../base
- cronjobs.yaml
patches:
- path: agency-web.yaml
- path: agency-worker.yaml
- path: configmap.yaml

it complains about my cronjobs.yaml , if I put it under patches like the rest it fails like this and it does not process the rest of files :

error: trouble configuring builtin PatchTransformer with config: `
path: cronjobs.yaml
target:
  kind: CronJob
`: unable to parse SM or JSON patch from [apiVersion: batch/v1

My cronjobs.yaml contains multiple cronjobs in the same files , they aren't templated due to differences , I know its terrible but its staging and production is the same so is there any way ?

@stormqueen1990
Copy link
Member

My cronjobs.yaml contains multiple cronjobs in the same files , they aren't templated due to differences , I know its terrible but its staging and production is the same so is there any way ?

Hi there, @mo-dayyeh! Could you please file a separate issue and add a minimal example that allows us to reproduce the problem so we can investigate it?

@mo-dayyeh
Copy link

My cronjobs.yaml contains multiple cronjobs in the same files , they aren't templated due to differences , I know its terrible but its staging and production is the same so is there any way ?

Hi there, @mo-dayyeh! Could you please file a separate issue and add a minimal example that allows us to reproduce the problem so we can investigate it?

#5649

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/accepted Indicates an issue or PR is ready to be actively worked on.