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

Update CertID format as per draft-ietf-acme-ari-02 #2066

Merged
merged 3 commits into from
Jan 24, 2024

Conversation

beautifulentropy
Copy link
Contributor

@beautifulentropy beautifulentropy commented Dec 6, 2023

  • Implemented the updated CertID format as specified in draft-ietf-acme-ari-02.
  • Removed the --ari-hash-name flag, which is no longer necessary with the updated format.
  • Enabled ARI usage for users with --no-bundle option. Previously, this was not possible as the draft-ietf-acme-ari-01 format required metadata available only in the issuer.

ATTENTION: this must NOT be merged and should remain in draft until letsencrypt/boulder#7184 is merged and deployed.

UPDATE: this change has been merged, but it will not be deployed to Production until a few days into the new year. I'll update this issue when that's the case.

@beautifulentropy
Copy link
Contributor Author

beautifulentropy commented Dec 6, 2023

I recall this same failure in #1912 which added inital support for ARI:

Error: cmd/cmd_renew.go:128:1: cyclomatic complexity 15 of func `renewForDomains` is high (> 12) (gocyclo)
func renewForDomains(ctx *cli.Context, client *lego.Client, certsStorage *CertificatesStorage, bundle bool, meta map[string]string) error {
^
make: *** [Makefile:36: checks] Error 1
Error: Process completed with exit code 2.
@ldez
Copy link
Member

ldez commented Dec 6, 2023

for now, you can ignore the linter about cyclomatic complexity, I will look at the best way to handle that.

@ldez
Copy link
Member

ldez commented Jan 18, 2024

I fixed the problem of cyclomatic complexity.
Just waiting for the deployment to Production.

@ldez ldez added contrib/waiting-for-feedback breaking-change Require or introduce a breaking change. labels Jan 18, 2024
@beautifulentropy
Copy link
Contributor Author

beautifulentropy commented Jan 24, 2024

I fixed the problem of cyclomatic complexity. Just waiting for the deployment to Production.

Thanks for fixing this @ldez. Just checked our SRE team and support for this portion of draft-ietf-acme-ari-02 has been successfully deployed to both staging and production.

@ldez
Copy link
Member

ldez commented Jan 24, 2024

Maybe you can set the PR as "Ready for review"?

@beautifulentropy beautifulentropy marked this pull request as ready for review January 24, 2024 20:11
@ldez ldez added this to the v4.15 milestone Jan 24, 2024
@ldez ldez enabled auto-merge (squash) January 24, 2024 20:13
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Note: this change is breaking but it's expected of a draft. So we will not create a major version only for this change.

@ldez ldez merged commit 3c73f62 into go-acme:master Jan 24, 2024
7 checks passed
@ldez ldez added the area/ari ACME Renewal Information Extension label Feb 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ari ACME Renewal Information Extension area/lib breaking-change Require or introduce a breaking change. enhancement
2 participants