fix the hash of gomodules.xyz/jsonpatch/v2 #13918
Conversation
|
Welcome @kalbasit! |
k8s-ci-robot
added
cncf-cla: yes
|
Hi @kalbasit. Thanks for your PR. I'm waiting for a kubernetes 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. 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
added
sig/testing
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
CI/Bazel disagree with my findings 🤔 I'll double-check locally through Bazel. EDIT: I can replicate the CI test result locally with Bazel. It seems that the result is inconsistent between Go directly and Go through Bazel. Output of local bazel test //hack:verify-depsDigging a little deeper, I thought that it could be a problem in my environment. Specifically the Go version coming from nixpkgs could be problematic. However, it seems that the offical Go image agrees with the Nix version over Bazel. Output of the docker build with my patchOutput of the docker build without my patch (test-infra at b164399) |
|
/cc @jayconrod |
|
@kalbasit: GitHub didn't allow me to request PR reviews from the following users: jayconrod. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
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. |
|
/cc @fejta |
|
The correct hash is @kalbasit If you're seeing A number of things can cause module cache corruption: the upstream tag could have changed (and changed back?), data could be corrupted in memory or on disk, and of course bugs. If you're able to reproduce this with an empty module cache, please file an issue in golang/go and cc me. |
|
Easier way to verify a specific sum: https://sum.golang.org/lookup/gomodules.xyz/jsonpatch/v2@v2.0.0 The Go command hits that endpoint when it starts to verify a new module. It downloads other parts of a Merkle tree to ensure the hashes haven't been tampered with, but that's the start of it. More details in the sumdb proposal. |
|
/uncc |
|
Different golang versions sometimes produce different hashes |
|
@jayconrod I can replicate using the official Go docker image version 1.12.7 with no cache. Can you replicate that? |
|
I cannot replicate with version You're right @fejta However! Bazel is using version 1.12.7, and it's computing the hash as seen by 1.13, how come? |
@fejta This isn't true in general. Between 1.11.3 and 1.11.4, we shipped a "fix" to the way module zip files were prepared that affected the hashes. This was a mistake: one we will be very careful not to repeat. The algorithm for preparing and hashing module zip files is now basically set in stone. sum.golang.org serves signed hashes that aren't specific to any version of Go.
@kalbasit I was able to reproduce this using the command you pasted. I was also able to reproduce this in go1.13beta1 with Looking at the differences between the zip files, I think the author of this module changed the commit that diff -urN proxy/gomodules.xyz/jsonpatch/v2@v2.0.0/.gitignore direct/gomodules.xyz/jsonpatch/v2@v2.0.0/.gitignore
--- proxy/gomodules.xyz/jsonpatch/v2@v2.0.0/.gitignore 1979-12-31 00:00:00.000000000 -0500
+++ direct/gomodules.xyz/jsonpatch/v2@v2.0.0/.gitignore 1969-12-31 19:00:00.000000000 -0500
@@ -1,27 +0,0 @@
-# Compiled Object files, Static and Dynamic libs (Shared Objects)
-*.o
-*.a
-*.so
-
-# Folders
-_obj
-_test
-
-# Architecture specific extensions/prefixes
-*.[568vq]
-[568vq].out
-
-*.cgo1.go
-*.cgo2.c
-_cgo_defun.c
-_cgo_gotypes.go
-_cgo_export.*
-
-_testmain.go
-
-*.exe
-*.test
-*.prof
-
-/.idea
-/vendor
diff -urN proxy/gomodules.xyz/jsonpatch/v2@v2.0.0/.travis.yml direct/gomodules.xyz/jsonpatch/v2@v2.0.0/.travis.yml
--- proxy/gomodules.xyz/jsonpatch/v2@v2.0.0/.travis.yml 1979-12-31 00:00:00.000000000 -0500
+++ direct/gomodules.xyz/jsonpatch/v2@v2.0.0/.travis.yml 1969-12-31 19:00:00.000000000 -0500
@@ -1,17 +0,0 @@
-language: go
-go:
- - 1.x
- - tip
-
-go_import_path: gomodules.xyz/jsonpatch
-
-cache:
- directories:
- - $HOME/.cache/go-build
- - $GOPATH/pkg/mod
-
-env:
- - GO111MODULE=on
-
-script:
- - go test -v
diff -urN proxy/gomodules.xyz/jsonpatch/v2@v2.0.0/CHANGELOG.md direct/gomodules.xyz/jsonpatch/v2@v2.0.0/CHANGELOG.md
--- proxy/gomodules.xyz/jsonpatch/v2@v2.0.0/CHANGELOG.md 1979-12-31 00:00:00.000000000 -0500
+++ direct/gomodules.xyz/jsonpatch/v2@v2.0.0/CHANGELOG.md 1969-12-31 19:00:00.000000000 -0500
@@ -1,37 +0,0 @@
-# Change Log
-
-## [v2.0.0](https://github.com/gomodules/jsonpatch/tree/v2.0.0) (2019-06-25)
-[Full Changelog](https://github.com/gomodules/jsonpatch/compare/1.0.0...v2.0.0)
-
-**Merged pull requests:**
-
-- Update go.mod and remove vendor folder [\#18](https://github.com/gomodules/jsonpatch/pull/18) ([tamalsaha](https://github.com/tamalsaha))
-- Change package path to gomodules.xyz/jsonpath [\#17](https://github.com/gomodules/jsonpatch/pull/17) ([tamalsaha](https://github.com/tamalsaha))
-- \[Emergency\] correct array index in backtrace [\#16](https://github.com/gomodules/jsonpatch/pull/16) ([kdada](https://github.com/kdada))
-- Added support for arrays at the root [\#15](https://github.com/gomodules/jsonpatch/pull/15) ([e-nikolov](https://github.com/e-nikolov))
-- Fix the example code in readme [\#14](https://github.com/gomodules/jsonpatch/pull/14) ([pytimer](https://github.com/pytimer))
-
-## [1.0.0](https://github.com/gomodules/jsonpatch/tree/1.0.0) (2019-01-08)
-**Fixed bugs:**
-
-- Correctly generate patch for nested object [\#8](https://github.com/gomodules/jsonpatch/issues/8)
-
-**Closed issues:**
-
-- Do releases and in SemVer [\#12](https://github.com/gomodules/jsonpatch/issues/12)
-- Generated patch incorrect for Array replacement [\#1](https://github.com/gomodules/jsonpatch/issues/1)
-
-**Merged pull requests:**
-
-- Add JsonPatchOperation as type alias for Operation [\#13](https://github.com/gomodules/jsonpatch/pull/13) ([tamalsaha](https://github.com/tamalsaha))
-- Migrate to go mod [\#10](https://github.com/gomodules/jsonpatch/pull/10) ([tamalsaha](https://github.com/tamalsaha))
-- Add test for nested object [\#9](https://github.com/gomodules/jsonpatch/pull/9) ([tamalsaha](https://github.com/tamalsaha))
-- Add test for edit distance computation [\#7](https://github.com/gomodules/jsonpatch/pull/7) ([tamalsaha](https://github.com/tamalsaha))
-- Append edit distance operations from end to start [\#6](https://github.com/gomodules/jsonpatch/pull/6) ([tamalsaha](https://github.com/tamalsaha))
-- Add travis file [\#4](https://github.com/gomodules/jsonpatch/pull/4) ([tamalsaha](https://github.com/tamalsaha))
-- Run go fmt [\#3](https://github.com/gomodules/jsonpatch/pull/3) ([tamalsaha](https://github.com/tamalsaha))
-- Fix array comparison [\#2](https://github.com/gomodules/jsonpatch/pull/2) ([tamalsaha](https://github.com/tamalsaha))
-
-
-
-\* *This Change Log was automatically generated by [github_changelog_generator](https://github.com/skywinder/Github-Changelog-Generator)*
\ No newline at end of file
diff -urN proxy/gomodules.xyz/jsonpatch/v2@v2.0.0/README.md direct/gomodules.xyz/jsonpatch/v2@v2.0.0/README.md
--- proxy/gomodules.xyz/jsonpatch/v2@v2.0.0/README.md 1979-12-31 00:00:00.000000000 -0500
+++ direct/gomodules.xyz/jsonpatch/v2@v2.0.0/README.md 1969-12-31 19:00:00.000000000 -0500
@@ -1,52 +0,0 @@
-# jsonpatch
-
-[](https://travis-ci.org/gomodules/jsonpatch)
-[](https://goreportcard.com/report/gomodules.xyz/jsonpatch)
-[](https://godoc.org/gomodules.xyz/jsonpatch)
-
-As per http://jsonpatch.com JSON Patch is specified in RFC 6902 from the IETF.
-
-JSON Patch allows you to generate JSON that describes changes you want to make to a document, so you don't have to send the whole doc. JSON Patch format is supported by HTTP PATCH method, allowing for standards based partial updates via REST APIs.
-
-```console
-go get gomodules.xyz/jsonpatch/v2
-```
-
-I tried some of the other "jsonpatch" go implementations, but none of them could diff two json documents and generate format like jsonpatch.com specifies. Here's an example of the patch format:
-
-```json
-[
- { "op": "replace", "path": "/baz", "value": "boo" },
- { "op": "add", "path": "/hello", "value": ["world"] },
- { "op": "remove", "path": "/foo"}
-]
-
-```
-The API is super simple
-
-## example
-
-```go
-package main
-
-import (
- "fmt"
- "gomodules.xyz/jsonpatch/v2"
-)
-
-var simpleA = `{"a":100, "b":200, "c":"hello"}`
-var simpleB = `{"a":100, "b":200, "c":"goodbye"}`
-
-func main() {
- patch, e := jsonpatch.CreatePatch([]byte(simpleA), []byte(simpleB))
- if e != nil {
- fmt.Printf("Error creating JSON patch:%v", e)
- return
- }
- for _, operation := range patch {
- fmt.Printf("%s\n", operation.Json())
- }
-}
-```
-
-This code needs more tests, as it's a highly recursive, type-fiddly monster. It's not a lot of code, but it has to deal with a lot of complexity.
Not sure about that. In module mode, |
|
@jayconrod what do you suggest we do here then? I applied this patch downstream to get it packaged, but I don't want to keep having to maintain that patch. |
|
How do we clean the
|
|
It looks like this was reported to the module author (gomodules/jsonpatch#21) two weeks ago, but there's been no response. 😞 There are a number of ways to workaround this. To keep Go and Bazel in sync, Instead of depending on A Bazel-only workaround would be to change the |
|
Looks like I inherited that problem because I'm depending on controller-runtime. So the fix for me was to add the following to the |
See discussion here: kubernetes/test-infra#13918
|
@tamalsaha FYI |
|
Hi, I had reapplied the tag when we were dealing with gomodules/jsonpatch#20 issue. The tag has not changed since then. It seems that Go is retroactively applying tag verification. Any thoughts how to fix this? |
|
@tamalsaha golang considers versions immutable. Once you published v2.0.0, patching this release requires incrementing the version to v2.0.1 (or later). Golang does not support replacing a published version. Can you address gomodules/jsonpatch#21 and release v2.0.1 at the current commit? Also consider resetting v2.0.0 to the initial commit if you can do so. Since proxy.golang.org considers the initial version of v2.0.0 the canonical one, this is what golang 1.13 will distribute to everyone. It is also what people using earlier versions of golang will receive if they |
|
So, what do I do? Just apply the v2.0.1 tag to old release and push to git repo? Also, I don't understand why GO is capturing tags and complaining when Go modules are not on by default. I thought this was the time we experiment and learn this. |
|
Also, should the tag be |
Yep, I think that will work nicely! And thanks for following up!
In 1.13 -- when modules are enabled -- golang will default to using a GOPROXY instead of downloading directly. This is important for things like CI since proxy.golang.org can handle a lot of concurrent requests without slowing down. It also makes sure a popular package doesn't DOS the tiny developer who publishes it. So people using modules tend to turn it on. For example in our repo our scripts to upgrade dependencies explicitly turn on modules and proxies: test-infra/hack/update-deps.sh Line 76 in 71dd306 test-infra/hack/update-deps.sh Lines 96 to 97 in 71dd306 |
|
@kalbasit, can you try refactoring this PR to upgrade the jsonpatch module to v2.0.1? |
Sure. |
kalbasit
force-pushed
the
test-infra_fix-go-sum
branch
from
b0ab95b
to
56d17f0
Compare
k8s-ci-robot
added
size/S
|
@fejta it's done. |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 6b5f383eb9ba1360736ea1e2b08543fae62b4e6c
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fejta, kalbasit 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
I'm seeing a hash mismatch for
gomodules.xyz/jsonpatch/v2on my end. Could be related to golang/go#29278, last change to that checksum was done in #12895. Looking at the WORKSPACE file at the time of the last change, the Go version was set to1.12.6🤔Environment: