-
Notifications
You must be signed in to change notification settings - Fork 12
refactor: accept expanded deviceInfo fields in v1 devices #1021
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| package dto | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "time" | ||
| ) | ||
|
|
||
|
|
@@ -37,13 +38,104 @@ type Device struct { | |
| } | ||
|
|
||
| type DeviceInfo struct { | ||
| FWVersion string `json:"fwVersion"` | ||
| FWBuild string `json:"fwBuild"` | ||
| FWSku string `json:"fwSku"` | ||
| CurrentMode string `json:"currentMode"` | ||
| Features string `json:"features"` | ||
| IPAddress string `json:"ipAddress"` | ||
| LastUpdated time.Time `json:"lastUpdated"` | ||
| FWVersion string `json:"fwVersion"` | ||
| FWBuild string `json:"fwBuild"` | ||
| FWSku string `json:"fwSku"` | ||
| CurrentMode string `json:"currentMode"` | ||
| Features string `json:"features"` | ||
| IPAddress string `json:"ipAddress"` | ||
| LastUpdated time.Time `json:"lastUpdated"` | ||
| TLSMode string `json:"tlsMode"` | ||
| UPID map[string]any `json:"upid,omitempty"` | ||
| AMTEnabledInBIOS *bool `json:"amtEnabledInBIOS,omitempty"` | ||
| MEInterfaceVersion string `json:"meInterfaceVersion"` | ||
| DHCPEnabled *bool `json:"dhcpEnabled,omitempty"` | ||
| CertHashes []string `json:"certHashes,omitempty"` | ||
| LMSInstalled *bool `json:"lmsInstalled,omitempty"` | ||
| LMSVersion string `json:"lmsVersion"` | ||
| OSName string `json:"osName"` | ||
|
Comment on lines
+47
to
+56
|
||
| OSVersion string `json:"osVersion"` | ||
| OSDistro string `json:"osDistro"` | ||
| CPUModel string `json:"cpuModel"` | ||
| OSIPAddress string `json:"osIpAddress"` | ||
| EthernetAdapterCount *int `json:"ethernetAdapterCount,omitempty"` | ||
| MonitorConnected *bool `json:"monitorConnected,omitempty"` | ||
| IEEE8021XEnabled *bool `json:"ieee8021xEnabled,omitempty"` | ||
| LastDiscovered *time.Time `json:"lastDiscovered,omitempty"` | ||
| ExtraFields map[string]json.RawMessage `json:"-"` | ||
| } | ||
|
|
||
| func (d *DeviceInfo) UnmarshalJSON(data []byte) error { | ||
| type alias DeviceInfo | ||
|
|
||
| var base alias | ||
| if err := json.Unmarshal(data, &base); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| var raw map[string]json.RawMessage | ||
| if err := json.Unmarshal(data, &raw); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| delete(raw, "fwVersion") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Drop the catch-all entirely. Unknown fields aren't part of the contract — if a field isn't known, it doesn't belong on the wire. That means this whole Follow-ups in this PR:
Net result is roughly -50 lines and removes a class of silent-drift bugs. |
||
| delete(raw, "fwBuild") | ||
| delete(raw, "fwSku") | ||
| delete(raw, "currentMode") | ||
| delete(raw, "features") | ||
| delete(raw, "ipAddress") | ||
| delete(raw, "lastUpdated") | ||
| delete(raw, "tlsMode") | ||
| delete(raw, "upid") | ||
| delete(raw, "amtEnabledInBIOS") | ||
| delete(raw, "meInterfaceVersion") | ||
| delete(raw, "dhcpEnabled") | ||
| delete(raw, "certHashes") | ||
| delete(raw, "lmsInstalled") | ||
| delete(raw, "lmsVersion") | ||
| delete(raw, "osName") | ||
| delete(raw, "osVersion") | ||
| delete(raw, "osDistro") | ||
| delete(raw, "cpuModel") | ||
| delete(raw, "osIpAddress") | ||
| delete(raw, "ethernetAdapterCount") | ||
| delete(raw, "monitorConnected") | ||
| delete(raw, "ieee8021xEnabled") | ||
| delete(raw, "lastDiscovered") | ||
|
|
||
| *d = DeviceInfo(base) | ||
| if len(raw) > 0 { | ||
| d.ExtraFields = raw | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (d DeviceInfo) MarshalJSON() ([]byte, error) { | ||
| type alias DeviceInfo | ||
|
|
||
| base := alias(d) | ||
| base.ExtraFields = nil | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moot — the entire |
||
|
|
||
| b, err := json.Marshal(base) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if len(d.ExtraFields) == 0 { | ||
| return b, nil | ||
| } | ||
|
|
||
| var raw map[string]json.RawMessage | ||
| if err := json.Unmarshal(b, &raw); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| for key, value := range d.ExtraFields { | ||
| raw[key] = value | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This concern goes away entirely if |
||
| } | ||
|
|
||
| return json.Marshal(raw) | ||
| } | ||
|
|
||
| type Explorer struct { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| package dto | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestDeviceInfoJSONRoundTrip(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| amtEnabled := true | ||
| dhcpEnabled := true | ||
| lmsInstalled := true | ||
| ethernetAdapterCount := 2 | ||
| monitorConnected := true | ||
| ieee8021xEnabled := false | ||
| lastDiscovered := time.Date(2026, 5, 14, 10, 23, 0, 0, time.UTC) | ||
|
|
||
| info := DeviceInfo{ | ||
| FWVersion: "16.1.30", | ||
| FWBuild: "3400", | ||
| FWSku: "11", | ||
| CurrentMode: "Admin", | ||
| Features: "SOL,IDER,KVM", | ||
| IPAddress: "10.0.0.12", | ||
| LastUpdated: time.Date(2026, 5, 21, 0, 0, 0, 0, time.UTC), | ||
| TLSMode: "TLS 1.2", | ||
| UPID: map[string]any{"oemPlatformIdType": "Not Set (0)", "oemId": "", "csmeId": "4A45A39C5ED94620"}, | ||
| AMTEnabledInBIOS: &amtEnabled, | ||
| MEInterfaceVersion: "16.1.25.2124", | ||
| DHCPEnabled: &dhcpEnabled, | ||
| CertHashes: []string{"a1b2c3", "d4e5f6"}, | ||
| LMSInstalled: &lmsInstalled, | ||
| LMSVersion: "2410.5.0.0", | ||
| OSName: "linux", | ||
| OSVersion: "6.8.0-51-generic", | ||
| OSDistro: "Ubuntu 24.04 LTS", | ||
| CPUModel: "Intel(R) Core(TM) Ultra 7 165H", | ||
| OSIPAddress: "10.49.76.163", | ||
| EthernetAdapterCount: ðernetAdapterCount, | ||
| MonitorConnected: &monitorConnected, | ||
| IEEE8021XEnabled: &ieee8021xEnabled, | ||
| LastDiscovered: &lastDiscovered, | ||
| ExtraFields: map[string]json.RawMessage{ | ||
| "customFlag": json.RawMessage("true"), | ||
| }, | ||
| } | ||
|
|
||
| encoded, err := json.Marshal(info) | ||
| require.NoError(t, err) | ||
|
|
||
| var decoded DeviceInfo | ||
| require.NoError(t, json.Unmarshal(encoded, &decoded)) | ||
|
|
||
| require.Equal(t, info.TLSMode, decoded.TLSMode) | ||
| require.Equal(t, info.MEInterfaceVersion, decoded.MEInterfaceVersion) | ||
| require.Equal(t, info.CertHashes, decoded.CertHashes) | ||
| require.Equal(t, info.LMSVersion, decoded.LMSVersion) | ||
| require.NotNil(t, decoded.LMSInstalled) | ||
| require.Equal(t, *info.LMSInstalled, *decoded.LMSInstalled) | ||
| require.NotNil(t, decoded.LastDiscovered) | ||
| require.True(t, decoded.LastDiscovered.Equal(lastDiscovered)) | ||
| require.Contains(t, decoded.ExtraFields, "customFlag") | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,10 @@ func ptr(s string) *string { | |
| return &s | ||
| } | ||
|
|
||
| func boolPtr(v bool) *bool { | ||
| return &v | ||
| } | ||
|
|
||
| type testUsecase struct { | ||
| name string | ||
| guid string | ||
|
|
@@ -740,6 +744,7 @@ func TestUpdatePartial(t *testing.T) { | |
| GUID: "device-guid-123", | ||
| TenantID: "tenant-id-456", | ||
| Hostname: "old-hostname", | ||
| DeviceInfo: `{"fwVersion":"11.8.50","ipAddress":"10.0.0.1","lmsInstalled":false}`, | ||
| Tags: "lab,floor-2", | ||
| MPSUsername: "admin", | ||
| Username: "amtadmin", | ||
|
|
@@ -753,14 +758,20 @@ func TestUpdatePartial(t *testing.T) { | |
| GUID: "device-guid-123", | ||
| TenantID: "tenant-id-456", | ||
| Hostname: "new-hostname", | ||
| DeviceInfo: &dto.DeviceInfo{ | ||
| FWVersion: "16.1.30", | ||
| IPAddress: "10.0.0.55", | ||
| LMSInstalled: boolPtr(true), | ||
| }, | ||
| } | ||
| fields := map[string]bool{"guid": true, "tenantId": true, "hostname": true} | ||
| fields := map[string]bool{"guid": true, "tenantId": true, "hostname": true, "deviceinfo": true} | ||
|
|
||
| // After merge + dtoToEntity (MockCrypto re-encrypts plaintext to "encrypted"): | ||
| expectedEntity := &entity.Device{ | ||
| GUID: "device-guid-123", | ||
| TenantID: "tenant-id-456", | ||
| Hostname: "new-hostname", | ||
| DeviceInfo: `{"fwVersion":"16.1.30","fwBuild":"","fwSku":"","currentMode":"","features":"","ipAddress":"10.0.0.55","lastUpdated":"0001-01-01T00:00:00Z","tlsMode":"","meInterfaceVersion":"","lmsInstalled":true,"lmsVersion":"","osName":"","osVersion":"","osDistro":"","cpuModel":"","osIpAddress":""}`, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This pins the expected entity's
|
||
| Tags: "lab,floor-2", | ||
| MPSUsername: "admin", | ||
| Username: "amtadmin", | ||
|
|
@@ -770,9 +781,14 @@ func TestUpdatePartial(t *testing.T) { | |
| } | ||
|
|
||
| expectedDTO := &dto.Device{ | ||
| GUID: "device-guid-123", | ||
| TenantID: "tenant-id-456", | ||
| Hostname: "new-hostname", | ||
| GUID: "device-guid-123", | ||
| TenantID: "tenant-id-456", | ||
| Hostname: "new-hostname", | ||
| DeviceInfo: &dto.DeviceInfo{ | ||
| FWVersion: "16.1.30", | ||
| IPAddress: "10.0.0.55", | ||
| LMSInstalled: boolPtr(true), | ||
| }, | ||
| Tags: []string{"lab", "floor-2"}, | ||
| MPSUsername: "admin", | ||
| Username: "amtadmin", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
map[string]anycoerces numeric JSON values tofloat64on Unmarshal. If any UPID subfield is ever sent as an integer (e.g."oemPlatformIdType": 0), the re-marshaled value loses type fidelity, and any 64-bit identifier larger than 2^53 loses precision outright.The Postman sample shows UPID has a knowable shape (
oemPlatformIdType,oemId,csmeId) — a typed struct would be safer thanmap[string]any. If a typed struct isn't practical yet,map[string]json.RawMessagepreserves the original bytes for round-trip without coercion.