feat: juniper classification #3093

Merged
mfreeman451 merged 1 commit from refs/pull/3093/head into staging 2026-03-27 22:04:15 +00:00
mfreeman451 commented 2026-03-27 22:03:47 +00:00 (Migrated from github.com)
Owner

Imported from GitHub pull request.

Original GitHub pull request: #3095
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/3095
Original created: 2026-03-27T22:03:47Z
Original updated: 2026-03-27T22:08:25Z
Original head: carverauto/serviceradar:3067-bug-snmp-discoverydevice-mapper-incorrectly-identifying-devices
Original base: staging
Original merged: 2026-03-27T22:04:15Z 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

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: #3095 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/3095 Original created: 2026-03-27T22:03:47Z Original updated: 2026-03-27T22:08:25Z Original head: carverauto/serviceradar:3067-bug-snmp-discoverydevice-mapper-incorrectly-identifying-devices Original base: staging Original merged: 2026-03-27T22:04:15Z 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?
Copilot commented 2026-03-27 22:08:24 +00:00 (Migrated from github.com)
Author
Owner

Imported GitHub PR review comment.

Original author: @Copilot
Original URL: https://github.com/carverauto/serviceradar/pull/3095#discussion_r3003467564
Original created: 2026-03-27T22:08:24Z
Original path: elixir/serviceradar_core/priv/device_enrichment/rules/juniper.yaml
Original line: 16

Rule id and reason imply this is a vJunos-specific router rule, but the match conditions also include generic "juniper"/"junos" tokens and will match non-vJunos Junos devices (as exercised by the new tests). Since classification_rule_id is persisted to device metadata, consider either (a) renaming the rule id/reason to reflect broader Junos router classification, or (b) tightening the match tokens to vJunos-specific values and adding a separate rule for generic Juniper/Junos routers.

Imported GitHub PR review comment. Original author: @Copilot Original URL: https://github.com/carverauto/serviceradar/pull/3095#discussion_r3003467564 Original created: 2026-03-27T22:08:24Z Original path: elixir/serviceradar_core/priv/device_enrichment/rules/juniper.yaml Original line: 16 --- Rule id and reason imply this is a vJunos-specific router rule, but the match conditions also include generic "juniper"/"junos" tokens and will match non-vJunos Junos devices (as exercised by the new tests). Since `classification_rule_id` is persisted to device metadata, consider either (a) renaming the rule id/reason to reflect broader Junos router classification, or (b) tightening the match tokens to vJunos-specific values and adding a separate rule for generic Juniper/Junos routers.
Copilot commented 2026-03-27 22:08:25 +00:00 (Migrated from github.com)
Author
Owner

Imported GitHub PR review comment.

Original author: @Copilot
Original URL: https://github.com/carverauto/serviceradar/pull/3095#discussion_r3003467578
Original created: 2026-03-27T22:08:25Z
Original path: elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex
Original line: 665

The jsonb upsert fragment hard-codes the list of classification metadata keys to strip, duplicating @classification_metadata_keys defined at the top of the module. To avoid the two lists drifting (and leaving stale classification keys behind), consider deriving the fragment from @classification_metadata_keys (e.g., building the subtraction expression at compile-time or using the jsonb - text[] operator with an array of keys).

              "(COALESCE(?, '{}'::jsonb) - ?::text[]) || COALESCE(EXCLUDED.metadata, '{}'::jsonb)",
              d.metadata,
              ^@classification_metadata_keys
Imported GitHub PR review comment. Original author: @Copilot Original URL: https://github.com/carverauto/serviceradar/pull/3095#discussion_r3003467578 Original created: 2026-03-27T22:08:25Z Original path: elixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex Original line: 665 --- The jsonb upsert fragment hard-codes the list of classification metadata keys to strip, duplicating `@classification_metadata_keys` defined at the top of the module. To avoid the two lists drifting (and leaving stale classification keys behind), consider deriving the fragment from `@classification_metadata_keys` (e.g., building the subtraction expression at compile-time or using the `jsonb - text[]` operator with an array of keys). ```suggestion "(COALESCE(?, '{}'::jsonb) - ?::text[]) || COALESCE(EXCLUDED.metadata, '{}'::jsonb)", d.metadata, ^@classification_metadata_keys ```
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
carverauto/serviceradar!3093
No description provided.