-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
feat(cli): error out when ignore file cannot be found #7624
feat(cli): error out when ignore file cannot be found #7624
Conversation
pkg/result/ignore.go
Outdated
@@ -184,6 +184,9 @@ func (c *IgnoreConfig) MatchLicense(licenseID, filePath string) *IgnoreFinding { | |||
func ParseIgnoreFile(ctx context.Context, ignoreFile string) (IgnoreConfig, error) { | |||
var conf IgnoreConfig | |||
if _, err := os.Stat(ignoreFile); errors.Is(err, fs.ErrNotExist) { | |||
if ignoreFile != DefaultIgnoreFile { | |||
return IgnoreConfig{}, xerrors.Errorf("%s does not exist", ignoreFile) |
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 we error out or just simply WARN
the user that their ignorefile doesn't exist? @knqyf263 WDYT?
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.
If I may, while a warning might be enough when a user calls trivy explicitly, in an automated context, unless there's some warning detection put in place, it might get missed and thus the scan output will not match expectation.
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.
Fair enough. It looks better to return an error.
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.
Thanks for your contribution. I left a small comment.
pkg/result/ignore.go
Outdated
@@ -184,6 +184,9 @@ func (c *IgnoreConfig) MatchLicense(licenseID, filePath string) *IgnoreFinding { | |||
func ParseIgnoreFile(ctx context.Context, ignoreFile string) (IgnoreConfig, error) { | |||
var conf IgnoreConfig | |||
if _, err := os.Stat(ignoreFile); errors.Is(err, fs.ErrNotExist) { | |||
if ignoreFile != DefaultIgnoreFile { |
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 check occurs at the end of scanning. We may want to fail on that at an earlier stage. Something like:
diff --git a/pkg/flag/report_flags.go b/pkg/flag/report_flags.go
index 67d553b65..d69443e89 100644
--- a/pkg/flag/report_flags.go
+++ b/pkg/flag/report_flags.go
@@ -6,6 +6,7 @@ import (
"github.com/mattn/go-shellwords"
"github.com/samber/lo"
+ "github.com/spf13/viper"
"golang.org/x/xerrors"
dbTypes "github.com/aquasecurity/trivy-db/pkg/types"
@@ -14,6 +15,7 @@ import (
"github.com/aquasecurity/trivy/pkg/log"
"github.com/aquasecurity/trivy/pkg/result"
"github.com/aquasecurity/trivy/pkg/types"
+ "github.com/aquasecurity/trivy/pkg/utils/fsutils"
xstrings "github.com/aquasecurity/trivy/pkg/x/strings"
)
@@ -238,6 +240,10 @@ func (f *ReportFlagGroup) ToOptions() (ReportOptions, error) {
}
}
+ if viper.IsSet(f.IgnoreFile.ConfigName) && !fsutils.FileExists(f.IgnoreFile.Value()) {
+ return ReportOptions{}, xerrors.Errorf("ignore file not found: %s", f.IgnoreFile.Value())
+ }
+
return ReportOptions{
Format: format,
ReportFormat: f.ReportFormat.Value(),
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.
Even better !
Thanks for improving my viper knowledge
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.
lgtm!
Description
This PR introduces a check for
--ignorefile
so that if the file is missing it will error out.This does not happen for the default ignore file as is currently already the case.
Related issues
--ignorefile
not found #7093Checklist