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

x/crypto/acme: delete a bunch of ACMEv1 code #46654

Closed
bradfitz opened this issue Jun 8, 2021 · 10 comments
Closed

x/crypto/acme: delete a bunch of ACMEv1 code #46654

bradfitz opened this issue Jun 8, 2021 · 10 comments
Labels
FrozenDueToAge Proposal Proposal-Accepted Proposal-FinalCommentPeriod
Projects
Proposals (old)
Milestone
Unreleased

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Jun 8, 2021

LetsEncrypt finally turned off ACMEv1 the other day.

AFAIU, they're the only CA who supported pre-RFC ACMEv1.

golang.org/x/crypto/acme contains both ACMEv1 and ACMEv2 clients and is pretty hard to read as a result. (both as a user reading godoc, and reading the code)

I propose we keep API compatibility but delete all the v1 code and do everything possible to hide (perhaps via embedding unexported types), deemphasize, or warn against its use in the godoc. And return errors if it's used.

And then rename a bunch of internal methods that have "RFC" in their name to drop the RFC suffix.

If there are no objections, I can send a beautifully red CL.

/cc @x1ddos, @FiloSottile @dmitshur
@gopherbot gopherbot added this to the Proposal milestone Jun 8, 2021
@bradfitz
Copy link
Contributor Author

bradfitz commented Jun 8, 2021

(Also in the acme/autocert package)

@dmitshur
Copy link
Contributor

dmitshur commented Jun 8, 2021

Also CC @rolandshoemaker via owners.

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Jun 16, 2021
@bradfitz
Copy link
Contributor Author

Oh, maybe we can do even better and nuke all the old symbols, as the package comment says:

This package is a work in progress and makes no API stability promises

@gopherbot
Copy link

Change https://golang.org/cl/342449 mentions this issue: acme: remove support for pre-RFC 8555 ACME spec

@rsc
Copy link
Contributor

rsc commented Oct 27, 2021

Not sure about deleting old symbols and breaking builds but deleting the code seems like a win.
If we just delete the code, that probably doesn't have to go through the proposal process.

@rsc
Copy link
Contributor

rsc commented Oct 27, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals (old) Oct 27, 2021
benburkert pushed a commit to benburkert/crypto that referenced this issue Oct 29, 2021
@bradfitz @benburkert
acme: remove support for pre-RFC 8555 ACME spec
5bd4f74 
LetsEncrypt removed it anyway.

No API changes. Just a lot of deleted code.

Fixes golang/go#46654

Change-Id: I6e22f95bbc771abde8a445d7a7cc09d11f5eff93
benburkert pushed a commit to benburkert/crypto that referenced this issue Oct 29, 2021
@bradfitz @benburkert
acme: remove support for pre-RFC 8555 ACME spec
dfba3c5 
LetsEncrypt removed it anyway.

No API changes. Just a lot of deleted code.

Fixes golang/go#46654

Change-Id: I6e22f95bbc771abde8a445d7a7cc09d11f5eff93
@rsc
Copy link
Contributor

rsc commented Nov 3, 2021

This seems like a likely accept, but can we leave the exported symbols around with stub implementations that panic?
I'd rather not break builds that were trying to do both v1 and v2.

@rsc rsc moved this from Active to Likely Accept in Proposals (old) Nov 3, 2021
@rsc
Copy link
Contributor

rsc commented Nov 3, 2021

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) Nov 10, 2021
@rsc
Copy link
Contributor

rsc commented Nov 10, 2021

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: x/crypto/acme: delete a bunch of ACMEv1 code x/crypto/acme: delete a bunch of ACMEv1 code Nov 10, 2021
@rsc rsc modified the milestones: Proposal, Backlog Nov 10, 2021
@gopherbot
Copy link

Change https://golang.org/cl/380314 mentions this issue: acme: remove support for pre-RFC 8555 ACME spec

@dmitshur dmitshur modified the milestones: Backlog, Unreleased Feb 9, 2022
owenthereal pushed a commit to owenthereal/upterm.crypto that referenced this issue Mar 5, 2022
@benburkert @bradfitz @owenthereal
acme: remove support for pre-RFC 8555 ACME spec
f0b4878 
LetsEncrypt removed it anyway.

No API changes. Just a lot of deleted code.

Fixes golang/go#46654

Co-authored-by: Brad Fitzpatrick <bradfitz@golang.org>
Change-Id: I65cd0d33236033682b767403ad92aa572bee4fdd
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/380314
Trust: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Brad Fitzpatrick <bradfitz@golang.org>
iamacarpet pushed a commit to affordablemobiles/xcrypto that referenced this issue Aug 2, 2022
@benburkert @bradfitz @iamacarpet
acme: remove support for pre-RFC 8555 ACME spec
d8d5c79 
LetsEncrypt removed it anyway.

No API changes. Just a lot of deleted code.

Fixes golang/go#46654

Co-authored-by: Brad Fitzpatrick <bradfitz@golang.org>
Change-Id: I65cd0d33236033682b767403ad92aa572bee4fdd
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/380314
Trust: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Feb 9, 2023
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
@benburkert @bradfitz
acme: remove support for pre-RFC 8555 ACME spec
e1ee087 
LetsEncrypt removed it anyway.

No API changes. Just a lot of deleted code.

Fixes golang/go#46654

Co-authored-by: Brad Fitzpatrick <bradfitz@golang.org>
Change-Id: I65cd0d33236033682b767403ad92aa572bee4fdd
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/380314
Trust: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Brad Fitzpatrick <bradfitz@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
@benburkert @bradfitz
acme: remove support for pre-RFC 8555 ACME spec
8cb5015 
LetsEncrypt removed it anyway.

No API changes. Just a lot of deleted code.

Fixes golang/go#46654

Co-authored-by: Brad Fitzpatrick <bradfitz@golang.org>
Change-Id: I65cd0d33236033682b767403ad92aa572bee4fdd
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/380314
Trust: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Brad Fitzpatrick <bradfitz@golang.org>
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 Proposal-FinalCommentPeriod
Projects
No open projects
Proposals (old)
Accepted
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@bradfitz @rsc @dmitshur @gopherbot