diff --git a/snmp-discovery/config/config.go b/snmp-discovery/config/config.go index 9d12a3b6..6b18274e 100644 --- a/snmp-discovery/config/config.go +++ b/snmp-discovery/config/config.go @@ -2,6 +2,16 @@ package config import "time" +// TargetType selects how a target is ingested into NetBox. +// TargetTypeDevice (default, empty/"device") emits a dcim.Device graph. +// TargetTypeVirtualMachine ("virtualmachine") emits a +// virtualization.VirtualMachine graph. Stored lowercase; user input is +// case-insensitive (see manager.applyDefaults). +const ( + TargetTypeDevice = "device" + TargetTypeVirtualMachine = "virtualmachine" +) + // Status represents the status of the snmp-discovery service type Status struct { StartTime time.Time `json:"start_time"` @@ -91,6 +101,9 @@ type Defaults struct { VLAN VLANDefaults `yaml:"vlan,omitempty"` InterfacePatterns []InterfacePattern `yaml:"interface_patterns,omitempty"` InterfaceExcludePatterns []string `yaml:"interface_exclude_patterns,omitempty"` + Type string `yaml:"type,omitempty"` // TargetTypeDevice | TargetTypeVirtualMachine; default Device + Cluster string `yaml:"cluster,omitempty"` // optional VM cluster name; ignored when Type == TargetTypeDevice + ClusterType string `yaml:"cluster_type,omitempty"` // optional VM cluster type (NetBox virtualization.ClusterType); required to auto-create a Cluster } // MergeDefaults merges target-level override defaults with policy-level defaults @@ -198,6 +211,17 @@ func MergeDefaults(policyDefaults, overrideDefaults *Defaults) *Defaults { merged.InterfaceExcludePatterns = overrideDefaults.InterfaceExcludePatterns } + // Override Type / Cluster / ClusterType if provided. + if overrideDefaults.Type != "" { + merged.Type = overrideDefaults.Type + } + if overrideDefaults.Cluster != "" { + merged.Cluster = overrideDefaults.Cluster + } + if overrideDefaults.ClusterType != "" { + merged.ClusterType = overrideDefaults.ClusterType + } + return &merged } diff --git a/snmp-discovery/config/config_test.go b/snmp-discovery/config/config_test.go index f2330cea..58f0e1d5 100644 --- a/snmp-discovery/config/config_test.go +++ b/snmp-discovery/config/config_test.go @@ -346,6 +346,41 @@ func TestMergeDefaults_PolicyDeviceModelManufacturerPlatformSurviveNilOverride(t assert.Equal(t, "policy-plat", merged.Device.Platform) } +func TestMergeDefaults_TypeAndCluster(t *testing.T) { + policyDef := &Defaults{Type: TargetTypeDevice, Cluster: ""} + override := &Defaults{Type: TargetTypeVirtualMachine, Cluster: "proxmox-a"} + merged := MergeDefaults(policyDef, override) + assert.Equal(t, TargetTypeVirtualMachine, merged.Type) + assert.Equal(t, "proxmox-a", merged.Cluster) + + // Empty override Type must not clobber a non-empty policy Type. + merged2 := MergeDefaults(&Defaults{Type: TargetTypeVirtualMachine, Cluster: "proxmox-a"}, &Defaults{}) + assert.Equal(t, TargetTypeVirtualMachine, merged2.Type) + assert.Equal(t, "proxmox-a", merged2.Cluster) + + // Override Cluster wins over policy Cluster when both set. + merged3 := MergeDefaults( + &Defaults{Type: TargetTypeVirtualMachine, Cluster: "policy-cluster"}, + &Defaults{Cluster: "override-cluster"}, + ) + assert.Equal(t, TargetTypeVirtualMachine, merged3.Type) + assert.Equal(t, "override-cluster", merged3.Cluster) + + // ClusterType merges and follows the same override-wins rule. + merged4 := MergeDefaults( + &Defaults{Type: TargetTypeVirtualMachine, ClusterType: "policy-type"}, + &Defaults{ClusterType: "override-type"}, + ) + assert.Equal(t, "override-type", merged4.ClusterType) + + // Empty override ClusterType must not clobber a non-empty policy ClusterType. + merged5 := MergeDefaults( + &Defaults{Type: TargetTypeVirtualMachine, ClusterType: "policy-type"}, + &Defaults{}, + ) + assert.Equal(t, "policy-type", merged5.ClusterType) +} + func TestTargetNetboxID_parsed(t *testing.T) { input := ` host: "192.168.1.1" diff --git a/snmp-discovery/mapping/mappers.go b/snmp-discovery/mapping/mappers.go index 3189f0a6..62d8e6ed 100644 --- a/snmp-discovery/mapping/mappers.go +++ b/snmp-discovery/mapping/mappers.go @@ -21,10 +21,15 @@ const ( maxInterfaceSpeed = 2147483647 ) -// MTU constants +// MTU constants. The upper bound matches NetBox's own +// PositiveIntegerField(MaxValueValidator(65536)) on dcim.Interface.mtu and +// virtualization.VMInterface.mtu — values outside [1, 65536] are rejected +// by the reconciler, which would drop the entire interface (and its IPs / +// MACs) from a change-set. We skip the field instead so the rest of the +// entity still ingests. const ( minInterfaceMTU = 1 - maxInterfaceMTU = 2147483647 + maxInterfaceMTU = 65536 ) // IPAddressMapper is a struct that maps IP addresses to entities @@ -837,9 +842,11 @@ func (m *InterfaceMapper) Map(values map[ObjectIDIndex]*ObjectIDValue, mappingEn m.logger.Debug("mtu is zero, skipping", "value", value.Value) continue } - // Check if MTU is within valid range (1 to 2147483647 inclusive) and not overflowing int32 + // Check if MTU is within NetBox's accepted range (1 to 65536 inclusive). + // Values outside this range are skipped (field left unset) rather + // than dropping the whole interface in the reconciler. if mtu < minInterfaceMTU || mtu > maxInterfaceMTU { - m.logger.Warn("interface MTU is outside valid range (1-2147483647) or overflows int32", "mtu", mtu, + m.logger.Warn("interface MTU is outside valid range (1-65536), skipping field", "mtu", mtu, "value", value.Value, "mapping_id", propertyMappingEntry.OID, "interface_index", objectID) continue } diff --git a/snmp-discovery/mapping/mappers_test.go b/snmp-discovery/mapping/mappers_test.go index bf919ea3..064981c5 100644 --- a/snmp-discovery/mapping/mappers_test.go +++ b/snmp-discovery/mapping/mappers_test.go @@ -1138,7 +1138,7 @@ func TestInterfaceMapper_Map(t *testing.T) { OID: "1.3.6.1.2.1.2.2.1.4.1", Index: "1", Parent: "1.3.6.1.2.1.2.2.1.4", - Value: "2147483647", + Value: "65536", Type: mapping.Integer, }, }, @@ -1167,7 +1167,7 @@ func TestInterfaceMapper_Map(t *testing.T) { defaults: nil, expectedEntity: &diode.Interface{ Name: mapping.StringPtr("eth0"), - Mtu: int64Ptr(2147483647), // MTU should be set when value is at maximum valid range + Mtu: int64Ptr(65536), // MTU should be set when value is at NetBox's maximum (65536) }, expectError: false, }, @@ -1226,7 +1226,7 @@ func TestInterfaceMapper_Map(t *testing.T) { expectError: false, }, { - name: "mapping with MTU just above maximum should result in nil MTU", + name: "mapping with MTU just above NetBox's maximum (65537) should result in nil MTU", values: map[mapping.ObjectIDIndex]*mapping.ObjectIDValue{ "1.3.6.1.2.1.2.2.1.1.1": { OID: "1.3.6.1.2.1.2.2.1.1.1", @@ -1246,7 +1246,7 @@ func TestInterfaceMapper_Map(t *testing.T) { OID: "1.3.6.1.2.1.2.2.1.4.1", Index: "1", Parent: "1.3.6.1.2.1.2.2.1.4", - Value: "2147483648", + Value: "65537", Type: mapping.Integer, }, }, @@ -1275,7 +1275,7 @@ func TestInterfaceMapper_Map(t *testing.T) { defaults: nil, expectedEntity: &diode.Interface{ Name: mapping.StringPtr("eth0"), - Mtu: nil, // MTU should be nil when value is just above maximum + Mtu: nil, // MTU should be nil when value is just above NetBox's maximum (65536) }, expectError: false, }, diff --git a/snmp-discovery/mapping/stubs.go b/snmp-discovery/mapping/stubs.go index 0154ed87..04097675 100644 --- a/snmp-discovery/mapping/stubs.go +++ b/snmp-discovery/mapping/stubs.go @@ -79,8 +79,15 @@ func newMACMatchStub(mac *diode.MACAddress) *diode.MACAddress { // // Metadata.source_match (e.g. netbox_id) is the diode-netbox-plugin's // PK-based match path, so it must not diverge between rich and stub. -// Annotation metadata such as run_id is intentionally NOT copied — -// stubs are matcher-only. +// Annotation metadata such as run_id is NOT copied here at stub +// construction — but note that the run_id annotation walker +// (annotateEntitiesWithRunID) DOES reach cycle-break stubs embedded at +// rich Device.PrimaryIp4.AssignedObject.Device because that chain is +// traversed before PruneNestedRefs runs. Diode ignores run_id on +// matcher refs (matching uses Name / Site / Tenant / source_match / +// asset_tag / primary_ip), so the leaked run_id is inert payload, not +// a correctness issue. The "stubs are matcher-only" property holds +// for everything that matters to NetBox resolution. func newDeviceStub(d *diode.Device) *diode.Device { if d == nil { return nil @@ -142,6 +149,150 @@ func CurrentDeviceFrom(entities []diode.Entity) *diode.Device { return nil } +// CurrentVirtualMachineFrom returns the first *diode.VirtualMachine in +// the slice, or nil. Mirrors CurrentDeviceFrom for the VM-target path +// added by TransformToVirtualMachine — when a target is flagged as a +// VM, the runner threads this into PruneNestedRefsVM instead of +// PruneNestedRefs. +func CurrentVirtualMachineFrom(entities []diode.Entity) *diode.VirtualMachine { + for _, e := range entities { + if vm, ok := e.(*diode.VirtualMachine); ok { + return vm + } + } + return nil +} + +// newVMMatchStub returns a VirtualMachine populated with matcher +// fields (Name, Site, Cluster, Tenant, Role, PrimaryIp4/6 matcher +// stubs) plus metadata.source_match when present on the source. Used +// wherever a VirtualMachine appears as a nested reference +// (VMInterface.VirtualMachine, and the stub assigned to a stubbed +// VMInterface used by IPAddress.AssignedObject). Cluster is included +// because the NetBox plugin VM matcher uses (name, cluster) and +// (name, cluster, tenant) — omitting it would fail to resolve the +// right VM when cluster is the discriminating field. +func newVMMatchStub(vm *diode.VirtualMachine) *diode.VirtualMachine { + if vm == nil { + return nil + } + stub := &diode.VirtualMachine{ + Name: vm.Name, + Site: vm.Site, + Cluster: vm.Cluster, + Tenant: vm.Tenant, + Role: vm.Role, + PrimaryIp4: newIPMatchStub(vm.PrimaryIp4), + PrimaryIp6: newIPMatchStub(vm.PrimaryIp6), + } + if sm, ok := vm.Metadata["source_match"]; ok { + stub.Metadata = diode.Metadata{"source_match": sm} + } + return stub +} + +// newVMInterfaceStub returns a VMInterface populated with matcher +// fields and a stubbed VirtualMachine ref. Used for +// IPAddress.AssignedObject and for VMInterface.Parent / .Bridge +// during VM-rooted pruning. PrimaryMacAddress is preserved (via +// newMACMatchStub) so the stub keeps the unique_primary_mac_address +// matcher precedence and resolves to the same VMInterface as the rich +// top-level entity. +func newVMInterfaceStub(iface *diode.VMInterface, vmStub *diode.VirtualMachine) *diode.VMInterface { + if iface == nil { + return nil + } + return &diode.VMInterface{ + Name: iface.Name, + VirtualMachine: vmStub, + PrimaryMacAddress: newMACMatchStub(iface.PrimaryMacAddress), + } +} + +// PruneNestedRefsVM is the VM-rooted analog of PruneNestedRefs. Walks +// entities once and replaces nested VirtualMachine and VMInterface +// references with matcher-only stubs. Top-level rich VirtualMachine +// and top-level VMInterface entities are left unchanged — only nested +// references *to* them are rewritten. +// +// Call from the runner AFTER annotateDeviceWithSourceMatch and +// annotateEntitiesWithRunID, BEFORE Ingest, matching the +// PruneNestedRefs call ordering. +// +// No-op if entities is empty or currentVM is nil. +func PruneNestedRefsVM(entities []diode.Entity, currentVM *diode.VirtualMachine) { + if len(entities) == 0 || currentVM == nil { + return + } + + vmStub := newVMMatchStub(currentVM) + + // Build a name -> top-level VMInterfaces index. Slice-valued to + // mirror PruneNestedRefs's ifaceByName treatment of cross-member + // duplicates: when a name is ambiguous (more than one top-level + // VMInterface shares it), the stubForIface owner-rewrite skips + // rather than silently rebinding to whichever entry happened to + // land in the map first. Single-VM walks have unique interface + // names so the unambiguous (len==1) branch is the common path. + vmIfaceByName := map[string][]*diode.VMInterface{} + for _, e := range entities { + if v, ok := e.(*diode.VMInterface); ok && v != nil && v.Name != nil { + vmIfaceByName[*v.Name] = append(vmIfaceByName[*v.Name], v) + } + } + + // Cache stubs per source VMInterface so Parent/Bridge/IP-assigned + // references to the same iface share one stub and pruning is O(N). + stubCache := map[*diode.VMInterface]*diode.VMInterface{} + stubForIface := func(ref *diode.VMInterface) *diode.VMInterface { + if ref == nil { + return nil + } + if cached, ok := stubCache[ref]; ok { + return cached + } + // Prefer the top-level VMInterface for this name (it's the + // authoritative entry — matches Device-prune's stubForIface). + // Only rewrite owner when the by-name lookup is unambiguous; + // duplicate names leave owner as `ref` so we don't silently + // repoint to a wrong sibling. + owner := ref + if ref.Name != nil { + if tops, ok := vmIfaceByName[*ref.Name]; ok && len(tops) == 1 && tops[0] != nil { + owner = tops[0] + } + } + stub := newVMInterfaceStub(owner, vmStub) + stubCache[ref] = stub + return stub + } + + for _, e := range entities { + switch v := e.(type) { + case *diode.VMInterface: + if v == nil { + continue + } + if v.VirtualMachine == currentVM { + v.VirtualMachine = vmStub + } + if v.Parent != nil { + v.Parent = stubForIface(v.Parent) + } + if v.Bridge != nil { + v.Bridge = stubForIface(v.Bridge) + } + case *diode.IPAddress: + if v == nil { + continue + } + if vmIf, ok := v.AssignedObject.(*diode.VMInterface); ok && vmIf != nil { + v.AssignedObject = stubForIface(vmIf) + } + } + } +} + // PruneNestedRefs walks entities once and replaces nested Device and // Interface references with matcher-only stubs. Top-level rich Device // entities are left unchanged — only nested references *to* them on diff --git a/snmp-discovery/mapping/stubs_test.go b/snmp-discovery/mapping/stubs_test.go index 78dff989..b3960564 100644 --- a/snmp-discovery/mapping/stubs_test.go +++ b/snmp-discovery/mapping/stubs_test.go @@ -564,3 +564,181 @@ func strDerefSafe(p *string) string { } return *p } + +// --- VM-rooted prune helpers --- + +func TestCurrentVirtualMachineFrom_NilWhenAbsent(t *testing.T) { + if got := CurrentVirtualMachineFrom(nil); got != nil { + t.Errorf("got %v, want nil", got) + } + dev := &diode.Device{Name: strPtr("d1")} + if got := CurrentVirtualMachineFrom([]diode.Entity{dev}); got != nil { + t.Errorf("Device-only slice should return nil; got %v", got) + } +} + +func TestCurrentVirtualMachineFrom_ReturnsFirstVM(t *testing.T) { + vm1 := &diode.VirtualMachine{Name: strPtr("a")} + vm2 := &diode.VirtualMachine{Name: strPtr("b")} + got := CurrentVirtualMachineFrom([]diode.Entity{vm1, vm2}) + if got != vm1 { + t.Errorf("got %p, want %p (first)", got, vm1) + } +} + +func TestNewVMMatchStub_KeepsMatcherFieldsAndSourceMatch(t *testing.T) { + vm := &diode.VirtualMachine{ + Name: strPtr("vyos-edge-1"), + Site: &diode.Site{Name: strPtr("Lab Site")}, + Cluster: &diode.Cluster{Name: strPtr("proxmox-a")}, + Tenant: &diode.Tenant{Name: strPtr("NetOps")}, + Role: &diode.DeviceRole{Name: strPtr("Router")}, + Description: strPtr("rich"), + Metadata: diode.Metadata{"source_match": diode.Metadata{"netbox_id": 42}, "run_id": "x"}, + } + stub := newVMMatchStub(vm) + require.NotNil(t, stub) + assert.NotSame(t, vm, stub, "stub must be a new pointer") + assert.Equal(t, "vyos-edge-1", *stub.Name) + assert.Equal(t, "Lab Site", *stub.Site.Name) + assert.Equal(t, "proxmox-a", *stub.Cluster.Name, "Cluster is a matcher field for plugin VM resolution") + assert.Equal(t, "NetOps", *stub.Tenant.Name) + assert.Equal(t, "Router", *stub.Role.Name) + assert.Nil(t, stub.Description, "rich-only field must be dropped") + // source_match carries; run_id does NOT (matcher-only stub). + sm, ok := stub.Metadata["source_match"].(diode.Metadata) + require.True(t, ok) + assert.Equal(t, 42, sm["netbox_id"]) + _, hasRun := stub.Metadata["run_id"] + assert.False(t, hasRun, "stub must NOT carry run_id") +} + +func TestNewVMInterfaceStub_KeepsNameVMRefAndMACStub(t *testing.T) { + vmStub := &diode.VirtualMachine{Name: strPtr("vyos-edge-1")} + mac := &diode.MACAddress{MacAddress: strPtr("aa:bb:cc:dd:ee:ff"), AssignedObject: &diode.Interface{Name: strPtr("nope")}} + src := &diode.VMInterface{ + Name: strPtr("eth0"), + VirtualMachine: &diode.VirtualMachine{Name: strPtr("rich")}, + Description: strPtr("WAN"), + PrimaryMacAddress: mac, + } + stub := newVMInterfaceStub(src, vmStub) + require.NotNil(t, stub) + assert.NotSame(t, src, stub) + assert.Equal(t, "eth0", *stub.Name) + assert.Same(t, vmStub, stub.VirtualMachine, "VirtualMachine ref must be the provided stub") + assert.Nil(t, stub.Description, "rich-only field must be dropped") + require.NotNil(t, stub.PrimaryMacAddress) + assert.NotSame(t, mac, stub.PrimaryMacAddress, "PrimaryMacAddress must itself be a matcher-only stub") + assert.Nil(t, stub.PrimaryMacAddress.AssignedObject, "MAC stub must strip AssignedObject") +} + +func TestPruneNestedRefsVM_StubsNestedVMAndVMInterfaceRefs(t *testing.T) { + vm := &diode.VirtualMachine{ + Name: strPtr("vyos-edge-1"), + Description: strPtr("rich-top-level"), + Site: &diode.Site{Name: strPtr("Dell Lab Site")}, + Cluster: &diode.Cluster{Name: strPtr("proxmox-a")}, + Tenant: &diode.Tenant{Name: strPtr("NetOps")}, + Metadata: diode.Metadata{"source_match": diode.Metadata{"netbox_id": 42}}, + } + parent := &diode.VMInterface{Name: strPtr("bond0"), VirtualMachine: vm} + vmIface := &diode.VMInterface{ + Name: strPtr("eth0"), + VirtualMachine: vm, + Parent: parent, + } + ip := &diode.IPAddress{Address: strPtr("192.0.2.1/24"), AssignedObject: vmIface} + entities := []diode.Entity{vm, parent, vmIface, ip} + + PruneNestedRefsVM(entities, CurrentVirtualMachineFrom(entities)) + + // Top-level VM still rich. + assert.Equal(t, "rich-top-level", *vm.Description, "top-level VM should retain rich fields") + // Top-level VMInterfaces still rich (Name etc.); the nested VM ref + // inside each got stubbed. + assert.NotSame(t, vm, parent.VirtualMachine, "top-level VMInterface.VirtualMachine should be a stub") + assert.NotSame(t, vm, vmIface.VirtualMachine, "top-level VMInterface.VirtualMachine should be a stub") + + // Nested VM stub: matcher fields + source_match preserved; rich-only fields dropped. + stub := vmIface.VirtualMachine + assert.Equal(t, "vyos-edge-1", *stub.Name) + assert.Equal(t, "Dell Lab Site", *stub.Site.Name) + assert.Equal(t, "proxmox-a", *stub.Cluster.Name, "Cluster matcher field required for plugin VM resolution") + assert.Equal(t, "NetOps", *stub.Tenant.Name) + sm, ok := stub.Metadata["source_match"].(diode.Metadata) + require.True(t, ok) + assert.Equal(t, 42, sm["netbox_id"]) + assert.Nil(t, stub.Description, "rich-only field on nested VM stub must be dropped") + + // VMInterface.Parent stubbed (Name matcher only, VM is the same VM stub). + require.NotNil(t, vmIface.Parent) + assert.NotSame(t, parent, vmIface.Parent, "VMInterface.Parent should be a stub") + assert.Equal(t, "bond0", *vmIface.Parent.Name) + + // IPAddress.AssignedObject stubbed to a matcher-only VMInterface. + assigned, ok := ip.AssignedObject.(*diode.VMInterface) + require.True(t, ok, "IPAddress.AssignedObject should be *VMInterface; got %T", ip.AssignedObject) + assert.NotSame(t, vmIface, assigned, "IPAddress.AssignedObject should be a stub, not the rich pointer") + assert.Equal(t, "eth0", *assigned.Name) +} + +func TestPruneNestedRefsVM_NoOpOnEmptyOrNil(t *testing.T) { + assert.NotPanics(t, func() { PruneNestedRefsVM(nil, nil) }) + assert.NotPanics(t, func() { PruneNestedRefsVM([]diode.Entity{}, nil) }) + assert.NotPanics(t, func() { + PruneNestedRefsVM([]diode.Entity{&diode.VirtualMachine{Name: strPtr("v")}}, nil) + }) +} + +// Regression: if two top-level VMInterfaces share the same Name, the +// stubForIface owner-rewrite must NOT silently rebind a nested ref to +// whichever entry happened to land in the by-name index first. Mirrors +// PruneNestedRefs's `len(tops) == 1` guard for cross-member duplicates. +// +// Discriminator: dupA and dupB carry distinct PrimaryMacAddress values. +// newVMInterfaceStub copies PrimaryMacAddress (via newMACMatchStub) from +// its `owner` argument. With the guard, owner stays as the original +// `ref` pointer (== dupB), so the stub's MAC must reflect dupB's MAC, +// NOT dupA's. A first-wins map (the buggy shape) would yield dupA's MAC. +func TestPruneNestedRefsVM_AmbiguousNameLeavesOwnerUnchanged(t *testing.T) { + vm := &diode.VirtualMachine{Name: strPtr("v1")} + dupAMac := "aa:aa:aa:aa:aa:aa" + dupBMac := "bb:bb:bb:bb:bb:bb" + dupA := &diode.VMInterface{ + Name: strPtr("dup"), + VirtualMachine: vm, + PrimaryMacAddress: &diode.MACAddress{MacAddress: &dupAMac}, + } + dupB := &diode.VMInterface{ + Name: strPtr("dup"), + VirtualMachine: vm, + PrimaryMacAddress: &diode.MACAddress{MacAddress: &dupBMac}, + } + // nested ref's pointer identity == dupB; if the prune rewires by + // name it would resolve to dupA (the first one inserted into the + // index) instead of preserving dupB's identity. + parent := &diode.VMInterface{ + Name: strPtr("child"), + VirtualMachine: vm, + Parent: dupB, + } + entities := []diode.Entity{vm, dupA, dupB, parent} + + PruneNestedRefsVM(entities, vm) + + require.NotNil(t, parent.Parent, "parent.Parent must remain set") + assert.Equal(t, "dup", *parent.Parent.Name, + "ambiguous-name parent ref must still resolve to the 'dup' identity, not be dropped") + assert.NotSame(t, dupA, parent.Parent, "parent.Parent must be a stub, not the rich dupA") + assert.NotSame(t, dupB, parent.Parent, "parent.Parent must be a stub, not the rich dupB") + + // The discriminating assertion: the stub MUST carry dupB's MAC, + // because the guard leaves owner = ref (== dupB) when the by-name + // lookup is ambiguous. A first-wins map would produce dupA's MAC. + require.NotNil(t, parent.Parent.PrimaryMacAddress, + "parent.Parent.PrimaryMacAddress must be populated from the unambiguous owner") + require.NotNil(t, parent.Parent.PrimaryMacAddress.MacAddress) + assert.Equal(t, dupBMac, *parent.Parent.PrimaryMacAddress.MacAddress, + "guard must leave owner = the original ref (dupB), not silently rebind to dupA") +} diff --git a/snmp-discovery/mapping/virtualmachine.go b/snmp-discovery/mapping/virtualmachine.go new file mode 100644 index 00000000..a11b9080 --- /dev/null +++ b/snmp-discovery/mapping/virtualmachine.go @@ -0,0 +1,331 @@ +package mapping + +import ( + "github.com/netboxlabs/diode-sdk-go/diode" + "github.com/netboxlabs/orb-discovery/snmp-discovery/config" +) + +// TransformToVirtualMachine rewrites a Device-rooted entity slice into +// a VirtualMachine-rooted one. The returned slice is fresh but +// contains a mix of newly-emitted VirtualMachine/VMInterface entities +// and IPAddress/VLAN entities reused from the input. Input IPAddress +// entities are mutated in place (AssignedObject is rewritten from +// *Interface to *VMInterface) — consistent with PruneNestedRefs and +// other downstream mutations. Caller must not retain the input slice +// expecting it to be unchanged. +// +// Mappings: +// - *diode.Device -> *diode.VirtualMachine +// - *diode.Interface -> *diode.VMInterface (with VirtualMachine +// ref); Parent/Bridge remapped to the +// corresponding VMInterface by identity +// (with name fallback). +// - *diode.IPAddress -> *diode.IPAddress with AssignedObject +// rewritten from *Interface to +// *VMInterface (identity-first lookup +// in the iface map, with a name +// fallback). Non-*Interface refs +// (*FHRPGroup, etc.) and nil refs pass +// through unchanged. IPAddress entity +// is mutated in place. +// - *diode.VLAN -> passthrough. +// - *diode.VirtualChassis, +// *diode.Device with +// VcPosition != nil, +// *diode.Module, +// *diode.ModuleBay -> dropped (no VM analog). +// +// Cluster on the VM is set only when defaults.Cluster is non-empty. +// +// The caller is responsible for guarding the call site on +// defaults.Type == config.TargetTypeVirtualMachine. +func TransformToVirtualMachine(entities []diode.Entity, defaults *config.Defaults) []diode.Entity { + if len(entities) == 0 { + return entities + } + + // Phase 1: enumerate every *diode.Interface referenced anywhere in + // the graph and emit one VMInterface per distinct *Interface. The + // snmp-discovery mapper intentionally suppresses top-level emission + // of any Interface that is also reachable via IPAddress.AssignedObject + // (mapping.go::getAssignedInterfaces), so a Device + an IP carrying + // the only Interface ref is a real input shape. A top-level-only + // walk would emit zero VMInterfaces and leave the IP carrying an + // *Interface ref — Codex round-1 BLOCKER. + var ( + vm *diode.VirtualMachine + srcDevice *diode.Device + ifaceMap = make(map[*diode.Interface]*diode.VMInterface, len(entities)) + nameMap = make(map[string]*diode.VMInterface, len(entities)) + srcIfaceOrder []*diode.Interface // canonical source-Interface insertion order (deterministic) + topIfaceOrder []*diode.Interface // top-level subset (preserves source order in output) + ips []*diode.IPAddress + vlans []*diode.VLAN + ) + + ensureVMInterface := func(src *diode.Interface) *diode.VMInterface { + if src == nil { + return nil + } + if vmIface, ok := ifaceMap[src]; ok { + return vmIface + } + vmIface := interfaceToVMInterface(src) + ifaceMap[src] = vmIface + srcIfaceOrder = append(srcIfaceOrder, src) // first-encounter insertion order + if src.Name != nil && *src.Name != "" { + // First write wins for the name map; later duplicate names + // keep the first VMInterface as the by-name resolution target. + if _, exists := nameMap[*src.Name]; !exists { + nameMap[*src.Name] = vmIface + } + } + return vmIface + } + + // First scan: top-level entities. Pins source order for the + // VMInterface output slice. + for _, e := range entities { + switch v := e.(type) { + case *diode.Device: + if v == nil || v.VcPosition != nil { + continue // skip stack members + } + if vm == nil { + srcDevice = v + vm = deviceToVM(v, defaults) + } + case *diode.Interface: + if v == nil { + continue + } + ensureVMInterface(v) + topIfaceOrder = append(topIfaceOrder, v) + case *diode.IPAddress: + if v != nil { + ips = append(ips, v) + } + case *diode.VLAN: + if v != nil { + vlans = append(vlans, v) + } + // All other entity types (VirtualChassis, Module, ModuleBay, + // chassis members reached as nested-only refs, etc.) are + // dropped on the floor. + } + } + + // Second scan: harvest *Interface refs that only exist nested. + for _, ip := range ips { + if src, ok := ip.AssignedObject.(*diode.Interface); ok && src != nil { + ensureVMInterface(src) + } + } + if srcDevice != nil { + if srcDevice.PrimaryIp4 != nil { + if src, ok := srcDevice.PrimaryIp4.AssignedObject.(*diode.Interface); ok && src != nil { + ensureVMInterface(src) + } + } + if srcDevice.PrimaryIp6 != nil { + if src, ok := srcDevice.PrimaryIp6.AssignedObject.(*diode.Interface); ok && src != nil { + ensureVMInterface(src) + } + } + } + // Parent/Bridge may also point at *Interface refs that aren't + // top-level. Walk to a fixed point so chains deeper than one + // (A.Parent=B, B.Parent=C) all land in the map. Iterate + // srcIfaceOrder by index — Go re-evaluates len() each iteration, + // so newly-appended entries are processed before the loop exits. + // Terminates because ensureVMInterface only appends for unseen + // source pointers. + for i := 0; i < len(srcIfaceOrder); i++ { + src := srcIfaceOrder[i] + if src.Parent != nil { + ensureVMInterface(src.Parent) + } + if src.Bridge != nil { + ensureVMInterface(src.Bridge) + } + } + + // Phase 2: attach VM ref + Parent/Bridge remap on every emitted + // VMInterface. + for srcIface, vmIface := range ifaceMap { + vmIface.VirtualMachine = vm + if srcIface.Parent != nil { + if mapped, ok := ifaceMap[srcIface.Parent]; ok { + vmIface.Parent = mapped + } else if srcIface.Parent.Name != nil { + if mapped, ok := nameMap[*srcIface.Parent.Name]; ok { + vmIface.Parent = mapped + } + } + } + if srcIface.Bridge != nil { + if mapped, ok := ifaceMap[srcIface.Bridge]; ok { + vmIface.Bridge = mapped + } else if srcIface.Bridge.Name != nil { + if mapped, ok := nameMap[*srcIface.Bridge.Name]; ok { + vmIface.Bridge = mapped + } + } + } + } + + // Phase 2b: rebuild the VM's PrimaryIp4/PrimaryIp6 so their + // AssignedObject points at a *matcher-only VMInterface stub*, not + // the rich top-level VMInterface that back-references the VM via + // `VirtualMachine`. Without this we'd create a cycle: + // VM -> PrimaryIp4 -> AssignedObject -> VMInterface(rich) + // -> VirtualMachine -> VM -> PrimaryIp4 -> ... + // which crashes the runner's debug-log proto conversion (proto + // conversion runs BEFORE PruneNestedRefsVM strips nested refs). + // Codex round-2 caught this; the Device path solves the analogous + // issue with detachForPrimaryIP in mapping.go. + rebuildPrimaryIP := func(src *diode.IPAddress) *diode.IPAddress { + if src == nil { + return nil + } + // Shallow copy so we don't mutate the Device's snapshot in place. + out := *src + if iface, ok := out.AssignedObject.(*diode.Interface); ok && iface != nil { + var vmIfRef *diode.VMInterface + if mapped, ok := ifaceMap[iface]; ok { + vmIfRef = mapped + } else if iface.Name != nil { + if mapped, ok := nameMap[*iface.Name]; ok { + vmIfRef = mapped + } + } + if vmIfRef != nil { + // Use the shared newVMInterfaceStub + newVMMatchStub + // helpers so the cycle-break stub carries the same + // matcher fields (Cluster, Site, Tenant, Role, + // source_match) that the NetBox plugin VM matcher + // uses to resolve `name+cluster` etc. — mirroring how + // the Device path's detachForPrimaryIP keeps the full + // Device snapshot (with PrimaryIp4/6 cleared) rather + // than a name-only stub. The inner VM's PrimaryIp4 + // runs through newIPMatchStub (AssignedObject=nil) so + // the cycle still terminates one hop deeper. + // newMACMatchStub on PrimaryMacAddress (inside + // newVMInterfaceStub) likewise strips the MAC's + // AssignedObject so proto conversion can't recurse + // MAC -> AssignedObject -> ... + out.AssignedObject = newVMInterfaceStub(vmIfRef, newVMMatchStub(vm)) + } else { + // Unresolvable: clear rather than leak the dropped *Interface. + out.AssignedObject = nil + } + } + return &out + } + if vm != nil && srcDevice != nil { + vm.PrimaryIp4 = rebuildPrimaryIP(srcDevice.PrimaryIp4) + vm.PrimaryIp6 = rebuildPrimaryIP(srcDevice.PrimaryIp6) + } + + // Phase 3: rewrite top-level IP AssignedObject from *Interface to + // *VMInterface (identity-first, name-fallback). Non-*Interface + // refs (*FHRPGroup, etc.) pass through unchanged. + for _, ip := range ips { + if src, ok := ip.AssignedObject.(*diode.Interface); ok && src != nil { + if mapped, ok := ifaceMap[src]; ok { + ip.AssignedObject = mapped + } else if src.Name != nil { + if mapped, ok := nameMap[*src.Name]; ok { + ip.AssignedObject = mapped + } + } + } + } + + // Assemble output. Order: VM, then VMInterfaces in top-level + // source order first, then any nested-only-harvested VMInterfaces + // in srcIfaceOrder (deterministic first-encounter), then IPs, then + // VLANs. + out := make([]diode.Entity, 0, 1+len(ifaceMap)+len(ips)+len(vlans)) + if vm != nil { + out = append(out, vm) + } + emitted := make(map[*diode.VMInterface]struct{}, len(ifaceMap)) + for _, src := range topIfaceOrder { + if vmIface, ok := ifaceMap[src]; ok { + out = append(out, vmIface) + emitted[vmIface] = struct{}{} + } + } + for _, src := range srcIfaceOrder { + vmIface := ifaceMap[src] + if _, already := emitted[vmIface]; already { + continue + } + out = append(out, vmIface) + } + for _, ip := range ips { + out = append(out, ip) + } + for _, vlan := range vlans { + out = append(out, vlan) + } + return out +} + +func deviceToVM(d *diode.Device, defaults *config.Defaults) *diode.VirtualMachine { + vm := &diode.VirtualMachine{ + Name: d.Name, + Status: d.Status, + Serial: d.Serial, + Role: d.Role, + Platform: d.Platform, + Site: d.Site, + Tenant: d.Tenant, + PrimaryIp4: d.PrimaryIp4, // rebuilt later in Phase 2b + PrimaryIp6: d.PrimaryIp6, // rebuilt later in Phase 2b + Description: d.Description, + Comments: d.Comments, + Tags: d.Tags, + CustomFields: d.CustomFields, + Metadata: d.Metadata, + Owner: d.Owner, + } + if defaults != nil && defaults.Cluster != "" { + cluster := defaults.Cluster + c := &diode.Cluster{Name: &cluster} + // NetBox's virtualization.Cluster requires a ClusterType FK + // when the cluster doesn't already exist. Without a Type set + // here, the Diode reconciler rejects every changeset with + // "virtualization.cluster: type: Field type is required". + // Operators who want on-the-fly cluster creation set + // defaults.cluster_type alongside defaults.cluster; operators + // referencing an existing NetBox cluster can omit cluster_type. + if defaults.ClusterType != "" { + ct := defaults.ClusterType + c.Type = &diode.ClusterType{Name: &ct} + } + vm.Cluster = c + } + return vm +} + +func interfaceToVMInterface(i *diode.Interface) *diode.VMInterface { + return &diode.VMInterface{ + Name: i.Name, + Enabled: i.Enabled, + Mtu: i.Mtu, + PrimaryMacAddress: i.PrimaryMacAddress, + Description: i.Description, + Mode: i.Mode, + UntaggedVlan: i.UntaggedVlan, + QinqSvlan: i.QinqSvlan, + VlanTranslationPolicy: i.VlanTranslationPolicy, + Vrf: i.Vrf, + Tags: i.Tags, + CustomFields: i.CustomFields, + TaggedVlans: i.TaggedVlans, + Metadata: i.Metadata, + Owner: i.Owner, + } +} diff --git a/snmp-discovery/mapping/virtualmachine_test.go b/snmp-discovery/mapping/virtualmachine_test.go new file mode 100644 index 00000000..3c14b85d --- /dev/null +++ b/snmp-discovery/mapping/virtualmachine_test.go @@ -0,0 +1,804 @@ +package mapping_test + +import ( + "testing" + + "github.com/netboxlabs/diode-sdk-go/diode" + "github.com/netboxlabs/orb-discovery/snmp-discovery/config" + "github.com/netboxlabs/orb-discovery/snmp-discovery/mapping" +) + +func sp(s string) *string { return &s } +func bp(b bool) *bool { return &b } +func ip64(i int64) *int64 { return &i } + +func TestTransformToVirtualMachine_DeviceToVM(t *testing.T) { + dev := &diode.Device{ + Name: sp("vyos-edge-1"), + Status: sp("active"), + Serial: sp("ABC123"), + Role: &diode.DeviceRole{Name: sp("Router")}, + Platform: &diode.Platform{Name: sp("VyOS 1.4")}, + Site: &diode.Site{Name: sp("Dell Lab Site")}, + Tenant: &diode.Tenant{Name: sp("NetOps")}, + Description: sp("VyOS edge router"), + Comments: sp("ingest"), + DeviceType: &diode.DeviceType{Model: sp("CSR")}, // must be dropped + Location: &diode.Location{Name: sp("rack-1")}, // must be dropped + AssetTag: sp("asset-1"), // must be dropped + } + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev}, + &config.Defaults{Type: config.TargetTypeVirtualMachine, Cluster: "proxmox-a"}, + ) + if len(out) != 1 { + t.Fatalf("entity count: got %d, want 1", len(out)) + } + vm, ok := out[0].(*diode.VirtualMachine) + if !ok { + t.Fatalf("entity type: got %T, want *diode.VirtualMachine", out[0]) + } + if vm.Name == nil || *vm.Name != "vyos-edge-1" { + t.Errorf("VM.Name: got %v", vm.Name) + } + if vm.Status == nil || *vm.Status != "active" { + t.Errorf("VM.Status: got %v", vm.Status) + } + if vm.Serial == nil || *vm.Serial != "ABC123" { + t.Errorf("VM.Serial: got %v", vm.Serial) + } + if vm.Role == nil || vm.Role.Name == nil || *vm.Role.Name != "Router" { + t.Errorf("VM.Role: got %v", vm.Role) + } + if vm.Platform == nil || vm.Platform.Name == nil || *vm.Platform.Name != "VyOS 1.4" { + t.Errorf("VM.Platform: got %v", vm.Platform) + } + if vm.Site == nil || vm.Site.Name == nil || *vm.Site.Name != "Dell Lab Site" { + t.Errorf("VM.Site: got %v", vm.Site) + } + if vm.Tenant == nil || vm.Tenant.Name == nil || *vm.Tenant.Name != "NetOps" { + t.Errorf("VM.Tenant: got %v", vm.Tenant) + } + if vm.Description == nil || *vm.Description != "VyOS edge router" { + t.Errorf("VM.Description: got %v", vm.Description) + } + if vm.Comments == nil || *vm.Comments != "ingest" { + t.Errorf("VM.Comments: got %v", vm.Comments) + } + if vm.Cluster == nil || vm.Cluster.Name == nil || *vm.Cluster.Name != "proxmox-a" { + t.Errorf("VM.Cluster: got %v", vm.Cluster) + } +} + +func TestTransformToVirtualMachine_DeviceFieldCarryAll(t *testing.T) { + tag := &diode.Tag{Name: sp("env-prod")} + owner := &diode.Owner{Name: sp("netops")} + primary4 := &diode.IPAddress{Address: sp("192.0.2.1/24")} + primary6 := &diode.IPAddress{Address: sp("2001:db8::1/64")} + dev := &diode.Device{ + Name: sp("vyos-edge-1"), + Tenant: &diode.Tenant{Name: sp("NetOps")}, + PrimaryIp4: primary4, + PrimaryIp6: primary6, + Tags: []*diode.Tag{tag}, + CustomFields: map[string]*diode.CustomFieldValue{"k": {}}, + Owner: owner, + } + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + vm := out[0].(*diode.VirtualMachine) + if vm.Tenant == nil || vm.Tenant.Name == nil || *vm.Tenant.Name != "NetOps" { + t.Errorf("VM.Tenant: got %v", vm.Tenant) + } + if vm.PrimaryIp4 == nil || vm.PrimaryIp4.Address == nil || *vm.PrimaryIp4.Address != "192.0.2.1/24" { + t.Errorf("VM.PrimaryIp4: got %v", vm.PrimaryIp4) + } + if vm.PrimaryIp6 == nil || vm.PrimaryIp6.Address == nil || *vm.PrimaryIp6.Address != "2001:db8::1/64" { + t.Errorf("VM.PrimaryIp6: got %v", vm.PrimaryIp6) + } + if len(vm.Tags) != 1 || vm.Tags[0] != tag { + t.Errorf("VM.Tags: got %v", vm.Tags) + } + if _, ok := vm.CustomFields["k"]; !ok { + t.Errorf("VM.CustomFields: missing 'k'; got %v", vm.CustomFields) + } + if vm.Owner != owner { + t.Errorf("VM.Owner: got %p, want %p", vm.Owner, owner) + } +} + +func TestTransformToVirtualMachine_InterfaceToVMInterface(t *testing.T) { + dev := &diode.Device{Name: sp("vyos-edge-1")} + iface := &diode.Interface{ + Device: dev, + Name: sp("eth0"), + Enabled: bp(true), + Mtu: ip64(1500), + PrimaryMacAddress: &diode.MACAddress{MacAddress: sp("00:11:22:33:44:55")}, + Description: sp("WAN"), + Mode: sp("access"), + UntaggedVlan: &diode.VLAN{Vid: ip64(10)}, + Vrf: &diode.VRF{Name: sp("default")}, + Speed: ip64(1000000), // must be dropped (no VMInterface.Speed) + Type: sp("1000base-t"), // must be dropped + Lag: &diode.Interface{Name: sp("bond0")}, // must be dropped + } + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev, iface}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + var vmIface *diode.VMInterface + var vm *diode.VirtualMachine + for _, e := range out { + switch v := e.(type) { + case *diode.VMInterface: + vmIface = v + case *diode.VirtualMachine: + vm = v + } + } + if vm == nil { + t.Fatal("expected a VirtualMachine entity in the output") + } + if vmIface == nil { + t.Fatal("expected a VMInterface entity in the output") + } + if vmIface.Name == nil || *vmIface.Name != "eth0" { + t.Errorf("VMInterface.Name: got %v", vmIface.Name) + } + if vmIface.VirtualMachine != vm { + t.Errorf("VMInterface.VirtualMachine: got %p, want the emitted VM %p", vmIface.VirtualMachine, vm) + } + if vmIface.Mtu == nil || *vmIface.Mtu != 1500 { + t.Errorf("VMInterface.Mtu: got %v", vmIface.Mtu) + } + if vmIface.PrimaryMacAddress == nil { + t.Errorf("VMInterface.PrimaryMacAddress: lost during transform") + } + if vmIface.UntaggedVlan == nil { + t.Errorf("VMInterface.UntaggedVlan: lost during transform") + } + if vmIface.Vrf == nil { + t.Errorf("VMInterface.Vrf: lost during transform") + } + if vmIface.Mode == nil || *vmIface.Mode != "access" { + t.Errorf("VMInterface.Mode: got %v", vmIface.Mode) + } +} + +func TestTransformToVirtualMachine_InterfaceFieldCarryAll(t *testing.T) { + tag := &diode.Tag{Name: sp("acl-trunk")} + owner := &diode.Owner{Name: sp("netops")} + vtp := &diode.VLANTranslationPolicy{Name: sp("vtp-1")} + dev := &diode.Device{Name: sp("vyos-edge-1")} + iface := &diode.Interface{ + Device: dev, + Name: sp("eth0"), + Tags: []*diode.Tag{tag}, + CustomFields: map[string]*diode.CustomFieldValue{"role": {}}, + Owner: owner, + VlanTranslationPolicy: vtp, + } + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev, iface}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + var vmIface *diode.VMInterface + for _, e := range out { + if v, ok := e.(*diode.VMInterface); ok { + vmIface = v + } + } + if vmIface == nil { + t.Fatal("missing VMInterface") + } + if len(vmIface.Tags) != 1 || vmIface.Tags[0] != tag { + t.Errorf("VMInterface.Tags: got %v", vmIface.Tags) + } + if _, ok := vmIface.CustomFields["role"]; !ok { + t.Errorf("VMInterface.CustomFields: missing 'role'; got %v", vmIface.CustomFields) + } + if vmIface.Owner != owner { + t.Errorf("VMInterface.Owner: got %p, want %p", vmIface.Owner, owner) + } + if vmIface.VlanTranslationPolicy != vtp { + t.Errorf("VMInterface.VlanTranslationPolicy: got %p, want %p", vmIface.VlanTranslationPolicy, vtp) + } +} + +func TestTransformToVirtualMachine_IPReanchoredOntoVMInterface(t *testing.T) { + dev := &diode.Device{Name: sp("vyos-edge-1")} + iface := &diode.Interface{Device: dev, Name: sp("eth0")} + ip := &diode.IPAddress{ + Address: sp("192.0.2.1/24"), + AssignedObject: iface, + } + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev, iface, ip}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + var vmIface *diode.VMInterface + var ipOut *diode.IPAddress + for _, e := range out { + switch v := e.(type) { + case *diode.VMInterface: + vmIface = v + case *diode.IPAddress: + ipOut = v + } + } + if vmIface == nil || ipOut == nil { + t.Fatalf("missing entity: vmIface=%p ipOut=%p", vmIface, ipOut) + } + assigned, ok := ipOut.AssignedObject.(*diode.VMInterface) + if !ok { + t.Fatalf("IPAddress.AssignedObject: got %T, want *diode.VMInterface", ipOut.AssignedObject) + } + if assigned != vmIface { + t.Errorf("IP not re-anchored onto the emitted VMInterface (got %p, want %p)", assigned, vmIface) + } +} + +func TestTransformToVirtualMachine_IPOnlyNestedInterface(t *testing.T) { + // Codex round-1 BLOCKER regression: the mapper filters + // IP-assigned interfaces out of the top-level slice, so an + // *Interface can exist only as IPAddress.AssignedObject. The + // transform must still emit a VMInterface for it and re-anchor + // the IP. + dev := &diode.Device{Name: sp("vyos-edge-1")} + hiddenIface := &diode.Interface{Device: dev, Name: sp("eth-hidden")} + ip := &diode.IPAddress{Address: sp("203.0.113.5/32"), AssignedObject: hiddenIface} + // Note: hiddenIface is NOT in the top-level slice. + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev, ip}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + var vmIface *diode.VMInterface + var ipOut *diode.IPAddress + for _, e := range out { + switch v := e.(type) { + case *diode.VMInterface: + vmIface = v + case *diode.IPAddress: + ipOut = v + } + } + if vmIface == nil { + t.Fatal("expected a VMInterface to be emitted from the IP-only nested *Interface") + } + if vmIface.Name == nil || *vmIface.Name != "eth-hidden" { + t.Errorf("VMInterface.Name: got %v, want eth-hidden", vmIface.Name) + } + if ipOut == nil { + t.Fatal("IPAddress was dropped") + } + assigned, ok := ipOut.AssignedObject.(*diode.VMInterface) + if !ok || assigned != vmIface { + t.Errorf("IP not re-anchored: got %T %p, want %p", ipOut.AssignedObject, ipOut.AssignedObject, vmIface) + } +} + +func TestTransformToVirtualMachine_IPWithDistinctPointerSameNameEmitsTwoVMInterfaces(t *testing.T) { + // When the IP's AssignedObject is a *Interface that's a different + // pointer than the top-level *Interface with the same Name, Phase + // 1's IP-assigned-iface harvest adds BOTH source pointers to + // ifaceMap as distinct entries. Identity lookup in Phase 3 resolves + // the IP to the harvested VMInterface (not the top-level one). + dev := &diode.Device{Name: sp("vyos-edge-1")} + topIface := &diode.Interface{Device: dev, Name: sp("eth0")} + differentPointerSameName := &diode.Interface{Device: dev, Name: sp("eth0")} + ip := &diode.IPAddress{Address: sp("192.0.2.1/24"), AssignedObject: differentPointerSameName} + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev, topIface, ip}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + vmIfsByName := map[string][]*diode.VMInterface{} + var ipOut *diode.IPAddress + for _, e := range out { + switch v := e.(type) { + case *diode.VMInterface: + if v.Name != nil { + vmIfsByName[*v.Name] = append(vmIfsByName[*v.Name], v) + } + case *diode.IPAddress: + ipOut = v + } + } + if len(vmIfsByName["eth0"]) != 2 { + t.Fatalf("expected two distinct eth0 VMInterfaces from the two distinct source pointers; got %d", len(vmIfsByName["eth0"])) + } + assigned, ok := ipOut.AssignedObject.(*diode.VMInterface) + if !ok { + t.Fatalf("IP.AssignedObject: got %T, want *VMInterface", ipOut.AssignedObject) + } + // Top-level eth0 lands earlier than the nested-only harvested one. + if assigned == vmIfsByName["eth0"][0] { + t.Errorf("IP resolved via identity to the top-level VMInterface; expected the harvested one (from the distinct source pointer)") + } + if assigned != vmIfsByName["eth0"][1] { + t.Errorf("IP.AssignedObject: got %p, want the harvested (second) VMInterface %p", assigned, vmIfsByName["eth0"][1]) + } +} + +func TestTransformToVirtualMachine_IPWithNonInterfaceAssignmentPassesThrough(t *testing.T) { + dev := &diode.Device{Name: sp("vyos-edge-1")} + fhrp := &diode.FHRPGroup{GroupId: ip64(42)} + ip := &diode.IPAddress{Address: sp("192.0.2.1/24"), AssignedObject: fhrp} + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev, ip}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + var ipOut *diode.IPAddress + for _, e := range out { + if v, ok := e.(*diode.IPAddress); ok { + ipOut = v + } + } + if ipOut == nil { + t.Fatal("IPAddress missing from output") + } + got, ok := ipOut.AssignedObject.(*diode.FHRPGroup) + if !ok { + t.Fatalf("IP.AssignedObject: got %T, want *FHRPGroup (untouched passthrough)", ipOut.AssignedObject) + } + if got != fhrp { + t.Errorf("FHRPGroup pointer should be identity-equal to input; got %p, want %p", got, fhrp) + } +} + +func TestTransformToVirtualMachine_VLANPassthrough(t *testing.T) { + vlan := &diode.VLAN{Vid: ip64(100), Name: sp("users")} + out := mapping.TransformToVirtualMachine( + []diode.Entity{vlan}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + if len(out) != 1 { + t.Fatalf("entity count: got %d, want 1", len(out)) + } + got, ok := out[0].(*diode.VLAN) + if !ok { + t.Fatalf("VLAN passthrough type: got %T", out[0]) + } + if got != vlan { + t.Errorf("VLAN passthrough identity: got %p, want %p", got, vlan) + } +} + +func TestTransformToVirtualMachine_DropsChassisAndVC(t *testing.T) { + dev := &diode.Device{Name: sp("vyos-edge-1")} + vc := &diode.VirtualChassis{Name: sp("vc-1")} + memberDev := &diode.Device{Name: sp("vyos-edge-1-2"), VcPosition: ip64(2), VirtualChassis: vc} + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev, vc, memberDev}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + for _, e := range out { + switch e.(type) { + case *diode.VirtualChassis: + t.Errorf("VirtualChassis should be dropped, got %v", e) + case *diode.Device: + t.Errorf("All *diode.Device should be transformed/dropped, got %v", e) + } + } +} + +func TestTransformToVirtualMachine_ModulesDropped(t *testing.T) { + dev := &diode.Device{Name: sp("vyos-edge-1")} + mod := &diode.Module{Serial: sp("mod-1")} + bay := &diode.ModuleBay{Name: sp("bay-1")} + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev, mod, bay}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + for _, e := range out { + switch e.(type) { + case *diode.Module: + t.Errorf("Module should be dropped: got %v", e) + case *diode.ModuleBay: + t.Errorf("ModuleBay should be dropped: got %v", e) + } + } +} + +func TestTransformToVirtualMachine_ClusterOnlyWhenConfigured(t *testing.T) { + dev := &diode.Device{Name: sp("vyos-edge-1")} + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev}, + &config.Defaults{Type: config.TargetTypeVirtualMachine, Cluster: ""}, + ) + vm := out[0].(*diode.VirtualMachine) + if vm.Cluster != nil { + t.Errorf("VM.Cluster: got %v, want nil when defaults.cluster is empty", vm.Cluster) + } +} + +func TestTransformToVirtualMachine_ClusterTypeSetWhenConfigured(t *testing.T) { + // When defaults.cluster_type is set alongside defaults.cluster, + // the emitted Cluster carries a Type — required by NetBox's + // virtualization.cluster model for on-the-fly cluster creation. + dev := &diode.Device{Name: sp("vyos-edge-1")} + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev}, + &config.Defaults{ + Type: config.TargetTypeVirtualMachine, + Cluster: "proxmox-a", + ClusterType: "Proxmox", + }, + ) + vm := out[0].(*diode.VirtualMachine) + if vm.Cluster == nil || vm.Cluster.Name == nil || *vm.Cluster.Name != "proxmox-a" { + t.Fatalf("VM.Cluster: got %v, want Name=proxmox-a", vm.Cluster) + } + if vm.Cluster.Type == nil || vm.Cluster.Type.Name == nil || *vm.Cluster.Type.Name != "Proxmox" { + t.Errorf("VM.Cluster.Type: got %v, want Name=Proxmox (required by NetBox for cluster auto-create)", vm.Cluster.Type) + } +} + +func TestTransformToVirtualMachine_ClusterTypeOmittedWhenUnset(t *testing.T) { + // defaults.cluster set, defaults.cluster_type unset: Cluster is + // emitted as reference-only (no Type). This relies on the cluster + // already existing in NetBox — the Diode reconciler resolves by + // name without needing a Type for an existing cluster. + dev := &diode.Device{Name: sp("vyos-edge-1")} + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev}, + &config.Defaults{ + Type: config.TargetTypeVirtualMachine, + Cluster: "pre-existing-cluster", + }, + ) + vm := out[0].(*diode.VirtualMachine) + if vm.Cluster == nil || vm.Cluster.Name == nil || *vm.Cluster.Name != "pre-existing-cluster" { + t.Fatalf("VM.Cluster: got %v, want Name=pre-existing-cluster", vm.Cluster) + } + if vm.Cluster.Type != nil { + t.Errorf("VM.Cluster.Type: got %v, want nil when defaults.cluster_type is empty", vm.Cluster.Type) + } +} + +func TestTransformToVirtualMachine_PrimaryIPRebuilt(t *testing.T) { + cluster := &diode.Cluster{Name: sp("vyos-cluster")} + dev := &diode.Device{Name: sp("vyos-edge-1"), Cluster: cluster} + primaryIface := &diode.Interface{Device: dev, Name: sp("eth0")} + primaryIP := &diode.IPAddress{Address: sp("192.0.2.1/24"), AssignedObject: primaryIface} + dev.PrimaryIp4 = primaryIP + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev, primaryIface}, + &config.Defaults{Type: config.TargetTypeVirtualMachine, Cluster: "vyos-cluster"}, + ) + var vm *diode.VirtualMachine + var vmIface *diode.VMInterface + for _, e := range out { + switch v := e.(type) { + case *diode.VirtualMachine: + vm = v + case *diode.VMInterface: + vmIface = v + } + } + if vm == nil || vmIface == nil { + t.Fatalf("missing entity: vm=%p vmIface=%p", vm, vmIface) + } + if vm.PrimaryIp4 == nil { + t.Fatal("VM.PrimaryIp4: got nil, want a rebuilt snapshot") + } + if vm.PrimaryIp4 == primaryIP { + t.Error("VM.PrimaryIp4: got the original Device snapshot pointer; want a fresh copy") + } + rebuiltAssigned, ok := vm.PrimaryIp4.AssignedObject.(*diode.VMInterface) + if !ok { + t.Fatalf("VM.PrimaryIp4.AssignedObject: got %T, want *diode.VMInterface", vm.PrimaryIp4.AssignedObject) + } + if rebuiltAssigned == vmIface { + t.Errorf("VM.PrimaryIp4.AssignedObject: aliased the rich top-level VMInterface, which would form a VM<->VMInterface cycle") + } + if rebuiltAssigned.Name == nil || *rebuiltAssigned.Name != "eth0" { + t.Errorf("VM.PrimaryIp4.AssignedObject.Name: got %v", rebuiltAssigned.Name) + } + stubVM := rebuiltAssigned.VirtualMachine + if stubVM == nil { + t.Fatal("VM.PrimaryIp4.AssignedObject.VirtualMachine: got nil, want matcher stub") + } + if stubVM == vm { + t.Errorf("VM.PrimaryIp4.AssignedObject.VirtualMachine: aliased the rich VM, cycle still present") + } + if stubVM.Name == nil || *stubVM.Name != "vyos-edge-1" { + t.Errorf("VM.PrimaryIp4.AssignedObject.VirtualMachine.Name: got %v, want vyos-edge-1", stubVM.Name) + } + // The stub must carry NetBox VM-matcher fields (name+cluster) so + // the reconciler resolves to the right VM. Cluster was missing on + // the original name-only stub and broke matching when VM names + // were unique only within a cluster. + if stubVM.Cluster == nil { + t.Fatal("stubVM.Cluster: got nil, want matcher field carried from rich VM") + } + if stubVM.Cluster.Name == nil || *stubVM.Cluster.Name != "vyos-cluster" { + t.Errorf("stubVM.Cluster.Name: got %v, want vyos-cluster", stubVM.Cluster.Name) + } + // Cycle-safety: stubVM.PrimaryIp4 may be a matcher IP, but its + // AssignedObject MUST be nil so the chain terminates one hop + // deeper. newIPMatchStub enforces this. + if stubVM.PrimaryIp4 != nil && stubVM.PrimaryIp4.AssignedObject != nil { + t.Errorf("stubVM.PrimaryIp4.AssignedObject: got %T, want nil (matcher-only IP, cycle must terminate)", stubVM.PrimaryIp4.AssignedObject) + } + if stubVM.PrimaryIp6 != nil && stubVM.PrimaryIp6.AssignedObject != nil { + t.Errorf("stubVM.PrimaryIp6.AssignedObject: got %T, want nil (matcher-only IP, cycle must terminate)", stubVM.PrimaryIp6.AssignedObject) + } +} + +func TestTransformToVirtualMachine_PrimaryIPHarvestedDuringRebuild(t *testing.T) { + dev := &diode.Device{Name: sp("vyos-edge-1")} + nestedOnly := &diode.Interface{Device: dev, Name: sp("nested-only")} + dev.PrimaryIp4 = &diode.IPAddress{Address: sp("192.0.2.1/24"), AssignedObject: nestedOnly} + // nestedOnly NOT in the top-level slice. + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + var vm *diode.VirtualMachine + for _, e := range out { + if v, ok := e.(*diode.VirtualMachine); ok { + vm = v + } + } + if vm == nil || vm.PrimaryIp4 == nil { + t.Fatalf("VM/PrimaryIp4 missing: vm=%p", vm) + } + assigned, ok := vm.PrimaryIp4.AssignedObject.(*diode.VMInterface) + if !ok { + t.Fatalf("VM.PrimaryIp4.AssignedObject: got %T, want *diode.VMInterface (resolved via primary-IP harvest)", vm.PrimaryIp4.AssignedObject) + } + if assigned.Name == nil || *assigned.Name != "nested-only" { + t.Errorf("harvested stub Name: got %v, want nested-only", assigned.Name) + } +} + +func TestTransformToVirtualMachine_PrimaryIPNilAssignmentPreserved(t *testing.T) { + dev := &diode.Device{Name: sp("vyos-edge-1")} + dev.PrimaryIp4 = &diode.IPAddress{ + Address: sp("192.0.2.1/24"), + AssignedObject: nil, + } + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + var vm *diode.VirtualMachine + for _, e := range out { + if v, ok := e.(*diode.VirtualMachine); ok { + vm = v + } + } + if vm == nil || vm.PrimaryIp4 == nil { + t.Fatalf("VM/PrimaryIp4 missing: vm=%p", vm) + } + if vm.PrimaryIp4.AssignedObject != nil { + t.Errorf("VM.PrimaryIp4.AssignedObject: got %T, want nil (passthrough)", vm.PrimaryIp4.AssignedObject) + } +} + +// When source_match is stamped on the source Device BEFORE the +// transform (the runner's pre-transform ordering), it must propagate +// onto the rich VM (via deviceToVM's Metadata carry) AND onto the +// cycle-break VM stub embedded at vm.PrimaryIp4.AssignedObject. +// VirtualMachine. newVMMatchStub captures source_match by value at +// stub-construction time, so any post-transform stamp would miss this +// embedded stub even if it reached the top-level VM. +func TestTransformToVirtualMachine_PrimaryIPStubCarriesSourceMatchWhenStampedPreTransform(t *testing.T) { + dev := &diode.Device{Name: sp("vyos-edge-1")} + primaryIface := &diode.Interface{Device: dev, Name: sp("eth0")} + dev.PrimaryIp4 = &diode.IPAddress{Address: sp("192.0.2.1/24"), AssignedObject: primaryIface} + // Simulate the runner's pre-transform annotateDeviceWithSourceMatch + // step: stamp source_match on the Device's Metadata. deviceToVM + // pointer-shares Metadata so the VM picks it up. + dev.Metadata = diode.Metadata{ + "source_match": diode.Metadata{"netbox_id": 42}, + } + + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev, primaryIface}, + &config.Defaults{Type: config.TargetTypeVirtualMachine, Cluster: "vyos-cluster"}, + ) + + var vm *diode.VirtualMachine + for _, e := range out { + if v, ok := e.(*diode.VirtualMachine); ok { + vm = v + } + } + if vm == nil { + t.Fatal("missing VM in transform output") + } + if sm, ok := vm.Metadata["source_match"]; !ok || sm == nil { + t.Fatalf("vm.Metadata[source_match]: got %v, want propagated from Device", sm) + } + if vm.PrimaryIp4 == nil { + t.Fatal("vm.PrimaryIp4: got nil, want rebuilt snapshot") + } + stubIface, ok := vm.PrimaryIp4.AssignedObject.(*diode.VMInterface) + if !ok { + t.Fatalf("vm.PrimaryIp4.AssignedObject: got %T, want *VMInterface", vm.PrimaryIp4.AssignedObject) + } + if stubIface.VirtualMachine == nil { + t.Fatal("cycle-break VM stub: got nil") + } + stubVM := stubIface.VirtualMachine + sm, ok := stubVM.Metadata["source_match"] + if !ok { + t.Fatal("cycle-break stub.Metadata[source_match]: missing — newVMMatchStub failed to capture it from rich VM") + } + smMap, ok := sm.(diode.Metadata) + if !ok { + t.Fatalf("cycle-break stub.Metadata[source_match]: got %T, want diode.Metadata", sm) + } + if smMap["netbox_id"] != 42 { + t.Errorf("cycle-break stub.Metadata[source_match].netbox_id: got %v, want 42", smMap["netbox_id"]) + } +} + +func TestTransformToVirtualMachine_NoVMCycleAfterTransform(t *testing.T) { + dev := &diode.Device{Name: sp("vyos-edge-1")} + primaryIface := &diode.Interface{Device: dev, Name: sp("eth0")} + dev.PrimaryIp4 = &diode.IPAddress{Address: sp("192.0.2.1/24"), AssignedObject: primaryIface} + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev, primaryIface}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + var vm *diode.VirtualMachine + for _, e := range out { + if v, ok := e.(*diode.VirtualMachine); ok { + vm = v + } + } + if vm == nil || vm.PrimaryIp4 == nil { + t.Fatal("missing VM/PrimaryIp4") + } + assigned := vm.PrimaryIp4.AssignedObject.(*diode.VMInterface) + if assigned.VirtualMachine == vm { + t.Errorf("cycle: VM.PrimaryIp4.AssignedObject.VirtualMachine aliases the rich VM") + } +} + +func TestTransformToVirtualMachine_PrimaryIPStubMACIsMatcherOnly(t *testing.T) { + dev := &diode.Device{Name: sp("vyos-edge-1")} + macAssigned := &diode.Interface{Device: dev, Name: sp("eth0")} + mac := &diode.MACAddress{ + MacAddress: sp("00:11:22:33:44:55"), + AssignedObject: macAssigned, // recursion if not stripped + } + primaryIface := &diode.Interface{Device: dev, Name: sp("eth0"), PrimaryMacAddress: mac} + dev.PrimaryIp4 = &diode.IPAddress{Address: sp("192.0.2.1/24"), AssignedObject: primaryIface} + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev, primaryIface}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + var vm *diode.VirtualMachine + for _, e := range out { + if v, ok := e.(*diode.VirtualMachine); ok { + vm = v + } + } + if vm == nil || vm.PrimaryIp4 == nil { + t.Fatal("missing VM/PrimaryIp4") + } + assigned := vm.PrimaryIp4.AssignedObject.(*diode.VMInterface) + if assigned.PrimaryMacAddress == nil { + t.Fatal("cycle-break stub.PrimaryMacAddress: got nil, want matcher-only stub") + } + if assigned.PrimaryMacAddress == mac { + t.Errorf("cycle-break stub.PrimaryMacAddress aliased the rich MACAddress (would carry AssignedObject)") + } + if assigned.PrimaryMacAddress.AssignedObject != nil { + t.Errorf("cycle-break stub.PrimaryMacAddress.AssignedObject must be nil; got %v", assigned.PrimaryMacAddress.AssignedObject) + } +} + +func TestTransformToVirtualMachine_DeepParentChain(t *testing.T) { + // Depth-2 Parent chain reachable only via nested refs: A.Parent=B, + // B.Parent=C. Only A is in the top-level slice. The harvest must + // walk to fixed-point so both B and C land in the iface map. + dev := &diode.Device{Name: sp("vyos-edge-1")} + c := &diode.Interface{Device: dev, Name: sp("eth-grandparent")} + b := &diode.Interface{Device: dev, Name: sp("eth-parent"), Parent: c} + a := &diode.Interface{Device: dev, Name: sp("eth-child"), Parent: b} + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev, a}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + names := map[string]*diode.VMInterface{} + for _, e := range out { + if v, ok := e.(*diode.VMInterface); ok && v.Name != nil { + names[*v.Name] = v + } + } + for _, want := range []string{"eth-child", "eth-parent", "eth-grandparent"} { + if _, ok := names[want]; !ok { + t.Errorf("expected VMInterface %q to be emitted via fixed-point harvest; got %v", want, names) + } + } + if names["eth-child"].Parent != names["eth-parent"] { + t.Errorf("child.Parent remap: got %p, want %p", names["eth-child"].Parent, names["eth-parent"]) + } + if names["eth-parent"].Parent != names["eth-grandparent"] { + t.Errorf("parent.Parent remap: got %p, want %p", names["eth-parent"].Parent, names["eth-grandparent"]) + } +} + +func TestTransformToVirtualMachine_ParentBridgeRemapped(t *testing.T) { + dev := &diode.Device{Name: sp("vyos-edge-1")} + parent := &diode.Interface{Device: dev, Name: sp("eth0")} + child := &diode.Interface{Device: dev, Name: sp("eth0.100"), Parent: parent} + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev, parent, child}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + var pVM, cVM *diode.VMInterface + for _, e := range out { + if v, ok := e.(*diode.VMInterface); ok { + switch *v.Name { + case "eth0": + pVM = v + case "eth0.100": + cVM = v + } + } + } + if pVM == nil || cVM == nil { + t.Fatalf("missing VMInterfaces: parent=%p child=%p", pVM, cVM) + } + if cVM.Parent != pVM { + t.Errorf("Parent remap: got %p, want %p", cVM.Parent, pVM) + } +} + +func TestTransformToVirtualMachine_NoDeviceVLANOnlyNoOp(t *testing.T) { + vlan := &diode.VLAN{Vid: ip64(100), Name: sp("users")} + out := mapping.TransformToVirtualMachine( + []diode.Entity{vlan}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + if len(out) != 1 || out[0] != vlan { + t.Errorf("VLAN-only input should pass through unchanged: got %v", out) + } +} + +func TestTransformToVirtualMachine_OutputOrder(t *testing.T) { + dev := &diode.Device{Name: sp("vyos-edge-1")} + iface1 := &diode.Interface{Device: dev, Name: sp("eth0")} + iface2 := &diode.Interface{Device: dev, Name: sp("eth1")} + ip := &diode.IPAddress{Address: sp("192.0.2.1/24"), AssignedObject: iface1} + vlan := &diode.VLAN{Vid: ip64(10)} + out := mapping.TransformToVirtualMachine( + []diode.Entity{dev, iface1, iface2, ip, vlan}, + &config.Defaults{Type: config.TargetTypeVirtualMachine}, + ) + // Expected: VM, then VMInterfaces (top-level source order: + // iface1, iface2), then IP, then VLAN. + if len(out) != 5 { + t.Fatalf("entity count: got %d, want 5", len(out)) + } + if _, ok := out[0].(*diode.VirtualMachine); !ok { + t.Errorf("out[0]: got %T, want *VirtualMachine", out[0]) + } + for i := 1; i <= 2; i++ { + if _, ok := out[i].(*diode.VMInterface); !ok { + t.Errorf("out[%d]: got %T, want *VMInterface", i, out[i]) + } + } + if _, ok := out[3].(*diode.IPAddress); !ok { + t.Errorf("out[3]: got %T, want *IPAddress", out[3]) + } + if _, ok := out[4].(*diode.VLAN); !ok { + t.Errorf("out[4]: got %T, want *VLAN", out[4]) + } + vm0 := out[1].(*diode.VMInterface) + vm1 := out[2].(*diode.VMInterface) + if vm0.Name == nil || *vm0.Name != "eth0" { + t.Errorf("out[1].Name: got %v, want eth0", vm0.Name) + } + if vm1.Name == nil || *vm1.Name != "eth1" { + t.Errorf("out[2].Name: got %v, want eth1", vm1.Name) + } +} diff --git a/snmp-discovery/policy/entities_pipeline.go b/snmp-discovery/policy/entities_pipeline.go new file mode 100644 index 00000000..98542311 --- /dev/null +++ b/snmp-discovery/policy/entities_pipeline.go @@ -0,0 +1,85 @@ +package policy + +import ( + "log/slog" + "strings" + + "github.com/netboxlabs/diode-sdk-go/diode" + "github.com/netboxlabs/orb-discovery/snmp-discovery/config" + "github.com/netboxlabs/orb-discovery/snmp-discovery/mapping" +) + +// prepareEntitiesForIngest applies the post-queryTarget pipeline: +// source_match annotation, optional Device→VM transform, run_id +// annotation, the optional debug-log of every entity, and finally +// nested-ref pruning. The Runner method calls this helper rather than +// inlining the four steps so any future reordering shows up here AND +// in TestPrepareEntitiesForIngest_*. Callers that bypass this helper +// (e.g. ad-hoc inlining) lose the protection the test provides. +// +// Ordering invariants — DO NOT REORDER without updating the tests: +// +// 1. annotateDeviceWithSourceMatch FIRST, on the Device-rooted graph. +// The VM transform's deviceToVM does `Metadata: d.Metadata` +// (pointer-share) and rebuildPrimaryIP's newVMMatchStub captures +// Metadata["source_match"] by value at construction time. Stamping +// source_match AFTER the transform would leave the cycle-break +// stub at vm.PrimaryIp4.AssignedObject.VirtualMachine missing +// source_match — breaking the NetBox plugin matcher path under +// target.netbox_id rediscovery. +// +// 2. TransformToVirtualMachine SECOND (only when Type == +// TargetTypeVirtualMachine). Rewrites Device→VM, Interface→ +// VMInterface, etc. +// +// 3. annotateEntitiesWithRunID THIRD, on the post-transform graph so +// the walker stamps run_id on the VM/VMInterface entities the +// ingest actually carries (the source Device is no longer in the +// output entities slice). +// +// 4. logEntities FOURTH (if non-nil), with the rich shared graph +// visible — easier to debug than the post-prune stubbed payload. +// +// 5. PruneNestedRefs / PruneNestedRefsVM LAST. Runs after annotation +// so the annotators can walk the rich shared graph with their +// unsafe.Pointer dedup intact — otherwise every stub would need +// its own metadata pass. +// +// `logger` may be nil; the transform debug message is then skipped. +// `logEntities` may be nil; the per-entity dump is then skipped. +func prepareEntitiesForIngest( + logger *slog.Logger, + entities []diode.Entity, + target config.Target, + def *config.Defaults, + runID, policyName string, + logEntities func([]diode.Entity), +) []diode.Entity { + isVM := def != nil && strings.EqualFold(def.Type, config.TargetTypeVirtualMachine) + + if target.NetboxID != nil { + annotateDeviceWithSourceMatch(entities, *target.NetboxID) + } + + if isVM { + entities = mapping.TransformToVirtualMachine(entities, def) + if logger != nil { + logger.Debug("transformed entities to VirtualMachine graph", + "host", target.Host, "policy", policyName, "entity_count", len(entities)) + } + } + + annotateEntitiesWithRunID(entities, runID) + + if logEntities != nil { + logEntities(entities) + } + + if isVM { + mapping.PruneNestedRefsVM(entities, mapping.CurrentVirtualMachineFrom(entities)) + } else { + mapping.PruneNestedRefs(entities, mapping.CurrentDeviceFrom(entities)) + } + + return entities +} diff --git a/snmp-discovery/policy/entities_pipeline_test.go b/snmp-discovery/policy/entities_pipeline_test.go new file mode 100644 index 00000000..9b4b6a3c --- /dev/null +++ b/snmp-discovery/policy/entities_pipeline_test.go @@ -0,0 +1,185 @@ +package policy + +import ( + "io" + "log/slog" + "testing" + + "github.com/netboxlabs/diode-sdk-go/diode" + "github.com/netboxlabs/orb-discovery/snmp-discovery/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func sp(s string) *string { return &s } + +func discardLogger() *slog.Logger { + return slog.New(slog.NewTextHandler(io.Discard, nil)) +} + +// TestPrepareEntitiesForIngest_SourceMatchReachesCycleBreakStub is the +// runner-level guard for the round-2 BLOCKER fix. Any future refactor +// that moves annotateDeviceWithSourceMatch after the transform inside +// prepareEntitiesForIngest must break this test. +// +// The test invokes prepareEntitiesForIngest directly — same code path +// the Runner method uses — with a VM target that has NetboxID set, and +// asserts source_match lands on: +// - the rich top-level VirtualMachine +// - the cycle-break stub at vm.PrimaryIp4.AssignedObject.VirtualMachine +// +// Both must match because Diode's matcher resolves the cycle-break stub +// independently from the rich VM under target.netbox_id rediscovery. +func TestPrepareEntitiesForIngest_SourceMatchReachesCycleBreakStub(t *testing.T) { + dev := &diode.Device{Name: sp("vyos-edge-1")} + primaryIface := &diode.Interface{Device: dev, Name: sp("eth0")} + dev.PrimaryIp4 = &diode.IPAddress{ + Address: sp("192.0.2.1/24"), + AssignedObject: primaryIface, + } + + netboxID := 42 + target := config.Target{Host: "192.0.2.1", Port: 161, NetboxID: &netboxID} + def := &config.Defaults{ + Type: config.TargetTypeVirtualMachine, + Cluster: "vyos-cluster", + } + + out := prepareEntitiesForIngest( + discardLogger(), + []diode.Entity{dev, primaryIface}, + target, def, "run-id-1", "policy-1", + nil, + ) + + var vm *diode.VirtualMachine + for _, e := range out { + if v, ok := e.(*diode.VirtualMachine); ok { + vm = v + } + } + require.NotNil(t, vm, "missing VirtualMachine in pipeline output") + + // Rich VM has source_match. + richSM, ok := vm.Metadata["source_match"].(diode.Metadata) + require.True(t, ok, "rich VM.Metadata[source_match]: got %T, want diode.Metadata", vm.Metadata["source_match"]) + assert.Equal(t, netboxID, richSM["netbox_id"]) + + // Cycle-break stub at vm.PrimaryIp4.AssignedObject.VirtualMachine + // also has source_match — the runner-ordering invariant. + require.NotNil(t, vm.PrimaryIp4, "vm.PrimaryIp4 missing") + stubIface, ok := vm.PrimaryIp4.AssignedObject.(*diode.VMInterface) + require.True(t, ok, "vm.PrimaryIp4.AssignedObject: got %T, want *VMInterface", vm.PrimaryIp4.AssignedObject) + require.NotNil(t, stubIface.VirtualMachine, "cycle-break stub missing VirtualMachine ref") + stubSM, ok := stubIface.VirtualMachine.Metadata["source_match"].(diode.Metadata) + require.True(t, ok, + "cycle-break stub.Metadata[source_match]: missing — runner ordering broken "+ + "(annotateDeviceWithSourceMatch must run BEFORE TransformToVirtualMachine)") + assert.Equal(t, netboxID, stubSM["netbox_id"]) +} + +// TestPrepareEntitiesForIngest_NoNetboxID_NoSourceMatch confirms the +// helper is a no-op for source_match when NetboxID is unset. +func TestPrepareEntitiesForIngest_NoNetboxID_NoSourceMatch(t *testing.T) { + dev := &diode.Device{Name: sp("vyos-edge-1")} + target := config.Target{Host: "192.0.2.1", Port: 161} // NetboxID nil + def := &config.Defaults{Type: config.TargetTypeVirtualMachine, Cluster: "c"} + + out := prepareEntitiesForIngest(discardLogger(), []diode.Entity{dev}, target, def, "run-1", "policy-1", nil) + + for _, e := range out { + if vm, ok := e.(*diode.VirtualMachine); ok { + _, hasSM := vm.Metadata["source_match"] + assert.False(t, hasSM, "VM must NOT carry source_match when target.NetboxID is nil") + } + } +} + +// TestPrepareEntitiesForIngest_DeviceTargetPathUnchanged checks that +// the Type-unset (Device) path still produces a Device-rooted graph +// with source_match stamped and PruneNestedRefs (not PruneNestedRefsVM) +// invoked. Catches a refactor that accidentally crossed the wires. +func TestPrepareEntitiesForIngest_DeviceTargetPathUnchanged(t *testing.T) { + dev := &diode.Device{Name: sp("router-1")} + netboxID := 7 + target := config.Target{Host: "10.0.0.1", Port: 161, NetboxID: &netboxID} + def := &config.Defaults{} // Type unset → Device target + + out := prepareEntitiesForIngest(discardLogger(), []diode.Entity{dev}, target, def, "run-1", "p", nil) + + // Top-level entity is still a Device (no VM transform fired). + var richDev *diode.Device + for _, e := range out { + if d, ok := e.(*diode.Device); ok { + richDev = d + } + _, isVM := e.(*diode.VirtualMachine) + assert.False(t, isVM, "Device-target path must not emit *VirtualMachine") + } + require.NotNil(t, richDev, "expected a top-level Device in output") + sm, ok := richDev.Metadata["source_match"].(diode.Metadata) + require.True(t, ok, "Device.Metadata[source_match]: got %T", richDev.Metadata["source_match"]) + assert.Equal(t, netboxID, sm["netbox_id"]) +} + +// TestPrepareEntitiesForIngest_RunIDStampedOnPostTransformEntities +// confirms run_id reaches the post-transform VM/VMInterface entities. +// If run_id annotation accidentally ran BEFORE the transform, it would +// land on the source Device (dropped from output) and the VM would +// have nil Metadata for run_id. +func TestPrepareEntitiesForIngest_RunIDStampedOnPostTransformEntities(t *testing.T) { + dev := &diode.Device{Name: sp("v1")} + iface := &diode.Interface{Device: dev, Name: sp("eth0")} + def := &config.Defaults{Type: config.TargetTypeVirtualMachine} + + out := prepareEntitiesForIngest(discardLogger(), []diode.Entity{dev, iface}, config.Target{}, def, "run-xyz", "p", nil) + + var sawVM, sawVMIface bool + for _, e := range out { + switch v := e.(type) { + case *diode.VirtualMachine: + sawVM = true + assert.Equal(t, "run-xyz", v.Metadata["run_id"], "VM must carry run_id stamped post-transform") + case *diode.VMInterface: + sawVMIface = true + assert.Equal(t, "run-xyz", v.Metadata["run_id"], "VMInterface must carry run_id stamped post-transform") + } + } + assert.True(t, sawVM, "expected VM in output") + assert.True(t, sawVMIface, "expected VMInterface in output") +} + +// TestPrepareEntitiesForIngest_LogEntitiesCalledBeforePrune asserts the +// logEntities callback fires while nested refs are still rich (a +// reordering that moves it after prune would defeat its debug purpose). +// Assertions run INSIDE the callback because the entities slice is a +// slice of pointers — any post-callback mutation by prune would taint +// a snapshot taken via slice copy. +func TestPrepareEntitiesForIngest_LogEntitiesCalledBeforePrune(t *testing.T) { + dev := &diode.Device{Name: sp("v1")} + iface := &diode.Interface{Device: dev, Name: sp("eth0")} + + called := false + logEntities := func(es []diode.Entity) { + called = true + var richVM *diode.VirtualMachine + for _, e := range es { + if vm, ok := e.(*diode.VirtualMachine); ok { + richVM = vm + } + } + require.NotNil(t, richVM, "expected a top-level VM in the logged entities") + for _, e := range es { + if v, ok := e.(*diode.VMInterface); ok { + assert.Same(t, richVM, v.VirtualMachine, + "VMInterface.VirtualMachine must be the rich VM at log time "+ + "(logEntities must run BEFORE PruneNestedRefsVM rewrites the ref to a stub)") + } + } + } + + def := &config.Defaults{Type: config.TargetTypeVirtualMachine} + prepareEntitiesForIngest(discardLogger(), []diode.Entity{dev, iface}, config.Target{}, def, "r", "p", logEntities) + + require.True(t, called, "logEntities callback must fire") +} diff --git a/snmp-discovery/policy/entity_run_metadata.go b/snmp-discovery/policy/entity_run_metadata.go index bd2d48c6..867b230b 100644 --- a/snmp-discovery/policy/entity_run_metadata.go +++ b/snmp-discovery/policy/entity_run_metadata.go @@ -39,6 +39,12 @@ func annotateDeviceWithSourceMatch(entities []diode.Entity, netboxID int) { if v != nil { setDeviceSourceMatch(v.Master, netboxID, seen) } + case *diode.VirtualMachine: + // VM-target path (TransformToVirtualMachine output): stamp + // netbox_id source_match on the top-level VirtualMachine so + // target.NetboxID-based re-discovery resolves the existing + // NetBox VM row instead of creating a new one. + setVMSourceMatch(v, netboxID, seen) } } } @@ -65,6 +71,26 @@ func setDeviceSourceMatch(d *diode.Device, netboxID int, seen map[unsafe.Pointer d.Metadata["source_match"] = diode.Metadata{"netbox_id": netboxID} } +// setVMSourceMatch mirrors setDeviceSourceMatch for the VM-target +// path. The top-level entity emitted by TransformToVirtualMachine is +// a *diode.VirtualMachine; this helper stamps the same source_match +// shape so Diode's resolver finds the existing NetBox row on +// re-discovery. +func setVMSourceMatch(vm *diode.VirtualMachine, netboxID int, seen map[unsafe.Pointer]struct{}) { + if vm == nil { + return + } + p := unsafe.Pointer(vm) + if _, ok := seen[p]; ok { + return + } + seen[p] = struct{}{} + if vm.Metadata == nil { + vm.Metadata = make(diode.Metadata) + } + vm.Metadata["source_match"] = diode.Metadata{"netbox_id": netboxID} +} + // annotateEntitiesWithRunID sets per-entity Diode metadata key "run_id" on each entity // in the batch and on nested Device, Interface, IPAddress, and VLAN references. func annotateEntitiesWithRunID(entities []diode.Entity, runID string) { @@ -81,6 +107,10 @@ func annotateEntitiesWithRunID(entities []diode.Entity, runID string) { annotateVLAN(v, runID, seen) case *diode.VirtualChassis: annotateVirtualChassis(v, runID, seen) + case *diode.VirtualMachine: + annotateVirtualMachine(v, runID, seen) + case *diode.VMInterface: + annotateVMInterface(v, runID, seen) } } } @@ -144,7 +174,10 @@ func annotateIPAddress(ip *diode.IPAddress, runID string, seen map[unsafe.Pointe case *diode.FHRPGroup: mergeRunID(&a.Metadata, runID) case *diode.VMInterface: - mergeRunID(&a.Metadata, runID) + // VM-target path: walk fully so the nested VirtualMachine / + // Parent / Bridge / VLAN refs reached via an IP-assigned + // VMInterface also get a run_id stamp. + annotateVMInterface(a, runID, seen) } } if ip.NatInside != nil { @@ -184,3 +217,45 @@ func annotateVLAN(vlan *diode.VLAN, runID string, seen map[unsafe.Pointer]struct seen[p] = struct{}{} mergeRunID(&vlan.Metadata, runID) } + +// annotateVirtualMachine stamps run_id on the VM and recurses into +// nested refs that carry their own Metadata (PrimaryIp4/6 — which can +// reach an IP-assigned VMInterface via annotateIPAddress, which is +// fine because the walker dedup-set breaks the cycle). +func annotateVirtualMachine(vm *diode.VirtualMachine, runID string, seen map[unsafe.Pointer]struct{}) { + if vm == nil { + return + } + p := unsafe.Pointer(vm) + if _, ok := seen[p]; ok { + return + } + seen[p] = struct{}{} + mergeRunID(&vm.Metadata, runID) + annotateIPAddress(vm.PrimaryIp4, runID, seen) + annotateIPAddress(vm.PrimaryIp6, runID, seen) +} + +// annotateVMInterface stamps run_id on the VMInterface and recurses +// into nested refs (VirtualMachine, Parent, Bridge, UntaggedVlan, +// QinqSvlan, TaggedVlans). Mirrors annotateInterface for the +// Device-rooted graph. +func annotateVMInterface(vmIface *diode.VMInterface, runID string, seen map[unsafe.Pointer]struct{}) { + if vmIface == nil { + return + } + p := unsafe.Pointer(vmIface) + if _, ok := seen[p]; ok { + return + } + seen[p] = struct{}{} + mergeRunID(&vmIface.Metadata, runID) + annotateVirtualMachine(vmIface.VirtualMachine, runID, seen) + annotateVMInterface(vmIface.Parent, runID, seen) + annotateVMInterface(vmIface.Bridge, runID, seen) + annotateVLAN(vmIface.UntaggedVlan, runID, seen) + annotateVLAN(vmIface.QinqSvlan, runID, seen) + for _, vlan := range vmIface.TaggedVlans { + annotateVLAN(vlan, runID, seen) + } +} diff --git a/snmp-discovery/policy/entity_run_metadata_test.go b/snmp-discovery/policy/entity_run_metadata_test.go index e70d0220..3f9a5fa4 100644 --- a/snmp-discovery/policy/entity_run_metadata_test.go +++ b/snmp-discovery/policy/entity_run_metadata_test.go @@ -4,6 +4,8 @@ import ( "testing" "github.com/netboxlabs/diode-sdk-go/diode" + "github.com/netboxlabs/orb-discovery/snmp-discovery/config" + "github.com/netboxlabs/orb-discovery/snmp-discovery/mapping" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -258,6 +260,80 @@ func TestAnnotateEntitiesWithRunID_StampsVirtualChassis(t *testing.T) { assert.Equal(t, "run-123", vc.Metadata["run_id"]) } +func TestAnnotateDeviceWithSourceMatch_VirtualMachine(t *testing.T) { + vm := &diode.VirtualMachine{Name: stringPtr("vyos-edge-1")} + annotateDeviceWithSourceMatch([]diode.Entity{vm}, 42) + require.NotNil(t, vm.Metadata, "expected Metadata to be populated by source_match annotation") + sm, ok := vm.Metadata["source_match"].(diode.Metadata) + require.True(t, ok, "source_match: got %T, want diode.Metadata", vm.Metadata["source_match"]) + assert.Equal(t, 42, sm["netbox_id"]) +} + +func TestAnnotateEntitiesWithRunID_VirtualMachineTopLevel(t *testing.T) { + vm := &diode.VirtualMachine{Name: stringPtr("vyos-edge-1")} + annotateEntitiesWithRunID([]diode.Entity{vm}, "run-xyz") + require.NotNil(t, vm.Metadata, "expected Metadata on top-level VM") + assert.Equal(t, "run-xyz", vm.Metadata["run_id"]) +} + +func TestAnnotateEntitiesWithRunID_VMInterfaceTopLevel(t *testing.T) { + // A VMInterface with NO attached IP must still get a run_id stamp, + // and the nested VM ref must also get stamped (one annotator pass + // reaches both via the new walker). + vm := &diode.VirtualMachine{Name: stringPtr("vyos-edge-1")} + vmIface := &diode.VMInterface{Name: stringPtr("eth0"), VirtualMachine: vm} + annotateEntitiesWithRunID([]diode.Entity{vmIface}, "run-xyz") + require.NotNil(t, vmIface.Metadata, "top-level VMInterface must get Metadata") + assert.Equal(t, "run-xyz", vmIface.Metadata["run_id"]) + require.NotNil(t, vm.Metadata, "nested VM via VMInterface.VirtualMachine must get Metadata") + assert.Equal(t, "run-xyz", vm.Metadata["run_id"]) +} + +// Runner-level ordering invariant: when target.NetboxID is set on a VM +// target, source_match must end up on every reference NetBox uses to +// resolve the VM — including the cycle-break stub embedded at +// vm.PrimaryIp4.AssignedObject.VirtualMachine. Mirrors the runner's +// production sequence: annotateDeviceWithSourceMatch BEFORE +// TransformToVirtualMachine. +func TestVMTargetSourceMatchPropagatesThroughTransform(t *testing.T) { + dev := &diode.Device{Name: stringPtr("vyos-edge-1")} + primaryIface := &diode.Interface{Device: dev, Name: stringPtr("eth0")} + dev.PrimaryIp4 = &diode.IPAddress{Address: stringPtr("192.0.2.1/24"), AssignedObject: primaryIface} + + // Production sequence (runner.go): source_match FIRST, then transform. + entities := []diode.Entity{dev, primaryIface} + annotateDeviceWithSourceMatch(entities, 42) + entities = mapping.TransformToVirtualMachine( + entities, + &config.Defaults{Type: config.TargetTypeVirtualMachine, Cluster: "vyos-cluster"}, + ) + + var vm *diode.VirtualMachine + for _, e := range entities { + if v, ok := e.(*diode.VirtualMachine); ok { + vm = v + } + } + require.NotNil(t, vm, "missing VM after transform") + + // Top-level VM has source_match (carried from Device via Metadata pointer share). + sm, ok := vm.Metadata["source_match"].(diode.Metadata) + require.True(t, ok, "vm.Metadata[source_match]: got %T, want diode.Metadata", vm.Metadata["source_match"]) + assert.Equal(t, 42, sm["netbox_id"]) + + // Cycle-break stub at vm.PrimaryIp4.AssignedObject.VirtualMachine + // carries the same source_match (newVMMatchStub captures it at + // construction time — would be missing if annotation ran AFTER the + // transform). + require.NotNil(t, vm.PrimaryIp4, "vm.PrimaryIp4: got nil, want rebuilt snapshot") + stubIface, ok := vm.PrimaryIp4.AssignedObject.(*diode.VMInterface) + require.True(t, ok, "vm.PrimaryIp4.AssignedObject: got %T, want *VMInterface", vm.PrimaryIp4.AssignedObject) + require.NotNil(t, stubIface.VirtualMachine, "cycle-break stub: missing VirtualMachine ref") + stubSM, ok := stubIface.VirtualMachine.Metadata["source_match"].(diode.Metadata) + require.True(t, ok, "cycle-break stub.Metadata[source_match]: missing — newVMMatchStub did not capture it from rich VM") + assert.Equal(t, 42, stubSM["netbox_id"]) +} + // Helper function for tests func int64Ptr(v int64) *int64 { return &v diff --git a/snmp-discovery/policy/manager.go b/snmp-discovery/policy/manager.go index af159bf6..2cd11e7d 100644 --- a/snmp-discovery/policy/manager.go +++ b/snmp-discovery/policy/manager.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "log/slog" + "strings" "time" "github.com/netboxlabs/diode-sdk-go/diode" @@ -120,6 +121,20 @@ func (m *Manager) applyDefaults(policy *config.Policy) { policy.Config.Defaults.Site = "undefined" } + // Normalize defaults.Type and any override Type to lowercase so + // the runner can compare against the TargetTypeDevice / + // TargetTypeVirtualMachine constants without case-sensitivity. + // Empty resolves to Device for backward compatibility. + policy.Config.Defaults.Type = strings.ToLower(strings.TrimSpace(policy.Config.Defaults.Type)) + if policy.Config.Defaults.Type == "" { + policy.Config.Defaults.Type = config.TargetTypeDevice + } + for i := range policy.Scope.Targets { + if policy.Scope.Targets[i].OverrideDefaults != nil { + policy.Scope.Targets[i].OverrideDefaults.Type = strings.ToLower(strings.TrimSpace(policy.Scope.Targets[i].OverrideDefaults.Type)) + } + } + if policy.Config.Options.CreateUnknownVlans == nil { trueVal := true policy.Config.Options.CreateUnknownVlans = &trueVal @@ -204,6 +219,35 @@ func (m *Manager) validatePolicy(policy config.Policy) error { } } + // Reject unknown defaults.type values at parse time so operators + // get an immediate error rather than silently degrading at scan + // time. Comparison is case-insensitive (applyDefaults runs AFTER + // validation, so the raw user input is what we see here). + rawType := strings.ToLower(strings.TrimSpace(policy.Config.Defaults.Type)) + if rawType != "" && + rawType != config.TargetTypeDevice && + rawType != config.TargetTypeVirtualMachine { + return fmt.Errorf( + "invalid defaults.type %q (allowed: %s, %s)", + policy.Config.Defaults.Type, + config.TargetTypeDevice, config.TargetTypeVirtualMachine, + ) + } + for _, target := range policy.Scope.Targets { + if target.OverrideDefaults == nil || target.OverrideDefaults.Type == "" { + continue + } + rawOverride := strings.ToLower(strings.TrimSpace(target.OverrideDefaults.Type)) + if rawOverride != config.TargetTypeDevice && + rawOverride != config.TargetTypeVirtualMachine { + return fmt.Errorf( + "target %s: invalid override_defaults.type %q (allowed: %s, %s)", + target.Host, target.OverrideDefaults.Type, + config.TargetTypeDevice, config.TargetTypeVirtualMachine, + ) + } + } + // Reject unknown enum values at parse time so operators get an // immediate error rather than silently degrading at scan time. if policy.Config.Options.DiscoverModules != nil { diff --git a/snmp-discovery/policy/manager_test.go b/snmp-discovery/policy/manager_test.go index 7963d457..2abe1e0a 100644 --- a/snmp-discovery/policy/manager_test.go +++ b/snmp-discovery/policy/manager_test.go @@ -1352,6 +1352,101 @@ func TestManagerApplyDefaults_CreateUnknownVlans(t *testing.T) { }) } +func TestManager_ParsePolicies_RejectsInvalidType(t *testing.T) { + logger := slog.New(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug, AddSource: false})) + manager, err := policy.NewManager(context.Background(), logger, nil, nil) + require.NoError(t, err) + + raw := []byte(` +policies: + bad-type: + config: + defaults: + type: KitchenSink + scope: + targets: + - host: 192.0.2.1 + authentication: + protocol_version: SNMPv2c + community: public +`) + _, err = manager.ParsePolicies(raw) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid defaults.type") + assert.Contains(t, err.Error(), "KitchenSink") +} + +func TestManager_ParsePolicies_TypeCaseInsensitiveNormalizedLowercase(t *testing.T) { + logger := slog.New(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug, AddSource: false})) + manager, err := policy.NewManager(context.Background(), logger, nil, nil) + require.NoError(t, err) + + raw := []byte(` +policies: + ok: + config: + defaults: + type: VirtualMachine + scope: + targets: + - host: 192.0.2.1 + authentication: + protocol_version: SNMPv2c + community: public +`) + policies, err := manager.ParsePolicies(raw) + require.NoError(t, err) + assert.Equal(t, config.TargetTypeVirtualMachine, policies["ok"].Config.Defaults.Type) +} + +func TestManager_ParsePolicies_TypeEmptyDefaultsToDevice(t *testing.T) { + logger := slog.New(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug, AddSource: false})) + manager, err := policy.NewManager(context.Background(), logger, nil, nil) + require.NoError(t, err) + + raw := []byte(` +policies: + ok: + config: {} + scope: + targets: + - host: 192.0.2.1 + authentication: + protocol_version: SNMPv2c + community: public +`) + policies, err := manager.ParsePolicies(raw) + require.NoError(t, err) + assert.Equal(t, config.TargetTypeDevice, policies["ok"].Config.Defaults.Type) +} + +func TestManager_ParsePolicies_RejectsInvalidOverrideType(t *testing.T) { + logger := slog.New(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug, AddSource: false})) + manager, err := policy.NewManager(context.Background(), logger, nil, nil) + require.NoError(t, err) + + raw := []byte(` +policies: + bad-override: + config: + defaults: + type: VirtualMachine + scope: + authentication: + protocol_version: SNMPv2c + community: public + targets: + - host: 192.0.2.1 + override_defaults: + type: BadOverride +`) + _, err = manager.ParsePolicies(raw) + require.Error(t, err) + assert.Contains(t, err.Error(), "target 192.0.2.1") + assert.Contains(t, err.Error(), "override_defaults.type") + assert.Contains(t, err.Error(), "BadOverride") +} + func TestManager_ParsePolicies_RejectsInvalidDiscoverModules(t *testing.T) { logger := slog.New(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug, AddSource: false})) manager, err := policy.NewManager(context.Background(), logger, nil, nil) diff --git a/snmp-discovery/policy/runner.go b/snmp-discovery/policy/runner.go index 4c901de3..a0f91d16 100644 --- a/snmp-discovery/policy/runner.go +++ b/snmp-discovery/policy/runner.go @@ -371,17 +371,11 @@ func (r *Runner) runWithMetadata(target config.Target, parentTarget string) { return } - if target.NetboxID != nil { - annotateDeviceWithSourceMatch(entities, *target.NetboxID) - } - annotateEntitiesWithRunID(entities, run.ID) - r.logEntitiesForIngestion(entities) - - // Strip nested Device/Interface refs to matcher-only stubs to shrink - // the wire payload. Runs after annotation so the annotators can walk - // the rich shared graph with their unsafe.Pointer dedup intact — - // otherwise every stub would need its own metadata pass. - mapping.PruneNestedRefs(entities, mapping.CurrentDeviceFrom(entities)) + def := r.resolveTargetDefaults(target) + entities = prepareEntitiesForIngest( + r.logger, entities, target, def, run.ID, policyName, + r.logEntitiesForIngestion, + ) resp, err := r.client.Ingest(r.ctx, entities, diode.WithIngestMetadata(diode.Metadata{ "policy_name": policyName,