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

Rust agent does not support dmidecode or hostname for uuid option. #454

Open
ansasaki opened this issue Sep 21, 2022 · 14 comments
Open

Rust agent does not support dmidecode or hostname for uuid option. #454

ansasaki opened this issue Sep 21, 2022 · 14 comments
Labels
configuration Involves changes to configuration file format enhancement help wanted

Comments

@ansasaki
Copy link
Contributor

The rust agent does not support the dmidecode nor the hostname keywords for the uuid option.

@ansasaki ansasaki added enhancement configuration Involves changes to configuration file format labels Feb 8, 2023
@ansasaki
Copy link
Contributor Author

ansasaki commented Feb 8, 2023

Related: #280

@arkivm
Copy link

arkivm commented Sep 25, 2023

For uuid, the dmidecode -s system-uuid is available only for privileged users at /sys/devices/virtual/dmi/id/product_uuid. There is a system-generated machine-id which can be read as a normal user /etc/machine-id. Any thoughts on which one we want to use?

@THS-on
Copy link
Member

THS-on commented Sep 26, 2023

I don't even think that we need that option anymore, because you can override the uuid via an env variable, if you need special generation of uuids.

@ansasaki what do you think?

@aplanas
Copy link
Contributor

aplanas commented Sep 26, 2023

I don't even think that we need that option anymore, because you can override the uuid via an env variable, if you need special generation of uuids.

As a workaround this is true. For deployments you would need to add a systemd override to the unit file to export a new env var to do that automatically.

If we decide to go to this path, at least we should document how to do that.

@arkivm
Copy link

arkivm commented Sep 27, 2023

Does this hold for hostname option as well, as hostname as UUID can be overridden via an env var

@THS-on
Copy link
Member

THS-on commented Sep 28, 2023

If we decide to go to this path, at least we should document how to do that.

@aplanas yes we should add that at least to the config file.

Does this hold for hostname option as well, as hostname as UUID can be overridden via an env var

@arkivm if it is easy to implement, we can implement the hostname option.

@aplanas
Copy link
Contributor

aplanas commented Sep 28, 2023

Does this hold for hostname option as well, as hostname as UUID can be overridden via an env var

There is this inconsistency that maybe we can try to resolve. Reading this makes me feel bad:

uuid = "cloud1.company.com"

Hostnames are useful: are unique and easy to remember when writing the tenant command to enrolling it, but they are clearly no UUIDs.

I am not sure how this should be resolved, but maybe we can introduce a new field named "ID", that can contain other strings that represent the agent, that can be used if the UUID field is not present, and force that the UUID is a valid UUID and not some random string.

@THS-on
Copy link
Member

THS-on commented Sep 29, 2023

@aplanas sounds like plan to introduce an id option 👍

@derpedda
Copy link

derpedda commented Jul 4, 2024

Any news in this matter?

@ansasaki
Copy link
Contributor Author

ansasaki commented Jul 4, 2024

Any news in this matter?

Not really, the priority on this is very low as nobody provided an use case where overriding the UUID using environment variables is not sufficient.

@derpedda Could you please describe your use case for this missing feature?

@derpedda
Copy link

derpedda commented Jul 4, 2024

Were going to use WereWulf to deploy keylime on an HPCcluster, overriding using environment variables would be possible of course)
I tried overriding with the FQDN hostname, but the agent complains (in a very understandable way) that it is not a UUID and still decides to go for a generated one. As far as I understand, the correct behaviour as of keylime-agent/src/config.rs
Did I miss something or should I file another issue?

@ansasaki
Copy link
Contributor Author

ansasaki commented Jul 5, 2024

I think there is no need for another issue as this problem was reported via #280

I believe the best way to fix this is to implement what @aplanas suggested:

  • Add a new configuration option ID separate from UUID which supports hostnames (and maybe dmidecode in future)
  • In case UUID is set as hostname, set ID as the hostname and generate an UUID
  • Add the support to use ID instead of UUID in the tenant. This way, hostnames could be used in the tenant command.

There is an extra complication of managing ID duplicates. And even bigger complication if it is expected that the ID are supported on the API in place of the UUID. I still need to figure out how the ID resolution would be implemented in the tenant, meaning how to obtain the correct UUID given the ID.

What do you think? Would this be acceptable?

I do not promise this will be implemented soon as the solution is complicated and affects many parts of the code.

@derpedda
Copy link

derpedda commented Jul 5, 2024

Adding the extra field ID following @aplanas solution would be the most clean way to implement it indeed. I also highly support the position that the UUID field should in any case stay a UUID field.
Of course this is not trivial as it impacts many places, as you said.

What comes to my mind with generated UUIDs and another identifying ID - how would duplicates with same ID but different generated UUID (same client or not) be handled? This can cause plenty of issues as well.

@ansasaki
Copy link
Contributor Author

ansasaki commented Jul 5, 2024

What comes to my mind with generated UUIDs and another identifying ID - how would duplicates with same ID but different generated UUID (same client or not) be handled? This can cause plenty of issues as well.

I see some possible solutions with various levels of complexity:

  • Do not allow duplicates (effectively making ID unique as well)
  • Allow duplicates
    • Show matches to the tenant when the ID is not unique (effectively making it necessary to use UUID when there are duplicates)
    • Same as above, but using some heuristic to help the user (e.g. remember the tenant who added the UUID and showing this visually to the user)
  • Create namespaces
    • Allow ID duplicates globally, but not inside the namespace
      • Do not allow duplicate namespaces

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configuration Involves changes to configuration file format enhancement help wanted
Projects
None yet
Development

No branches or pull requests

5 participants