Skip to content

Replace the iprules LRU with an lru.LRU#11579

Merged
iain-macdonald merged 1 commit intomasterfrom
im/ca3cc326c9d
Mar 13, 2026
Merged

Replace the iprules LRU with an lru.LRU#11579
iain-macdonald merged 1 commit intomasterfrom
im/ca3cc326c9d

Conversation

@iain-macdonald
Copy link
Contributor

@iain-macdonald iain-macdonald commented Mar 12, 2026

Depends on: #11565

@iain-macdonald iain-macdonald force-pushed the im/ca3cc326c9d branch 2 times, most recently from 365d047 to b2fa573 Compare March 13, 2026 14:33
@iain-macdonald iain-macdonald marked this pull request as ready for review March 13, 2026 14:35
Copilot AI reviewed Mar 13, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the IP rules in-memory cache implementation to use the shared server/util/lru cache (with TTL + thread-safety) instead of a custom LRU wrapper, aligning with the expiring LRU work from #11565.

Changes:

  • Replace the custom memIpRuleCache implementation with a generic lru.LRU[[]*net.IPNet].
  • Update the cache interface to match lru.LRU’s Add signature (returns bool).
  • Configure the cache with TTL-based expiry and thread-safety.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

enterprise/server/iprules/iprules.go
return lru.New[[]*net.IPNet](&lru.Config[[]*net.IPNet]{
TTL: *cacheTTL,
MaxSize: cacheSize,
ThreadSafe: true,
Copy link
Contributor

@vanja-p vanja-p Mar 13, 2026

Choose a reason for hiding this comment

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

this seems like a good suggestion

Copy link
Contributor Author

@iain-macdonald iain-macdonald Mar 13, 2026

Choose a reason for hiding this comment

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

Yeah, definitely. Fixed.

Copy link
Contributor Author

@iain-macdonald iain-macdonald Mar 13, 2026

Choose a reason for hiding this comment

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

Also added a test that caught this

vanja-p
vanja-p approved these changes Mar 13, 2026
View reviewed changes
enterprise/server/iprules/iprules.go Outdated
return &memIpRuleCache{
lru: l,
}, nil
return lru.New[[]*net.IPNet](&lru.Config[[]*net.IPNet]{
Copy link
Contributor

@vanja-p vanja-p Mar 13, 2026

Choose a reason for hiding this comment

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

nit: also don't need the type parameter on New here.

Copy link
Contributor Author

@iain-macdonald iain-macdonald Mar 13, 2026

Choose a reason for hiding this comment

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

Done

enterprise/server/iprules/iprules.go
return lru.New[[]*net.IPNet](&lru.Config[[]*net.IPNet]{
TTL: *cacheTTL,
MaxSize: cacheSize,
ThreadSafe: true,
Copy link
Contributor

@vanja-p vanja-p Mar 13, 2026

Choose a reason for hiding this comment

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

this seems like a good suggestion

@iain-macdonald iain-macdonald merged commit 3f235a7 into master Mar 13, 2026
12 checks passed
@iain-macdonald iain-macdonald deleted the im/ca3cc326c9d branch March 13, 2026 18:36
@iain-macdonald iain-macdonald mentioned this pull request Mar 13, 2026
Merged
iain-macdonald added a commit that referenced this pull request Mar 13, 2026
@iain-macdonald
Replace the ip_rules_enforcer LRU with an lru.LRU (#11596)
3fc9a7c 
I accidentally overwrote
#11579 in
#11555 so here it is
again 🤦
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@vanja-p vanja-p vanja-p approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@iain-macdonald @vanja-p