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

Let kyaml keep white space formatting #3559

Closed
jonaskello opened this issue Feb 9, 2021 · 11 comments
Closed

Let kyaml keep white space formatting #3559

jonaskello opened this issue Feb 9, 2021 · 11 comments
Assignees
Labels
area/kyaml issues for kyaml kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@jonaskello
Copy link

Is your feature request related to a problem? Please describe.

Using fluxcd2 with ImageUpdateAutomatation it will rewrite the kustomization.yaml file using kyaml and in doing so all whitespace is altered. Flux will commit these changes back into a gitops repo. In that commit the whole file looks like it has changed but all changes are whitespace only except for one line. This makes it very hard to tell what really changed from just looking at the commit diff. See this issue for more background.

Describe the solution you'd like

I would like kyaml to preserve white space formatting so that flux can commit without whitespace changes.

@Shell32-Natsu Shell32-Natsu added area/kyaml issues for kyaml kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 9, 2021
@Shell32-Natsu
Copy link
Contributor

Can you provide an example that can demonstrate what kind of whitespace format is changed?

@jonaskello
Copy link
Author

Yes, see screenshot below (it only shows the head of the file, it is longer with more diff in total). The only real change, discounting whitespace, is the newtag field.

image

@stefanprodan
Copy link

kyaml alos borks ConfigMaps, ref: fluxcd/image-automation-controller#68 (comment)
102201067-e831f680-3ec5-11eb-9c6a-b3f3054c4bbe

@Shell32-Natsu
Copy link
Contributor

@monopole This is an example of the indentation differences in go-yaml.

@stefanprodan What's the version of kustomize are you using. The issue in your image is different from the one that @jonaskello reported.

@stefanprodan
Copy link

stefanprodan commented Feb 13, 2021

@Shell32-Natsu fluxcd/image-automation-controller doesn't uses kustomize as a whole, but kyaml as a library:

// Force downgrade to version used by kyaml.
replace gopkg.in/yaml.v3 => gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c

require (
	sigs.k8s.io/kustomize/kyaml v0.10.9
)

@Shell32-Natsu
Copy link
Contributor

@stefanprodan OK that should be a different issue.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

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 17, 2021
@jonaskello
Copy link
Author

/remove-lifecycle stale

This is still an issue AFAIK.

@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 May 17, 2021
@KnVerey
Copy link
Contributor

KnVerey commented Jun 2, 2021

This looks to me like the changes we saw when attempting to bump go-yaml in #3789. So it's ultimately being caused by kyaml using a different go-yaml version than your other tool, and go-yaml.v3 making what I'd consider to be a breaking formatting change.

Since Kustomize has long emitted the older format, we have been investigating the possibility of avoiding the change, which will likely be disruptive to many users. Relevant to this particular issue, it seems likely that that will require kyaml to interfere with the whitespace formatting goyaml currently forces. @Shell32-Natsu how feasible is that looking?

@monopole
Copy link
Contributor

monopole commented Jun 5, 2021

@stefanprodan would you please take a look at the discussion in #3946 and comment there? Thanks!

@KnVerey KnVerey self-assigned this Aug 4, 2021
@KnVerey
Copy link
Contributor

KnVerey commented Aug 4, 2021

Per #3946, the underlying yaml lib we use changed its indentation style, causing diffs like this. Kustomize pinned to an older version to keep the indentation stable, but we couldn't automatically do the same for folks using kyaml directly. As of kyaml 0.11.0, kyaml itself also uses kustomize's preferred indentation style. The next release of kyaml will contain #4043, which allows kyaml/yaml users to override the default style, and kyaml/kio to detect and retain the style in the source document.

/close

@KnVerey KnVerey closed this as completed Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kyaml issues for kyaml kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
None yet
Development

No branches or pull requests

7 participants