carverauto/serviceradar

Fork 0

2793 feat mikrotik support api #3018

Merged

mfreeman451 merged 3 commits from refs/pull/3018/head into staging

2026-03-07 00:34:17 +00:00

mfreeman451 commented

2026-03-06 21:15:08 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2998
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2998
Original created: 2026-03-06T21:15:08Z
Original updated: 2026-03-07T00:34:19Z
Original head: carverauto/serviceradar:2793-feat-mikrotik-support-api
Original base: staging
Original merged: 2026-03-07T00:34:17Z by @mfreeman451

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?

Imported from GitHub pull request. Original GitHub pull request: #2998 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2998 Original created: 2026-03-06T21:15:08Z Original updated: 2026-03-07T00:34:19Z Original head: carverauto/serviceradar:2793-feat-mikrotik-support-api Original base: staging Original merged: 2026-03-07T00:34:17Z by @mfreeman451 --- ## 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?

qodo-code-review[bot] commented

2026-03-06 21:16: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/2998#issuecomment-4014200864
Original created: 2026-03-06T21:16:06Z

Review Summary by Qodo

Add MikroTik RouterOS REST API discovery support with encrypted credentials and device enrichment

✨ Enhancement 🧪 Tests 📝 Documentation

Walkthroughs

Description

• Added comprehensive MikroTik RouterOS REST API discovery support alongside existing UniFi
  integration
• Implemented new MapperMikrotikController Ash resource with encrypted password storage and URL
  normalization for RouterOS API endpoint configuration
• Created Go RouterOS poller module (mikrotik_poller.go) querying system identity, resources,
  interfaces, addresses, bridges, VLANs, and neighbors
• Integrated MikroTik discovery into mapper phases with generic API selector utility for endpoint
  filtering
• Extended device inventory enrichment to infer and merge RouterOS metadata (OS name/version, serial
  number, CPU architecture) during device upserts
• Refactored UniFi poller to use new generic selectNamedBaseURLConfigs() API selector function
• Added database migration for mapper_mikrotik_controllers table with encrypted password and
  unique constraints
• Updated mapper configuration gateway structures to support MikroTik API endpoints and discovery
  mode specification
• Implemented MikroTik-specific form handling and two-column grid UI for API discovery in settings
• Added comprehensive test coverage for RouterOS poller, controller resource validation, metadata
  ingestion, and configuration parsing
• Included design documentation, user guides, and implementation task tracking for RouterOS
  discovery feature

Diagram

flowchart LR
  A["MikroTik<br/>Controller Config"] -->|encrypted credentials| B["MapperMikrotikController<br/>Ash Resource"]
  B -->|load & compile| C["Mapper Compiler"]
  C -->|generate config| D["Gateway Config<br/>with MikroTik APIs"]
  D -->|parse & convert| E["Go Mapper Engine"]
  E -->|query REST API| F["RouterOS Poller"]
  F -->|discover devices<br/>interfaces, topology| G["Discovery Phase"]
  G -->|ingest metadata| H["Sync Ingestor"]
  H -->|enrich inventory| I["Device Inventory<br/>with OS/HW Info"]

File Changes

1. elixir/web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex ✨ Enhancement +262/-43

MikroTik RouterOS API discovery UI and form handling

• Added MapperMikrotikController alias and integrated MikroTik form handling alongside UniFi
• Implemented MikroTik-specific form initialization, normalization, and persistence functions
• Refactored API discovery UI to display UniFi and MikroTik controllers in a two-column grid layout
• Extended mapper job compilation to load and pass MikroTik controller data to the agent

elixir/web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex

2. elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_compiler_test.exs 🧪 Tests +103/-19

MikroTik controller compilation and mapper config tests

• Added MapperMikrotikController alias and test setup for MikroTik table creation
• Refactored existing tests to use unique identifiers and improved test isolation
• Added new integration test validating MikroTik controller compilation into mapper config
• Updated test assertions to handle multiple scheduled jobs and verify MikroTik API selectors

elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_compiler_test.exs

3. elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex ✨ Enhancement +71/-0

RouterOS metadata inference and device enrichment

• Added infer_os() function to extract RouterOS name and version from metadata
• Added infer_hw_info() function to extract serial number and CPU architecture
• Added routeros_metadata?() helper to detect RouterOS devices by version, vendor, or sys_descr
• Extended device merge logic to combine OS and hardware info maps during upserts

elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex

View more (24)

4. elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/mapper_compiler.ex ✨ Enhancement +33/-2

MikroTik controller loading and compiler output generation

• Added MapperMikrotikController to source resources and load pipeline
• Implemented load_mikrotik_controllers() and compile_mikrotik_controller() functions
• Extended job compilation to extract and pass MikroTik API names and URLs as CSV options
• Updated compiler output to include mikrotik_apis alongside existing unifi_apis

elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/mapper_compiler.ex

5. elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_mikrotik_controller.ex ✨ Enhancement +157/-0

New MikroTik controller Ash resource with encryption

• New Ash resource for storing MikroTik RouterOS REST API endpoint configuration
• Implements encrypted password storage via AshCloak vault integration
• Includes NormalizeMikrotikBaseUrl change to validate and normalize URLs to /rest path
• Defines policies for admin/operator create/update and system bypass access

elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_mikrotik_controller.ex

6. elixir/web-ng/test/phoenix/live/settings/networks_live_test.exs 🧪 Tests +61/-1

MikroTik form rendering and credential masking tests
• Added MapperMikrotikController alias and MikroTik table setup in test fixtures
• Added test validating MikroTik API form fields render in discovery job form
• Added test verifying masked password placeholder for stored MikroTik credentials
elixir/web-ng/test/phoenix/live/settings/networks_live_test.exs

7. elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs 🧪 Tests +73/-0

RouterOS metadata ingestion and device enrichment tests
• Added test validating RouterOS metadata mapping to canonical os and hw_info fields
• Added test verifying enrichment of existing device with RouterOS metadata while preserving
 identity state
elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs

8. elixir/serviceradar_core/lib/serviceradar/network_discovery/changes/normalize_mikrotik_base_url.ex ✨ Enhancement +103/-0

