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 set image" is too opinionated about the formatting and moves comments around #4481

Open
MichaelSp opened this issue Feb 21, 2022 · 13 comments
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@MichaelSp
Copy link

Describe the bug

After running kustomize edit set image. The comments

Files that can reproduce the issue

before

resources:
  # some important comment to below file:
  - https://raw.githubusercontent.com/argoproj/argo-cd/v2.2.5/manifests/ha/install.yaml
  - 
images:
- name: argo
  newName: argo
  newTag: "123"

running command kustomize edit set image argo=argo:234
Expected output

resources:
  # some important comments to below file:
  - https://raw.githubusercontent.com/argoproj/argo-cd/v2.2.5/manifests/ha/install.yaml

images:
- name: argo
  newName: argo
  newTag: "234"

I could also live with this output:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
  # some important comments to below file:
  - https://raw.githubusercontent.com/argoproj/argo-cd/v2.2.5/manifests/ha/install.yaml

images:
- name: argo
  newName: argo
  newTag: "234"

Actual output

  # some important comments to below file:
resources:
- https://raw.githubusercontent.com/argoproj/argo-cd/v2.2.5/manifests/ha/install.yaml
images:
- name: argo
  newName: argo
  newTag: "234"
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

Especially the comment has to stay in the line above the resource!

Kustomize version
{Version:kustomize/v4.5.2 GitCommit:9091919699baf1c5a5bf71b32ca73a993e98088b BuildDate:2022-02-09T23:19:28Z GoOs:darwin GoArch:amd64}

@MichaelSp MichaelSp added the kind/bug Categorizes issue or PR as related to a bug. label Feb 21, 2022
@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 21, 2022
@k8s-triage-robot
Copy link

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

This bot triages issues and PRs 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 or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR 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 May 22, 2022
@pietervincken
Copy link
Contributor

/remove-lifecycle stale

Hit us today. This hurts our automation and made us revert to YQ instead (which is far from ideal)

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 7, 2022
@annasong20
Copy link
Contributor

annasong20 commented Aug 5, 2022

I can reproduce this issue. Same thing happens on v4.5.7 as well. My output is mostly the same, except the second resource is an empty string instead of being removed. Please see below.

   # some important comment to below file:
resources:
- https://raw.githubusercontent.com/argoproj/argo-cd/v2.2.5/manifests/ha/install.yaml
- ""
images:
- name: argo
  newName: argo
  newTag: "234"
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

@natasha41575
Copy link
Contributor

natasha41575 commented Aug 5, 2022

/triage accepted

If anyone is interested in picking this up, I think one potential solution could be to use kyaml's CopyComments and SyncOrder functions to maintain the comments and field order of the kustomization.

This probably isn't specific to set image, I imagine all the kustomize edit commands have similar problems.

/help
/label "good first issue"

@k8s-ci-robot
Copy link
Contributor

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

/triage accepted

If anyone is interested in picking this up, I think one potential solution could be to use kyaml's CopyComments and SyncOrder functions to maintain the comments and field order of the kustomization.

/help
/label "good first issue"

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 k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 5, 2022
@natasha41575 natasha41575 added the good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. label Aug 5, 2022
@natasha41575
Copy link
Contributor

Thanks @annasong20 for confirming the issue!

@kanekoh
Copy link

kanekoh commented Aug 7, 2022

I'm new to contribute this repo. If it is ok, I'll try this issue.

@kanekoh
Copy link

kanekoh commented Aug 9, 2022

Is here right place to discuss how I fix this issue? Or should I discuss a question on slack?

sigs.k8s.io/kustomize/kyaml/yaml.Parse(string) removes some blank lines in kustomization file and it causes breaking some test cases.

For example, break lines in a Yaml file are removed when the Parse was called. Here is a failed test case I added.

--- FAIL: TestCommentOrder (0.13s)
    /home/cloud-user/go/src/k8s-sig.io/kustomize/kustomize/commands/internal/kustfile/kustomizationfile_test.go:239: Mismatch (-expected, +actual):
          []uint8(
          	"""
        - 	
        - 		
          	apiVersion: kustomize.config.k8s.io/v1beta1
          	kind: Kustomization
          	... // 3 identical lines
          	# some comments to below kubernetes file:
          	- https://example.com/kubernetes.yaml # comment
        - 	
          	images:
          	- name: example
          	... // 3 identical lines
          	"""

Is there a way to avoid above issue?

@k8s-triage-robot
Copy link

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

This bot triages PRs 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 PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR 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 Feb 8, 2023
@m00lecule
Copy link

m00lecule commented Mar 10, 2023

Hi,

Today I have also faced this issue. Probably we have similar case, when pipeline triggered by new repo tag has to commit new dockerimage version to separate repository. Also I have found out that this issue has been for a while without sufficient solution.

{Version:kustomize/v4.4.0 GitCommit:63ec6bdb3d737a7c66901828c5743656c49b60e1 BuildDate:2021-09-27T16:24:12Z GoOs:linux GoArch:amd64}

I have managed to bypass this issue using yq:

TAG=$TAG yq -i '(.images[] | select( .name == "image") | .newTag) = strenv(TAG)' kustomization.yaml

ref:
#3323

@gxwilkerson33
Copy link

/assign

@vaibhav2107
Copy link

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 27, 2023
@stormqueen1990
Copy link
Member

/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Development

Successfully merging a pull request may close this issue.