-
-
Notifications
You must be signed in to change notification settings - Fork 629
Add AlreadyRevoked handling to revokedCertificates table inserts #8408
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay. I had a pending comment in here I thought I'd already sent.
e1c002b to
d444864
Compare
|
I've updated this PR to make the serials index unique, and then rely on detecting duplicate inserts, like the first draft. I've updated IN-11835 to indicate that the new index should be unique. I think that including the index update in this PR, rather than as a predecessor PR that must be deployed first, is fine because the inserts into this table are still actually protected by the updates to the |
Within
sa.addRevokedCertificate, detect if the insert failed due to a duplicate primary key, and returnberrors.AlreadyRevokedin that case. This matches the behavior ofsa.RevokeCertificatewhen an update to thecertificateStatustable modifies no rows.This change has no immediate impact, because addRevokedCertificate is only called after RevokeCertificate's update to the certificateStatus table has succeeded: if the first operation returns AlreadyRevoked, this code is never reached. However, we plan to remove the certificateStatus table in the future, so this code will need to return AlreadyRevoked when appropriate.
Part of #8322
Warning
Do not merge before #8427Do not merge before IN-11835 is complete