MikroTik URL normalization and validation change
• New Ash change module validating and normalizing MikroTik RouterOS REST API URLs
• Accepts host-only URLs (e.g., https://192.168.88.1) and normalizes to /rest path
• Rejects non-REST paths and validates scheme/host presence
• Handles missing scheme by prepending https://
elixir/serviceradar_core/lib/serviceradar/network_discovery/changes/normalize_mikrotik_base_url.ex

9. elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_mikrotik_controller_test.exs 🧪 Tests +82/-0

MikroTik controller resource validation tests
• New test module for MapperMikrotikController resource validation
• Tests URL normalization from host-only to /rest endpoint
• Tests rejection of non-REST paths with appropriate error messages
elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_mikrotik_controller_test.exs

10. elixir/serviceradar_core/priv/repo/migrations/20260306180000_add_mapper_mikrotik_controllers.exs ⚙️ Configuration changes +53/-0

Database migration for MikroTik controller storage
• New Ecto migration creating mapper_mikrotik_controllers table in platform schema
• Defines columns for name, base_url, username, encrypted_password, insecure_skip_verify, and
 mapper_job_id
• Creates indexes on mapper_job_id and unique constraint on (mapper_job_id, base_url) pair
elixir/serviceradar_core/priv/repo/migrations/20260306180000_add_mapper_mikrotik_controllers.exs

11. elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_job.ex ✨ Enhancement +4/-0

MapperJob relationship to MikroTik controllers
• Added has_many relationship to MapperMikrotikController resources
elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_job.ex

12. elixir/serviceradar_core/lib/serviceradar/network_discovery.ex ✨ Enhancement +1/-0

NetworkDiscovery domain resource registration
• Registered MapperMikrotikController resource in the NetworkDiscovery domain
elixir/serviceradar_core/lib/serviceradar/network_discovery.ex

13. go/pkg/mapper/mikrotik_poller.go ✨ Enhancement +693/-0

RouterOS REST API poller implementation

• New Go module implementing RouterOS REST API discovery via HTTP client
• Queries system identity, resource, routerboard, interfaces, addresses, bridge ports, VLANs, and
 neighbors
• Normalizes RouterOS responses into DiscoveredDevice, DiscoveredInterface, and TopologyLink
 structures
• Handles unsupported endpoints gracefully and extracts metadata for device enrichment

go/pkg/mapper/mikrotik_poller.go

14. go/pkg/mapper/mikrotik_poller_test.go 🧪 Tests +248/-0

RouterOS poller integration tests

• New test module with HTTP mock server validating RouterOS API queries
• Tests successful device, interface, and topology link discovery from RouterOS endpoints
• Tests graceful handling of unsupported routerboard endpoint on CHR instances

go/pkg/mapper/mikrotik_poller_test.go

15. go/pkg/mapper/discovery.go ✨ Enhancement +41/-9

MikroTik discovery integration into mapper phases

• Added sourceAdapterMikroTikV1 constant for MikroTik source adapter versioning
• Integrated queryMikroTikDevices() call into UniFi discovery phase with error handling
• Updated phase logging to reference generic "API discovery" instead of UniFi-specific terminology
• Extended applySourceAdapterVersion() to tag MikroTik topology links with adapter metadata

go/pkg/mapper/discovery.go

16. go/pkg/agent/mapper_config_gateway.go ✨ Enhancement +61/-30

MikroTik API configuration gateway structures

• Added MikroTikAPIs field to gatewayMapperConfig struct
• Added mapperMikroTikSpec struct for MikroTik endpoint configuration
• Implemented convertMapperMikroTik() function to transform gateway config to engine config
• Added DiscoveryMode field to mapperJobSpec for job type specification

go/pkg/agent/mapper_config_gateway.go

17. go/pkg/agent/mapper_config_gateway_test.go 🧪 Tests +143/-0

MikroTik gateway configuration parsing tests

• New test module validating MikroTik endpoint parsing and round-tripping
• Tests mapper engine config building with MikroTik endpoints and discovery mode
• Tests config hash computation for change detection

go/pkg/agent/mapper_config_gateway_test.go

18. go/pkg/mapper/api_selector.go ✨ Enhancement +67/-0

Generic API endpoint selector utility

• New generic API selector module implementing selectNamedBaseURLConfigs() function
• Provides reusable filtering logic for named and URL-based API endpoint selection
• Supports both UniFi and MikroTik API filtering with consistent conventions

go/pkg/mapper/api_selector.go

19. go/pkg/mapper/ubnt_poller.go ✨ Enhancement +11/-30

UniFi poller refactored to use generic API selector
• Refactored unifiAPIsForJob() to use new selectNamedBaseURLConfigs() generic selector
• Simplified UniFi API filtering logic by delegating to shared selector function
go/pkg/mapper/ubnt_poller.go

20. go/pkg/mapper/types.go ✨ Enhancement +10/-0

 Mapper configuration types for MikroTik API support

go/pkg/mapper/types.go

21. openspec/changes/add-mikrotik-routeros-discovery/design.md 📝 Documentation +156/-0

MikroTik RouterOS discovery design and architecture

• New design document outlining MikroTik RouterOS REST API discovery architecture
• Documents goals, non-goals, and key decisions (Go implementation, REST API first, read-only)
• Specifies data coverage (device identity, interfaces, L2/L3 context, topology evidence)
• Includes validation plan and demo baseline for CHR target

openspec/changes/add-mikrotik-routeros-discovery/design.md

22. docs/docs/discovery.md 📝 Documentation +71/-0

MikroTik RouterOS discovery user documentation

• Added section documenting MikroTik RouterOS API discovery setup and requirements
• Documented phase 1 coverage including device identity, interfaces, L2/L3 context, and neighbors
• Provided demo validation commands and success criteria for RouterOS API integration

docs/docs/discovery.md

23. openspec/changes/add-mikrotik-routeros-discovery/specs/network-discovery/spec.md 📝 Documentation +49/-0

MikroTik RouterOS API discovery requirements and scenarios

• Defines requirements for MikroTik RouterOS API discovery source supporting device identity,
 interfaces, bridges, VLANs, and IP address collection
• Specifies graceful degradation when RouterOS resources are unavailable due to firmware/version
 differences
• Documents RouterOS connection settings storage with encrypted secrets and UI redaction
• Establishes topology evidence ingestion rules ensuring SNMP-attributed evidence remains
 authoritative while RouterOS API evidence serves as supplemental structural data

openspec/changes/add-mikrotik-routeros-discovery/specs/network-discovery/spec.md

24. openspec/changes/add-mikrotik-routeros-discovery/tasks.md 📝 Documentation +32/-0

MikroTik RouterOS discovery implementation task checklist

• Outlines implementation tasks across four areas: core configuration, Go mapper,
 ingestion/inventory, and documentation
• Tracks completion status with checkmarks for tasks 1.1-4.3 and pending validation task 4.4
• Documents current baseline state showing shallow SNMP-era inventory for live MikroTik CHR demo
 environment

openspec/changes/add-mikrotik-routeros-discovery/tasks.md

25. openspec/changes/add-mikrotik-routeros-discovery/proposal.md 📝 Documentation +37/-0

MikroTik RouterOS API discovery feature proposal and scope

• Explains motivation: existing MikroTik enrichment rules lack corresponding API discovery path
 compared to UniFi integration
• Details scope including read-only RouterOS discovery source, encrypted credentials, Go poller, and
 inventory enrichment
• Lists affected specifications and code modules across Elixir core and Go mapper
• Clarifies non-goals excluding configuration changes, CAPsMAN, BGP, and socket tunneling

openspec/changes/add-mikrotik-routeros-discovery/proposal.md

26. openspec/changes/add-mikrotik-routeros-discovery/specs/device-inventory/spec.md 📝 Documentation +23/-0

RouterOS API device inventory enrichment requirements

• Specifies RouterOS API enrichment of canonical device inventory for vendor, model, OS, and
 hardware metadata
• Defines scenarios for populating vendor_name, model, os, and hw_info fields from RouterOS
 discovery data
• Establishes precedence rules preserving existing stronger user-managed values and preventing
 duplicate records

openspec/changes/add-mikrotik-routeros-discovery/specs/device-inventory/spec.md

27. go/pkg/mapper/BUILD.bazel ✨ Enhancement +2/-0

Add MikroTik RouterOS poller to Go mapper build

• Adds mikrotik_poller.go to the Go library sources for RouterOS API polling implementation
• Adds mikrotik_poller_test.go to the test sources for RouterOS poller unit tests

go/pkg/mapper/BUILD.bazel

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2998#issuecomment-4014200864 Original created: 2026-03-06T21:16:06Z --- <h3>Review Summary by Qodo</h3> Add MikroTik RouterOS REST API discovery support with encrypted credentials and device enrichment <code>✨ Enhancement</code> <code>🧪 Tests</code> <code>📝 Documentation</code> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <h3>Walkthroughs</h3> <details open> <summary>Description</summary> <pre> • Added comprehensive MikroTik RouterOS REST API discovery support alongside existing UniFi integration • Implemented new MapperMikrotikController Ash resource with encrypted password storage and URL normalization for RouterOS API endpoint configuration • Created Go RouterOS poller module (mikrotik_poller.go) querying system identity, resources, interfaces, addresses, bridges, VLANs, and neighbors • Integrated MikroTik discovery into mapper phases with generic API selector utility for endpoint filtering • Extended device inventory enrichment to infer and merge RouterOS metadata (OS name/version, serial number, CPU architecture) during device upserts • Refactored UniFi poller to use new generic selectNamedBaseURLConfigs() API selector function • Added database migration for mapper_mikrotik_controllers table with encrypted password and unique constraints • Updated mapper configuration gateway structures to support MikroTik API endpoints and discovery mode specification • Implemented MikroTik-specific form handling and two-column grid UI for API discovery in settings • Added comprehensive test coverage for RouterOS poller, controller resource validation, metadata ingestion, and configuration parsing • Included design documentation, user guides, and implementation task tracking for RouterOS discovery feature </pre> </details> <details> <summary>Diagram</summary> > ```mermaid flowchart LR A["MikroTik Controller Config"] -->|encrypted credentials| B["MapperMikrotikController Ash Resource"] B -->|load & compile| C["Mapper Compiler"] C -->|generate config| D["Gateway Config with MikroTik APIs"] D -->|parse & convert| E["Go Mapper Engine"] E -->|query REST API| F["RouterOS Poller"] F -->|discover devices interfaces, topology| G["Discovery Phase"] G -->|ingest metadata| H["Sync Ingestor"] H -->|enrich inventory| I["Device Inventory with OS/HW Info"] ``` </details> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <h3>File Changes</h3> <details> <summary>1. elixir/web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex <code>✨ Enhancement</code> <code> +262/-43 </code> </summary> >MikroTik RouterOS API discovery UI and form handling ><pre> >• Added MapperMikrotikController alias and integrated MikroTik form handling alongside UniFi >• Implemented MikroTik-specific form initialization, normalization, and persistence functions >• Refactored API discovery UI to display UniFi and MikroTik controllers in a two-column grid layout >• Extended mapper job compilation to load and pass MikroTik controller data to the agent ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-f013f3501a2fa2f6d451c632a9c3fab35491171275db26597ac3c3d2e8cee481'> elixir/web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex </a> <hr/> </details> <details> <summary>2. elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_compiler_test.exs <code>🧪 Tests</code> <code> +103/-19 </code> </summary> >MikroTik controller compilation and mapper config tests ><pre> >• Added MapperMikrotikController alias and test setup for MikroTik table creation >• Refactored existing tests to use unique identifiers and improved test isolation >• Added new integration test validating MikroTik controller compilation into mapper config >• Updated test assertions to handle multiple scheduled jobs and verify MikroTik API selectors ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-a66580ac3bd92e18fbff9a00f884c2f826061fbb9ae6535a704eb9b29ab5c5bd'> elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_compiler_test.exs </a> <hr/> </details> <details> <summary>3. elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex <code>✨ Enhancement</code> <code> +71/-0 </code> </summary> >RouterOS metadata inference and device enrichment ><pre> >• Added infer_os() function to extract RouterOS name and version from metadata >• Added infer_hw_info() function to extract serial number and CPU architecture >• Added routeros_metadata?() helper to detect RouterOS devices by version, vendor, or sys_descr >• Extended device merge logic to combine OS and hardware info maps during upserts ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-fdf70a310cef758f735fae943c2a33bc7f851a1c3d1ba66499e911fd2bc5611a'> elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex </a> <hr/> </details> <details><summary><ins>View more (24)</ins></summary> <details> <summary>4. elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/mapper_compiler.ex <code>✨ Enhancement</code> <code> +33/-2 </code> </summary> >MikroTik controller loading and compiler output generation ><pre> >• Added MapperMikrotikController to source resources and load pipeline >• Implemented load_mikrotik_controllers() and compile_mikrotik_controller() functions >• Extended job compilation to extract and pass MikroTik API names and URLs as CSV options >• Updated compiler output to include mikrotik_apis alongside existing unifi_apis ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-2b9795cd8468dee006a68b823afa9a155e96f24d0895077e9772ac6399a6ac3e'> elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/mapper_compiler.ex </a> <hr/> </details> <details> <summary>5. elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_mikrotik_controller.ex <code>✨ Enhancement</code> <code> +157/-0 </code> </summary> >New MikroTik controller Ash resource with encryption ><pre> >• New Ash resource for storing MikroTik RouterOS REST API endpoint configuration >• Implements encrypted password storage via AshCloak vault integration >• Includes NormalizeMikrotikBaseUrl change to validate and normalize URLs to /rest path >• Defines policies for admin/operator create/update and system bypass access ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-51c05c59b3a9f4418fd8932e943734792ba08d5c3371e31d9be39de774c25851'> elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_mikrotik_controller.ex </a> <hr/> </details> <details> <summary>6. elixir/web-ng/test/phoenix/live/settings/networks_live_test.exs <code>🧪 Tests</code> <code> +61/-1 </code> </summary> >MikroTik form rendering and credential masking tests ><pre> >• Added MapperMikrotikController alias and MikroTik table setup in test fixtures >• Added test validating MikroTik API form fields render in discovery job form >• Added test verifying masked password placeholder for stored MikroTik credentials ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-c086990f74b3d6b1a80d3b962579d8f728a710725c7f102599e7ead75d93716f'> elixir/web-ng/test/phoenix/live/settings/networks_live_test.exs </a> <hr/> </details> <details> <summary>7. elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs <code>🧪 Tests</code> <code> +73/-0 </code> </summary> >RouterOS metadata ingestion and device enrichment tests ><pre> >• Added test validating RouterOS metadata mapping to canonical os and hw_info fields >• Added test verifying enrichment of existing device with RouterOS metadata while preserving > identity state ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-9a0dabe8ab58de6380fcc7be4060502a561ba1b2b378366c16b4734a41545026'> elixir/serviceradar_core/test/serviceradar/inventory/sync_ingestor_vendor_type_test.exs </a> <hr/> </details> <details> <summary>8. elixir/serviceradar_core/lib/serviceradar/network_discovery/changes/normalize_mikrotik_base_url.ex <code>✨ Enhancement</code> <code> +103/-0 </code> </summary> >MikroTik URL normalization and validation change ><pre> >• New Ash change module validating and normalizing MikroTik RouterOS REST API URLs >• Accepts host-only URLs (e.g., https://192.168.88.1) and normalizes to /rest path >• Rejects non-REST paths and validates scheme/host presence >• Handles missing scheme by prepending https:// ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-54a8e3dfd2cf02dee3f730fff1da9595ebeab77cc48015fda4f62784de68ad8f'> elixir/serviceradar_core/lib/serviceradar/network_discovery/changes/normalize_mikrotik_base_url.ex </a> <hr/> </details> <details> <summary>9. elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_mikrotik_controller_test.exs <code>🧪 Tests</code> <code> +82/-0 </code> </summary> >MikroTik controller resource validation tests ><pre> >• New test module for MapperMikrotikController resource validation >• Tests URL normalization from host-only to /rest endpoint >• Tests rejection of non-REST paths with appropriate error messages ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-ee11b306aefda568a63826d1087a68f6377c82faa5db7ad9c85d546b56783fd3'> elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_mikrotik_controller_test.exs </a> <hr/> </details> <details> <summary>10. elixir/serviceradar_core/priv/repo/migrations/20260306180000_add_mapper_mikrotik_controllers.exs <code>⚙️ Configuration changes</code> <code> +53/-0 </code> </summary> >Database migration for MikroTik controller storage ><pre> >• New Ecto migration creating mapper_mikrotik_controllers table in platform schema >• Defines columns for name, base_url, username, encrypted_password, insecure_skip_verify, and > mapper_job_id >• Creates indexes on mapper_job_id and unique constraint on (mapper_job_id, base_url) pair ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-7d628cd7843fb3136b74c21066be2e3f71659f2597db12d08e47c4e57ac6c4e6'> elixir/serviceradar_core/priv/repo/migrations/20260306180000_add_mapper_mikrotik_controllers.exs </a> <hr/> </details> <details> <summary>11. elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_job.ex <code>✨ Enhancement</code> <code> +4/-0 </code> </summary> >MapperJob relationship to MikroTik controllers ><pre> >• Added has_many relationship to MapperMikrotikController resources ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-67a2c9ea1e7a049af23d3e1c5d8ae13352dfc9c882047abd563b5df376a29500'> elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_job.ex </a> <hr/> </details> <details> <summary>12. elixir/serviceradar_core/lib/serviceradar/network_discovery.ex <code>✨ Enhancement</code> <code> +1/-0 </code> </summary> >NetworkDiscovery domain resource registration ><pre> >• Registered MapperMikrotikController resource in the NetworkDiscovery domain ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-d024970a916af4f3b35bcdef12f6a83aed3585cb96cd7f80adbd9cbba0530833'> elixir/serviceradar_core/lib/serviceradar/network_discovery.ex </a> <hr/> </details> <details> <summary>13. go/pkg/mapper/mikrotik_poller.go <code>✨ Enhancement</code> <code> +693/-0 </code> </summary> >RouterOS REST API poller implementation ><pre> >• New Go module implementing RouterOS REST API discovery via HTTP client >• Queries system identity, resource, routerboard, interfaces, addresses, bridge ports, VLANs, and > neighbors >• Normalizes RouterOS responses into DiscoveredDevice, DiscoveredInterface, and TopologyLink > structures >• Handles unsupported endpoints gracefully and extracts metadata for device enrichment ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-516c61018d5eb469ae324b9196124789055daf87a527604b3f0399963a8b267f'> go/pkg/mapper/mikrotik_poller.go </a> <hr/> </details> <details> <summary>14. go/pkg/mapper/mikrotik_poller_test.go <code>🧪 Tests</code> <code> +248/-0 </code> </summary> >RouterOS poller integration tests ><pre> >• New test module with HTTP mock server validating RouterOS API queries >• Tests successful device, interface, and topology link discovery from RouterOS endpoints >• Tests graceful handling of unsupported routerboard endpoint on CHR instances ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-f140b92214757f7b1678aedb5ca7172a73ba438d8e034ac790da5a2a6f1140e4'> go/pkg/mapper/mikrotik_poller_test.go </a> <hr/> </details> <details> <summary>15. go/pkg/mapper/discovery.go <code>✨ Enhancement</code> <code> +41/-9 </code> </summary> >MikroTik discovery integration into mapper phases ><pre> >• Added sourceAdapterMikroTikV1 constant for MikroTik source adapter versioning >• Integrated queryMikroTikDevices() call into UniFi discovery phase with error handling >• Updated phase logging to reference generic "API discovery" instead of UniFi-specific terminology >• Extended applySourceAdapterVersion() to tag MikroTik topology links with adapter metadata ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-2ba7e064eb7e00e4c9fd0d978cf058b900bfaa9dc1a448f3f9e252400f8f7547'> go/pkg/mapper/discovery.go </a> <hr/> </details> <details> <summary>16. go/pkg/agent/mapper_config_gateway.go <code>✨ Enhancement</code> <code> +61/-30 </code> </summary> >MikroTik API configuration gateway structures ><pre> >• Added MikroTikAPIs field to gatewayMapperConfig struct >• Added mapperMikroTikSpec struct for MikroTik endpoint configuration >• Implemented convertMapperMikroTik() function to transform gateway config to engine config >• Added DiscoveryMode field to mapperJobSpec for job type specification ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-66484d170131144651eb5003f157efb563ef6a6babda2fb948ae5668a8543100'> go/pkg/agent/mapper_config_gateway.go </a> <hr/> </details> <details> <summary>17. go/pkg/agent/mapper_config_gateway_test.go <code>🧪 Tests</code> <code> +143/-0 </code> </summary> >MikroTik gateway configuration parsing tests ><pre> >• New test module validating MikroTik endpoint parsing and round-tripping >• Tests mapper engine config building with MikroTik endpoints and discovery mode >• Tests config hash computation for change detection ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-e89a55b2ec63283fbef7eace41fb490b2556a151bf973c2078a0792ead4ad27c'> go/pkg/agent/mapper_config_gateway_test.go </a> <hr/> </details> <details> <summary>18. go/pkg/mapper/api_selector.go <code>✨ Enhancement</code> <code> +67/-0 </code> </summary> >Generic API endpoint selector utility ><pre> >• New generic API selector module implementing selectNamedBaseURLConfigs() function >• Provides reusable filtering logic for named and URL-based API endpoint selection >• Supports both UniFi and MikroTik API filtering with consistent conventions ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-3b448075c01f8d4d40b53ed3328b9ef9f48e311f885875499b704588b8117187'> go/pkg/mapper/api_selector.go </a> <hr/> </details> <details> <summary>19. go/pkg/mapper/ubnt_poller.go <code>✨ Enhancement</code> <code> +11/-30 </code> </summary> >UniFi poller refactored to use generic API selector ><pre> >• Refactored unifiAPIsForJob() to use new selectNamedBaseURLConfigs() generic selector >• Simplified UniFi API filtering logic by delegating to shared selector function ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-a7c300124d887f5ef86d8f2caad4749b869fcd0172e45231db32e1394dd4b89a'> go/pkg/mapper/ubnt_poller.go </a> <hr/> </details> <details> <summary>20. go/pkg/mapper/types.go <code>✨ Enhancement</code> <code> +10/-0 </code> </summary> > ><pre> > Mapper configuration types for MikroTik API support ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-26e2888812e660d6a166d6f4e12fef119870f2b6870855690b45ec473e5f5a1f'> go/pkg/mapper/types.go </a> <hr/> </details> <details> <summary>21. openspec/changes/add-mikrotik-routeros-discovery/design.md <code>📝 Documentation</code> <code> +156/-0 </code> </summary> >MikroTik RouterOS discovery design and architecture ><pre> >• New design document outlining MikroTik RouterOS REST API discovery architecture >• Documents goals, non-goals, and key decisions (Go implementation, REST API first, read-only) >• Specifies data coverage (device identity, interfaces, L2/L3 context, topology evidence) >• Includes validation plan and demo baseline for CHR target ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-bc578e09e7af67605c683a0a418a73d105fa632bd74ec276a0541796b354256e'> openspec/changes/add-mikrotik-routeros-discovery/design.md </a> <hr/> </details> <details> <summary>22. docs/docs/discovery.md <code>📝 Documentation</code> <code> +71/-0 </code> </summary> >MikroTik RouterOS discovery user documentation ><pre> >• Added section documenting MikroTik RouterOS API discovery setup and requirements >• Documented phase 1 coverage including device identity, interfaces, L2/L3 context, and neighbors >• Provided demo validation commands and success criteria for RouterOS API integration ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-a7f02a003a0e9aaf09d6aa072689ca8ad8b7b298ee014c6ffb878d2a5ecbab28'> docs/docs/discovery.md </a> <hr/> </details> <details> <summary>23. openspec/changes/add-mikrotik-routeros-discovery/specs/network-discovery/spec.md <code>📝 Documentation</code> <code> +49/-0 </code> </summary> >MikroTik RouterOS API discovery requirements and scenarios ><pre> >• Defines requirements for MikroTik RouterOS API discovery source supporting device identity, > interfaces, bridges, VLANs, and IP address collection >• Specifies graceful degradation when RouterOS resources are unavailable due to firmware/version > differences >• Documents RouterOS connection settings storage with encrypted secrets and UI redaction >• Establishes topology evidence ingestion rules ensuring SNMP-attributed evidence remains > authoritative while RouterOS API evidence serves as supplemental structural data ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-6ce1d7737d52bae9e8229e60834c568e6410c5825f5b727aa5680b5431ba552e'> openspec/changes/add-mikrotik-routeros-discovery/specs/network-discovery/spec.md </a> <hr/> </details> <details> <summary>24. openspec/changes/add-mikrotik-routeros-discovery/tasks.md <code>📝 Documentation</code> <code> +32/-0 </code> </summary> >MikroTik RouterOS discovery implementation task checklist ><pre> >• Outlines implementation tasks across four areas: core configuration, Go mapper, > ingestion/inventory, and documentation >• Tracks completion status with checkmarks for tasks 1.1-4.3 and pending validation task 4.4 >• Documents current baseline state showing shallow SNMP-era inventory for live MikroTik CHR demo > environment ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-d366530e982cf28448ebec00bb84a0e97c5935e5a35b330a250dcd0c9fdd252b'> openspec/changes/add-mikrotik-routeros-discovery/tasks.md </a> <hr/> </details> <details> <summary>25. openspec/changes/add-mikrotik-routeros-discovery/proposal.md <code>📝 Documentation</code> <code> +37/-0 </code> </summary> >MikroTik RouterOS API discovery feature proposal and scope ><pre> >• Explains motivation: existing MikroTik enrichment rules lack corresponding API discovery path > compared to UniFi integration >• Details scope including read-only RouterOS discovery source, encrypted credentials, Go poller, and > inventory enrichment >• Lists affected specifications and code modules across Elixir core and Go mapper >• Clarifies non-goals excluding configuration changes, CAPsMAN, BGP, and socket tunneling ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-e281c7ee852383c8fe213245bbbb5453801b3e28fc58e5a91fb16dfcf51427ff'> openspec/changes/add-mikrotik-routeros-discovery/proposal.md </a> <hr/> </details> <details> <summary>26. openspec/changes/add-mikrotik-routeros-discovery/specs/device-inventory/spec.md <code>📝 Documentation</code> <code> +23/-0 </code> </summary> >RouterOS API device inventory enrichment requirements ><pre> >• Specifies RouterOS API enrichment of canonical device inventory for vendor, model, OS, and > hardware metadata >• Defines scenarios for populating vendor_name, model, os, and hw_info fields from RouterOS > discovery data >• Establishes precedence rules preserving existing stronger user-managed values and preventing > duplicate records ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-281ab179938966ffa7045392a8f2f72e5e468ca3449ac2785b134bb2b82f82e8'> openspec/changes/add-mikrotik-routeros-discovery/specs/device-inventory/spec.md </a> <hr/> </details> <details> <summary>27. go/pkg/mapper/BUILD.bazel <code>✨ Enhancement</code> <code> +2/-0 </code> </summary> >Add MikroTik RouterOS poller to Go mapper build ><pre> >• Adds mikrotik_poller.go to the Go library sources for RouterOS API polling implementation >• Adds mikrotik_poller_test.go to the test sources for RouterOS poller unit tests ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2998/files#diff-9e5f10b3e2980b1c0575d6b37a1830a6c5d18f829608ac5b217e05a73c6d3125'> go/pkg/mapper/BUILD.bazel </a> <hr/> </details> </details> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <a href="https://www.qodo.ai"><img src="https://www.qodo.ai/wp-content/uploads/2025/03/qodo-logo.svg" width="80" alt="Qodo Logo"></a>

qodo-code-review[bot] commented

2026-03-06 21:16:07 +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/2998#issuecomment-4014200928
Original created: 2026-03-06T21:16:07Z

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1) 📎 Requirement gaps (1)

