restore core gRPC server tests & add testing framework for gateway handlers#2381
Draft
restore core gRPC server tests & add testing framework for gateway handlers#2381
Conversation
# Conflicts: # crates/defguard_core/tests/integration/grpc/gateway.rs
There was a problem hiding this comment.
Pull request overview
Restores and modernizes gRPC-related test coverage by (1) re-enabling defguard_core integration gRPC tests aligned with the current “reversed communication” approach, and (2) introducing a dedicated gateway-manager handler test harness that exercises handler behavior without full TLS/cert setup.
Changes:
- Add a Unix-socket based mock gateway +
HandlerTestContextharness and comprehensive gateway handler behavior tests indefguard_gateway_manager. - Re-enable
defguard_coreintegration gRPC test module, replacing obsolete gateway tests with Worker service + health checks using a lightweight in-process test server. - Refactor gateway-manager handler connection code to support a test transport (Unix socket) and expose a test-only handler entrypoint.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/defguard_gateway_manager/tests/mod.rs | Adds integration test module roots for gateway-manager tests. |
| crates/defguard_gateway_manager/tests/gateway_manager/mod.rs | Wires gateway-manager handler test module. |
| crates/defguard_gateway_manager/tests/gateway_manager/handler.rs | Adds extensive gateway handler behavioral tests (config handshake, updates routing, disconnect behavior, peer-stats filtering). |
| crates/defguard_gateway_manager/tests/common/mod.rs | Introduces reusable mock gateway server + handler test context (DB setup, stream control, assertions). |
| crates/defguard_gateway_manager/src/tests.rs | Removes old in-crate test scaffolding (superseded by new integration harness). |
| crates/defguard_gateway_manager/src/lib.rs | Re-exports TestGatewayHandler for integration tests. |
| crates/defguard_gateway_manager/src/handler.rs | Adds test transport (Unix socket) and TestGatewayHandler; refactors connection loop to support single-iteration handling for tests. |
| crates/defguard_gateway_manager/Cargo.toml | Moves hyper-util to regular dependencies (now used by non-test code path). |
| crates/defguard_core/tests/integration/main.rs | Re-enables the grpc integration test module. |
| crates/defguard_core/tests/integration/grpc/mod.rs | Updates gRPC integration test module list (health + worker). |
| crates/defguard_core/tests/integration/grpc/health.rs | Adds health check test validating Worker service is marked Serving. |
| crates/defguard_core/tests/integration/grpc/worker.rs | Adds Worker service integration tests (registration, job lifecycle, interceptor auth). |
| crates/defguard_core/tests/integration/grpc/gateway.rs | Removes outdated gateway gRPC integration tests. |
| crates/defguard_core/tests/integration/grpc/common/mod.rs | Reworks gRPC test server utilities (in-process server/router, JWT helpers). |
| crates/defguard_core/tests/integration/grpc/common/mock_gateway.rs | Removes legacy mock gateway implementation tied to old gateway gRPC API. |
| crates/defguard_core/src/grpc/mod.rs | Makes router builder public and marks Worker service as Serving in health reporter. |
| crates/defguard_core/Cargo.toml | Adds hyper-util for integration-test utilities. |
| Cargo.lock | Updates lockfile for new dependency usage. |
Comments suppressed due to low confidence (1)
crates/defguard_core/src/grpc/mod.rs:103
- Making
build_grpc_service_routerfullypubexpands the public API ofdefguard_coreprimarily to support integration tests. Ifdefguard_coreis intended to keep a smaller public surface, consider gating this behind atest-utilsfeature (enabled for tests) or moving the helper into a dedicated test support module/crate to avoid exposing internal server wiring as a stable API.
pub async fn build_grpc_service_router(
server: Server,
pool: PgPool,
worker_state: Arc<Mutex<WorkerState>>,
failed_logins: Arc<Mutex<FailedLoginMap>>,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+171
to
+176
| fn initialize_jwt_secrets() { | ||
| JWT_SECRETS.call_once(|| unsafe { | ||
| env::set_var(AUTH_SECRET_ENV, "defguard-test-auth-secret"); | ||
| env::set_var(GATEWAY_SECRET_ENV, "defguard-test-gateway-secret"); | ||
| env::set_var(YUBIBRIDGE_SECRET_ENV, "defguard-test-yubibridge-secret"); | ||
| }); |
Comment on lines
+308
to
+312
| clients | ||
| .lock() | ||
| .expect("GatewayHandler failed to lock clients") | ||
| .insert(self.gateway.id, client.clone()); | ||
| let (tx, rx) = mpsc::unbounded_channel(); |
| match resp_stream.message().await { | ||
| Ok(None) => { | ||
| info!("Stream was closed by the sender."); | ||
| self.mark_disconnected().await; |
wojcik91
force-pushed
the
restore_grpc_tests
branch
from
5e55c6e to
78075b2
Compare
# Conflicts: # crates/defguard_core/tests/integration/grpc/gateway.rs # crates/defguard_gateway_manager/src/tests.rs
There was a problem hiding this comment.
Pull request overview
This PR restores and modernizes the gRPC/integration test setup around the “reversed communication” gateway flow by adding a dedicated test harness for defguard_gateway_manager and re-enabling defguard_core gRPC integration tests (focused on Worker + Health).
Changes:
- Adds a Unix-socket-based mock gateway server + shared test contexts, and comprehensive integration tests for gateway handler/manager behavior (handshake, lifecycle, retries, event routing, MFA, firewall, peer-stats filtering).
- Extends
defguard_gateway_managerwith test-support hooks (test socket transport, retry-delay overrides, counters/notifications) and updates connection/disconnection handling. - Re-enables
defguard_coregRPC integration tests (Worker service + Health) and adjusts gRPC router health reporting accordingly.
Reviewed changes
Copilot reviewed 28 out of 29 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/defguard_gateway_manager/tests/mod.rs | Adds top-level integration test modules. |
| crates/defguard_gateway_manager/tests/gateway_manager/mod.rs | Splits tests into handler vs manager suites. |
| crates/defguard_gateway_manager/tests/gateway_manager/manager.rs | Manager integration tests (startup, notifications, retries, reconnect behavior). |
| crates/defguard_gateway_manager/tests/gateway_manager/handler.rs | Test driver that includes handler test cases. |
| crates/defguard_gateway_manager/tests/gateway_manager/handler/support.rs | Shared helpers/assertions for handler tests (peer/network/firewall update assertions, device creation helpers). |
| crates/defguard_gateway_manager/tests/gateway_manager/handler/handshake.rs | Tests config request handshake behavior. |
| crates/defguard_gateway_manager/tests/gateway_manager/handler/lifecycle.rs | Tests connected/disconnected DB state transitions. |
| crates/defguard_gateway_manager/tests/gateway_manager/handler/stats.rs | Tests peer stats forwarding and endpoint validation behavior. |
| crates/defguard_gateway_manager/tests/gateway_manager/handler/network_events.rs | Tests network event routing and update payloads. |
| crates/defguard_gateway_manager/tests/gateway_manager/handler/firewall_events.rs | Tests firewall event routing and payloads. |
| crates/defguard_gateway_manager/tests/gateway_manager/handler/device_events.rs | Tests device create/modify/delete event routing and payloads. |
| crates/defguard_gateway_manager/tests/gateway_manager/handler/mfa.rs | Tests MFA session events -> peer updates. |
| crates/defguard_gateway_manager/tests/common/mod.rs | Provides mock gateway gRPC service, test contexts, socket-path registry, helper DB builders. |
| crates/defguard_gateway_manager/src/tests.rs | Removes old in-crate test scaffolding. |
| crates/defguard_gateway_manager/src/lib.rs | Adds test-support plumbing to GatewayManager (socket registry, counters, listener-ready notify, retry overrides). |
| crates/defguard_gateway_manager/src/handler.rs | Adds test socket transport option, reconnection logic refactor, exposes internal test handler wrapper, adds unit tests for config/stats conversion. |
| crates/defguard_gateway_manager/src/certs.rs | Adds unit/integration tests for certificate collection + refresh publishing. |
| crates/defguard_gateway_manager/Cargo.toml | Moves hyper-util to normal dependencies (used outside dev/test-only cfg paths now). |
| crates/defguard_core/tests/integration/main.rs | Re-enables grpc integration test module. |
| crates/defguard_core/tests/integration/grpc/mod.rs | Switches gRPC integration tests to health + worker modules. |
| crates/defguard_core/tests/integration/grpc/health.rs | Adds health check integration test. |
| crates/defguard_core/tests/integration/grpc/worker.rs | Adds worker service integration tests (auth, job flow, side effects). |
| crates/defguard_core/tests/integration/grpc/common/mod.rs | Reworks gRPC test server harness; adds JWT secret initialization + helpers for authorization metadata. |
| crates/defguard_core/tests/integration/grpc/gateway.rs | Removes legacy gateway integration tests (no longer aligned with new gateway comms model). |
| crates/defguard_core/tests/integration/grpc/common/mock_gateway.rs | Removes legacy mock gateway client (no longer used). |
| crates/defguard_core/src/grpc/mod.rs | Makes router builder public for tests; adds Worker service health “serving” reporting. |
| crates/defguard_core/Cargo.toml | Adds hyper-util dev-dependency for mock transport. |
| crates/defguard_common/src/db/models/gateway.rs | Moves Gateway::url() to generic Gateway<I> impl (usable before insert/save). |
| Cargo.lock | Adds hyper-util to workspace lockfile. |
Comments suppressed due to low confidence (1)
crates/defguard_core/src/grpc/mod.rs:106
build_grpc_service_routerwas made fullypub, which expands defguard_core's public API surface with a primarily test-oriented constructor. To avoid locking this into a supported API, consider moving this into a#[doc(hidden)] pub mod test_support(or gating it behind a feature liketest-support) and keeping the main constructorpub(crate)/private.
pub async fn build_grpc_service_router(
server: Server,
pool: PgPool,
worker_state: Arc<Mutex<WorkerState>>,
failed_logins: Arc<Mutex<FailedLoginMap>>,
// incompatible_components: Arc<RwLock<IncompatibleComponents>>,
) -> Result<Router, anyhow::Error> {
let auth_service = AuthServiceServer::new(AuthServer::new(pool.clone(), failed_logins));
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
317
to
336
| clients | ||
| .lock() | ||
| .expect("GatewayHandler failed to lock clients") | ||
| .insert(self.gateway.id, client.clone()); | ||
| let (tx, rx) = mpsc::unbounded_channel(); | ||
| let retry_delay = self | ||
| .test_support | ||
| .as_ref() | ||
| .map_or(TEN_SECS, GatewayManagerTestSupport::handler_reconnect_delay); | ||
| let response = match client.bidi(UnboundedReceiverStream::new(rx)).await { | ||
| Ok(response) => response, | ||
| Err(err) => { | ||
| error!("Failed to connect to Gateway {uri}, retrying: {err}"); | ||
| if retry_on_connect_failure { | ||
| sleep(retry_delay).await; | ||
| return Ok(()); | ||
| } | ||
|
|
||
| if let Some(mut gateway) = Gateway::find_by_id(&self.pool, self.gateway.id).await? { | ||
| gateway.version = Some(version.to_string()); | ||
| gateway.save(&self.pool).await?; | ||
| return Err(err.into()); | ||
| } |
| info!("Sent purge request to Gateway {old}"); | ||
| self.remove_client(old.id); | ||
| if let Some(abort_handle) = abort_handles.remove(&old.id) { | ||
| old.touch_disconnected(&self.pool).await?; |
| // Kill the `GatewayHandler` and the connection. | ||
| if let Some(abort_handle) = abort_handles.remove(&old.id) { | ||
| info!( | ||
| "Aborting connection to Gateway {old}, it has disappeard from the \ |
There was a problem hiding this comment.
Pull request overview
This PR re-enables and expands automated testing around DefGuard’s gRPC-based gateway/worker interactions by introducing a dedicated gateway-manager integration test harness (mock gateway over UDS) and restoring defguard_core gRPC integration coverage (health + worker), aligned with the newer “reversed communication” approach referenced in #1854.
Changes:
- Add a UDS-based mock gateway + test contexts to integration-test
defguard_gateway_managerhandler/manager behavior (handshake, lifecycle, retries, event routing, stats filtering). - Restore
defguard_coregRPC integration tests (health + worker) and expose a test-support router builder. - Refactor gateway-manager internals to support test-only transport/control hooks (test socket, retry overrides, metrics) and add focused unit tests for cert refresh and stats/config mapping.
Reviewed changes
Copilot reviewed 29 out of 30 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/defguard_gateway_manager/tests/mod.rs | Test module entrypoint wiring for the new integration suite. |
| crates/defguard_gateway_manager/tests/gateway_manager/mod.rs | Splits gateway-manager tests into handler/manager modules. |
| crates/defguard_gateway_manager/tests/gateway_manager/manager.rs | Integration tests covering manager startup, notifications, restarts, and retry behavior. |
| crates/defguard_gateway_manager/tests/gateway_manager/handler.rs | Integration test driver that includes the handler test cases. |
| crates/defguard_gateway_manager/tests/gateway_manager/handler/support.rs | Shared test helpers/assertions and DB setup helpers for handler tests. |
| crates/defguard_gateway_manager/tests/gateway_manager/handler/handshake.rs | Tests for config handshake behavior and idempotency. |
| crates/defguard_gateway_manager/tests/gateway_manager/handler/lifecycle.rs | Tests for connected/disconnected DB state transitions. |
| crates/defguard_gateway_manager/tests/gateway_manager/handler/stats.rs | Tests for peer-stats forwarding and endpoint validation. |
| crates/defguard_gateway_manager/tests/gateway_manager/handler/network_events.rs | Tests for network create/modify/delete event routing. |
| crates/defguard_gateway_manager/tests/gateway_manager/handler/firewall_events.rs | Tests for firewall config/disable event routing. |
| crates/defguard_gateway_manager/tests/gateway_manager/handler/device_events.rs | Tests for device create/modify/delete event routing and filtering. |
| crates/defguard_gateway_manager/tests/gateway_manager/handler/mfa.rs | Tests for MFA session authorized/disconnected events producing peer updates. |
| crates/defguard_gateway_manager/tests/common/mod.rs | New reusable harness: mock gateway service + manager/handler test contexts. |
| crates/defguard_gateway_manager/src/tests.rs | Removes obsolete in-crate test scaffolding. |
| crates/defguard_gateway_manager/src/lib.rs | Adds test-support plumbing (control hooks, counters, retry overrides) and manager behavior tweaks (skip disabled on startup, listener-ready notify). |
| crates/defguard_gateway_manager/src/handler.rs | Adds test transport support (UDS connector), test handler wrapper, refactors reconnection/disconnection handling, and adds unit tests. |
| crates/defguard_gateway_manager/src/certs.rs | Adds unit tests for cert collection/refresh publication semantics. |
| crates/defguard_gateway_manager/Cargo.toml | Moves hyper-util from dev-dependency to dependency to support new transport code. |
| crates/defguard_core/tests/integration/main.rs | Re-enables the grpc integration test module. |
| crates/defguard_core/tests/integration/grpc/mod.rs | Switches grpc integration coverage to health + worker modules. |
| crates/defguard_core/tests/integration/grpc/health.rs | Adds gRPC health test for worker service. |
| crates/defguard_core/tests/integration/grpc/worker.rs | Adds/updates worker-service integration tests (registration, jobs, interceptor behavior). |
| crates/defguard_core/tests/integration/grpc/common/mod.rs | Reworks gRPC test server harness to use test-support router builder + JWT secret overrides. |
| crates/defguard_core/tests/integration/grpc/gateway.rs | Removes legacy gateway integration tests that no longer match the new comms model. |
| crates/defguard_core/tests/integration/grpc/common/mock_gateway.rs | Removes legacy gateway mock used by the deleted gateway integration tests. |
| crates/defguard_core/src/grpc/mod.rs | Marks worker service as “serving” for health; exposes a doc(hidden) test-support router builder. |
| crates/defguard_core/Cargo.toml | Adds hyper-util to dev-dependencies for grpc integration harness. |
| crates/defguard_common/src/db/models/gateway.rs | Moves Gateway::url() to the generic impl so it’s available for Gateway<NoId> too. |
| crates/defguard_common/src/auth/claims.rs | Adds JWT secret override facility via OnceLock and exposes test-support initializer. |
| Cargo.lock | Adds hyper-util to the workspace lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+285
to
+289
| #[doc(hidden)] | ||
| pub mod test_support { | ||
| use std::{collections::HashMap, path::PathBuf, sync::Arc, time::Duration}; | ||
|
|
||
| use defguard_common::{ |
Comment on lines
+135
to
+140
| #[doc(hidden)] | ||
| pub mod test_support { | ||
| use super::{JWT_SECRET_OVERRIDES, JwtSecretOverrides}; | ||
|
|
||
| pub fn initialize_jwt_secret_overrides( | ||
| auth_secret: impl Into<String>, |
Comment on lines
94
to
+98
| pub(crate) async fn make_grpc_test_server(pool: &PgPool) -> TestGrpcServer { | ||
| // create communication channel for clients | ||
| let (client_stream, server_stream) = tokio::io::duplex(1024); | ||
| let client_channel = create_client_channel(client_stream).await; | ||
|
|
||
| // setup helper structs | ||
| let (grpc_event_tx, grpc_event_rx) = unbounded_channel::<GrpcEvent>(); | ||
| let (app_event_tx, _app_event_rx) = unbounded_channel::<AppEvent>(); | ||
| let worker_state = Arc::new(Mutex::new(WorkerState::new(app_event_tx.clone()))); | ||
| let (wg_tx, _wg_rx) = broadcast::channel::<GatewayEvent>(16); | ||
| let (peer_stats_tx, peer_stats_rx) = unbounded_channel::<PeerStatsUpdate>(); | ||
| let gateway_state = Arc::new(Mutex::new(GatewayMap::new())); | ||
| let client_state = Arc::new(Mutex::new(ClientMap::new())); | ||
|
|
||
| let failed_logins = FailedLoginMap::new(); | ||
| let failed_logins = Arc::new(Mutex::new(failed_logins)); | ||
|
|
||
| initialize_jwt_secrets(); | ||
| initialize_users(pool).await; | ||
| initialize_current_settings(pool) | ||
| .await | ||
| .expect("Could not initialize settings"); | ||
|
|
||
| let license = License::new( | ||
| "test_customer".to_string(), | ||
| false, | ||
| // Permanent license | ||
| None, | ||
| None, | ||
| None, | ||
| LicenseTier::Business, | ||
| ); | ||
|
|
||
| set_cached_license(Some(license)); | ||
| let server = Server::builder(); | ||
| let (client_stream, server_stream) = tokio::io::duplex(1024); |
There was a problem hiding this comment.
Pull request overview
This PR re-enables and modernizes gRPC-related integration tests in defguard_core and introduces a new test harness in defguard_gateway_manager to validate gateway handler/manager behavior under the “reversed communication” model (Core as gRPC client, Gateway as server).
Changes:
- Add a comprehensive mock-gateway + context framework and new SQLx-backed tests for
GatewayManagerandGatewayHandler(handshake, lifecycle, retries, event routing, peer stats filtering). - Refactor
defguard_gateway_managerruntime code to support deterministic testing (test socket transport + test instrumentation hooks) while keeping production behavior intact. - Restore
defguard_coregRPC integration coverage via WorkerService + health checks, plus test-only JWT secret overrides to make auth deterministic in tests.
Reviewed changes
Copilot reviewed 29 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/defguard_gateway_manager/src/tests/mod.rs | Adds test module entrypoints. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/mod.rs | Splits gateway-manager tests into handler/manager suites. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/manager.rs | New integration-style tests for manager startup, updates, retries, purge, and reconnect behavior. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler.rs | Test aggregator including handler scenario suites. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/support.rs | Shared helper/assertion utilities for handler tests. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/handshake.rs | Tests config handshake behavior and idempotency. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/lifecycle.rs | Tests connected/disconnected DB state transitions. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/stats.rs | Tests peer stats forwarding and endpoint validation rules. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/network_events.rs | Tests network event → gateway update routing/filtering. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/firewall_events.rs | Tests firewall events routing/filtering. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/device_events.rs | Tests device create/modify/delete events routing/filtering. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/mfa.rs | Tests MFA session events → peer update behavior. |
| crates/defguard_gateway_manager/src/tests/common/mod.rs | Adds mock gateway gRPC server + ManagerTestContext/HandlerTestContext scaffolding. |
| crates/defguard_gateway_manager/src/tests.rs | Removes legacy/obsolete gateway manager test. |
| crates/defguard_gateway_manager/src/lib.rs | Adds test support hooks (socket overrides, counters, retry overrides) and improves startup/notification handling. |
| crates/defguard_gateway_manager/src/handler.rs | Adds test transport (Unix socket) + connection-attempt instrumentation and refactors connection/disconnection loop for testability. |
| crates/defguard_gateway_manager/src/certs.rs | Adds unit/integration tests for cert refresh publishing behavior. |
| crates/defguard_gateway_manager/Cargo.toml | Updates dependencies to support new test transport code. |
| crates/defguard_core/tests/integration/main.rs | Re-enables grpc integration test module. |
| crates/defguard_core/tests/integration/grpc/mod.rs | Switches grpc integration suite to health/worker coverage. |
| crates/defguard_core/tests/integration/grpc/health.rs | Adds health check test for WorkerService. |
| crates/defguard_core/tests/integration/grpc/worker.rs | Adds WorkerService integration tests (register, job lifecycle, interceptor auth). |
| crates/defguard_core/tests/integration/grpc/gateway.rs | Removes outdated gateway grpc tests (no longer matching the new comms model). |
| crates/defguard_core/tests/integration/grpc/common/mod.rs | Reworks grpc test server scaffolding; adds deterministic JWT helpers for tests. |
| crates/defguard_core/tests/integration/grpc/common/mock_gateway.rs | Removes obsolete mock gateway used by the old gateway grpc tests. |
| crates/defguard_core/src/grpc/mod.rs | Marks WorkerService as serving in health reporter; exposes test_support router builder. |
| crates/defguard_core/Cargo.toml | Adds dev dependency needed by grpc integration test transport. |
| crates/defguard_common/src/db/models/gateway.rs | Makes Gateway::url() available for all Gateway<I> variants. |
| crates/defguard_common/src/auth/claims.rs | Adds test-only JWT secret override facility (OnceLock-backed) for deterministic tests. |
| Cargo.lock | Locks new dependency usage. |
| .sqlx/query-59d048f8....json | Updates SQLx query metadata snapshot. |
| .sqlx/query-47c40636....json | Updates SQLx query metadata snapshot. |
Files not reviewed (2)
- .sqlx/query-47c406366d0b53ca805cff303dfe2a67880adeaca1e10e50bea9b9fc53e08845.json: Language not supported
- .sqlx/query-59d048f8110a53745afd80c607fc52cb537dfded663e6bbee73d5f908f0ca89e.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
18
to
22
| anyhow.workspace = true | ||
| chrono.workspace = true | ||
| hyper-util = "0.1" | ||
| hyper-rustls.workspace = true | ||
| reqwest.workspace = true |
# Conflicts: # crates/defguard_gateway_manager/src/handler.rs
There was a problem hiding this comment.
Pull request overview
This PR re-enables and modernizes gRPC integration tests in defguard_core (aligned with the newer “reversed” gateway communication) and introduces a dedicated testing harness + suite for defguard_gateway_manager handlers/managers to validate runtime behavior (handshake, event routing, retries, peer-stats filtering).
Changes:
- Re-enable
defguard_coregRPC integration tests (replacing old gateway-mock tests with worker/health-focused tests and updated test router utilities). - Add a full
defguard_gateway_managertest framework (mock gateway over Unix sockets, manager/handler contexts, and scenario coverage). - Reduce test flakiness via shared-state guards/locks across setup/API/enterprise tests and supporting helpers (JWT secret overrides, test-only gRPC router access).
Reviewed changes
Copilot reviewed 48 out of 49 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/defguard_setup/tests/wizard_state.rs | Serializes setup-server tests via a shared async test guard. |
| crates/defguard_setup/tests/session_info.rs | Serializes setup/migration session-info tests via the test guard. |
| crates/defguard_setup/tests/migration_wizard.rs | Updates migration wizard tests to use the shared test guard. |
| crates/defguard_setup/tests/initial_setup.rs | Updates initial setup tests to use the shared test guard. |
| crates/defguard_setup/tests/common/mod.rs | Introduces a global async mutex guard and threads it through the test client to prevent concurrent interference. |
| crates/defguard_setup/tests/auto_adoption_wizard.rs | Updates auto-adoption tests to use the shared test guard. |
| crates/defguard_gateway_manager/src/tests/mod.rs | Adds the new unit-test module root for gateway manager tests. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/mod.rs | Adds submodules for handler/manager test suites. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/manager.rs | Adds manager-level behavior tests (startup, notifications, reconnect/retry semantics). |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/support.rs | Adds shared test helpers/assertions for handler update payloads and seeded DB state. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/stats.rs | Adds handler tests ensuring peer-stats are ignored pre-handshake and malformed endpoints are dropped. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/network_events.rs | Adds handler tests for routing network create/modify/delete events. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/mfa.rs | Adds handler tests for MFA session authorized/disconnected → peer create/delete updates. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/lifecycle.rs | Adds handler lifecycle tests (DB connected/disconnected timestamps). |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/handshake.rs | Adds handler handshake tests (config request/response, ignore repeats). |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/firewall_events.rs | Adds handler tests for firewall config changed/disabled events. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/device_events.rs | Adds handler tests for device create/modify/delete updates + ignore rules. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler.rs | Aggregates handler test modules via include! and shared support module. |
| crates/defguard_gateway_manager/src/tests/common/mod.rs | Implements mock gateway gRPC service + test contexts for manager/handler, using Unix sockets. |
| crates/defguard_gateway_manager/src/tests.rs | Removes legacy/obsolete gateway handler test scaffold. |
| crates/defguard_gateway_manager/src/lib.rs | Adds test-support plumbing (socket overrides, counters/notifiers, retry overrides) and improves task lifecycle control. |
| crates/defguard_gateway_manager/src/handler.rs | Adds test transport over Unix sockets, refactors connection loop for testability, and improves reconnect handling structure. |
| crates/defguard_gateway_manager/src/certs.rs | Adds unit/integration tests for cert refresh behavior and filtering semantics. |
| crates/defguard_gateway_manager/Cargo.toml | Moves hyper-util dependency (currently to normal deps). |
| crates/defguard_core/tests/integration/main.rs | Re-enables the gRPC integration test module. |
| crates/defguard_core/tests/integration/grpc/worker.rs | Adds worker-service integration tests (register/get_job/set_job_done + auth interceptor coverage). |
| crates/defguard_core/tests/integration/grpc/mod.rs | Switches gRPC test modules to health and worker (drops old gateway tests). |
| crates/defguard_core/tests/integration/grpc/health.rs | Adds gRPC health-check integration test for the worker service. |
| crates/defguard_core/tests/integration/grpc/gateway.rs | Removes legacy gateway gRPC integration tests (outdated comms direction/auth model). |
| crates/defguard_core/tests/integration/grpc/common/mod.rs | Reworks gRPC test server harness (JWT override init, test_support router, worker/app events). |
| crates/defguard_core/tests/integration/grpc/common/mock_gateway.rs | Removes legacy mock gateway (no longer matches current gateway↔core approach). |
| crates/defguard_core/tests/integration/api/common/mod.rs | Adds a global async lock and plumbs it through API test client creation. |
| crates/defguard_core/tests/integration/api/common/client.rs | Keeps API lock guard alive for the lifetime of the test client/server task. |
| crates/defguard_core/tests/integration/api/acl/mod.rs | Updates ACL API tests to acquire/pass the API test guard. |
| crates/defguard_core/src/handlers/user.rs | Adjusts LDAP sync-allowed checks to use the new executor expectations. |
| crates/defguard_core/src/grpc/mod.rs | Marks Worker service as “serving” in health and exposes a doc-hidden router builder for tests. |
| crates/defguard_core/src/enterprise/mod.rs | Adds a shared async mutex lock for enterprise tests that mutate global license state. |
| crates/defguard_core/src/enterprise/ldap/tests.rs | Serializes specific LDAP enterprise tests using the shared enterprise test lock. |
| crates/defguard_core/src/enterprise/ldap/model.rs | Changes ldap_sync_allowed_for_user to Acquire and reads settings from DB via an acquired connection. |
| crates/defguard_core/src/enterprise/firewall/tests/mod.rs | Introduces an enterprise-test lock helper for firewall test modules. |
| crates/defguard_core/src/enterprise/firewall/tests/destination.rs | Uses the enterprise-test lock guard to serialize license/stateful tests. |
| crates/defguard_core/src/enterprise/firewall/tests/all_locations.rs | Uses the enterprise-test lock guard to serialize license/stateful tests. |
| crates/defguard_core/src/enterprise/directory_sync/tests.rs | Serializes directory sync enterprise tests using the enterprise test lock. |
| crates/defguard_core/Cargo.toml | Adds hyper-util for dev/integration-test support. |
| crates/defguard_common/src/db/models/gateway.rs | Moves Gateway::url() to the generic impl so it’s available for NoId gateways (test usage). |
| crates/defguard_common/src/config.rs | Rewrites new_test_config to avoid clap parsing; sets explicit test defaults. |
| crates/defguard_common/src/auth/claims.rs | Adds OnceLock-based JWT secret overrides for deterministic test token generation. |
| Cargo.lock | Updates lockfile for added dependency usage. |
| .cargo/config.toml | Forces RUST_TEST_THREADS=1 globally for cargo test. |
Comments suppressed due to low confidence (1)
crates/defguard_gateway_manager/Cargo.toml:24
hyper-utilappears to be used only inside#[cfg(test)]code (e.g., the Unix-socket connector insrc/handler.rs). Keeping it in[dependencies]pulls it into non-test builds unnecessarily. Consider moving it back to[dev-dependencies](or making it optional behind a test-only feature) so production builds don’t compile/link it.
anyhow.workspace = true
chrono.workspace = true
hyper-util = "0.1"
hyper-rustls.workspace = true
reqwest.workspace = true
semver.workspace = true
serde_json.workspace = true
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.cargo/config.toml
Outdated
Comment on lines
+4
to
+6
|
|
||
| [env] | ||
| RUST_TEST_THREADS = { value = "1", force = true } |
|
|
||
| let mut transaction = appstate.pool.begin().await?; | ||
| let ldap_sync_allowed = ldap_sync_allowed_for_user(&user, &mut *transaction).await?; | ||
| let ldap_sync_allowed = ldap_sync_allowed_for_user(&user, &appstate.pool).await?; |
| ); | ||
| let mut transaction = appstate.pool.begin().await?; | ||
| let user_for_ldap = if ldap_sync_allowed_for_user(&user, &mut *transaction).await? { | ||
| let user_for_ldap = if ldap_sync_allowed_for_user(&user, &appstate.pool).await? { |
crates/defguard_common/src/config.rs
Outdated
Comment on lines
+262
to
+264
| check_period: Duration::from(std::time::Duration::from_secs(12 * 3600)), | ||
| check_period_no_license: Duration::from(std::time::Duration::from_secs(24 * 3600)), | ||
| check_period_renewal_window: Duration::from(std::time::Duration::from_secs(3600)), |
There was a problem hiding this comment.
Pull request overview
This PR re-enables and reshapes parts of the gRPC test suite and introduces a more comprehensive test harness for defguard_gateway_manager gateway handlers/managers (including Unix-socket based mock gateways), while also adding test serialization guards to reduce flakiness in areas that rely on global/shared state.
Changes:
- Added a new
defguard_gateway_managertest framework (mock gateway service + manager/handler contexts) and a broad set of handler/manager tests aligned with the “reversed communication” flow. - Restored
crates/defguard_core/tests/integration/grpcas an enabled module, adding worker + health coverage and removing the legacy gateway integration tests/mocks. - Introduced test-only global locks/guards in multiple test suites and added JWT secret override support for deterministic auth in tests.
Reviewed changes
Copilot reviewed 47 out of 48 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/defguard_setup/tests/wizard_state.rs | Passes a shared async test guard into setup test client to serialize setup tests. |
| crates/defguard_setup/tests/session_info.rs | Passes a shared async test guard into setup/migration test clients. |
| crates/defguard_setup/tests/migration_wizard.rs | Passes a shared async test guard into migration test client. |
| crates/defguard_setup/tests/initial_setup.rs | Serializes setup-flow tests via guard; updates setup test client creation. |
| crates/defguard_setup/tests/common/mod.rs | Adds global OnceLock + async Mutex guard and threads it through TestClient constructors. |
| crates/defguard_setup/tests/auto_adoption_wizard.rs | Passes a shared async test guard into setup test client usage. |
| crates/defguard_gateway_manager/src/tests/mod.rs | Introduces new internal test module layout. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/mod.rs | Splits gateway manager tests into handler + manager modules. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/manager.rs | Adds manager lifecycle/retry/notification behavior tests. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/support.rs | Adds helper builders/assertions for gateway handler tests. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/stats.rs | Adds peer-stats forwarding/validation tests. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/network_events.rs | Adds network create/modify/delete routing tests. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/mfa.rs | Adds MFA session event → peer update tests. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/lifecycle.rs | Adds connected/disconnected timestamp behavior tests. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/handshake.rs | Adds config request handshake tests (first request / repeated request / no unsolicited config). |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/firewall_events.rs | Adds firewall config/disable event routing tests. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler/device_events.rs | Adds device create/modify/delete event → peer update tests. |
| crates/defguard_gateway_manager/src/tests/gateway_manager/handler.rs | Wires handler test suite via include! + shared support module. |
| crates/defguard_gateway_manager/src/tests/common/mod.rs | Implements MockGateway gRPC service + test contexts for manager/handler. |
| crates/defguard_gateway_manager/src/tests.rs | Removes legacy gateway manager test file. |
| crates/defguard_gateway_manager/src/lib.rs | Adds test support hooks (socket override, counters, listener readiness), improves reconnect timing control, and refactors notification handling. |
| crates/defguard_gateway_manager/src/handler.rs | Adds Unix-socket test transport, test-support hooks, refactors connection loop/disconnect handling, and adds targeted unit tests for config/stats conversion. |
| crates/defguard_gateway_manager/src/certs.rs | Adds unit/integration tests for certificate collection/refresh publishing behavior. |
| crates/defguard_gateway_manager/Cargo.toml | Moves hyper-util into regular dependencies (was dev-only). |
| crates/defguard_core/tests/integration/main.rs | Re-enables the grpc integration test module. |
| crates/defguard_core/tests/integration/grpc/worker.rs | Adds worker service integration tests (registration/job lifecycle/auth). |
| crates/defguard_core/tests/integration/grpc/mod.rs | Switches grpc test modules to health + worker. |
| crates/defguard_core/tests/integration/grpc/health.rs | Adds health check integration test for worker service. |
| crates/defguard_core/tests/integration/grpc/gateway.rs | Removes legacy gateway gRPC integration tests. |
| crates/defguard_core/tests/integration/grpc/common/mod.rs | Reworks gRPC test server scaffolding, adds JWT secret overrides + helpers for auth metadata. |
| crates/defguard_core/tests/integration/grpc/common/mock_gateway.rs | Removes legacy mock gateway helper. |
| crates/defguard_core/tests/integration/api/common/mod.rs | Adds a global async lock to serialize API integration tests and threads guard through client creation. |
| crates/defguard_core/tests/integration/api/common/client.rs | Stores the async test guard in TestClient to keep serialization active for the client lifetime. |
| crates/defguard_core/tests/integration/api/acl/mod.rs | Acquires and passes the API test guard for ACL client setup. |
| crates/defguard_core/src/handlers/user.rs | Adjusts LDAP sync check calls after ldap executor signature change. |
| crates/defguard_core/src/grpc/mod.rs | Marks Worker service as serving in health reporter; exposes test_support::build_grpc_service_router. |
| crates/defguard_core/src/enterprise/mod.rs | Adds a global async lock to serialize enterprise tests touching shared cached/global state. |
| crates/defguard_core/src/enterprise/ldap/tests.rs | Serializes LDAP enterprise tests using enterprise test-state lock. |
| crates/defguard_core/src/enterprise/ldap/model.rs | Changes ldap sync eligibility helper to acquire a connection and read settings from DB. |
| crates/defguard_core/src/enterprise/firewall/tests/mod.rs | Adds helper to acquire enterprise test-state lock for firewall tests. |
| crates/defguard_core/src/enterprise/firewall/tests/destination.rs | Serializes firewall destination tests via enterprise lock. |
| crates/defguard_core/src/enterprise/firewall/tests/all_locations.rs | Serializes firewall all-locations tests via enterprise lock. |
| crates/defguard_core/src/enterprise/directory_sync/tests.rs | Serializes directory sync enterprise tests via enterprise lock. |
| crates/defguard_core/Cargo.toml | Adds hyper-util as a dev-dependency for grpc test scaffolding. |
| crates/defguard_common/src/db/models/gateway.rs | Moves Gateway::url() to impl<I> so it’s available across ID/no-ID variants. |
| crates/defguard_common/src/config.rs | Replaces clap-based test config parsing with an explicit DefGuardConfig test constructor. |
| crates/defguard_common/src/auth/claims.rs | Adds process-wide JWT secret overrides for tests via OnceLock. |
| Cargo.lock | Records new dependency usage (hyper-util). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+726
to
+727
| let mut transaction = appstate.pool.begin().await?; | ||
| let ldap_sync_allowed = ldap_sync_allowed_for_user(&user, &mut *transaction).await?; | ||
| let ldap_sync_allowed = ldap_sync_allowed_for_user(&user, &appstate.pool).await?; |
Comment on lines
893
to
895
| let mut transaction = appstate.pool.begin().await?; | ||
| let user_for_ldap = if ldap_sync_allowed_for_user(&user, &mut *transaction).await? { | ||
| let user_for_ldap = if ldap_sync_allowed_for_user(&user, &appstate.pool).await? { | ||
| Some(user.clone().as_noid()) |
Comment on lines
17
to
23
|
|
||
| anyhow.workspace = true | ||
| chrono.workspace = true | ||
| hyper-util = "0.1" | ||
| hyper-rustls.workspace = true | ||
| reqwest.workspace = true | ||
| semver.workspace = true |
crates/defguard_common/src/config.rs
Outdated
| } | ||
|
|
||
| // this is an ugly workaround to avoid `cargo test` args being captured by `clap` | ||
| #[allow(deprecated)] |
| mod api; | ||
| mod common; | ||
| // mod grpc; | ||
| mod grpc; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1854