downgrade go-difflib and go-spew to tagged releases #5830

thaJeztah · 2025-01-03T22:41:05Z

relates to:

commit d35edbf updated these dependencies to untagged versions. The diff in both dependencies show that there's no code changes, and it's unlikely for those modules to do new releases.

Unfortunate, because of that change all projects depending on kubernetes or any of it's modules now had to upgrade to unreleased versions of these.

This patch reverts those updates (but it may take some time before all other projects can be reverted).

k8s-ci-robot · 2025-01-03T22:41:14Z

Welcome @thaJeztah!

It looks like this is your first PR to kubernetes-sigs/kustomize 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kustomize has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2025-01-03T22:41:15Z

Hi @thaJeztah. 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-sigs/prow repository.

dims · 2025-01-04T00:26:00Z

/ok-to-test

@koba1t yes please to the downgrade proposed by @thaJeztah - we should try to use as much of tagged releases as possible unless absolutely necessary (for things in our control!). If we do use untagged release, we should treat that as tech debt and try to close it by working with those repos that we are using (by trying to get them to tag things for us to use)

These dependencies were updated to "master" in some modules we depend on, but have no code-changes since their last release. Unfortunately, this also causes a ripple effect, forcing all users of the containerd module to also update these dependencies to an unrelease / un-tagged version. Both these dependencies will unlikely do a new release in the near future, so exclude these versions so that we can downgrade to the current release. For additional details, see [this PR][1] and links mentioned in it. [1]: kubernetes-sigs/kustomize#5830 (comment) Signed-off-by: Sebastiaan van Stijn <[email protected]>

stormqueen1990

/lgtm

koba1t · 2025-01-04T12:18:42Z

/assign

thaJeztah · 2025-01-04T14:56:18Z

For reviewers; make sure I did the changes correctly; I wasn't sure what the correct procedure was, so my steps were;

did a "find and replace" for the unwanted versions to revert them
ran make test-go-mod (which I saw did a go mod tidy for all modules)

I assume CI should flag if I did it wrong, but just in case 😅

koba1t · 2025-01-04T15:08:43Z

Looks like this problem was introduced at this commit in PR #5541.
The commit d35edbf was just synced version for every go.mod file.

@liggitt was mentioned this dependency. Maybe I should have downgraded this at this time.
kubernetes/kubernetes#124217 (comment)

koba1t · 2025-01-04T15:12:14Z

HI @thaJeztah!

Could you try to execute make workspace-sync command?

kustomize/Makefile

Lines 181 to 185 in 214aa2a

    
           # Pushes dependencies in the go.work file back to go.mod files of each workspace module. 
        
           .PHONY: workspace-sync 
        
           workspace-sync: 
        
           	go work sync 
        
           	./hack/doGoMod.sh tidy

That syncs go module version for every go.mod and go.sum files in this repo.

Signed-off-by: Sebastiaan van Stijn <[email protected]>

commit d35edbf updated these dependencies to untagged versions. The diff in both dependencies show that there's no code changes, and it's unlikely for those modules to do new releases. Unfortunate, because of that change all projects depending on kubernetes or any of it's modules now had to upgrade to unreleased versions of these. This patch reverts those updates (but it may take some time before all other projects can be reverted). Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah · 2025-01-04T19:44:01Z

Ah, right, I see the problem, and that explains why that other PR chose to upgrade it even though it was not a dependency of protobuf.

And digging deeper, it's indeed related to spf13/viper; there's two modules that depend on that;

hack
cmd/gorepomod (through the hack module)

cd hack
go mod graph | grep ' github.com/davecgh/[email protected]'
sigs.k8s.io/kustomize/cmd/gorepomod github.com/davecgh/[email protected]
sigs.k8s.io/kustomize/hack github.com/davecgh/[email protected]
github.com/spf13/[email protected] github.com/davecgh/[email protected]


cd ../cmd/gorepomod
go mod graph | grep ' github.com/davecgh/[email protected]'
sigs.k8s.io/kustomize/cmd/gorepomod github.com/davecgh/[email protected]
sigs.k8s.io/kustomize/hack github.com/davecgh/[email protected]
github.com/spf13/[email protected] github.com/davecgh/[email protected]

So the actual commit introducing this seems to be ed2ca23 which brings us back to the PR introducing it there (spf13/viper#1574), and the merged, but not yet released fix;

Revert to go-difflib and go-spew releases spf13/viper#1861

If all modules must be at the same version, I can add an exclude to the hack/go.mod. This is one of the rare occasions where exclude works, because we knoww both go-spec and go-difflib are unlikely to receive updates in the near future, so we can exluce these specific commits / pseudo-versions, which should downgrade them to their latest release without having to upgrade viper and other things.

k8s-ci-robot · 2025-01-04T19:44:24Z

This PR has multiple commits, and the default merge method is: merge.
You can request commits to be squashed using the label: tide/merge-method-squash

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-sigs/prow repository.

thaJeztah · 2025-01-04T19:45:10Z

api/go.mod

-	golang.org/x/sys v0.23.0 // indirect
+	golang.org/x/sys v0.28.0 // indirect


Pushed an extra commit to do a workspace sync before the downgrade, because I got a diff that was unrelated to the downgrade

thaJeztah · 2025-01-04T19:46:26Z

cmd/gorepomod/go.mod

+exclude (
+	// These dependencies were updated to "master" in spf13/viper, but have no
+	// code-changes since the last release. While a fix was merged in viper,
+	// it's not released yet, and it may take time before other (indirect)
+	// dependencies also downgraded.
+	//
+	// Exclude these versions so that go modules picks their latest release
+	// before that. For additional details, see; https://github.com/kubernetes-sigs/kustomize/pull/5830
+	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
+	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2
+)


Added a exclude in this module

thaJeztah · 2025-01-04T19:48:00Z

hack/go.mod

+exclude (
+	// These dependencies were updated to "master" in spf13/viper, but have no
+	// code-changes since the last release. While a fix was merged in viper,
+	// it's not released yet, and it may take time before other (indirect)
+	// dependencies also downgraded.
+	//
+	// Exclude these versions so that go modules picks their latest release
+	// before that. For additional details, see; https://github.com/kubernetes-sigs/kustomize/pull/5830
+	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
+	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2
+)


And here (there other module depending on viper)

These dependencies were updated to "master" in some modules we depend on, but have no code-changes since their last release. Unfortunately, this also causes a ripple effect, forcing all users of the containerd module to also update these dependencies to an unrelease / un-tagged version. Both these dependencies will unlikely do a new release in the near future, so exclude these versions so that we can downgrade to the current release. For additional details, see [this PR][1] and links mentioned in it. [1]: kubernetes-sigs/kustomize#5830 (comment) Signed-off-by: Sebastiaan van Stijn <[email protected]>

skitt · 2025-01-06T13:53:46Z

Thanks for taking care of this, I was waiting for a viper release but it makes sense to add excludes until that happens.

skitt · 2025-01-06T14:38:42Z

#5832 shows what a viper bump would look like. I vote for getting this in first so we don’t have to wait for the release (see spf13/viper#1971).

thaJeztah · 2025-01-06T14:53:39Z

Thanks for looking @skitt - I saw you went down the same rabbit hole as I did 😂 🫶 (but I missed the viper <-> kustomize connection).

Yeah, my main intent was to try and cut the cycle with "minimal" changes, knowing that k8s, containerd, moby have a very complex dependency tree; I opened a similar PR in containerd to do the same / with the same intent.

liggitt · 2025-01-06T16:33:38Z

/lgtm
/approve

(note to observers, the only reason the exclude directives are an ok resolution to the viper issue is that the two modules using them are not ones we expect to become transitive dependencies for any other modules)

koba1t · 2025-01-07T02:46:58Z

I believe we should not use exclude directives basically because it could potentially break dependencies.
However, the revert is being done on the https://github.com/spf13/viper side. I think it is acceptable to use exclude only until that release.
spf13/viper#1861

/approve

k8s-ci-robot · 2025-01-07T02:47:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: koba1t, liggitt, thaJeztah

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [koba1t]

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

thaJeztah · 2025-01-07T10:12:57Z

I believe we should not use exclude directives basically because it could potentially break dependencies. However, the revert is being done on the https://github.com/spf13/viper side. I think it is acceptable to use exclude only until that release. spf13/viper#1861

/approve

Yeah, I usually would avoid replace as well, but in both these cases there were no code-changes (only changes in README and their CI config), so excluding these specific commits would not impact any module depending on it;

These dependencies were updated to "master" in some modules we depend on, but have no code-changes since their last release. Unfortunately, this also causes a ripple effect, forcing all users of the containerd module to also update these dependencies to an unrelease / un-tagged version. Both these dependencies will unlikely do a new release in the near future, so exclude these versions so that we can downgrade to the current release. For additional details, see [this PR][1] and links mentioned in it. [1]: kubernetes-sigs/kustomize#5830 (comment) Signed-off-by: Sebastiaan van Stijn <[email protected]>

koba1t · 2025-01-15T01:31:27Z

Hi @thaJeztah @dims
We made a new kustomize release tag. Please use those release tags to resolve this problem.

https://github.com/kubernetes-sigs/kustomize/releases/tag/kustomize%2Fv5.6.0
https://github.com/kubernetes-sigs/kustomize/releases/tag/kyaml%2Fv0.19.0
https://github.com/kubernetes-sigs/kustomize/releases/tag/cmd%2Fconfig%2Fv0.19.0
https://github.com/kubernetes-sigs/kustomize/releases/tag/api%2Fv0.19.0

dims · 2025-01-15T01:34:22Z

@koba1t thank you! Done! kubernetes/kubernetes#129622

k8s-ci-robot requested review from stormqueen1990 and varshaprasad96 January 3, 2025 22:41

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 3, 2025

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 3, 2025

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 3, 2025

thaJeztah mentioned this pull request Jan 3, 2025

Update kustomize, use canonical json-patch v4 import kubernetes/kubernetes#123339

Merged

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 4, 2025

This was referenced Jan 4, 2025

downgrade go-difflib and go-spew to tagged releases containerd/containerd#11220

Merged

Update to containerd 2.0 moby/moby#48872

Open

stormqueen1990 reviewed Jan 4, 2025

View reviewed changes

k8s-ci-robot assigned stormqueen1990 Jan 4, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 4, 2025

k8s-ci-robot assigned koba1t Jan 4, 2025

thaJeztah added 2 commits January 4, 2025 20:33

workspace sync

a42cc3d

Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah force-pushed the downgrade_tagged_releases branch from b6ef55b to 6bcbf9d Compare January 4, 2025 19:44

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 4, 2025

thaJeztah commented Jan 4, 2025

View reviewed changes

k8s-ci-robot assigned liggitt Jan 6, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 6, 2025

koba1t added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jan 7, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 7, 2025

k8s-ci-robot merged commit 2867f35 into kubernetes-sigs:master Jan 7, 2025
10 checks passed

thaJeztah deleted the downgrade_tagged_releases branch January 7, 2025 10:13

crazy-max mentioned this pull request Jan 14, 2025

downgrade go-difflib and go-spew to tagged releases moby/buildkit#5654

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

downgrade go-difflib and go-spew to tagged releases #5830

downgrade go-difflib and go-spew to tagged releases #5830

thaJeztah commented Jan 3, 2025 •

edited

Loading

k8s-ci-robot commented Jan 3, 2025

k8s-ci-robot commented Jan 3, 2025

dims commented Jan 4, 2025

stormqueen1990 left a comment

koba1t commented Jan 4, 2025

thaJeztah commented Jan 4, 2025

koba1t commented Jan 4, 2025 •

edited

Loading

koba1t commented Jan 4, 2025

thaJeztah commented Jan 4, 2025

k8s-ci-robot commented Jan 4, 2025

thaJeztah Jan 4, 2025

thaJeztah Jan 4, 2025

thaJeztah Jan 4, 2025

skitt commented Jan 6, 2025

skitt commented Jan 6, 2025

thaJeztah commented Jan 6, 2025

liggitt commented Jan 6, 2025

koba1t commented Jan 7, 2025

k8s-ci-robot commented Jan 7, 2025

thaJeztah commented Jan 7, 2025

koba1t commented Jan 15, 2025

dims commented Jan 15, 2025

		golang.org/x/sys v0.23.0 // indirect
		golang.org/x/sys v0.28.0 // indirect

downgrade go-difflib and go-spew to tagged releases #5830

downgrade go-difflib and go-spew to tagged releases #5830

Conversation

thaJeztah commented Jan 3, 2025 • edited Loading

k8s-ci-robot commented Jan 3, 2025

k8s-ci-robot commented Jan 3, 2025

dims commented Jan 4, 2025

stormqueen1990 left a comment

Choose a reason for hiding this comment

koba1t commented Jan 4, 2025

thaJeztah commented Jan 4, 2025

koba1t commented Jan 4, 2025 • edited Loading

koba1t commented Jan 4, 2025

thaJeztah commented Jan 4, 2025

k8s-ci-robot commented Jan 4, 2025

thaJeztah Jan 4, 2025

Choose a reason for hiding this comment

thaJeztah Jan 4, 2025

Choose a reason for hiding this comment

thaJeztah Jan 4, 2025

Choose a reason for hiding this comment

skitt commented Jan 6, 2025

skitt commented Jan 6, 2025

thaJeztah commented Jan 6, 2025

liggitt commented Jan 6, 2025

koba1t commented Jan 7, 2025

k8s-ci-robot commented Jan 7, 2025

thaJeztah commented Jan 7, 2025

koba1t commented Jan 15, 2025

dims commented Jan 15, 2025

thaJeztah commented Jan 3, 2025 •

edited

Loading

koba1t commented Jan 4, 2025 •

edited

Loading