Dynamic whitelisting of client IPs in Hetzner firewall #147
Conversation
I will use this in tests to control the passing of time (e.g. that sessions get removed after 1 week).
In tests, I want to be able to use @JwtSecurity for fake authentication, but the current code requires the security identity's principal to be the more specific class OidcJwtCallerPrincipal.
We will need this to remove the whitelist only for that user in a session when that user closes the session.
We will need this when a peer closes the WebRTC session, but the session itself is still alive.
📝 WalkthroughWalkthroughAdds Hetzner-backed dynamic IP whitelisting for TURN servers: DB migration and persistence, a Hetzner REST client and stub, a batched/rate-limited HetznerFirewallService with messaging/updater, reactive session handler signatures, request-scoped current-user IP extraction, clock injection, and tests/utilities. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CurrentUserService
participant SessionService
participant CoturnHandler
participant HetznerService
participant RabbitMQ
participant HetznerUpdater
participant HetznerAPI
Client->>CurrentUserService: requestTurnServers()
CurrentUserService->>CurrentUserService: extract userId + clientIp
Client->>SessionService: getSession(..., userId, clientIp)
SessionService->>CoturnHandler: createSession(id, userId, clientIp)
CoturnHandler->>HetznerService: whitelistIpForSession(id, userId, clientIp)
HetznerService->>HetznerService: persistOrGet(entry) / record
HetznerService->>RabbitMQ: emit SyncMessage (hetzner-request-out)
RabbitMQ->>HetznerUpdater: deliver SyncMessage (hetzner-request-in)
HetznerUpdater->>HetznerUpdater: batch requests, build SetFirewallRulesRequest
HetznerUpdater->>HetznerAPI: POST /firewalls/{id}/actions/set_rules
alt success
HetznerAPI-->>HetznerUpdater: SetFirewallRulesResponse
HetznerUpdater->>RabbitMQ: emit ack (hetzner-response-out)
RabbitMQ->>HetznerService: ack (hetzner-response-in)
HetznerService-->>CoturnHandler: complete Uni success
else failure
HetznerAPI-->>HetznerUpdater: error
HetznerUpdater->>HetznerService: complete futures exceptionally
HetznerService-->>CoturnHandler: complete Uni failed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (9)
README.md (1)
82-88: Documentation addition is clear and well-placed.The new "Formatting your code" section follows the existing README structure and provides a straightforward instruction for developers to apply code formatting. The section fits naturally in the developer workflow documentation.
As an optional enhancement, consider documenting the complementary
./gradlew spotlessCheckcommand to verify formatting without applying changes—useful for CI pipelines or pre-commit verification.src/test/kotlin/com/faforever/icebreaker/util/FakeClock.kt (1)
17-21: Auto-advancinginstant()may cause surprising test behavior.Each call to
instant()advances time by 1 second as a side effect. This can lead to flaky or hard-to-debug tests when code callsinstant()multiple times (e.g., for logging, metrics, or multiple time checks). Consider making time advancement explicit via a dedicatedadvance(Duration)method, keepinginstant()pure.🔎 Suggested refactor
class FakeClock : Clock() { private var now = Instant.now() private var zone: ZoneId = ZoneOffset.UTC - override fun instant(): Instant { - val oldNow = now - now += Duration.ofSeconds(1) - return oldNow - } + override fun instant(): Instant = now + + fun advance(duration: Duration) { + now += duration + } fun setNow(newNow: Instant) { now = newNow }src/test/kotlin/com/faforever/icebreaker/web/SessionControllerTest.kt (1)
33-51: Consider cleaning up firewall whitelist entries in test setup.The
@BeforeAllmethod clearscoturnServerRepositorybut notfirewallWhitelistRepository. If tests run in a shared database context or if test order changes, stale whitelist entries from previous runs could cause false positives/negatives in assertions at lines 104-106.🔎 Proposed fix
@BeforeAll @Transactional fun insertTestData() { coturnServerRepository.deleteAll() + firewallWhitelistRepository.deleteAll() coturnServerRepository.persist( CoturnServerEntity(src/main/kotlin/com/faforever/icebreaker/service/xirsys/XirsysSessionHandler.kt (2)
33-37: Blocking call inside Uni-returning method may cause issues.
xirsysApiAdapter.createChannel(id)is a synchronous blocking call executed before returning the Uni. If this method is invoked on an I/O thread (e.g., Vert.x event loop), the blocking call could cause thread starvation or warnings. Consider wrapping the blocking call inUni.createFrom().item { ... }or using@Blockingannotation if intended.🔎 Proposed fix
override fun createSession(id: String, userId: Long, clientIp: String): Uni<Unit> { LOG.debug("Creating session id $id") - xirsysApiAdapter.createChannel(id) - return Uni.createFrom().nullItem() + return Uni.createFrom().item { + xirsysApiAdapter.createChannel(id) + } }
39-42: Same blocking call concern applies here.
xirsysApiAdapter.deleteChannel()is also a synchronous blocking call. Consider wrapping it consistently with the suggested pattern forcreateSession.🔎 Proposed fix
override fun deleteSession(id: String): Uni<Unit> { - xirsysApiAdapter.deleteChannel(channelName = id) - return Uni.createFrom().nullItem() + return Uni.createFrom().item { + xirsysApiAdapter.deleteChannel(channelName = id) + } }src/main/kotlin/com/faforever/icebreaker/service/hetzner/StubHetznerApiClient.kt (1)
10-33: Consider adding a method to reset stored rules for test isolation.The stub is well-implemented with thread-safe storage. For better test isolation between test cases, consider adding a
reset()method that clears both the call count and the stored rules:🔎 Optional enhancement for test isolation
fun resetCallCount() { callCount.set(0) } + + fun reset() { + callCount.set(0) + rulesByFirewallId.clear() + }src/test/kotlin/com/faforever/icebreaker/persistence/InMemoryFirewallWhitelistRepository.kt (1)
44-60: Consider more idiomatic replaceAll usage (optional).The
replaceAllcalls mutatedeletedAtin-place and return the same instance. While this works becausedeletedAtis avarfield, it's unconventional—replaceAlltypically expects a transformation that returns a (potentially new) instance. For test code clarity, you might consider creating a copy with the updated field, though the current implementation is functionally correct.Optional refactor example
override fun markSessionAsDeleted(sessionId: String) { allowedIps.replaceAll { if (it.sessionId == sessionId && it.deletedAt == null) { - it.deletedAt = clock.instant() + it.copy(deletedAt = clock.instant()) + } else { + it } - it } }Note: This requires
copyto support all fields, which may need adjustments to the entity definition.src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt (2)
122-122: Consider increasing the timeout or handling backpressure.The 10-second timeout may be insufficient if multiple sync requests queue up. Since the updater runs every 1 second and processes batches, a burst of requests could cause later ones to time out. Consider:
- Increasing the timeout to account for queue depth.
- Monitoring queue size and failing fast if backlog is too large.
- Documenting the expected latency under load.
156-158: Document the 3-second startup delay.The
@Scheduledannotation includesdelayed = "3s"to avoid spurious errors during integration tests. Consider adding a code comment explaining this delay, as it's non-obvious why syncing doesn't start immediately.Suggested comment
- // We delay by 3s to make it more likely that RabbitMQ is running by the time this method - // runs during integration tests. Otherwise, we get spurious errors logged. @Scheduled(every = "1s", delayed = "3s", concurrentExecution = Scheduled.ConcurrentExecution.SKIP) + // Delayed start gives RabbitMQ time to initialize during integration tests, avoiding spurious errors. fun syncFirewallWithHetzner() {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
README.md(1 hunks)build.gradle.kts(1 hunks)src/main/kotlin/com/faforever/icebreaker/persistence/FirewallWhitelistEntity.kt(1 hunks)src/main/kotlin/com/faforever/icebreaker/security/CurrentUserService.kt(1 hunks)src/main/kotlin/com/faforever/icebreaker/service/SessionHandler.kt(1 hunks)src/main/kotlin/com/faforever/icebreaker/service/SessionService.kt(11 hunks)src/main/kotlin/com/faforever/icebreaker/service/cloudflare/CloudflareSessionHandler.kt(2 hunks)src/main/kotlin/com/faforever/icebreaker/service/coturn/CoturnSessionHandler.kt(3 hunks)src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerApiClient.kt(1 hunks)src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt(1 hunks)src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerProperties.kt(1 hunks)src/main/kotlin/com/faforever/icebreaker/service/hetzner/StubHetznerApiClient.kt(1 hunks)src/main/kotlin/com/faforever/icebreaker/service/xirsys/XirsysApiAdapter.kt(3 hunks)src/main/kotlin/com/faforever/icebreaker/service/xirsys/XirsysSessionHandler.kt(2 hunks)src/main/kotlin/com/faforever/icebreaker/util/ClockProducer.kt(1 hunks)src/main/resources/application.yaml(6 hunks)src/main/resources/db/migration/V1.3.0__firewall_whitelist.sql(1 hunks)src/test/kotlin/com/faforever/icebreaker/persistence/FirewallWhitelistPanacheRepositoryTest.kt(1 hunks)src/test/kotlin/com/faforever/icebreaker/persistence/InMemoryFirewallWhitelistRepository.kt(1 hunks)src/test/kotlin/com/faforever/icebreaker/service/SessionServiceTest.kt(1 hunks)src/test/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallServiceTest.kt(1 hunks)src/test/kotlin/com/faforever/icebreaker/sync/TestHelpers.kt(1 hunks)src/test/kotlin/com/faforever/icebreaker/util/FakeClock.kt(1 hunks)src/test/kotlin/com/faforever/icebreaker/web/SessionControllerTest.kt(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-25T06:00:13.933Z
Learnt from: FjolleJagt
Repo: FAForever/faf-icebreaker PR: 140
File: src/main/resources/application.yaml:47-48
Timestamp: 2025-10-25T06:00:13.933Z
Learning: In the faf-icebreaker project, StubHetznerApiClient is selected as an alternative in dev and test profiles via `quarkus.arc.selected-alternatives`. This means the HETZNER_BASE_URL configuration is not required in those profiles, as the stub doesn't use the actual REST client.
Applied to files:
src/main/resources/application.yamlsrc/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerProperties.ktsrc/main/kotlin/com/faforever/icebreaker/service/hetzner/StubHetznerApiClient.kt
🧬 Code graph analysis (3)
src/main/kotlin/com/faforever/icebreaker/persistence/FirewallWhitelistEntity.kt (1)
src/test/kotlin/com/faforever/icebreaker/persistence/InMemoryFirewallWhitelistRepository.kt (1)
persist(15-27)
src/test/kotlin/com/faforever/icebreaker/persistence/InMemoryFirewallWhitelistRepository.kt (1)
src/main/kotlin/com/faforever/icebreaker/persistence/FirewallWhitelistEntity.kt (2)
getAllActive(36-36)getAllActive(72-73)
src/test/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallServiceTest.kt (1)
src/test/kotlin/com/faforever/icebreaker/sync/TestHelpers.kt (1)
waitUntil(12-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (20)
build.gradle.kts (2)
50-52: LGTM!The addition of Quarkus test security and Mockito dependencies appropriately supports testing the new IP whitelisting functionality, including security context handling, JWT-based session management, and mocking of external API calls to Hetzner.
49-49: AssertJ 3.27.6 is current and secure for test assertions.The AssertJ documentation recommends version 3.27.6, and the 3.27.x version line is supported. No security vulnerabilities are associated with this version. The dependency is appropriately scoped to test-only and uses explicit versioning.
src/main/kotlin/com/faforever/icebreaker/util/ClockProducer.kt (1)
7-12: LGTM!Clean CDI producer pattern for injectable
Clock. UsingClock.systemUTC()enables testability via theFakeClockmock, following best practices for time-dependent code.src/test/kotlin/com/faforever/icebreaker/sync/TestHelpers.kt (1)
12-25: LGTM!Well-structured async test helper with clear timeout semantics and descriptive failure message. The 100ms polling interval is a reasonable balance between test responsiveness and resource usage.
src/main/kotlin/com/faforever/icebreaker/security/CurrentUserService.kt (2)
26-34: Verify therealIpHeaderis set by a trusted proxy and stripped from untrusted sources.The IP extraction logic correctly handles comma-separated values (e.g.,
X-Forwarded-For). However, if this header can be set by clients (not stripped by the load balancer), attackers could spoof their IP to bypass firewall whitelisting or evade rate limits.Ensure the infrastructure (load balancer/reverse proxy) overwrites or sanitizes this header before it reaches the application.
17-20: LGTM!Clean extraction of user ID from JWT subject with appropriate error handling. Using
toLongOrNull()safely handles non-numeric subjects.src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerApiClient.kt (2)
52-90: LGTM!Well-structured response DTOs with
@JsonIgnoreProperties(ignoreUnknown = true)for API evolution resilience. Status enum properly covers Hetzner's action states.
92-104: LGTM!Standard MicroProfile REST client setup with proper Bearer token injection from configuration. The synchronous nature is acceptable since the PR description indicates rate limiting is handled by a background poller (1-second polling interval enforces the Hetzner API rate limit).
src/test/kotlin/com/faforever/icebreaker/web/SessionControllerTest.kt (1)
103-106: LGTM!The assertions correctly verify that the firewall whitelist entry is created with the expected session ID and client IP. The use of AssertJ provides clear and readable assertions.
src/main/kotlin/com/faforever/icebreaker/service/xirsys/XirsysApiAdapter.kt (1)
98-99: LGTM!The refactoring to use
CurrentUserServicefor IP resolution centralizes the logic and aligns with the broader PR changes. TherunCatchingblock at line 96 handles potential exceptions fromInetAddress.getByName().src/main/kotlin/com/faforever/icebreaker/service/xirsys/XirsysSessionHandler.kt (1)
44-48: LGTM!The no-op implementation with an explanatory comment is appropriate since Xirsys doesn't track per-peer state.
src/main/resources/application.yaml (2)
97-110: LGTM!The
single-active-consumer: trueconfiguration onhetzner-request-inqueue correctly ensures only one replica handles Hetzner API calls, addressing the rate-limiting constraint mentioned in the PR objectives.
175-179: LGTM!The stub client selection and helpful comments for enabling real Hetzner API calls in dev mode are well-structured. Based on learnings, the
StubHetznerApiClientcorrectly bypasses the need forHETZNER_BASE_URLin dev/test profiles.src/main/kotlin/com/faforever/icebreaker/service/cloudflare/CloudflareSessionHandler.kt (1)
33-46: LGTM!The no-op implementations are appropriate since Cloudflare uses global access without session-specific handling. The comments clearly explain the rationale, and the method signatures correctly align with the updated
SessionHandlerinterface.src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerProperties.kt (1)
1-22: Well-structured configuration interface.The interface is clean and well-documented. The use of
Optional<String>forfirewallIdallows graceful handling when Hetzner integration is disabled (dev/test profiles where StubHetznerApiClient is used). Based on learnings, the stub client is selected viaquarkus.arc.selected-alternativesin dev/test profiles, so the non-optionalapiKey()should only be required in production.src/main/resources/db/migration/V1.3.0__firewall_whitelist.sql (1)
1-17: Well-designed schema for IP whitelisting with soft deletes.The schema correctly handles:
- IPv6 addresses with
varchar(45)- Soft delete pattern with
deleted_at- The NULL uniqueness problem using a sentinel value in a generated column
- Appropriate indexing for expected query patterns
The unique constraint on
(session_id, user_id, deleted_at_or_sentinel)effectively prevents duplicate active entries while allowing historical records after soft deletion.src/main/kotlin/com/faforever/icebreaker/service/SessionService.kt (2)
31-48: Good use ofClockinjection for testability.Injecting
Clockinstead of usingInstant.now()directly enables deterministic time-based testing. This is a solid improvement for test reliability.
197-205: Good security validation on sender identity.The validation that
eventMessage.senderIdmatchescurrentUserIdwith aForbiddenExceptionprevents message spoofing attacks. The subsequent handling ofPeerClosingMessageto trigger session cleanup is logically correct.Note: Ensure the
deletePeerSessionUni subscription issue (flagged above) is addressed so the cleanup actually executes.src/main/kotlin/com/faforever/icebreaker/service/SessionHandler.kt (1)
8-19: Verify thatUnireturn values are properly composed or subscribed in non-endpoint contexts.The async
Uni<Unit>return types forcreateSession,deleteSession, anddeletePeerSessionare appropriate for I/O-bound operations. In Quarkus REST endpoints, Quarkus automatically handles subscription when Uni is returned. However, if these methods are called from service or business logic code (rather than REST endpoints), returning an unsubscribed Uni will not trigger execution—the operations must either be composed with other Uni operations or explicitly subscribed to using.subscribe(). Verify that callers either return the Uni through a REST endpoint or properly subscribe to it using.subscribe().with()to ensure side effects (like database writes) execute.src/main/kotlin/com/faforever/icebreaker/persistence/FirewallWhitelistEntity.kt (1)
49-66: The try-persist-catch-clear-query pattern correctly handles constraint violations for the "insert or get" operation. ThegetEntityManager().clear()call safely resets the Hibernate session to prevent flush errors when re-querying for the existing entry. This is the standard approach in Quarkus Panache for constraint-based deduplication, and no issues are present: eachpersistOrGet()call is isolated within its own transaction (class-level@Transactional), with no preceding persistent operations that could be lost.
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerApiClient.kt
Show resolved
Hide resolved
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt
Show resolved
Hide resolved
| override fun withZone(newZone: ZoneId): Clock { | ||
| zone = newZone | ||
| return this | ||
| } |
There was a problem hiding this comment.
withZone() violates Clock contract by mutating state.
Per java.time.Clock documentation, withZone() should return a copy with the new zone, not mutate the current instance. If production code calls withZone() expecting immutable semantics, tests may not catch issues.
🔎 Suggested fix
override fun withZone(newZone: ZoneId): Clock {
- zone = newZone
- return this
+ val copy = FakeClock()
+ copy.now = this.now
+ copy.zone = newZone
+ return copy
}🤖 Prompt for AI Agents
In src/test/kotlin/com/faforever/icebreaker/util/FakeClock.kt around lines 28 to
31, the override of withZone(newZone: ZoneId) mutates the existing FakeClock by
setting zone = newZone; instead, implement withZone to return a new FakeClock
instance with the same current instant supplier/state and the provided newZone
(preserving any other internal fields) so the method follows java.time.Clock's
immutability contract and does not change the receiver.
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt (2)
83-95: Consider validating IP address format before persistence.The IP address is stored directly in the database without validation. While
toCidr()will filter out invalid IPs during sync (line 202), storing invalid IPs wastes database space and could lead to confusion when debugging.🔎 Add validation before persistence
fun whitelistIpForSession(sessionId: String, userId: Long, ipAddress: String): Uni<Unit> { LOG.debug("Whitelisting IP {} for session {} in Hetzner cloud firewall", ipAddress, sessionId) + if (ipAddress.toCidr() == null) { + LOG.warn("Invalid IP address format: {}", ipAddress) + return Uni.createFrom().failure(IllegalArgumentException("Invalid IP address format: $ipAddress")) + } repository.persistOrGet( FirewallWhitelistEntity(
228-235: Optional: More idiomatic Kotlin for queue draining.The current implementation is correct and clear, but could be made more concise using
generateSequence.🔎 Idiomatic Kotlin alternative
private fun <T> takeAll(queue: Queue<T>): List<T> { - val result = mutableListOf<T>() - while (true) { - val x = queue.poll() ?: break - result.add(x) - } - return result + return generateSequence { queue.poll() }.toList() }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-20T19:34:30.331Z
Learnt from: Brutus5000
Repo: FAForever/faf-icebreaker PR: 147
File: src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt:117-123
Timestamp: 2025-12-20T19:34:30.331Z
Learning: In Kotlin, prefer bracket syntax map[key] = value over map.set(key, value). Both forms delegate to put(), but bracket syntax is more idiomatic Kotlin. Apply this guidance to code using Map-like types (e.g., ConcurrentHashMap) across Kotlin files to ensure idiomatic usage.
Applied to files:
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (3)
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt (3)
40-53: LGTM: IP to CIDR conversion is well-implemented.The extension function correctly handles IPv4 and IPv6 addresses with appropriate CIDR notation, includes proper error handling, and provides logging visibility for failures.
200-225: LGTM: Firewall rule builder correctly addresses Hetzner constraints.The implementation properly:
- Filters invalid IPs with
mapNotNulland defensivetrim()- Removes duplicates to optimize rule count
- Chunks IPs to respect Hetzner's per-rule limit (addressing PR objective #132)
- Creates both TCP and UDP ingress rules without port restrictions
172-198: Design trade-offs already documented in code comments.The implementation includes two acknowledged design decisions:
Timing race (lines 172-182): The comment already explains that futures may not complete until the next sync cycle, even when the firewall was updated. This is a documented trade-off.
Coarse-grained error handling (line 183): If any action in the batch fails, all requests are marked as failed. The code explicitly documents this approach.
Both are intentional design choices with documented rationale rather than unaddressed issues. Consider adding operational monitoring to track how often these edge cases occur in production.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt (1)
131-137: Consider logging unexpected response IDs.When a response arrives for an unknown ID, it's silently ignored (line 135). While this doesn't break functionality, logging a warning would help identify bugs or race conditions during debugging.
🔎 Add logging for unexpected responses
@Incoming("hetzner-response-in") fun handle(json: JsonObject) { val response = json.mapTo(SyncMessage::class.java) - // The message is a response to a previous request; we - // complete the future that that request is waiting for. - awaitedMessagesById.remove(response.id)?.complete(Unit) + val future = awaitedMessagesById.remove(response.id) + if (future != null) { + future.complete(Unit) + } else { + LOG.warn("Received response for unknown request ID: {}", response.id) + } // The response is acked when this function returns }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-20T19:34:30.331Z
Learnt from: Brutus5000
Repo: FAForever/faf-icebreaker PR: 147
File: src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt:117-123
Timestamp: 2025-12-20T19:34:30.331Z
Learning: In Kotlin, prefer bracket syntax map[key] = value over map.set(key, value). Both forms delegate to put(), but bracket syntax is more idiomatic Kotlin. Apply this guidance to code using Map-like types (e.g., ConcurrentHashMap) across Kotlin files to ensure idiomatic usage.
Applied to files:
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (4)
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt (4)
40-53: LGTM! Proper error handling for IP parsing.The extension function correctly converts IPv4 and IPv6 addresses to CIDR notation, returns null on parse failures, and logs warnings with context. The null return value integrates well with
mapNotNullusage at line 207.
56-59: LGTM! Clear message structure.The data class appropriately models the request/response pairing mechanism with good documentation.
140-231: LGTM! Well-structured batch processing with proper error handling.The
HetznerFirewallUpdatercorrectly implements:
- RabbitMQ message acking tied to actual processing completion (lines 153-159)
- Batch processing with all code paths completing ack futures (lines 173-202)
- Defensive checks for missing configuration (line 165) and empty batches (line 168)
- Proper IP normalization with
trim()andtoCidr(), filtering invalid IPs withmapNotNull(line 207)- Duplicate removal and chunking to respect Hetzner's limits (lines 208-209)
- TCP and UDP rule generation per the requirements (lines 211-226)
233-240: LGTM! Standard queue draining implementation.The helper function correctly drains the queue using
poll()until empty.
| private fun syncFirewall(): Uni<Unit> { | ||
| val requestId = UUID.randomUUID().toString() | ||
| val future = CompletableFuture<Unit>() | ||
| awaitedMessagesById[requestId] = future | ||
| requestEmitter.send(SyncMessage(requestId)) | ||
| return Uni.createFrom().completionStage(future) | ||
| .ifNoItem().after(Duration.ofSeconds(10)).fail() | ||
| .onFailure().invoke { e -> | ||
| LOG.error("Failed to sync firewall rule for request id {}", requestId, e) | ||
| awaitedMessagesById.remove(requestId) | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: Memory leak when requestEmitter.send() throws.
The timeout leak from the previous review has been addressed by the onFailure() handler. However, a similar leak remains if requestEmitter.send() throws at line 121:
- Line 120 adds the future to
awaitedMessagesById - Line 121's
send()can throw (e.g., backpressure, emitter not ready) - The exception propagates before the Uni is created at line 122
- The
onFailure()handler is never attached - The future remains in the map indefinitely
🔎 Wrap send() in try-catch to ensure cleanup
private fun syncFirewall(): Uni<Unit> {
val requestId = UUID.randomUUID().toString()
val future = CompletableFuture<Unit>()
awaitedMessagesById[requestId] = future
- requestEmitter.send(SyncMessage(requestId))
+ try {
+ requestEmitter.send(SyncMessage(requestId))
+ } catch (e: Exception) {
+ awaitedMessagesById.remove(requestId)
+ throw e
+ }
return Uni.createFrom().completionStage(future)
.ifNoItem().after(Duration.ofSeconds(10)).fail()
.onFailure().invoke { e ->
LOG.error("Failed to sync firewall rule for request id {}", requestId, e)
awaitedMessagesById.remove(requestId)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private fun syncFirewall(): Uni<Unit> { | |
| val requestId = UUID.randomUUID().toString() | |
| val future = CompletableFuture<Unit>() | |
| awaitedMessagesById[requestId] = future | |
| requestEmitter.send(SyncMessage(requestId)) | |
| return Uni.createFrom().completionStage(future) | |
| .ifNoItem().after(Duration.ofSeconds(10)).fail() | |
| .onFailure().invoke { e -> | |
| LOG.error("Failed to sync firewall rule for request id {}", requestId, e) | |
| awaitedMessagesById.remove(requestId) | |
| } | |
| } | |
| private fun syncFirewall(): Uni<Unit> { | |
| val requestId = UUID.randomUUID().toString() | |
| val future = CompletableFuture<Unit>() | |
| awaitedMessagesById[requestId] = future | |
| try { | |
| requestEmitter.send(SyncMessage(requestId)) | |
| } catch (e: Exception) { | |
| awaitedMessagesById.remove(requestId) | |
| throw e | |
| } | |
| return Uni.createFrom().completionStage(future) | |
| .ifNoItem().after(Duration.ofSeconds(10)).fail() | |
| .onFailure().invoke { e -> | |
| LOG.error("Failed to sync firewall rule for request id {}", requestId, e) | |
| awaitedMessagesById.remove(requestId) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt
around lines 117 to 128, wrap the requestEmitter.send(SyncMessage(requestId))
call in a try-catch so any synchronous exception does not leave the
CompletableFuture stored in awaitedMessagesById; on catch remove
awaitedMessagesById[requestId], completeExceptionally the future (or cancel it)
with the caught exception, then rethrow the exception so normal upstream
handling occurs; keep the existing Uni creation and onFailure handler for async
failures unchanged.
There was a problem hiding this comment.
@coderabbitai Maybe theres a requestEmitter.send() method that already returns an asynchronous result, that we can use instead and then continue with async call chain?
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
SmallRye Reactive Messaging Emitter API methods async
💡 Result:
Short summary of the async emitter methods in SmallRye Reactive Messaging:
-
org.eclipse.microprofile.reactive.messaging.Emitter
- CompletionStage send(T payload) — async send that completes when the message is acknowledged. [1]
- void send(Message<? extends T> msg) — send a Message (sync void overload). [1]
- void complete(), void error(Exception), boolean hasRequests(), boolean isCancelled(). [1]
-
io.smallrye.reactive.messaging.MutinyEmitter (Mutiny API)
- Uni send(T payload) — returns a Uni that, when subscribed, performs the send and completes when acknowledged. [2]
- void sendAndAwait(T payload) — blocks until acknowledged. [2]
- Cancellable sendAndForget(T payload) — fire-and-forget (no acknowledgement wait). [2]
- void send(Message<? extends T> msg), void complete(), void error(Exception), boolean hasRequests(), boolean isCancelled(). [2]
(Concept docs explaining send/sendAndAwait/sendAndForget and overflow/hasRequests behavior.) [3]
Sources:
[1] SmallRye / MicroProfile Emitter API docs (Emitter).
[2] SmallRye MutinyEmitter API docs.
[3] SmallRye Reactive Messaging emitter concept guide.
@Brutus5000, excellent idea! Yes, there is an async variant. The Emitter.send(T payload) method returns CompletionStage<Void>, and SmallRye also provides MutinyEmitter<T> with a send(T payload) method that returns Uni<Void>.
Since your codebase already uses Mutiny extensively, you can switch to MutinyEmitter and chain the operations:
@Singleton
class HetznerFirewallService(
private val repository: FirewallWhitelistRepository,
private val clock: Clock,
@param:Channel("hetzner-request-out") private val requestEmitter: MutinyEmitter<SyncMessage>,
) {
// ... rest of the class ...
private fun syncFirewall(): Uni<Unit> {
val requestId = UUID.randomUUID().toString()
val future = CompletableFuture<Unit>()
awaitedMessagesById[requestId] = future
return requestEmitter.send(SyncMessage(requestId))
.onFailure().invoke { e ->
LOG.error("Failed to send sync request {}", requestId, e)
awaitedMessagesById.remove(requestId)
}
.replaceWith(Uni.createFrom().completionStage(future))
.ifNoItem().after(Duration.ofSeconds(10)).fail()
.onFailure().invoke { e ->
LOG.error("Failed to sync firewall rule for request id {}", requestId, e)
awaitedMessagesById.remove(requestId)
}
}
}You'll also need to add the import:
import io.smallrye.reactive.messaging.MutinyEmitterThis approach:
- Uses the async
send()that returnsUni<Void> - Handles send failures with the first
onFailure()handler - Chains to the acknowledgment future with
replaceWith() - Maintains the existing timeout and acknowledgment failure handling
This is cleaner and avoids the try-catch pattern entirely! 🎯
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt (1)
117-128: Critical: Memory leak ifrequestEmitter.send()throws.This issue was identified in the previous review but has not yet been addressed. If
requestEmitter.send()throws a synchronous exception at line 121, theCompletableFuturestored inawaitedMessagesByIdat line 120 will never be removed, causing a memory leak.The previous review recommended switching from
Emitter<SyncMessage>toMutinyEmitter<SyncMessage>to use the asyncsend()method that returnsUni<Void>, allowing proper error handling in the reactive chain.🔎 Recommended fix (from previous review)
Change line 74:
- @param:Channel("hetzner-request-out") private val requestEmitter: Emitter<SyncMessage>, + @param:Channel("hetzner-request-out") private val requestEmitter: MutinyEmitter<SyncMessage>,Add import:
import io.smallrye.reactive.messaging.MutinyEmitterUpdate
syncFirewall()method:private fun syncFirewall(): Uni<Unit> { val requestId = UUID.randomUUID().toString() val future = CompletableFuture<Unit>() awaitedMessagesById[requestId] = future - requestEmitter.send(SyncMessage(requestId)) - return Uni.createFrom().completionStage(future) + return requestEmitter.send(SyncMessage(requestId)) + .onFailure().invoke { e -> + LOG.error("Failed to send sync request {}", requestId, e) + awaitedMessagesById.remove(requestId) + } + .replaceWith(Uni.createFrom().completionStage(future)) .ifNoItem().after(Duration.ofSeconds(10)).fail() .onFailure().invoke { e -> LOG.error("Failed to sync firewall rule for request id {}", requestId, e) awaitedMessagesById.remove(requestId) } }Based on learnings, the bracket syntax at line 120 is idiomatic Kotlin and should be retained.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-20T19:34:30.331Z
Learnt from: Brutus5000
Repo: FAForever/faf-icebreaker PR: 147
File: src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt:117-123
Timestamp: 2025-12-20T19:34:30.331Z
Learning: In Kotlin, prefer bracket syntax map[key] = value over map.set(key, value). Both forms delegate to put(), but bracket syntax is more idiomatic Kotlin. Apply this guidance to code using Map-like types (e.g., ConcurrentHashMap) across Kotlin files to ensure idiomatic usage.
Applied to files:
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (5)
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt (5)
40-53: LGTM: Clean error handling for IP-to-CIDR conversion.The extension function correctly handles IPv4/IPv6 conversion with proper error handling and logging for unparseable addresses.
55-59: LGTM: Simple and appropriate message type.
130-137: LGTM: Safe response handling.The safe-call operator correctly handles cases where the response ID is not found (e.g., already timed out).
215-240: LGTM: Correct firewall rule construction.The logic correctly:
- Retrieves active IPs and converts them to CIDR notation
- Filters out invalid IPs and removes duplicates
- Chunks IPs according to
maxIpsPerRuleto respect Hetzner's limits- Creates both TCP and UDP rules for each source block
The comment explaining why ports are not specified is helpful.
243-250: LGTM: Simple and correct queue-draining utility.
| fun whitelistIpForSession(sessionId: String, userId: Long, ipAddress: String): Uni<Unit> { | ||
| LOG.debug("Whitelisting IP {} for session {} in Hetzner cloud firewall", ipAddress, sessionId) | ||
| repository.persistOrGet( | ||
| FirewallWhitelistEntity( | ||
| userId = userId, | ||
| sessionId = sessionId, | ||
| allowedIp = ipAddress, | ||
| createdAt = clock.instant(), | ||
| deletedAt = null, | ||
| ), | ||
| ) | ||
| return syncFirewall() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the FirewallWhitelistRepository file
fd -type f -name "*FirewallWhitelistRepository*"
# Search for the repository interface/class and persistOrGet method
rg -n "FirewallWhitelistRepository" --type kotlin
# Search for persistOrGet method definition
rg -n -A 3 "fun persistOrGet" --type kotlinRepository: FAForever/faf-icebreaker
Length of output: 3877
🏁 Script executed:
#!/bin/bash
# View the full HetznerFirewallService.kt file to understand the context
cat -n src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt | head -150
# Also check the repository implementation details
cat -n src/main/kotlin/com/faforever/icebreaker/persistence/FirewallWhitelistEntity.ktRepository: FAForever/faf-icebreaker
Length of output: 11040
Block the event loop with a synchronous database operation in a reactive method.
The repository.persistOrget() call performs a blocking database operation before returning a reactive Uni<Unit>. If this method is called from a Quarkus request handler running on the event loop, the blocking operation will block the event loop thread and degrade performance.
Apply one of these fixes:
- Add
@Blockingannotation towhitelistIpForSession(),removeWhitelistsForSession(), andremoveWhitelistForSessionUser()to offload these methods to a blocking executor pool. - Wrap the blocking calls in
Uni.createFrom().item { }or useUni.createFrom().voidItem()for side-effect-only calls.
| if (firewall == null) { | ||
| batch.forEach { | ||
| responseEmitter.send(it.payload) | ||
| it.ack.complete(Unit) | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
Critical: Incomplete acks if responseEmitter.send() throws.
When no firewall is configured, the code sends responses and completes acks synchronously. If responseEmitter.send(it.payload) throws at line 170, the exception will propagate, terminating the forEach loop. This leaves all remaining acks in the batch incomplete indefinitely.
🔎 Wrap the loop in try-catch to ensure all acks complete
if (firewall == null) {
- batch.forEach {
- responseEmitter.send(it.payload)
- it.ack.complete(Unit)
- }
+ try {
+ batch.forEach {
+ responseEmitter.send(it.payload)
+ it.ack.complete(Unit)
+ }
+ } catch (e: Exception) {
+ LOG.error("Failed to send response messages when no firewall configured", e)
+ batch.forEach { it.ack.completeExceptionally(e) }
+ }
return
}Alternatively, consider using MutinyEmitter for responseEmitter (line 145) to handle send() asynchronously, similar to the fix recommended for requestEmitter.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (firewall == null) { | |
| batch.forEach { | |
| responseEmitter.send(it.payload) | |
| it.ack.complete(Unit) | |
| } | |
| return | |
| } | |
| if (firewall == null) { | |
| try { | |
| batch.forEach { | |
| responseEmitter.send(it.payload) | |
| it.ack.complete(Unit) | |
| } | |
| } catch (e: Exception) { | |
| LOG.error("Failed to send response messages when no firewall configured", e) | |
| batch.forEach { it.ack.completeExceptionally(e) } | |
| } | |
| return | |
| } |
🤖 Prompt for AI Agents
In
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt
around lines 168-174, the synchronous batch.forEach that calls
responseEmitter.send(it.payload) can throw and abort the loop leaving remaining
it.ack incomplete; wrap each send in a try-catch (or use a per-item try {
responseEmitter.send(...) } catch(e) { log.error(...) } finally {
it.ack.complete(Unit) }) so that every ack is completed regardless of send
failures, or switch responseEmitter to an asynchronous MutinyEmitter and handle
failures per-item to ensure ack completion.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt (4)
83-95: Block the event loop with synchronous database operations.The
repository.persistOrGet()call at line 85 is a blocking database operation executed before returning the reactiveUni<Unit>. If this method is invoked from a Quarkus request handler on the event loop, it will block the event loop thread and degrade performance.Consider adding the
@Blockingannotation to this method (andremoveWhitelistsForSession,removeWhitelistForSessionUser) or wrapping the blocking repository calls inUni.createFrom().item { }.
117-128: Memory leak whenrequestEmitter.send()throws.If
requestEmitter.send()at line 121 throws synchronously (e.g., due to backpressure or emitter unavailability):
- Line 120 has already added the future to
awaitedMessagesById- The exception propagates before the Uni is created at line 122
- The
onFailure()handler at line 124 is never attached- The future remains in the map indefinitely
The previous review suggested switching to
MutinyEmitter<SyncMessage>(line 74) and using its asyncsend()method that returnsUni<Void>, which would eliminate this issue entirely.
168-174: Incomplete acks ifresponseEmitter.send()throws.When no firewall is configured, this code sends responses and completes acks synchronously. If
responseEmitter.send(it.payload)at line 170 throws, the exception propagates and terminates theforEachloop, leaving all remaining acks incomplete indefinitely.Consider wrapping each iteration in try-catch to ensure all acks complete, or switch
responseEmittertoMutinyEmitterfor async handling (as suggested in the previous review forrequestEmitter).
199-202: Incomplete acks ifresponseEmitter.send()throws.Similar to the issue at lines 168-174, if
responseEmitter.send(it.payload)at line 200 throws, theforEachloop terminates early and remaining acks in the batch are never completed.Ensure all acks complete even if send() fails, either by per-item try-catch or by switching to
MutinyEmitter.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-20T19:34:30.331Z
Learnt from: Brutus5000
Repo: FAForever/faf-icebreaker PR: 147
File: src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt:117-123
Timestamp: 2025-12-20T19:34:30.331Z
Learning: In Kotlin, prefer bracket syntax map[key] = value over map.set(key, value). Both forms delegate to put(), but bracket syntax is more idiomatic Kotlin. Apply this guidance to code using Map-like types (e.g., ConcurrentHashMap) across Kotlin files to ensure idiomatic usage.
Applied to files:
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (3)
src/main/kotlin/com/faforever/icebreaker/service/hetzner/HetznerFirewallService.kt (3)
40-53: LGTM!The
toCidr()extension function correctly handles IPv4 and IPv6 addresses, includes appropriate error handling with logging, and returns null for unparseable inputs (which are safely filtered out at line 215).
56-59: LGTM!The
SyncMessagedata class is appropriately simple and well-documented for its use in request/response pairing.
241-248: LGTM!The
takeAllhelper correctly drains the queue into a list. The implementation is straightforward and correct.
| private fun buildSetFirewallRequest(): SetFirewallRulesRequest { | ||
| val sourceIps = repository.getAllActive().mapNotNull { entry -> | ||
| entry.allowedIp.trim().toCidr() | ||
| }.distinct() | ||
| val sourceBlocks: List<List<String>> = sourceIps.chunked(hetznerProperties.maxIpsPerRule()) | ||
| val rules = | ||
| sourceBlocks.flatMap { sources -> | ||
| listOf( | ||
| // We don't specify the ports for either rule, because the port might | ||
| // be different on each TURN server. | ||
| FirewallRule( | ||
| direction = Direction.IN, | ||
| sourceIps = sources, | ||
| protocol = Protocol.TCP, | ||
| ), | ||
| FirewallRule( | ||
| direction = Direction.IN, | ||
| sourceIps = sources, | ||
| protocol = Protocol.UDP, | ||
| ), | ||
| ) | ||
| } | ||
| val request = SetFirewallRulesRequest(rules) | ||
| LOG.debug("Hetzner request summary: rules={}, totalSourceIps={}", rules.size, sourceIps.size) | ||
| return request | ||
| } |
There was a problem hiding this comment.
Validate rule count against Hetzner's firewall limit.
The method creates 2 rules (TCP + UDP) per IP chunk. With sourceIps.chunked(hetznerProperties.maxIpsPerRule()) at line 217, the total rule count is 2 * (sourceIps.size / maxIpsPerRule). The PR description notes Hetzner's limit of ≈50 rules, but there is no validation here.
For example, with 1000 active IPs and maxIpsPerRule = 10, this would generate 200 rules, far exceeding the limit. The Hetzner API call at line 184 would fail, causing all pending requests in the batch to fail at line 205.
Consider adding:
- A runtime check that
sourceBlocks.size * 2 <= HETZNER_RULE_LIMIT(e.g., 50) - Graceful degradation (e.g., log a warning and apply only the first N blocks, or prioritize recent IPs)
- Operational monitoring/alerting when approaching the limit
Fixes #132. When a WebRTC session is created, a whitelist entry for that IP is created in the database. When the session is cleaned up due to age or a peer sends a PeerClosing message, the entries for that session or that peer are marked as deleted. Every second, a thread checks whether any modifications have been made to the database since the last API update, and if there are any updates, makes an API request to Hetzner to update the cloud firewalls with the current whitelist. (This polling every second enforces a ratelimit on the upstream calls. We use RabbitMQ's single-active-consumer mode to ensure that, if there are multiple replicas of the server, only one of them makes calls to Hetzner's APIs.)
Testing
Without Hetzner
./gradlew quarkusDevcurl -H "Authorization: Bearer $FAF_TOKEN" localhost:8080/session/game/100 -fbut no log line
We insert into the firewall_whitelist table but otherwise hetzner is disabled.
With Hetzner
firewall_whitelisttable.HETZNER_API_KEY=$(cat /tmp/hetzner-key) HETZNER_BASE_URL="https://api.hetzner.cloud/v1" HETZNER_FIREWALL_ID=$(cat /tmp/hetzner-fw) ./gradlew quarkusDevcurl -H "Authorization: Bearer $FAF_TOKEN" localhost:8080/session/game/100 -fand no repeated polling of the database for new entries.
curl -H "Authorization: Bearer $(cat /tmp/hetzner-key)" "https://api.hetzner.cloud/v1/firewalls/$(cat /tmp/hetzner-fw)"mysql -u faf-icebreaker -p -h 127.0.0.1 -P 3306 faf-icebreaker -e 'select * from firewall_whitelist'Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.