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

cmd/go: permit marking a module as private in go.mod #33985

Open
donatj opened this issue Aug 30, 2019 · 17 comments

Comments

@donatj
Copy link

commented Aug 30, 2019

The new proxy system requires individual machines environmental variables to be set up for private modules. This means knowledge about the build that is not kept in the repo. This is not ideal.

I believe a good feature would be the ability to mark a module as private in the go.mod rather than depending on each machine setting it's environmental variables correctly.

Originally posted by @donatj in #33980 (comment)

@tommyknows

This comment has been minimized.

Copy link

commented Aug 30, 2019

Definitely agree. Telling everybody at your company to "set the GOPRIVATE=..." seems impractical, and having this information lying in the go.mod file, and thus checked in, would be a more straightforward solution.

That would make it possible to pull a repo from a git server and built it, without configuring you environment.

Although I wonder how to solve the syntax-problem.
The modules-syntax seems to be fairly simple and I cannot think of an intuitive way to mark modules as private.
For example:

module myprivate.git/module

require (
    myprivate.git/dependency // private
    myprivate.git/another // private indirect
)

Feels clunky (and embedding machine-information in comments is not a nice solution imho)
Also, could you set the main module to private, too (is there a need for it)? If it is coming from a private repo, should it be noted as such?
e.g. module myprivate.git/module // private?

@henderjon

This comment has been minimized.

Copy link

commented Aug 30, 2019

It feels like we got rid of GOPATH only to replace it by a convoluted series of new GOPATHs (i.e. GOPRIVATE, GOPROXY, GONOPROXY, GOSUMDB).

@ianlancetaylor ianlancetaylor changed the title Mark a module as private in go.mod cmd/go: permit marking a module as private in go.mod Aug 30, 2019

@ianlancetaylor ianlancetaylor added this to the Go1.14 milestone Aug 30, 2019

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2019

@bcmills

This comment has been minimized.

Copy link
Member

commented Aug 30, 2019

(I could've sworn I'd seen this issue before, but I can't find a duplicate right now...)

CC @hyangah @marwan-at-work @thepudds @rogpeppe

@bcmills

This comment has been minimized.

Copy link
Member

commented Aug 30, 2019

The major problem with indicating private repos in the go.mod file is that “private” is relative to the proxy.

For example: if your company is running its own (authenticated) corp module proxy, then it is entirely reasonable for the go command to be able to fetch your private-to-corp modules from that proxy when it is in use. If you switch to some other proxy — regardless of whether it is proxy.golang.org or something else — then those modules become “private” (because the proxy doesn't have access to them).

So “private” is fundamentally relative to the proxy, not relative to the module dependency.

@marwan-at-work

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2019

I remember the issue very well, but I can't seem to find it either :/

However, I do sympathize with the issue here, because you can't force all of your team members to set up the correct configuration and environment variables on their machines.

And since "private" is relative to the proxy, maybe it's worth considering adding the GOPROXY variable to the go.mod file itself (as well as the GONOSUMDB and friends)

Or maybe, just be able to set any of the go env options in the go.mod file itself. However, the non zero value of an environment variable could take precdence.

This way, you don't need to set a "private directive" in the go.mod file, but still resolve the issue mentioned above.

That said, one can simply have a .env file in their repo and indicate in the README that those environment variables must be set before running a go command, so I'm also happy to not add that kind of complexity to go.mod.

@thepudds

This comment has been minimized.

Copy link

commented Aug 30, 2019

(I could've sworn I'd seen this issue before, but I can't find a duplicate right now...)

I think it was discussed as part of the larger #25530 discussion, such as in #25530 (comment) and some related comments there, as well as some of that discussion I think then triggered some additions to the proposal document, such as:

One possibility to further reduce exposure of private module path text is to provide additional ways to set $GONOSUMDB, although it is not clear what those should be. A top-level module's source code repository is an attractive place to want to store configuration such as $GONOSUMDB and $GOPROXY, but then that configuration changes depending on which version of the repo is checked out, which would cause interesting behavior when testing old versions, whether by hand or using tools like git bisect.

I don't recall a specific issue aside from #25530.

@tommyknows

This comment has been minimized.

Copy link

commented Aug 30, 2019

So “private” is fundamentally relative to the proxy, not relative to the module dependency.

That is true.
However, from your statement, it seems like running your own proxy will be "expected"?

For example, at a company with ~50 people, we have our own (private) Gitlab instance running. However, we are only ~5 devs actually using Go.
Setting up and maintaining a proxy for 5 people seems like overkill.
(Obviously, coordinating 5 people to set the same environment variable is doable, but is it nice?)

Normally, public projects will not depend on private repos.
This means that private projects will most likely live in the same environment as their dependencies.

That said, setting GOPROXY in the modules file would be needed for this to work.
Having a private proxy indicated in the go.mod file would mean no env-config would be needed, and "private" repos would not need to be declared.
Having no proxy (or a public one) set in the go.mod file would mean one could use "private" within the go.mod file to indicate that go should talk directly to the specified git server for these repos.

@heschik

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2019

@tommyknows: I think it's expected that some organizations will want to, but certainly not everybody. However, keep in mind that (as touched on in #25530 (comment)) the go.mod file is immutable in your module's version control. If your company grows to the point where it does make sense for you to run your own proxy, you probably won't want it to be bypassed if someone happens to depend on an old module version from before the proxy was set up.

@bcmills

This comment has been minimized.

Copy link
Member

commented Aug 30, 2019

@tommyknows, if you configure a proxy in your go.mod file, and I write some other module that depends on yours, and we have annotated private modules separately, what should happen?

Moreover: suppose that I have five developers but fifty repos: now adding the environment variable is actually less work overall than annotating all of the go.mod files individually.

At the other extreme, suppose that I have fifty developers and only one repo: now I don't need to annotate dependencies at all, because there's only one module containing private things and it doesn't depend on any other private dependencies.

The cost/benefit tradeoff you're describing is only favorable to the go.mod annotation in between those two extremes. Do we have solid evidence that that's where most developers fall? (Absent evidence, we should arguably bias toward not adding new annotations.)

@tommyknows

This comment has been minimized.

Copy link

commented Aug 30, 2019

You are right, definitely agree with that point.

What I don't like is that one needs to set an environment variable to not expose "private" information.

imho, the "default" value of GOPROXY (variable is not set / empty) should be to not use a proxy at all. Opt-In instead of Opt-out.

@thepudds

This comment has been minimized.

Copy link

commented Aug 30, 2019

A goal could be to use information in go.mod (or another file checked into VCS) to mitigate the impact of "whoops, I didn't configure any env vars myself", or perhaps even "whoops, I misconfigured my env vars". While I'm not sure it would be possible to eliminate the impact of those mistakes in all cases, it might be worth thinking through what mistakes could be caught in what workflows.

The major problem with indicating private repos in the go.mod file is that “private” is relative to the proxy.

I think the real goal would be to avoid accidentally interacting with public proxies and the public sum.golang.org. The most important public proxy to help people accidentally hit after a mistake would be the public proxy that is used in the absence of any configuration -- proxy.golang.org. In other words, one class of mistake is forgetting to make any configuration, and in that case, the public infrastructure would be in use (and not any private proxy, given using a private proxy requires some configuration). So maybe that could simplify the problem some?

And since "private" is relative to the proxy, maybe it's worth considering adding the GOPROXY variable to the go.mod file itself (as well as the GONOSUMDB and friends)

Adding the GOPROXY variable (or equivalent content) to go.mod might be problematic, but also maybe not needed if the focus is on avoiding hitting public infrastructure in the face of a mistake.

You could imagine a go.mod containing information along the lines of:

no-public-infra-for-modules-matching-patterns=*.secret.git.com,*.something.else.com

(with a deliberately bad name chosen to emphasize I am not suggesting an actual syntax or actual name).

That could help some mistakes, for example if someone does a git clone of a repo that has a go.mod with that info.

However, it's not immediately obvious how go.mod-based info would help if someone does something like the following (without having first properly configured privacy env vars):

$ cd $(mktemp -d)

$ go mod init newmodule

$ cat > main.go
package main
import _ "secret.git.com/double/secret/import/path"
func main() {}

$ go build

You could make a non-default / user-specified privacy pattern a requirement for every go.mod before cmd/go will do anything else, but (a) that might not be popular, and (b) that still doesn't mean people would set it properly.

In summary, I am not sure a bullet proof go.mod-based solution is possible... but maybe the problem could be recast to focus on eliminating certain classes of mistakes (while not preventing other classes of mistakes)? Not sure.

@thepudds

This comment has been minimized.

Copy link

commented Aug 30, 2019

Continuing on the "things that might not be popular" brainstorming tack:

A heavier-handed approach could be refuse to access the network until the user has taken some specific step regarding privacy settings (either in the current environmental settings, or based on go.mod).

For example, if you are starting a new module (and hence no go.mod-based privacy settings) and have no privacy settings in your user's environment, it could be an error to try to access the network, with some message along the lines of:

The go command cannot access the network until you have set a privacy pattern, 
or indicated you are opting out of privacy patterns. For example, you could:

 1. Set a privacy pattern. See 'go help privacy-patterns'.
 
 2. Opt out of privacy patterns for this module by running
    'go mod edit -no-privacy-patterns-for-me'.
 
 3. Opt out of privacy patterns for this user by running
    'go env -w no-privacy-patterns-for-me=true'.

See 'go help some-topic' for details and additional options.

Or maybe it does not key off of privacy patterns... but the general comment is that it could force the user to take some action to eliminate the "whoops, I forgot to configure anything" type of error.

@vearutop

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2019

I like the idea of disabling default proxy value by a flag in go.mod, so that proxy can only be used if explicitly set with env var.

Or as a more flexible/complicated solution we can have tags on dependencies and only resolve such with appropriate proxies.

module myprivate.git/module

require (
    myprivate.git/dependency // acme
    myprivate.git/another // acme
)

GOPROXY=https://proxy.golang.org,acme+https://athens.acme.com,direct

@lopezator

This comment has been minimized.

Copy link

commented Aug 31, 2019

imho, the "default" value of GOPROXY (variable is not set / empty) should be to not use a proxy at all. Opt-In instead of Opt-out.

Coldn't agree more with this point.

IMHO the defaults shouldn't be "expose info to a 3rd company/public proxy" event if that info isn't too much but import paths.

In my opinion someone that knows what is doing would easily set an ENV in order to get the benefits of the public proxy. On the contrary, someone that just clones the project and doesn't know about the proxies or is just using a new computer and forgot to set the envs would automatically leak info.

Not sure about the solution, I agree with @thepudds & @bcmills in the point that the go.mod solution might not suite all needs, and feels complicated.

A couple of ideas I can think of:

A. GOPROXY=direct by default. opt-in instead of opt-out as @tommyknows said.
B. Ask for confirmation preparsing the go.mod file when detecting non-public/known urls on it like gitlab.mydomain.com, mydomain.com etc and you don't have the GONOPROXY and friends set.

@zaddok

This comment has been minimized.

Copy link

commented Sep 3, 2019

Am I correct to assume this won’t be fixed until 1.14? I’m not sure I like the idea of having to skip 1.13 to avoid simple user error leaking information.

(Also, Can someone please remember to update the app engine docs for cloud build and regular app engine deploys to avoid leaking private repos.)

@bcmills

This comment has been minimized.

Copy link
Member

commented Sep 3, 2019

@zaddok, Go 1.13 has been released, and this issue was filed well after the freeze window. The proposed changes will not happen in 1.13, but don't assume they will necessarily happen in 1.14 either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.