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

Fix(inventory): 'uuid' criterion must be identical to what VirtualMachine does #18929

Merged

Conversation

stonebuzz
Copy link
Contributor

Checklist before requesting a review

Please delete options that are not relevant.

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my code.
  • I have added tests that prove my fix is effective or that my feature works.
  • This change requires a documentation update.

Description

This PR enhances the GLPI reconciliation engine by integrating the getUUIDRestrictCriteria() method into UUID-based matching rules.

Currently, virtual machine processing correctly identifies and associates computers using this method. However, the reconciliation rules do not account for "mixed" UUIDs in their queries, leading to inconsistencies.

As a result, while the native inventory successfully links computers even when UUIDs are "mixed," the reconciliation process fails to do so.

Use Case:

1️⃣ Step 1: Importing an inventory file (Computer with Agent)

  • The file contains "uuid": "BC4C4D56-A9C6-6919-3FEE-FA0F3887B6A5".
  • GLPI correctly imports it as a Computer → ✅ OK

2️⃣ Step 2: ESX Inventory & VM Reconciliation

  • The ESX inventory detects a VM with "uuid": "564d4cbc-c6a9-1969-3fee-fa0f3887b6a5".
  • GLPI correctly reconciles it with the related computer, even though the UUID is "mixed."
  • During the process, the Computer's UUID is updated to match the ESX one → ✅ OK

3️⃣ Step 3: Importing an inventory file (Computer without Agent / Agentless)

  • The file contains "uuid": "BC4C4D56-A9C6-6919-3FEE-FA0F3887B6A5".
  • Instead of reconciling it with the existing Computer, GLPI creates a new one.
  • This happens because the UUID-based matching rules do not use getUUIDRestrictCriteria() → ❌ KO

This leads to an error in ESX (the virtual machine manager) because ESX cannot correctly identify the associated computer.

image

Screenshots (if appropriate):

Copy link
Contributor

@trasher trasher left a comment

Choose a reason for hiding this comment

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

I don't know if that solve reported issue; but it seems legit getUUIDRestrictCriteria to be used here.
I don't know if this change should target 10.0/bf branch, no idea what could be the impact on an existing production environment.

@trasher trasher requested a review from cedric-anne February 5, 2025 14:39
@stonebuzz stonebuzz changed the base branch from main to 10.0/bugfixes February 5, 2025 15:10
Copy link
Member

@cedric-anne cedric-anne left a comment

Choose a reason for hiding this comment

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

Seems OK.

@cedric-anne
Copy link
Member

I don't know if that solve reported issue; but it seems legit getUUIDRestrictCriteria to be used here. I don't know if this change should target 10.0/bf branch, no idea what could be the impact on an existing production environment.

If there is already some duplications on a GLPI instance, fixing this will not change much. Maybe it will permit to see that there is some duplicate data that has to be fixed manually.
If there is no duplication yet, then it should not change anything.

@eduardomozart
Copy link
Contributor

I can confirm this PR fixes #18921

@trasher trasher force-pushed the fix_uuid_rules_import_asset branch from dc6da28 to a902ab4 Compare February 6, 2025 07:26
@trasher trasher merged commit 860ba93 into glpi-project:10.0/bugfixes Feb 6, 2025
8 checks passed
@cedric-anne cedric-anne added this to the 10.0.18 milestone Feb 6, 2025
@cedric-anne cedric-anne added the bug label Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants