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

⭐ show assessments inside of blocks #5247

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

arlimus
Copy link
Member

@arlimus arlimus commented Feb 20, 2025

This has been a very long time coming. Despite being an important step forward, it's also only just the first step in terms of the code changes that will be necessary to clean this up long-term.

So, ...

What is this?

Currently if we run this:

> [1,2,3].containsOnly([1,2])
[failed] [].containsOnly()
  expected: == []
  actual:   [
    0: 3
  ]

It shows us what's up.

But what if we put it inside a block?

> mondoo { [1,2,3].containsOnly([1,2]) }
mondoo: {
  [].containsOnly(): false
}

Darn, all the useful output just disappeared.

This PR make it so that instead we get:

> mondoo { [1,2,3].containsOnly([1,2]) }
mondoo: {
  [failed] [].containsOnly()
    expected: == []
    actual:   [
      0: 3
    ]
}

This example is a bit constructed, but works just as well with naturally occurring blocks like:

TODO

Fixes #5246

How does it work?

This is where things get a bit tricky, because this PR is only a first step to solve the problem, but ultimately needs follow-ups to increase clarity.

1: Expand Code Bundles

By default, code bundles don't need to translate from CodeIDs into refs. This only ever becomes useful when we print things and deal with CodeIDs and try to go back to the compiled code (CodeBundle) and figure out how best to render it. We add a type that is only used for printing so we don't need to extend the CodeBundle for this task (no foreseeable future where we'd ever want to transmit this data, ie not part of proto).

2: Start to building assessments inside the data printer

Printing is currently split up in an assessment and a data-printing phase. If something is handled in the assessment phase it will be printed as an assessment and that's it. If not, it will be printed as data.

When we receive an outer block, it switches into data mode. However, the block may contain assertions, that are then ignored and never collected nor printed. This PR changes this behavior and starts to look for assertions inside of blocks.

3. Start to collect datapoints inside of blocks

An old piece of code had previously removed this:

	// BUG (jaym): collectRefDatapoints prevents us from collecting datapoints.
	// Collecting datapoints for blocks didn't work correctly until 6.7.0.
	// See https://gitlab.com/mondoolabs/mondoo/-/merge_requests/2639
	// We can fix this after some time has passed. If we fix it too soon
	// people will start having their queries fail if a falsy datapoint
	// is collected.

Looks like we are far past that point, so let's start collecting.

@arlimus arlimus requested a review from jaym February 20, 2025 09:26
Signed-off-by: Dominik Richter <dominik.richter@gmail.com>
@arlimus arlimus force-pushed the dom/fix-block-assessment branch from c93490e to d2ca84d Compare February 20, 2025 09:28
Copy link
Contributor

Test Results

  1 files   -    29  101 suites   - 290   45s ⏱️ -53s
917 tests  - 2 433  907 ✅  - 2 439  1 💤  - 3  9 ❌ +9 
916 runs   - 2 434  906 ✅  - 2 440  1 💤  - 3  9 ❌ +9 

For more details on these failures, see this check.

Results for commit d2ca84d. ± Comparison against base commit 53a692f.

This pull request removes 2434 and adds 1 tests. Note that renamed tests count towards both.
go.mondoo.com/cnquery/v11/cli/printer ‑ TestPrinter
go.mondoo.com/cnquery/v11/cli/printer ‑ TestPrinter/['1-2']_{__.split('-')_}
go.mondoo.com/cnquery/v11/cli/printer ‑ TestPrinter/[1].where(___>_0_)
go.mondoo.com/cnquery/v11/cli/printer ‑ TestPrinter/[]
go.mondoo.com/cnquery/v11/cli/printer ‑ TestPrinter/a_=_3__if(true)_{__a_==_3__}
go.mondoo.com/cnquery/v11/cli/printer ‑ TestPrinter/file('zzz')_{_content_}
go.mondoo.com/cnquery/v11/cli/printer ‑ TestPrinter/if_(_mondoo.version_!=_null_)_{_mondoo.build_}
go.mondoo.com/cnquery/v11/cli/printer ‑ TestPrinter/mondoo
go.mondoo.com/cnquery/v11/cli/printer ‑ TestPrinter/mondoo_{__.version_}
go.mondoo.com/cnquery/v11/cli/printer ‑ TestPrinter/mondoo_{_version_}
…
TestMain

@scottford-io
Copy link
Contributor

@arlimus I am looking to use this in a policy check using props. The check will be an array of over 50 items. Are there any concerns here with the expected: == [] output? 🤔

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.

Improve Readability of .containsNone Function Output in Terraform Checks
2 participants