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

✨ Set asset.kind to virtualmachine when in cloud #5267

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

afiune
Copy link
Contributor

@afiune afiune commented Feb 26, 2025

First pass to set and consolidate asset.kind to virtualmachine when
running in the cloud.

This needs testing and some backend changes.

Copy link
Contributor

github-actions bot commented Feb 26, 2025

Test Results

0 tests   - 3 375   0 ✅  - 3 371   0s ⏱️ - 1m 54s
0 suites  -   392   0 💤  -     4 
0 files    -    30   0 ❌ ±    0 

Results for commit c7c1173. ± Comparison against base commit d955ff4.

♻️ This comment has been updated with latest results.

@afiune afiune force-pushed the afiune/asset.kind/virtual_machine branch from 72cfe53 to 25e7f47 Compare February 26, 2025 01:04
@@ -29,16 +29,25 @@ var detectors = []detectorFunc{
gcp.Detect,
}

const AssetKind = "virtualmachine"
Copy link
Member

Choose a reason for hiding this comment

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

Lets find a better name for this

Copy link
Member

Choose a reason for hiding this comment

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

The shared kinds probably belong into the provider sdk

Copy link
Member

Choose a reason for hiding this comment

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

lets move it into platform.go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't put it there since we will have a cyclical dependency, but I think it would be a great idea to move these constants (for the near future) into the actual inventory package.

I think we will benefit from using them in the backend.

type detectResult struct {
platformId string
platformName string
relatedPlatformIds []string
}

func Detect(conn shared.Connection, p *inventory.Platform) (PlatformID, PlatformName, []RelatedPlatformID) {
type PlatformInfo struct {
Copy link
Member

Choose a reason for hiding this comment

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

nit: add comments

mgr, err := smbios.ResolveManager(conn, p)
if err != nil {
return "", "", nil
return PlatformInfo{"", "", "", nil}
Copy link
Member

Choose a reason for hiding this comment

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

Should this return nil? But we need to check the error handling on the other side.

@afiune afiune marked this pull request as ready for review February 27, 2025 19:45
@afiune afiune force-pushed the afiune/asset.kind/virtual_machine branch from eff89a2 to 55c17ea Compare February 27, 2025 19:46
@afiune afiune mentioned this pull request Mar 3, 2025
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.

2 participants