POC: VRF-aware IP matching in the graph matcher#566
Draft
ldrozdz93 wants to merge 1 commit into
Draft
Conversation
Address-only IPAddress match rule collapses same-address/different-VRF IPs to one graph node. Add ipAddress.vrf.name as a required primary rule (paths switched to camelCase to match stored protojson) so they stay distinct. POC only. Proof + drift notes in controller-integrations .session/diode-graph-vrf-proof/PROOF.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Vulnerability Scan: Failed — diode-authImage:
Commit: 081ac39 |
Vulnerability Scan: Failed — diode-reconcilerImage:
Commit: 081ac39 |
|
Go test coverage
Total coverage: 55.1% |
Vulnerability Scan: Failed — diode-ingesterImage:
Commit: 081ac39 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is a proof-of-concept to make IP address entity matching VRF-aware by aligning matching rule field paths with the JSON shape the graph stores and by including VRF identity in the match criteria.
Changes:
- Update the example entity matching config to use protojson-style
ipAddress.*paths and includeipAddress.vrf.namein IP identity. - Add a focused unit test suite that exercises
Matcher.FindBestMatchagainst an in-memoryNodeFinderto demonstrate/address VRF collisions and field-path drift.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| diode-server/examples/entity_matching_config.yaml | Updates IPAddress matching rules to use ipAddress.* paths and add VRF name to prevent cross-VRF collapses. |
| diode-server/entitymatcher/vrf_ip_matching_test.go | Adds POC tests validating how field paths and VRF affect IPAddress matching behavior. |
Comments suppressed due to low confidence (2)
diode-server/examples/entity_matching_config.yaml:104
- Requiring
ipAddress.vrf.namewill prevent matching (and likely increase duplicate nodes) for IP addresses that have no VRF set (global VRF).IPAddress.vrfis optional in the proto, and the codebase already constructs IPAddress entities without a VRF (e.g., reconciler/server_internal_test.go:169–172). Consider either normalizing “global VRF” to an explicit VRF value at ingestion time, or updating the matcher/config semantics so “missing on both sides” can be treated as a match for this identity field.
- field_path: "ipAddress.vrf.name"
match_type: exact
weight: 1.0
required: true
confidence: 0.95
diode-server/entitymatcher/vrf_ip_matching_test.go:47
- In the in-memory finder, JSON unmarshal errors are silently ignored, which can make tests pass/fail for the wrong reason if node Data is malformed. It’s safer to fail fast (return the error) so the test output clearly indicates bad fixture data.
var m map[string]any
if err := json.Unmarshal(n.Data, &m); err != nil {
continue
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+86
to
+90
| # POC (VRF-aware IP matching): paths are camelCase to match the protojson the | ||
| # graph actually stores/serialises (`ipAddress`, not `ip_address`), and | ||
| # `ipAddress.vrf.name` is added so two allocations of the same address in | ||
| # different VRFs stay distinct nodes. rd is intentionally ignored; VRF name | ||
| # is sufficient identity. See .session/diode-graph-vrf-proof/PROOF.md. |
Comment on lines
+3
to
+6
| // POC + PROOF: does the diode graph matcher honour VRF when matching IP addresses? | ||
| // | ||
| // Full write-up: .session/diode-graph-vrf-proof/PROOF.md | ||
| // |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AFK POC