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.

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

operator: rework label templating #808

Merged
merged 9 commits into from
Aug 5, 2024
Merged

Conversation

fgiudici
Copy link
Member

@fgiudici fgiudici commented Aug 2, 2024

Reworked the code to provide templating to MachineInventory labels and name.
The template source data is the "System Information" (BIOS) and "System Data" (which we usually called 'HW Labels') data sent by the machines via the register client.

This rework fixes some bugs and duplicated code and slightly changes the behavior in few cases.
The behavioral changes address the following corner cases:

  • . is added to the allowed characters in label values (previously was substituted with -).
  • when the first character of a label value is an accepted one but not alphanumeric (i.e., - , _ or .) we drop it (previously we prepended m to the label).
  • if the last character of a label value is not alphanumeric (i.e., - , _ or .) we drop it (previously was not checked).
    note that for the MachineInventory name (which will also be the machine hostname after k8s provisioning) the allowed set of characters has not changed: it's the same of the label case but without the _ (i.e., - or .).

Regarding the MachineRegistration.spec.machineName:

  • if it is empty, a default m-${UUID} value is assigned to the MachineInventory.name (as previously)
  • if it contains a template value which doesn't exists (wrong template value or nosmbios option) the MachineInventory.name is assigned a default m-${UUID} name (new behavior).
  • if it is not empty, but after resolving the template values and sanitizing the string it gets empty, the name assignment process will error out failing the registration process.

Fixes #807

@fgiudici fgiudici requested a review from a team as a code owner August 2, 2024 16:43
@fgiudici fgiudici changed the title operator operator: rework label templating Aug 2, 2024
@github-actions github-actions bot added area/operator operator related changes area/tests test related changes labels Aug 2, 2024
Copy link
Contributor

@anmazzotti anmazzotti left a comment

Choose a reason for hiding this comment

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

Looks good

no code changes, just label templating functions moved to labeltmpl.go.

Signed-off-by: Francesco Giudici <francesco.giudici@suse.com>
Signed-off-by: Francesco Giudici <francesco.giudici@suse.com>
Signed-off-by: Francesco Giudici <francesco.giudici@suse.com>
Fixes rancher#807

Signed-off-by: Francesco Giudici <francesco.giudici@suse.com>
Signed-off-by: Francesco Giudici <francesco.giudici@suse.com>
Signed-off-by: Francesco Giudici <francesco.giudici@suse.com>
1. '.' is added to the allowed characted in label values (previously was
   sobsituted with '-').
2. when the first character of a label value is not alphanumeric ("-" or
   "_" or ".") we drop it (previously we prepended 'm').
3. if the last characted of a label value is not alphanumeric ("-" or
   "_" or ".") we drop it (previously was not checked).

Signed-off-by: Francesco Giudici <francesco.giudici@suse.com>
Signed-off-by: Francesco Giudici <francesco.giudici@suse.com>
Signed-off-by: Francesco Giudici <francesco.giudici@suse.com>
@fgiudici fgiudici merged commit 6681ea5 into rancher:main Aug 5, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/operator operator related changes area/tests test related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid MachineRegistration.spec.machineName ends up in a MachineInventory hostname equal to 'm'
2 participants