-
Notifications
You must be signed in to change notification settings - Fork 17.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
proxy.golang.org: accidentally publishing a major version interplays poorly with Go modules #34189
Comments
Thanks for reporting this. It appears to me that the only sensible solution to this issue is indeed the one you propose:
But as it stands, you're basically asking for #24031 (cmd/go: allow package authors to mark older package versions as insecure, incompatible, or broken), so this issue is essentially a dup of that one. |
(you may want to add this use case to the discussion there). |
@ALTree I think this issue is trying to get to the heart of a larger problem.
We are all humans, and mistakes like the above happen. It's bound to happen in the GitHub Octoverse, due to the lack of tag protection. The proxy you operate is like an unforgiving watchdog. There should be some kind of workflow to approve/retract caching of releases in the proxy. There's a reason why all package managers out there have a management user interface or explicit steps for release publishing (npm, bintray, maven, cargo). I know the go team likes to be minimalistic, but disregarding the human factor is a footgun. Something as simple as adding an "authors" directive in the go.mod file with a list of email addresses, and having the proxy send a confirmation email to approve new releases for proxy caching, would go a long way. But ideally, you'd offer explicit management over releases, remove the proxy default in go1.13.1 (this was a questionable idea to begin with), and/or make proxy.golang.org caching opt-in. |
I have also imagined this scenario. I'm thinking, can we provide a "withdrawal" option? As long as someone can prove that he/she is the owner of a module version, then he/she can withdraw it by sending a request email to the [email protected]. Or, submit an issue with a prefix like "proxy.golang.org: withdrawal: ". /cc @katiehockman here. |
I hoped the go command wouldn't pick the incompatible versions when there are matching, valid pseudo versions and the module major version in go.mod does not match. But that's not the case and indeed the go picks up the incompatible version. go list --versions -json -m github.com/libp2p/go-libp2p go: finding github.com/libp2p/go-libp2p v6.0.23+incompatible { "Path": "github.com/libp2p/go-libp2p", "Version": "v6.0.23+incompatible", "Versions": [ "v0.0.1", "v0.0.2", "v0.0.3", "v0.0.4", "v0.0.5", "v0.0.6", "v0.0.7", "v0.0.8", "v0.0.9", "v0.0.10", "v0.0.11", "v0.0.12", "v0.0.13", "v0.0.14", "v0.0.15", "v0.0.16", "v0.0.17", "v0.0.18", "v0.0.19", "v0.0.20", "v0.0.21", "v0.0.22", "v0.0.23", "v0.0.24", "v0.0.25", "v0.0.26", "v0.0.27", "v0.0.28", "v0.0.29", "v0.0.30", "v0.1.0", "v0.1.1", "v0.1.2", "v0.2.0", "v0.2.1", "v0.3.0", "v0.3.1", "v1.0.0", "v2.0.1+incompatible", "v2.0.2+incompatible", "v2.0.3+incompatible", "v3.0.0+incompatible", "v3.1.0+incompatible", "v3.2.0+incompatible", "v3.2.1+incompatible", "v3.2.2+incompatible", "v3.2.3+incompatible", "v3.3.0+incompatible", "v3.3.1+incompatible", "v3.3.2+incompatible", "v3.3.3+incompatible", "v3.3.4+incompatible", "v3.3.6+incompatible", "v3.3.7+incompatible", "v3.4.0+incompatible", "v3.4.1+incompatible", "v3.4.2+incompatible", "v3.4.3+incompatible", "v3.5.0+incompatible", "v3.5.1+incompatible", "v3.5.2+incompatible", "v3.5.3+incompatible", "v3.5.4+incompatible", "v3.6.0+incompatible", "v4.0.0+incompatible", "v4.0.1+incompatible", "v4.0.2+incompatible", "v4.0.3+incompatible", "v4.0.4+incompatible", "v4.1.0+incompatible", "v4.2.0+incompatible", "v4.3.0+incompatible", "v4.3.1+incompatible", "v4.3.2+incompatible", "v4.3.3+incompatible", "v4.3.4+incompatible", "v4.3.5+incompatible", "v4.3.6+incompatible", "v4.3.7+incompatible", "v4.3.8+incompatible", "v4.3.9+incompatible", "v4.3.10+incompatible", "v4.3.11+incompatible", "v4.3.12+incompatible", "v4.4.0+incompatible", "v4.4.1+incompatible", "v4.4.2+incompatible", "v4.4.3+incompatible", "v4.4.4+incompatible", "v4.4.5+incompatible", "v4.5.0+incompatible", "v4.5.1+incompatible", "v4.5.2+incompatible", "v4.5.3+incompatible", "v4.5.4+incompatible", "v4.5.5+incompatible", "v5.0.0+incompatible", "v5.0.1+incompatible", "v5.0.2+incompatible", "v5.0.3+incompatible", "v5.0.4+incompatible", "v5.0.5+incompatible", "v5.0.6+incompatible", "v5.0.7+incompatible", "v5.0.8+incompatible", "v5.0.9+incompatible", "v5.0.10+incompatible", "v5.0.11+incompatible", "v5.0.12+incompatible", "v5.0.13+incompatible", "v5.0.14+incompatible", "v5.0.15+incompatible", "v5.0.16+incompatible", "v5.0.17+incompatible", "v5.0.18+incompatible", "v5.0.19+incompatible", "v5.0.20+incompatible", "v5.0.21+incompatible", "v6.0.0+incompatible", "v6.0.1+incompatible", "v6.0.2+incompatible", "v6.0.3+incompatible", "v6.0.4+incompatible", "v6.0.5+incompatible", "v6.0.6+incompatible", "v6.0.7+incompatible", "v6.0.8+incompatible", "v6.0.9+incompatible", "v6.0.10+incompatible", "v6.0.11+incompatible", "v6.0.12+incompatible", "v6.0.13+incompatible", "v6.0.14+incompatible", "v6.0.15+incompatible", "v6.0.16+incompatible", "v6.0.17+incompatible", "v6.0.18+incompatible", "v6.0.19+incompatible", "v6.0.20+incompatible", "v6.0.21+incompatible", "v6.0.22+incompatible", "v6.0.23+incompatible" ], "Time": "2018-10-24T21:56:21Z" } The module mirror and the go command may offer withdrawal or blacklisting options in the context of #24031 as @ALTree said, but removing and reclaiming the published version tags from the global check sum database will not be possible or desirable by design. |
@heschik noticed |
To spell out a factual point:
This is incorrect: So, if you want, you can fix/work around this right now (regardless of any decision about removing content from proxy.golang.org) by publishing a later tag, Note that there are other proxies in the world. It appears that gocenter.io has already picked up many of these versions, for example. |
I think I follow the general concern expressed in this issue, but just on that one point I quoted here -- to my knowledge, accidentally publishing a version where the sole mistake is incorrectly applying VCS tags with a higher-than-intended major version would not require downstream consumers to modify import paths, at least not to my knowledge. Using the example from this issue, In general, a module can import a v2+ package that has not opted into modules, and if the imported v2+ package has a valid semver tag, it will be recorded with an +incompatible suffix, and the consuming module does not use |
@heschik
All downstream users would be forced to modify their import paths by suffixing v6 or v7 -- all due to a human error and the inexistence of checks or confirmation before perpetuating something in history. The DX here is terrible.
The nuance here is that these tags were created before Go modules existed, and before those major versions had any implications on import paths. So when we introduced go.mod, we archived those "ignorant" tags under the |
👍
The scenario I'm illustrating is one where the project does use |
To my knowledge, I think cmd/go will not allow you to get a version that is mismatched in the way that you described (at least as of Go 1.13) with a go.mod that reads |
I don't think that is true. If you publish with a later tag after deleting If you decide to keep |
Here is a concrete example of the Go 1.12 version of cmd/go rejecting a mismatch in that scenario, where the lz4
|
If the last statement were true, we wouldn't have much of an issue here. Unfortunately, that's not the case. Those versions are accessible without version suffixing. A user importing go-libp2p (without version suffixes) in a fresh project will get |
@raulk sorry that's my typo. I meant "Users can access that without v6 or v7 suffixed import paths". Actually, users shouldn't use those suffixes to pick up the intended version. |
As I understand, the issue is that I think #24031 is probably the long-term solution for this. If that were implemented, you'd be able to tell proxies to stop advertising "bad" versions via the In this situation, you could mark all of the accidentally published versions as "bad", then you could continue publishing Would this solve the problem for you? |
If there are a large number of intervening commits, you would also need to invalidate all pseudo-versions generated for those commits using the erroneous tag as the pseudo-version base. (But presumably you could identify at least the majority of those using |
Note that (at least with Go 1.13) this problem can only occur for versions of the module that lack an explicit In retrospect, I think we should not have allowed However, since we know that major-version bumps represent breaking changes, perhaps we can change the behavior of That wouldn't help much if users already explicitly |
Another option might be to declare For example: we know that a ...come to think of it, I like that idea a lot. I think I'll file a proposal! |
If that is done, it would probably also make sense to not upgrade from |
Agreed — that was my intent. |
Filed my proposal as #34217. I believe it would address this issue as well, by allowing users to ignore the accidental (An accidental |
We do not want to contribute to informing Google of every single user that uses go-libp2p, thanks. Also, the default proxy (proxy.golang.org) contains old and deprecated `+incompatible` versions that the Go toolchain selects over the more recent go-modded versions. See golang/go#34189 and golang/go#34217.
We do not want to contribute to informing Google of every single user that uses go-libp2p, thanks. Also, the default proxy (proxy.golang.org) contains old and deprecated `+incompatible` versions that the Go toolchain selects over the more recent go-modded versions. See golang/go#34189 and golang/go#34217.
Change https://golang.org/cl/204440 mentions this issue: |
Change https://golang.org/cl/204439 mentions this issue: |
codeRepo.Versions previously checked every possible +incompatible version for a 'go.mod' file. That is wasteful and counterproductive. It is wasteful because typically, a project will adopt modules at some major version, after which they will (be required to) use semantic import paths for future major versions. It is counterproductive because it causes an accidental '+incompatible' tag to exist, and no compatible tag can have higher semantic precedence. This change prunes out some of the +incompatible versions in codeRepo.Versions, eliminating the “wasteful” part but not all of the “counterproductive” part: the extraneous versions can still be fetched explicitly, and proxies may include them in the @v/list endpoint. Updates #34165 Updates #34189 Updates #34533 Change-Id: Ifc52c725aa396f7fde2afc727d0d5950acd06946 Reviewed-on: https://go-review.googlesource.com/c/go/+/204439 Run-TryBot: Bryan C. Mills <[email protected]> Reviewed-by: Jay Conrod <[email protected]>
…tible one has a go.mod file Previously we would always “upgrade” to the semantically-highest version, even if a newer compatible version exists. That made certain classes of mistakes irreversible: in general we expect users to address bad releases by releasing a new (higher) version, but if the bad release was an unintended +incompatible version, then no release that includes a go.mod file can ever have a higher version, and the bad release will be treated as “latest” forever. Instead, when considering a +incompatible version we now consult the latest compatible (v0 or v1) release first. If the compatible release contains a go.mod file, we ignore the +incompatible releases unless they are expicitly requested (by version, commit ID, or branch name). Fixes #34165 Updates #34189 Change-Id: I7301eb963bbb91b21d3b96a577644221ed988ab7 Reviewed-on: https://go-review.googlesource.com/c/go/+/204440 Run-TryBot: Bryan C. Mills <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Jay Conrod <[email protected]>
#33558 (comment) is an interesting example here. There was a branch named Probably we should not treat branches as semantic versions. One may (very reasonably) expect to add further commits on the branch, and those commits would make the meaning of |
Filed #35671 for treating the branch as a release in the first place. |
@bcmills here is an example for v1.0.0 https://pkg.go.dev/mod/go.opentelemetry.io/otel/exporter/trace/[email protected]?tab=versions As a summary, no workaround other than bumping to v1.0.0+, right? |
I'm wondering whether it might be a good idea, instead of allowing versions to be mutated, to instead have package owners be able to publish a badlist of release tags that should not be used for purposes of ... oh that's #24031, carry on ;) |
Is there something to do here for 1.14? |
The parts that were feasible for 1.14 are done. |
While the fixes for 1.14 solve the immediate problem for libp2p that I described in #34189 (comment), the problem that an accidental tagging of a major version cannot be undone (as it happened to OpenTelemetry) is still unresolved. |
@ferhatelmas, @marten-seemann: that is correct. The OpenTelemetry problem has no workaround in Go 1.14 or earlier. #24031 may provide a solution for similar cases in the longer term. |
The |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes. It only occurs with Go 1.13.
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
When publishing a new tag on Github, the proxy caches all available module versions and their respective checksums, as soon as it receives a request for the module. When
go get
ting the module,go get
will download the latest version available (which is determined according to semantic versioning).If you accidentally publish a patch tag or a minor tag, this is easy to fix. Just publishing another patch release ensures that all downstream users will receive a working version of the module.
If you accidentally publish a major tag though, this is a practically unrecoverable situation: Since the introduction of Go modules, major versions include the version number in the import path. Now every single downstream user has to adjust the import path to receive a working version of the module.
This is what happened to us with go-libp2p. Before the introduction of Go modules, we used a hash-based package manager called gx for the libp2p project. Releases were identified by their hash value, but we nevertheless assigned version numbers, in order to increase human readability. When we switched to Go modules, we deleted all old tags, and started anew from
v0.0.1
. A few weeks ago, someone apparently accidentally pushed the old gx-tags back to our Github repo. We detected this within a few hours and removed the tags, but it was already too late: the Go checksum database already archived them for all eternity.Running
go get github.com/libp2p/go-libp2p
now retrievesv6.0.23+incompatible
instead of the actual current versionv0.3.1
.What did you expect to see?
It seems like we're encountering an unfortunate interplay of the way Go handles major version updates (by including them in the import path) with the now irreversible one-step process (pushing a tag) of creating a new release.
Replacing the code associated with a given version is exactly what the Go checksum database is supposed to prevent. However, programmers are humans, and humans make mistakes, sometimes stupid ones like pushing an erroneous major version tag. Relying on a one-step process to create new releases and push them to a fleet of proxies, with no way of managing or reverting them seems like a brittle process.
In practice, many developers use GitHub. Unfortunately, GitHub doesn't support rejecting tag pushes, so it's not even possible to implement an automated sanity check for tag pushes.
I suggest that there needs to be some way to recover from such a mistake without forcing all downstream users to adjust their import paths. Maybe it would be possible to mark a certain version as known broken. The entry would still remain in the checksum database, but
go get
would ignore this version when determining which version islatest
.The text was updated successfully, but these errors were encountered: