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

Add parser for aquasec trivy container vulnability scanner #542

Merged
merged 16 commits into from
Jan 3, 2021

Conversation

tofuatjava
Copy link
Contributor

No description provided.

@uhafner
Copy link
Member

uhafner commented Dec 20, 2020

Can you please have a look at the analysis issues first?

@codecov
Copy link

codecov bot commented Jan 1, 2021

Codecov Report

Merging #542 (afbd64d) into master (e1aeb67) will decrease coverage by 0.00%.
The diff coverage is 89.18%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #542      +/-   ##
============================================
- Coverage     90.15%   90.14%   -0.01%     
- Complexity     1573     1583      +10     
============================================
  Files           175      176       +1     
  Lines          4762     4799      +37     
  Branches        520      525       +5     
============================================
+ Hits           4293     4326      +33     
- Misses          297      299       +2     
- Partials        172      174       +2     
Impacted Files Coverage Δ Complexity Δ
...ava/edu/hm/hafner/analysis/parser/TrivyParser.java 89.18% <89.18%> (ø) 10.00 <10.00> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1aeb67...afbd64d. Read the comment docs.

@tofuatjava
Copy link
Contributor Author

now it looks well, a 93,1% coverage should be good enough
until this is merged I will go one with the Warnings-ng implementation, I have still troubles with compiling it, due to dependency issues

Copy link
Member

@uhafner uhafner 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 pull request!

Does it make sense to check for the correct file ending? Then please add a test like
https://github.com/jenkinsci/analysis-model/blob/master/src/test/java/edu/hm/hafner/analysis/parser/HadoLintParserTest.java#L50

(In this example you can also see a test with corrupt input)

@tofuatjava
Copy link
Contributor Author

tofuatjava commented Jan 3, 2021

Thanks for the review.
In my opinion, checking for file endings in this case is unnecessary.
The report need to be done by command line and could be named without any convention. In linux it is also not very common to give fileendings. But it is not clear what the better fileending would be ".json", ".jsn" or ".txt". there are no rules which specifies the fileending.
In worst case it makes no difference, because it is not garanteed that in a file with ".json" fileending a valid JSON is in it.

@tofuatjava tofuatjava requested a review from uhafner January 3, 2021 11:11
@tofuatjava
Copy link
Contributor Author

I do not understand why I will break the API. Please advice...

@uhafner
Copy link
Member

uhafner commented Jan 3, 2021

Can you please update the version in pom.xml to 9.5.0-SNAPSHOT. Adding a new parser requires to increase the minor version.

@tofuatjava
Copy link
Contributor Author

I'm wondering the version should be already 9.5.0-SNAPSHOT
revision is set to 9.5.0 and changelist is set to -SNAPSHOT
target artifact is build as analysis-model-9.5.0-SNAPSHOT

maybe i need to increase once again after the rebase. next try with 9.6.0-SNAPSHOT

@uhafner uhafner merged commit 9673029 into jenkinsci:master Jan 3, 2021
@uhafner uhafner added the feature New features label Jan 3, 2021
@uhafner uhafner changed the title additonal parser for aquasec trivy container vulnability scanner Add parser for aquasec trivy container vulnability scanner Jan 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants