Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add extra labels with smbios data #155

Merged
merged 1 commit into from
Sep 1, 2022
Merged

Add extra labels with smbios data #155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions pkg/server/register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const defaultName = "m-${System Information/Manufacturer}-${System Information/P
var (
sanitize = regexp.MustCompile("[^0-9a-zA-Z]")
doubleDash = regexp.MustCompile("--+")
start = regexp.MustCompile("^[a-zA-Z]")
start = regexp.MustCompile("^[a-zA-Z0-9]")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is required? I don't know from where this restriction came from, but I am wondering if relaxing it will cause issues.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was used just to check if the label (was the name) had a valid first character, otherwise m is prepended.
Yep, this code is a bit cumbersome maybe it deserves a rework 😅

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then as a result of this change a name staring with a digit has no m prepended right? will it be an issue?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK no, kubernetes resource names and labels may start with a digit too

Copy link
Contributor Author

@Itxaka Itxaka Sep 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, this was done only with the machine name in mind, so the regex never expected to find a machine name starting with a digit :D

This is mostly due to protection against someone setting a machine name like -sdfa or an empty name ""

And yeah k8s names/labels are alphanumeric + - and . but not at the start or end -> https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names

)

func (i *InventoryServer) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
Expand Down Expand Up @@ -235,7 +235,7 @@ func (i *InventoryServer) writeMachineInventoryCloudConfig(conn *websocket.Conn,
})
}

func buildName(data map[string]interface{}, name string) string {
func buildStringFromSmbiosData(data map[string]interface{}, name string) string {
str := name
result := &strings.Builder{}
for {
Expand Down Expand Up @@ -296,7 +296,22 @@ func (i *InventoryServer) serveLoop(conn *websocket.Conn, inventory *elm.Machine
if err = json.Unmarshal(data, &smbiosData); err != nil {
return msgType, fmt.Errorf("cannot extract SMBios data: %w", err)
}
inventory.Name = buildName(smbiosData, inventory.Name)
inventory.Name = buildStringFromSmbiosData(smbiosData, inventory.Name)
logrus.Debug("Adding extra labels")
// Add extra label info from data coming from smbios
newLabels := map[string]string{
"manufacturer": buildStringFromSmbiosData(smbiosData, "${System Information/Manufacturer}"),
"productName": buildStringFromSmbiosData(smbiosData, "${System Information/Product Name}"),
"serialNumber": buildStringFromSmbiosData(smbiosData, "${System Information/Serial Number}"),
"uuid": buildStringFromSmbiosData(smbiosData, "${System Information/UUID}"),
"cpuManufacturer": buildStringFromSmbiosData(smbiosData, "${Processor Information/Manufacturer}"),
"cpuFamily": buildStringFromSmbiosData(smbiosData, "${Processor Information/Family}"),
"cpuCore": buildStringFromSmbiosData(smbiosData, "${Processor Information/Core Count}"),
"cpuThreads": buildStringFromSmbiosData(smbiosData, "${Processor Information/Thread Count}"),
}
for k, v := range newLabels {
inventory.Labels[k] = strings.TrimSuffix(strings.TrimPrefix(v, "-"), "-")
}
logrus.Debugf("received SMBIOS data - generated machine name: %s", inventory.Name)
case register.MsgLabels:
if err := mergeInventoryLabels(inventory, data); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/register_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func TestBuildName(t *testing.T) {
}

for _, testCase := range testCase {
assert.Equal(t, testCase.Output, buildName(data, testCase.Format))
assert.Equal(t, testCase.Output, buildStringFromSmbiosData(data, testCase.Format))
}
}

Expand Down