Skip to content

Comments

Update dependency-check plugin#10883

Merged
suneet-s merged 2 commits intoapache:masterfrom
suneet-s:owasp
Feb 17, 2021
Merged

Update dependency-check plugin#10883
suneet-s merged 2 commits intoapache:masterfrom
suneet-s:owasp

Conversation

@suneet-s
Copy link
Contributor

@suneet-s suneet-s commented Feb 11, 2021

Description

This PR aims to make the dependency-check job less flaky. It addresses intermittent failures with error messages like

Failed to request component-reports

This was inspired by the workaround suggested in jeremylong/DependencyCheck#1908
Local testing appears to support the workaround mentioned in this issue.

The drawback of this approach is that the job will no longer produce a dependency vulnerability report - so if the job fails, devs need to run mvn dependency-check:check locally to see which project the vulnerability is reported from.

This PR also updates the version to the latest. The upgrade from 5.x to 6.x is a breaking change for some use cases, but AFAICT nothing was needed as part of the version upgrade.


This PR has:

  • been self-reviewed.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.

@suneet-s suneet-s added the Area - Dev For items related to the project itself, like dev docs and checklists, but not CI label Feb 11, 2021
Copy link
Contributor

@capistrant capistrant left a comment

Choose a reason for hiding this comment

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

I'm cool with this. It will be interesting to see how the lack of reporting in CI itself will effect the mood of devs when they realize they need to generate the report locally and then share results :)

but I think it is acceptable trade-off for increased stability of CI

@suneet-s suneet-s merged commit bc70040 into apache:master Feb 17, 2021
@suneet-s suneet-s deleted the owasp branch February 17, 2021 03:22
@suneet-s
Copy link
Contributor Author

I didn't find this too bad when I was looking for failures. Hopefully we have to look at these warnings a lot less frequently now if it's less flaky :)

@suneet-s
Copy link
Contributor Author

So it turns out I was wrong. It was hard to know that an actual CVE was flagged because this job had been flaky in the past. I'll create a follow up PR with a better error message so that it makes it more obvious what should be done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area - Dev For items related to the project itself, like dev docs and checklists, but not CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants