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

ratelimit: Remove legacy registrations per IP implementation #7760

Merged
merged 6 commits into from
Nov 19, 2024

Conversation

beautifulentropy
Copy link
Member

@beautifulentropy beautifulentropy commented Oct 18, 2024

Part of #7671

@beautifulentropy beautifulentropy force-pushed the remove-legacy-registrations-per-ip-impl branch 2 times, most recently from 52963aa to 884dc1f Compare October 18, 2024 17:01
@beautifulentropy beautifulentropy force-pushed the remove-legacy-registrations-per-ip-impl branch from 884dc1f to b51c916 Compare October 18, 2024 17:04
@beautifulentropy beautifulentropy marked this pull request as ready for review November 18, 2024 17:34
@beautifulentropy beautifulentropy requested a review from a team as a code owner November 18, 2024 17:34
sa/model.go Outdated Show resolved Hide resolved
sa/model.go Outdated Show resolved Hide resolved
aarongable
aarongable previously approved these changes Nov 19, 2024
Copy link
Contributor

@jprenken jprenken left a comment

Choose a reason for hiding this comment

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

Is this deployable in a single release as-is? It seems like if a WFE gets updated before an RA, the RA might reject new registrations due to incomplete gRPC fields.

core/proto/core.proto Outdated Show resolved Hide resolved
@beautifulentropy beautifulentropy changed the title ratelimits: Remove legacy registrations per IP implementation ratelimit: Remove legacy registrations per IP implementation Nov 19, 2024
aarongable
aarongable previously approved these changes Nov 19, 2024
wfe2/wfe.go Outdated Show resolved Hide resolved
@@ -1543,7 +1543,6 @@ func TestNewECDSAAccount(t *testing.T) {
test.Assert(t, len(*acct.Contact) >= 1, "No contact field in account")
test.AssertEquals(t, (*acct.Contact)[0], "mailto:[email protected]")
test.AssertEquals(t, acct.Agreement, "")
test.AssertEquals(t, acct.InitialIP.String(), "1.1.1.1")
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth preserving a single test that confirms that the WFE is hardcoding this to 0.0.0.0 now.

Copy link
Member Author

@beautifulentropy beautifulentropy Nov 19, 2024

Choose a reason for hiding this comment

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

We can't preserve this test unless we also preserve the code that marshals the corepb.Registration back into the core.Registration object we can show the subscriber.

Co-authored-by: Aaron Gable <[email protected]>
@beautifulentropy beautifulentropy merged commit a8cdaf8 into main Nov 19, 2024
12 checks passed
@beautifulentropy beautifulentropy deleted the remove-legacy-registrations-per-ip-impl branch November 19, 2024 23:39
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