fix: preserve original YAML formatting in resource output #6046
Conversation
k8s-ci-robot
added
the
do-not-merge/work-in-progress
|
Welcome @Rindrics! |
k8s-ci-robot
added
needs-ok-to-test
|
Hi @Rindrics. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Rindrics The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
k8s-ci-robot
added
size/M
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
Rindrics
force-pushed
the
issue-947-long-line
branch
from
a86c62a to
3372098
Compare
Previously, Resource.AsYAML() used yaml.JSONToYAML which caused two issues:
1. Long lines were automatically wrapped at ~80 characters, breaking
shell commands and template strings (e.g., Helm-style {{ }})
2. Quote styles were inconsistently modified - sometimes removed,
sometimes added
This change switches to using kyaml's encoder (RNode.MustString) which
preserves the original YAML formatting, including:
- Long lines remain on a single line
- Quote styles ('single', "double", or unquoted) are preserved
This is a more consistent behavior that respects the user's original
YAML structure.
Related to:
- kubernetes-sigs#947
- kubernetes-sigs#3969
Rindrics
force-pushed
the
issue-947-long-line
branch
from
3372098 to
faaf4d2
Compare
|
Hi @varshaprasad96 @koba1t |
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
Restore duplicate key detection that was accidentally removed during the MustString() migration. The detection uses Map() + json.Marshal() to avoid double serialization overhead while still catching duplicate keys that would cause issues. This ensures TestDuplicateKeys continues to work correctly and prevents invalid YAML with duplicate keys from being silently accepted.
|
This PR has multiple commits, and the default merge method is: merge. DetailsInstructions 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. |
After switching Resource.AsYAML() to use RNode.MustString(), YAML output now preserves the original field order from source YAML instead of alphabetical ordering. Update all golden files and test expectations to match the new output order. This is a follow-up to the MustString() migration in PR kubernetes-sigs#6046.
After switching Resource.AsYAML() to use RNode.MustString(), YAML output now preserves original quotes from the source YAML. The template uses %q which generates quoted values, and these quotes are now preserved in the output. Update the test expectation accordingly. This is a follow-up to the MustString() migration in PR kubernetes-sigs#6046.
After switching Resource.AsYAML() to use RNode.MustString(), YAML output now preserves the original field order from source YAML instead of alphabetical ordering. Update all golden files and test expectations to match the new output order. This is a follow-up to the MustString() migration in PR kubernetes-sigs#6046.
After switching Resource.AsYAML() to use RNode.MustString(), YAML output now preserves original quotes from the source YAML. The template uses %q which generates quoted values, and these quotes are now preserved in the output. Update the test expectation accordingly. This is a follow-up to the MustString() migration in PR kubernetes-sigs#6046.
|
@Rindrics: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions 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. I understand the commands that are listed here. |
After switching Resource.AsYAML() to use RNode.MustString(), YAML output now preserves the original field order from source YAML instead of alphabetical ordering. Update all golden files and test expectations to match the new output order. This is a follow-up to the MustString() migration in PR kubernetes-sigs#6046.
After switching Resource.AsYAML() to use RNode.MustString(), YAML output now preserves original quotes from the source YAML. The template uses %q which generates quoted values, and these quotes are now preserved in the output. Update the test expectation accordingly. This is a follow-up to the MustString() migration in PR kubernetes-sigs#6046.
|
thank you @koba1t for triaging this PR! As you can see, CI is currently failing. Since this change affects kustomize's output behavior, a large number of existing test expectations need to be updated. Given the volume of changes required, I believe adopting golden file testing would be the best approach here. I see two possible ways to introduce golden tests: Option A (my preference): Introduce golden tests before this PR
Option B: Introduce golden tests within this PR
I'd prefer Option A because it keeps the concerns clearly separated — the golden test migration can be reviewed independently with no behavior change, and this PR stays focused on the actual fix. This should make both reviews easier and less error-prone. If you agree, here's the plan I'd like to follow:
Would love to hear your thoughts. Thanks! |
3 participants
What this PR does
Changes
Resource.AsYAML()to use kyaml's encoder instead ofsigs.k8s.io/yaml.JSONToYAML.Why
The previous implementation had issues with:
Breaking changes
Quote styles are now preserved instead of being normalized. For example:
'quoted_value'stays as'quoted_value'(previously becamequoted_value)This is considered an improvement as it provides consistent, predictable output.
Related to: