feat: network config by JanZachmann · Pull Request #63 · omnect/omnect-ui

JanZachmann · 2025-11-01T21:40:54Z

Summary

Add network configuration management UI and backend with support for static IP and DHCP configuration.

Backend

Add network module with support for static IP and DHCP configuration
Implement network rollback mechanism with 90-second timeout
Add API endpoint for setting network configuration with validation
Refactor certificate management to use DeviceServiceClient reference
Move certificate creation to server startup after service client initialization
Remove certificate regeneration from network rollback flow
Add server restart channel for handling network changes
Cancel pending rollback on successful authentication

Frontend

Add Network page with network settings UI
Add NetworkSettings component for editing network configuration
Add NetworkActions component for network-related actions
Add useWaitForNewIp composable for handling IP changes
Update DeviceNetworks component to show network details
Refactor DeviceActions to use composable pattern
Update Vuetify and Biome configurations
Add route for /network page

Replace hyper-util with reqwest for all HTTP/Unix socket communication, remove Arc wrappers, and eliminate the socket_client module to simplify the codebase. Changes: - Remove Arc wrappers from Api struct fields (OmnectDeviceServiceClient and SingleSignOnProvider are already cheap to clone via internal pools) - Replace hyper-util Client with reqwest for Unix socket communication using reqwest 0.12.23's native unix_socket() support - Remove socket_client.rs module entirely (~109 lines removed) - Inline HTTP request logic directly in OmnectDeviceServiceClient with cleaner helper methods: get(), post(), post_json() - Convert keycloak_client from blocking to async reqwest - Update certificate.rs to use reqwest::Client directly Dependencies: - Remove: http-body-util, hyper, hyperlocal, hyper-util - Update: reqwest from 0.12 to 0.12.23 (minimum for Unix socket support) - Remove: blocking feature from reqwest (no longer needed) Benefits: - Single HTTP client library throughout codebase - Simpler, more maintainable code with direct reqwest usage - Better error messages and logging - Removed unnecessary abstraction layers - Zero runtime overhead (no Arc, reqwest uses internal Arc) Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

This commit implements comprehensive refactorings across the codebase: Performance Optimizations: - Cache Keycloak HTTP client using OnceLock to avoid repeated instantiation - Consolidate HTTP client usage patterns Code Quality Improvements: - Add HTTP response handling helper to reduce duplication - Consolidate path macros for consistency - Add API error handling helper with standardized logging - Use endpoint constants instead of string literals - Remove unnecessary .map(|_| ()) patterns - Standardize error logging format to {e:#} - Standardize all error messages to use "failed to..." format Maintainability: - Replace magic string "omnect-ui" with env!("CARGO_PKG_NAME") - Improve string interpolation to use direct variable references - Optimize imports (remove wildcard imports, use explicit imports) - Fix inconsistent crate imports in main.rs Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

This commit introduces HttpClientFactory to centralize HTTP client creation, eliminating code duplication and ensuring consistent configuration. New Module: http_client.rs - HttpClientFactory with three client types: - https_client(): Cached HTTPS client for external APIs - unix_socket_client(): For local Unix socket communication - workload_client(): For IoT Edge workload API Testing: - Unit tests: 8 tests covering all three client factory methods - test_https_client_is_cached - test_https_client_returns_valid_client - test_https_client_multiple_calls_same_instance - test_workload_client_parses_uri - test_workload_client_rejects_invalid_scheme - test_unix_socket_client_creates_client - test_unix_socket_client_with_relative_path - test_unix_socket_client_with_empty_path - Integration tests: 14 tests in tests/http_client.rs - 6 tests for https_client (using httpbin.org and example.com) - 4 tests for unix_socket_client (using mock Unix socket servers) - 4 tests for workload_client (using mock IoT Edge workload API) - Test configuration: Added .cargo/config.toml - Set RUST_TEST_THREADS=1 to prevent rate-limiting issues - Ensures reliable test execution for external API tests - Fixed dead_code warning for workload_client when using mock feature by adding #[cfg_attr(feature = "mock", allow(dead_code))] Benefits: - Eliminates 3 separate OnceLock instances across modules - Single source of truth for HTTP client configuration - Shared connection pools improve performance - Easier to add timeout/retry configuration in future - Better testability with centralized mocking point - Comprehensive test coverage for all client types Changes by Module: - keycloak_client.rs: Use HttpClientFactory::https_client() - Removed local OnceLock<Client> - 4 lines removed, cleaner code - omnect_device_service_client.rs: Use HttpClientFactory::unix_socket_client() - Simplified client creation - 3 lines removed - certificate.rs: Use HttpClientFactory::workload_client() - Removed manual URI parsing - 8 lines removed, moved to factory Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

This commit introduces a centralized TokenManager for handling JWT token creation and verification, eliminating scattered token logic. New Module: auth/token.rs - TokenManager: Centralized token management - create_token(): Generate JWT tokens with configured expiration - verify_token(): Validate tokens with proper verification options - Configurable expiration, subject, and secret Benefits: - Single source of truth for token handling - Eliminates duplicate token creation/verification logic - Consistent verification options (expiration, subject, tolerance) - Better testability with 4 comprehensive tests - Easier to change token strategy in future Changes by Module: - middleware.rs: Use TokenManager for verify_token() - Removed inline HS256Key creation - Removed manual VerificationOptions setup - 15 lines simplified to 1-line call - api.rs: Use TokenManager for session_token() - Removed inline HS256Key and Claims creation - Cleaner error handling - 8 lines simplified to 6 lines Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

…h lifecycle management This refactoring improves the architecture of OmnectDeviceServiceClient by: - Implementing a type-safe builder pattern as the only way to create the client - Making certificate setup optional and injectable via closure - Making publish endpoint registration optional - Extracting client creation into dedicated build_service_client() function - Bumping version to 1.0.6 Key changes: - OmnectDeviceServiceClient fields are now private, enforcing builder usage - Builder supports optional certificate setup via with_certificate_setup() - Builder supports optional publish endpoint via with_publish_endpoint() - Certificate payload (CreateCertPayload) is now public and reusable - Removed intermediate WorkloadCertPayload struct for simplicity - Shutdown is now a DeviceServiceClient trait method called explicitly from main - Improved Dockerfile to copy entire src directory structure This design provides better encapsulation, clearer lifecycle management, and looser coupling between modules through dependency injection. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

…-settings Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

…tings Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

ronny-standtke

LGTM. Some nits.

ronny-standtke · 2025-11-12T08:42:44Z

Cargo.toml

@@ -37,6 +38,7 @@ log-panics = { version = "2.1", default-features = false, features = [
 mockall = { version = "0.13", optional = true, default-features = false }
 rand_core = { version = "0.9", default-features = false, features = ["std"] }
 reqwest = { version = "0.12.23", default-features = false, features = ["json", "rustls-tls"] }


Shoudn't it be just 0.12 to match the other entries?

no we need exactly that version

ronny-standtke · 2025-11-12T08:43:12Z

Cargo.toml

  "net",
  "process",
 ] }
+trait-variant = { version = "0.1" }


default-features = false is missing

ronny-standtke · 2025-11-12T09:08:03Z

src/main.rs

+                Cors::default()
+                    .allow_any_origin()
+                    .allow_any_header()
+                    .allowed_methods(vec!["GET"])
+                    .supports_credentials()
+                    .max_age(3600),


This is something we should change in future since it weakens the api. We currently use it so that when the IP changes, the browser can check whether the new IP is accessible. There are other methods (image ping) we should consider.

I create a task

empwilli

Overall LGTM, minor nits

empwilli · 2025-11-12T09:01:16Z

src/main.rs

    let mut centrifugo = run_centrifugo();
    let service_client = OmnectDeviceServiceClientBuilder::new()


If we log shutdown, IMHO we should log here the startup, as well. This will help in case of restarts.

empwilli · 2025-11-12T09:03:25Z

src/network.rs

+
+// ============================================================================
+// Macros
+// ============================================================================
+


IMHO we shouldn't use such markers or use them consistently.

will be part of future PR

empwilli · 2025-11-12T09:09:08Z

src/network.rs

+// ============================================================================
+// Static State
+// ============================================================================


see other review

empwilli · 2025-11-12T09:09:15Z

src/network.rs

+// ============================================================================
+// Constants
+// ============================================================================


see other review

empwilli · 2025-11-12T09:09:21Z

src/network.rs

+// ============================================================================
+// Structs
+// ============================================================================


see other review

empwilli · 2025-11-12T09:09:29Z

src/network.rs

+// ============================================================================
+// Service
+// ============================================================================


see other review

empwilli · 2025-11-12T09:09:34Z

src/network.rs

+    // ========================================================================
+    // Public methods
+    // ========================================================================


see other review

empwilli · 2025-11-12T09:09:44Z

src/network.rs

+    // ========================================================================
+    // Private helper methods
+    // ========================================================================


see other review

empwilli · 2025-11-12T09:28:31Z

src/network.rs

+        if let Ok(true) = fs::exists(&config_file) {
+            fs::copy(&config_file, &backup_file)
+                .context(format!("failed to back up {}", config_file.display()))?;
+        } else {


IMHO sth. like the following is a little more robust, as it atomically checks file presence and performs the operation.

fn try_copy_if_exists(src: PathBuf&, dest: PathBuf&) -> Result<bool> match fs::copy(src, dest) { Ok(_) | Err(e) if e.kind() == ErrorKind::NotFound => Ok(true), Err(e) => Err(e) } }

Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

JanZachmann force-pushed the feature/network-settings branch 2 times, most recently from 00c3338 to dcd9025 Compare November 5, 2025 14:03

JanZachmann added 3 commits November 7, 2025 15:41

JanZachmann force-pushed the feature/network-settings branch from dcd9025 to ec453fb Compare November 7, 2025 15:15

feature: introduce NetworkConfigService

b8a3dfe

Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

JanZachmann force-pushed the feature/network-settings branch from e0e05c5 to b8a3dfe Compare November 10, 2025 09:03

JanZachmann added 2 commits November 11, 2025 15:26

Merge branch 'refactor-device-service-singleton' into feature/network…

4b1e148

…-settings Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

JanZachmann force-pushed the feature/network-settings branch from 3290b2a to 4b1e148 Compare November 12, 2025 07:08

Merge remote-tracking branch 'upstream/main' into feature/network-set…

c87507d

…tings Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

JanZachmann force-pushed the feature/network-settings branch from 96cbf78 to c87507d Compare November 12, 2025 08:23

JanZachmann requested review from empwilli and ronny-standtke November 12, 2025 08:27

ronny-standtke approved these changes Nov 12, 2025

View reviewed changes

empwilli approved these changes Nov 12, 2025

View reviewed changes

fix: fixed findings

b21cdfd

Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

JanZachmann merged commit 1e9ad90 into omnect:main Nov 12, 2025
2 checks passed

JanZachmann deleted the feature/network-settings branch December 18, 2025 08:44

		let mut centrifugo = run_centrifugo();
		let service_client = OmnectDeviceServiceClientBuilder::new()

Conversation

JanZachmann commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Backend

Frontend

Uh oh!

ronny-standtke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

empwilli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JanZachmann commented Nov 1, 2025 •

edited

Loading