Remove ipRulesProvider.get's skipRule parameter#11610
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the IP rules enforcement API to remove the skipCache flag, replacing it with an explicit cache invalidation method, and updates the IP rules service + tests to use the new flow.
Changes:
- Update
interfaces.IPRulesEnforcerto addInvalidateCache()and simplifyCheck()signature. - Update
ip_rules_serviceto invalidate cached rules before performing lockout-prevention checks. - Rework
ip_rules_enforcercaching to retain rule IDs in cached entries soskipRuleIDcan be applied at check time.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| server/interfaces/interfaces.go | Adjusts IPRulesEnforcer interface to add cache invalidation and simplify Check. |
| enterprise/server/ip_rules_service/ip_rules_service.go | Uses InvalidateCache() before Check() when enabling enforcement or validating rule deletion. |
| enterprise/server/ip_rules_service/ip_rules_service_test.go | Updates fake enforcer + assertions to reflect the new invalidation/check call sequence. |
| enterprise/server/ip_rules_enforcer/ip_rules_enforcer.go | Introduces ipRule (ID + CIDR) cache entries, adds invalidation plumbing, and updates Check() behavior. |
| enterprise/server/ip_rules_enforcer/ip_rules_enforcer_test.go | Updates tests for the new Check() signature. |
💡 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.
de81629 to
350675a
Compare
| allowed *net.IPNet | ||
| } | ||
|
|
||
| type ipRuleCache interface { |
There was a problem hiding this comment.
This type has some weird stuff going on with exports. It's not exported, but it's method are, but the method operate on the unexported ipRule type.
There was a problem hiding this comment.
I'd like to just use an lru.LRU as an implementation, so the methods need to be public, unless I'm missing something.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
350675a to
e6e11c7
Compare
Related issues: https://github.com/buildbuddy-io/buildbuddy-internal/issues/6797