carverauto/serviceradar

Fork 0

1915 create common onboarding library to eliminate edge deployment friction #2397

Merged

mfreeman451 merged 22 commits from refs/pull/2397/head into main

2025-11-03 04:52:34 +00:00

mfreeman451 commented

2025-11-03 00:59:10 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #1916
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1916
Original created: 2025-11-03T00:59:10Z
Original updated: 2025-11-03T04:53:25Z
Original head: carverauto/serviceradar:1915-create-common-onboarding-library-to-eliminate-edge-deployment-friction
Original base: main
Original merged: 2025-11-03T04:52:34Z by @mfreeman451

User description

IMPORTANT: Please sign the Developer Certificate of Origin

Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include
a DCO sign-off statement indicating the DCO acceptance in one commit message. Here
is an example DCO Signed-off-by line in a commit message:

Signed-off-by: J. Doe <j.doe@domain.com>

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

I have signed the DCO?
The build completes without errors?
All tests are passing when running make test?

PR Type

Enhancement, Tests

Description

Implements comprehensive edge onboarding library to eliminate deployment friction for edge-deployed services (pollers, agents, checkers)
Creates service registry infrastructure with registration, lifecycle management, and heartbeat tracking for pollers, agents, and checkers
Adds device registry integration for service components with auto-registration and tombstone filtering for deleted devices
Implements SPIRE credential configuration and deployment environment detection (Kubernetes, Docker, bare-metal) for edge services
Integrates edge onboarding into all service components (poller, agent, checkers, datasvc, consumers) with automatic configuration generation
Adds DataSvc Core registration mechanism with periodic heartbeat updates via gRPC
Extends API server with new endpoints for device registry queries, agent discovery, datasvc instance listing, and device deletion
Implements device lifecycle event publishing for audit trail and lifecycle tracking
Adds Kong configuration support for web service routing to device endpoints
Updates database layer with agent discovery methods and poller registration fields
Regenerates mocks with standardized parameters and new edge onboarding support

Diagram Walkthrough

flowchart LR
  A["Edge Services<br/>Poller/Agent/Checker"] -->|TryOnboard| B["Bootstrapper"]
  B -->|Download| C["Package"]
  B -->|Configure| D["SPIRE"]
  B -->|Generate Config| E["Service Config"]
  C -->|Register| F["Service Registry"]
  F -->|Track| G["Poller/Agent/Checker"]
  G -->|Heartbeat| H["Core Server"]
  H -->|Device Update| I["Device Registry"]
  I -->|Lifecycle Event| J["Event Publisher"]
  H -->|API Endpoints| K["Agent/DataSvc/Device APIs"]

File Walkthrough

Relevant files

Tests

5 files

mock_db.go `Regenerate mocks with standardized parameters and edge onboarding` `support` pkg/db/mock_db.go Regenerated mock file with updated mockgen command path to include `pkg/db/` prefix Removed `isgomock` struct fields from mock types (`MockService`, `MockSysmonMetricsProvider`, `MockRows`, `MockQueryExecutor`) Standardized all mock method parameter names from semantic names (e.g., `ctx`, `pollerID`) to generic names (e.g., `arg0`, `arg1`) Added new mock methods for edge onboarding functionality: `DeleteEdgeOnboardingPackage`, `GetEdgeOnboardingPackage`, `InsertEdgeOnboardingEvent`, `ListEdgeOnboardingEvents`, `ListEdgeOnboardingPackages`, `ListEdgeOnboardingPollerIDs` Added new mock methods for agent management: `ListAgentsByPoller`, `ListAgentsWithPollers`	+394/-368
service_device_test.go `Device registration test coverage for all service types` pkg/registry/service_device_test.go Comprehensive test suite for device registration functionality Tests poller, agent, and checker device updates with metadata validation Validates device ID generation and uniqueness across service types Tests high-cardinality scenarios (100+ checkers) and mixed batch operations Verifies service devices with empty IPs are allowed while network devices require IPs	+462/-0
service_device_test.go `Unit tests for service device model functions` pkg/models/service_device_test.go Tests for service device ID generation and validation functions Validates `IsServiceDevice` detection logic for poller/agent/checker prefixes Tests network device ID generation with partition support Verifies device ID uniqueness across different service types Tests high-cardinality scenarios and multiple services on same IP	+400/-0
edge_onboarding_test.go `Update edge onboarding service test signatures` pkg/core/edge_onboarding_test.go Updates all test calls to `newEdgeOnboardingService()` to include new `nil` parameter Maintains consistency across 12 test functions with updated function signature	+15/-15
registry_test.go `Tests for device tombstone filtering logic` pkg/registry/registry_test.go Adds test `TestProcessBatchDeviceUpdates_DropsSelfReportedAfterDelete()` verifying self-reported updates are dropped for deleted devices Adds test `TestProcessBatchDeviceUpdates_AllowsFreshNonSelfReportedAfterDelete()` verifying fresh non-self-reported updates bypass deletion filter Tests tombstone filtering logic with mock database and deletion timestamp metadata	+124/-0

Enhancement

32 files

edge_onboarding.go `Add service registry integration and checker template support` pkg/core/edge_onboarding.go Added `ServiceManager` interface and registration type structs (`PollerRegistration`, `AgentRegistration`, `CheckerRegistration`) to avoid import cycles Added `ErrUnsupportedComponentType` error constant for unsupported component types Extended `edgeOnboardingService` with `deviceRegistryCallback` and `serviceRegistry` fields Added `SetDeviceRegistryCallback` method to register device registry callbacks Implemented `registerServiceComponent` method to register components in the service registry based on type Enhanced `CreatePackage` to inject `datasvc_endpoint` into metadata and register services Added `markServiceDeviceUnavailable` method to emit tombstone updates when packages are revoked Implemented checker-specific KV handling with template substitution in `applyComponentKVUpdates` Added `putKVDocument`, `substituteTemplateVariables`, and `substituteInMap` helper methods for template variable replacement Updated `kvKeyForPackage` to use correct KV path for checker configs (`agents/{agent_id}/checkers/{checker_kind}.json`)	+348/-8
edge_onboarding.go `Add DataSvc endpoint configuration to edge package creation` pkg/core/api/edge_onboarding.go Added `DataSvcEndpoint` field to `edgePackageCreateRequest` struct for DataSvc gRPC endpoint configuration Updated `handleCreateEdgePackage` to extract and trim `DataSvcEndpoint` from request and pass to `CreatePackage`	+2/-0
service_registry.go `Core service registry implementation with registration and lifecycle` `management` pkg/registry/service_registry.go Implements core `ServiceRegistry` struct managing lifecycle of pollers, agents, and checkers Provides registration methods (`RegisterPoller`, `RegisterAgent`, `RegisterChecker`) with validation and event emission Implements heartbeat recording with auto-activation of pending services Includes service deletion and purge functionality for inactive/revoked services Uses ClickHouse/Proton batch operations for efficient database writes	+752/-0
service_registry_queries.go `Query and retrieval operations for service registry` pkg/registry/service_registry_queries.go Implements query methods for retrieving registered services (`GetPoller`, `GetAgent`, `GetChecker`) Provides list operations with filtering by status and registration source Implements `IsKnownPoller` with TTL-based caching for performance Includes status update and service lifecycle management queries Supports hierarchical queries (agents by poller, checkers by agent)	+679/-0
pollers.go `Service registry integration into poller status handling` pkg/core/pollers.go Integrates service registry auto-registration on poller heartbeat Adds device registration for pollers, agents, and checkers Implements `ensurePollerRegistered`, `ensureAgentRegistered`, `ensureCheckerRegistered` methods Updates `isKnownPoller` to check service registry as primary path with fallback to legacy methods Adds device registry integration for inventory tracking	+259/-0
device_registry.go `HTTP API endpoints for device registry queries and deletion` pkg/core/api/device_registry.go Implements HTTP API endpoints for device registry information retrieval Provides `getDeviceRegistryInfo` endpoint to query service registration details Implements `deleteDevice` endpoint for tombstoning devices with audit trail Includes device type detection and partition extraction utilities Supports user attribution and metadata preservation on deletion	+310/-0
config.go `Edge onboarding configuration generation for service components` pkg/edgeonboarding/config.go Implements configuration generation for edge-deployed services (poller, agent, checker) Generates deployment-specific configuration with SPIFFE identity and SPIRE integration Handles metadata parsing and address resolution for different deployment types Supports storage path configuration and site-specific settings Includes comprehensive error handling for missing required metadata fields	+314/-0
service_registration.go `Service device update creation helper functions` pkg/models/service_registration.go Implements helper functions for creating device updates for service components `CreatePollerDeviceUpdate`, `CreateAgentDeviceUpdate`, `CreateCheckerDeviceUpdate` functions Generates service-aware device IDs with component type prefixes Automatically adds component-specific metadata to device updates Sets high confidence for self-reported service devices	+101/-0
bootstrap.go `Core edge onboarding bootstrapper implementation` pkg/edgeonboarding/bootstrap.go Introduces core `Bootstrapper` struct and `Config` for edge service onboarding workflow Implements `Bootstrap()` method orchestrating package download, SPIRE configuration, and service config generation Defines error types and deployment type constants for edge deployments Provides accessor methods for SPIFFE ID, join token, and generated configurations	+288/-0
spire.go `SPIRE credential configuration for edge components` pkg/edgeonboarding/spire.go Implements SPIRE credential configuration for pollers (nested server), agents, and checkers Generates SPIRE server and agent HCL configuration templates Handles trust bundle and join token file writing Provides SPIFFE ID extraction and trust domain parsing utilities	+247/-0
registry.go `Support service components in device registry` pkg/registry/registry.go Adds support for service component device IDs (pollers, agents, checkers) alongside network devices Implements `filterObsoleteUpdates()` to prevent stale updates for deleted devices Allows empty IPs for service components identified by service-aware device IDs Adds deletion timestamp extraction and bypass logic for tombstoned devices	+151/-12
core_registration.go `DataSvc Core registration and heartbeat service` pkg/datasvc/core_registration.go Implements background registration and heartbeat mechanism for datasvc with Core service Creates gRPC connection to Core using security provider and SPIFFE mTLS Sends periodic heartbeat updates via `ReportStatus` RPC Handles SPIFFE ID retrieval from workload API or environment	+206/-0
integration.go `Edge onboarding integration layer for services` pkg/edgeonboarding/integration.go Provides `TryOnboard()` entry point for services to attempt edge onboarding via environment variables Implements `IntegrationResult` struct for returning onboarding artifacts to services Handles generated config file writing and SPIRE workload API socket path retrieval Includes config unmarshaling helper for service integration	+206/-0
server.go `API server enhancements for service and event management` pkg/core/api/server.go Adds `WithEventPublisher()` and `WithServiceRegistry()` option functions for API server configuration Adds new routes for device deletion, registry info, datasvc instances, and agent listing Implements `handleDeviceByID()` router for GET/DELETE methods on device endpoints Adds service registry interface to API server type	+43/-1
kong.go `Kong configuration support for web service routing` pkg/cli/kong.go Adds `web-service` flag for optional Web service URL routing to `/api/devices` endpoints Implements Kong DB-less configuration generation for web service with JWT authentication Adds output directory creation logic to ensure parent directories exist Extends `renderKongDBLess()` to support separate web service routing	+31/-2
deployment.go `Deployment environment detection for edge services` pkg/edgeonboarding/deployment.go Implements `detectDeploymentType()` to identify Kubernetes, Docker, or bare-metal environments Provides environment detection helpers (`isKubernetes()`, `isDocker()`) checking env vars and filesystem Includes deployment-specific address resolution stubs for SPIRE and service discovery Handles deployment type configuration override from config	+152/-0
pollers.go `Poller registration and agent discovery database methods` pkg/db/pollers.go Updates `insertPollerStatus()` to include new poller registration fields (component_id, registration_source, status, etc.) Adds `AgentInfo` struct for agent-poller relationship queries Implements `ListAgentsWithPollers()` and `ListAgentsByPoller()` methods for agent discovery Queries services table to retrieve agent information with service types	+110/-4
service_models.go `Service registry domain models and types` pkg/registry/service_models.go Defines service lifecycle models: `ServiceStatus`, `RegistrationSource`, and registration request types Introduces `RegisteredPoller`, `RegisteredAgent`, `RegisteredChecker` structs for service state Defines `ServiceHeartbeat`, `ServiceFilter`, and `RegistrationEvent` for service management Establishes data structures for service registry operations	+147/-0
datasvc_registry.go `DataSvc instance registry API endpoint` pkg/core/api/datasvc_registry.go Implements `handleListDataSvcInstances()` endpoint for listing registered datasvc instances Queries services table for datasvc registrations with endpoint and availability information Returns empty list placeholder pending full datasvc registration implementation Includes type assertion and error handling for database connections	+103/-0
events.go `Event publishing refactor and device lifecycle events` pkg/natsutil/events.go Refactors `PublishPollerHealthEvent()` to use new `publishEvent()` helper method Implements `PublishDeviceLifecycleEvent()` for device lifecycle events (delete, restore, etc.) Adds generic `publishEvent()` method handling stream creation and event marshaling Supports device lifecycle event data with severity and level fields	+65/-27
interfaces.go `Service manager interface definition` pkg/registry/interfaces.go Adds `ServiceManager` interface defining service registration and lifecycle operations Includes methods for registering pollers, agents, and checkers Defines heartbeat recording, service queries, and status management operations Adds mock generation directive for `ServiceManager` interface	+63/-0
server.go `Core server service registry initialization` pkg/core/server.go Initializes `ServiceRegistry` in `NewServer()` with type assertion to `*db.DB` Creates service registry adapter to avoid import cycles Adds device registry callback for service cleanup on package revocation Implements `EventPublisher()` getter method for accessing event publisher	+32/-1
types.go `API server type enhancements for service and event support` pkg/core/api/types.go Adds `SetDeviceRegistryCallback()` method to `EdgeOnboardingService` interface Adds `serviceRegistry` field to `APIServer` struct for service management Adds `eventPublisher` field to `APIServer` struct for lifecycle events Imports `natsutil` package for event publishing support	+4/-0
services.go `Auto-registration of agents and checkers as devices` pkg/core/services.go Adds agent auto-registration as device when `AgentId` is present in service status Adds checker auto-registration as device for checker service types (snmp, sysmon, rperf, mapper) Implements `isCheckerService()` to identify checker service types Implements `generateCheckerID()` to create stable checker identifiers Calls service registry methods for agent and checker registration	+59/-0
server.go `DataSvc logging and Core registration initialization` pkg/datasvc/server.go Adds `logger` field to datasvc `Server` struct Implements `datasvcLogger` wrapper adapting zerolog to `logger.Logger` interface Initializes logger in `NewServer()` with service name and timestamp context Calls `StartCoreRegistration()` in `Start()` method for Core registration	+43/-1
agent_registry.go `Agent registry API endpoints` pkg/core/api/agent_registry.go Implements `handleListAgents()` endpoint for retrieving all agents with poller associations Implements `handleListAgentsByPoller()` endpoint for filtering agents by poller ID Defines `agentInfoView` struct for API responses with agent, poller, and service type information Includes error handling and JSON response formatting	+72/-0
main.go `Edge onboarding integration for db-event-writer consumer` cmd/consumers/db-event-writer/main.go Adds command-line flags for onboarding token and KV endpoint Calls `edgeonboarding.TryOnboard()` to attempt edge onboarding before loading config Uses generated config path from onboarding result if available Logs SPIFFE ID when onboarding is performed	+23/-4
processor.go `Device lifecycle event processing` pkg/consumers/db-event-writer/processor.go Adds `tryDeviceLifecycleEvent()` handler for device lifecycle CloudEvents Extracts device lifecycle event data and populates event row fields Maps device action, actor, severity, and timestamp to GELF format Handles device lifecycle events in event processing pipeline	+52/-0
unified_device.go `Device model support for service components` pkg/models/unified_device.go Adds `DiscoverySourceServiceRadar` constant for ServiceRadar infrastructure components Adds `ServiceType` and `ServiceID` fields to `DeviceUpdate` struct for service component identification Updates `GetSourceConfidence()` to return high confidence for ServiceRadar source Reformats discovery source constants for consistency	+16/-11
main.go `Edge onboarding integration for sysmon-vm checker` cmd/checkers/sysmon-vm/main.go Adds command-line flags for onboarding token and KV endpoint Calls `edgeonboarding.TryOnboard()` with `EdgeOnboardingComponentTypeChecker` Uses generated config path from onboarding result if available Logs SPIFFE ID when onboarding is performed	+17/-0
main.go `Edge onboarding integration for faker agent` cmd/faker/main.go Adds command-line flags for onboarding token and KV endpoint Calls `edgeonboarding.TryOnboard()` with `EdgeOnboardingComponentTypeAgent` Uses generated config path from onboarding result if available Logs SPIFFE ID when onboarding is performed	+21/-2
main.go `Edge onboarding integration for dusk checker` cmd/checkers/dusk/main.go Adds command-line flags for onboarding token and KV endpoint Calls `edgeonboarding.TryOnboard()` with `EdgeOnboardingComponentTypeChecker` Uses generated config path from onboarding result if available Logs SPIFFE ID when onboarding is performed	+17/-0

Configuration changes

1 files

app.go

Wire service registry and event publisher to API server

cmd/core/app/app.go

Added api.WithServiceRegistry(server.ServiceRegistry) option to pass
service registry to API server
Added api.WithEventPublisher(server.EventPublisher()) option to pass
event publisher to API server

+2/-0

Additional files

63 files

issues.jsonl	+2/-1
serviceradar.db-shm	[link]
serviceradar.db-wal	[link]
EDGE_ONBOARDING_STATUS.md	+345/-0
BUILD.bazel	+6/-0
BUILD.bazel	+1/-0
main.go	+16/-0
main.go	+19/-0
BUILD.bazel	+4/-1
Cargo.toml	+1/-0
default_template.json	+24/-0
lib.rs	+1/-0
main.rs	+6/-0
template.rs	+69/-0
BUILD.bazel	+1/-0
main.go	+21/-3
BUILD.bazel	+2/-0
main.go	+17/-0
main.go	+17/-0
main.go	+16/-0
main.go	+19/-3
AGENTS.md	+495/-0
checker-template-registration.md	+467/-0
onboarding-review-2025.md	+881/-0
service-registry-design.md	+1058/-0
service-registry-status.md	+450/-0
edge-poller.env	+9/-9
configmap.yaml	+7/-7
serviceradar-db-event-writer-config.yaml	+4/-0
serviceradar-kong.yaml	+2/-14
types.go	+1/-0
processor_test.go	+52/-1
BUILD.bazel	+3/-2
BUILD.bazel	+11/-5
edge_onboarding_test.go	+2/-0
interfaces.go	+11/-0
service_registry_adapter.go	+54/-0
types.go	+1/-0
BUILD.bazel	+4/-0
types.go	+9/-0
db.go	+11/-4
interfaces.go	+2/-0
00000000000001_consolidated_serviceradar_schema.up.sql	+0/-2
00000000000009_service_registry.up.sql	+184/-0
BUILD.bazel	+19/-0
README.md	+352/-0
BUILD.bazel	+3/-0
edge_onboarding.go	+1/-0
events.go	+14/-0
service_device.go	+44/-0
BUILD.bazel	+8/-2
lib.rs	+8/-4
upstream-join-token	+1/-0
page.tsx	+34/-4
route.ts	+59/-0
route.ts	+96/-0
route.ts	+59/-0
Dashboard.tsx	+18/-1
DeleteDeviceButton.tsx	+179/-0
DeviceDetail.tsx	+83/-5
DeviceTable.tsx	+23/-11
DeviceTypeIndicator.tsx	+173/-0
ServiceRegistryPanel.tsx	+286/-0

Imported from GitHub pull request. Original GitHub pull request: #1916 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1916 Original created: 2025-11-03T00:59:10Z Original updated: 2025-11-03T04:53:25Z Original head: carverauto/serviceradar:1915-create-common-onboarding-library-to-eliminate-edge-deployment-friction Original base: main Original merged: 2025-11-03T04:52:34Z by @mfreeman451 --- ### **User description** ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test? ___ ### **PR Type** Enhancement, Tests ___ ### **Description** - Implements comprehensive edge onboarding library to eliminate deployment friction for edge-deployed services (pollers, agents, checkers) - Creates service registry infrastructure with registration, lifecycle management, and heartbeat tracking for pollers, agents, and checkers - Adds device registry integration for service components with auto-registration and tombstone filtering for deleted devices - Implements SPIRE credential configuration and deployment environment detection (Kubernetes, Docker, bare-metal) for edge services - Integrates edge onboarding into all service components (poller, agent, checkers, datasvc, consumers) with automatic configuration generation - Adds DataSvc Core registration mechanism with periodic heartbeat updates via gRPC - Extends API server with new endpoints for device registry queries, agent discovery, datasvc instance listing, and device deletion - Implements device lifecycle event publishing for audit trail and lifecycle tracking - Adds Kong configuration support for web service routing to device endpoints - Updates database layer with agent discovery methods and poller registration fields - Regenerates mocks with standardized parameters and new edge onboarding support ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Edge Services Poller/Agent/Checker"] -->|TryOnboard| B["Bootstrapper"] B -->|Download| C["Package"] B -->|Configure| D["SPIRE"] B -->|Generate Config| E["Service Config"] C -->|Register| F["Service Registry"] F -->|Track| G["Poller/Agent/Checker"] G -->|Heartbeat| H["Core Server"] H -->|Device Update| I["Device Registry"] I -->|Lifecycle Event| J["Event Publisher"] H -->|API Endpoints| K["Agent/DataSvc/Device APIs"] ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td>Tests</td><td><details><summary>5 files</summary><table> <tr> <td> <details> <summary>mock_db.go<dd><code>Regenerate mocks with standardized parameters and edge onboarding </code> <code>support</code></dd></summary> <hr> pkg/db/mock_db.go <ul><li>Regenerated mock file with updated mockgen command path to include <code>pkg/db/</code> prefix <li> Removed <code>isgomock</code> struct fields from mock types (<code>MockService</code>, <code>MockSysmonMetricsProvider</code>, <code>MockRows</code>, <code>MockQueryExecutor</code>) <li> Standardized all mock method parameter names from semantic names (e.g., <code>ctx</code>, <code>pollerID</code>) to generic names (e.g., <code>arg0</code>, <code>arg1</code>) <li> Added new mock methods for edge onboarding functionality: <code>DeleteEdgeOnboardingPackage</code>, <code>GetEdgeOnboardingPackage</code>, <code>InsertEdgeOnboardingEvent</code>, <code>ListEdgeOnboardingEvents</code>, <code>ListEdgeOnboardingPackages</code>, <code>ListEdgeOnboardingPollerIDs</code> <li> Added new mock methods for agent management: <code>ListAgentsByPoller</code>, <code>ListAgentsWithPollers</code></ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-30e38f888d4849fc40d7ebb1559c2a84c43aa8cd13b3b89fd7ec6cf873b243c7">+394/-368</a></td> </tr> <tr> <td> <details> <summary>service_device_test.go<dd><code>Device registration test coverage for all service types</code>    </dd></summary> <hr> pkg/registry/service_device_test.go <ul><li>Comprehensive test suite for device registration functionality <li> Tests poller, agent, and checker device updates with metadata validation <li> Validates device ID generation and uniqueness across service types <li> Tests high-cardinality scenarios (100+ checkers) and mixed batch operations <li> Verifies service devices with empty IPs are allowed while network devices require IPs</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-2996ff7907c1495651fb6a345130a6ebdca6d8d03558e0d9e33542b3acebf2be">+462/-0</a>  </td> </tr> <tr> <td> <details> <summary>service_device_test.go<dd><code>Unit tests for service device model functions</code>                        </dd></summary> <hr> pkg/models/service_device_test.go <ul><li>Tests for service device ID generation and validation functions <li> Validates <code>IsServiceDevice</code> detection logic for poller/agent/checker prefixes <li> Tests network device ID generation with partition support <li> Verifies device ID uniqueness across different service types <li> Tests high-cardinality scenarios and multiple services on same IP</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-6b85e0c22bc48ca6678b23cd683d6b8b5dee9d20f7a8e822d6d13502460f3689">+400/-0</a>  </td> </tr> <tr> <td> <details> <summary>edge_onboarding_test.go<dd><code>Update edge onboarding service test signatures</code>                      </dd></summary> <hr> pkg/core/edge_onboarding_test.go <ul><li>Updates all test calls to <code>newEdgeOnboardingService()</code> to include new <code>nil</code> parameter <li> Maintains consistency across 12 test functions with updated function signature</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-83ba269452783436d693c0f17ab42c959efdef44c7cf6ec27301dd1b0e7d5744">+15/-15</a>  </td> </tr> <tr> <td> <details> <summary>registry_test.go<dd><code>Tests for device tombstone filtering logic</code>                              </dd></summary> <hr> pkg/registry/registry_test.go <ul><li>Adds test <code>TestProcessBatchDeviceUpdates_DropsSelfReportedAfterDelete()</code> verifying self-reported updates are dropped for deleted devices <li> Adds test <code>TestProcessBatchDeviceUpdates_AllowsFreshNonSelfReportedAfterDelete()</code> verifying fresh non-self-reported updates bypass deletion filter <li> Tests tombstone filtering logic with mock database and deletion timestamp metadata</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-f010972d104404be52d2a8e6e784cb56e31194f90795a69571a12696bcbdc075">+124/-0</a>  </td> </tr> </table></details></td></tr><tr><td>Enhancement</td><td><details><summary>32 files</summary><table> <tr> <td> <details> <summary>edge_onboarding.go<dd><code>Add service registry integration and checker template support</code></dd></summary> <hr> pkg/core/edge_onboarding.go <ul><li>Added <code>ServiceManager</code> interface and registration type structs (<code>PollerRegistration</code>, <code>AgentRegistration</code>, <code>CheckerRegistration</code>) to avoid import cycles <li> Added <code>ErrUnsupportedComponentType</code> error constant for unsupported component types <li> Extended <code>edgeOnboardingService</code> with <code>deviceRegistryCallback</code> and <code>serviceRegistry</code> fields <li> Added <code>SetDeviceRegistryCallback</code> method to register device registry callbacks <li> Implemented <code>registerServiceComponent</code> method to register components in the service registry based on type <li> Enhanced <code>CreatePackage</code> to inject <code>datasvc_endpoint</code> into metadata and register services <li> Added <code>markServiceDeviceUnavailable</code> method to emit tombstone updates when packages are revoked <li> Implemented checker-specific KV handling with template substitution in <code>applyComponentKVUpdates</code> <li> Added <code>putKVDocument</code>, <code>substituteTemplateVariables</code>, and <code>substituteInMap</code> helper methods for template variable replacement <li> Updated <code>kvKeyForPackage</code> to use correct KV path for checker configs (<code>agents/{agent_id}/checkers/{checker_kind}.json</code>)</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-85874e3c4bdcc9110db09909f10648d44cdee554b26c987f910502321eb20b5c">+348/-8</a>  </td> </tr> <tr> <td> <details> <summary>edge_onboarding.go<dd><code>Add DataSvc endpoint configuration to edge package creation</code></dd></summary> <hr> pkg/core/api/edge_onboarding.go <ul><li>Added <code>DataSvcEndpoint</code> field to <code>edgePackageCreateRequest</code> struct for DataSvc gRPC endpoint configuration <li> Updated <code>handleCreateEdgePackage</code> to extract and trim <code>DataSvcEndpoint</code> from request and pass to <code>CreatePackage</code></ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-c494568149a0b526ca5349c1d78d5e5ee7ec4144ecfd324c44f25fef584292bc">+2/-0</a>      </td> </tr> <tr> <td> <details> <summary>service_registry.go<dd><code>Core service registry implementation with registration and lifecycle </code> <code>management</code></dd></summary> <hr> pkg/registry/service_registry.go <ul><li>Implements core <code>ServiceRegistry</code> struct managing lifecycle of pollers, agents, and checkers <li> Provides registration methods (<code>RegisterPoller</code>, <code>RegisterAgent</code>, <code>RegisterChecker</code>) with validation and event emission <li> Implements heartbeat recording with auto-activation of pending services <li> Includes service deletion and purge functionality for inactive/revoked services <li> Uses ClickHouse/Proton batch operations for efficient database writes</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-d6d6d09a58edde934a1d3f571ff5ce02f0475c5e85ecdec27888892aff8d9d1d">+752/-0</a>  </td> </tr> <tr> <td> <details> <summary>service_registry_queries.go<dd><code>Query and retrieval operations for service registry</code>            </dd></summary> <hr> pkg/registry/service_registry_queries.go <ul><li>Implements query methods for retrieving registered services (<code>GetPoller</code>, <code>GetAgent</code>, <code>GetChecker</code>) <li> Provides list operations with filtering by status and registration source <li> Implements <code>IsKnownPoller</code> with TTL-based caching for performance <li> Includes status update and service lifecycle management queries <li> Supports hierarchical queries (agents by poller, checkers by agent)</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-3591e8646384be68811f862b986342253cabc23176c6f0e09996453baf88a2e9">+679/-0</a>  </td> </tr> <tr> <td> <details> <summary>pollers.go<dd><code>Service registry integration into poller status handling</code>  </dd></summary> <hr> pkg/core/pollers.go <ul><li>Integrates service registry auto-registration on poller heartbeat <li> Adds device registration for pollers, agents, and checkers <li> Implements <code>ensurePollerRegistered</code>, <code>ensureAgentRegistered</code>, <code>ensureCheckerRegistered</code> methods <li> Updates <code>isKnownPoller</code> to check service registry as primary path with fallback to legacy methods <li> Adds device registry integration for inventory tracking</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-fe81e2a32f1ac64bcdc6f25f55c5fa918d17bad8c0546f2cf80c757ff4051816">+259/-0</a>  </td> </tr> <tr> <td> <details> <summary>device_registry.go<dd><code>HTTP API endpoints for device registry queries and deletion</code></dd></summary> <hr> pkg/core/api/device_registry.go <ul><li>Implements HTTP API endpoints for device registry information retrieval <li> Provides <code>getDeviceRegistryInfo</code> endpoint to query service registration details <li> Implements <code>deleteDevice</code> endpoint for tombstoning devices with audit trail <li> Includes device type detection and partition extraction utilities <li> Supports user attribution and metadata preservation on deletion</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-34b7da1b2845de83ee2b0eeba93ef1c8b7abf40517f20617c861abffec32ee1c">+310/-0</a>  </td> </tr> <tr> <td> <details> <summary>config.go<dd><code>Edge onboarding configuration generation for service components</code></dd></summary> <hr> pkg/edgeonboarding/config.go <ul><li>Implements configuration generation for edge-deployed services (poller, agent, checker) <li> Generates deployment-specific configuration with SPIFFE identity and SPIRE integration <li> Handles metadata parsing and address resolution for different deployment types <li> Supports storage path configuration and site-specific settings <li> Includes comprehensive error handling for missing required metadata fields</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-e8ebde92155beb028a380b450be87da815cccfb50fede2faf7f10b1785d70f65">+314/-0</a>  </td> </tr> <tr> <td> <details> <summary>service_registration.go<dd><code>Service device update creation helper functions</code>                    </dd></summary> <hr> pkg/models/service_registration.go <ul><li>Implements helper functions for creating device updates for service components <li> <code>CreatePollerDeviceUpdate</code>, <code>CreateAgentDeviceUpdate</code>, <code>CreateCheckerDeviceUpdate</code> functions <li> Generates service-aware device IDs with component type prefixes <li> Automatically adds component-specific metadata to device updates <li> Sets high confidence for self-reported service devices</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-3ad8d9e7f1f17e0198a6a5a53398cc9bcae94a111f907d965dfcc43daeeb95e8">+101/-0</a>  </td> </tr> <tr> <td> <details> <summary>bootstrap.go<dd><code>Core edge onboarding bootstrapper implementation</code>                  </dd></summary> <hr> pkg/edgeonboarding/bootstrap.go <ul><li>Introduces core <code>Bootstrapper</code> struct and <code>Config</code> for edge service onboarding workflow <li> Implements <code>Bootstrap()</code> method orchestrating package download, SPIRE configuration, and service config generation <li> Defines error types and deployment type constants for edge deployments <li> Provides accessor methods for SPIFFE ID, join token, and generated configurations</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-b3b312d7d184427a6fd1bd68408e0af79fd1b1124e18d59878b9b0399f7937e0">+288/-0</a>  </td> </tr> <tr> <td> <details> <summary>spire.go<dd><code>SPIRE credential configuration for edge components</code>              </dd></summary> <hr> pkg/edgeonboarding/spire.go <ul><li>Implements SPIRE credential configuration for pollers (nested server), agents, and checkers <li> Generates SPIRE server and agent HCL configuration templates <li> Handles trust bundle and join token file writing <li> Provides SPIFFE ID extraction and trust domain parsing utilities</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-5d784c136f3db0128ea90d628cbefdff9e2ba18eba9a1775f54fc630bde8ee1d">+247/-0</a>  </td> </tr> <tr> <td> <details> <summary>registry.go<dd><code>Support service components in device registry</code>                        </dd></summary> <hr> pkg/registry/registry.go <ul><li>Adds support for service component device IDs (pollers, agents, checkers) alongside network devices <li> Implements <code>filterObsoleteUpdates()</code> to prevent stale updates for deleted devices <li> Allows empty IPs for service components identified by service-aware device IDs <li> Adds deletion timestamp extraction and bypass logic for tombstoned devices</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-cb61d8f79451b9541de4a8cc0811523a68d15452b2f5971c7618ea5b423cf4ec">+151/-12</a></td> </tr> <tr> <td> <details> <summary>core_registration.go<dd><code>DataSvc Core registration and heartbeat service</code>                    </dd></summary> <hr> pkg/datasvc/core_registration.go <ul><li>Implements background registration and heartbeat mechanism for datasvc with Core service <li> Creates gRPC connection to Core using security provider and SPIFFE mTLS <li> Sends periodic heartbeat updates via <code>ReportStatus</code> RPC <li> Handles SPIFFE ID retrieval from workload API or environment</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-ce69d5d45ea63ae8aae8e6e6badd856c481f439567d2a5611520855fafb75c50">+206/-0</a>  </td> </tr> <tr> <td> <details> <summary>integration.go<dd><code>Edge onboarding integration layer for services</code>                      </dd></summary> <hr> pkg/edgeonboarding/integration.go <ul><li>Provides <code>TryOnboard()</code> entry point for services to attempt edge onboarding via environment variables <li> Implements <code>IntegrationResult</code> struct for returning onboarding artifacts to services <li> Handles generated config file writing and SPIRE workload API socket path retrieval <li> Includes config unmarshaling helper for service integration</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-e0086702d4c86140315e9347dc19ccae52a3eb64b66a15ccfafae8981c7945d8">+206/-0</a>  </td> </tr> <tr> <td> <details> <summary>server.go<dd><code>API server enhancements for service and event management</code>  </dd></summary> <hr> pkg/core/api/server.go <ul><li>Adds <code>WithEventPublisher()</code> and <code>WithServiceRegistry()</code> option functions for API server configuration <li> Adds new routes for device deletion, registry info, datasvc instances, and agent listing <li> Implements <code>handleDeviceByID()</code> router for GET/DELETE methods on device endpoints <li> Adds service registry interface to API server type</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-1bb99367fdd853c728b7cfbf5893a293f6d217144dfb5282cb8dd32e5261021e">+43/-1</a>    </td> </tr> <tr> <td> <details> <summary>kong.go<dd><code>Kong configuration support for web service routing</code>              </dd></summary> <hr> pkg/cli/kong.go <ul><li>Adds <code>web-service</code> flag for optional Web service URL routing to <code>/api/devices</code> endpoints <li> Implements Kong DB-less configuration generation for web service with JWT authentication <li> Adds output directory creation logic to ensure parent directories exist <li> Extends <code>renderKongDBLess()</code> to support separate web service routing</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-860792168fff085b1fca0052d2c6c9eac4d6726b07d7790a0162f803ea2c87c2">+31/-2</a>    </td> </tr> <tr> <td> <details> <summary>deployment.go<dd><code>Deployment environment detection for edge services</code>              </dd></summary> <hr> pkg/edgeonboarding/deployment.go <ul><li>Implements <code>detectDeploymentType()</code> to identify Kubernetes, Docker, or bare-metal environments <li> Provides environment detection helpers (<code>isKubernetes()</code>, <code>isDocker()</code>) checking env vars and filesystem <li> Includes deployment-specific address resolution stubs for SPIRE and service discovery <li> Handles deployment type configuration override from config</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-ec21f17eea21a6ea80f89a2f1d31cabaa524020c0a542eaa3ebf579c5197cd26">+152/-0</a>  </td> </tr> <tr> <td> <details> <summary>pollers.go<dd><code>Poller registration and agent discovery database methods</code>  </dd></summary> <hr> pkg/db/pollers.go <ul><li>Updates <code>insertPollerStatus()</code> to include new poller registration fields (component_id, registration_source, status, etc.) <li> Adds <code>AgentInfo</code> struct for agent-poller relationship queries <li> Implements <code>ListAgentsWithPollers()</code> and <code>ListAgentsByPoller()</code> methods for agent discovery <li> Queries services table to retrieve agent information with service types</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-4e4ff6d32240a5f2e8d053438abcc4a77959d521bc99028ed2bbcf1e07145631">+110/-4</a>  </td> </tr> <tr> <td> <details> <summary>service_models.go<dd><code>Service registry domain models and types</code>                                  </dd></summary> <hr> pkg/registry/service_models.go <ul><li>Defines service lifecycle models: <code>ServiceStatus</code>, <code>RegistrationSource</code>, and registration request types <li> Introduces <code>RegisteredPoller</code>, <code>RegisteredAgent</code>, <code>RegisteredChecker</code> structs for service state <li> Defines <code>ServiceHeartbeat</code>, <code>ServiceFilter</code>, and <code>RegistrationEvent</code> for service management <li> Establishes data structures for service registry operations</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-3f9834ff929a8557454ae9d65656dc2a25134184e36c75afa0e47821e9b40894">+147/-0</a>  </td> </tr> <tr> <td> <details> <summary>datasvc_registry.go<dd><code>DataSvc instance registry API endpoint</code>                                      </dd></summary> <hr> pkg/core/api/datasvc_registry.go <ul><li>Implements <code>handleListDataSvcInstances()</code> endpoint for listing registered datasvc instances <li> Queries services table for datasvc registrations with endpoint and availability information <li> Returns empty list placeholder pending full datasvc registration implementation <li> Includes type assertion and error handling for database connections</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-9218ee6431c62dd960cdde73b650080803141719665b3429d440483f7e05ffe4">+103/-0</a>  </td> </tr> <tr> <td> <details> <summary>events.go<dd><code>Event publishing refactor and device lifecycle events</code>        </dd></summary> <hr> pkg/natsutil/events.go <ul><li>Refactors <code>PublishPollerHealthEvent()</code> to use new <code>publishEvent()</code> helper method <li> Implements <code>PublishDeviceLifecycleEvent()</code> for device lifecycle events (delete, restore, etc.) <li> Adds generic <code>publishEvent()</code> method handling stream creation and event marshaling <li> Supports device lifecycle event data with severity and level fields</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-3503010fc6a66fb16b8fbc055b7daf53be305d7284efe58d37a7fbf1813a6b63">+65/-27</a>  </td> </tr> <tr> <td> <details> <summary>interfaces.go<dd><code>Service manager interface definition</code>                                          </dd></summary> <hr> pkg/registry/interfaces.go <ul><li>Adds <code>ServiceManager</code> interface defining service registration and lifecycle operations <li> Includes methods for registering pollers, agents, and checkers <li> Defines heartbeat recording, service queries, and status management operations <li> Adds mock generation directive for <code>ServiceManager</code> interface</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-21496eb3d34ec02e874f36c98cdd2ca7f39130369caf774c672dcf4be1619503">+63/-0</a>    </td> </tr> <tr> <td> <details> <summary>server.go<dd><code>Core server service registry initialization</code>                            </dd></summary> <hr> pkg/core/server.go <ul><li>Initializes <code>ServiceRegistry</code> in <code>NewServer()</code> with type assertion to <code>*db.DB</code> <li> Creates service registry adapter to avoid import cycles <li> Adds device registry callback for service cleanup on package revocation <li> Implements <code>EventPublisher()</code> getter method for accessing event publisher</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-a12d34beb8640efbbedad28f8610fd58afeea457572d82e4fd295ab2a6bb8ee6">+32/-1</a>    </td> </tr> <tr> <td> <details> <summary>types.go<dd><code>API server type enhancements for service and event support</code></dd></summary> <hr> pkg/core/api/types.go <ul><li>Adds <code>SetDeviceRegistryCallback()</code> method to <code>EdgeOnboardingService</code> interface <li> Adds <code>serviceRegistry</code> field to <code>APIServer</code> struct for service management <li> Adds <code>eventPublisher</code> field to <code>APIServer</code> struct for lifecycle events <li> Imports <code>natsutil</code> package for event publishing support</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-39f024121630282f9e3eeee5b77e5a63a87950b9eae4f479277a289796b42d56">+4/-0</a>      </td> </tr> <tr> <td> <details> <summary>services.go<dd><code>Auto-registration of agents and checkers as devices</code>            </dd></summary> <hr> pkg/core/services.go <ul><li>Adds agent auto-registration as device when <code>AgentId</code> is present in service status <li> Adds checker auto-registration as device for checker service types (snmp, sysmon, rperf, mapper) <li> Implements <code>isCheckerService()</code> to identify checker service types <li> Implements <code>generateCheckerID()</code> to create stable checker identifiers <li> Calls service registry methods for agent and checker registration</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-b75091f9768dcdaf46aedeee40cb2eaa33b46a484d77d5d432bab19fe437237f">+59/-0</a>    </td> </tr> <tr> <td> <details> <summary>server.go<dd><code>DataSvc logging and Core registration initialization</code>          </dd></summary> <hr> pkg/datasvc/server.go <ul><li>Adds <code>logger</code> field to datasvc <code>Server</code> struct <li> Implements <code>datasvcLogger</code> wrapper adapting zerolog to <code>logger.Logger</code> interface <li> Initializes logger in <code>NewServer()</code> with service name and timestamp context <li> Calls <code>StartCoreRegistration()</code> in <code>Start()</code> method for Core registration</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-ca4e0dbf349806d8de2c79e953a7c2bb55bce593f8818613dc92c55ca7411195">+43/-1</a>    </td> </tr> <tr> <td> <details> <summary>agent_registry.go<dd><code>Agent registry API endpoints</code>                                                          </dd></summary> <hr> pkg/core/api/agent_registry.go <ul><li>Implements <code>handleListAgents()</code> endpoint for retrieving all agents with poller associations <li> Implements <code>handleListAgentsByPoller()</code> endpoint for filtering agents by poller ID <li> Defines <code>agentInfoView</code> struct for API responses with agent, poller, and service type information <li> Includes error handling and JSON response formatting</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-0df374b9f39280c9e16603d3dfb872be6d49eaef78507a9c26a2bc9610cbb041">+72/-0</a>    </td> </tr> <tr> <td> <details> <summary>main.go<dd><code>Edge onboarding integration for db-event-writer consumer</code>  </dd></summary> <hr> cmd/consumers/db-event-writer/main.go <ul><li>Adds command-line flags for onboarding token and KV endpoint <li> Calls <code>edgeonboarding.TryOnboard()</code> to attempt edge onboarding before loading config <li> Uses generated config path from onboarding result if available <li> Logs SPIFFE ID when onboarding is performed</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-c9a73828b631e4618af51a47bc4c618d72ad1726fef3c3cbe12ab73b57b0eb63">+23/-4</a>    </td> </tr> <tr> <td> <details> <summary>processor.go<dd><code>Device lifecycle event processing</code>                                                </dd></summary> <hr> pkg/consumers/db-event-writer/processor.go <ul><li>Adds <code>tryDeviceLifecycleEvent()</code> handler for device lifecycle CloudEvents <li> Extracts device lifecycle event data and populates event row fields <li> Maps device action, actor, severity, and timestamp to GELF format <li> Handles device lifecycle events in event processing pipeline</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-c55d73b621975e3797271d69fc43b78fa44eb184437392f9e40e18d4568589a8">+52/-0</a>    </td> </tr> <tr> <td> <details> <summary>unified_device.go<dd><code>Device model support for service components</code>                            </dd></summary> <hr> pkg/models/unified_device.go <ul><li>Adds <code>DiscoverySourceServiceRadar</code> constant for ServiceRadar infrastructure components <li> Adds <code>ServiceType</code> and <code>ServiceID</code> fields to <code>DeviceUpdate</code> struct for service component identification <li> Updates <code>GetSourceConfidence()</code> to return high confidence for ServiceRadar source <li> Reformats discovery source constants for consistency</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-e05758fa3d62acc9613f0b1191e034c8d4d115ec85debcbe9bda2968d34adaba">+16/-11</a>  </td> </tr> <tr> <td> <details> <summary>main.go<dd><code>Edge onboarding integration for sysmon-vm checker</code>                </dd></summary> <hr> cmd/checkers/sysmon-vm/main.go <ul><li>Adds command-line flags for onboarding token and KV endpoint <li> Calls <code>edgeonboarding.TryOnboard()</code> with <code>EdgeOnboardingComponentTypeChecker</code> <li> Uses generated config path from onboarding result if available <li> Logs SPIFFE ID when onboarding is performed</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-b1ab49590901666d7536778838bbf3eda1cac7c33257e8e89cb3ab74df8ae376">+17/-0</a>    </td> </tr> <tr> <td> <details> <summary>main.go<dd><code>Edge onboarding integration for faker agent</code>                            </dd></summary> <hr> cmd/faker/main.go <ul><li>Adds command-line flags for onboarding token and KV endpoint <li> Calls <code>edgeonboarding.TryOnboard()</code> with <code>EdgeOnboardingComponentTypeAgent</code> <li> Uses generated config path from onboarding result if available <li> Logs SPIFFE ID when onboarding is performed</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-f101715b3bafbe3843ac9df0ffa3566fd0278b8f730efa3665557ce98acf6d23">+21/-2</a>    </td> </tr> <tr> <td> <details> <summary>main.go<dd><code>Edge onboarding integration for dusk checker</code>                          </dd></summary> <hr> cmd/checkers/dusk/main.go <ul><li>Adds command-line flags for onboarding token and KV endpoint <li> Calls <code>edgeonboarding.TryOnboard()</code> with <code>EdgeOnboardingComponentTypeChecker</code> <li> Uses generated config path from onboarding result if available <li> Logs SPIFFE ID when onboarding is performed</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-2beb066906fd7a77f5812c5dcdb294a4886c9c2943378f49bce239cfe51b8035">+17/-0</a>    </td> </tr> </table></details></td></tr><tr><td>Configuration changes</td><td><details><summary>1 files</summary><table> <tr> <td> <details> <summary>app.go<dd><code>Wire service registry and event publisher to API server</code>    </dd></summary> <hr> cmd/core/app/app.go <ul><li>Added <code>api.WithServiceRegistry(server.ServiceRegistry)</code> option to pass service registry to API server <li> Added <code>api.WithEventPublisher(server.EventPublisher())</code> option to pass event publisher to API server</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-4ad8a289575edf3b163088617b7a40ae1305c29ced0c7d59b3751c57d6938072">+2/-0</a>      </td> </tr> </table></details></td></tr><tr><td>Additional files</td><td><details><summary>63 files</summary><table> <tr> <td>issues.jsonl</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-364c90732b0e821bfb3467071cccb7130acddf44aaea27ad1e2ae5eb94c0150d">+2/-1</a>      </td> </tr> <tr> <td>serviceradar.db-shm</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-2264bf0ee1a10234bc8655dc737312b67ffe66bb260369c7f7a443fc31deef9a">[link]</a>    </td> </tr> <tr> <td>serviceradar.db-wal</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-4dd28774bad48dbc031a276aac44d025fc297f85a894eec42e0ebaa28dbfb794">[link]</a>    </td> </tr> <tr> <td>EDGE_ONBOARDING_STATUS.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-9e9386605878fe4ad9cf63b382932350495fd6d6351b378a613104d4bd18673b">+345/-0</a>  </td> </tr> <tr> <td>BUILD.bazel</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-884fa9353a5226345e44fbabea3300efc7a87dfbcde0b6a42521ca51823f1b68">+6/-0</a>      </td> </tr> <tr> <td>BUILD.bazel</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-143f8d1549d52f28906f19ce28e5568a5be474470ff103c2c1e63c3e6b08d670">+1/-0</a>      </td> </tr> <tr> <td>main.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-61358711e980ccf505246fd3915f97cbd3a380e9b66f6fa5aad46749968c5ca3">+16/-0</a>    </td> </tr> <tr> <td>main.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-f25402eade63525184cb5e7437accff93c7b9338eebe81add6dc5f2a9eb12550">+19/-0</a>    </td> </tr> <tr> <td>BUILD.bazel</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-6ab6e69dfb3cd621d100077fa496690634adb5fcd88806f891575024f1835480">+4/-1</a>      </td> </tr> <tr> <td>Cargo.toml</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-6f8e9ee62fdd4b2de3f99c8760f26dcb7c0b1e6b83ade0a888898d23f6abcbf3">+1/-0</a>      </td> </tr> <tr> <td>default_template.json</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-13a2206be25eb545a343eebfa161a2d5e78148a9c1308ac729b6b3452e1cbe8e">+24/-0</a>    </td> </tr> <tr> <td>lib.rs</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-e7e8897bb436806d2e6fb61aa2bdf32e7bf6c97915595d0e0987e43c725efad5">+1/-0</a>      </td> </tr> <tr> <td>main.rs</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-cef134403622089f759cfa006eb04dc8f1cfda08497a9d38586cc4de03d14820">+6/-0</a>      </td> </tr> <tr> <td>template.rs</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-6c350f1b07294809ab71f3f412c118374e069d1e74ec50dc150604371d195283">+69/-0</a>    </td> </tr> <tr> <td>BUILD.bazel</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-ba4bbabb721d98056e2dc9a701e251b851b58e572cb720010b7c8fe7c66b4f74">+1/-0</a>      </td> </tr> <tr> <td>main.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-7e1a99c83e5e5c2d5deec445135861c820d0aeddb2d3b5365e24b4c3b6955f3a">+21/-3</a>    </td> </tr> <tr> <td>BUILD.bazel</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-c62c0139ebdb337369f4067567cd2c52b8e7decb3ddfabc77f9f67b2f6e5789c">+2/-0</a>      </td> </tr> <tr> <td>main.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-5e7731adfb877918cd65d9d5531621312496450fd550fea2682efca4ca8fe816">+17/-0</a>    </td> </tr> <tr> <td>main.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-1b7aa67fec348b5072091f561abc40b75a0a18b660af797c9efcf90803840e39">+17/-0</a>    </td> </tr> <tr> <td>main.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-4b8ec845da50cd58d011e69f9d1c30530ee1968df26616b8768bb1fc03433bbe">+16/-0</a>    </td> </tr> <tr> <td>main.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-78dc6bc53f1c760c66f43ff5f486bfe78a65bee8b2e0d4862293ec0892da2b29">+19/-3</a>    </td> </tr> <tr> <td>AGENTS.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-d00743dc4e878f258330deadcd4008e361276d760b58b672150c54bb1ac9758a">+495/-0</a>  </td> </tr> <tr> <td>checker-template-registration.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-592a3d8a2648917d208e668b887122e9cdb33acb7af4ed99c6a506d5d9ad7c02">+467/-0</a>  </td> </tr> <tr> <td>onboarding-review-2025.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-1728c73ae8e684d13b7b166b90cbfc5363c0794dcd63f7fa3733e1c624940e98">+881/-0</a>  </td> </tr> <tr> <td>service-registry-design.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-eeff62de7c13c07e98f187ab6b5173585e47ab26969ddae687e69783483116fa">+1058/-0</a></td> </tr> <tr> <td>service-registry-status.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-14b25655121cc9d193e18e37912ff77cc784ac513d53ec8240b45d440bd72575">+450/-0</a>  </td> </tr> <tr> <td>edge-poller.env</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-7ea544cfb575c0bd12077e5d92984bd84f3241ee441dce8a683af20ab2cd543f">+9/-9</a>      </td> </tr> <tr> <td>configmap.yaml</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-f4548beaa0a3a01a46971c82c5647a0f3f49eb38d66dd939d06d19018173fcd6">+7/-7</a>      </td> </tr> <tr> <td>serviceradar-db-event-writer-config.yaml</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-57ddd4cb03c5c669ff6dc5b3b60334e9205d01ae95a2b5316c147ed2114ebe49">+4/-0</a>      </td> </tr> <tr> <td>serviceradar-kong.yaml</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-e1426b570f5caae8eee31f02984392f1bc68d0c329ae2f1118de3272d654856e">+2/-14</a>    </td> </tr> <tr> <td>types.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-26b14ac5781e43bc8b1fd47ab9430976f17a1a25ca5df12c6acf869d2518e7d5">+1/-0</a>      </td> </tr> <tr> <td>processor_test.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-f31604ff27a484e1f24302cdbd1fa3e7610da6ee10e76d990bba5a72bf1bb9ad">+52/-1</a>    </td> </tr> <tr> <td>BUILD.bazel</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-af133ce6c45ec51c66bcb5fe0b424291d78688285b614275765003b83c177534">+3/-2</a>      </td> </tr> <tr> <td>BUILD.bazel</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-ee91bfc5d078aab2befc64f89c36dca0de7342465f414ecd70292d092153bfcc">+11/-5</a>    </td> </tr> <tr> <td>edge_onboarding_test.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-ff98b520d3df871ff219eb56ab1a2ac2d55afb266b87393bd63360635ba02880">+2/-0</a>      </td> </tr> <tr> <td>interfaces.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-ed4bf67095345b7e387caeb64a5033a276047cf9d7ea1c0f3fe215a8bbfa14f4">+11/-0</a>    </td> </tr> <tr> <td>service_registry_adapter.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-cfce7f6079001e11903e4e7992f09385c1c72d33bd47194971ee7a207586f1ce">+54/-0</a>    </td> </tr> <tr> <td>types.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-717f128517472d3dc4091e67bfc0f4fe4c36e32096c5ef87d78f34cbc64d2399">+1/-0</a>      </td> </tr> <tr> <td>BUILD.bazel</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-1ab2c84c6a180159fcd99071e948ec1b7b2006f988db0981b8ee1f507624d9f3">+4/-0</a>      </td> </tr> <tr> <td>types.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-4a380e5b61f591d2764cd7d10c67e3ba1e844b93addb3c686932ffa866bac662">+9/-0</a>      </td> </tr> <tr> <td>db.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-5da3684806835246d262230050593f460b12b6c0e3966df174e6061be0e9e575">+11/-4</a>    </td> </tr> <tr> <td>interfaces.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-c230fe0c315251837357bfde4ae7f7b34080398d8e48af6bf78badb2124271f3">+2/-0</a>      </td> </tr> <tr> <td>00000000000001_consolidated_serviceradar_schema.up.sql</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-1e05de747238f2112bb2230aac8db388e4c80eebe84c071eb78e035d64e67eb6">+0/-2</a>      </td> </tr> <tr> <td>00000000000009_service_registry.up.sql</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-75bf0dc09f4f1f526604ab35b5cfbb03593387e332323a505068cf1db09dd98f">+184/-0</a>  </td> </tr> <tr> <td>BUILD.bazel</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-8ee5f905776b79c7b337eb0f2cb906a6dda3252cde9459e4c2acc9a53dd0d8ba">+19/-0</a>    </td> </tr> <tr> <td>README.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-d7619f1ddb599cfda59aaaed573a6d3c96da82293bd5cddfe1ddbf96cabc5d31">+352/-0</a>  </td> </tr> <tr> <td>BUILD.bazel</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-36fef0a2d38bbe571cc0b8943b3465eff547b0d47c666777ac84c7a8dd3473aa">+3/-0</a>      </td> </tr> <tr> <td>edge_onboarding.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-0c66da94363b25ecb9e4766c8f12cde387c1874aeb9099447c609c4e867b4fe0">+1/-0</a>      </td> </tr> <tr> <td>events.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-8430b0f58da1cbe15deac7e09bb4acaa1b1e3f2e8772d883a7df24ef6173fa2b">+14/-0</a>    </td> </tr> <tr> <td>service_device.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-f6d3995ae69f2f6d1e7f2c68e4c8a50759aebb26df3149af4cde2a05a018f14c">+44/-0</a>    </td> </tr> <tr> <td>BUILD.bazel</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-3e838afce9a84935e04b7ff8fd3e48d5452c21538a3ea1d36e3fd00aa3c30cd0">+8/-2</a>      </td> </tr> <tr> <td>lib.rs</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-91f4558d22540a64796ec2ad9844eaaec577d90b0d8d2738eea0d2041837ead7">+8/-4</a>      </td> </tr> <tr> <td>upstream-join-token</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-f33542de4d19f92570e0975ae78dcb21af38e9211df87d74011686c75226298b">+1/-0</a>      </td> </tr> <tr> <td>page.tsx</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-d627c4ba6a102a46a28024061371276f01053b63f20c75986aef2631b967bafc">+34/-4</a>    </td> </tr> <tr> <td>route.ts</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-c1db4e1be2726a7a59a3f56cfc09581699792a21ad80be938bc06a6561c57edc">+59/-0</a>    </td> </tr> <tr> <td>route.ts</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-ea725df24de22d7f31e66dc5258e45d8970aa1cf0fdbe46a752e3929b8a5e228">+96/-0</a>    </td> </tr> <tr> <td>route.ts</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-8383b3b401bd9c9f9d5bf42926d8d8fa3cfb99c5e0346b349db92e17a8ad0b92">+59/-0</a>    </td> </tr> <tr> <td>Dashboard.tsx</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-da1f03ea181f8f19b672164eb86b914cef8bcef6ea7df61d174e3c19421a3461">+18/-1</a>    </td> </tr> <tr> <td>DeleteDeviceButton.tsx</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-e9974d6c840555621408d6ea6dd484eab600772e553a7aed2f95d6d5ca153483">+179/-0</a>  </td> </tr> <tr> <td>DeviceDetail.tsx</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-2dbb02c57d308332683ec3795ef41e65387b5d1c30c559f01b535b5e704ba872">+83/-5</a>    </td> </tr> <tr> <td>DeviceTable.tsx</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-62d65ad4e929bc5554024981aa1c3418d70d54333e3afd80af69d93cea2edec3">+23/-11</a>  </td> </tr> <tr> <td>DeviceTypeIndicator.tsx</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-70083a16b5737a5fe30eaf78e6d1e6a1ce6a3f42b13de4cb128fcf820196abb3">+173/-0</a>  </td> </tr> <tr> <td>ServiceRegistryPanel.tsx</td> <td><a href="https://github.com/carverauto/serviceradar/pull/1916/files#diff-284ce670567ea39a0afa54761daa95d9c4ddb90dec4dfdc906371d304a9bbd5d">+286/-0</a>  </td> </tr> </table></details></td></tr></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2025-11-03 01:00:48 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1916#issuecomment-3478615809
Original created: 2025-11-03T01:00:48Z

PR Compliance Guide 🔍

(Compliance updated until commit `github.com/carverauto/serviceradar@5760e8406d`)

Below is a summary of compliance checks for this PR:

Security Compliance

⚪

Authorization/Access control

Description: The DELETE device endpoint allows tombstoning any device by ID without explicit RBAC
checks shown in the handler, potentially enabling unauthorized deletion if upstream auth
middleware is misconfigured.
device_registry.go [156-287]

Referred Code

func (s *APIServer) deleteDevice(w http.ResponseWriter, r *http.Request) {
	vars := mux.Vars(r)
	deviceID := vars["id"]

	if deviceID == "" {
		writeError(w, "Device ID is required", http.StatusBadRequest)
		return
	}

	if s.dbService == nil {
		writeError(w, "Database not configured", http.StatusInternalServerError)
		return
	}

	now := time.Now().UTC()
	ctx := r.Context()
	partition := partitionFromDeviceID(deviceID)

	update := &models.DeviceUpdate{
		DeviceID:    deviceID,
		Partition:   partition,



 ... (clipped 111 lines)

Template injection risk

Description: Template variable substitution for checker configs performs simple string replacement
which could allow injecting unintended values into JSON configuration if templates include
sensitive fields.
edge_onboarding.go [1542-1611]

Referred Code

	return err
}

componentType := pkg.ComponentType
if componentType == models.EdgeOnboardingComponentTypeNone {
	componentType = models.EdgeOnboardingComponentTypePoller
}

// For checkers, write the checker config directly to KV
// The agent expects the checker config JSON at agents/{agent_id}/checkers/{checker_kind}.json
if componentType == models.EdgeOnboardingComponentTypeChecker {
	// Check if instance config already exists to prevent overwriting user modifications
	existing, err := s.kvClient.Get(ctx, &proto.GetRequest{Key: key})
	if err != nil {
		return fmt.Errorf("edge onboarding: failed to check existing config: %w", err)
	}

	if existing.GetFound() {
		// Config already exists, don't overwrite it
		// Just store the revision and return
		pkg.KVRevision = existing.GetRevision()



 ... (clipped 49 lines)

Destructive delete behavior

Description: Hard DELETE operations directly remove rows from versioned_kv tables which may bypass
audit/history expectations; if exposed, could aid tampering or data loss.
service_registry.go [659-706]

Referred Code

// Verify service is not active or pending
var status string
var query string
var source string

switch serviceType {
case serviceTypePoller:
	query = `SELECT status, registration_source FROM pollers WHERE poller_id = ? LIMIT 1`
case serviceTypeAgent:
	query = `SELECT status, registration_source FROM agents WHERE agent_id = ? LIMIT 1`
case serviceTypeChecker:
	query = `SELECT status, registration_source FROM checkers WHERE checker_id = ? LIMIT 1`
default:
	return fmt.Errorf("%w: %s", ErrUnknownServiceType, serviceType)
}

row := r.db.Conn.QueryRow(ctx, query, serviceID)
if err := row.Scan(&status, &source); err != nil {
	return fmt.Errorf("service not found: %w", err)
}




 ... (clipped 27 lines)

Ticket Compliance

🟡

🎫 #1915

🟢	Implement service registry with registration, lifecycle management, and heartbeats for pollers/agents/checkers.
	Replace ConfigMap-based known pollers with KV/database-backed approach and modify isKnownPoller() to check KV/database.
	Publish device lifecycle events and provide API endpoints for device registry queries and device deletion.
	Auto-register services (poller/agent/checker) with Core via KV/database and maintain dynamic config in KV.
	Handle checker config via KV with template substitution and per-agent checker config keys.
	Regenerate/add unit tests for registry/device publishing behaviors.
🔴	Add allowed_poller_id to edge_packages and ensure package creation populates it; use statuses Issued/Delivered/Activated for allowlist.
	Create edge-checker-stack.compose.yml and ensure main docker-compose is unchanged; document differences.
	Add CLI edge commands for package lifecycle; fix API authentication; document endpoints; consider service account tokens.
	Remove legacy shell scripts and update edge documentation and migration guides.
⚪	Create a common onboarding library pkg/edgeonboarding to simplify edge onboarding for pollers, agents, and checkers using a token-based flow.
	Handle package download, validation, and extraction for edge onboarding.
	Configure nested SPIRE credentials specifically for edge (not k8s SPIRE) and support rotation.
	Detect deployment type (docker vs bare-metal) and generate appropriate configuration.
	Integrate onboarding library into poller, agent, and checkers; support ONBOARDING_TOKEN env var; do not modify k8s or main docker-compose logic.
	Add API endpoints for agent discovery and datasvc listing.
	Documentation updates across edge deployment guides and differences between stacks.

Codebase Duplication Compliance

⚪

Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance

🟢

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

⚪

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing audit context: Registration/heartbeat/deletion events are emitted and logged but lack explicit inclusion
of user identifiers and full action context in all paths (e.g., auto-registration uses
actor "system"), which may be compliant but needs verification of upstream
logging to ensure audit trail completeness.

Referred Code

	eventMetadata := map[string]string{
		"component_id": reg.ComponentID,
	}
	if reg.SPIFFEIdentity != "" {
		eventMetadata["spiffe_id"] = reg.SPIFFEIdentity
	}
	if err := r.emitRegistrationEvent(ctx, "registered", serviceTypePoller, reg.PollerID, "", reg.RegistrationSource, reg.CreatedBy, eventMetadata); err != nil {
		r.logger.Warn().Err(err).Msg("Failed to emit registration event")
	}

	// Invalidate cache
	r.invalidatePollerCache()

	r.logger.Info().
		Str("poller_id", reg.PollerID).
		Str("component_id", reg.ComponentID).
		Str("source", string(reg.RegistrationSource)).
		Msg("Registered poller")

	return nil
}



 ... (clipped 8 lines)

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Input validation gaps: Template variable substitution for checker configs validates only a whitelist of metadata
keys and simple patterns, but additional external inputs (e.g., kv template contents) are
trusted and substituted recursively without schema validation which may be acceptable but
warrants further review.

Referred Code

// substituteTemplateVariables replaces placeholder values in a checker template
// with instance-specific values from the edge onboarding package.
func (s *edgeOnboardingService) substituteTemplateVariables(templateJSON string, pkg *models.EdgeOnboardingPackage) (string, error) {
	// Parse the template as a generic map
	var template map[string]interface{}
	if err := json.Unmarshal([]byte(templateJSON), &template); err != nil {
		return "", fmt.Errorf("failed to parse template JSON: %w", err)
	}

	// Parse metadata to get addresses and other values
	metadata := make(map[string]string)
	if pkg.MetadataJSON != "" {
		if err := json.Unmarshal([]byte(pkg.MetadataJSON), &metadata); err != nil {
			return "", fmt.Errorf("failed to parse metadata for template substitution: %w", err)
		}
	}

	// Whitelist of allowed metadata keys to prevent injection
	allowedMetadataKeys := map[string]bool{
		"agent_address":  true,
		"core_address":   true,



 ... (clipped 105 lines)

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Detailed errors: Errors returned include specific file paths and configuration details (e.g., bundle/join
token paths) which are useful for debugging but may expose internal details to callers if
surfaced to end users.

Referred Code

spireDir := filepath.Join(b.cfg.StoragePath, "spire")
if err := os.MkdirAll(spireDir, 0755); err != nil {
	return fmt.Errorf("create spire directory: %w", err)
}

// Write trust bundle
bundlePath := filepath.Join(spireDir, "upstream-bundle.pem")
if err := os.WriteFile(bundlePath, b.downloadResult.BundlePEM, 0644); err != nil {
	return fmt.Errorf("write bundle PEM: %w", err)
}

b.logger.Debug().
	Str("bundle_path", bundlePath).
	Int("bundle_size", len(b.downloadResult.BundlePEM)).
	Msg("Wrote SPIRE trust bundle")

// Component-specific configuration
switch b.pkg.ComponentType {
case models.EdgeOnboardingComponentTypePoller:
	return b.configurePollerSPIRE(ctx, spireDir)
case models.EdgeOnboardingComponentTypeAgent:



 ... (clipped 48 lines)

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Sensitive log data: Logging includes SPIRE configuration details such as trust bundle sizes and potentially
socket/paths which might disclose infrastructure details if logs are accessible beyond
internal systems.

Referred Code

	Str("component_type", string(b.pkg.ComponentType)).
	Msg("Configuring SPIRE credentials")

// Create SPIRE storage directories
spireDir := filepath.Join(b.cfg.StoragePath, "spire")
if err := os.MkdirAll(spireDir, 0755); err != nil {
	return fmt.Errorf("create spire directory: %w", err)
}

// Write trust bundle
bundlePath := filepath.Join(spireDir, "upstream-bundle.pem")
if err := os.WriteFile(bundlePath, b.downloadResult.BundlePEM, 0644); err != nil {
	return fmt.Errorf("write bundle PEM: %w", err)
}

b.logger.Debug().
	Str("bundle_path", bundlePath).
	Int("bundle_size", len(b.downloadResult.BundlePEM)).
	Msg("Wrote SPIRE trust bundle")

// Component-specific configuration



 ... (clipped 3 lines)

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Template trust risk: Checker template JSON fetched from KV is substituted and written without explicit schema
enforcement or strict sanitization of all fields, which could allow unsafe configuration
injection depending on template contents.

Referred Code

}

// Get the checker config - either from request or from template
checkerConfigJSON := pkg.CheckerConfigJSON
if checkerConfigJSON == "" {
	// Fetch template from KV
	templateKey := fmt.Sprintf("templates/checkers/%s.json", pkg.CheckerKind)
	template, err := s.kvClient.Get(ctx, &proto.GetRequest{Key: templateKey})
	if err != nil {
		return fmt.Errorf("edge onboarding: failed to fetch template from %s: %w", templateKey, err)
	}

	if !template.GetFound() {
		return fmt.Errorf("%w: no template found at %s and no checker_config_json provided", models.ErrEdgeOnboardingInvalidRequest, templateKey)
	}

	// Apply variable substitution to the template
	checkerConfigJSON, err = s.substituteTemplateVariables(string(template.GetValue()), pkg)
	if err != nil {
		return fmt.Errorf("edge onboarding: failed to substitute template variables: %w", err)
	}



 ... (clipped 22 lines)

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit 1cb76cd

Security Compliance

⚪

SQL injection

Description: Raw SQL string is built using fmt.Sprintf and manual quoting (quoteLiteral) for
identifiers like poller_id, risking SQL injection if inputs are not sanitized;
parameterized queries should be used instead.
service_registry_queries.go [16-25]

Referred Code

func (r *ServiceRegistry) GetPoller(ctx context.Context, pollerID string) (*RegisteredPoller, error) {
	query := fmt.Sprintf(`SELECT
		poller_id, component_id, status, registration_source,
		first_registered, first_seen, last_seen, metadata,
		spiffe_identity, created_by, agent_count, checker_count
	FROM table(pollers)
	WHERE poller_id = %s
	ORDER BY _tp_time DESC
	LIMIT 1`, quoteLiteral(pollerID))

Config injection

Description: Template variable substitution writes checker configuration from KV into another KV key
without strict schema or whitelisting, allowing potential injection of unexpected settings
if template or metadata are attacker-controlled.
edge_onboarding.go [1542-1611]

Referred Code

	return err
}

componentType := pkg.ComponentType
if componentType == models.EdgeOnboardingComponentTypeNone {
	componentType = models.EdgeOnboardingComponentTypePoller
}

// For checkers, write the checker config directly to KV
// The agent expects the checker config JSON at agents/{agent_id}/checkers/{checker_kind}.json
if componentType == models.EdgeOnboardingComponentTypeChecker {
	// Check if instance config already exists to prevent overwriting user modifications
	existing, err := s.kvClient.Get(ctx, &proto.GetRequest{Key: key})
	if err != nil {
		return fmt.Errorf("edge onboarding: failed to check existing config: %w", err)
	}

	if existing.GetFound() {
		// Config already exists, don't overwrite it
		// Just store the revision and return
		pkg.KVRevision = existing.GetRevision()



 ... (clipped 49 lines)

Ticket Compliance

🟡

🎫 #1915

🟢	Create a common onboarding library pkg/edgeonboarding enabling token-based onboarding for edge services (poller, agent, checker).
	Library downloads package, validates, extracts, configures nested SPIRE (edge), auto-registers with Core via KV/database, generates service config, handles rotation, and starts service.
	Shift automatic poller registration to KV/database with allowed_poller_id and modify isKnownPoller() to use KV/database instead of ConfigMaps.
	Add unit/integration/E2E tests for onboarding and KV-backed registration.
🔴	Provide separate Docker Compose stacks for edge poller and edge checker; keep main stack unchanged.
	Add CLI commands for edge package management and fix API authentication; document endpoints.
	Remove old shell scripts and update documentation; add migration and troubleshooting guides.
⚪	Detect deployment type (docker/bare-metal) and configure addresses appropriately.
	Do not modify k8s or main docker-compose SPIFFE enrollment flows.
	Success: Edge deployment via single onboarding token; no manual kubectl/ConfigMap/KV changes; works for Docker and bare metal; automatic registration and rotation.

Codebase Duplication Compliance

⚪

Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance

🟢

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

🔴

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Unsafe query build: Direct string interpolation in SQL queries (using fmt.Sprintf) with untrusted IDs risks
SQL injection; parameterized queries should be used consistently.

Referred Code

func (r *ServiceRegistry) GetPoller(ctx context.Context, pollerID string) (*RegisteredPoller, error) {
	query := fmt.Sprintf(`SELECT
		poller_id, component_id, status, registration_source,
		first_registered, first_seen, last_seen, metadata,
		spiffe_identity, created_by, agent_count, checker_count
	FROM table(pollers)
	WHERE poller_id = %s
	ORDER BY _tp_time DESC
	LIMIT 1`, quoteLiteral(pollerID))

⚪

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing audit context: Registration and deletion events are emitted but lack guaranteed inclusion of user
identity and full action context in all paths (e.g., auto-registration, purge), making
completeness of audit trails uncertain.

Referred Code

// emitRegistrationEvent emits an audit event to the service_registration_events stream.
func (r *ServiceRegistry) emitRegistrationEvent(ctx context.Context, eventType, serviceType, serviceID, parentID string, source RegistrationSource, actor string, metadata map[string]string) error {
	eventID := uuid.New().String()
	now := time.Now().UTC()

	metadataJSON, _ := json.Marshal(metadata)

	// Use PrepareBatch/Append/Send pattern for Proton/ClickHouse streams
	batch, err := r.db.Conn.PrepareBatch(ctx,
		`INSERT INTO service_registration_events (
			event_id, event_type, service_id, service_type, parent_id,
			registration_source, actor, timestamp, metadata
		)`)
	if err != nil {
		return fmt.Errorf("failed to prepare batch for registration event: %w", err)
	}

	err = batch.Append(
		eventID,
		eventType,
		serviceID,



 ... (clipped 13 lines)

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
SQL injection risk: Queries for GetPoller/GetAgent/GetChecker use string formatting to interpolate IDs, which
can be unsafe and bypass parameter binding protections.

Referred Code

func (r *ServiceRegistry) GetPoller(ctx context.Context, pollerID string) (*RegisteredPoller, error) {
	query := fmt.Sprintf(`SELECT
		poller_id, component_id, status, registration_source,
		first_registered, first_seen, last_seen, metadata,
		spiffe_identity, created_by, agent_count, checker_count
	FROM table(pollers)
	WHERE poller_id = %s
	ORDER BY _tp_time DESC
	LIMIT 1`, quoteLiteral(pollerID))

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Potential sensitive logs: Logs include dynamic identifiers and IP addresses (e.g., source IP and device IDs) which
may be sensitive depending on policy; ensure redaction or policy approval for such fields.

Referred Code

	return &proto.PollerStatusResponse{Received: true}, nil
}

// Auto-register poller if not already in service registry
if err := s.ensurePollerRegistered(ctx, req.PollerId, req.SourceIp); err != nil {
	s.logger.Warn().Err(err).
		Str("poller_id", req.PollerId).
		Msg("Failed to auto-register poller in service registry")
}

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1916#issuecomment-3478615809 Original created: 2025-11-03T01:00:48Z --- ## PR Compliance Guide 🔍  #### (Compliance updated until commit https://github.com/carverauto/serviceradar/commit/5760e8406da43a5c5b31811f5c13de99576b9a07) Below is a summary of compliance checks for this PR: <table><tbody><tr><td colspan='2'>Security Compliance</td></tr> <tr><td rowspan=3>⚪</td> <td><details><summary>Authorization/Access control </summary> Description: The DELETE device endpoint allows tombstoning any device by ID without explicit RBAC checks shown in the handler, potentially enabling unauthorized deletion if upstream auth middleware is misconfigured. <a href='https://github.com/carverauto/serviceradar/pull/1916/files#diff-34b7da1b2845de83ee2b0eeba93ef1c8b7abf40517f20617c861abffec32ee1cR156-R287'>device_registry.go [156-287]</a> <details open><summary>Referred Code</summary> ```go func (s *APIServer) deleteDevice(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) deviceID := vars["id"] if deviceID == "" { writeError(w, "Device ID is required", http.StatusBadRequest) return } if s.dbService == nil { writeError(w, "Database not configured", http.StatusInternalServerError) return } now := time.Now().UTC() ctx := r.Context() partition := partitionFromDeviceID(deviceID) update := &models.DeviceUpdate{ DeviceID: deviceID, Partition: partition, ... (clipped 111 lines) ``` </details></details></td></tr> <tr><td><details><summary>Template injection risk </summary> Description: Template variable substitution for checker configs performs simple string replacement which could allow injecting unintended values into JSON configuration if templates include sensitive fields. <a href='https://github.com/carverauto/serviceradar/pull/1916/files#diff-85874e3c4bdcc9110db09909f10648d44cdee554b26c987f910502321eb20b5cR1542-R1611'>edge_onboarding.go [1542-1611]</a> <details open><summary>Referred Code</summary> ```go return err } componentType := pkg.ComponentType if componentType == models.EdgeOnboardingComponentTypeNone { componentType = models.EdgeOnboardingComponentTypePoller } // For checkers, write the checker config directly to KV // The agent expects the checker config JSON at agents/{agent_id}/checkers/{checker_kind}.json if componentType == models.EdgeOnboardingComponentTypeChecker { // Check if instance config already exists to prevent overwriting user modifications existing, err := s.kvClient.Get(ctx, &proto.GetRequest{Key: key}) if err != nil { return fmt.Errorf("edge onboarding: failed to check existing config: %w", err) } if existing.GetFound() { // Config already exists, don't overwrite it // Just store the revision and return pkg.KVRevision = existing.GetRevision() ... (clipped 49 lines) ``` </details></details></td></tr> <tr><td><details><summary>Destructive delete behavior</summary> Description: Hard DELETE operations directly remove rows from versioned_kv tables which may bypass audit/history expectations; if exposed, could aid tampering or data loss. <a href='https://github.com/carverauto/serviceradar/pull/1916/files#diff-d6d6d09a58edde934a1d3f571ff5ce02f0475c5e85ecdec27888892aff8d9d1dR659-R706'>service_registry.go [659-706]</a> <details open><summary>Referred Code</summary> ```go // Verify service is not active or pending var status string var query string var source string switch serviceType { case serviceTypePoller: query = `SELECT status, registration_source FROM pollers WHERE poller_id = ? LIMIT 1` case serviceTypeAgent: query = `SELECT status, registration_source FROM agents WHERE agent_id = ? LIMIT 1` case serviceTypeChecker: query = `SELECT status, registration_source FROM checkers WHERE checker_id = ? LIMIT 1` default: return fmt.Errorf("%w: %s", ErrUnknownServiceType, serviceType) } row := r.db.Conn.QueryRow(ctx, query, serviceID) if err := row.Scan(&status, &source); err != nil { return fmt.Errorf("service not found: %w", err) } ... (clipped 27 lines) ``` </details></details></td></tr> <tr><td colspan='2'>Ticket Compliance</td></tr> <tr><td>🟡</td> <td> <details> <summary>🎫 <a href=https://github.com/carverauto/serviceradar/issues/1915>#1915</a></summary> <table width='100%'><tbody> <tr><td rowspan=6>🟢</td> <td>Implement service registry with registration, lifecycle management, and heartbeats for pollers/agents/checkers.</td></tr> <tr><td>Replace ConfigMap-based known pollers with KV/database-backed approach and modify isKnownPoller() to check KV/database.</td></tr> <tr><td>Publish device lifecycle events and provide API endpoints for device registry queries and device deletion.</td></tr> <tr><td>Auto-register services (poller/agent/checker) with Core via KV/database and maintain dynamic config in KV.</td></tr> <tr><td>Handle checker config via KV with template substitution and per-agent checker config keys.</td></tr> <tr><td>Regenerate/add unit tests for registry/device publishing behaviors.</td></tr> <tr><td rowspan=4>🔴</td> <td>Add allowed_poller_id to edge_packages and ensure package creation populates it; use statuses Issued/Delivered/Activated for allowlist.</td></tr> <tr><td>Create edge-checker-stack.compose.yml and ensure main docker-compose is unchanged; document differences.</td></tr> <tr><td>Add CLI edge commands for package lifecycle; fix API authentication; document endpoints; consider service account tokens.</td></tr> <tr><td>Remove legacy shell scripts and update edge documentation and migration guides.</td></tr> <tr><td rowspan=7>⚪</td> <td>Create a common onboarding library pkg/edgeonboarding to simplify edge onboarding for pollers, agents, and checkers using a token-based flow.</td></tr> <tr><td>Handle package download, validation, and extraction for edge onboarding.</td></tr> <tr><td>Configure nested SPIRE credentials specifically for edge (not k8s SPIRE) and support rotation.</td></tr> <tr><td>Detect deployment type (docker vs bare-metal) and generate appropriate configuration.</td></tr> <tr><td>Integrate onboarding library into poller, agent, and checkers; support ONBOARDING_TOKEN env var; do not modify k8s or main docker-compose logic.</td></tr> <tr><td>Add API endpoints for agent discovery and datasvc listing.</td></tr> <tr><td>Documentation updates across edge deployment guides and differences between stacks.</td></tr> </tbody></table> </details> </td></tr> <tr><td colspan='2'>Codebase Duplication Compliance</td></tr> <tr><td>⚪</td><td><details><summary>Codebase context is not defined </summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'>Custom Compliance</td></tr> <tr><td rowspan=1>🟢</td><td> <details><summary>Generic: Meaningful Naming and Self-Documenting Code</summary> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code self-documenting **Status:** Passed </details></td></tr> <tr><td rowspan=5>⚪</td> <td><details> <summary>Generic: Comprehensive Audit Trails</summary> **Objective:** To create a detailed and reliable record of critical system actions for security analysis and compliance. **Status:** <a href='https://github.com/carverauto/serviceradar/pull/1916/files#diff-d6d6d09a58edde934a1d3f571ff5ce02f0475c5e85ecdec27888892aff8d9d1dR116-R144'>Missing audit context</a>: Registration/heartbeat/deletion events are emitted and logged but lack explicit inclusion of user identifiers and full action context in all paths (e.g., auto-registration uses actor "system"), which may be compliant but needs verification of upstream logging to ensure audit trail completeness. <details open><summary>Referred Code</summary> ```go eventMetadata := map[string]string{ "component_id": reg.ComponentID, } if reg.SPIFFEIdentity != "" { eventMetadata["spiffe_id"] = reg.SPIFFEIdentity } if err := r.emitRegistrationEvent(ctx, "registered", serviceTypePoller, reg.PollerID, "", reg.RegistrationSource, reg.CreatedBy, eventMetadata); err != nil { r.logger.Warn().Err(err).Msg("Failed to emit registration event") } // Invalidate cache r.invalidatePollerCache() r.logger.Info(). Str("poller_id", reg.PollerID). Str("component_id", reg.ComponentID). Str("source", string(reg.RegistrationSource)). Msg("Registered poller") return nil } ... (clipped 8 lines) ``` </details></details></td></tr> <tr><td><details> <summary>Generic: Robust Error Handling and Edge Case Management</summary> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful degradation **Status:** <a href='https://github.com/carverauto/serviceradar/pull/1916/files#diff-85874e3c4bdcc9110db09909f10648d44cdee554b26c987f910502321eb20b5cR1752-R1877'>Input validation gaps</a>: Template variable substitution for checker configs validates only a whitelist of metadata keys and simple patterns, but additional external inputs (e.g., kv template contents) are trusted and substituted recursively without schema validation which may be acceptable but warrants further review. <details open><summary>Referred Code</summary> ```go // substituteTemplateVariables replaces placeholder values in a checker template // with instance-specific values from the edge onboarding package. func (s *edgeOnboardingService) substituteTemplateVariables(templateJSON string, pkg *models.EdgeOnboardingPackage) (string, error) { // Parse the template as a generic map var template map[string]interface{} if err := json.Unmarshal([]byte(templateJSON), &template); err != nil { return "", fmt.Errorf("failed to parse template JSON: %w", err) } // Parse metadata to get addresses and other values metadata := make(map[string]string) if pkg.MetadataJSON != "" { if err := json.Unmarshal([]byte(pkg.MetadataJSON), &metadata); err != nil { return "", fmt.Errorf("failed to parse metadata for template substitution: %w", err) } } // Whitelist of allowed metadata keys to prevent injection allowedMetadataKeys := map[string]bool{ "agent_address": true, "core_address": true, ... (clipped 105 lines) ``` </details></details></td></tr> <tr><td><details> <summary>Generic: Secure Error Handling</summary> **Objective:** To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. **Status:** <a href='https://github.com/carverauto/serviceradar/pull/1916/files#diff-5d784c136f3db0128ea90d628cbefdff9e2ba18eba9a1775f54fc630bde8ee1dR41-R109'>Detailed errors</a>: Errors returned include specific file paths and configuration details (e.g., bundle/join token paths) which are useful for debugging but may expose internal details to callers if surfaced to end users. <details open><summary>Referred Code</summary> ```go spireDir := filepath.Join(b.cfg.StoragePath, "spire") if err := os.MkdirAll(spireDir, 0755); err != nil { return fmt.Errorf("create spire directory: %w", err) } // Write trust bundle bundlePath := filepath.Join(spireDir, "upstream-bundle.pem") if err := os.WriteFile(bundlePath, b.downloadResult.BundlePEM, 0644); err != nil { return fmt.Errorf("write bundle PEM: %w", err) } b.logger.Debug(). Str("bundle_path", bundlePath). Int("bundle_size", len(b.downloadResult.BundlePEM)). Msg("Wrote SPIRE trust bundle") // Component-specific configuration switch b.pkg.ComponentType { case models.EdgeOnboardingComponentTypePoller: return b.configurePollerSPIRE(ctx, spireDir) case models.EdgeOnboardingComponentTypeAgent: ... (clipped 48 lines) ``` </details></details></td></tr> <tr><td><details> <summary>Generic: Secure Logging Practices</summary> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. **Status:** <a href='https://github.com/carverauto/serviceradar/pull/1916/files#diff-5d784c136f3db0128ea90d628cbefdff9e2ba18eba9a1775f54fc630bde8ee1dR37-R60'>Sensitive log data</a>: Logging includes SPIRE configuration details such as trust bundle sizes and potentially socket/paths which might disclose infrastructure details if logs are accessible beyond internal systems. <details open><summary>Referred Code</summary> ```go Str("component_type", string(b.pkg.ComponentType)). Msg("Configuring SPIRE credentials") // Create SPIRE storage directories spireDir := filepath.Join(b.cfg.StoragePath, "spire") if err := os.MkdirAll(spireDir, 0755); err != nil { return fmt.Errorf("create spire directory: %w", err) } // Write trust bundle bundlePath := filepath.Join(spireDir, "upstream-bundle.pem") if err := os.WriteFile(bundlePath, b.downloadResult.BundlePEM, 0644); err != nil { return fmt.Errorf("write bundle PEM: %w", err) } b.logger.Debug(). Str("bundle_path", bundlePath). Int("bundle_size", len(b.downloadResult.BundlePEM)). Msg("Wrote SPIRE trust bundle") // Component-specific configuration ... (clipped 3 lines) ``` </details></details></td></tr> <tr><td><details> <summary>Generic: Security-First Input Validation and Data Handling</summary> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities **Status:** <a href='https://github.com/carverauto/serviceradar/pull/1916/files#diff-85874e3c4bdcc9110db09909f10648d44cdee554b26c987f910502321eb20b5cR1569-R1611'>Template trust risk</a>: Checker template JSON fetched from KV is substituted and written without explicit schema enforcement or strict sanitization of all fields, which could allow unsafe configuration injection depending on template contents. <details open><summary>Referred Code</summary> ```go } // Get the checker config - either from request or from template checkerConfigJSON := pkg.CheckerConfigJSON if checkerConfigJSON == "" { // Fetch template from KV templateKey := fmt.Sprintf("templates/checkers/%s.json", pkg.CheckerKind) template, err := s.kvClient.Get(ctx, &proto.GetRequest{Key: templateKey}) if err != nil { return fmt.Errorf("edge onboarding: failed to fetch template from %s: %w", templateKey, err) } if !template.GetFound() { return fmt.Errorf("%w: no template found at %s and no checker_config_json provided", models.ErrEdgeOnboardingInvalidRequest, templateKey) } // Apply variable substitution to the template checkerConfigJSON, err = s.substituteTemplateVariables(string(template.GetValue()), pkg) if err != nil { return fmt.Errorf("edge onboarding: failed to substitute template variables: %w", err) } ... (clipped 22 lines) ``` </details></details></td></tr> <tr><td align="center" colspan="2">   </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant 🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label </details> ___ #### Previous compliance checks <details> <summary>Compliance check up to commit <a href='https://github.com/carverauto/serviceradar/commit/1cb76cd8c5672ae1c51c3739367aa9eb2713a7dc'>1cb76cd</a></summary> <table><tbody><tr><td colspan='2'>Security Compliance</td></tr> <tr><td rowspan=2>⚪</td> <td><details><summary>SQL injection </summary> Description: Raw SQL string is built using fmt.Sprintf and manual quoting (quoteLiteral) for identifiers like poller_id, risking SQL injection if inputs are not sanitized; parameterized queries should be used instead. <a href='https://github.com/carverauto/serviceradar/pull/1916/files#diff-3591e8646384be68811f862b986342253cabc23176c6f0e09996453baf88a2e9R16-R25'>service_registry_queries.go [16-25]</a> <details open><summary>Referred Code</summary> ```go func (r *ServiceRegistry) GetPoller(ctx context.Context, pollerID string) (*RegisteredPoller, error) { query := fmt.Sprintf(`SELECT poller_id, component_id, status, registration_source, first_registered, first_seen, last_seen, metadata, spiffe_identity, created_by, agent_count, checker_count FROM table(pollers) WHERE poller_id = %s ORDER BY _tp_time DESC LIMIT 1`, quoteLiteral(pollerID)) ``` </details></details></td></tr> <tr><td><details><summary>Config injection</summary> Description: Template variable substitution writes checker configuration from KV into another KV key without strict schema or whitelisting, allowing potential injection of unexpected settings if template or metadata are attacker-controlled. <a href='https://github.com/carverauto/serviceradar/pull/1916/files#diff-85874e3c4bdcc9110db09909f10648d44cdee554b26c987f910502321eb20b5cR1542-R1611'>edge_onboarding.go [1542-1611]</a> <details open><summary>Referred Code</summary> ```go return err } componentType := pkg.ComponentType if componentType == models.EdgeOnboardingComponentTypeNone { componentType = models.EdgeOnboardingComponentTypePoller } // For checkers, write the checker config directly to KV // The agent expects the checker config JSON at agents/{agent_id}/checkers/{checker_kind}.json if componentType == models.EdgeOnboardingComponentTypeChecker { // Check if instance config already exists to prevent overwriting user modifications existing, err := s.kvClient.Get(ctx, &proto.GetRequest{Key: key}) if err != nil { return fmt.Errorf("edge onboarding: failed to check existing config: %w", err) } if existing.GetFound() { // Config already exists, don't overwrite it // Just store the revision and return pkg.KVRevision = existing.GetRevision() ... (clipped 49 lines) ``` </details></details></td></tr> <tr><td colspan='2'>Ticket Compliance</td></tr> <tr><td>🟡</td> <td> <details> <summary>🎫 <a href=https://github.com/carverauto/serviceradar/issues/1915>#1915</a></summary> <table width='100%'><tbody> <tr><td rowspan=4>🟢</td> <td>Create a common onboarding library pkg/edgeonboarding enabling token-based onboarding for edge services (poller, agent, checker).</td></tr> <tr><td>Library downloads package, validates, extracts, configures nested SPIRE (edge), auto-registers with Core via KV/database, generates service config, handles rotation, and starts service.</td></tr> <tr><td>Shift automatic poller registration to KV/database with allowed_poller_id and modify isKnownPoller() to use KV/database instead of ConfigMaps.</td></tr> <tr><td>Add unit/integration/E2E tests for onboarding and KV-backed registration.</td></tr> <tr><td rowspan=3>🔴</td> <td>Provide separate Docker Compose stacks for edge poller and edge checker; keep main stack unchanged.</td></tr> <tr><td>Add CLI commands for edge package management and fix API authentication; document endpoints.</td></tr> <tr><td>Remove old shell scripts and update documentation; add migration and troubleshooting guides.</td></tr> <tr><td rowspan=3>⚪</td> <td>Detect deployment type (docker/bare-metal) and configure addresses appropriately.</td></tr> <tr><td>Do not modify k8s or main docker-compose SPIFFE enrollment flows.</td></tr> <tr><td>Success: Edge deployment via single onboarding token; no manual kubectl/ConfigMap/KV changes; works for Docker and bare metal; automatic registration and rotation.</td></tr> </tbody></table> </details> </td></tr> <tr><td colspan='2'>Codebase Duplication Compliance</td></tr> <tr><td>⚪</td><td><details><summary>Codebase context is not defined </summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'>Custom Compliance</td></tr> <tr><td rowspan=2>🟢</td><td> <details><summary>Generic: Meaningful Naming and Self-Documenting Code</summary> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code self-documenting **Status:** Passed </details></td></tr> <tr><td> <details><summary>Generic: Secure Error Handling</summary> **Objective:** To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. **Status:** Passed </details></td></tr> <tr><td rowspan=1>🔴</td> <td><details> <summary>Generic: Security-First Input Validation and Data Handling</summary> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities **Status:** <a href='https://github.com/carverauto/serviceradar/pull/1916/files#diff-3591e8646384be68811f862b986342253cabc23176c6f0e09996453baf88a2e9R16-R25'>Unsafe query build</a>: Direct string interpolation in SQL queries (using fmt.Sprintf) with untrusted IDs risks SQL injection; parameterized queries should be used consistently. <details open><summary>Referred Code</summary> ```go func (r *ServiceRegistry) GetPoller(ctx context.Context, pollerID string) (*RegisteredPoller, error) { query := fmt.Sprintf(`SELECT poller_id, component_id, status, registration_source, first_registered, first_seen, last_seen, metadata, spiffe_identity, created_by, agent_count, checker_count FROM table(pollers) WHERE poller_id = %s ORDER BY _tp_time DESC LIMIT 1`, quoteLiteral(pollerID)) ``` </details></details></td></tr> <tr><td rowspan=3>⚪</td> <td><details> <summary>Generic: Comprehensive Audit Trails</summary> **Objective:** To create a detailed and reliable record of critical system actions for security analysis and compliance. **Status:** <a href='https://github.com/carverauto/serviceradar/pull/1916/files#diff-d6d6d09a58edde934a1d3f571ff5ce02f0475c5e85ecdec27888892aff8d9d1dR547-R580'>Missing audit context</a>: Registration and deletion events are emitted but lack guaranteed inclusion of user identity and full action context in all paths (e.g., auto-registration, purge), making completeness of audit trails uncertain. <details open><summary>Referred Code</summary> ```go // emitRegistrationEvent emits an audit event to the service_registration_events stream. func (r *ServiceRegistry) emitRegistrationEvent(ctx context.Context, eventType, serviceType, serviceID, parentID string, source RegistrationSource, actor string, metadata map[string]string) error { eventID := uuid.New().String() now := time.Now().UTC() metadataJSON, _ := json.Marshal(metadata) // Use PrepareBatch/Append/Send pattern for Proton/ClickHouse streams batch, err := r.db.Conn.PrepareBatch(ctx, `INSERT INTO service_registration_events ( event_id, event_type, service_id, service_type, parent_id, registration_source, actor, timestamp, metadata )`) if err != nil { return fmt.Errorf("failed to prepare batch for registration event: %w", err) } err = batch.Append( eventID, eventType, serviceID, ... (clipped 13 lines) ``` </details></details></td></tr> <tr><td><details> <summary>Generic: Robust Error Handling and Edge Case Management</summary> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful degradation **Status:** <a href='https://github.com/carverauto/serviceradar/pull/1916/files#diff-3591e8646384be68811f862b986342253cabc23176c6f0e09996453baf88a2e9R16-R25'>SQL injection risk</a>: Queries for GetPoller/GetAgent/GetChecker use string formatting to interpolate IDs, which can be unsafe and bypass parameter binding protections. <details open><summary>Referred Code</summary> ```go func (r *ServiceRegistry) GetPoller(ctx context.Context, pollerID string) (*RegisteredPoller, error) { query := fmt.Sprintf(`SELECT poller_id, component_id, status, registration_source, first_registered, first_seen, last_seen, metadata, spiffe_identity, created_by, agent_count, checker_count FROM table(pollers) WHERE poller_id = %s ORDER BY _tp_time DESC LIMIT 1`, quoteLiteral(pollerID)) ``` </details></details></td></tr> <tr><td><details> <summary>Generic: Secure Logging Practices</summary> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. **Status:** <a href='https://github.com/carverauto/serviceradar/pull/1916/files#diff-fe81e2a32f1ac64bcdc6f25f55c5fa918d17bad8c0546f2cf80c757ff4051816R907-R916'>Potential sensitive logs</a>: Logs include dynamic identifiers and IP addresses (e.g., source IP and device IDs) which may be sensitive depending on policy; ensure redaction or policy approval for such fields. <details open><summary>Referred Code</summary> ```go return &proto.PollerStatusResponse{Received: true}, nil } // Auto-register poller if not already in service registry if err := s.ensurePollerRegistered(ctx, req.PollerId, req.SourceIp); err != nil { s.logger.Warn().Err(err). Str("poller_id", req.PollerId). Msg("Failed to auto-register poller in service registry") } ``` </details></details></td></tr> <tr><td align="center" colspan="2">  </td></tr></tbody></table> </details>

qodo-code-review[bot] commented

2025-11-03 01:02:06 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1916#issuecomment-3478617100
Original created: 2025-11-03T01:02:06Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Security	✅ ~~Prevent SQL injection with parameters~~ Suggestion Impact: The commit removed quoteLiteral and replaced fmt.Sprintf-built queries with parameterized queries using placeholders (?) and passing the IDs as parameters in GetPoller, GetAgent, and GetChecker. code diff: -func quoteLiteral(value string) string { - return fmt.Sprintf("'%s'", strings.ReplaceAll(value, "'", "''")) -} +const ( + serviceTypePoller = "poller" + serviceTypeAgent = "agent" + serviceTypeChecker = "checker" +) // GetPoller retrieves a poller by ID. func (r ServiceRegistry) GetPoller(ctx context.Context, pollerID string) (RegisteredPoller, error) { - query := fmt.Sprintf(`SELECT + query := `SELECT poller_id, component_id, status, registration_source, first_registered, first_seen, last_seen, metadata, spiffe_identity, created_by, agent_count, checker_count FROM table(pollers) - WHERE poller_id = %s + WHERE poller_id = ? ORDER BY _tp_time DESC - LIMIT 1`, quoteLiteral(pollerID)) - - row := r.db.Conn.QueryRow(ctx, query) + LIMIT 1` + + row := r.db.Conn.QueryRow(ctx, query, pollerID) var ( poller RegisteredPoller @@ -71,16 +72,16 @@ // GetAgent retrieves an agent by ID. func (r ServiceRegistry) GetAgent(ctx context.Context, agentID string) (RegisteredAgent, error) { - query := fmt.Sprintf(`SELECT + query := `SELECT agent_id, poller_id, component_id, status, registration_source, first_registered, first_seen, last_seen, metadata, spiffe_identity, created_by, checker_count FROM table(agents) - WHERE agent_id = %s + WHERE agent_id = ? ORDER BY _tp_time DESC - LIMIT 1`, quoteLiteral(agentID)) - - row := r.db.Conn.QueryRow(ctx, query) + LIMIT 1` + + row := r.db.Conn.QueryRow(ctx, query, agentID) var ( agent RegisteredAgent @@ -128,16 +129,16 @@ // GetChecker retrieves a checker by ID. func (r ServiceRegistry) GetChecker(ctx context.Context, checkerID string) (RegisteredChecker, error) { - query := fmt.Sprintf(`SELECT + query := `SELECT checker_id, agent_id, poller_id, checker_kind, component_id, status, registration_source, first_registered, first_seen, last_seen, metadata, spiffe_identity, created_by FROM table(checkers) - WHERE checker_id = %s + WHERE checker_id = ? ORDER BY _tp_time DESC - LIMIT 1`, quoteLiteral(checkerID)) - - row := r.db.Conn.QueryRow(ctx, query) + LIMIT 1` + + row := r.db.Conn.QueryRow(ctx, query, checkerID) Replace `fmt.Sprintf` with parameterized queries in `GetPoller`, `GetAgent`, and `GetChecker` to prevent potential SQL injection vulnerabilities. pkg/registry/service_registry_queries.go [11-26] -func quoteLiteral(value string) string { - return fmt.Sprintf("'%s'", strings.ReplaceAll(value, "'", "''")) -} - // GetPoller retrieves a poller by ID. func (r ServiceRegistry) GetPoller(ctx context.Context, pollerID string) (RegisteredPoller, error) { - query := fmt.Sprintf(`SELECT + query := `SELECT poller_id, component_id, status, registration_source, first_registered, first_seen, last_seen, metadata, spiffe_identity, created_by, agent_count, checker_count FROM table(pollers) - WHERE poller_id = %s + WHERE poller_id = ? ORDER BY _tp_time DESC - LIMIT 1`, quoteLiteral(pollerID)) + LIMIT 1` - row := r.db.Conn.QueryRow(ctx, query) + row := r.db.Conn.QueryRow(ctx, query, pollerID) ... `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies a critical SQL injection vulnerability in `GetPoller`, `GetAgent`, and `GetChecker` and proposes the standard, secure fix of using parameterized queries.	High
Possible issue	✅ ~~Handle metadata parsing error properly~~ Suggestion Impact: The commit changed the code to return an error on metadata JSON parsing failure, replacing the previous warning log. It also added sanitization logic, but the key part of the suggestion was implemented. code diff: `@@ -1762,24 +1762,49 @@ metadata := make(map[string]string) if pkg.MetadataJSON != "" { if err := json.Unmarshal([]byte(pkg.MetadataJSON), &metadata); err != nil { - s.logger.Warn().Err(err).Msg("Failed to parse metadata for template substitution") + return "", fmt.Errorf("failed to parse metadata for template substitution: %w", err) + } + }` In `substituteTemplateVariables`, return an error if parsing `pkg.MetadataJSON` fails, instead of just logging a warning, to prevent silent failures and invalid configurations. pkg/core/edge_onboarding.go [1754-1791] // substituteTemplateVariables replaces placeholder values in a checker template // with instance-specific values from the edge onboarding package. func (s edgeOnboardingService) substituteTemplateVariables(templateJSON string, pkg models.EdgeOnboardingPackage) (string, error) { // Parse the template as a generic map var template map[string]interface{} if err := json.Unmarshal([]byte(templateJSON), &template); err != nil { return "", fmt.Errorf("failed to parse template JSON: %w", err) } // Parse metadata to get addresses and other values metadata := make(map[string]string) if pkg.MetadataJSON != "" { if err := json.Unmarshal([]byte(pkg.MetadataJSON), &metadata); err != nil { - s.logger.Warn().Err(err).Msg("Failed to parse metadata for template substitution") + return "", fmt.Errorf("failed to parse metadata for template substitution: %w", err) } } // Recursively substitute variables in the template substituted := s.substituteInMap(template, map[string]string{ "DOWNSTREAM_SPIFFE_ID": pkg.DownstreamSPIFFEID, "AGENT_ADDRESS": metadata["agent_address"], "CORE_ADDRESS": metadata["core_address"], "CORE_SPIFFE_ID": metadata["core_spiffe_id"], "KV_ADDRESS": metadata["kv_address"], "KV_SPIFFE_ID": metadata["kv_spiffe_id"], "TRUST_DOMAIN": metadata["trust_domain"], "LOG_LEVEL": metadata["log_level"], "COMPONENT_ID": pkg.ComponentID, "CHECKER_KIND": pkg.CheckerKind, "AGENT_ID": pkg.ParentID, }) // Marshal back to JSON result, err := json.Marshal(substituted) if err != nil { return "", fmt.Errorf("failed to marshal substituted template: %w", err) } return string(result), nil } `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that ignoring a JSON parsing error can lead to silent failures and invalid configurations, and proposes the correct fix of returning the error.	Medium
	✅ ~~Allow re-onboarding of deleted devices~~ Suggestion Impact: The commit updated the tombstone filter to only block self-reported updates when the update timestamp is not after the deletion time, and allowed newer updates with logging—enabling re-onboarding. It also added extra logging fields/messages. code diff: if update.Source == models.DiscoverySourceSelfReported \|\| update.Source == models.DiscoverySourceServiceRadar { - dropped++ + // Block self-reported updates for tombstoned devices unless the update is fresh, + // which can happen during re-onboarding when a device comes back online. + if !update.Timestamp.After(deletedAt) { + dropped++ + r.logger.Info(). + Str("device_id", update.DeviceID). + Str("source", string(update.Source)). + Time("deleted_at", deletedAt). + Time("update_ts", update.Timestamp). + Msg("Blocking self-reported update for tombstoned device") + continue + } + // Update is newer than deletion - allow re-onboarding r.logger.Info(). Str("device_id", update.DeviceID). Str("source", string(update.Source)). Time("deleted_at", deletedAt). - Msg("Blocking self-reported update for tombstoned device") - continue + Time("update_ts", update.Timestamp). + Msg("Allowing self-reported update for re-onboarding (update is newer than deletion)") } Modify the tombstone filter to allow self-reported updates for a deleted device if the update's timestamp is after the deletion time, enabling device re-onboarding. pkg/registry/registry.go [863-871] if update.Source == models.DiscoverySourceSelfReported \|\| update.Source == models.DiscoverySourceServiceRadar { - dropped++ - r.logger.Info(). - Str("device_id", update.DeviceID). - Str("source", string(update.Source)). - Time("deleted_at", deletedAt). - Msg("Blocking self-reported update for tombstoned device") - continue + // Block self-reported updates for tombstoned devices unless the update is fresh, + // which can happen during re-onboarding. + if !update.Timestamp.After(deletedAt) { + dropped++ + r.logger.Info(). + Str("device_id", update.DeviceID). + Str("source", string(update.Source)). + Time("deleted_at", deletedAt). + Msg("Blocking self-reported update for tombstoned device") + continue + } } `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 8 __ Why: This suggestion fixes a significant logic flaw where a deleted device could never be re-onboarded, correctly proposing to allow fresh updates that occur after the deletion timestamp.	Medium
	✅ ~~Improve SPIFFE ID parsing logic~~ Suggestion Impact: The commit updated extractTrustDomain to use strings.TrimPrefix and strings.Index, returning the substring before the first slash and handling empty cases—implementing the suggested robust parsing approach. code diff: // Example: spiffe://carverauto.dev/ns/edge/poller-1 -> carverauto.dev func extractTrustDomain(spiffeID string) string { // Remove spiffe:// prefix - id := spiffeID - if len(id) > 9 { - id = id[9:] // Remove "spiffe://" - } + id := strings.TrimPrefix(spiffeID, "spiffe://") // Find first / and return everything before it - if idx := len(id); idx > 0 { - for i, c := range id { - if c == '/' { - return id[:i] - } - } + if slashIdx := strings.Index(id, "/"); slashIdx != -1 { + return id[:slashIdx] + } + + if id != "" { return id } Refactor the `extractTrustDomain` function to use `strings.HasPrefix` and `strings.Index` for more robustly parsing the trust domain from a SPIFFE ID. pkg/edgeonboarding/spire.go [231-247] `-// Remove spiffe:// prefix id := spiffeID -if len(id) > 9 { - id = id[9:] // Remove "spiffe://" +if strings.HasPrefix(id, "spiffe://") { + id = id[9:] } -// Find first / and return everything before it -if idx := len(id); idx > 0 { - for i, c := range id { - if c == '/' { - return id[:i] - } - } +if slashIdx := strings.Index(id, "/"); slashIdx != -1 { + return id[:slashIdx] +} + +if id != "" { return id } return "unknown"` `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies a fragile implementation and proposes using idiomatic Go functions (`strings.HasPrefix`, `strings.Index`) to make the SPIFFE ID parsing more robust and readable.	Low
General	Improve performance with batch database operations Refactor `RecordBatchHeartbeats` to use true database batching for `SELECT` and `INSERT` operations to fix the N+1 query problem and improve performance. pkg/registry/service_registry.go [294-305] // RecordBatchHeartbeats handles batch heartbeat updates efficiently. func (r ServiceRegistry) RecordBatchHeartbeats(ctx context.Context, heartbeats []ServiceHeartbeat) error { + // This function can be improved to perform true batching by: + // 1. Grouping heartbeats by service type. + // 2. For each type, fetching all existing services with a single `SELECT ... WHERE id IN (...)`. + // 3. Preparing a single batch insert (`PrepareBatch`). + // 4. Iterating through the heartbeats, determining the new state for each, and appending to the batch. + // 5. Sending the batch once. + // The current implementation is kept for simplicity, but for high-throughput scenarios, + // a full batch implementation is recommended. for _, hb := range heartbeats { if err := r.RecordHeartbeat(ctx, hb); err != nil { r.logger.Warn(). Err(err). Str("service_type", hb.ServiceType). Str("service_id", hb.ServiceID). Msg("Failed to record heartbeat") } } return nil } `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a significant performance issue (N+1 problem) where a batch function iterates and makes individual DB calls, but the improved code only adds a comment explaining the problem instead of implementing the fix.	Medium
General	✅ ~~Fix discarded partition value assignment~~ Suggestion Impact: The commit updated the code to assign partitionFromDeviceID(existing.DeviceID) to partition and also set update.Partition, preventing the partition value from being discarded. code diff: `if existing, err := s.dbService.GetUnifiedDevice(ctx, deviceID); err == nil { update.IP = existing.IP if partition == "" { - _ = partitionFromDeviceID(existing.DeviceID) + partition = partitionFromDeviceID(existing.DeviceID) + update.Partition = partition }` In `deleteDevice`, assign the result of `partitionFromDeviceID` to the `partition` variable and update `DeviceUpdate` to ensure the partition is not lost. pkg/core/api/device_registry.go [197-206] `if existing, err := s.dbService.GetUnifiedDevice(ctx, deviceID); err == nil { update.IP = existing.IP if partition == "" { - _ = partitionFromDeviceID(existing.DeviceID) + partition = partitionFromDeviceID(existing.DeviceID) + update.Partition = partition } if existing.Hostname != nil && existing.Hostname.Value != "" { ...` `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that a calculated partition value is discarded, and fixing this ensures the device's partition is correctly preserved when it is tombstoned.	Medium
Update

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1916#issuecomment-3478617100 Original created: 2025-11-03T01:02:06Z --- ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td>Category</td><td align=left>Suggestion                                                                                                                                    </td><td align=center>Impact</td></tr><tbody><tr><td rowspan=1>Security</td> <td> <details><summary>✅ <s>Prevent SQL injection with parameters</s></summary> ___ <details><summary>Suggestion Impact:</summary>The commit removed quoteLiteral and replaced fmt.Sprintf-built queries with parameterized queries using placeholders (?) and passing the IDs as parameters in GetPoller, GetAgent, and GetChecker. code diff: ```diff -func quoteLiteral(value string) string { - return fmt.Sprintf("'%s'", strings.ReplaceAll(value, "'", "''")) -} +const ( + serviceTypePoller = "poller" + serviceTypeAgent = "agent" + serviceTypeChecker = "checker" +) // GetPoller retrieves a poller by ID. func (r *ServiceRegistry) GetPoller(ctx context.Context, pollerID string) (*RegisteredPoller, error) { - query := fmt.Sprintf(`SELECT + query := `SELECT poller_id, component_id, status, registration_source, first_registered, first_seen, last_seen, metadata, spiffe_identity, created_by, agent_count, checker_count FROM table(pollers) - WHERE poller_id = %s + WHERE poller_id = ? ORDER BY _tp_time DESC - LIMIT 1`, quoteLiteral(pollerID)) - - row := r.db.Conn.QueryRow(ctx, query) + LIMIT 1` + + row := r.db.Conn.QueryRow(ctx, query, pollerID) var ( poller RegisteredPoller @@ -71,16 +72,16 @@ // GetAgent retrieves an agent by ID. func (r *ServiceRegistry) GetAgent(ctx context.Context, agentID string) (*RegisteredAgent, error) { - query := fmt.Sprintf(`SELECT + query := `SELECT agent_id, poller_id, component_id, status, registration_source, first_registered, first_seen, last_seen, metadata, spiffe_identity, created_by, checker_count FROM table(agents) - WHERE agent_id = %s + WHERE agent_id = ? ORDER BY _tp_time DESC - LIMIT 1`, quoteLiteral(agentID)) - - row := r.db.Conn.QueryRow(ctx, query) + LIMIT 1` + + row := r.db.Conn.QueryRow(ctx, query, agentID) var ( agent RegisteredAgent @@ -128,16 +129,16 @@ // GetChecker retrieves a checker by ID. func (r *ServiceRegistry) GetChecker(ctx context.Context, checkerID string) (*RegisteredChecker, error) { - query := fmt.Sprintf(`SELECT + query := `SELECT checker_id, agent_id, poller_id, checker_kind, component_id, status, registration_source, first_registered, first_seen, last_seen, metadata, spiffe_identity, created_by FROM table(checkers) - WHERE checker_id = %s + WHERE checker_id = ? ORDER BY _tp_time DESC - LIMIT 1`, quoteLiteral(checkerID)) - - row := r.db.Conn.QueryRow(ctx, query) + LIMIT 1` + + row := r.db.Conn.QueryRow(ctx, query, checkerID) ``` </details> ___ **Replace <code>fmt.Sprintf</code> with parameterized queries in <code>GetPoller</code>, <code>GetAgent</code>, and <code>GetChecker</code> to prevent potential SQL injection vulnerabilities.** [pkg/registry/service_registry_queries.go [11-26]](https://github.com/carverauto/serviceradar/pull/1916/files#diff-3591e8646384be68811f862b986342253cabc23176c6f0e09996453baf88a2e9R11-R26) ```diff -func quoteLiteral(value string) string { - return fmt.Sprintf("'%s'", strings.ReplaceAll(value, "'", "''")) -} - // GetPoller retrieves a poller by ID. func (r *ServiceRegistry) GetPoller(ctx context.Context, pollerID string) (*RegisteredPoller, error) { - query := fmt.Sprintf(`SELECT + query := `SELECT poller_id, component_id, status, registration_source, first_registered, first_seen, last_seen, metadata, spiffe_identity, created_by, agent_count, checker_count FROM table(pollers) - WHERE poller_id = %s + WHERE poller_id = ? ORDER BY _tp_time DESC - LIMIT 1`, quoteLiteral(pollerID)) + LIMIT 1` - row := r.db.Conn.QueryRow(ctx, query) + row := r.db.Conn.QueryRow(ctx, query, pollerID) ... ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 10</summary> __ Why: The suggestion correctly identifies a critical SQL injection vulnerability in `GetPoller`, `GetAgent`, and `GetChecker` and proposes the standard, secure fix of using parameterized queries. </details></details></td><td align=center>High </td></tr><tr><td rowspan=3>Possible issue</td> <td> <details><summary>✅ <s>Handle metadata parsing error properly</s></summary> ___ <details><summary>Suggestion Impact:</summary>The commit changed the code to return an error on metadata JSON parsing failure, replacing the previous warning log. It also added sanitization logic, but the key part of the suggestion was implemented. code diff: ```diff @@ -1762,24 +1762,49 @@ metadata := make(map[string]string) if pkg.MetadataJSON != "" { if err := json.Unmarshal([]byte(pkg.MetadataJSON), &metadata); err != nil { - s.logger.Warn().Err(err).Msg("Failed to parse metadata for template substitution") + return "", fmt.Errorf("failed to parse metadata for template substitution: %w", err) + } + } ``` </details> ___ **In <code>substituteTemplateVariables</code>, return an error if parsing <code>pkg.MetadataJSON</code> fails, instead of just logging a warning, to prevent silent failures and invalid configurations.** [pkg/core/edge_onboarding.go [1754-1791]](https://github.com/carverauto/serviceradar/pull/1916/files#diff-85874e3c4bdcc9110db09909f10648d44cdee554b26c987f910502321eb20b5cR1754-R1791) ```diff // substituteTemplateVariables replaces placeholder values in a checker template // with instance-specific values from the edge onboarding package. func (s *edgeOnboardingService) substituteTemplateVariables(templateJSON string, pkg *models.EdgeOnboardingPackage) (string, error) { // Parse the template as a generic map var template map[string]interface{} if err := json.Unmarshal([]byte(templateJSON), &template); err != nil { return "", fmt.Errorf("failed to parse template JSON: %w", err) } // Parse metadata to get addresses and other values metadata := make(map[string]string) if pkg.MetadataJSON != "" { if err := json.Unmarshal([]byte(pkg.MetadataJSON), &metadata); err != nil { - s.logger.Warn().Err(err).Msg("Failed to parse metadata for template substitution") + return "", fmt.Errorf("failed to parse metadata for template substitution: %w", err) } } // Recursively substitute variables in the template substituted := s.substituteInMap(template, map[string]string{ "DOWNSTREAM_SPIFFE_ID": pkg.DownstreamSPIFFEID, "AGENT_ADDRESS": metadata["agent_address"], "CORE_ADDRESS": metadata["core_address"], "CORE_SPIFFE_ID": metadata["core_spiffe_id"], "KV_ADDRESS": metadata["kv_address"], "KV_SPIFFE_ID": metadata["kv_spiffe_id"], "TRUST_DOMAIN": metadata["trust_domain"], "LOG_LEVEL": metadata["log_level"], "COMPONENT_ID": pkg.ComponentID, "CHECKER_KIND": pkg.CheckerKind, "AGENT_ID": pkg.ParentID, }) // Marshal back to JSON result, err := json.Marshal(substituted) if err != nil { return "", fmt.Errorf("failed to marshal substituted template: %w", err) } return string(result), nil } ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: The suggestion correctly identifies that ignoring a JSON parsing error can lead to silent failures and invalid configurations, and proposes the correct fix of returning the error. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>✅ <s>Allow re-onboarding of deleted devices</s></summary> ___ <details><summary>Suggestion Impact:</summary>The commit updated the tombstone filter to only block self-reported updates when the update timestamp is not after the deletion time, and allowed newer updates with logging—enabling re-onboarding. It also added extra logging fields/messages. code diff: ```diff if update.Source == models.DiscoverySourceSelfReported || update.Source == models.DiscoverySourceServiceRadar { - dropped++ + // Block self-reported updates for tombstoned devices unless the update is fresh, + // which can happen during re-onboarding when a device comes back online. + if !update.Timestamp.After(deletedAt) { + dropped++ + r.logger.Info(). + Str("device_id", update.DeviceID). + Str("source", string(update.Source)). + Time("deleted_at", deletedAt). + Time("update_ts", update.Timestamp). + Msg("Blocking self-reported update for tombstoned device") + continue + } + // Update is newer than deletion - allow re-onboarding r.logger.Info(). Str("device_id", update.DeviceID). Str("source", string(update.Source)). Time("deleted_at", deletedAt). - Msg("Blocking self-reported update for tombstoned device") - continue + Time("update_ts", update.Timestamp). + Msg("Allowing self-reported update for re-onboarding (update is newer than deletion)") } ``` </details> ___ **Modify the tombstone filter to allow self-reported updates for a deleted device if the update's timestamp is after the deletion time, enabling device re-onboarding.** [pkg/registry/registry.go [863-871]](https://github.com/carverauto/serviceradar/pull/1916/files#diff-cb61d8f79451b9541de4a8cc0811523a68d15452b2f5971c7618ea5b423cf4ecR863-R871) ```diff if update.Source == models.DiscoverySourceSelfReported || update.Source == models.DiscoverySourceServiceRadar { - dropped++ - r.logger.Info(). - Str("device_id", update.DeviceID). - Str("source", string(update.Source)). - Time("deleted_at", deletedAt). - Msg("Blocking self-reported update for tombstoned device") - continue + // Block self-reported updates for tombstoned devices unless the update is fresh, + // which can happen during re-onboarding. + if !update.Timestamp.After(deletedAt) { + dropped++ + r.logger.Info(). + Str("device_id", update.DeviceID). + Str("source", string(update.Source)). + Time("deleted_at", deletedAt). + Msg("Blocking self-reported update for tombstoned device") + continue + } } ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: This suggestion fixes a significant logic flaw where a deleted device could never be re-onboarded, correctly proposing to allow fresh updates that occur after the deletion timestamp. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>✅ <s>Improve SPIFFE ID parsing logic</s></summary> ___ <details><summary>Suggestion Impact:</summary>The commit updated extractTrustDomain to use strings.TrimPrefix and strings.Index, returning the substring before the first slash and handling empty cases—implementing the suggested robust parsing approach. code diff: ```diff // Example: spiffe://carverauto.dev/ns/edge/poller-1 -> carverauto.dev func extractTrustDomain(spiffeID string) string { // Remove spiffe:// prefix - id := spiffeID - if len(id) > 9 { - id = id[9:] // Remove "spiffe://" - } + id := strings.TrimPrefix(spiffeID, "spiffe://") // Find first / and return everything before it - if idx := len(id); idx > 0 { - for i, c := range id { - if c == '/' { - return id[:i] - } - } + if slashIdx := strings.Index(id, "/"); slashIdx != -1 { + return id[:slashIdx] + } + + if id != "" { return id } ``` </details> ___ **Refactor the <code>extractTrustDomain</code> function to use <code>strings.HasPrefix</code> and <code>strings.Index</code> for more robustly parsing the trust domain from a SPIFFE ID.** [pkg/edgeonboarding/spire.go [231-247]](https://github.com/carverauto/serviceradar/pull/1916/files#diff-5d784c136f3db0128ea90d628cbefdff9e2ba18eba9a1775f54fc630bde8ee1dR231-R247) ```diff -// Remove spiffe:// prefix id := spiffeID -if len(id) > 9 { - id = id[9:] // Remove "spiffe://" +if strings.HasPrefix(id, "spiffe://") { + id = id[9:] } -// Find first / and return everything before it -if idx := len(id); idx > 0 { - for i, c := range id { - if c == '/' { - return id[:i] - } - } +if slashIdx := strings.Index(id, "/"); slashIdx != -1 { + return id[:slashIdx] +} + +if id != "" { return id } return "unknown" ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: The suggestion correctly identifies a fragile implementation and proposes using idiomatic Go functions (`strings.HasPrefix`, `strings.Index`) to make the SPIFFE ID parsing more robust and readable. </details></details></td><td align=center>Low </td></tr><tr><td rowspan=2>General</td> <td> <details><summary>Improve performance with batch database operations</summary> ___ **Refactor <code>RecordBatchHeartbeats</code> to use true database batching for <code>SELECT</code> and <code>INSERT</code> operations to fix the N+1 query problem and improve performance.** [pkg/registry/service_registry.go [294-305]](https://github.com/carverauto/serviceradar/pull/1916/files#diff-d6d6d09a58edde934a1d3f571ff5ce02f0475c5e85ecdec27888892aff8d9d1dR294-R305) ```diff // RecordBatchHeartbeats handles batch heartbeat updates efficiently. func (r *ServiceRegistry) RecordBatchHeartbeats(ctx context.Context, heartbeats []*ServiceHeartbeat) error { + // This function can be improved to perform true batching by: + // 1. Grouping heartbeats by service type. + // 2. For each type, fetching all existing services with a single `SELECT ... WHERE id IN (...)`. + // 3. Preparing a single batch insert (`PrepareBatch`). + // 4. Iterating through the heartbeats, determining the new state for each, and appending to the batch. + // 5. Sending the batch once. + // The current implementation is kept for simplicity, but for high-throughput scenarios, + // a full batch implementation is recommended. for _, hb := range heartbeats { if err := r.RecordHeartbeat(ctx, hb); err != nil { r.logger.Warn(). Err(err). Str("service_type", hb.ServiceType). Str("service_id", hb.ServiceID). Msg("Failed to record heartbeat") } } return nil } ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies a significant performance issue (N+1 problem) where a batch function iterates and makes individual DB calls, but the improved code only adds a comment explaining the problem instead of implementing the fix. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>✅ <s>Fix discarded partition value assignment</s></summary> ___ <details><summary>Suggestion Impact:</summary>The commit updated the code to assign partitionFromDeviceID(existing.DeviceID) to partition and also set update.Partition, preventing the partition value from being discarded. code diff: ```diff if existing, err := s.dbService.GetUnifiedDevice(ctx, deviceID); err == nil { update.IP = existing.IP if partition == "" { - _ = partitionFromDeviceID(existing.DeviceID) + partition = partitionFromDeviceID(existing.DeviceID) + update.Partition = partition } ``` </details> ___ **In <code>deleteDevice</code>, assign the result of <code>partitionFromDeviceID</code> to the <code>partition</code> variable and update <code>DeviceUpdate</code> to ensure the partition is not lost.** [pkg/core/api/device_registry.go [197-206]](https://github.com/carverauto/serviceradar/pull/1916/files#diff-34b7da1b2845de83ee2b0eeba93ef1c8b7abf40517f20617c861abffec32ee1cR197-R206) ```diff if existing, err := s.dbService.GetUnifiedDevice(ctx, deviceID); err == nil { update.IP = existing.IP if partition == "" { - _ = partitionFromDeviceID(existing.DeviceID) + partition = partitionFromDeviceID(existing.DeviceID) + update.Partition = partition } if existing.Hostname != nil && existing.Hostname.Value != "" { ... ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies that a calculated partition value is discarded, and fixing this ensures the device's partition is correctly preserved when it is tombstoned. </details></details></td><td align=center>Medium </td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td><td></td></tr></tbody></table>

qodo-code-review[bot] commented

2025-11-03 04:13:48 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1916#issuecomment-3478847447
Original created: 2025-11-03T04:13:48Z

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: cpufreq-clang-tidy

Failed stage: Run clang-tidy via Bazel [❌]

Failure summary:

Bazel failed during analysis because the OPAM extension could not install the OCaml package dream
non-interactively:
- In external/tools_opam+/extensions/opam/opam_ops.bzl:142:13, fail(...) was
triggered with rc=10 from the command opam install dream --switch 5.2.0 --root
/Users/runner/.local/share/obazl/opam/2.4.1/root --yes.
- OPAM detected missing external system
dependencies and prompted for action, but the CI run is non-interactive. It suggested rerunning with
--assume-depexts or setting opam option depext=false, and noted that running the system package
manager non-interactively requires --confirm-level=unsafe-yes.
- As a result, not all targets were
analyzed and Bazel reported "Build did NOT complete successfully."

Relevant error logs:

1:  ##[group]Runner Image Provisioner
2:  Hosted Compute Agent
...

96:  ##[endgroup]
97:  [command]/opt/homebrew/bin/git log -1 --format=%H
98:  7d2178c8b40be9ce936a8947ad7b5ef848f59779
99:  ##[group]Run bazelbuild/setup-bazelisk@v3
100:  with:
101:  bazelisk-version: 1.x
102:  token: ***
103:  env:
104:  BUILDBUDDY_ORG_API_KEY: ***
105:  ##[endgroup]
106:  Attempting to download 1.x...
107:  Acquiring v1.27.0 from https://github.com/bazelbuild/bazelisk/releases/download/v1.27.0/bazelisk-darwin-arm64
108:  Adding to the cache ...
109:  Successfully cached bazelisk to /Users/runner/hostedtoolcache/bazelisk/1.27.0/arm64
110:  Added bazelisk to the path
111:  ##[warning]Failed to restore: Cache service responded with 400
112:  Restored bazelisk cache dir @ /Users/runner/Library/Caches/bazelisk
...

3822:  ^[[1A^[[K(04:13:34) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\
3823:  ded, 23 targets configured)
3824:  Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 479s
3825:  ^[[1A^[[K
3826:  ^[[1A^[[K
3827:  ^[[1A^[[K(04:13:35) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\
3828:  ded, 23 targets configured)
3829:  Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 480s
3830:  ^[[1A^[[K
3831:  ^[[1A^[[K
3832:  ^[[1A^[[K(04:13:36) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\
3833:  ded, 23 targets configured)
3834:  Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 481s
3835:  ^[[1A^[[K
3836:  ^[[1A^[[K
3837:  ^[[1A^[[K(04:13:37) ^[[31m^[[1mERROR: ^[[0m/private/var/tmp/_bazel_runner/ec3d3b853b03cdb473182512380bf060/external/tools_opam+/extensions/opam/opam_ops.bzl:142:13: Traceback (most recent call last):
3838:  File "/private/var/tmp/_bazel_runner/ec3d3b853b03cdb473182512380bf060/external/tools_opam+/extensions/opam.bzl", line 440, column 53, in _opam_ext_impl
3839:  ocaml_version, deps) = config_xdg_toolchain(
3840:  File "/private/var/tmp/_bazel_runner/ec3d3b853b03cdb473182512380bf060/external/tools_opam+/extensions/opam/opam_toolchain_xdg.bzl", line 375, column 29, in config_xdg_toolchain
3841:  opam_install_pkg(mctx,
3842:  File "/private/var/tmp/_bazel_runner/ec3d3b853b03cdb473182512380bf060/external/tools_opam+/extensions/opam/opam_ops.bzl", line 142, column 13, in opam_install_pkg
3843:  fail("opam install failed; cmd=%s rc=%s\nstdout:%s\nstderr:%s" % (
3844:  Error in fail: opam install failed; cmd=["/Users/runner/.local/share/obazl/opam/2.4.1/bin/opam", "install", "dream", "--switch", "5.2.0", "--root", "/Users/runner/.local/share/obazl/opam/2.4.1/root", "--yes"] rc=10
3845:  stdout:The following actions will be performed:
...

3906:  libev
3907:  <><> Handling external dependencies <><><><><><><><><><><><><><><><><><><><>  🐫 
3908:  opam believes some required external dependencies are missing. opam can:
3909:  > 1. Run brew to install them (may need root/sudo access)
3910:  2. Display the recommended brew command and wait while you run it manually (e.g. in another terminal)
3911:  3. Continue anyway, and, upon success, permanently register that this external dependency is present, but not detectable
3912:  4. Abort the installation
3913:  [1/2/3/4] 4
3914:  stderr:[NOTE] You can retry with '--assume-depexts' to skip this check, or run 'opam option depext=false' to permanently disable handling of system packages.
3915:  Running the system package manager non-interactively requires '--confirm-level=unsafe-yes'.
3916:  (04:13:37) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\
3917:  ded, 23 targets configured)
3918:  Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 482s
3919:  ^[[1A^[[K
3920:  ^[[1A^[[K
3921:  ^[[1A^[[K(04:13:37) ^[[35mWARNING: ^[[0merrors encountered while analyzing target '//pkg/cpufreq:hostfreq_darwin_cc', it will not be built.
3922:  error evaluating module extension @@tools_opam+//extensions:opam.bzl%opam
3923:  (04:13:37) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\
3924:  ded, 23 targets configured)
3925:  Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 482s
3926:  ^[[1A^[[K
3927:  ^[[1A^[[K
3928:  ^[[1A^[[K(04:13:37) ^[[32mINFO: ^[[0mFound 0 targets...
3929:  (04:13:37) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\
3930:  ded, 23 targets configured)
3931:  Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 482s
3932:  ^[[1A^[[K
3933:  ^[[1A^[[K
3934:  ^[[1A^[[K(04:13:37) ^[[31m^[[1mERROR: ^[[0mcommand succeeded, but not all targets were analyzed
3935:  (04:13:37) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\
...

3937:  Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 482s
3938:  ^[[1A^[[K
3939:  ^[[1A^[[K
3940:  ^[[1A^[[K(04:13:37) ^[[32mINFO: ^[[0mElapsed time: 495.754s, Critical Path: 1.96s
3941:  (04:13:37) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\
3942:  ded, 23 targets configured)
3943:  Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 482s
3944:  ^[[1A^[[K
3945:  ^[[1A^[[K
3946:  ^[[1A^[[K(04:13:37) ^[[32mINFO: ^[[0m1 process: 1 internal.
3947:  (04:13:37) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\
3948:  ded, 23 targets configured)
3949:  Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 482s
3950:  ^[[1A^[[K
3951:  ^[[1A^[[K
3952:  ^[[1A^[[K(04:13:37) ^[[31m^[[1mERROR: ^[[0mBuild did NOT complete successfully
3953:  (04:13:37) ^[[31m^[[1mFAILED:^[[0m 
3954:  Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 482s
3955:  ^[[1A^[[K
3956:  ^[[1A^[[K(04:13:37) ^[[31m^[[1mFAILED:^[[0m 
3957:  Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 482s
3958:  ^[[0m
3959:  ##[error]Process completed with exit code 1.
3960:  Post job cleanup.

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1916#issuecomment-3478847447 Original created: 2025-11-03T04:13:48Z --- ## CI Feedback 🧐 A test triggered by this PR failed. Here is an AI-generated analysis of the failure: <table><tr><td> **Action:** cpufreq-clang-tidy</td></tr> <tr><td> **Failed stage:** [Run clang-tidy via Bazel](https://github.com/carverauto/serviceradar/actions/runs/19023562578/job/54323155330) [❌] </td></tr> <tr><td> **Failure summary:** Bazel failed during analysis because the OPAM extension could not install the OCaml package <code>dream</code> non-interactively: - In <code>external/tools_opam+/extensions/opam/opam_ops.bzl:142:13</code>, <code>fail(...)</code> was triggered with rc=10 from the command <code>opam install dream --switch 5.2.0 --root </code> <code>/Users/runner/.local/share/obazl/opam/2.4.1/root --yes</code>. - OPAM detected missing external system dependencies and prompted for action, but the CI run is non-interactive. It suggested rerunning with <code>--assume-depexts</code> or setting <code>opam option depext=false</code>, and noted that running the system package manager non-interactively requires <code>--confirm-level=unsafe-yes</code>. - As a result, not all targets were analyzed and Bazel reported "Build did NOT complete successfully." </td></tr> <tr><td> <details><summary>Relevant error logs:</summary> ```yaml 1: ##[group]Runner Image Provisioner 2: Hosted Compute Agent ... 96: ##[endgroup] 97: [command]/opt/homebrew/bin/git log -1 --format=%H 98: 7d2178c8b40be9ce936a8947ad7b5ef848f59779 99: ##[group]Run bazelbuild/setup-bazelisk@v3 100: with: 101: bazelisk-version: 1.x 102: token: *** 103: env: 104: BUILDBUDDY_ORG_API_KEY: *** 105: ##[endgroup] 106: Attempting to download 1.x... 107: Acquiring v1.27.0 from https://github.com/bazelbuild/bazelisk/releases/download/v1.27.0/bazelisk-darwin-arm64 108: Adding to the cache ... 109: Successfully cached bazelisk to /Users/runner/hostedtoolcache/bazelisk/1.27.0/arm64 110: Added bazelisk to the path 111: ##[warning]Failed to restore: Cache service responded with 400 112: Restored bazelisk cache dir @ /Users/runner/Library/Caches/bazelisk ... 3822: ^[[1A^[[K(04:13:34) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\ 3823: ded, 23 targets configured) 3824: Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 479s 3825: ^[[1A^[[K 3826: ^[[1A^[[K 3827: ^[[1A^[[K(04:13:35) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\ 3828: ded, 23 targets configured) 3829: Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 480s 3830: ^[[1A^[[K 3831: ^[[1A^[[K 3832: ^[[1A^[[K(04:13:36) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\ 3833: ded, 23 targets configured) 3834: Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 481s 3835: ^[[1A^[[K 3836: ^[[1A^[[K 3837: ^[[1A^[[K(04:13:37) ^[[31m^[[1mERROR: ^[[0m/private/var/tmp/_bazel_runner/ec3d3b853b03cdb473182512380bf060/external/tools_opam+/extensions/opam/opam_ops.bzl:142:13: Traceback (most recent call last): 3838: File "/private/var/tmp/_bazel_runner/ec3d3b853b03cdb473182512380bf060/external/tools_opam+/extensions/opam.bzl", line 440, column 53, in _opam_ext_impl 3839: ocaml_version, deps) = config_xdg_toolchain( 3840: File "/private/var/tmp/_bazel_runner/ec3d3b853b03cdb473182512380bf060/external/tools_opam+/extensions/opam/opam_toolchain_xdg.bzl", line 375, column 29, in config_xdg_toolchain 3841: opam_install_pkg(mctx, 3842: File "/private/var/tmp/_bazel_runner/ec3d3b853b03cdb473182512380bf060/external/tools_opam+/extensions/opam/opam_ops.bzl", line 142, column 13, in opam_install_pkg 3843: fail("opam install failed; cmd=%s rc=%s\nstdout:%s\nstderr:%s" % ( 3844: Error in fail: opam install failed; cmd=["/Users/runner/.local/share/obazl/opam/2.4.1/bin/opam", "install", "dream", "--switch", "5.2.0", "--root", "/Users/runner/.local/share/obazl/opam/2.4.1/root", "--yes"] rc=10 3845: stdout:The following actions will be performed: ... 3906: libev 3907: <><> Handling external dependencies <><><><><><><><><><><><><><><><><><><><> 🐫 3908: opam believes some required external dependencies are missing. opam can: 3909: > 1. Run brew to install them (may need root/sudo access) 3910: 2. Display the recommended brew command and wait while you run it manually (e.g. in another terminal) 3911: 3. Continue anyway, and, upon success, permanently register that this external dependency is present, but not detectable 3912: 4. Abort the installation 3913: [1/2/3/4] 4 3914: stderr:[NOTE] You can retry with '--assume-depexts' to skip this check, or run 'opam option depext=false' to permanently disable handling of system packages. 3915: Running the system package manager non-interactively requires '--confirm-level=unsafe-yes'. 3916: (04:13:37) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\ 3917: ded, 23 targets configured) 3918: Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 482s 3919: ^[[1A^[[K 3920: ^[[1A^[[K 3921: ^[[1A^[[K(04:13:37) ^[[35mWARNING: ^[[0merrors encountered while analyzing target '//pkg/cpufreq:hostfreq_darwin_cc', it will not be built. 3922: error evaluating module extension @@tools_opam+//extensions:opam.bzl%opam 3923: (04:13:37) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\ 3924: ded, 23 targets configured) 3925: Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 482s 3926: ^[[1A^[[K 3927: ^[[1A^[[K 3928: ^[[1A^[[K(04:13:37) ^[[32mINFO: ^[[0mFound 0 targets... 3929: (04:13:37) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\ 3930: ded, 23 targets configured) 3931: Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 482s 3932: ^[[1A^[[K 3933: ^[[1A^[[K 3934: ^[[1A^[[K(04:13:37) ^[[31m^[[1mERROR: ^[[0mcommand succeeded, but not all targets were analyzed 3935: (04:13:37) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\ ... 3937: Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 482s 3938: ^[[1A^[[K 3939: ^[[1A^[[K 3940: ^[[1A^[[K(04:13:37) ^[[32mINFO: ^[[0mElapsed time: 495.754s, Critical Path: 1.96s 3941: (04:13:37) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\ 3942: ded, 23 targets configured) 3943: Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 482s 3944: ^[[1A^[[K 3945: ^[[1A^[[K 3946: ^[[1A^[[K(04:13:37) ^[[32mINFO: ^[[0m1 process: 1 internal. 3947: (04:13:37) ^[[32mAnalyzing:^[[0m target //pkg/cpufreq:hostfreq_darwin_cc (89 packages loa\ 3948: ded, 23 targets configured) 3949: Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 482s 3950: ^[[1A^[[K 3951: ^[[1A^[[K 3952: ^[[1A^[[K(04:13:37) ^[[31m^[[1mERROR: ^[[0mBuild did NOT complete successfully 3953: (04:13:37) ^[[31m^[[1mFAILED:^[[0m 3954: Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 482s 3955: ^[[1A^[[K 3956: ^[[1A^[[K(04:13:37) ^[[31m^[[1mFAILED:^[[0m 3957: Fetching ...extensions:opam.bzl%opam; Installing pkg dream (21 of 22) 482s 3958: ^[[0m 3959: ##[error]Process completed with exit code 1. 3960: Post job cleanup. ``` </details></td></tr></table>

mfreeman451 commented

2025-11-03 04:47:02 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1916#discussion_r2485300587
Original created: 2025-11-03T04:47:02Z
Original path: pkg/edgeonboarding/bootstrap.go
Original line: 244

should probably be storing this in the NATS JetStream object store instead

Imported GitHub PR review comment. Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1916#discussion_r2485300587 Original created: 2025-11-03T04:47:02Z Original path: pkg/edgeonboarding/bootstrap.go Original line: 244 --- should probably be storing this in the NATS JetStream object store instead