bump sigs.k8s.io/yaml to v1.3.0

[zhouhaibing089]

v1.3.0 was recently released and upgrades gopkg.in/yaml.v2 to v2.4.0.
This version fixes the inconsistent long line wrapping issue.

ALLOW_MODULE_SPAN

[k8s-ci-robot]

Hi @zhouhaibing089. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zhouhaibing089
To complete the pull request process, please assign knverey after the PR has been reviewed.
You can assign the PR to them by writing /assign @knverey in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[natasha41575]

/ok-to-test

This version fixes the inconsistent long line wrapping issue.

Could you link the tracking issue or provide an example in your PR comment?

EDIT: Never mind I see it linked below, #3969. Thanks!

[natasha41575]

/retest

[zhouhaibing089]

/hold

I have not tested it yet. Any suggestions on how should I put an automated test?

[natasha41575]

@zhouhaibing089 our e2e tests go in here https://github.com/kubernetes-sigs/kustomize/tree/master/api/krusty

I would suggest you create a new file in there (or add a test to an existing file). You can view the other tests in that folder as examples for how to write your test.

Perhaps it could be a very simple test with the same kustomization and resource file as in the issue #3969.

[zhouhaibing089]

The function that matters here is FutureLineWrap. Does this change(call this function) require any further review?

[zhouhaibing089]

To summarize:

gopkg.in/yaml.v2 was already upgraded to v2.4.0 at #3667. From 2.3.0 to 2.4.0, there is a behavior change, the line length is updated from infinite to 80, that's the reason why the long line wraps now.

The patch here really doesn't change anything unless it calls yaml.FutureLineWrap to restore the behavior of non wrapping scenario.

[akram]

The patch here really doesn't change anything unless it calls yaml.FutureLineWrap to restore the behavior of non wrapping scenario.

yaml. FutureLineWrap must be called to ensure that Line Wrapping is not happening with this version.
The function is called this way because it helps ensuring smooth migration to the future yaml.v3 api that offers more flexibility on how to control line wrapping.

So, to make that kustomize does not wrap long lines, there should be a call somewhere in an init method to yaml. FutureLineWrap.

[francesco-beccaria]

I tested that adding a call to yaml.FutureLineWrap in an init method in kustomize/main.go works and stops the wrapping of long lines.

If this PR is not gonna be updated I'd be happy to create a new one.
If so, where exactly should the call to yaml.FutureLineWrap go?
Is it "correct"/"fine" how I've added it?

Here's my test diff as compared to the current state of this PR:

$ git --no-pager diff
diff --git a/kustomize/go.mod b/kustomize/go.mod
index e89ce614d..5a918f8ae 100644
--- a/kustomize/go.mod
+++ b/kustomize/go.mod
@@ -8,6 +8,7 @@ require (
 	github.com/spf13/cobra v1.0.0
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.7.0
+	gopkg.in/yaml.v2 v2.4.0 // indirect
 	sigs.k8s.io/kustomize/api v0.10.0
 	sigs.k8s.io/kustomize/cmd/config v0.10.1
 	sigs.k8s.io/kustomize/kyaml v0.12.0
diff --git a/kustomize/main.go b/kustomize/main.go
index 059fd2fbd..0a761b6c9 100644
--- a/kustomize/main.go
+++ b/kustomize/main.go
@@ -7,9 +7,14 @@ package main
 import (
 	"os"
 
+	"gopkg.in/yaml.v2"
 	"sigs.k8s.io/kustomize/kustomize/v4/commands"
 )
 
+func init() {
+	yaml.FutureLineWrap()
+}
+
 func main() {
 	if err := commands.NewDefaultCommand().Execute(); err != nil {
 		os.Exit(1)

[akram]

@zhouhaibing089 can you apply the patch that @francesco-beccaria proposed and tested?
We need some validation from a core contributor team, but for me, having it in init of main.go looks good.

[zhouhaibing089]

I'm actually interested to learn what is broken with line wrap? Or is it just a preference?

From what I understand, either with line wrap or without wrap, the YAML is still essentially the same, or isn't it?

[k8s-ci-robot]

@zhouhaibing089: PR needs rebase.

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-triage-robot]

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

[karlschriek]

I would just like to point out this comment #3969 (comment). My understanding is that line wrapping is currently the desired default behaviour, and is probably also why sigs.k8s.io/yaml completely skipped go-yaml 2.3.0 (https://github.com/go-yaml/yaml/releases/tag/v2.3.0) which set line lengths to inifinity and went straight to 2.4.0 (https://github.com/go-yaml/yaml/releases/tag/v2.4.0) which reverted the change but added the possibility to to reactivate it using yaml.FutureLineWrap()

Here is the text from the go-yaml 2.4.0 release

It was clearly a mistake to accept the default formatting change in v2,
and now there's no ideal choice. Either we revert the change and break
some projects twice, or we keep the change and break some projects once.
Given the report comes from Kubernetes, which has a relevant community
and code size, we'll revert it. At the same time, to simplify the life
of those that already started migrating towards the v3 behavior, a new
FutureLineWrap function is being introduced to trivially preserve the
new behavior where desired.

The v3 branch is not affected by this, and will retain the default
non-wrapping behavior. It will also be changed soon to support per
arbitrary line-wrapping for individual encoding operations.

Thanks to everyone who offered code and ideas, and apologies for
the trouble.

Might I suggest that we instead introduce a flag and have the init only trigger if that flag is set? For example kustomize build --future-line-wrap .

[k8s-triage-robot]

The Kubernetes project currently lacks enough active 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 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 rotten

[KnVerey]

Reading the comment history here, it sounds like we wanted a test showing the line wrapping change, but that doesn't actually happen by default with this dependency bump. If that's the case, let's rebase this PR and merge it with the bump only. If we decide to enable infinite line length (which very likely isn't possible until we do a major version bump), then it should be a separate PR from the dependency upgrade.

[k8s-triage-robot]

The Kubernetes project currently lacks enough active 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:

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

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

/close

[k8s-ci-robot]

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active 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:

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

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

/close

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.