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

Allows more customisation to audit-scanner #282

Conversation

nnelas
Copy link
Contributor

@nnelas nnelas commented Aug 24, 2023

Description

After using audit-scanner, deployed using Helm chart kubewarden-controller 1.6.0-rc2, I've noticed that there are a couple of parameters that are available in audit-scanner binary that unfortunately we can't use during the chart installation. This commit tries to improve that.

Additional Information

Potential improvement

Please let me know if you agree with this change or if this isn't at all aligned with the vision that you have for audit-scanner. Thank you!

After using audit-scanner, deployed using Helm chart `kubewarden-controller` 1.6.0-rc2, I've noticed that there are a couple of parameters that are available in `audit-scanner` binary that unfortunately we can't use during the chart installation. This commit tries to improve that.
@nnelas nnelas requested a review from a team as a code owner August 24, 2023 09:23
@flavio flavio added the kind/enhancement New feature or request label Aug 29, 2023
Copy link
Member

@flavio flavio left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution and sorry about the review delay. I left some comments, let's see what the other @kubewarden/kubewarden-developers think about it

charts/kubewarden-controller/chart-values.yaml Outdated Show resolved Hide resolved
charts/kubewarden-controller/templates/_helpers.tpl Outdated Show resolved Hide resolved
Copy link
Member

@viccuad viccuad left a comment

Choose a reason for hiding this comment

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

Thanks for the PR and looking into audit-scanner :).

I wonder though, it seems that the useful feature here would be to provide all log output in JSON, or an additional endpoint for audit-scanner.

What are you expecting from audit-scanner logs?
Is it obtaining all logs in JSON, to consume somewhere else?
We happily accept feat requests, maybe there's a better way to achieve this result.

So far we have made a conscious decision to format the log output as JSON so it could be consumed later on. But we aren't ensuring all output is JSON formatted, for example if the logLevel is raised from info. I think we should do so before we promise printing JSON to logs. With the proposed .Values.enableJsonReport , users would still need to sanitize the output if debug messages are not JSON.

@flavio flavio added the kind/question Further information is requested label Aug 31, 2023
@flavio
Copy link
Member

flavio commented Aug 31, 2023

Moving to blocked, waiting for more feedback

@viccuad
Copy link
Member

viccuad commented Aug 31, 2023

Sorry, realized now that you opened kubewarden/audit-scanner#102 for that feature request. Unblocking then!

@viccuad
Copy link
Member

viccuad commented Sep 5, 2023

This PR is blocked by kubewarden/audit-scanner#103.

- Removes customisation of the CA cert since this is generated by `kubewarden-controller` and there's no way to provide a custom one.
- Aligns `--print` flag with kubewarden/audit-scanner#103
@nnelas
Copy link
Contributor Author

nnelas commented Sep 6, 2023

hallo @flavio and @viccuad 👋 apologies for the delay in my response! I've pushed a new commit (c6096bd) which addresses your comments and aligns with --output-scan. looking at the available audit-scanner parameters, I don't think that there's anything else that may be worth exposing, but let me know if you think otherwise! 😄

flavio and others added 2 commits September 7, 2023 10:09
Co-authored-by: Víctor Cuadrado Juan <[email protected]>
Signed-off-by: Flavio Castelli <[email protected]>
Ensure the same comments are done inside of kubewarden-controller's
chart-values.yaml and values.yaml files.

Signed-off-by: Flavio Castelli <[email protected]>
Copy link
Member

@flavio flavio left a comment

Choose a reason for hiding this comment

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

Thanks for your submission!

@flavio flavio merged commit 6139c90 into kubewarden:main Sep 7, 2023
2 checks passed
@nnelas nnelas deleted the minor/allows_more_customisation_to_audit-scanner branch September 7, 2023 09:36
@viccuad
Copy link
Member

viccuad commented Sep 15, 2023

This has been released as appVersion 1.7.0-rc3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request kind/question Further information is requested
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants