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

Jump to bottom

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

Closed
witchard opened this issue Feb 27, 2020 · 28 comments
Closed

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

witchard opened this issue Feb 27, 2020 · 28 comments
Labels
FrozenDueToAge Proposal Proposal-Accepted
Projects
Proposals (old)
Milestone
Go1.17

Comments

@witchard
Copy link
Contributor

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
@bcmills
Copy link
Contributor

bcmills commented Feb 28, 2020

CC @jayconrod @matloob @FiloSottile @katiehockman

@bcmills
Copy link
Contributor

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

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?

@rsc rsc added this to Active in Proposals (old) Mar 4, 2020
@rsc rsc mentioned this issue Mar 4, 2020
Open
@bcmills
Copy link
Contributor

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
Contributor

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 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
Contributor

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 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
Contributor

bcmills commented Mar 6, 2020

Just a CL is fine.

@rsc
Copy link
Contributor

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 (old) Mar 25, 2020
@witchard
Copy link
Contributor Author

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!

@perillo perillo mentioned this issue Mar 27, 2020
Closed
@bcmills
Copy link
Contributor

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.

@witchard witchard mentioned this issue Apr 21, 2020
Closed
@gopherbot
Copy link

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
@witchard
cmd/go/internal/modget: improve GOINSECURE docs
708ac9a 
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>
@witchard witchard mentioned this issue Apr 23, 2020
Closed
@gopherbot
Copy link

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
@witchard
cmd/go/internal/get: add GOINSECURE support
ac55d58 
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 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 moved this from Hold to Active in Proposals (old) Sep 2, 2020
witchard added a commit to witchard/go that referenced this issue Sep 22, 2020
@witchard
cmd/go/internal/get: add -insecure deprecation to release notes
64ea1ad 
Updates golang#37519.
@witchard witchard mentioned this issue Sep 22, 2020
Closed
@gopherbot
Copy link

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

@rsc
Copy link
Contributor

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 (old) Sep 23, 2020
gopherbot pushed a commit that referenced this issue Sep 24, 2020
@witchard
doc/go1.16: add -insecure deprecation to release notes
9e073b5 
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

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

witchard added a commit to witchard/go that referenced this issue Sep 24, 2020
@witchard
cmd/go/internal/get: improve -insecure deprecation docs
7a2c9b4 
Updates golang#37519
@witchard witchard mentioned this issue Sep 24, 2020
Closed
@gopherbot
Copy link

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

@rsc
Copy link
Contributor

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 (old) 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
@witchard
cmd/go/internal/get: improve -insecure deprecation docs
1fb149f 
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>
@hyangah hyangah mentioned this issue Oct 22, 2020
Closed
@witchard
Copy link
Contributor Author

The go 1.16 modules blog (https://blog.golang.org/go116-module-changes) suggests that GOPATH mode will be dropped in go 1.17. Therefore, does it make sense to also drop go get -insecure at the same time given that the flag is not all that useful with modules mode as it isn't supported by other modules related commands (and hence one of the reasons for GOINSECURE in the first place)? Or is there too much flux with the removal of GOPATH mode that you'd prefer to hold off?

@jayconrod
Copy link
Contributor

I think it makes sense to remove -insecure in Go 1.17. I wouldn't expect that to cause problems with removing GOPATH-mode.

@jayconrod jayconrod modified the milestones: Backlog, Go1.17 Mar 1, 2021
@witchard witchard mentioned this issue Mar 1, 2021
Closed
@gopherbot
Copy link

Change https://golang.org/cl/297709 mentions this issue: cmd/go: remove -insecure flag on go get

@bcmills bcmills mentioned this issue Mar 9, 2021
Closed
@golang golang locked and limited conversation to collaborators Mar 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal Proposal-Accepted
Projects
No open projects
Proposals (old)
Accepted
Milestone
Go1.17
Development

Successfully merging a pull request may close this issue.

cmd/go: remove -insecure flag on go get witchard/go
5 participants
@rsc @jayconrod @witchard @bcmills @gopherbot