From 2b9bbbcf516164b99090df9f3c68b5dfb78bdba1 Mon Sep 17 00:00:00 2001 From: Michal Fiedorowicz Date: Wed, 22 May 2024 11:07:17 +0100 Subject: [PATCH 1/4] fix: OBS-441 - diode-sdk-python: populate manufacturer from root for nested messages if empty Signed-off-by: Michal Fiedorowicz --- .../netboxlabs/diode/sdk/ingester.py | 42 ++++++++++++ diode-sdk-python/tests/test_wrappers.py | 68 +++++++++++++++++++ 2 files changed, 110 insertions(+) diff --git a/diode-sdk-python/netboxlabs/diode/sdk/ingester.py b/diode-sdk-python/netboxlabs/diode/sdk/ingester.py index af97b148..503f94ef 100644 --- a/diode-sdk-python/netboxlabs/diode/sdk/ingester.py +++ b/diode-sdk-python/netboxlabs/diode/sdk/ingester.py @@ -175,12 +175,26 @@ def __new__( if isinstance(platform, str): platform = PlatformPb(name=platform, manufacturer=manufacturer) + if ( + isinstance(platform, PlatformPb) + and not platform.HasField("manufacturer") + and manufacturer is not None + ): + platform.manufacturer.CopyFrom(manufacturer) + if isinstance(site, str): site = SitePb(name=site) if isinstance(device_type, str): device_type = DeviceTypePb(model=device_type, manufacturer=manufacturer) + if ( + isinstance(device_type, DeviceTypePb) + and not device_type.HasField("manufacturer") + and manufacturer is not None + ): + device_type.manufacturer.CopyFrom(manufacturer) + if isinstance(role, str): role = RolePb(name=role) @@ -242,12 +256,26 @@ def __new__( if isinstance(platform, str): platform = PlatformPb(name=platform, manufacturer=manufacturer) + if ( + isinstance(platform, PlatformPb) + and not platform.HasField("manufacturer") + and manufacturer is not None + ): + platform.manufacturer.CopyFrom(manufacturer) + if isinstance(site, str): site = SitePb(name=site) if isinstance(device_type, str): device_type = DeviceTypePb(model=device_type, manufacturer=manufacturer) + if ( + isinstance(device_type, DeviceTypePb) + and not device_type.HasField("manufacturer") + and manufacturer is not None + ): + device_type.manufacturer.CopyFrom(manufacturer) + if isinstance(role, str): role = RolePb(name=role) @@ -307,12 +335,26 @@ def __new__( if isinstance(platform, str): platform = PlatformPb(name=platform, manufacturer=manufacturer) + if ( + isinstance(platform, PlatformPb) + and not platform.HasField("manufacturer") + and manufacturer is not None + ): + platform.manufacturer.CopyFrom(manufacturer) + if isinstance(site, str): site = SitePb(name=site) if isinstance(device_type, str): device_type = DeviceTypePb(model=device_type, manufacturer=manufacturer) + if ( + isinstance(device_type, DeviceTypePb) + and not device_type.HasField("manufacturer") + and manufacturer is not None + ): + device_type.manufacturer.CopyFrom(manufacturer) + if isinstance(device_role, str): device_role = RolePb(name=device_role) diff --git a/diode-sdk-python/tests/test_wrappers.py b/diode-sdk-python/tests/test_wrappers.py index 106e4720..dda171ce 100644 --- a/diode-sdk-python/tests/test_wrappers.py +++ b/diode-sdk-python/tests/test_wrappers.py @@ -214,6 +214,24 @@ def test_device_wrapper(): assert device.asset_tag == "123456" assert device.status == "active" + device = Device( + name="Device ABC", + device_type=DeviceType(model="Device Type ABC"), + platform=Platform(name="Platform ABC"), + manufacturer="Cisco", + ) + assert device.device_type.manufacturer.name == "Cisco" + assert device.platform.manufacturer.name == "Cisco" + + device = Device( + name="Device ABC", + device_type=DeviceType(model="Device Type ABC", manufacturer="Manufacturer A"), + platform=Platform(name="Platform ABC", manufacturer="Manufacturer A"), + manufacturer="Cisco", + ) + assert device.device_type.manufacturer.name == "Manufacturer A" + assert device.platform.manufacturer.name == "Manufacturer A" + def test_interface_wrapper(): """Ensure Interface wrapper instantiates InterfacePb.""" @@ -254,6 +272,30 @@ def test_interface_wrapper(): assert interface.mac_address == "00:00:00:00:00:00" assert interface.description == "Description ABC" + interface = Interface( + name="Interface ABC", + device="Device ABC", + device_type="Device Type ABC", + role="Role ABC", + platform="Platform ABC", + site="Site ABC", + manufacturer="Cisco", + ) + assert interface.device.device_type.manufacturer.name == "Cisco" + assert interface.device.platform.manufacturer.name == "Cisco" + + interface = Interface( + name="Interface ABC", + device="Device ABC", + device_type=DeviceType(model="Device Type ABC", manufacturer="Manufacturer A"), + role="Role ABC", + platform=Platform(name="Platform ABC", manufacturer="Manufacturer A"), + site="Site ABC", + manufacturer="Cisco", + ) + assert interface.device.device_type.manufacturer.name == "Manufacturer A" + assert interface.device.platform.manufacturer.name == "Manufacturer A" + def test_ip_address_wrapper(): """Ensure IPAddress wrapper instantiates IPAddressPb.""" @@ -301,6 +343,32 @@ def test_ip_address_wrapper(): assert isinstance(ip_address.interface.device.platform.manufacturer, ManufacturerPb) assert ip_address.interface.device.platform.manufacturer.name == "Cisco" + ip_address = IPAddress( + address="192.168.0.1/24", + interface="Interface ABC", + device="Device ABC", + device_type="Device Type ABC", + device_role="Role ABC", + platform="Platform ABC", + manufacturer="Cisco", + site="Site ABC", + ) + assert ip_address.interface.device.device_type.manufacturer.name == "Cisco" + assert ip_address.interface.device.platform.manufacturer.name == "Cisco" + + ip_address = IPAddress( + address="192.168.0.1/24", + interface="Interface ABC", + device="Device ABC", + device_type=DeviceType(model="Device Type ABC", manufacturer="Manufacturer A"), + device_role="Role ABC", + platform=Platform(name="Platform ABC", manufacturer="Manufacturer A"), + manufacturer="Cisco", + site="Site ABC", + ) + assert ip_address.interface.device.device_type.manufacturer.name == "Manufacturer A" + assert ip_address.interface.device.platform.manufacturer.name == "Manufacturer A" + def test_prefix_wrapper(): """Ensure Prefix wrapper instantiates PrefixPb.""" From 242c3b876adccc5e7ddc29bd9b744c4aae8ab790 Mon Sep 17 00:00:00 2001 From: Michal Fiedorowicz Date: Wed, 22 May 2024 12:10:23 +0100 Subject: [PATCH 2/4] fix: OBS-441 - diode-server: recognize existing manufacturer in device and device type Signed-off-by: Michal Fiedorowicz --- diode-server/netbox/wrappers.go | 61 +---- .../reconciler/changeset/changeset_test.go | 214 +++++++++++++++--- 2 files changed, 198 insertions(+), 77 deletions(-) diff --git a/diode-server/netbox/wrappers.go b/diode-server/netbox/wrappers.go index c3754a16..2ed7b55d 100644 --- a/diode-server/netbox/wrappers.go +++ b/diode-server/netbox/wrappers.go @@ -132,8 +132,6 @@ func (dw *DcimDeviceDataWrapper) NestedObjects() ([]ComparableData, error) { objects = append(objects, dto...) - objects = dedupManufacturers(objects) - dw.Device.DeviceType = deviceType.DeviceType deviceRole := DcimDeviceRoleDataWrapper{DeviceRole: dw.Device.Role, placeholder: dw.placeholder, hasParent: true, intended: dw.intended} @@ -163,53 +161,6 @@ func (dw *DcimDeviceDataWrapper) NestedObjects() ([]ComparableData, error) { return objects, nil } -func dedupManufacturers(objects []ComparableData) []ComparableData { - type indexedComparableData struct { - index int - data ComparableData - } - - var nonPlaceholderManufacturer *indexedComparableData - - var manufacturerObjects []ComparableData - for i, obj := range objects { - if obj.DataType() == DcimManufacturerObjectType { - manufacturerObjects = append(manufacturerObjects, obj) - if !obj.IsPlaceholder() { - nonPlaceholderManufacturer = &indexedComparableData{index: i, data: obj} - } - } - } - - if len(manufacturerObjects) < 2 { - return objects - } - - var dedupObjects []ComparableData - - for i, obj := range objects { - if obj.DataType() == DcimManufacturerObjectType { - if nonPlaceholderManufacturer != nil { - if i == nonPlaceholderManufacturer.index { - continue - } - - if obj.IsPlaceholder() { - objData := obj.Data().(*DcimManufacturer) - data := nonPlaceholderManufacturer.data - *objData = *data.Data().(*DcimManufacturer) - objects[i] = data - dedupObjects = append(dedupObjects, data) - continue - } - } - } - dedupObjects = append(dedupObjects, obj) - } - - return dedupObjects -} - // DataType returns the data type func (dw *DcimDeviceDataWrapper) DataType() string { return DcimDeviceObjectType @@ -650,9 +601,13 @@ func (dw *DcimDeviceTypeDataWrapper) DataType() string { // ObjectStateQueryParams returns the query parameters needed to retrieve its object state func (dw *DcimDeviceTypeDataWrapper) ObjectStateQueryParams() map[string]string { - return map[string]string{ + params := map[string]string{ "q": dw.DeviceType.Model, } + if dw.DeviceType.Manufacturer != nil { + params["manufacturer__name"] = dw.DeviceType.Manufacturer.Name + } + return params } // ID returns the ID of the data @@ -1375,9 +1330,13 @@ func (dw *DcimPlatformDataWrapper) DataType() string { // ObjectStateQueryParams returns the query parameters needed to retrieve its object state func (dw *DcimPlatformDataWrapper) ObjectStateQueryParams() map[string]string { - return map[string]string{ + params := map[string]string{ "q": dw.Platform.Name, } + if dw.Platform.Manufacturer != nil { + params["manufacturer__name"] = dw.Platform.Manufacturer.Name + } + return params } // ID returns the ID of the data diff --git a/diode-server/reconciler/changeset/changeset_test.go b/diode-server/reconciler/changeset/changeset_test.go index 54e0c8d9..f7a7b482 100644 --- a/diode-server/reconciler/changeset/changeset_test.go +++ b/diode-server/reconciler/changeset/changeset_test.go @@ -536,7 +536,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "ISR4321"}, + queryParams: map[string]string{"q": "ISR4321", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: nil, @@ -605,7 +605,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "ISR4321"}, + queryParams: map[string]string{"q": "ISR4321", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: &netbox.DcimDeviceType{ @@ -675,7 +675,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "ISR4321"}, + queryParams: map[string]string{"q": "ISR4321", "manufacturer__name": "Cisco"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: nil, @@ -823,7 +823,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "ISR4321"}, + queryParams: map[string]string{"q": "ISR4321", "manufacturer__name": "Cisco"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: &netbox.DcimDeviceType{ @@ -1020,7 +1020,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "ISR4321"}, + queryParams: map[string]string{"q": "ISR4321", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: &netbox.DcimDeviceType{ @@ -1162,7 +1162,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "ISR4321"}, + queryParams: map[string]string{"q": "ISR4321", "manufacturer__name": "Cisco"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: &netbox.DcimDeviceType{ @@ -1283,7 +1283,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "undefined"}, + queryParams: map[string]string{"q": "undefined", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: nil, @@ -1437,7 +1437,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "undefined"}, + queryParams: map[string]string{"q": "undefined", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: &netbox.DcimDeviceType{ @@ -1552,7 +1552,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "undefined"}, + queryParams: map[string]string{"q": "undefined", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: &netbox.DcimDeviceType{ @@ -1674,7 +1674,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "ISR4321"}, + queryParams: map[string]string{"q": "ISR4321", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: nil, @@ -1861,7 +1861,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "ISR4321"}, + queryParams: map[string]string{"q": "ISR4321", "manufacturer__name": "Cisco"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: nil, @@ -2066,7 +2066,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "ISR4321"}, + queryParams: map[string]string{"q": "ISR4321", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: nil, @@ -2288,7 +2288,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "ISR4321"}, + queryParams: map[string]string{"q": "ISR4321", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: nil, @@ -2478,7 +2478,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "ISR4321"}, + queryParams: map[string]string{"q": "ISR4321", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: nil, @@ -2692,7 +2692,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "ISR4321"}, + queryParams: map[string]string{"q": "ISR4321", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: &netbox.DcimDeviceType{ @@ -2895,7 +2895,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "ISR4321"}, + queryParams: map[string]string{"q": "ISR4321", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: nil, @@ -3113,7 +3113,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "ISR4321"}, + queryParams: map[string]string{"q": "ISR4321", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: &netbox.DcimDeviceType{ @@ -3587,7 +3587,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "undefined"}, + queryParams: map[string]string{"q": "undefined", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: &netbox.DcimDeviceType{ @@ -3819,7 +3819,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "undefined"}, + queryParams: map[string]string{"q": "undefined", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: &netbox.DcimDeviceType{ @@ -3997,7 +3997,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "undefined"}, + queryParams: map[string]string{"q": "undefined", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: &netbox.DcimDeviceType{ @@ -4253,7 +4253,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "undefined"}, + queryParams: map[string]string{"q": "undefined", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: &netbox.DcimDeviceType{ @@ -4477,7 +4477,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "undefined"}, + queryParams: map[string]string{"q": "undefined", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: &netbox.DcimDeviceType{ @@ -4717,7 +4717,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "undefined"}, + queryParams: map[string]string{"q": "undefined", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: &netbox.DcimDeviceType{ @@ -5005,7 +5005,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "undefined"}, + queryParams: map[string]string{"q": "undefined", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: &netbox.DcimDeviceType{ @@ -5287,7 +5287,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "undefined"}, + queryParams: map[string]string{"q": "undefined", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: &netbox.DcimDeviceType{ @@ -5575,7 +5575,7 @@ func TestPrepare(t *testing.T) { { objectType: "dcim.devicetype", objectID: 0, - queryParams: map[string]string{"q": "undefined"}, + queryParams: map[string]string{"q": "undefined", "manufacturer__name": "undefined"}, objectChangeID: 0, object: &netbox.DcimDeviceTypeDataWrapper{ DeviceType: &netbox.DcimDeviceType{ @@ -6090,6 +6090,168 @@ func TestPrepare(t *testing.T) { }, wantErr: false, }, + { + name: "[P16] ingest dcim.device with device type and manufacturer - device type and manufacturer objects found - create device with existing device type and manufacturer", + rawIngestEntity: []byte(`{ + "request_id": "cfa0f129-125c-440d-9e41-e87583cd7d89", + "data_type": "dcim.device", + "entity": { + "Device": { + "name": "Device A", + "device_type": { + "model": "Device Type A", + "manufacturer": { + "name": "Manufacturer A" + } + }, + "role": { + "name": "Role ABC" + }, + "platform": { + "name": "Platform A", + "manufacturer": { + "name": "Manufacturer A" + } + }, + "serial": "123456", + "site": { + "name": "Site ABC" + } + } + }, + "state": 0 + }`), + retrieveObjectStates: []mockRetrieveObjectState{ + { + objectType: "dcim.site", + objectID: 0, + queryParams: map[string]string{"q": "Site ABC"}, + objectChangeID: 0, + object: &netbox.DcimSiteDataWrapper{ + Site: &netbox.DcimSite{ + ID: 1, + Name: "Site ABC", + Slug: "site-abc", + Status: (*netbox.DcimSiteStatus)(strPtr(string(netbox.DcimSiteStatusActive))), + }, + }, + }, + { + objectType: "dcim.manufacturer", + objectID: 0, + queryParams: map[string]string{"q": "Manufacturer A"}, + objectChangeID: 0, + object: &netbox.DcimManufacturerDataWrapper{ + Manufacturer: &netbox.DcimManufacturer{ + ID: 1, + Name: "Manufacturer A", + Slug: "manufacturer-a", + }, + }, + }, + { + objectType: "dcim.platform", + objectID: 0, + queryParams: map[string]string{"q": "Platform A", "manufacturer__name": "Manufacturer A"}, + objectChangeID: 0, + object: &netbox.DcimPlatformDataWrapper{ + Platform: &netbox.DcimPlatform{ + ID: 1, + Name: "Platform A", + Slug: "platform-a", + Manufacturer: &netbox.DcimManufacturer{ + ID: 1, + Name: "Manufacturer A", + Slug: "manufacturer-a", + }, + }, + }, + }, + { + objectType: "dcim.devicetype", + objectID: 0, + queryParams: map[string]string{"q": "Device Type A", "manufacturer__name": "Manufacturer A"}, + objectChangeID: 0, + object: &netbox.DcimDeviceTypeDataWrapper{ + DeviceType: &netbox.DcimDeviceType{ + ID: 1, + Model: "Device Type A", + Slug: "device-type-a", + Manufacturer: &netbox.DcimManufacturer{ + ID: 1, + Name: "Manufacturer A", + Slug: "manufacturer-a", + }, + }, + }, + }, + { + objectType: "dcim.devicerole", + objectID: 0, + queryParams: map[string]string{"q": "Role ABC"}, + objectChangeID: 0, + object: &netbox.DcimDeviceRoleDataWrapper{ + DeviceRole: &netbox.DcimDeviceRole{ + ID: 1, + Name: "Role ABC", + Slug: "role-abc", + Color: strPtr("000000"), + }, + }, + }, + { + objectType: "dcim.device", + objectID: 0, + queryParams: map[string]string{"q": "Device A", "site__name": "Site ABC"}, + objectChangeID: 0, + object: &netbox.DcimDeviceDataWrapper{ + Device: &netbox.DcimDevice{ + ID: 1, + Name: "Device A", + Site: &netbox.DcimSite{ + ID: 1, + Name: "Site ABC", + Slug: "site-abc", + Status: (*netbox.DcimSiteStatus)(strPtr(string(netbox.DcimSiteStatusActive))), + }, + DeviceType: &netbox.DcimDeviceType{ + ID: 1, + Model: "Device Type A", + Slug: "device-type-a", + Manufacturer: &netbox.DcimManufacturer{ + ID: 1, + Name: "Manufacturer A", + Slug: "manufacturer-a", + }, + }, + Role: &netbox.DcimDeviceRole{ + ID: 1, + Name: "Role ABC", + Slug: "role-abc", + Color: strPtr("000000"), + }, + Platform: &netbox.DcimPlatform{ + ID: 1, + Name: "Platform A", + Slug: "platform-a", + Manufacturer: &netbox.DcimManufacturer{ + ID: 1, + Name: "Manufacturer A", + Slug: "manufacturer-a", + }, + }, + Serial: strPtr("123456"), + Status: (*netbox.DcimDeviceStatus)(strPtr(string(netbox.DcimDeviceStatusActive))), + }, + }, + }, + }, + wantChangeSet: changeset.ChangeSet{ + ChangeSetID: "5663a77e-9bad-4981-afe9-77d8a9f2b8b5", + ChangeSet: []changeset.Change{}, + }, + wantErr: false, + }, } for _, tt := range tests { From 3fb8886948cdbe9630970f949946ea321facfa00 Mon Sep 17 00:00:00 2001 From: Michal Fiedorowicz Date: Wed, 22 May 2024 12:13:59 +0100 Subject: [PATCH 3/4] fix: OBS-441 - tests: use diode-sdk-python message wrappers Signed-off-by: Michal Fiedorowicz --- tests/features/steps/ingestion_device_object.py | 15 ++++++--------- .../steps/ingestion_device_type_object.py | 2 +- .../features/steps/ingestion_interface_object.py | 2 +- .../features/steps/ingestion_ip_address_object.py | 2 +- .../steps/ingestion_manufacturer_object.py | 2 +- tests/features/steps/ingestion_prefix_object.py | 2 +- tests/features/steps/ingestion_role_object.py | 2 +- tests/features/steps/ingestion_site_object.py | 2 +- 8 files changed, 13 insertions(+), 16 deletions(-) diff --git a/tests/features/steps/ingestion_device_object.py b/tests/features/steps/ingestion_device_object.py index ea09e11c..b2659e7b 100644 --- a/tests/features/steps/ingestion_device_object.py +++ b/tests/features/steps/ingestion_device_object.py @@ -1,10 +1,7 @@ from behave import given, when, then -from netboxlabs.diode.sdk.diode.v1.ingester_pb2 import ( +from netboxlabs.diode.sdk.ingester import ( Device, - DeviceType, Entity, - Role, - Site, ) from steps.utils import ( get_object_state, @@ -44,7 +41,7 @@ def ingest_device_without_site(context): """Ingest the device using the Diode SDK""" entities = [ - Entity(device=Device(name=context.device_name)), + Entity(device=context.device_name), ] response = ingester(entities) @@ -123,7 +120,7 @@ def ingest_device_with_site(context): Entity( device=Device( name=context.device_name, - site=Site(name=context.site_name), + site=context.site_name, ), ), ] @@ -152,9 +149,9 @@ def ingest_device_with_site_device_type_and_role(context): Entity( device=Device( name=context.device_name, - site=Site(name=context.site_name), - device_type=DeviceType(model=context.device_type_model), - role=Role(name=context.device_role_name), + site=context.site_name, + device_type=context.device_type_model, + role=context.device_role_name, ), ), ] diff --git a/tests/features/steps/ingestion_device_type_object.py b/tests/features/steps/ingestion_device_type_object.py index 024afae8..01b7e7bd 100644 --- a/tests/features/steps/ingestion_device_type_object.py +++ b/tests/features/steps/ingestion_device_type_object.py @@ -1,7 +1,7 @@ import time from behave import given, when, then -from netboxlabs.diode.sdk.diode.v1.ingester_pb2 import DeviceType, Entity, Manufacturer +from netboxlabs.diode.sdk.ingester import DeviceType, Entity, Manufacturer from steps.utils import ( get_object_by_name, send_delete_request, diff --git a/tests/features/steps/ingestion_interface_object.py b/tests/features/steps/ingestion_interface_object.py index e3ccfff9..63e8f9d6 100644 --- a/tests/features/steps/ingestion_interface_object.py +++ b/tests/features/steps/ingestion_interface_object.py @@ -1,7 +1,7 @@ import time from behave import given, when, then -from netboxlabs.diode.sdk.diode.v1.ingester_pb2 import Entity, Interface +from netboxlabs.diode.sdk.ingester import Entity, Interface from steps.utils import ( get_object_by_name, ingester, diff --git a/tests/features/steps/ingestion_ip_address_object.py b/tests/features/steps/ingestion_ip_address_object.py index 4e23f8d5..ae6af335 100644 --- a/tests/features/steps/ingestion_ip_address_object.py +++ b/tests/features/steps/ingestion_ip_address_object.py @@ -1,5 +1,5 @@ from behave import given, when, then -from netboxlabs.diode.sdk.diode.v1.ingester_pb2 import Entity, Interface, IPAddress +from netboxlabs.diode.sdk.ingester import Entity, Interface, IPAddress from steps.utils import ( get_object_state, ingester, diff --git a/tests/features/steps/ingestion_manufacturer_object.py b/tests/features/steps/ingestion_manufacturer_object.py index 95674299..4fff5265 100644 --- a/tests/features/steps/ingestion_manufacturer_object.py +++ b/tests/features/steps/ingestion_manufacturer_object.py @@ -1,7 +1,7 @@ import time from behave import given, when, then -from netboxlabs.diode.sdk.diode.v1.ingester_pb2 import Entity, Manufacturer +from netboxlabs.diode.sdk.ingester import Entity, Manufacturer from steps.utils import get_object_by_name, ingester endpoint = "dcim/manufacturers/" diff --git a/tests/features/steps/ingestion_prefix_object.py b/tests/features/steps/ingestion_prefix_object.py index cee62751..bbe3b7c6 100644 --- a/tests/features/steps/ingestion_prefix_object.py +++ b/tests/features/steps/ingestion_prefix_object.py @@ -1,7 +1,7 @@ import time from behave import given, when, then -from netboxlabs.diode.sdk.diode.v1.ingester_pb2 import Entity, Prefix +from netboxlabs.diode.sdk.ingester import Entity, Prefix from steps.utils import ( get_object_by_name, ingester, diff --git a/tests/features/steps/ingestion_role_object.py b/tests/features/steps/ingestion_role_object.py index 7de9a330..1f128075 100644 --- a/tests/features/steps/ingestion_role_object.py +++ b/tests/features/steps/ingestion_role_object.py @@ -1,7 +1,7 @@ import time from behave import given, when, then -from netboxlabs.diode.sdk.diode.v1.ingester_pb2 import Entity, Role +from netboxlabs.diode.sdk.ingester import Entity, Role from steps.utils import get_object_by_name, ingester endpoint = "dcim/device-roles/" diff --git a/tests/features/steps/ingestion_site_object.py b/tests/features/steps/ingestion_site_object.py index a5da9647..1a548331 100644 --- a/tests/features/steps/ingestion_site_object.py +++ b/tests/features/steps/ingestion_site_object.py @@ -1,5 +1,5 @@ from behave import given, when, then -from netboxlabs.diode.sdk.diode.v1.ingester_pb2 import Entity, Site +from netboxlabs.diode.sdk.ingester import Entity, Site from steps.utils import get_object_state, ingester, send_delete_request endpoint = "dcim/sites/" From 0073cf25f9dc976ba752ed6f1cd672cf65a4a013 Mon Sep 17 00:00:00 2001 From: Michal Fiedorowicz Date: Wed, 22 May 2024 12:48:14 +0100 Subject: [PATCH 4/4] fix: OBS-441 - diode-sdk-python: ingester wrappers - decrease complexity Signed-off-by: Michal Fiedorowicz --- .../netboxlabs/diode/sdk/ingester.py | 174 +++++++++--------- 1 file changed, 86 insertions(+), 88 deletions(-) diff --git a/diode-sdk-python/netboxlabs/diode/sdk/ingester.py b/diode-sdk-python/netboxlabs/diode/sdk/ingester.py index 503f94ef..9a88ffc1 100644 --- a/diode-sdk-python/netboxlabs/diode/sdk/ingester.py +++ b/diode-sdk-python/netboxlabs/diode/sdk/ingester.py @@ -2,7 +2,7 @@ # Copyright 2024 NetBox Labs Inc """NetBox Labs, Diode - SDK - ingester protobuf message wrappers.""" -from typing import Mapping as _Mapping +from typing import Any from typing import Optional as _Optional from typing import Union as _Union @@ -43,6 +43,13 @@ ) +def convert_to_protobuf(value: Any, protobuf_class, **kwargs): + """Convert a value to a protobuf message.""" + if isinstance(value, str): + return protobuf_class(**kwargs) + return value + + class Manufacturer: """Manufacturer message wrapper.""" @@ -77,8 +84,9 @@ def __new__( tags: _Optional[list[str]] = None, ) -> PlatformPb: """Create a new Platform protobuf message.""" - if isinstance(manufacturer, str): - manufacturer = ManufacturerPb(name=manufacturer) + manufacturer = convert_to_protobuf( + manufacturer, ManufacturerPb, name=manufacturer + ) if isinstance(tags, list) and all(isinstance(t, str) for t in tags): tags = [TagPb(name=tag) for tag in tags] @@ -130,8 +138,9 @@ def __new__( tags: _Optional[list[str]] = None, ) -> DeviceTypePb: """Create a new DeviceType protobuf message.""" - if isinstance(manufacturer, str): - manufacturer = ManufacturerPb(name=manufacturer) + manufacturer = convert_to_protobuf( + manufacturer, ManufacturerPb, name=manufacturer + ) if isinstance(tags, list) and all(isinstance(t, str) for t in tags): tags = [TagPb(name=tag) for tag in tags] @@ -169,11 +178,12 @@ def __new__( manufacturer: _Optional[_Union[str, Manufacturer, ManufacturerPb]] = None, ) -> DevicePb: """Create a new Device protobuf message.""" - if isinstance(manufacturer, str): - manufacturer = ManufacturerPb(name=manufacturer) - - if isinstance(platform, str): - platform = PlatformPb(name=platform, manufacturer=manufacturer) + manufacturer = convert_to_protobuf( + manufacturer, ManufacturerPb, name=manufacturer + ) + platform = convert_to_protobuf( + platform, PlatformPb, name=platform, manufacturer=manufacturer + ) if ( isinstance(platform, PlatformPb) @@ -182,11 +192,10 @@ def __new__( ): platform.manufacturer.CopyFrom(manufacturer) - if isinstance(site, str): - site = SitePb(name=site) - - if isinstance(device_type, str): - device_type = DeviceTypePb(model=device_type, manufacturer=manufacturer) + site = convert_to_protobuf(site, SitePb, name=site) + device_type = convert_to_protobuf( + device_type, DeviceTypePb, model=device_type, manufacturer=manufacturer + ) if ( isinstance(device_type, DeviceTypePb) @@ -195,17 +204,13 @@ def __new__( ): device_type.manufacturer.CopyFrom(manufacturer) - if isinstance(role, str): - role = RolePb(name=role) + role = convert_to_protobuf(role, RolePb, name=role) if isinstance(tags, list) and all(isinstance(t, str) for t in tags): tags = [TagPb(name=tag) for tag in tags] - if isinstance(primary_ip4, str): - primary_ip4 = IPAddressPb(address=primary_ip4) - - if isinstance(primary_ip6, str): - primary_ip6 = IPAddressPb(address=primary_ip6) + primary_ip4 = convert_to_protobuf(primary_ip4, IPAddressPb, address=IPAddressPb) + primary_ip6 = convert_to_protobuf(primary_ip6, IPAddressPb, address=IPAddressPb) return DevicePb( name=name, @@ -250,11 +255,13 @@ def __new__( tags: _Optional[list[str]] = None, ) -> InterfacePb: """Create a new Interface protobuf message.""" - if isinstance(manufacturer, str): - manufacturer = ManufacturerPb(name=manufacturer) + manufacturer = convert_to_protobuf( + manufacturer, ManufacturerPb, name=manufacturer + ) - if isinstance(platform, str): - platform = PlatformPb(name=platform, manufacturer=manufacturer) + platform = convert_to_protobuf( + platform, PlatformPb, name=platform, manufacturer=manufacturer + ) if ( isinstance(platform, PlatformPb) @@ -263,11 +270,11 @@ def __new__( ): platform.manufacturer.CopyFrom(manufacturer) - if isinstance(site, str): - site = SitePb(name=site) + site = convert_to_protobuf(site, SitePb, name=site) - if isinstance(device_type, str): - device_type = DeviceTypePb(model=device_type, manufacturer=manufacturer) + device_type = convert_to_protobuf( + device_type, DeviceTypePb, model=device_type, manufacturer=manufacturer + ) if ( isinstance(device_type, DeviceTypePb) @@ -276,17 +283,17 @@ def __new__( ): device_type.manufacturer.CopyFrom(manufacturer) - if isinstance(role, str): - role = RolePb(name=role) + role = convert_to_protobuf(role, RolePb, name=role) - if isinstance(device, str): - device = DevicePb( - name=device, - device_type=device_type, - platform=platform, - site=site, - role=role, - ) + device = convert_to_protobuf( + device, + DevicePb, + name=device, + device_type=device_type, + platform=platform, + site=site, + role=role, + ) if isinstance(tags, list) and all(isinstance(t, str) for t in tags): tags = [TagPb(name=tag) for tag in tags] @@ -329,11 +336,13 @@ def __new__( tags: _Optional[list[str]] = None, ) -> IPAddressPb: """Create a new IPAddress protobuf message.""" - if isinstance(manufacturer, str): - manufacturer = ManufacturerPb(name=manufacturer) + manufacturer = convert_to_protobuf( + manufacturer, ManufacturerPb, name=manufacturer + ) - if isinstance(platform, str): - platform = PlatformPb(name=platform, manufacturer=manufacturer) + platform = convert_to_protobuf( + platform, PlatformPb, name=platform, manufacturer=manufacturer + ) if ( isinstance(platform, PlatformPb) @@ -342,11 +351,11 @@ def __new__( ): platform.manufacturer.CopyFrom(manufacturer) - if isinstance(site, str): - site = SitePb(name=site) + site = convert_to_protobuf(site, SitePb, name=site) - if isinstance(device_type, str): - device_type = DeviceTypePb(model=device_type, manufacturer=manufacturer) + device_type = convert_to_protobuf( + device_type, DeviceTypePb, model=device_type, manufacturer=manufacturer + ) if ( isinstance(device_type, DeviceTypePb) @@ -355,19 +364,24 @@ def __new__( ): device_type.manufacturer.CopyFrom(manufacturer) - if isinstance(device_role, str): - device_role = RolePb(name=device_role) + device_role = convert_to_protobuf(device_role, RolePb, name=device_role) - if isinstance(device, str): - device = DevicePb( - name=device, - device_type=device_type, - platform=platform, - site=site, - role=device_role, - ) - if isinstance(interface, str): - interface = InterfacePb(name=interface, device=device) + device = convert_to_protobuf( + device, + DevicePb, + name=device, + device_type=device_type, + platform=platform, + site=site, + role=device_role, + ) + + interface = convert_to_protobuf( + interface, + InterfacePb, + name=interface, + device=device, + ) if isinstance(tags, list) and all(isinstance(t, str) for t in tags): tags = [TagPb(name=tag) for tag in tags] @@ -399,8 +413,7 @@ def __new__( tags: _Optional[list[str]] = None, ) -> PrefixPb: """Create a new Prefix protobuf message.""" - if isinstance(site, str): - site = SitePb(name=site) + site = convert_to_protobuf(site, SitePb, name=site) if isinstance(tags, list) and all(isinstance(t, str) for t in tags): tags = [TagPb(name=tag) for tag in tags] @@ -464,32 +477,17 @@ def __new__( timestamp: _Optional[_timestamp_pb2.Timestamp] = None, ): """Create a new Entity protobuf message.""" - if isinstance(site, str): - site = SitePb(name=site) - - if isinstance(platform, str): - platform = PlatformPb(name=platform) - - if isinstance(manufacturer, str): - manufacturer = ManufacturerPb(name=manufacturer) - - if isinstance(device, str): - device = DevicePb(name=device) - - if isinstance(device_role, str): - device_role = RolePb(name=device_role) - - if isinstance(device_type, str): - device_type = DeviceTypePb(model=device_type) - - if isinstance(ip_address, str): - ip_address = IPAddressPb(address=ip_address) - - if isinstance(interface, str): - interface = InterfacePb(name=interface) - - if isinstance(prefix, str): - prefix = PrefixPb(prefix=prefix) + site = convert_to_protobuf(site, SitePb, name=site) + platform = convert_to_protobuf(platform, PlatformPb, name=platform) + manufacturer = convert_to_protobuf( + manufacturer, ManufacturerPb, name=manufacturer + ) + device = convert_to_protobuf(device, DevicePb, name=device) + device_role = convert_to_protobuf(device_role, RolePb, name=device_role) + device_type = convert_to_protobuf(device_type, DeviceTypePb, model=device_type) + ip_address = convert_to_protobuf(ip_address, IPAddressPb, address=ip_address) + interface = convert_to_protobuf(interface, InterfacePb, name=interface) + prefix = convert_to_protobuf(prefix, PrefixPb, prefix=prefix) return EntityPb( site=site,