Update kustomize, use canonical json-patch v4 import #123339
Merged
+1,581
−2,976
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
release-note-none
The canonical import for json-patch v4 is gopkg.in/evanphx/json-patch.v4 (see https://github.com/evanphx/json-patch/blob/master/README.md#get-it for reference). Using the v4-specific path should also reduce the risk of unwanted v5 upgrade attempts, because they won't be offered as automated upgrades by dependency upgrade management tools, and they won't happen through indirect dependencies (see kubernetes#120327 for context). Signed-off-by: Stephen Kitt <skitt@redhat.com>
skitt
force-pushed
the
canonical-json-patch
branch
from
f798498
to
5300466
Compare
k8s-ci-robot
removed
the
needs-rebase
This was referenced May 29, 2024
liggitt
reviewed
May 29, 2024
| @@ -50,12 +49,12 @@ require ( | |||
| github.com/opencontainers/runc v1.1.12 | |||
| github.com/opencontainers/selinux v1.11.0 | |||
| github.com/pkg/errors v0.9.1 | |||
| github.com/pmezard/go-difflib v1.0.0 | |||
| github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 | |||
There was a problem hiding this comment.
is it intentional this is pulling in a version between tags? do we know if this library expects to be used on a non-tagged version and is stable there?
There was a problem hiding this comment.
I don’t think it’s intentional; the bump ultimately comes from https://github.com/hashicorp/consul (through https://github.com/spf13/viper) where it was first done in hashicorp/consul#15959 and then propagated.
There won’t be a non-tagged version of the library, it’s unmaintained; in fact the difference between 1.0.0 and the commit referenced above is just a couple of documentation updates to fix an example and add a notice about the end of the project’s maintenance.
liggitt
reviewed
May 29, 2024
| @@ -139,7 +139,7 @@ require ( | |||
| github.com/containerd/ttrpc v1.2.2 // indirect | |||
| github.com/coredns/caddy v1.1.1 // indirect | |||
| github.com/coreos/go-semver v0.3.1 // indirect | |||
| github.com/davecgh/go-spew v1.1.1 // indirect | |||
| github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect | |||
There was a problem hiding this comment.
is it intentional this is pulling in a version between tags? do we know if this library expects to be used on a non-tagged version and is stable there?
There was a problem hiding this comment.
Same as above, this comes from hashicorp/consul#15959.
I also doubt there will be a new release of go-spew; the difference between 1.1.1 and the commit pulled in above is only a CI change to test using Go 1.11 (!).
|
I realized we have 3 parallel PRs attempting to update kustomize... (#124812 #124217 #123339) This PR is the oldest and also took care of switching k/k to the v4 json-patch library to avoid using two names for the same library, so I'm inclined to merge this one. Just a couple questions on pulling in untagged versions of a couple deps, then lgtm |
skitt
added a commit
to skitt/consul
that referenced
this pull request
May 29, 2024
In both go-difflib and go-spew, the changes since the last release are insignificant (only touching documentation or CI): * pmezard/go-difflib@v1.0.0...master * davecgh/go-spew@v1.1.1...master This reverts the dependencies to the last release, as far as possible. The root go.mod and troubleshoot/go.mod will have to be revisited after the next release of the relevant consul sub-modules. See also the discussion in kubernetes/kubernetes#123339 Signed-off-by: Stephen Kitt <skitt@redhat.com>
4 tasks
skitt
added a commit
to skitt/consul
that referenced
this pull request
May 29, 2024
In both go-difflib and go-spew, the changes since the last release are insignificant (only touching documentation or CI): * pmezard/go-difflib@v1.0.0...master * davecgh/go-spew@v1.1.1...master This reverts the dependencies to the last release, as far as possible, in MPL-2.0-licensed components. See also the discussion in kubernetes/kubernetes#123339 Signed-off-by: Stephen Kitt <skitt@redhat.com>
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 59bc70d4b2cf4acfd3f3955b369be015aafe485f
|
k8s-ci-robot
added
the
approved
|
/retest Looks like max depth went up a bit. |
|
/triage accepted |
k8s-ci-robot
added
triage/accepted
skitt
added a commit
to skitt/kustomize
that referenced
this pull request
Jun 6, 2024
spf13/viper recently split its remote package into a separate module, greatly simplifying its module dependency tree. This doesn't affect package dependencies but reduces the constraints on module versions (see the discussion on go-difflib and go-spew in kubernetes/kubernetes#123339 for an example of the knock-on effects). Signed-off-by: Stephen Kitt <skitt@redhat.com>
skitt
added a commit
to skitt/viper
that referenced
this pull request
Jun 6, 2024
Now that consul/api is gone from the module dependency tree, these dependencies can be reverted to their latest releases; the changes since then are only CI-related or documentation updates, which aren't relevant for downstream users. (This is very minor but avoids questions like kubernetes/kubernetes#123339 (comment) about the use of untagged dependencies.) Signed-off-by: Stephen Kitt <skitt@redhat.com>
sagikazarmark
pushed a commit
to spf13/viper
that referenced
this pull request
Jun 6, 2024
Now that consul/api is gone from the module dependency tree, these dependencies can be reverted to their latest releases; the changes since then are only CI-related or documentation updates, which aren't relevant for downstream users. (This is very minor but avoids questions like kubernetes/kubernetes#123339 (comment) about the use of untagged dependencies.) Signed-off-by: Stephen Kitt <skitt@redhat.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
The canonical import for json-patch v4 is
gopkg.in/evanphx/json-patch.v4 (see
https://github.com/evanphx/json-patch/blob/master/README.md#get-it for reference).
Using the v4-specific path should also reduce the risk of unwanted v5 upgrade attempts (see
#120327 for context).
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
This duplicates json-patch in
vendor; the remaining dependency comes from kustomize, and will be fixed by kubernetes-sigs/kustomize#5541.Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: