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

patchesStrategicMerge fix with go FixKustomizationPreMarshalling is broken #5499

Closed
Noamrok opened this issue Dec 24, 2023 · 12 comments
Closed
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@Noamrok
Copy link

Noamrok commented Dec 24, 2023

What happened?

We have the following kustomization.yaml file:

kind: Kustomization
apiVersion: kustomize.config.k8s.io/v1beta1
patchesStrategicMerge:
- gateway-patches/gcp-us-west1.yaml
- gateway-patches/azure-eastus.yaml
- gateway-patches/aws-eu-central-1.yaml
- health-domain-patches/dal3x-health-patch.yaml
- health-domain-patches/ric2x-health-patch.yaml
- health-domain-patches/fra1x-health-patch.yaml

We are running kustomize using the golang api v0.15.0, and using the FixKustomizationPreMarshalling function.
The output is:

patches:
  - patch: gateway-patches/gcp-us-west1.yaml
  - patch: gateway-patches/azure-eastus.yaml
  - patch: gateway-patches/aws-eu-central-1.yaml
  - patch: health-domain-patches/dal3x-health-patch.yaml
  - patch: health-domain-patches/ric2x-health-patch.yaml
  - patch: health-domain-patches/fra1x-health-patch.yaml

and the kustomize on this file returns the following error:

trouble configuring builtin PatchTransformer with config: `
patch: gateway-patches/gcp-us-west1.yaml
`: unable to parse SM or JSON patch from [patch: “gateway-patches/gcp-us-west1.yaml”]

What did you expect to happen?

We solved it by manually converting patchesStrategicMerge to patches:

patches:
  - path: gateway-patches/gcp-us-west1.yaml
  - path: gateway-patches/azure-eastus.yaml
  - path: gateway-patches/aws-eu-central-1.yaml
  - path: health-domain-patches/dal3x-health-patch.yaml
  - path: health-domain-patches/ric2x-health-patch.yaml
  - path: health-domain-patches/fra1x-health-patch.yaml

We think this is the intended output that suppose to be.

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

kind: Kustomization
apiVersion: kustomize.config.k8s.io/v1beta1
patchesStrategicMerge:
- aaa.yaml

Expected output

kind: Kustomization
apiVersion: kustomize.config.k8s.io/v1beta1
patches:
- path: aaa.yaml

Actual output

kind: Kustomization
apiVersion: kustomize.config.k8s.io/v1beta1
patches:
- patch: aaa.yaml

Kustomize version

v0.15.0

Operating system

None

@Noamrok Noamrok added the kind/bug Categorizes issue or PR as related to a bug. label Dec 24, 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 Dec 24, 2023
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

SIG CLI takes a lead on issue triage for this repo, but any Kubernetes member can accept issues by applying the triage/accepted label.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

@charles-chenzz
Copy link
Member

We are running kustomize using the golang api v0.15.0

what's the output of kustomize version ?
in the lastest version the patchesStrategicMerge is deprecated and we suggest using patches

@Noamrok
Copy link
Author

Noamrok commented Dec 27, 2023

We are running kustomize using the golang api v0.15.0

what's the output of kustomize version ? in the lastest version the patchesStrategicMerge is deprecated and we suggest using patches

On my PC it's:

{Version:kustomize/v4.5.4 GitCommit:cf3a452ddd6f83945d39d582243b8592ec627ae3 BuildDate:2022-03-28T23:12:45Z GoOs:linux GoArch:amd64}

But it is unrelated.
We aren't using the kustomize cli, we are using the go pkgs v0.15.0 to do the kustomize, we just recently upgraded from:
sigs.k8s.io/kustomize/api v0.11.4
sigs.k8s.io/kustomize/kyaml v0.13.6
to:
sigs.k8s.io/kustomize/api v0.15.0
sigs.k8s.io/kustomize/kyaml v0.15.0
So the code automatically helping us migrating from patchesStrategicMerge to patches. Although they are deprecated they arent removed (yet).
We will remove them eventually. But now we are using this function FixKustomizationPreMarshalling to fix it

@charles-chenzz
Copy link
Member

you're doing make kustomizer object and call Run to perform like kustomize cli right? I haven't try it before, I may need time to reproduce it to see what happen

@koba1t
Copy link
Member

koba1t commented Dec 28, 2023

@Noamrok
Currently, kustomize uses whether that field string is a readable file to determine whether to write a path or patch field.

if k.PatchesStrategicMerge != nil {
for _, patchStrategicMerge := range k.PatchesStrategicMerge {
// check this patch is file path select.
if _, err := fSys.ReadFile(string(patchStrategicMerge)); err == nil {
// path patch
k.Patches = append(k.Patches, Patch{Path: string(patchStrategicMerge)})
} else {
// inline string patch
k.Patches = append(k.Patches, Patch{Patch: string(patchStrategicMerge)})
}
}
k.PatchesStrategicMerge = nil
}

Have you put valid files when you exec fix?

@Noamrok
Copy link
Author

Noamrok commented Dec 28, 2023

Hi,
Thank you for the response.

My files are valid (suppose to be) and this is what I dont understand, in my case:

kind: Kustomization
apiVersion: kustomize.config.k8s.io/v1beta1
patchesStrategicMerge:
- gateway-patches/gcp-us-west1.yaml

the gateway-patches/gcp-us-west1.yaml is a path and not inline string so it suppose to do a path not a patch.
gateway-patches/gcp-us-west1.yaml file content is:

apiVersion: Z/v1alpha1
kind: A
metadata:
  name: B
  namespace: C
spec:
  lb:
    address: D

@koba1t
Copy link
Member

koba1t commented Dec 28, 2023

@Noamrok

I wasn't reproducing in my computer with kustomize cli v5.3.0.

kind: Kustomization
apiVersion: kustomize.config.k8s.io/v1beta1
patches:
- path: gateway-patches/gcp-us-west1.yaml

Maybe you failed to call FixKustomizationPreMarshalling or other functions.

@tal-sapan
Copy link

Hi,

After further investigating @Noamrok's issue I think the problem is that the current working directory wasn't set to the folder of the kustomization.yaml file, so the issue can be fixed by changing the current working directory.

I think it would be useful to be able to create a filesys.FileSystem implementation which can get the current working directory as input, similar to os.DirFS(), so it isn't necessary to actually switch the working directory before calling FixKustomizationPreMarshalling.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 30, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Apr 29, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

@k8s-ci-robot
Copy link
Contributor

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot closed this as not planned Won't fix, can't repro, duplicate, stale May 29, 2024
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. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
None yet
Development

No branches or pull requests

6 participants