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

bug(cli): Don't error out on empty trivy config yaml file #7934

Closed
simar7 opened this issue Nov 16, 2024 · 4 comments · Fixed by #7962
Closed

bug(cli): Don't error out on empty trivy config yaml file #7934

simar7 opened this issue Nov 16, 2024 · 4 comments · Fixed by #7962
Assignees
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.
Milestone

Comments

@simar7
Copy link
Member

simar7 commented Nov 16, 2024

We should handle empty trivy config yaml files more gracefully.

$ touch invalid-file.yaml
$ ./trivy config --ignorefile ./invalid-file.yaml  /tmp
2024-11-01T16:15:15-06:00       INFO    [misconfig] Misconfiguration scanning is enabled
2024-11-01T16:15:15-06:00       INFO    Detected config files   num=0
2024-11-01T16:15:15-06:00       FATAL   Fatal error     filter error: filtering error: ./invalid-file.yaml error: ./invalid-file.yaml parse error: yaml decode error: EOF

Originally posted by @EdKingscote in #7856 (comment)

@simar7 simar7 self-assigned this Nov 16, 2024
@simar7 simar7 added this to the v0.58.0 milestone Nov 16, 2024
@simar7 simar7 added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Nov 16, 2024
@itaysk
Copy link
Contributor

itaysk commented Nov 18, 2024

small nit but this should be either a bug or a feature

@simar7 simar7 changed the title fix(cli): Don't error out on empty trivy config yaml file feat(cli): Don't error out on empty trivy config yaml file Nov 18, 2024
@simar7 simar7 changed the title feat(cli): Don't error out on empty trivy config yaml file bug(cli): Don't error out on empty trivy config yaml file Nov 18, 2024
@simar7
Copy link
Member Author

simar7 commented Nov 18, 2024

small nit but this should be either a bug or a feature

You're right. Since empty YAML files are technically valid YAML, this is a bug fix. I've updated the title.

@knqyf263
Copy link
Collaborator

knqyf263 commented Nov 20, 2024

If an empty file is a valid YAML, IMO, the YAML library should not return an error. Then, I found this issue.
go-yaml/yaml#805

I still feel like Decode should work in the same way as Unmarshal, but if that is the decision in the library, we may want to change it to Unmarshal here to work it around.

if err = yaml.NewDecoder(f).Decode(&ignoreConfig); err != nil {

@simar7
Copy link
Member Author

simar7 commented Nov 20, 2024

If an empty file is a valid YAML, IMO, the YAML library should not return an error. Then, I found this issue. go-yaml/yaml#805

I still feel like Decode should work in the same way as Unmarshal, but if that is the decision in the library, we may want to change it to Unmarshal here to work it around.

if err = yaml.NewDecoder(f).Decode(&ignoreConfig); err != nil {

Yes, I also observed that but I thought I was doing something wrong as I didn't find the issue you linked.

But I agree, in theory the two methods should behave the same. I've updated the PR for now to use Unmarshal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

3 participants