RA: Control MaxNames via profile#8019
Conversation
|
@aarongable, this PR appears to contain configuration and/or SQL schema changes. Please ensure that a corresponding deployment ticket has been filed with the new values. |
206e3a8 to
ef66241
Compare
ef66241 to
f846336
Compare
beautifulentropy
left a comment
There was a problem hiding this comment.
Looks great, just two optional changes. I'm happy to see this coming to fruition so quickly!
jsha
left a comment
There was a problem hiding this comment.
The maxNames config in WFE was added in #7201. I don't see the reasoning in the description or review comments, but from context I'm pretty sure the idea was to prevent arbitrarily many writes to our key-value rate limit storage from a single new-order.
That check was hoisted up from transaction construction to WFE's NewOrder in #7530.
I think this PR would re-allow that behavior. Perhaps the right fix is, in our rate limit system, to just hardcode the highest maxNames we expect to ever have (100) and reject queries above that (for the relevant rate limits).
8ed0cae
|
I've addressed comments, moved the WFE's check into the ratelimit package, and fixed a WFE bug that I uncovered in the process. PTAL! |
Add MaxNames to the set of things that can be configured on a per-profile basis. Remove all references to the RA's global maxNames, replacing them with reference's to the current profile's maxNames. Add code to the RA's main() to copy a globally-configured MaxNames into each profile, for deployability.
Also remove any understanding of MaxNames from the WFE, as it is redundant with the RA and is not configured in staging or prod. Instead, hardcode the upper limit of 100 into the ratelimit package itself.
IN-11055 tracks the corresponding prod config changes.
Fixes #7993