1. ~~Tests run CREATE TABLE~~ ☑ 📘 Rule violation ✓ Correctness

Description

New test helpers execute raw DDL (CREATE TABLE IF NOT EXISTS ...) at runtime instead of relying
solely on Elixir migrations under elixir/serviceradar_core/priv/repo/migrations/. This violates
the requirement to do schema changes only via migrations and risks schema drift between
environments.

Code

elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_compiler_test.exs[R196-213]

+  defp ensure_mikrotik_table! do
+    Ecto.Adapters.SQL.query!(
+      ServiceRadar.Repo,
+      """
+      CREATE TABLE IF NOT EXISTS platform.mapper_mikrotik_controllers (
+        id uuid PRIMARY KEY,
+        name text,
+        base_url text NOT NULL,
+        username text NOT NULL,
+        encrypted_password bytea,
+        insecure_skip_verify boolean NOT NULL DEFAULT false,
+        mapper_job_id uuid NOT NULL,
+        inserted_at timestamp(6) without time zone NOT NULL DEFAULT (now() AT TIME ZONE 'utc'),
+        updated_at timestamp(6) without time zone NOT NULL DEFAULT (now() AT TIME ZONE 'utc')
+      )
+      """,
+      []
+    )

Evidence
PR Compliance ID 9 forbids implementing schema/DDL changes outside Elixir migrations. The new test
code issues CREATE TABLE IF NOT EXISTS platform.mapper_mikrotik_controllers (...) via
Ecto.Adapters.SQL.query!/3, which is runtime DDL outside the migration system.
AGENTS.md
elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_compiler_test.exs[196-213]
elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_mikrotik_controller_test.exs[63-80]
elixir/web-ng/test/phoenix/live/settings/networks_live_test.exs[211-228]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Several tests create the `platform.mapper_mikrotik_controllers` table via raw SQL (`CREATE TABLE IF NOT EXISTS`) at runtime. Compliance requires all schema/DDL changes to be performed only through Elixir migrations under `elixir/serviceradar_core/priv/repo/migrations/`.
## Issue Context
A proper migration already exists in this PR. Tests should rely on the migrated schema (by ensuring migrations run in the test environment) rather than executing DDL in test code.
## Fix Focus Areas
- elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_compiler_test.exs[17-213]
- elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_mikrotik_controller_test.exs[7-82]
- elixir/web-ng/test/phoenix/live/settings/networks_live_test.exs[12-229]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Null secret breaks parsing~~ ☑ 🐞 Bug ⛯ Reliability

Description

MapperCompiler emits controller secrets directly; when password/api_key is nil (allowed by Ash
resources and possible via UI omission), Elixir serializes JSON null which Go cannot unmarshal into
string fields, causing the agent to reject the entire mapper config and stop applying updates.

Code

elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/mapper_compiler.ex[R84-91]

+  defp compile_mikrotik_controller(controller) do
+    %{
+      "base_url" => controller.base_url,
+      "username" => controller.username,
+      "password" => controller.password,
+      "name" => controller.name,
+      "insecure_skip_verify" => controller.insecure_skip_verify
+    }

Evidence
Core allows MikroTik controller password to be nil, the web UI drops blank passwords (so
creates/updates can omit the password key), and the compiler forwards the password value as-is. On
the agent, mapper endpoint password is a non-pointer string, so JSON null will cause json.Unmarshal
to error; applyMapperConfig then logs and returns without applying mapper config.
elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_mikrotik_controller.ex[101-112]
elixir/web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex[3448-3452]
elixir/web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex[3547-3554]
elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/mapper_compiler.ex[84-91]
go/pkg/agent/mapper_config_gateway.go[67-73]
go/pkg/agent/push_loop.go[2538-2543]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`MapperCompiler` forwards controller secrets as-is. Because `MapperMikrotikController.password` (and `MapperUnifiController.api_key`) allow `nil`, and the web UI may omit these keys (via `drop_blank`), the compiled JSON can contain `null` for password/api_key. Go unmarshalling into `string` fails on `null`, causing the agent to reject the full mapper config.
## Issue Context
- Go structs use `string` for `password`/`api_key` fields, so `null` will cause `json.Unmarshal` errors.
- The agent aborts mapper config apply on parse errors.
## Fix Focus Areas
- elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/mapper_compiler.ex[84-109]
- elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_mikrotik_controller.ex[101-112]
- elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_unifi_controller.ex[93-105]
- elixir/web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex[3442-3452]
- go/pkg/agent/mapper_config_gateway.go[60-73]
## Suggested approach
1. In `compile_mikrotik_controller/1` and `compile_unifi_controller/1`, ensure secrets are never `nil` (e.g., `controller.password || &amp;quot;&amp;quot;`, `controller.api_key || &amp;quot;&amp;quot;`) or omit the key entirely when nil.
2. Optionally harden the Go side by changing `Password`/`APIKey` to `*string` with `omitempty` and treating `nil` as empty.
3. Add validations in Ash resources (or action changes) to require secrets when `base_url` is present (or explicitly document/handle empty-password scenarios).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. No reproducible CHR run notes 📎 Requirement gap ⛯ Reliability

Description

The PR documents MikroTik RouterOS REST API usage but does not identify a reproducible runnable
MikroTik target (appliance/container/etc.) with name/version/source and basic run/deploy notes. This
makes it hard to reproduce or validate the integration path as required.

Code

docs/docs/discovery.md[R53-59]

+### Demo Validation
+
+Use the live CHR target in `demo` as the validation baseline:
+
+- Device UID: `sr:36e0e348-6da6-4474-bb3c-f7af1eb4d5b8`
+- Expected management IP: `192.168.6.167`
+

Evidence

PR Compliance ID 1 requires documenting a viable API integration path plus a specific runnable
option with enough detail to reproduce. The added documentation references a live demo CHR device
UID/IP for validation but does not provide runnable artifact details (name/version/source) or how to
run/deploy it.

Investigate Mikrotik integration feasibility (API and runnable target)
docs/docs/discovery.md[53-59]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Compliance requires documenting a concrete runnable MikroTik target (virtual appliance/container/alternative) with enough detail to reproduce, but current documentation only references an already-running demo CHR instance.
## Issue Context
Add the runnable artifact details (name, RouterOS/CHR version, source URL, and minimal run/deploy steps) so others can stand up a test target independently.
## Fix Focus Areas
- docs/docs/discovery.md[28-97]
- openspec/changes/add-mikrotik-routeros-discovery/design.md[129-150]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. All-zero MAC device IDs 🐞 Bug ✓ Correctness

Description

MikroTik device identity is derived from the first interface MAC without validation; if the first
MAC is all-zero (e.g., loopback) the generated DeviceID can collide across devices and cause
incorrect inventory reconciliation.

Code

go/pkg/mapper/mikrotik_poller.go[R492-498]

+func firstMikroTikMAC(rawInterfaces []map[string]any) string {
+	for _, raw := range rawInterfaces {
+		mac := NormalizeMAC(stringValue(raw, "mac-address"))
+		if mac != "" {
+			return mac
+		}
+	}

Evidence

firstMikroTikMAC returns the first normalized MAC that is non-empty, and NormalizeMAC does not
reject placeholder values. The repository’s own test fixture includes a loopback interface with
mac-address 00:00:00:00:00:00, illustrating a plausible response shape that would be treated as a
valid MAC if it appears first.

go/pkg/mapper/mikrotik_poller.go[492-500]
go/pkg/mapper/utils.go[347-368]
go/pkg/mapper/mikrotik_poller_test.go[175-193]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Device identity uses the first interface MAC returned by RouterOS. Placeholder MACs like `00:00:00:00:00:00` are treated as valid, which can generate non-unique device IDs and break reconciliation.
## Issue Context
- RouterOS can expose loopback/tunnel interfaces with placeholder MACs.
- Identity collisions are hard to debug and can create cross-device merges.
## Fix Focus Areas
- go/pkg/mapper/mikrotik_poller.go[249-269]
- go/pkg/mapper/mikrotik_poller.go[492-500]
- go/pkg/mapper/utils.go[347-368]
## Suggested approach
1. Update `firstMikroTikMAC` to skip known invalid MACs (all-zero, empty, maybe multicast) and prefer physical interface types when available.
2. If no valid MAC is found, fall back to `GenerateDeviceIDFromIP(deviceIP)` (or another stable identifier) instead of producing `mac-000000000000`.
3. Add a unit test where the first interface MAC is all-zero and ensure the fallback behavior is used.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ The new review experience is currently in Beta. Learn more

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2998#issuecomment-4014200928 Original created: 2026-03-06T21:16:07Z --- <h3>Code Review by Qodo</h3> <code>🐞 Bugs (2)</code> <code>📘 Rule violations (1)</code> <code>📎 Requirement gaps (1)</code> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <img src="https://www.qodo.ai/wp-content/uploads/2026/01/action-required.png" height="20" alt="Action required"> <details> <summary> 1. <s>Tests run CREATE TABLE</s> ☑ <code>📘 Rule violation</code> <code>✓ Correctness</code></summary> > <details open> ><summary>Description</summary> > > ><pre> >New test helpers execute raw DDL (CREATE TABLE IF NOT EXISTS ...) at runtime instead of relying >solely on Elixir migrations under elixir/serviceradar_core/priv/repo/migrations/. This violates >the requirement to do schema changes only via migrations and risks schema drift between >environments. ></pre> ></details> > <details open> ><summary>Code</summary> > > ><code>[elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_compiler_test.exs[R196-213]](https://github.com/carverauto/serviceradar/pull/2998/files#diff-a66580ac3bd92e18fbff9a00f884c2f826061fbb9ae6535a704eb9b29ab5c5bdR196-R213)</code> > >```diff >+ defp ensure_mikrotik_table! do >+ Ecto.Adapters.SQL.query!( >+ ServiceRadar.Repo, >+ """ >+ CREATE TABLE IF NOT EXISTS platform.mapper_mikrotik_controllers ( >+ id uuid PRIMARY KEY, >+ name text, >+ base_url text NOT NULL, >+ username text NOT NULL, >+ encrypted_password bytea, >+ insecure_skip_verify boolean NOT NULL DEFAULT false, >+ mapper_job_id uuid NOT NULL, >+ inserted_at timestamp(6) without time zone NOT NULL DEFAULT (now() AT TIME ZONE 'utc'), >+ updated_at timestamp(6) without time zone NOT NULL DEFAULT (now() AT TIME ZONE 'utc') >+ ) >+ """, >+ [] >+ ) >``` ></details> > <details > ><summary>Evidence</summary> > > ><pre> >PR Compliance ID 9 forbids implementing schema/DDL changes outside Elixir migrations. The new test >code issues CREATE TABLE IF NOT EXISTS platform.mapper_mikrotik_controllers (...) via >Ecto.Adapters.SQL.query!/3, which is runtime DDL outside the migration system. ></pre> > > <code>AGENTS.md</code> > <code>[elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_compiler_test.exs[196-213]](https://github.com/carverauto/serviceradar/blob/cca6dba5fc2bccf91f243d4048c70369cade89ab/elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_compiler_test.exs/#L196-L213)</code> > <code>[elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_mikrotik_controller_test.exs[63-80]](https://github.com/carverauto/serviceradar/blob/cca6dba5fc2bccf91f243d4048c70369cade89ab/elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_mikrotik_controller_test.exs/#L63-L80)</code> > <code>[elixir/web-ng/test/phoenix/live/settings/networks_live_test.exs[211-228]](https://github.com/carverauto/serviceradar/blob/cca6dba5fc2bccf91f243d4048c70369cade89ab/elixir/web-ng/test/phoenix/live/settings/networks_live_test.exs/#L211-L228)</code> ></details> > <details> ><summary>Agent prompt</summary> > > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >Several tests create the `platform.mapper_mikrotik_controllers` table via raw SQL (`CREATE TABLE IF NOT EXISTS`) at runtime. Compliance requires all schema/DDL changes to be performed only through Elixir migrations under `elixir/serviceradar_core/priv/repo/migrations/`. >## Issue Context >A proper migration already exists in this PR. Tests should rely on the migrated schema (by ensuring migrations run in the test environment) rather than executing DDL in test code. >## Fix Focus Areas >- elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_compiler_test.exs[17-213] >- elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_mikrotik_controller_test.exs[7-82] >- elixir/web-ng/test/phoenix/live/settings/networks_live_test.exs[12-229] >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <details> <summary> 2. <s>Null secret breaks parsing</s> ☑ <code>🐞 Bug</code> <code>⛯ Reliability</code></summary> > <details open> ><summary>Description</summary> > > ><pre> >MapperCompiler emits controller secrets directly; when password/api_key is nil (allowed by Ash >resources and possible via UI omission), Elixir serializes JSON null which Go cannot unmarshal into >string fields, causing the agent to reject the entire mapper config and stop applying updates. ></pre> ></details> > <details open> ><summary>Code</summary> > > ><code>[elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/mapper_compiler.ex[R84-91]](https://github.com/carverauto/serviceradar/pull/2998/files#diff-2b9795cd8468dee006a68b823afa9a155e96f24d0895077e9772ac6399a6ac3eR84-R91)</code> > >```diff >+ defp compile_mikrotik_controller(controller) do >+ %{ >+ "base_url" => controller.base_url, >+ "username" => controller.username, >+ "password" => controller.password, >+ "name" => controller.name, >+ "insecure_skip_verify" => controller.insecure_skip_verify >+ } >``` ></details> > <details > ><summary>Evidence</summary> > > ><pre> >Core allows MikroTik controller password to be nil, the web UI drops blank passwords (so >creates/updates can omit the password key), and the compiler forwards the password value as-is. On >the agent, mapper endpoint password is a non-pointer string, so JSON null will cause json.Unmarshal >to error; applyMapperConfig then logs and returns without applying mapper config. ></pre> > > <code>[elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_mikrotik_controller.ex[101-112]](https://github.com/carverauto/serviceradar/blob/cca6dba5fc2bccf91f243d4048c70369cade89ab/elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_mikrotik_controller.ex/#L101-L112)</code> > <code>[elixir/web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex[3448-3452]](https://github.com/carverauto/serviceradar/blob/cca6dba5fc2bccf91f243d4048c70369cade89ab/elixir/web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex/#L3448-L3452)</code> > <code>[elixir/web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex[3547-3554]](https://github.com/carverauto/serviceradar/blob/cca6dba5fc2bccf91f243d4048c70369cade89ab/elixir/web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex/#L3547-L3554)</code> > <code>[elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/mapper_compiler.ex[84-91]](https://github.com/carverauto/serviceradar/blob/cca6dba5fc2bccf91f243d4048c70369cade89ab/elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/mapper_compiler.ex/#L84-L91)</code> > <code>[go/pkg/agent/mapper_config_gateway.go[67-73]](https://github.com/carverauto/serviceradar/blob/cca6dba5fc2bccf91f243d4048c70369cade89ab/go/pkg/agent/mapper_config_gateway.go/#L67-L73)</code> > <code>[go/pkg/agent/push_loop.go[2538-2543]](https://github.com/carverauto/serviceradar/blob/cca6dba5fc2bccf91f243d4048c70369cade89ab/go/pkg/agent/push_loop.go/#L2538-L2543)</code> ></details> > <details> ><summary>Agent prompt</summary> > > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >`MapperCompiler` forwards controller secrets as-is. Because `MapperMikrotikController.password` (and `MapperUnifiController.api_key`) allow `nil`, and the web UI may omit these keys (via `drop_blank`), the compiled JSON can contain `null` for password/api_key. Go unmarshalling into `string` fails on `null`, causing the agent to reject the full mapper config. >## Issue Context >- Go structs use `string` for `password`/`api_key` fields, so `null` will cause `json.Unmarshal` errors. >- The agent aborts mapper config apply on parse errors. >## Fix Focus Areas >- elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/mapper_compiler.ex[84-109] >- elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_mikrotik_controller.ex[101-112] >- elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_unifi_controller.ex[93-105] >- elixir/web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex[3442-3452] >- go/pkg/agent/mapper_config_gateway.go[60-73] >## Suggested approach >1. In `compile_mikrotik_controller/1` and `compile_unifi_controller/1`, ensure secrets are never `nil` (e.g., `controller.password || &quot;&quot;`, `controller.api_key || &quot;&quot;`) or omit the key entirely when nil. >2. Optionally harden the Go side by changing `Password`/`APIKey` to `*string` with `omitempty` and treating `nil` as empty. >3. Add validations in Ash resources (or action changes) to require secrets when `base_url` is present (or explicitly document/handle empty-password scenarios). >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <img src="https://www.qodo.ai/wp-content/uploads/2026/01/review-recommended.png" height="20" alt="Remediation recommended"> <details> <summary> 3. No reproducible CHR run notes <code>📎 Requirement gap</code> <code>⛯ Reliability</code></summary> > <details open> ><summary>Description</summary> > > ><pre> >The PR documents MikroTik RouterOS REST API usage but does not identify a reproducible runnable >MikroTik target (appliance/container/etc.) with name/version/source and basic run/deploy notes. This >makes it hard to reproduce or validate the integration path as required. ></pre> ></details> > <details open> ><summary>Code</summary> > > ><code>[docs/docs/discovery.md[R53-59]](https://github.com/carverauto/serviceradar/pull/2998/files#diff-a7f02a003a0e9aaf09d6aa072689ca8ad8b7b298ee014c6ffb878d2a5ecbab28R53-R59)</code> > >```diff >+### Demo Validation >+ >+Use the live CHR target in `demo` as the validation baseline: >+ >+- Device UID: `sr:36e0e348-6da6-4474-bb3c-f7af1eb4d5b8` >+- Expected management IP: `192.168.6.167` >+ >``` ></details> > <details > ><summary>Evidence</summary> > > ><pre> >PR Compliance ID 1 requires documenting a viable API integration path plus a specific runnable >option with enough detail to reproduce. The added documentation references a live demo CHR device >UID/IP for validation but does not provide runnable artifact details (name/version/source) or how to >run/deploy it. ></pre> > > <code>Investigate Mikrotik integration feasibility (API and runnable target)</code> > <code>[docs/docs/discovery.md[53-59]](https://github.com/carverauto/serviceradar/blob/cca6dba5fc2bccf91f243d4048c70369cade89ab/docs/docs/discovery.md/#L53-L59)</code> ></details> > <details> ><summary>Agent prompt</summary> > > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >Compliance requires documenting a concrete runnable MikroTik target (virtual appliance/container/alternative) with enough detail to reproduce, but current documentation only references an already-running demo CHR instance. >## Issue Context >Add the runnable artifact details (name, RouterOS/CHR version, source URL, and minimal run/deploy steps) so others can stand up a test target independently. >## Fix Focus Areas >- docs/docs/discovery.md[28-97] >- openspec/changes/add-mikrotik-routeros-discovery/design.md[129-150] >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <details> <summary> 4. All-zero MAC device IDs <code>🐞 Bug</code> <code>✓ Correctness</code></summary> > <details open> ><summary>Description</summary> > > ><pre> >MikroTik device identity is derived from the first interface MAC without validation; if the first >MAC is all-zero (e.g., loopback) the generated DeviceID can collide across devices and cause >incorrect inventory reconciliation. ></pre> ></details> > <details open> ><summary>Code</summary> > > ><code>[go/pkg/mapper/mikrotik_poller.go[R492-498]](https://github.com/carverauto/serviceradar/pull/2998/files#diff-516c61018d5eb469ae324b9196124789055daf87a527604b3f0399963a8b267fR492-R498)</code> > >```diff >+func firstMikroTikMAC(rawInterfaces []map[string]any) string { >+ for _, raw := range rawInterfaces { >+ mac := NormalizeMAC(stringValue(raw, "mac-address")) >+ if mac != "" { >+ return mac >+ } >+ } >``` ></details> > <details > ><summary>Evidence</summary> > > ><pre> >firstMikroTikMAC returns the first normalized MAC that is non-empty, and NormalizeMAC does not >reject placeholder values. The repository’s own test fixture includes a loopback interface with >mac-address 00:00:00:00:00:00, illustrating a plausible response shape that would be treated as a >valid MAC if it appears first. ></pre> > > <code>[go/pkg/mapper/mikrotik_poller.go[492-500]](https://github.com/carverauto/serviceradar/blob/cca6dba5fc2bccf91f243d4048c70369cade89ab/go/pkg/mapper/mikrotik_poller.go/#L492-L500)</code> > <code>[go/pkg/mapper/utils.go[347-368]](https://github.com/carverauto/serviceradar/blob/cca6dba5fc2bccf91f243d4048c70369cade89ab/go/pkg/mapper/utils.go/#L347-L368)</code> > <code>[go/pkg/mapper/mikrotik_poller_test.go[175-193]](https://github.com/carverauto/serviceradar/blob/cca6dba5fc2bccf91f243d4048c70369cade89ab/go/pkg/mapper/mikrotik_poller_test.go/#L175-L193)</code> ></details> > <details> ><summary>Agent prompt</summary> > > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >Device identity uses the first interface MAC returned by RouterOS. Placeholder MACs like `00:00:00:00:00:00` are treated as valid, which can generate non-unique device IDs and break reconciliation. >## Issue Context >- RouterOS can expose loopback/tunnel interfaces with placeholder MACs. >- Identity collisions are hard to debug and can create cross-device merges. >## Fix Focus Areas >- go/pkg/mapper/mikrotik_poller.go[249-269] >- go/pkg/mapper/mikrotik_poller.go[492-500] >- go/pkg/mapper/utils.go[347-368] >## Suggested approach >1. Update `firstMikroTikMAC` to skip known invalid MACs (all-zero, empty, maybe multicast) and prefer physical interface types when available. >2. If no valid MAC is found, fall back to `GenerateDeviceIDFromIP(deviceIP)` (or another stable identifier) instead of producing `mac-000000000000`. >3. Add a unit test where the first interface MAC is all-zero and ensure the fallback behavior is used. >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <pre>ⓘ The new review experience is currently in Beta. <a href="https://docs.qodo.ai/qodo-documentation/code-review">Learn more</a></pre> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <a href="https://www.qodo.ai"><img src="https://www.qodo.ai/wp-content/uploads/2025/03/qodo-logo.svg" width="80" alt="Qodo Logo"></a>

github-advanced-security[bot] commented

2026-03-06 21:18:38 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @github-advanced-security[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2998#discussion_r2897918722
Original created: 2026-03-06T21:18:38Z
Original path: go/pkg/mapper/mikrotik_poller.go
Original line: 350

Incorrect conversion between integer types

Incorrect conversion of an integer with architecture-dependent bit size from strconv.Atoi to a lower bit size type int32 without an upper bound check.

Show more details

Imported GitHub PR review comment. Original author: @github-advanced-security[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2998#discussion_r2897918722 Original created: 2026-03-06T21:18:38Z Original path: go/pkg/mapper/mikrotik_poller.go Original line: 350 --- ## Incorrect conversion between integer types Incorrect conversion of an integer with architecture-dependent bit size from [strconv.Atoi](1) to a lower bit size type int32 without an upper bound check. [Show more details](https://github.com/carverauto/serviceradar/security/code-scanning/100)

qodo-code-review[bot] commented

2026-03-06 21:23:04 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2998#discussion_r2897934654
Original created: 2026-03-06T21:23:04Z
Original path: elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_compiler_test.exs
Original line: 213

1. Tests run create table 📘 Rule violation ✓ Correctness

New test helpers execute raw DDL (CREATE TABLE IF NOT EXISTS ...) at runtime instead of relying
solely on Elixir migrations under elixir/serviceradar_core/priv/repo/migrations/. This violates
the requirement to do schema changes only via migrations and risks schema drift between
environments.

Agent Prompt

## Issue description
Several tests create the `platform.mapper_mikrotik_controllers` table via raw SQL (`CREATE TABLE IF NOT EXISTS`) at runtime. Compliance requires all schema/DDL changes to be performed only through Elixir migrations under `elixir/serviceradar_core/priv/repo/migrations/`.

## Issue Context
A proper migration already exists in this PR. Tests should rely on the migrated schema (by ensuring migrations run in the test environment) rather than executing DDL in test code.

## Fix Focus Areas
- elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_compiler_test.exs[17-213]
- elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_mikrotik_controller_test.exs[7-82]
- elixir/web-ng/test/phoenix/live/settings/networks_live_test.exs[12-229]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Imported GitHub PR review comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2998#discussion_r2897934654 Original created: 2026-03-06T21:23:04Z Original path: elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_compiler_test.exs Original line: 213 --- <img src="https://www.qodo.ai/wp-content/uploads/2025/12/v2-action-required.svg" height="20" alt="Action required"> 1\. Tests run create table <code>📘 Rule violation</code> <code>✓ Correctness</code> <pre> New test helpers execute raw DDL (CREATE TABLE IF NOT EXISTS ...) at runtime instead of relying solely on Elixir migrations under elixir/serviceradar_core/priv/repo/migrations/. This violates the requirement to do schema changes only via migrations and risks schema drift between environments. </pre> <details> <summary>Agent Prompt</summary> ``` ## Issue description Several tests create the `platform.mapper_mikrotik_controllers` table via raw SQL (`CREATE TABLE IF NOT EXISTS`) at runtime. Compliance requires all schema/DDL changes to be performed only through Elixir migrations under `elixir/serviceradar_core/priv/repo/migrations/`. ## Issue Context A proper migration already exists in this PR. Tests should rely on the migrated schema (by ensuring migrations run in the test environment) rather than executing DDL in test code. ## Fix Focus Areas - elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_compiler_test.exs[17-213] - elixir/serviceradar_core/test/serviceradar/network_discovery/mapper_mikrotik_controller_test.exs[7-82] - elixir/web-ng/test/phoenix/live/settings/networks_live_test.exs[12-229] ``` <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> </details>

qodo-code-review[bot] commented

2026-03-06 21:23:04 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2998#discussion_r2897934655
Original created: 2026-03-06T21:23:04Z
Original path: elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/mapper_compiler.ex
Original line: 91

2. Null secret breaks parsing 🐞 Bug ⛯ Reliability

MapperCompiler emits controller secrets directly; when password/api_key is nil (allowed by Ash
resources and possible via UI omission), Elixir serializes JSON null which Go cannot unmarshal into
string fields, causing the agent to reject the entire mapper config and stop applying updates.

Agent Prompt

## Issue description
`MapperCompiler` forwards controller secrets as-is. Because `MapperMikrotikController.password` (and `MapperUnifiController.api_key`) allow `nil`, and the web UI may omit these keys (via `drop_blank`), the compiled JSON can contain `null` for password/api_key. Go unmarshalling into `string` fails on `null`, causing the agent to reject the full mapper config.

## Issue Context
- Go structs use `string` for `password`/`api_key` fields, so `null` will cause `json.Unmarshal` errors.
- The agent aborts mapper config apply on parse errors.

## Fix Focus Areas
- elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/mapper_compiler.ex[84-109]
- elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_mikrotik_controller.ex[101-112]
- elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_unifi_controller.ex[93-105]
- elixir/web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex[3442-3452]
- go/pkg/agent/mapper_config_gateway.go[60-73]

## Suggested approach
1. In `compile_mikrotik_controller/1` and `compile_unifi_controller/1`, ensure secrets are never `nil` (e.g., `controller.password || ""`, `controller.api_key || ""`) or omit the key entirely when nil.
2. Optionally harden the Go side by changing `Password`/`APIKey` to `*string` with `omitempty` and treating `nil` as empty.
3. Add validations in Ash resources (or action changes) to require secrets when `base_url` is present (or explicitly document/handle empty-password scenarios).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Imported GitHub PR review comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2998#discussion_r2897934655 Original created: 2026-03-06T21:23:04Z Original path: elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/mapper_compiler.ex Original line: 91 --- <img src="https://www.qodo.ai/wp-content/uploads/2025/12/v2-action-required.svg" height="20" alt="Action required"> 2\. Null secret breaks parsing <code>🐞 Bug</code> <code>⛯ Reliability</code> <pre> MapperCompiler emits controller secrets directly; when password/api_key is nil (allowed by Ash resources and possible via UI omission), Elixir serializes JSON null which Go cannot unmarshal into string fields, causing the agent to reject the entire mapper config and stop applying updates. </pre> <details> <summary>Agent Prompt</summary> ``` ## Issue description `MapperCompiler` forwards controller secrets as-is. Because `MapperMikrotikController.password` (and `MapperUnifiController.api_key`) allow `nil`, and the web UI may omit these keys (via `drop_blank`), the compiled JSON can contain `null` for password/api_key. Go unmarshalling into `string` fails on `null`, causing the agent to reject the full mapper config. ## Issue Context - Go structs use `string` for `password`/`api_key` fields, so `null` will cause `json.Unmarshal` errors. - The agent aborts mapper config apply on parse errors. ## Fix Focus Areas - elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/mapper_compiler.ex[84-109] - elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_mikrotik_controller.ex[101-112] - elixir/serviceradar_core/lib/serviceradar/network_discovery/mapper_unifi_controller.ex[93-105] - elixir/web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex[3442-3452] - go/pkg/agent/mapper_config_gateway.go[60-73] ## Suggested approach 1. In `compile_mikrotik_controller/1` and `compile_unifi_controller/1`, ensure secrets are never `nil` (e.g., `controller.password || ""`, `controller.api_key || ""`) or omit the key entirely when nil. 2. Optionally harden the Go side by changing `Password`/`APIKey` to `*string` with `omitempty` and treating `nil` as empty. 3. Add validations in Ash resources (or action changes) to require secrets when `base_url` is present (or explicitly document/handle empty-password scenarios). ``` <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> </details>

qodo-code-review[bot] commented

2026-03-07 00:26:37 +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/2998#issuecomment-4014959471
Original created: 2026-03-07T00:26:37Z

CI Feedback 🧐

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

Action: build

Failed stage: Configure SRQL fixture database for tests [❌]

Failed test name: ""

Failure summary:

The action failed because a required secret for the test/fixture setup was not provided:
- The
workflow explicitly exited with SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL
fixture TLS. (exit code 1), and the environment shows SRQL_TEST_DATABASE_CA_CERT: was empty.
- A
later fatal: No url found for submodule path ... in .gitmodules occurred during post-job cleanup and
is logged as a warning; it did not cause the main job failure.

Relevant error logs:

1:  Runner name: 'arc-runner-set-hk6mk-runner-k9dkr'
2:  Runner group name: 'Default'
...

139:  ^[[36;1mif command -v apt-get >/dev/null 2>&1; then^[[0m
140:  ^[[36;1m  sudo apt-get update^[[0m
141:  ^[[36;1m  sudo apt-get install -y build-essential pkg-config libssl-dev protobuf-compiler cmake flex bison^[[0m
142:  ^[[36;1melif command -v dnf >/dev/null 2>&1; then^[[0m
143:  ^[[36;1m  sudo dnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m
144:  ^[[36;1melif command -v yum >/dev/null 2>&1; then^[[0m
145:  ^[[36;1m  sudo yum install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m
146:  ^[[36;1melif command -v microdnf >/dev/null 2>&1; then^[[0m
147:  ^[[36;1m  sudo microdnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m
148:  ^[[36;1melse^[[0m
149:  ^[[36;1m  echo "Unsupported package manager; please install gcc, g++ (or clang), make, OpenSSL headers, pkg-config, and protoc manually." >&2^[[0m
150:  ^[[36;1m  exit 1^[[0m
151:  ^[[36;1mfi^[[0m
152:  ^[[36;1m^[[0m
153:  ^[[36;1mensure_pkg_config^[[0m
154:  ^[[36;1mprotoc --version || (echo "protoc installation failed" && exit 1)^[[0m
155:  shell: /usr/bin/bash -e {0}
...

387:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
388:  env:
389:  BUILDBUDDY_ORG_API_KEY: ***
390:  SRQL_TEST_DATABASE_URL: ***
391:  SRQL_TEST_ADMIN_URL: ***
392:  SRQL_TEST_DATABASE_CA_CERT: 
393:  DOCKERHUB_USERNAME: ***
394:  DOCKERHUB_TOKEN: ***
395:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
396:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
397:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
398:  ##[endgroup]
399:  ##[group]Run : install rustup if needed
400:  ^[[36;1m: install rustup if needed^[[0m
401:  ^[[36;1mif ! command -v rustup &>/dev/null; then^[[0m
402:  ^[[36;1m  curl --proto '=https' --tlsv1.2 --retry 10 --retry-connrefused --location --silent --show-error --fail https://sh.rustup.rs | sh -s -- --default-toolchain none -y^[[0m
403:  ^[[36;1m  echo "$CARGO_HOME/bin" >> $GITHUB_PATH^[[0m
...

543:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
544:  env:
545:  BUILDBUDDY_ORG_API_KEY: ***
546:  SRQL_TEST_DATABASE_URL: ***
547:  SRQL_TEST_ADMIN_URL: ***
548:  SRQL_TEST_DATABASE_CA_CERT: 
549:  DOCKERHUB_USERNAME: ***
550:  DOCKERHUB_TOKEN: ***
551:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
552:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
553:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
554:  CARGO_HOME: /home/runner/.cargo
555:  CARGO_INCREMENTAL: 0
556:  CARGO_TERM_COLOR: always
557:  ##[endgroup]
558:  ##[group]Run : work around spurious network errors in curl 8.0
559:  ^[[36;1m: work around spurious network errors in curl 8.0^[[0m
560:  ^[[36;1m# https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/timeout.20investigation^[[0m
...

611:  SRQL_TEST_DATABASE_CA_CERT: 
612:  DOCKERHUB_USERNAME: ***
613:  DOCKERHUB_TOKEN: ***
614:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
615:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
616:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
617:  CARGO_HOME: /home/runner/.cargo
618:  CARGO_INCREMENTAL: 0
619:  CARGO_TERM_COLOR: always
620:  ##[endgroup]
621:  Attempting to download 1.x...
622:  Acquiring v1.28.1 from https://github.com/bazelbuild/bazelisk/releases/download/v1.28.1/bazelisk-linux-amd64
623:  Adding to the cache ...
624:  Successfully cached bazelisk to /home/runner/_work/_tool/bazelisk/1.28.1/x64
625:  Added bazelisk to the path
626:  ##[warning]Failed to restore: Cache service responded with 400
627:  Restored bazelisk cache dir @ /home/runner/.cache/bazelisk
...

693:  env:
694:  BUILDBUDDY_ORG_API_KEY: ***
695:  SRQL_TEST_DATABASE_URL: ***
696:  SRQL_TEST_ADMIN_URL: ***
697:  SRQL_TEST_DATABASE_CA_CERT: 
698:  DOCKERHUB_USERNAME: ***
699:  DOCKERHUB_TOKEN: ***
700:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
701:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
702:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
703:  CARGO_HOME: /home/runner/.cargo
704:  CARGO_INCREMENTAL: 0
705:  CARGO_TERM_COLOR: always
706:  ##[endgroup]
707:  SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS.
708:  ##[error]Process completed with exit code 1.
709:  Post job cleanup.
710:  [command]/usr/bin/git version
711:  git version 2.52.0
712:  Temporarily overriding HOME='/home/runner/_work/_temp/361bb1cd-450e-4625-b6f0-634487dd9354' before making global git config changes
713:  Adding repository directory to the temporary git global config as a safe directory
714:  [command]/usr/bin/git config --global --add safe.directory /home/runner/_work/serviceradar/serviceradar
715:  Removing SSH command configuration
716:  [command]/usr/bin/git config --local --name-only --get-regexp core\.sshCommand
717:  [command]/usr/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
718:  fatal: No url found for submodule path 'swift/FieldSurvey/LocalPackages/arrow-swift' in .gitmodules
719:  ##[warning]The process '/usr/bin/git' failed with exit code 128
720:  Cleaning up orphan processes

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2998#issuecomment-4014959471 Original created: 2026-03-07T00:26:37Z --- ## CI Feedback 🧐 A test triggered by this PR failed. Here is an AI-generated analysis of the failure: <table><tr><td> **Action:** build</td></tr> <tr><td> **Failed stage:** [Configure SRQL fixture database for tests](https://github.com/carverauto/serviceradar/actions/runs/22787611021/job/66107686551) [❌] </td></tr> <tr><td> **Failed test name:** "" </td></tr> <tr><td> **Failure summary:** The action failed because a required secret for the test/fixture setup was not provided: - The workflow explicitly exited with <code>SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL </code> <code>fixture TLS.</code> (exit code 1), and the environment shows <code>SRQL_TEST_DATABASE_CA_CERT:</code> was empty. - A later <code>fatal: No url found for submodule path ... in .gitmodules</code> occurred during post-job cleanup and is logged as a warning; it did not cause the main job failure. </td></tr> <tr><td> <details><summary>Relevant error logs:</summary> ```yaml 1: Runner name: 'arc-runner-set-hk6mk-runner-k9dkr' 2: Runner group name: 'Default' ... 139: ^[[36;1mif command -v apt-get >/dev/null 2>&1; then^[[0m 140: ^[[36;1m sudo apt-get update^[[0m 141: ^[[36;1m sudo apt-get install -y build-essential pkg-config libssl-dev protobuf-compiler cmake flex bison^[[0m 142: ^[[36;1melif command -v dnf >/dev/null 2>&1; then^[[0m 143: ^[[36;1m sudo dnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 144: ^[[36;1melif command -v yum >/dev/null 2>&1; then^[[0m 145: ^[[36;1m sudo yum install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 146: ^[[36;1melif command -v microdnf >/dev/null 2>&1; then^[[0m 147: ^[[36;1m sudo microdnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 148: ^[[36;1melse^[[0m 149: ^[[36;1m echo "Unsupported package manager; please install gcc, g++ (or clang), make, OpenSSL headers, pkg-config, and protoc manually." >&2^[[0m 150: ^[[36;1m exit 1^[[0m 151: ^[[36;1mfi^[[0m 152: ^[[36;1m^[[0m 153: ^[[36;1mensure_pkg_config^[[0m 154: ^[[36;1mprotoc --version || (echo "protoc installation failed" && exit 1)^[[0m 155: shell: /usr/bin/bash -e {0} ... 387: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 388: env: 389: BUILDBUDDY_ORG_API_KEY: *** 390: SRQL_TEST_DATABASE_URL: *** 391: SRQL_TEST_ADMIN_URL: *** 392: SRQL_TEST_DATABASE_CA_CERT: 393: DOCKERHUB_USERNAME: *** 394: DOCKERHUB_TOKEN: *** 395: TEST_CNPG_DATABASE: serviceradar_web_ng_test 396: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 397: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 398: ##[endgroup] 399: ##[group]Run : install rustup if needed 400: ^[[36;1m: install rustup if needed^[[0m 401: ^[[36;1mif ! command -v rustup &>/dev/null; then^[[0m 402: ^[[36;1m curl --proto '=https' --tlsv1.2 --retry 10 --retry-connrefused --location --silent --show-error --fail https://sh.rustup.rs | sh -s -- --default-toolchain none -y^[[0m 403: ^[[36;1m echo "$CARGO_HOME/bin" >> $GITHUB_PATH^[[0m ... 543: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 544: env: 545: BUILDBUDDY_ORG_API_KEY: *** 546: SRQL_TEST_DATABASE_URL: *** 547: SRQL_TEST_ADMIN_URL: *** 548: SRQL_TEST_DATABASE_CA_CERT: 549: DOCKERHUB_USERNAME: *** 550: DOCKERHUB_TOKEN: *** 551: TEST_CNPG_DATABASE: serviceradar_web_ng_test 552: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 553: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 554: CARGO_HOME: /home/runner/.cargo 555: CARGO_INCREMENTAL: 0 556: CARGO_TERM_COLOR: always 557: ##[endgroup] 558: ##[group]Run : work around spurious network errors in curl 8.0 559: ^[[36;1m: work around spurious network errors in curl 8.0^[[0m 560: ^[[36;1m# https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/timeout.20investigation^[[0m ... 611: SRQL_TEST_DATABASE_CA_CERT: 612: DOCKERHUB_USERNAME: *** 613: DOCKERHUB_TOKEN: *** 614: TEST_CNPG_DATABASE: serviceradar_web_ng_test 615: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 616: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 617: CARGO_HOME: /home/runner/.cargo 618: CARGO_INCREMENTAL: 0 619: CARGO_TERM_COLOR: always 620: ##[endgroup] 621: Attempting to download 1.x... 622: Acquiring v1.28.1 from https://github.com/bazelbuild/bazelisk/releases/download/v1.28.1/bazelisk-linux-amd64 623: Adding to the cache ... 624: Successfully cached bazelisk to /home/runner/_work/_tool/bazelisk/1.28.1/x64 625: Added bazelisk to the path 626: ##[warning]Failed to restore: Cache service responded with 400 627: Restored bazelisk cache dir @ /home/runner/.cache/bazelisk ... 693: env: 694: BUILDBUDDY_ORG_API_KEY: *** 695: SRQL_TEST_DATABASE_URL: *** 696: SRQL_TEST_ADMIN_URL: *** 697: SRQL_TEST_DATABASE_CA_CERT: 698: DOCKERHUB_USERNAME: *** 699: DOCKERHUB_TOKEN: *** 700: TEST_CNPG_DATABASE: serviceradar_web_ng_test 701: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 702: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 703: CARGO_HOME: /home/runner/.cargo 704: CARGO_INCREMENTAL: 0 705: CARGO_TERM_COLOR: always 706: ##[endgroup] 707: SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS. 708: ##[error]Process completed with exit code 1. 709: Post job cleanup. 710: [command]/usr/bin/git version 711: git version 2.52.0 712: Temporarily overriding HOME='/home/runner/_work/_temp/361bb1cd-450e-4625-b6f0-634487dd9354' before making global git config changes 713: Adding repository directory to the temporary git global config as a safe directory 714: [command]/usr/bin/git config --global --add safe.directory /home/runner/_work/serviceradar/serviceradar 715: Removing SSH command configuration 716: [command]/usr/bin/git config --local --name-only --get-regexp core\.sshCommand 717: [command]/usr/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :" 718: fatal: No url found for submodule path 'swift/FieldSurvey/LocalPackages/arrow-swift' in .gitmodules 719: ##[warning]The process '/usr/bin/git' failed with exit code 128 720: Cleaning up orphan processes ``` </details></td></tr></table>