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

admin block-key failed #7460

Closed
Tracked by #7351
aarongable opened this issue Apr 30, 2024 · 0 comments · Fixed by #7465 or #7466
Closed
Tracked by #7351

admin block-key failed #7460

aarongable opened this issue Apr 30, 2024 · 0 comments · Fixed by #7465 or #7466
Assignees

Comments

@aarongable
Copy link
Contributor

aarongable commented Apr 30, 2024

Redacted logs below:

$ admin -config config.json block-key -private-key /path/to/key.pem
Debug server listening on :8000
Versions: admin=(7ee5b469) Golang=(go1.22.2) BuildHost=(...)
[AUDIT] admin tool executing a dry-run with the following arguments: "admin -config config.json block-key -private-key /path/to/key.pem"
[AUDIT] failed to block <...>: streaming serials from SA: rpc error: code = Unknown desc = reading db: reading db: failed to select keyHashToSerial: sql: converting argument $1 type: unsupported type []interface {}, a slice of interface
[AUDIT] admin tool has successfully completed executing a dry-run with the following arguments: "admin -config config.json block-key -private-key /path/to/key.pem"
Dry run complete. Pass -dry-run=false to mutate the database.

Two issues:

  1. The SA's GetSerialsByKey gRPC method failed
  2. The admin tool acted as though it had succeeded
@aarongable aarongable added this to the Sprint 2024-04-30 milestone Apr 30, 2024
@aarongable aarongable self-assigned this Apr 30, 2024
aarongable added a commit that referenced this issue May 1, 2024
Correctly explode the params slice with Go's "..." notation so that
gorp/go-sql-driver correctly receives each element of the params slice,
rather than receiving the slice as a whole. Also use the SA's clock,
rather than the DB's, to control which certs are selected -- in
deployments this wouldn't make a difference but in test those clocks can
be very different.

Add two unit tests to ensure this query does not regress, and create a
generic fake gRPC server stream for use in several SA tests including
the new ones.

Fixes #7460
aarongable added a commit that referenced this issue May 1, 2024
…7466)

While we don't want to halt the admin tool in the midst of its parallel
processing, we can keep track of whether it has encountered any errors
and raise one meta-error at the end of its execution. This will prevent
the top-level admin code from claiming that execution succeeded, and
ensure operators notice any previously-logged errors.

As part of this, fix the SA's GetLintPrecertificate wrapper to actually
call the SARO's GetLintPrecertificate, instead of incorrectly calling
the SARO's GetCertificate.

Fixes #7460
vbaranovskiy-plesk pushed a commit to plesk/boulder that referenced this issue May 30, 2024
Correctly explode the params slice with Go's "..." notation so that
gorp/go-sql-driver correctly receives each element of the params slice,
rather than receiving the slice as a whole. Also use the SA's clock,
rather than the DB's, to control which certs are selected -- in
deployments this wouldn't make a difference but in test those clocks can
be very different.

Add two unit tests to ensure this query does not regress, and create a
generic fake gRPC server stream for use in several SA tests including
the new ones.

Fixes letsencrypt#7460
vbaranovskiy-plesk pushed a commit to plesk/boulder that referenced this issue May 30, 2024
…etsencrypt#7466)

While we don't want to halt the admin tool in the midst of its parallel
processing, we can keep track of whether it has encountered any errors
and raise one meta-error at the end of its execution. This will prevent
the top-level admin code from claiming that execution succeeded, and
ensure operators notice any previously-logged errors.

As part of this, fix the SA's GetLintPrecertificate wrapper to actually
call the SARO's GetLintPrecertificate, instead of incorrectly calling
the SARO's GetCertificate.

Fixes letsencrypt#7460
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant