Skip to content

ratelimit: Remove legacy registrations per IP implementation#7760

Merged
beautifulentropy merged 6 commits into
mainfrom
remove-legacy-registrations-per-ip-impl
Nov 19, 2024
Merged

ratelimit: Remove legacy registrations per IP implementation#7760
beautifulentropy merged 6 commits into
mainfrom
remove-legacy-registrations-per-ip-impl

Conversation

@beautifulentropy

@beautifulentropy beautifulentropy commented Oct 18, 2024

Copy link
Copy Markdown
Member

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
Comment thread sa/model.go Outdated
Comment thread sa/model.go Outdated
@aarongable aarongable requested review from jprenken and jsha November 18, 2024 17:57
aarongable
aarongable previously approved these changes Nov 19, 2024

@jprenken jprenken left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread core/proto/core.proto
@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
Comment thread wfe2/wfe.go Outdated
Comment thread wfe2/wfe_test.go
test.Assert(t, len(*acct.Contact) >= 1, "No contact field in account")
test.AssertEquals(t, (*acct.Contact)[0], "mailto:person@mail.com")
test.AssertEquals(t, acct.Agreement, "")
test.AssertEquals(t, acct.InitialIP.String(), "1.1.1.1")

Copy link
Copy Markdown
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.

@beautifulentropy beautifulentropy Nov 19, 2024

Copy link
Copy Markdown
Member Author

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 <aaron@letsencrypt.org>
@beautifulentropy beautifulentropy merged commit a8cdaf8 into main Nov 19, 2024
@beautifulentropy beautifulentropy deleted the remove-legacy-registrations-per-ip-impl branch November 19, 2024 23:39
beautifulentropy added a commit that referenced this pull request Jan 13, 2025
The initialIP column has been defaulted to 0.0.0.0 since #7760. Remove
this field from the all structs while leaving the schema itself intact.

Part of #7917
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