Skip to content

Conversation

mcpherrinm
Copy link
Contributor

@mcpherrinm mcpherrinm commented Sep 29, 2025

Switch to outputting the new checksum format.

Part of #8414


Warning

Do not merge until #8413 has been successfully deployed, likely Oct 9

This adds a new log checksum format without the variable-width integer encoding
currently used in Boulder. It's still CRC32, just encoded directly into base64
instead of with an extra layer of varint-encoding. Note that despite using the
varint encoding, Boulder always writes the full buffer out, so it's zero-padded
to 7 bytes.

This doesn't switch to using the new format yet, but just accepts it in the log
validator.

This is change 1/3 in a sequence:

* Allow new format
* Switch to new format
* Remove support for old format.

Each of these 3 changes should land one release apart.
Update test case to use the same string
This must be in a release after #8413

It is the 2nd of three changes switching the checksum algorithm.
@mcpherrinm mcpherrinm marked this pull request as ready for review September 29, 2025 19:48
@mcpherrinm mcpherrinm requested a review from a team as a code owner September 29, 2025 19:48
@aarongable aarongable changed the base branch from main to mattm-new-format-1 September 29, 2025 19:53
aarongable
aarongable previously approved these changes Sep 29, 2025
Base automatically changed from mattm-new-format-1 to main September 29, 2025 20:07
@jsha jsha dismissed aarongable’s stale review September 29, 2025 20:07

The base branch was changed.

jsha
jsha previously approved these changes Sep 29, 2025
Copy link
Contributor

@jsha jsha left a comment

Choose a reason for hiding this comment

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

Approved in principle, waiting for dependency to deploy.

jprenken
jprenken previously approved these changes Sep 29, 2025
@mcpherrinm mcpherrinm dismissed stale reviews from jprenken and jsha via 64164ec September 30, 2025 14:40
@mcpherrinm
Copy link
Contributor Author

Merged main to resolve merge conflicts. This branch is a bit gnarly now though; how would the reviewers feel if I rebased and force-pushed to get a clean single-commit PR here?

Copy link
Contributor

@aarongable aarongable left a comment

Choose a reason for hiding this comment

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

Eh, the diff is really clean, and everything gets squashed when we merge the PR, so I think it's fine as-is. LGTM for landing any time since the prereq change made it into yesterday's tag.

(I do frequently rebase and force-push changes in situations like this, but only when I've kept them in draft mode up to that point.)

@aarongable aarongable requested review from jsha and jprenken September 30, 2025 16:44
@aarongable
Copy link
Contributor

Waiting to merge until after next week's tag has been cut and fully deployed (likely Oct 9), since this week's tag had to be rolled back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants