-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add support for multiple vulnerability database repositories #7
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Benjamin Bordes <[email protected]>
DBRepository string `env:"SCANNER_TRIVY_DB_REPOSITORY" envDefault:"ghcr.io/aquasecurity/trivy-db"` | ||
JavaDBRepository string `env:"SCANNER_TRIVY_JAVA_DB_REPOSITORY" envDefault:"ghcr.io/aquasecurity/trivy-java-db"` | ||
DBRepository []string `env:"SCANNER_TRIVY_DB_REPOSITORY"` | ||
JavaDBRepository []string `env:"SCANNER_TRIVY_JAVA_DB_REPOSITORY"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I hardcode the default repositories? I was thinking using trivy's default might be better but if they change in future the README documentations would be wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
two days ago a new trivy version has been released: v0.57.1
The default URLs have changed:
aquasecurity/trivy#7679
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is indeed what I am referring to and the reason why I waited before creating this PR (and had it as a draft) because I needed to update the documentation.
I intend to bump up the adapter to v0.32.1 consuming trivy v0.57.1 IMO this change is probably not needed for v0.32.1, thoughts? @benji78 |
@@ -34,8 +34,8 @@ type Trivy struct { | |||
IgnorePolicy string `env:"SCANNER_TRIVY_IGNORE_POLICY"` | |||
SkipDBUpdate bool `env:"SCANNER_TRIVY_SKIP_UPDATE" envDefault:"false"` | |||
SkipJavaDBUpdate bool `env:"SCANNER_TRIVY_SKIP_JAVA_DB_UPDATE" envDefault:"false"` | |||
DBRepository string `env:"SCANNER_TRIVY_DB_REPOSITORY" envDefault:"ghcr.io/aquasecurity/trivy-db"` | |||
JavaDBRepository string `env:"SCANNER_TRIVY_JAVA_DB_REPOSITORY" envDefault:"ghcr.io/aquasecurity/trivy-java-db"` | |||
DBRepository []string `env:"SCANNER_TRIVY_DB_REPOSITORY"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can remain as a string and we just document the value should be comma separated URIs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could do that, but even though we are not changing or using the URIs separately right now, I believe it is clearer and more future proof to use an array of strings.
We could, for example, strip any spaces around the comma (may not be such a good practice though) or change from comma separated string to using one --db-repository
or --java-db-repository
per URI.
Trivy |
At the time of my previous pull request (#3), this scanner adapter only supported trivy
v0.54.1
. In trivyv0.56.0
support for multiple vulnerability database repositories was added.Here is how it can be used:
I also added these to the
config
test and updated thewrapper
test.This also includes the same README fix as #6, updates the default repositories and adds some info to
values.yaml
.I am just waiting for trivy to update the default registries to mark this PR as ready.