-
-
Notifications
You must be signed in to change notification settings - Fork 1.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
aggregate goal seems to ignore reactor modules (always fetches from repo) in certain circumstances #740
Comments
Thanks for the bug report and clear example of the issue. |
@jeff303 can you tell me what build command you used? I'm getting a rats failure on |
In addition, I made a minor change to the ODC maven plugin, found in the issue_740_reactor branch. Not sure if you could test the branch while I'm trying to get a good build of data collector - to see if the change resolves the remote repo issue you reported. Also, I am wondering if there might be some questionable build orders in the pom.xml... Specifically, in the build.log you provided I'm not disagreeing that there is a bug in ODC with regards to using the local repo vs. remote repo - but I would expect to see the build.log provided from a clean system (such as a travis-ci build) - where the previously built snapshot of data-collector-common gets downloaded from the snapshot repo instead of using the one being built in the reactor. |
@jeremylong , apologies for the late reply. I have been fairly swamped this week. As soon as I get a chance, I will produce a build output log from our CI system (Jenkins), and also try the branch you mentioned. As fort he build order, that's essentially the crux of it. The |
After thinking about this a bit more and looking at the changes that were made to the aggregate goal - the main change that I believe is causing this issue is: @Mojo(
name = "aggregate",
defaultPhase = LifecyclePhase.VERIFY,
+ aggregator = true,
threadSafe = false,
requiresDependencyResolution = ResolutionScope.COMPILE_PLUS_RUNTIME,
requiresOnline = true
)
public class AggregateMojo extends BaseDependencyCheckMojo { The download is happening prior to invoking dependency-check which I'm pretty sure means it is not an issue with the plugin, but rather the build. I'm pretty sure you would see similar behavior using a noop aggregate mojo (i.e. an aggregate plugin that does nothing) that required compile and run time scoped dependencies to be resolved. Commons needs to be moved higher in the build order, etc. Just from trying to get the build to work I noticed the issue (see maven install hack). |
Coming back to this now... I tried updating the version we're using to
Is there a particular log line where I can see that the plugin is being invoked? I turned on debug logging (in the commands above) but wasn't clear on where that occurred. I saw these lines, but wasn't sure if they were it:
Can you have a quick look and see if the way we have even configured the plugin invocation makes sense? My impression is that there could be some problem there. We are putting the invocation inside a Thanks for your time and help! |
Yes, the three lines is the start of the execution of dependency-check in the build log. However, everything until you see the
As to the execution of the <build>
<plugins>
<plugin>
<groupId>org.owasp</groupId>
<artifactId>dependency-check-maven</artifactId>
<version>2.1.0</version>
<inherited>false</inherited>
<configuration>
<!-- skip non-bundled jars -->
<skipProvidedScope>true</skipProvidedScope>
<skipRuntimeScope>true</skipRuntimeScope>
<!-- We want HTML for easy viewing, but XML for reporting via SonarQube -->
<format>ALL</format>
<suppressionFile>${basedir}/../dependency-check-suppression.xml</suppressionFile>
</configuration>
<executions>
<execution>
<goals>
<goal>aggregate</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build> |
@jeremylong Tried applying your changes in issue_740_reactor onto current master. That doesn't solve the breaking build on missing dependencies from within the current reactor.
|
@jeremylong I wonder if there is any value in including reactor artifacts in the scanning for the aggregate goal. I would expect projects to KNOW whether or not their own composing parts contain known vulnerabilities. So I guess it would be OK to just ignore the ArtifactResolverException in BaseDependencyCheckMojo#collectDependencies IF the current dependencyNode.getArtifact() is an artifact with the coordinates of a project in the reactor. |
I have been running into the same issue. When running the Because the parent pom is built first and the plugin causes maven to try and resolve the (not yet built) submodules it ultimately crashes. |
We're having the same issue which appears to be directly related to how the dependency-check plugin gathers the dependencies list during a multi-module build. In a completely pristine environment there will be a new version number and none of the child modules will have been packaged, installed or deployed (using the Maven terminology for artifacts that have been placed in the reactor, local repository or remote repository). When building our micro-services, it's common for us to have three modules such as common, client and server with the common module containing code that's used by both the client and server (therefore the common artifact is a dependency in both the client and server). When the aggregate goal is run on the parent project, the dependency-check plugin tries to analyze the dependencies for the common module but since the parent is run first, the only place a POM with that version exists is in the common module's nested directory. It has not been added to the local or remote repositories. It's curious that the list of vulnerabilities that is output before the build fails looks correct. I haven't looked at how the plugin code works to determine if this is feasible but from the debug output it appears that automatically excluding the common module when scanning the client and server modules would fix the problem. Note that any vulnerable dependencies of the common module would still be reported by the aggregate function since it will still be considered as a standalone module. I'll see if I can alter one of our projects in this way and report back. Would a simple multi-module test project that exhibits this problem be helpful? I should also note that we've included the dependency-check plugin in our organizational POM so that all our projects are checked by default. Therefore setting |
Well ... ignore the conclusions I drew in the last comment. We have a lot of multi-module projects here and one of the managers asked why we weren't having this issue with all our multi-module projects. It turns out that (from looking at a small subset of the projects thus far) projects which include their modules in the parent's dependency management section don't seem to have this issue. Modules which manage simply include the artifact as another unmanaged dependency will break in the way described above. |
@jeremylong - I haven't completely confirmed your #740 (comment) above related to aggregator plugins but I do have what I believe is another interesting data-point. The projects where this is failing fail the same way when trying to run the
I'm going to go out on a limb and say that the error was in the shared Maven dependencies library. Forcing my You have a note in the
So I can understand that it's not quite as easy as just changing the version. |
@jeremylong From a quick look it seems that the remark in the parent POM was outdated. I needed to make only a minor change to the current logic to get a passing build of dependency-check. Will do some testing with https://github.com/aikebah/DependencyCheck/tree/upgrade-dependency-tree-version and if it appears stable to me will make a PR for it. Will also try to evaluate whether the upgrade would solve this issue. |
First results of my testing: needs more work... [ERROR] Failed to execute goal org.owasp:dependency-check-maven:3.1.2-SNAPSHOT:check (default-cli) on project api_portal: Execution default-cli of goal org.owasp:dependency-check-maven:3.1.2-SNAPSHOT:check failed.: NullPointerException -> [Help 1] |
Updated branch now appears to be stable, so I'll make a PR for the update to the current dependency-tree shared module of Maven.... however it does not solve this issue. |
All - I have a patch for most of this issue. However, I am betting those affected by this will say that the patch is not correct as in some cases the dependency will still be pulled from the repo instead of using the dependency that will be built later in the reactor - and the reason behind this is the above referenced |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
There seems to be some issue with dependency-check-maven where, when using the
aggregate
type output, the plugin is downloading all dependencies from the repositories instead of using the local build versions via Reactor. This started happening somewhere between version 1.4.0 and 1.4.5, since we did not experience this problem in 1.4.0. And this forum post suggests a large part of this was rewritten within that range.Full build log available here. Relevant snippets pasted below. Notice the
Downloading
part near the end. This should not be happening, since this is a local module, so it should be built first, and its local artifacts used in the analysis.To reproduce, check out
streamsets/datacollector
and reverse this commit (in other words, turning the report back toaggregate
instead ofcheck
). Observe that the download above does not happen. Alternately, change the plugin version from1.4.5
back to1.4.0
inroot-proto/pom.xml
, leavingaggregate
output, and observe the downloads do not happen in that circumstance either.Any further info is available as needed. Thank you!
The text was updated successfully, but these errors were encountered: