🧹 use 1 global resource schema instead of per runtime

[imilchev]

Use 1 schema for the coordinator instead of creating copies for every runtime we start. That should improve our memory footprint significantly for large scans.

Scanning a cluster with 366 assets with cnspec 10.3.0:

Scanning the same cluster with the changes from this branch:

this builds on top of #3308

[github-actions]

Test Results

2 751 tests  ±0   2 750 ✅ ±0   49s ⏱️ -1s
  186 suites ±0       1 💤 ±0 
    5 files   ±0       0 ❌ ±0 

Results for commit f2fcff1. ± Comparison against base commit cf4fad5.

This pull request removes 6 and adds 6 tests. Note that renamed tests count towards both.

go.mondoo.com/cnquery/v10/llx ‑ TestRawData_JSON/0001-01-01_00:00:00_+0000_UTC
go.mondoo.com/cnquery/v10/llx ‑ TestRawData_JSON/292277026596-12-04_15:30:07_+0000_UTC
go.mondoo.com/cnquery/v10/llx ‑ TestRawData_JSON/292277026596-12-04_15:30:09_+0000_UTC
go.mondoo.com/cnquery/v10/llx ‑ TestSuccess/2024-02-14_11:56:07.030723245_+0000_UTC_m=+0.011946430
go.mondoo.com/cnquery/v10/llx ‑ TestTruthy/2024-02-14_11:56:07.030723245_+0000_UTC_m=+0.011946430
go.mondoo.com/cnquery/v10/llx ‑ TestTruthy/2024-02-14_11:56:07.030723245_+0000_UTC_m=+0.011946430#01

go.mondoo.com/cnquery/v10/llx ‑ TestRawData_JSON/0001-01-01_00:53:28_+0053_LMT
go.mondoo.com/cnquery/v10/llx ‑ TestRawData_JSON/292277026596-12-04_16:23:37_+0053_LMT
go.mondoo.com/cnquery/v10/llx ‑ TestRawData_JSON/292277026596-12-04_16:30:07_+0100_CET
go.mondoo.com/cnquery/v10/llx ‑ TestSuccess/2024-02-16_18:44:25.556046445_+0100_CET_m=+0.007813243
go.mondoo.com/cnquery/v10/llx ‑ TestTruthy/2024-02-16_18:44:25.556046445_+0100_CET_m=+0.007813243
go.mondoo.com/cnquery/v10/llx ‑ TestTruthy/2024-02-16_18:44:25.556046445_+0100_CET_m=+0.007813243#01

♻️ This comment has been updated with latest results.

[imilchev]

we cannot really do this with a global schema. It seems like before we were prioritizing the IDs with the specific provider for the asset coming in first. I am not sure if that would actually break anything. @arlimus not sure if you recall why we needed that

[arlimus]

Yeah this one is going to break things if we don't keep it around...

The reason is that some keywords are equally implemented in two different providers (like vulnmgmt in os and vsphere) to provide the appropriate level of functionality in either, without having to learn new keywords. It is functionally correct, but it forces us to detect the right provider depending on the current prioritization...

(Long-term this is not a problem, because all keywords will be anchored to a parent, which entirely removes the guessword, but we can't force this into v10 right now...)

There are a few ways to handle this. First we need the list of resource definitions that have the same identifier. Once we have those, we can prioritize when we look up the resource/field based on the current connection.

(a) We can introduce a new field into ResourceInfo that we can later easily remove (once everything is anchored to parents). We can't modify any part of ResourceInfo because it may be different between providers (which is ok). We could however add another field to it to chain things for now: ResourceInfo.Other which is either nil if no other provider offers it or is the next available info.

(b) The alternative is to change the schema lookup from a simple map[string]ResourceInfo into a map[string][]ResourceInfo. It requires a bit more work and rewrite.

[imilchev]

I found 2 cases around this:

• 1 resource that can be defined in multiple providers (not extending, just having the same resource in multiple places)
• resource extensions where the root is in 1 place and the extensions are potentially in multiple providers

To cover both cases I picked approach (a) and added Others field that stores all other definitions of the ResourceInfo. However, I had to add the same Others field to the Field struct, because fields can also exist in multiple providers. Just like the vulnmgmt field that exists in the vsphere and os providers.

I think both cases work now. I still need to clean up the code a bit and will definitely add tests for both cases. Took me quite a bit of time to understand how this should work...

[arlimus]

Saw that you added others to cover fields as well, that's awesome!

[arlimus]

nit: would you mind adding a comment above this and the others field below:

// This field contains references to other providers with the same resource/field.
// Note: Please do not use this field, it is only temporary and will be removed
// in the future once binding resources are mandatory for all executions.

(if this merges first no worries, we can add it later)

[arlimus]

nit:

This code will read all schemas that are installed on the system by default on startup. For regular use of scan/run/shell I had originally avoided this, so that we only load schemas we need and dynamically load additional schemas (making startup more lightweight).

Is it necessary to load all schemas for this change?
If that is the case and we require them here, would you mind adding a comment so that we change this back to dynamic loading later on (I'm sure I'll forget otherwise 🙈 ) (something like FIXME: dynamically load providers on startup).

[arlimus]

Yoho @imilchev , great updates!! 😁

This is super close now, enough for me to +1 from my side.
I left two comment and marked them as nits so that this PR isn't blocked :) Feel free to get another approval to finish this in case I'm not around (I'm back Tuesday morning PST time, which is a bit late)

[imilchev]

Fixed the comments. Will merge this into #3308 now