-
-
Notifications
You must be signed in to change notification settings - Fork 632
Add IP prefix blocklist #8278
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
Add IP prefix blocklist #8278
Conversation
|
@jprenken, 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. |
Thanks, bot! But these config changes are merely optional. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed an opportunity to improve lookup performance during run time with a little pre-processing at load time. Otherwise this change looks good to me!
| } | ||
| prefixes = append(prefixes, prefix) | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By removing redundant prefixes at load time, we could reduce the size of the list and improve per-request lookup performance at run time.
// canonicalizePrefixes returns a sorted, deduplicated, and non-overlapping
// slice of netip.Prefix.
func canonicalizePrefixes(in []netip.Prefix) []netip.Prefix {
if len(in) == 0 {
return nil
}
cp := make([]netip.Prefix, len(in))
copy(cp, in)
slices.SortFunc(cp, func(a, b netip.Prefix) int {
if a.Addr() == b.Addr() {
return a.Bits() - b.Bits()
}
if a.Addr().Less(b.Addr()) {
return -1
}
return 1
})
out := make([]netip.Prefix, 0, len(cp))
for _, p := range cp {
if len(out) > 0 && out[len(out)-1].Contains(p.Addr()) &&
out[len(out)-1].Bits() <= p.Bits() {
continue
}
out = append(out, p)
}
return out
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, if we decide to add more complex code to improve lookup performance, I'd rather integrate bart.
jsha
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks good! A naming and a comment comment.
Fixes #8237