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

🐛 load flags via viper/cnquery providers #1508

Merged
merged 3 commits into from
Dec 4, 2024
Merged

Conversation

afiune
Copy link
Contributor

@afiune afiune commented Dec 4, 2024

Holy 🐮 cow! Are you ready for this fix!??

Based of mondoohq/cnquery#4863 where we now properly load provider flags via viper and from environment variables, we depend of the viper binding of such flags and, since cnspec did not have a PreRun() function but a PreRunE(), we did not load them.

This simple change fixes that issue, we now load the flags since we have the PreRun() function that is required to bind our flags at https://github.com/mondoohq/cnquery/blob/main/cli/providers/providers.go#L378-L399

This code is very similar to the code in cnquery at:

https://github.com/mondoohq/cnquery/blob/main/apps/cnquery/cmd/scan.go#L88

Based of mondoohq/cnquery#4863 where we now
properly load provider flags via `viper` and from environment variables,
we depend of the viper binding of such flags and, since `cnspec` did not
have a `PreRun()` function but a `PreRunE()`, we did not load them.

This simple change fixes that issue and we now load the flags since we
have the `PreRun()` function now, very similar to the code in `cnquery`
at:

https://github.com/mondoohq/cnquery/blob/main/apps/cnquery/cmd/scan.go#L88

Signed-off-by: Salim Afiune Maya <[email protected]>
Copy link
Contributor

github-actions bot commented Dec 4, 2024

Test Results

  1 files  ± 0   27 suites  +1   1m 3s ⏱️ +42s
508 tests +18  507 ✅ +18  1 💤 ±0  0 ❌ ±0 
509 runs  +18  508 ✅ +18  1 💤 ±0  0 ❌ ±0 

Results for commit 6f391cd. ± Comparison against base commit 7faffe7.

♻️ This comment has been updated with latest results.

@afiune afiune force-pushed the afiune/flags-not-loading branch from 8527f5b to 89915aa Compare December 4, 2024 22:12
Signed-off-by: Salim Afiune Maya <[email protected]>
@afiune afiune force-pushed the afiune/flags-not-loading branch from 89915aa to 75a1872 Compare December 4, 2024 22:15
Copy link
Member

@chris-rock chris-rock left a comment

Choose a reason for hiding this comment

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

Thank you @afiune

@afiune afiune force-pushed the afiune/flags-not-loading branch from b936e1b to fb201aa Compare December 4, 2024 23:29
Signed-off-by: Salim Afiune Maya <[email protected]>
@afiune afiune force-pushed the afiune/flags-not-loading branch from fb201aa to 6f391cd Compare December 4, 2024 23:41
@afiune afiune merged commit c50accf into main Dec 4, 2024
15 checks passed
@afiune afiune deleted the afiune/flags-not-loading branch December 4, 2024 23:45
@github-actions github-actions bot locked and limited conversation to collaborators Dec 4, 2024
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