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

docs: update info about config file #6547

Merged
merged 5 commits into from
Apr 25, 2024

Conversation

DmitriyLewen
Copy link
Contributor

Description

config file page missed some flags and contains mistakes.

Update this page.

Related discussions

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

@DmitriyLewen DmitriyLewen self-assigned this Apr 24, 2024
@@ -81,6 +81,15 @@ severity:
- MEDIUM
- HIGH
- CRITICAL

scan:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use scan:

ConfigName: "scan.compliance",

But looks like it should be report.
@knqyf263 wdyt?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, agree. Also, --show-suppressed should be under report.

ConfigName: "scan.show-suppressed",

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Apr 25, 2024

Choose a reason for hiding this comment

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

hm... there is 1 problem:
We already have report:

ConfigName: "report",

We can add report prefix for all report flags (I mean report.format etc).
But in this case we will have report.report.
@knqyf263 do you have another thought?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Flags frequently used can be global without the report prefix.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We already have report:

Ah, I missed it. Hmm... it's a problem...

Comment on lines 489 to 507
all:
namespaces: false

# Same as '--k8s-version'
# Default is empty
k8s:
version: 1.21.0

# Same as '--node-collector-imageref'
# Default is 'ghcr.io/aquasecurity/node-collector:0.0.9'
node:
collector:
imageref: ghcr.io/aquasecurity/node-collector:0.0.9

# Same as '--node-collector-namespace'
# Default is 'trivy-temp'
node:
collector:
namespace: ~/.kube/config2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chen-keinan can you take a look? Looks like we don't need nested structs for these flags. Perhaps we can use - instead of ..

e.g. for --node-collector-imageref:

NodeCollectorImageRef = Flag[string]{
Name: "node-collector-imageref",
ConfigName: "kubernetes.node.collector.imageref",
Default: "ghcr.io/aquasecurity/node-collector:0.0.9",
Usage: "indicate the image reference for the node-collector scan job",
}

we can use kubernetes.node-collector-imageref and get :

kubernetes 
 node-collector-imageref: ghcr.io/aquasecurity/node-collector:0.0.9

Copy link
Contributor

Choose a reason for hiding this comment

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

@DmitriyLewen sound good. you can update this flag as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chen-keinan take a looks, please - 15e6fca

@DmitriyLewen DmitriyLewen marked this pull request as ready for review April 24, 2024 08:03
Co-authored-by: simar7 <1254783+simar7@users.noreply.github.com>
Copy link
Collaborator

@knqyf263 knqyf263 left a comment

Choose a reason for hiding this comment

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

There are some considerations, but let's merge this PR now and keep improving our configuration.

@knqyf263 knqyf263 added this pull request to the merge queue Apr 25, 2024
Merged via the queue into aquasecurity:main with commit 7811ad0 Apr 25, 2024
17 checks passed
@DmitriyLewen DmitriyLewen deleted the docs/config-file branch April 26, 2024 01:01
fl0pp5 pushed a commit to altlinux/trivy that referenced this pull request May 6, 2024
Co-authored-by: simar7 <1254783+simar7@users.noreply.github.com>
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.

4 participants