Skip to content

Conversation

@jprenken
Copy link
Contributor

Fixes #8237

@jprenken jprenken marked this pull request as ready for review July 5, 2025 06:20
@jprenken jprenken requested a review from a team as a code owner July 5, 2025 06:20
@jprenken jprenken requested a review from beautifulentropy July 5, 2025 06:20
@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2025

@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.

@jprenken
Copy link
Contributor Author

jprenken commented Jul 5, 2025

@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.

Copy link
Member

@beautifulentropy beautifulentropy left a 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)
}

Copy link
Member

@beautifulentropy beautifulentropy Jul 7, 2025

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
}

Copy link
Contributor Author

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
jsha previously approved these changes Jul 8, 2025
Copy link
Contributor

@jsha jsha left a 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.

@jprenken jprenken dismissed stale reviews from jsha and beautifulentropy via f2a6d8a July 8, 2025 23:31
@jprenken jprenken requested review from beautifulentropy and jsha July 8, 2025 23:31
@jsha jsha merged commit 6ba4207 into main Jul 9, 2025
14 checks passed
@jsha jsha deleted the ip-denylists branch July 9, 2025 18:28
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.

policy: Implement IP address blocklists

5 participants