Split IPRulesService into IPRulesEnforcer and IPRulesService

[iain-macdonald]

Related issues: https://github.com/buildbuddy-io/buildbuddy-internal/issues/6797

[Copilot]

Pull request overview

This PR splits the previous IPRulesService responsibilities into two components: an IPRulesEnforcer used on request/auth paths, and an IPRulesService used for IP rule CRUD/config management APIs.

Changes:

• Introduces interfaces.IPRulesEnforcer and wires it into gRPC/HTTP interceptors and other auth-adjacent callsites.
• Adds new enterprise packages ip_rules_enforcer (enforcement + caching) and ip_rules_service (CRUD/config + validation), with corresponding tests and BUILD targets.
• Updates the enterprise server startup registration to register both components.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
server/rpc/interceptors/interceptors.go	Switches unary/stream IP auth interceptor to use GetIPRulesEnforcer()
server/http/interceptors/interceptors.go	Switches HTTP IP authorization middleware to use GetIPRulesEnforcer()
server/real_environment/real_environment.go	Adds storage + getters/setters for IPRulesEnforcer on RealEnv
server/interfaces/interfaces.go	Splits IPRulesService into IPRulesEnforcer + IPRulesService interfaces
server/environment/environment.go	Extends Env interface with GetIPRulesEnforcer()
server/buildbuddy_server/buildbuddy_server.go	Updates GetUser to consult IPRulesEnforcer for selected-group access
server/build_event_protocol/build_event_handler/build_event_handler.go	Uses IPRulesEnforcer for authenticated BEP requests
enterprise/server/ip_rules_enforcer/ip_rules_enforcer.go	New enforcement implementation (formerly part of iprules)
enterprise/server/ip_rules_enforcer/ip_rules_enforcer_test.go	New enforcer test coverage
enterprise/server/ip_rules_enforcer/BUILD	Bazel targets for enforcer
enterprise/server/ip_rules_service/ip_rules_service.go	New CRUD/config service which delegates lockout-checks to enforcer
enterprise/server/ip_rules_service/ip_rules_service_test.go	New service test coverage
enterprise/server/ip_rules_service/BUILD	Bazel targets for service
enterprise/server/cmd/server/main.go	Registers ip_rules_enforcer then ip_rules_service
enterprise/server/cmd/server/BUILD	Updates deps to new packages
enterprise/server/iprules/iprules.go	Removed (logic moved to the two new packages)
enterprise/server/iprules/iprules_test.go	Removed (tests moved/split accordingly)

---

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

[vadimberezniker]

Do they need to be split into separate packages? Can they co-exist in the iprules package?

[iain-macdonald]

Do they need to be split into separate packages? Can they co-exist in the iprules package?

They could coexist in the iprules package, it's just a question of how large we'd like our packages to be. The reason I split them up is because they're responsible for different things, as evidenced by their APIs. I've refactored some of our larger packages for use in the cache proxy and I always find it easier to split large packages at reasonable API boundaries, which is what I tried to do in this PR. That helps steer us away from situations like the authentication code, which I find really difficult to change. If you'd prefer to keep these in the same package let me know and I can recombine them. My plan was to add a new remote_ip_rules_enforcer that depends on the ip_rules_enforcer but not the ip_rules_service, though I don't have any ideas for removing the database dependency from ip_rules_enforcer, we'd have to add a new package (local_ip_rules_enforcer) for that 😛.