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

aggregate goal seems to ignore reactor modules (always fetches from repo) in certain circumstances #740

Closed
jeff303 opened this issue May 22, 2017 · 18 comments
Labels

Comments

@jeff303
Copy link

jeff303 commented May 22, 2017

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.

Apache Maven 3.3.9 (bb52d8502b132ec0a5a3f4c09453c07478323dc5; 2015-11-10T10:41:47-06:00)
Maven home: /usr/local/Cellar/maven/3.3.9/libexec
Java version: 1.8.0_111, vendor: Oracle Corporation
Java home: /Library/Java/JavaVirtualMachines/jdk1.8.0_111.jdk/Contents/Home/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "mac os x", version: "10.12.4", arch: "x86_64", family: "mac"
...<snip>...
[INFO] Reactor Build Order:
[INFO]
[INFO] StreamSets Data Collector RB Gen Maven Plugin
[INFO] StreamSets Data Collector
[INFO] StreamSets Data Collector Root Prototype POM
[INFO] StreamSets Data Collector Root POM
[INFO] StreamSets Testing Library
[INFO] StreamSets Data Collector Bootstrap
[INFO] StreamSets Utils
[INFO] StreamSets SSO
[INFO] StreamSets Data Collector Common
...<snip>...
...NOTE: this is where the undesired download happens.  It should be using the local Maven module...
[DEBUG] Using connector BasicRepositoryConnector with priority 0.0 for https://repo.streamsets.net/artifactory/libs-snapshot
Downloading: https://repo.streamsets.net/artifactory/libs-snapshot/com/streamsets/streamsets-datacollector-common/2.6.0.0-SNAPSHOT/streamsets-datacollector-common-2.6.0.0-20170501.145725-107.jar

To reproduce, check out streamsets/datacollector and reverse this commit (in other words, turning the report back to aggregate instead of check). Observe that the download above does not happen. Alternately, change the plugin version from 1.4.5 back to 1.4.0 in root-proto/pom.xml, leaving aggregate output, and observe the downloads do not happen in that circumstance either.

Any further info is available as needed. Thank you!

@jeremylong jeremylong added the bug label May 28, 2017
@jeremylong
Copy link
Owner

Thanks for the bug report and clear example of the issue.

@jeremylong
Copy link
Owner

@jeff303 can you tell me what build command you used? I'm getting a rats failure on mvn install.

@jeremylong
Copy link
Owner

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 StreamSets Data Collector Common isn't being built until line 139471. Yet it appears that the referenced download is occurring while building Building StreamSets Data Collector 2.6.0.0-SNAPSHOT (starting on line 2009).

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.

@jeff303
Copy link
Author

jeff303 commented Jun 8, 2017

@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 StreamSets Data Collector module, through a series of dependencies, depends on StreamSets Data Collector Common. Therefore, when building the entire project locally, it should build the local version of the latter and use that as the dependency for the first. Something that is seemingly triggered from dependency check is causing Maven to download StreamSets Data Collector Common from the repo. When switching away from aggregate report (back to check), this download does not occur. I will produce another build output log showing that scenario so maybe we can tell what's happening by looking at the difference.

@jeremylong
Copy link
Owner

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).

@jeff303
Copy link
Author

jeff303 commented Aug 11, 2017

Coming back to this now... I tried updating the version we're using to 2.1.0 (which I believe is currently the latest stable), and am seeing the same behavior. I also am able to trigger it with the verify goal rather than using install. So, the following steps can trigger the behavior:

# modify root-proto/pom.xml to switch to aggregate goal instead of check
# remove all local streamsets artifacts
rm -rf ~/.m2/repository/com/streamsets/

mvn clean verify -X -e >/tmp/build.out 2>/tmp/build.err

# examine /tmp/build.out, observe that (for instance) streamsets-datacollector-common.*jar is being downloaded

# modify root-proto/pom.xml to switch back to check instead of aggregate

# again remove all local streamsets artifacts
rm -rf ~/.m2/repository/com/streamsets/

mvn clean verify -X -e >/tmp/build.out 2>/tmp/build.err

# examine /tmp/build.out, observe that streamsets-common*jar is not being downloaded

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:

[DEBUG] Goal:          org.owasp:dependency-check-maven:2.1.0:aggregate (default)
[DEBUG] Style:         Aggregating
[DEBUG] Configuration: <?xml version="1.0" encoding="UTF-8"?>

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 pluginManagment section of a "prototype" POM, which many other separate modules end up inheriting from. To me, that doesn't quite seem right with respect to aggregation (i.e. running the aggregate goal from a series of individual modules based on inheriting that invocation from a parent pom). Should the aggregate instead be done on the project root itself, the one with all the modules? Note also that we are skipping provided and runtime scopes via the configuration.

Thanks for your time and help!

@jeremylong
Copy link
Owner

Yes, the three lines is the start of the execution of dependency-check in the build log. However, everything until you see the Properties loaded: statement Is core Maven preparing for the execution of the plugin based on the configuration.

[DEBUG] Goal:          org.owasp:dependency-check-maven:2.1.0:aggregate (default)
[DEBUG] Style:         Aggregating
[DEBUG] Configuration: <?xml version="1.0" encoding="UTF-8"?>

---removed log entries for brevity

[DEBUG] -- end configuration --
[DEBUG] Properties loaded:

As to the execution of the dependency-check-maven:aggregate my suggestion would be to place this in the parent pom with inherited set to false:

<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>

@aikebah
Copy link
Collaborator

aikebah commented Aug 30, 2017

@jeremylong
It looks like due to 37b9f49 this issue started breaking our builds (for pristine multimodule-projects that have inter-module dependencies such as when building those projects in the context of a mvn release:prepare (which is where it surfaced for us after updating DependencyCheck from 2.0.0 to 2.1.0)).

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.

abu:multimodule-sample aikebah$ mvn clean verify
[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Build Order:
[INFO] 
[INFO] multimodule-sample
[INFO] sampleChild1
[INFO] sampleChild2
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] Building multimodule-sample 0.1-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO] 
[INFO] --- maven-clean-plugin:2.5:clean (default-clean) @ multimodule-sample ---
[INFO] Deleting /Volumes/LaCie/Gitlab/multimodule-sample/target
Downloading: https://*****/nexus/content/groups/public/net/aikebah/dependencycheck/sampleChild1/0.1-SNAPSHOT/maven-metadata.xml
Downloading: https://*****/nexus/content/groups/public/net/aikebah/dependencycheck/sampleChild1/0.1-SNAPSHOT/sampleChild1-0.1-SNAPSHOT.jar
[WARNING] The following dependencies could not be resolved at this point of the build but seem to be part of the reactor:
[WARNING] o net.aikebah.dependencycheck:sampleChild1:jar:0.1-SNAPSHOT (compile)
[WARNING] Try running the build up to the lifecycle phase "package"
[INFO] 
[INFO] --- dependency-check-maven:2.1.2-SNAPSHOT:aggregate (default) @ multimodule-sample ---
[INFO] Checking for updates
[INFO] Skipping NVD check since last check was within 4 hours.
[INFO] Check for updates complete (5 ms)
[INFO] Analysis Started
[INFO] Finished File Name Analyzer (0 seconds)
[INFO] Finished Central Analyzer (0 seconds)
[INFO] Finished Dependency Merging Analyzer (0 seconds)
[INFO] Finished Version Filter Analyzer (0 seconds)
[INFO] Finished Hint Analyzer (0 seconds)
[INFO] Created CPE Index (1 seconds)
[INFO] Finished CPE Analyzer (1 seconds)
[INFO] Finished False Positive Analyzer (0 seconds)
[INFO] Finished Cpe Suppression Analyzer (0 seconds)
[INFO] Finished NVD CVE Analyzer (0 seconds)
[INFO] Finished Vulnerability Suppression Analyzer (0 seconds)
[INFO] Finished Dependency Bundling Analyzer (0 seconds)
[INFO] Analysis Complete (1 seconds)
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO] 
[INFO] multimodule-sample ................................. FAILURE [  3.321 s]
[INFO] sampleChild1 ....................................... SKIPPED
[INFO] sampleChild2 ....................................... SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 3.625 s
[INFO] Finished at: 2017-08-30T19:51:03+02:00
[INFO] Final Memory: 62M/1138M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.owasp:dependency-check-maven:2.1.2-SNAPSHOT:aggregate (default) on project multimodule-sample: One or more exceptions occurred during dependency-check analysis: One or more exceptions occurred during analysis:
[ERROR] 	Failure to find net.aikebah.dependencycheck:sampleChild1:jar:0.1-SNAPSHOT in https://*****/nexus/content/groups/public/ was cached in the local repository, resolution will not be reattempted until the update interval of nexus has elapsed or updates are forced
[ERROR] 	Failure to find net.aikebah.dependencycheck:sampleChild1:jar:0.1-SNAPSHOT in https://*****/nexus/content/groups/public/ was cached in the local repository, resolution will not be reattempted until the update interval of nexus has elapsed or updates are forced
[ERROR] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException

@aikebah
Copy link
Collaborator

aikebah commented Aug 30, 2017

@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.

@CWolff92
Copy link

CWolff92 commented Mar 5, 2018

I have been running into the same issue. When running the verify goal on the parent project after increasing the version number, it fails.

Because the parent pom is built first and the plugin causes maven to try and resolve the (not yet built) submodules it ultimately crashes.

@smoyer64
Copy link

smoyer64 commented Mar 14, 2018

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 <inherited>false</inherited> results in the project's success because the dependency-check plugin is never executed. Once it is executed, it fails in the same manner.

@smoyer64
Copy link

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.

@smoyer64
Copy link

@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 maven-dependency-plugin's dependency:tree command. Looking at the dependency-check-maven code, the following classes seem to be shared:

import org.apache.maven.shared.dependency.graph.DependencyGraphBuilder;
import org.apache.maven.shared.dependency.graph.DependencyGraphBuilderException;
import org.apache.maven.shared.dependency.graph.DependencyNode;

I'm going to go out on a limb and say that the error was in the shared Maven dependencies library. Forcing my maven-dependency-plugin to the newest version fixes the issue so perhaps to a 3.x version of the shared maven-dependency-tree library would also solve this problem?

You have a note in the dependency-check-plugin's parent POM as follows:

upgrading beyond 2.2 requires reworking the dependency resolution

So I can understand that it's not quite as easy as just changing the version.

@aikebah
Copy link
Collaborator

aikebah commented Mar 28, 2018

@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.

@aikebah
Copy link
Collaborator

aikebah commented Mar 28, 2018

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]
org.apache.maven.lifecycle.LifecycleExecutionException: 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.

@aikebah
Copy link
Collaborator

aikebah commented Mar 28, 2018

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.

@jeremylong
Copy link
Owner

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 maven install hack. Basically, your build order is not quite right and modules earlier in your build depend on modules built later.

@lock
Copy link

lock bot commented Sep 27, 2018

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.

@lock lock bot locked and limited conversation to collaborators Sep 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants