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: deprecate -insecure on go get #37519

Open
witchard opened this issue Feb 27, 2020 · 25 comments
Open

cmd/go: deprecate -insecure on go get #37519

witchard opened this issue Feb 27, 2020 · 25 comments

Comments

@witchard
Copy link
Contributor

@witchard witchard commented Feb 27, 2020

Following the addition of the GOINSECURE environment variable under #32966, it is now possible to select specific hosts to access insecurely when fetching dependencies. This is much neater than the -insecure flag currently supported by go get which will download insecurely for any host.

Given we now have GOINSECURE as of go 1.14, I propose the removal -insecure from go get.

#34568 is also potentially relevant, presumably the issue with GIT_SSL_NO_VERIFY still exists when using GOINSECURE.

@witchard witchard changed the title cmd/go: Deprecate -insecure on go get cmd/go: deprecate -insecure on go get Feb 27, 2020
@bcmills bcmills changed the title cmd/go: deprecate -insecure on go get proposal: cmd/go: deprecate -insecure on go get Feb 28, 2020
@gopherbot gopherbot added this to the Proposal milestone Feb 28, 2020
@gopherbot gopherbot added the Proposal label Feb 28, 2020
@bcmills
Copy link
Member

@bcmills bcmills commented Feb 28, 2020

I think we probably need to keep -insecure around long enough for folks to start using GOINSECURE (and report and work out any deficiencies with it), but I'd be happy to remove -insecure after that point — say, in Go 1.16 (when every still-supported version of the Go toolchain will have GOINSECURE).

@witchard
Copy link
Contributor Author

@witchard witchard commented Feb 28, 2020

Does it make sense then to log something like “-insecure will be deprecated soon, please consider using GOINSECURE” when someone uses the -insecure flag?

There is also the non-modules go get which doesn’t support GOINSECURE - https://github.com/golang/go/blob/master/src/cmd/go/internal/get/get.go#L391; is it worth back porting GOINSECURE into that or will that code disappear at some point?

@bcmills
Copy link
Member

@bcmills bcmills commented Mar 4, 2020

Does it make sense then to log something like “-insecure will be deprecated soon, please consider using GOINSECURE” when someone uses the -insecure flag?

That's a very good idea. We could start doing that as soon as Go 1.15.

@bcmills
Copy link
Member

@bcmills bcmills commented Mar 4, 2020

There is also the non-modules go get which doesn’t support GOINSECURE - https://github.com/golang/go/blob/master/src/cmd/go/internal/get/get.go#L391; is it worth back porting GOINSECURE into that or will that code disappear at some point?

Possibly both? It shouldn't be hard to retrofit GOINSECURE into GOPATH mode, but hopefully GOPATH mode will disappear in a similar timeframe anyway.

@witchard
Copy link
Contributor Author

@witchard witchard commented Mar 5, 2020

I would be up for contributing to either / both of those. Should I change the proposal to focus on these features rather than deprecation?

@bcmills
Copy link
Member

@bcmills bcmills commented Mar 6, 2020

I think we should keep this issue focused on the proposed deprecation: we probably shouldn't warn about the flag if we aren't formally deprecating it, even if we decide to keep it for a while.

If you want to go ahead and send a CL for GOINSECURE in GOPATH mode, we can get that rolling independent of this proposal.

@witchard
Copy link
Contributor Author

@witchard witchard commented Mar 6, 2020

Ok great, should I open a proposal for that or is it fine just to work on it given it’s providing feature parity with modules mode?

@bcmills
Copy link
Member

@bcmills bcmills commented Mar 6, 2020

Just a CL is fine.

@rsc
Copy link
Contributor

@rsc rsc commented Mar 25, 2020

It sounds like this is on hold for having GOINSECURE apply in GOPATH mode, although it's unclear to me that significant changes to GOPATH mode would be worthwhile at this point. Please change back to Active once it is appropriate to consider the deprecation again.

@rsc rsc moved this from Active to Hold in Proposals Mar 25, 2020
@witchard
Copy link
Contributor Author

@witchard witchard commented Mar 25, 2020

I am intending on looking into GOINSECURE for GOPATH, but life and now this whole virus thing are getting in the way of my free time currently 😂. I’ll be sure to let you know once my free time is back on track!

@bcmills
Copy link
Member

@bcmills bcmills commented Mar 27, 2020

In #38108, @perillo notes that if and when we deprecate the -insecure flag, we should be sure to document the different behaviors w.r.t. the checksum database. -insecure today implies GOSUMDB=off, but GOINSECURE does not imply GONOSUMDB for the same modules.

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 21, 2020

Change https://golang.org/cl/229223 mentions this issue: cmd/go/internal/modget: improve GOINSECURE docs

gopherbot pushed a commit that referenced this issue Apr 23, 2020
Recommend use of GOINSECURE over -insecure flang and clarify that GOINSECURE
environment variable does not also imply GONOSUMDB.

Updates #37519 by adding documentation as discussed.

Change-Id: Ia8ab6b3ed1aa559343b72e4ca76c372ee6bf1941
GitHub-Last-Rev: 8d86991
GitHub-Pull-Request: #38572
Reviewed-on: https://go-review.googlesource.com/c/go/+/229223
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Apr 23, 2020

Change https://golang.org/cl/229758 mentions this issue: cmd/go/internal/get: add GOINSECURE support

gopherbot pushed a commit that referenced this issue Sep 1, 2020
Adds support for the GOINSECURE environment variable to GOPATH mode.

Updates #37519.

Change-Id: Ibe3f52b7f30b1395edb000998905ee93abe6cada
GitHub-Last-Rev: e298c00
GitHub-Pull-Request: #38628
Reviewed-on: https://go-review.googlesource.com/c/go/+/229758
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@witchard
Copy link
Contributor Author

@witchard witchard commented Sep 2, 2020

GOINSECURE is now supported in GOPATH mode, but I'm not sure if I can remove the proposal-hold label to open this back up?

@bcmills bcmills removed the Proposal-Hold label Sep 2, 2020
@bcmills bcmills moved this from Hold to Active in Proposals Sep 2, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Sep 16, 2020

This was placed on hold until GOPATH supported GOINSECURE.
Now it does.
What do people think? Should we deprecate and then remove -insecure?
Perhaps deprecate (make it work but print a warning) in Go 1.16 and remove in Go 1.17?

@bcmills
Copy link
Member

@bcmills bcmills commented Sep 16, 2020

I think it makes sense to deprecate the -insecure flag in 1.16 and remove it in 1.17.

witchard added a commit to witchard/go that referenced this issue Sep 19, 2020
Adds deprecation warning for -insecure flag on go get in both modules
and GOPATH mode.

Updates golang#37519.
witchard added a commit to witchard/go that referenced this issue Sep 19, 2020
Adds deprecation warning for -insecure flag on go get in both modules
and GOPATH mode.

Updates golang#37519.
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 19, 2020

Change https://golang.org/cl/255882 mentions this issue: cmd/go/internal/get: warn about -insecure deprecation

gopherbot pushed a commit that referenced this issue Sep 21, 2020
Adds deprecation warning for -insecure flag on go get in both modules
and GOPATH mode.

Updates #37519.

Change-Id: Ie2efeeb4a91e6dda92955295969e9715314ae50e
GitHub-Last-Rev: a9ebe21
GitHub-Pull-Request: #41497
Reviewed-on: https://go-review.googlesource.com/c/go/+/255882
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Trust: Michael Matloob <matloob@golang.org>
Trust: Jay Conrod <jayconrod@google.com>
@witchard
Copy link
Contributor Author

@witchard witchard commented Sep 21, 2020

The deprecation warning is now merged :). Should we put something about it in the release notes for 1.16? What is the process for doing that, is it just a PR to edit https://github.com/golang/go/blob/master/doc/go1.16.html?

@bcmills
Copy link
Member

@bcmills bcmills commented Sep 21, 2020

Yep, exactly.

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 22, 2020

Change https://golang.org/cl/256419 mentions this issue: cmd/go/internal/get: add -insecure deprecation to release notes

@rsc
Copy link
Contributor

@rsc rsc commented Sep 23, 2020

Based on the discussion above this seems like a likely accept.
(I see the code has already landed but might as well finish the process.)

@rsc rsc moved this from Active to Likely Accept in Proposals Sep 23, 2020
gopherbot pushed a commit that referenced this issue Sep 24, 2020
Updates #37519.

Change-Id: Iddf88a24334d4740f9c40caa2354127298692eeb
GitHub-Last-Rev: deda4c8
GitHub-Pull-Request: #41545
Reviewed-on: https://go-review.googlesource.com/c/go/+/256419
Reviewed-by: Jay Conrod <jayconrod@google.com>
Trust: Jay Conrod <jayconrod@google.com>
Trust: Bryan C. Mills <bcmills@google.com>
@witchard
Copy link
Contributor Author

@witchard witchard commented Sep 24, 2020

Yes, sorry about that! I got keen and put up the code changes because I didn't want to miss the code freeze.

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 24, 2020

Change https://golang.org/cl/257157 mentions this issue: cmd/go/internal/get: improve -insecure deprecation docs

@rsc
Copy link
Contributor

@rsc rsc commented Sep 30, 2020

No change in consensus, so accepted.

(@witchard, the code freeze is not for another month, for what it's worth.)

@rsc rsc moved this from Likely Accept to Accepted in Proposals Sep 30, 2020
@rsc rsc modified the milestones: Proposal, Backlog Sep 30, 2020
@rsc rsc changed the title proposal: cmd/go: deprecate -insecure on go get cmd/go: deprecate -insecure on go get Sep 30, 2020
gopherbot pushed a commit that referenced this issue Oct 6, 2020
Updates #37519

Change-Id: I212607f1839b729d7da24b1258e56997b13ad830
GitHub-Last-Rev: db6d3c8
GitHub-Pull-Request: #41613
Reviewed-on: https://go-review.googlesource.com/c/go/+/257157
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Jay Conrod <jayconrod@google.com>
Trust: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Accepted
Linked pull requests

Successfully merging a pull request may close this issue.

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