Skip to content

copy entity to user & hosts structs#3791

Merged
chennn1990 merged 8 commits intomainfrom
nested-entity-inside-user-host
Dec 31, 2025
Merged

copy entity to user & hosts structs#3791
chennn1990 merged 8 commits intomainfrom
nested-entity-inside-user-host

Conversation

@chennn1990
Copy link
Copy Markdown
Contributor

@chennn1990 chennn1990 commented Dec 24, 2025

Summary of your changes

Asset Inventory change:
create a nested hierarchy inside User & Host so the entity will be appeared also there instead of a sibling.

Related Issues

Checklist

  • I have added tests that prove my fix is effective or that my feature works

@chennn1990 chennn1990 requested a review from a team as a code owner December 24, 2025 12:11
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Dec 24, 2025

This pull request does not have a backport label. Could you fix it @chennn1990? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit
  • backport-active-all is the label that automatically backports to all active branches.
  • backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
  • backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

Copy link
Copy Markdown
Collaborator

@uri-weisman uri-weisman left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 A few notes:

  1. Let’s validate this on GCP and Azure before merging.
  2. We should open a ticket to remove the redundant entity.* fields for host and user entities once we pivot to user|host.entity.id. This can be tracked as an action item under elastic/kibana#233603.
  3. There are currently no mappings for these fields. we’ll need to add them in the integrations repo:
  4. Please don't backport to 9.3

Name: vmProperties.Extended.InstanceView.ComputerName,
Type: vmProperties.HardwareProfile.VmSize,
}
asset.Host.Entity = &asset.Entity
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not necessary related to your changes, but I wonder why we don't use the inventory.WithHost utility here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Seems like a bad practice in this single place.
Do we have a way to enforce those utilities?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not with linting or something like that, maybe unit tests can help enforce it, but I'd prefer the tests to be focus on a single class and not on the utilities.

@chennn1990 chennn1990 enabled auto-merge (squash) December 31, 2025 10:43
@chennn1990 chennn1990 merged commit 063df03 into main Dec 31, 2025
10 checks passed
@chennn1990 chennn1990 deleted the nested-entity-inside-user-host branch December 31, 2025 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Asset Discovery] Migrate to new nested *.entity.* schema

2 participants