Open bcmills opened 4 years ago
Sorry, but I don't understand what the actual feature request here is. My best guess is:
module X
has an @latest
entry.
module X/Y
has an @latest
entry.
attempted fetches of module X/Y
begin failing -- possibly because the origin is flaky, possibly because it's been deleted, or possibly because something is internally wrong with proxy.golang.org.
module X
continues to work.
In this situation, you're saying that we should assume that X/Y
is failing because it's been deleted, check that a version of X
later than the version of X/Y
has been fetched, and forget the @latest
for X/Y
?
If that's wrong please be very specific about the proposal. I can't imagine a version of this that I think is a good idea so I'm struggling to understand the suggestion.
fyi - https://github.com/golang/go/issues/32239 is related. We tried this as another way to address this type of issue, but we had to give it up because of noticeable performance degradation.
In this situation, you're saying that we should assume that
X/Y
is failing because it's been deleted, check that a version ofX
later than the version ofX/Y
has been fetched, and forget the@latest
forX/Y
?
Behaviorally, yes.
But I would phrase the causality of it a bit differently:
X/Y
@latest
is fetched and resolves to (pseudo-)version v₁, with metadata indicating commit timestamp τ₁.X/Y
@yyyyyyyyyyyy
is fetched and resolves to version v₃, with metadata indicating commit timestamp τ₃ (where τ₄ > τ₃ > τ₂ > τ₁).Since τ₃ > τ₁ and τ₄ > τ₂, we can infer that the state of the repo probably changed at some point between τ₂ and τ₃, and thus the @latest
cache entry from that point in time is suspect. So we should remove the suspected-stale cache entry, and either not replace it or re-resolve and replace it “soonish”.
The next time X/Y
@latest
is requested (at τ₅), we should re-resolve it.
We can't tell whether a module version is on the default branch, so we would have to invalidate @latest
on any commit to any branch that we became aware of. If we then experienced a transient failure refreshing @latest
, that would break users.
I think this is a very specific workaround to a very specific problem that has better solutions and causes some amount of collateral damage. It certainly increases complexity. I would rather not do it.
I agree that this seems like a lot of complexity for a pretty niche situation. I definitely empathize with @bcmills' point that we don't want users to have to a special workaround for this, but the tradeoff of this could harm other users (who then may also need a workaround, etc etc). I'm also not confident that I know every single side effect of this change.
If this becomes a bigger problem that more people experience, then let's re-visit this. For now, a reasonable solution is to use https://github.com/gomodule/redigo/pull/440#issuecomment-621933191 as a guideline if anyone else enounters this. Maybe documenting it if that issue isn't good enough.
This also relates back to https://github.com/golang/go/issues/30134, where we could do more here if we had more information from the error message (like @heschik alluded to). It's difficult to make the right decision about whether or not to drop one of our cache entries if there was a go
command error, since it could be from a legitimate error that we can save, or it could be from a transient server error that we should throw away.
Could a user exploit this to break a repo simply by putting in a PR that is then cached in this way?
@stevenh, no: the go
command refuses to use commits that are not reachable from at least one tag or branch, and unmerged PRs are not reachable from any tag or branch.
We can't tell whether a module version is on the default branch, so we would have to invalidate
@latest
on any commit to any branch that we became aware of.
Yes, that's true.
If we then experienced a transient failure refreshing
@latest
, that would break users.
It would not break any existing users, since they are all requesting explicit versions. It could indeed break new users of the module, but a module author can already break new users of the module in all sorts of ways (such as by pushing a latest
version that explicitly removes all of the source code from within the module). I think it clearly is the proxy's responsibility to provide continuity for existing users, but ensuring that new users can always find a viable latest
to use seems outside its charter.
I agree that this seems like a lot of complexity for a pretty niche situation.
A similar approach that would reduce overall complexity would be to apply the same TTL for latest
that we apply for branch-to-version resolution in general. (After all, latest
is just the branch-to-version resolution for “the default branch”.)
That would have fixed github.com/gomodule/redigo/redis
after an hour, instead of requiring explicit tagging.
To me the source of complexity here is the desire to treat @latest
as somehow different from other branches — and I don't agree that it is different.
The reason that proxy is treating @latest
differently is because that's part of the module mirror's promise (no more left-pad
) and affects end users. There are many tools that do not use the versioned tags. For example, the vscode go extension depends on many tools and many of them still go by pseudo-versions.
It's not even that. As far as I recall this is exactly the same behavior everything else gets. If a module goes from existing to failing to download, we'll continue to serve it for as long as we can. That goes for branches, tags, pseudoversions, everything.
If a module goes from existing to failing to download, we'll continue to serve it for as long as we can. That goes for branches, tags, pseudoversions, everything.
That seems like a major problem. What happens if the branch is, say, security-backports
, and the module author decides to rename the branch to lts-security-backports
? Users who set up a branch-oriented workflow will end up stuck on the last update, where they should have gotten an explicit error.
(CC @FiloSottile)
The reason that proxy is treating
@latest
differently is because that's part of the module mirror's promise (no more left-pad) and affects end users.
The “mirror's promise” is that existing users will be able to continue to use the dependency versions that they were already using. It does not promise that the latest
version of a module will always be a usable version of that module.
As noted in https://github.com/golang/go/issues/39007#issuecomment-628351207, it is already possible for a module author to break latest
for new users if they so choose, and in the case of left-pad
the decision to break users seems to have been intentional.
There are many tools that do not use the versioned tags. For example, the vscode go extension depends on many tools and many of them still go by pseudo-versions.
Expiring the cache entry for latest
would not prevent any of those existing users from continuing to use the versions they already depend on.
One of the primary goals of the proxy is to protect users from flaky upstreams, in particular outages. Since the go command gives us no way to distinguish outages from deliberate actions, we are required to be conservative and not try to infer intent.
Adding new dependencies is not a trivial use case and it is not acceptable for the ecosystem to break the next time some origin server goes down for a day. I don't think that's negotiable but if you would like to argue that case, this is probably not the place to do it.
If the fix for #24031 isn't sufficient to cover the security use case that sounds like a critical problem with it. The vast majority of people are presumably not going to be tracking a branch.
I don't think this discussion is going anywhere and I think this issue should be closed.
Here's another poor interaction from caching @latest
for modules that no longer exist, from the #modules
channel in Gopher Slack (https://gophers.slack.com/archives/C9BMAAFFB/p1632295080119400):
Having some problems with go install - it takes an old version of package for some reason (see v1.4.0 in output)
$ go install github.com/tetafro/godot/cmd/godot@latest go: downloading github.com/tetafro/godot/cmd/godot v0.0.0-20210115153532-96ff7da53d7b go: downloading github.com/tetafro/godot v1.4.11 go: downloading github.com/tetafro/godot v1.4.0
It works correctly when I run it with
@v1.4.11
. Is there a way to use@latest
with actually latest version? Maybe placego.mod
incmd/
directory or some other hack? What’s the common way to deal with it?
In the current release of the parent module (v1.4.11
), the package github.com/tetafro/godot/cmd/godot
is in the module github.com/tetafro/godot
, as it arguably should be. However, until https://github.com/tetafro/godot/commit/e5d5541c86b65d6d9fcbf2a918fe55f00405901d it was in its own (nested) module, so that module is still served as latest
when someone looks up github.com/tetafro/godot/cmd/godot@latest
in the proxy.
It is possible for the module author to fix the problem by restoring the go.mod
file, with a retract
directive, in an off-master commit and tagging that commit with something like cmd/godot/v0.0.0-retracted
, but that workaround is hard to discover, delicate to apply, and fairly counterintuitive.
In gomodule/redigo#440, a nested
github.com/gomodule/redigo/redis
module was created and subsequently removed from thegithub.com/gomodule/redigo
repository: today, the nested module does not exist at the head revision of that repo, and also does not exist in the latest tagged release of the parent module (github.com/gomodule/redigo
).In https://github.com/gomodule/redigo/pull/440#issuecomment-621892705 and https://github.com/gomodule/redigo/pull/440#issuecomment-621930004, I noted that
proxy.golang.org
was still serving an old commit for/@latest
for the nested module over an hour later, even thoughGOPROXY=direct
correctly reported that there was nolatest
version of that module, and even thoughproxy.golang.org
did know about a higher (and later) pseudo-version of that module.As a workaround, the owners of the repo had to tag an explicit release of the nested module, even though they did not intend for anyone to actually use that release.
I don't think that workaround should have been necessary: I think
proxy.golang.org
should not serve known-stale contents for the/@latest
endpoint. That endpoint should be invalidated whenever a newer version of the named module is successfully fetched, even if that version is not itselflatest
(as interpreted by thego
command).That may mean that
/@latest
transitions from advertising a version of the module to no longer advertising any such version, but that behavior is no worse than what would happen if the repo owner took some other drastic action to remove the module — such as publishing a commit on the default branch with all of the source code deleted, or publishing a commit with themodule
line declaring some other path. Regardless of the reason,proxy.golang.org
should view the affirmative non-existence of a module as a valid “latest” state for that module, and not get stuck advertising “the last version that did exist”.This only really matters for modules that never explicitly tagged any release or pre-release version, since the
go
command does not consult the/@latest
endpoint unless the list of release and pre-release versions is empty.(Note that, even if the
/@latest
endpoint itself does not advertise any version, the proxy should continue to serve all previously-fetched versions of any module with a suitable license. I am not proposing that we evict any explicit versions from the cache — only the/@latest
endpoint itself.)CC @jayconrod @matloob @katiehockman @heschik @hyangah @stevenh