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

Merged
merged 9 commits into from
Mar 5, 2025

Conversation

afiune
Copy link
Contributor

@afiune afiune commented Feb 26, 2025

This change consolidates asset.kind for virtualmachine and baremetal.

I am proposing having a single file where we define these asset kinds so that we
can later use them in the backend (follow up PR).

We are also changing the kind of an asset if:

  • We are running an OS scan
  • The scan is happening in a cloud instance

How to test this change

  • Build cnquery binary
  • Upload it and open a shell on a cloud instance (ec2, google vm, azure vm) (cnquery shell)
  • Check that the asset.kind comes back as virtualmachine

Alternatively, build the binary locally and connect to an aws instance via:

cnquery shell aws --discover instances

Select and asset and check that asset.kind comes back as virtualmachine

Copy link
Contributor

github-actions bot commented Feb 26, 2025

Test Results

3 478 tests  ±0   3 474 ✅ ±0   2m 5s ⏱️ +23s
  392 suites ±0       4 💤 ±0 
   30 files   ±0       0 ❌ ±0 

Results for commit f3be8e2. ± Comparison against base commit 293a023.

♻️ 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
@@ -14,7 +16,7 @@ func GetPlatformForObject(platformName string, accountId string) *inventory.Plat
return &inventory.Platform{
Name: platformName,
Title: getTitleForPlatformName(platformName),
Kind: "aws-object",
Kind: getPlatformKind(platformName),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This needs to be reverted. We only want to label os scans.

@vjeffrey vjeffrey merged commit 848bd62 into main Mar 5, 2025
17 checks passed
@vjeffrey vjeffrey deleted the afiune/asset.kind/virtual_machine branch March 5, 2025 16:54
@github-actions github-actions bot locked and limited conversation to collaborators Mar 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants