Skip to content
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

proposal: cmd/go: deprecate direct fallback in GOPROXY #40580

Open
arnottcr opened this issue Aug 5, 2020 · 9 comments
Open

proposal: cmd/go: deprecate direct fallback in GOPROXY #40580

arnottcr opened this issue Aug 5, 2020 · 9 comments
Labels
Projects
Milestone

Comments

@arnottcr
Copy link

@arnottcr arnottcr commented Aug 5, 2020

background

As noted in #40358, the ,direct suffix to the default value of GOPROXY is confusing. In isolation it makes sense, but with the added requirement of a sumdb, it seems broken.

If you cannot access sum.golang.org, the toolchain fails:

[user@localhost ~]$ go get golang.org/x/exp
go: downloading golang.org/x/exp v0.0.0-20200513190911-00229845015e
go get golang.org/x/exp: golang.org/x/exp@v0.0.0-20200513190911-00229845015e: verifying module: golang.org/x/exp@v0.0.0-20200513190911-00229845015e: Get "https://sum.golang.org/lookup/golang.org/x/exp@v0.0.0-20200513190911-00229845015e": dial tcp [::1]:443: connect: connection refused

If you fallback to direct, this implies that proxy.golang.org cannot fetch your source, thus sum.golang.org cannot either. As such, even if you manage to fetch a module directly, the sumdb verification will fail:

[user@localhost ~]$ go get gitern.com/urandom/errors
go: downloading gitern.com/urandom/errors.git v0.0.0-20200723020514-49681dcbcc21
go get gitern.com/urandom/errors.git: gitern.com/urandom/errors.git@v0.0.0-20200723020514-49681dcbcc21: verifying module: gitern.com/urandom/errors.git@v0.0.0-20200723020514-49681dcbcc21: reading https://sum.golang.org/lookup/gitern.com/urandom/errors.git@v0.0.0-20200723020514-49681dcbcc21: 410 Gone
        server response:
        not found: gitern.com/urandom/errors.git@v0.0.0-20200723020514-49681dcbcc21: invalid version: git fetch -f origin refs/heads/*:refs/heads/* refs/tags/*:refs/tags/* in /tmp/gopath/pkg/mod/cache/vcs/c58e8953608705577f340d5d716eb03f031f4f09a3698dde27ce667a4baf661a: exit status 128:
                fatal: unable to connect to gitern.com:
                gitern.com[0: 100.20.209.4]: errno=Connection refused

The only use case I can conceive for this feature, is if you wanted to fetch from vcs, and still retain the benefits of sumdb verification, but let me know if there is something I am missing.

Based on this understanding, it seems like there are two type of packages: accessible and inaccessible (wrt the proxy/sumbd). The former case are things that can be served by proxy.golang.org, or your (private) module proxy of choice. The latter are things that must be fetched from vcs directly. Documentation suggests using GOPRIVATE for the latter, and we have shown that GOPROXY=direct will not work.

description

I propose we remove the ,direct suffix from the GOPROXY default value. We should keep the keyword around to prevent breaking changes, but modify its behaviour so that it is a nop and prints a warning to the user.

While this may seems like we are forcing people to use proxy.golang.org exclusively, this was already done with GOSUMDB=sum.golang.org.

alternatives

We could actually make direct work by having it skip the sumdb verification, however there were concerns about it "destroying the security model" in #40358.

While we could also leave it alone, but I hope that I have shown that ,direct cannot work the way one expects it to today, thus is just noise and confusion to most users.

@gopherbot gopherbot added this to the Proposal milestone Aug 5, 2020
@gopherbot gopherbot added the Proposal label Aug 5, 2020
@martisch
Copy link
Contributor

@martisch martisch commented Aug 5, 2020

Isnt there the possibility of companies running their own internal sum db with internal url set in GOSUMDB? This does not require that the external proxy has the same visibility to packages as the internal sum db server.

@arnottcr
Copy link
Author

@arnottcr arnottcr commented Aug 5, 2020

Running a sumbd without a proxy would be another use case, but it is pretty weak, since there are few sumdb only implementations, and most are both proxy and sumdb.

Furthermore, my main suggestion is to remove ,direct as a default, since once we start talking about custom setups, anything is possible. I will fixup the description to reflect this preference.

@martisch
Copy link
Contributor

@martisch martisch commented Aug 5, 2020

There is still the possibility of the go proxy being unavailble while the sumdb is not. And the proxy might not have the module or be able to fetch the module while the sumdb already knows about it. Its nice to have a usable fallback to improve overall reliability.

Letting clients fetch directly also allows the proxy to loadshed requests without immediatly impacting the clients ability to get modules. Otherwise the default behaviour of clients would force them to continue to reach out to the module proxy continuing the load.

@arnottcr
Copy link
Author

@arnottcr arnottcr commented Aug 5, 2020

If we are assuming that general network instability needs to be solved by the toolchain, then why do we hard fail when we cannot reach sum.golang.org?

Furthermore, it is inconcevable that, for the default values, one could reach sum.golang.org, but not proxy.golang.org, since they are the same service. This reflects all internal module deployments that i have seen too.

@martisch
Copy link
Contributor

@martisch martisch commented Aug 5, 2020

Furthermore, it is inconcevable that, for the default values, one could reach sum.golang.org, but not proxy.golang.org.

Im certain if given the access controls or due to the right set of circumstances to disable traffic to proxy.golang.org while sum.golang.org will be fine.

since they are the same service.

They may or may not be currently. They dont have to be in the future and they dont have to use the same load balancer config.

@heschik
Copy link
Contributor

@heschik heschik commented Aug 5, 2020

The go command does not need to consult sum.golang.org if there is a fully-populated go.sum file in the project. In general, only operations that change dependencies need to access sum.golang.org.

As I said in #40358, there may be room for a usability improvement in the error messages, but I don't think there's much chance at all of this proposal being accepted.

@arnottcr
Copy link
Author

@arnottcr arnottcr commented Aug 6, 2020

@ianlancetaylor: can you add this to the proposals project?

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Aug 6, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 6, 2020

Done. (You could also just e-mail me a list of issues.)

CC @bcmills @jayconrod

@arnottcr
Copy link
Author

@arnottcr arnottcr commented Aug 6, 2020

Sorry, that would have been easier, but thanks a bunch; if I get bored, I may look through for open tickets with the proposal label and milestone without the project, and I will email that list to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Incoming
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.