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

Preserve formatting with in place writing #465

Open
tamalsaha opened this issue Jun 15, 2020 · 8 comments
Open

Preserve formatting with in place writing #465

tamalsaha opened this issue Jun 15, 2020 · 8 comments
Labels

Comments

@tamalsaha
Copy link

Describe the bug
yq w -1 command removes blank lines and changes spacing before comments.

Input Yaml
Concise yaml document(s) (as simple as possible to show the bug)
data1.yml:

# Log level for operator
logLevel: 3

# Annotations applied to operator deployment
annotations: {}

# Annotations passed to operator pod(s).
podAnnotations: {}

# Node labels for pod assignment
nodeSelector:  # +doc-gen:break
  beta.kubernetes.io/os: linux
  beta.kubernetes.io/arch: amd64

Command
The command you ran:

yq w -i test.yaml logLevel 4

Actual behavior

# Log level for operator
logLevel: 4
# Annotations applied to operator deployment
annotations: {}
# Annotations passed to operator pod(s).
podAnnotations: {}
# Node labels for pod assignment
nodeSelector: # +doc-gen:break
  beta.kubernetes.io/os: linux
  beta.kubernetes.io/arch: amd64

Expected behavior

# Log level for operator
logLevel: 4

# Annotations applied to operator deployment
annotations: {}

# Annotations passed to operator pod(s).
podAnnotations: {}

# Node labels for pod assignment
nodeSelector:  # +doc-gen:break
  beta.kubernetes.io/os: linux
  beta.kubernetes.io/arch: amd64

Additional context
Add any other context about the problem here.

$ yq --version
yq version 3.3.2
@tamalsaha tamalsaha added the bug label Jun 15, 2020
@mikefarah
Copy link
Owner

Yep - this is due to the underlying go-yaml parser, I see someone has already raised an issue there:

go-yaml/yaml#627

@tamalsaha
Copy link
Author

Thanks for the confirmation. Something else is also broken between v3.3.0 (working) and v3.3.2. I am running the following command:

yq r api/crds/installer.voyagermesh.com_voyageroperators.v1.yaml spec.versions[0].schema.openAPIV3Schema.properties.spec > /tmp/voyager-values.openapiv3_schema.yaml

You can see the files here: https://gist.github.com/tamalsaha/eeee8d9d2db6623bf1c77717ba3b2c2d

In v3.3.0, the formatting of strings remain unchanged (newlines are preserved). In v3.3.2, the newlines are gone.

@mikefarah
Copy link
Owner

Ah yeah I updated the go-yaml library in 3.3.1 which had a number of changes and fixes. That is an open issue against the lib (go-yaml/yaml#627)

@tamalsaha
Copy link
Author

Looks like go-yaml decided to not fix the issue, rather just document it. 😞

@aakrem
Copy link

aakrem commented Jul 22, 2020

Would be helpful to preserve the original formatting!

samvantran added a commit to mesosphere/charts that referenced this issue Aug 17, 2020
This commit updates the patch script to use a docker image as the
previous code did not run on OSX.

Also, due to the underlying go-parser lib used in the yq docker image,
updating a yaml file automatically reformats the yaml removing empty
newlines and converting multiline strings into one line[1]. This makes
it hard to see the actual needed change for patch7 which is just to
update the crd volumeClaimTemplate.metadata stanza.

Thus, patch7 is changed to apply two commits, one to simply let yq
format the file with no new changes and a second commit with the needed
change. Similar to what we do with chart revision changes.

Also updated the helper script to accept a commit message.

[1] mikefarah/yq#465 (comment)

Signed-off-by: Sam Tran <stran@mesosphere.com>
samvantran added a commit to mesosphere/charts that referenced this issue Aug 17, 2020
This commit updates the patch script to use a docker image as the
previous code did not run on OSX.

Also, due to the underlying go-parser lib used in the yq docker image,
updating a yaml file automatically reformats the yaml removing empty
newlines and converting multiline strings into one line[1]. This makes
it hard to see the actual needed change for patch7 which is just to
update the crd volumeClaimTemplate.metadata stanza.

Thus, patch7 is changed to apply two commits, one to simply let yq
format the file with no new changes and a second commit with the needed
change. Similar to what we do with chart revision changes.

Also updated the helper script to accept a commit message.

[1] mikefarah/yq#465 (comment)

Signed-off-by: Sam Tran <stran@mesosphere.com>
samvantran added a commit to mesosphere/charts that referenced this issue Aug 18, 2020
* chore: move all mesosphere files to patch

This moves all the mesosphere-related templates to the patch folder.
This makes it easier to update the operator with a copy and replace as
none of the mesosphere files will get lost.

Signed-off-by: Sam Tran <stran@mesosphere.com>

* feat: add patch files

This adds patch files that copy all the mesosphere files into the
templates/ folder. This way they get deployed as part of the chart.

It makes it easier to keep all the patches in one place and know what
changes are needed/differ from upstream.

Signed-off-by: Sam Tran <stran@mesosphere.com>

* feat: add helpers script for git commands

This adds a helper file so that all the patch files can use the
git_add_and_commit function which adds and commits all the files that
were changed as part of the patch script. This should make it easy to
see the commit history of what changed.

Signed-off-by: Sam Tran <stran@mesosphere.com>

* chore: update prometheus crd patch script

This commit updates the patch script to use a docker image as the
previous code did not run on OSX.

Also, due to the underlying go-parser lib used in the yq docker image,
updating a yaml file automatically reformats the yaml removing empty
newlines and converting multiline strings into one line[1]. This makes
it hard to see the actual needed change for patch7 which is just to
update the crd volumeClaimTemplate.metadata stanza.

Thus, patch7 is changed to apply two commits, one to simply let yq
format the file with no new changes and a second commit with the needed
change. Similar to what we do with chart revision changes.

Also updated the helper script to accept a commit message.

[1] mikefarah/yq#465 (comment)

Signed-off-by: Sam Tran <stran@mesosphere.com>

* feat: add update_operator script

This script does a sparse clone of helm/stable/prometheus-operator and
replaces all the files in our fork with latest upstream.

Then it applies all the patches in the /patch folder which should now
make updating our prometheus chart a bit easier.

Signed-off-by: Sam Tran <stran@mesosphere.com>

* chore: add newlines from PR review

Signed-off-by: Sam Tran <stran@mesosphere.com>

* chore: add doc for upgrading + patch comments

Add an UPGRADING doc so users know what to do to upgrade.
Add comments to the patch scripts so users know what each one does.
crd patch script needed explaining.

Signed-off-by: Sam Tran <stran@mesosphere.com>

* chore: add patch for checking CRD exists again

This adds back the check for `monitoring.coreos.com/v1` that was removed
from upstream.

Signed-off-by: Sam Tran <stran@mesosphere.com>

* chore: fixup shellcheck errors

Co-authored-by: Joe Julian <me@joejulian.name>
d2iq-dispatch added a commit to mesosphere/charts that referenced this issue Aug 18, 2020
… * chore: move all mesosphere files to patch This moves all the mesosphere-related templates to the patch folder. This makes it easier to update the operator with a copy and replace as none of the mesosphere files will get lost. Signed-off-by: Sam Tran <stran@mesosphere.com> * feat: add patch files This adds patch files that copy all the mesosphere files into the templates/ folder. This way they get deployed as part of the chart. It makes it easier to keep all the patches in one place and know what changes are needed/differ from upstream. Signed-off-by: Sam Tran <stran@mesosphere.com> * feat: add helpers script for git commands This adds a helper file so that all the patch files can use the git_add_and_commit function which adds and commits all the files that were changed as part of the patch script. This should make it easy to see the commit history of what changed. Signed-off-by: Sam Tran <stran@mesosphere.com> * chore: update prometheus crd patch script This commit updates the patch script to use a docker image as the previous code did not run on OSX. Also, due to the underlying go-parser lib used in the yq docker image, updating a yaml file automatically reformats the yaml removing empty newlines and converting multiline strings into one line[1]. This makes it hard to see the actual needed change for patch7 which is just to update the crd volumeClaimTemplate.metadata stanza. Thus, patch7 is changed to apply two commits, one to simply let yq format the file with no new changes and a second commit with the needed change. Similar to what we do with chart revision changes. Also updated the helper script to accept a commit message. [1] mikefarah/yq#465 (comment)  Signed-off-by: Sam Tran <stran@mesosphere.com>  * feat: add update_operator script  This script does a sparse clone of helm/stable/prometheus-operator and replaces all the files in our fork with latest upstream.  Then it applies all the patches in the /patch folder which should now make updating our prometheus chart a bit easier.  Signed-off-by: Sam Tran <stran@mesosphere.com>  * chore: add newlines from PR review  Signed-off-by: Sam Tran <stran@mesosphere.com>  * chore: add doc for upgrading + patch comments  Add an UPGRADING doc so users know what to do to upgrade. Add comments to the patch scripts so users know what each one does. crd patch script needed explaining.  Signed-off-by: Sam Tran <stran@mesosphere.com>  * chore: add patch for checking CRD exists again  This adds back the check for  that was removed from upstream.  Signed-off-by: Sam Tran <stran@mesosphere.com>  * chore: fixup shellcheck errors  Co-authored-by: Joe Julian <me@joejulian.name>
gracedo added a commit to mesosphere/charts that referenced this issue Aug 19, 2020
…ier (#742)  * chore: move all mesosphere files to patch  This moves all the mesosphere-related templates to the patch folder. This makes it easier to update the operator with a copy and replace as none of the mesosphere files will get lost.  Signed-off-by: Sam Tran <stran@mesosphere.com>  * feat: add patch files  This adds patch files that copy all the mesosphere files into the templates/ folder. This way they get deployed as part of the chart.  It makes it easier to keep all the patches in one place and know what changes are needed/differ from upstream.  Signed-off-by: Sam Tran <stran@mesosphere.com>  * feat: add helpers script for git commands  This adds a helper file so that all the patch files can use the git_add_and_commit function which adds and commits all the files that were changed as part of the patch script. This should make it easy to see the commit history of what changed.  Signed-off-by: Sam Tran <stran@mesosphere.com>  * chore: update prometheus crd patch script  This commit updates the patch script to use a docker image as the previous code did not run on OSX.  Also, due to the underlying go-parser lib used in the yq docker image, updating a yaml file automatically reformats the yaml removing empty newlines and converting multiline strings into one line[1]. This makes it hard to see the actual needed change for patch7 which is just to update the crd volumeClaimTemplate.metadata stanza.  Thus, patch7 is changed to apply two commits, one to simply let yq format the file with no new changes and a second commit with the needed change. Similar to what we do with chart revision changes.  Also updated the helper script to accept a commit message.  [1] mikefarah/yq#465 (comment)  Signed-off-by: Sam Tran <stran@mesosphere.com>  * feat: add update_operator script  This script does a sparse clone of helm/stable/prometheus-operator and replaces all the files in our fork with latest upstream.  Then it applies all the patches in the /patch folder which should now make updating our prometheus chart a bit easier.  Signed-off-by: Sam Tran <stran@mesosphere.com>  * chore: add newlines from PR review  Signed-off-by: Sam Tran <stran@mesosphere.com>  * chore: add doc for upgrading + patch comments  Add an UPGRADING doc so users know what to do to upgrade. Add comments to the patch scripts so users know what each one does. crd patch script needed explaining.  Signed-off-by: Sam Tran <stran@mesosphere.com>  * chore: add patch for checking CRD exists again  This adds back the check for  that was removed from upstream.  Signed-off-by: Sam Tran <stran@mesosphere.com>  * chore: fixup shellcheck errors  Co-authored-by: Joe Julian <me@joejulian.name>"

This reverts commit 54b4815.
gracedo added a commit to mesosphere/charts that referenced this issue Aug 19, 2020
* Revert "[prometheus-operator] chore: upgrade to v9.3.1 (#743)  * chore: copy upstream chart version: 9.3.1  * chore: apply patch_1_mesosphere_dashboards.sh  * chore: apply patch_2_mesosphere_cron_jobs.sh  * chore: apply patch_3_mesosphere_hooks.sh  * chore: apply patch_4_ingress_rbac.sh  * chore: apply patch_5_mesosphere_rules.sh  * chore: apply patch_6_mesosphere_values.sh  * chore: apply patch_7_prometheus_crd.sh - reformat yaml with yq (no new changes)  * chore: apply patch_7_prometheus_crd.sh - update volumeClaimTemplate  * chore: apply patch_8_prometheus_operator_crd.sh"

This reverts commit b88c046.

* Revert "[prometheus-operator] feat: add scripts to make upgrading easier (#742)  * chore: move all mesosphere files to patch  This moves all the mesosphere-related templates to the patch folder. This makes it easier to update the operator with a copy and replace as none of the mesosphere files will get lost.  Signed-off-by: Sam Tran <stran@mesosphere.com>  * feat: add patch files  This adds patch files that copy all the mesosphere files into the templates/ folder. This way they get deployed as part of the chart.  It makes it easier to keep all the patches in one place and know what changes are needed/differ from upstream.  Signed-off-by: Sam Tran <stran@mesosphere.com>  * feat: add helpers script for git commands  This adds a helper file so that all the patch files can use the git_add_and_commit function which adds and commits all the files that were changed as part of the patch script. This should make it easy to see the commit history of what changed.  Signed-off-by: Sam Tran <stran@mesosphere.com>  * chore: update prometheus crd patch script  This commit updates the patch script to use a docker image as the previous code did not run on OSX.  Also, due to the underlying go-parser lib used in the yq docker image, updating a yaml file automatically reformats the yaml removing empty newlines and converting multiline strings into one line[1]. This makes it hard to see the actual needed change for patch7 which is just to update the crd volumeClaimTemplate.metadata stanza.  Thus, patch7 is changed to apply two commits, one to simply let yq format the file with no new changes and a second commit with the needed change. Similar to what we do with chart revision changes.  Also updated the helper script to accept a commit message.  [1] mikefarah/yq#465 (comment)  Signed-off-by: Sam Tran <stran@mesosphere.com>  * feat: add update_operator script  This script does a sparse clone of helm/stable/prometheus-operator and replaces all the files in our fork with latest upstream.  Then it applies all the patches in the /patch folder which should now make updating our prometheus chart a bit easier.  Signed-off-by: Sam Tran <stran@mesosphere.com>  * chore: add newlines from PR review  Signed-off-by: Sam Tran <stran@mesosphere.com>  * chore: add doc for upgrading + patch comments  Add an UPGRADING doc so users know what to do to upgrade. Add comments to the patch scripts so users know what each one does. crd patch script needed explaining.  Signed-off-by: Sam Tran <stran@mesosphere.com>  * chore: add patch for checking CRD exists again  This adds back the check for  that was removed from upstream.  Signed-off-by: Sam Tran <stran@mesosphere.com>  * chore: fixup shellcheck errors  Co-authored-by: Joe Julian <me@joejulian.name>"

This reverts commit 54b4815.
leseb added a commit to leseb/rook that referenced this issue Apr 19, 2021
The schema generation was not generating the metadata properties
under the VolumeClaimTemplate type, resulting in the properties being
ignored by the api server. The preserve unknown fields tag was
not working recursively on the volumeClaimTemplates since the subtypes
were defined. Now we post-process the schema so we can preserve
the unknown fields for the volume claim templates metadata.

We now use 'yq' to hot-fix the CRDs, the only 'downside' is that 'yq'
will not preserve the existing formatting, this is a known problem:

mikefarah/yq#465

Signed-off-by: Sébastien Han <seb@redhat.com>
mergify bot pushed a commit to rook/rook that referenced this issue Apr 19, 2021
The schema generation was not generating the metadata properties
under the VolumeClaimTemplate type, resulting in the properties being
ignored by the api server. The preserve unknown fields tag was
not working recursively on the volumeClaimTemplates since the subtypes
were defined. Now we post-process the schema so we can preserve
the unknown fields for the volume claim templates metadata.

We now use 'yq' to hot-fix the CRDs, the only 'downside' is that 'yq'
will not preserve the existing formatting, this is a known problem:

mikefarah/yq#465

Signed-off-by: Sébastien Han <seb@redhat.com>
(cherry picked from commit 21d6001)
@twbecker
Copy link

It would also be nice to be able to preserve document markers (---). Seems like a great tool but unfortunately not being able to preserve virtually all the formatting is a dealbreaker for us.

subhamkrai pushed a commit to subhamkrai/rook that referenced this issue Oct 1, 2021
The schema generation was not generating the metadata properties
under the VolumeClaimTemplate type, resulting in the properties being
ignored by the api server. The preserve unknown fields tag was
not working recursively on the volumeClaimTemplates since the subtypes
were defined. Now we post-process the schema so we can preserve
the unknown fields for the volume claim templates metadata.

We now use 'yq' to hot-fix the CRDs, the only 'downside' is that 'yq'
will not preserve the existing formatting, this is a known problem:

mikefarah/yq#465

Signed-off-by: Sébastien Han <seb@redhat.com>
@markzporter
Copy link

Just wanted to second this, love the tool but also love our blank lines 😁

@Justinzobel
Copy link

It also changes our yml files from 4 spaces indentation to 2 :(

marcusdacoregio added a commit to spring-io/spring-security-release-tools that referenced this issue May 3, 2024
yq does not preserve formatting with in place writing, see mikefarah/yq#465
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants