✨ add serial number to asset ids #4539
Conversation
| @@ -161,6 +162,17 @@ func GatherPlatformInfo(conn shared.Connection, pf *inventory.Platform, idDetect | |||
| }, hostErr | |||
| } | |||
| return &PlatformInfo{}, nil | |||
| case idDetector == ids.IdDetector_SerialNumber: | |||
| serial, err := serialnumber.SerialNumber(conn, pf) | |||
There was a problem hiding this comment.
Have we tested that for macos, windows and linux?
There was a problem hiding this comment.
This is taken from the existing resource - machine.system.serial - which works on all aforementioned OS's
My tests so far show that the on the systems which don't provide that data (for example CI containers) serial evaluates to empty string, so it's omitted in the asset.ids
providers/os/id/platform.go
Outdated
| @@ -52,7 +53,7 @@ func IdentifyPlatform(conn shared.Connection, p *inventory.Platform, idDetectors | |||
| // fallback to default id detectors | |||
| switch conn.Type() { | |||
| case shared.Type_Local: | |||
| idDetectors = []string{ids.IdDetector_Hostname, ids.IdDetector_CloudDetect} | |||
| idDetectors = []string{ids.IdDetector_Hostname, ids.IdDetector_SerialNumber, ids.IdDetector_CloudDetect} | |||
There was a problem hiding this comment.
I think we want to do some more real-life testing before we enable this as default.
There was a problem hiding this comment.
As expected, if host can't provide serial name - it's just returned empty, therefore is not used as asset id.
Asset id is actually asset.ids, so server seeks for and uses the id server prefers (hostname at the moment).
If asset returns two or more ids this doesn't influence existing processes.
Tested on:
- MacOS
- Darwin Container
- Linux Container
- Linux Desktop
- Windows Desktop
- Windows VM
|
i tested this pr by running a scan on my local mac, changing my hostname, and then scanning again. the hostname value was updates as expected but i had the same serial number recorded in both cases (as expected). |
No that does not seem ok. Here's a scenario. We have an asset Now, asset |
|
Isn't detectors (therefore ids) order predefined? |
|
@atomic111 id detectors aren't OS specific and using existing resources from os provider where they can, so i'm not sure about this one. |
|
@jaym when can we merge this |
|
I think we need a way to prioritize platform ids if we want to take this change, otherwise it will be a breaking change. We will get into a state where if there are already duplicate representations of an asset where the hostname is flopping around, it will succeed once with a specific hostname in the platform id. Once it comes back with a different hostname in the platform id, the asset will not be able to scan |
slntopp
force-pushed
the
mik/add-serial-number-to-asset-ids
branch
from
1d319fd
to
591d726
Compare
slntopp
force-pushed
the
mik/add-serial-number-to-asset-ids
branch
from
42078e0
to
ff8c8a6
Compare
No description provided.