ratelimits: Fix loading of override limits from the database

[beautifulentropy]

NewTransactionBuilderFromDatabase called hydrateOverrideLimit(), which should only be used to hydrate overrides from YAML, where they’re stored as limits and corresponding raw IDs (example.com, 8.8.8.8, 12345, etc.) that must be normalized and combined with the limit enum to form a bucketKey. However, the database stores bucketKeys that have already been validated and normalized before being written. As a result, every override entry was treated as invalid.

Instead, we now skip hydrateOverrideLimit() and use the bucketKey exactly as stored by SA, with no additional normalization when loading overrides from the database.

Also dropped rehydration of the comment field. It’s never referenced at runtime and consumes unnecessary memory.

Important: this change makes the overrides table the source of truth.

Fixes #8590

[aarongable]

I think there's a disconnect between the contents of this diff and how its described in the PR description.

This diff removes a call to hydrateOverrideLimit. That function takes a CertificatesPerDomain bucket key that looks like 1.2.3.4 and converts it to 1.2.3.4/32, and it takes a CertificatesPerFQDNSet bucket key that looks like example.com,example.org and converts it to 0xDEADBEEF. Are we really storing CIDR prefixes and hashed fqdnsets in the database, such that we don't have to rehydrate these bucket keys? I thought we'd decided that the database would be storing human-readable bucket keys, just like the yaml files do. (Editor's note: it does indeed look like we're storing unreadable bucket keys, since both the admin tool and the sfe importer use BuildBucketKey, which in turn uses HashIdentifiers. I'm sure I reviewed these PRs at the time, but I am nonetheless surprised right now.)

But that said, the PR description sounds like the problem is the difference between 5:example.com and just example.com, rather than the problem being the difference between example.com and 0xDEADBEEF.

Beyond that, we're also transitively losing calls to limitName.isValid() (which should be unaffected by rehydration, and therefore would be nice to preserve) and to validateIdForName (which I'm guessing is the real culprit here, since 0xDEADBEEF isn't a valid ID for CertificatesPerFQDNSet). But if we were storing the same kinds of bucket keys as were in the yaml files, then those validations could be preserved.

[aarongable]

Chatted about this out-of-band. Makes sense now! Based on the PR description I was thinking that the whole NewTransactionBuilderFromDatabase function was assuming keys would arrive in the old yaml-esque format, and was very confused about where and how it was doing the transformation between bare IDs and bucketKeys. But in fact most of this function is totally correct, and it's only the call tp hydrateOverrideLimit that was making incorrect assumptions.

Diff now completely LGTM, I think the PR description should be updated to more specifically call out hydrateOverrideLimit and why it's not necessary when loading from the DB.