Uploaded image for project: 'jBPM'
  1. jBPM
  2. JBPM-9860

ArtifactResolver's getAllDependecies method returns a list of dependencies unfiltered by maven resolver's standard version conflict resolution process

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: Minor Minor
    • None
    • 7.58.0.Final
    • KieServer
    • None
    • False
    • False
    • NEW
    • NEW
    • undefined
    • Hide

      Example)
      A
      ├── B
      │ └── C 18.0.0
      └── C 21.0.0
      Where C=some fantastic library like guava.

      The maven way to resolve the dependencies will correctly prune 18.0.0 as nearest neighbor is preferred when a conflict in the graph is found.

      My observation is that because this method (https://github.com/kiegroup/kie-soup/blob/master/kie-soup-maven-utils/kie-soup-maven-integration/src/main/java/org/appformer/maven/integration/ArtifactResolver.java#L155) does not resolve conflicts there are cases (like in kie-server deployments) where the inability to resolve the dependency graph "superset" prior to the conflict resolution will cause a deployment to fail.

      To put a finer point on it, if the goal is to prepackage all necessary dependencies for a kjar to avoid runtime resolution from a remote maven repository as you likely would in the case of a containerized kie-server to truly have something immutable you will find that the kiecontainer deployment will fail when you don't package the unnecessary conflicted libraries (e.g. both C 18.0.0 and 21.0.0).

      It was in this context I stumbled into the issue and was curious about the root cause. I doubt there is a good use case for this method to be returning "all" dependencies without conflict resolution filtering, and believe the fix is actually as simple as removing this custom code for calculating the dependencies as its already available elsewhere.

      Additional steps to reproduce documented here:
      https://github.com/Hillrunner2008/JBPM-9860

      Show
      Example) A ├── B │ └── C 18.0.0 └── C 21.0.0 Where C=some fantastic library like guava. The maven way to resolve the dependencies will correctly prune 18.0.0 as nearest neighbor is preferred when a conflict in the graph is found. My observation is that because this method ( https://github.com/kiegroup/kie-soup/blob/master/kie-soup-maven-utils/kie-soup-maven-integration/src/main/java/org/appformer/maven/integration/ArtifactResolver.java#L155 ) does not resolve conflicts there are cases (like in kie-server deployments) where the inability to resolve the dependency graph "superset" prior to the conflict resolution will cause a deployment to fail. To put a finer point on it, if the goal is to prepackage all necessary dependencies for a kjar to avoid runtime resolution from a remote maven repository as you likely would in the case of a containerized kie-server to truly have something immutable you will find that the kiecontainer deployment will fail when you don't package the unnecessary conflicted libraries (e.g. both C 18.0.0 and 21.0.0). It was in this context I stumbled into the issue and was curious about the root cause. I doubt there is a good use case for this method to be returning "all" dependencies without conflict resolution filtering, and believe the fix is actually as simple as removing this custom code for calculating the dependencies as its already available elsewhere. Additional steps to reproduce documented here: https://github.com/Hillrunner2008/JBPM-9860

      Much of this is just copied from issue report on the appropriate repo https://github.com/kiegroup/kie-soup/issues/190.

      I'm unclear on the reason for the getAllDependencies method.

      https://github.com/kiegroup/kie-soup/blob/master/kie-soup-maven-utils/kie-soup-maven-integration/src/main/java/org/appformer/maven/integration/ArtifactResolver.java#L155

      This method aggregates together all dependencies both direct and transitive into a Set; however, this Set is only constrained by duplicates and does not attempt to resolve version conflicts in the standard way maven would in the same case. (see https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Transitive_Dependencies).

      The drools project leverages this method (see https://github.com/kiegroup/drools/blob/master/kie-ci/src/main/java/org/kie/scanner/KieModuleMetaDataImpl.java#L173) and a side effect of this turns out to be many transitive artifacts are resolved (i.e. downloaded to local disk repo). Additionally, this project goes on to attempt to load all classes from all versions of a dependency into its KieContainer instance's classloader.

      Luckily this turns out to fail since that classloader was not backed by the same set of resolved artifacts, but its easy to imagine a worse outcome and the chaos this would create.

      A side effect of this "bug" that impacts the kie-server project in the drools ecosystem is that deployments of kjars to the kie-server application require a maven repository either contain or actively resolve these unnecessary dependencies. In the context of containerizing a kjar backed application this presents an interesting challenge as approaches such as this (see https://github.com/kiegroup/droolsjbpm-integration/blob/master/kie-maven-plugin/src/main/java/org/kie/maven/plugin/PackageKjarDependenciesMojo.java) seem destined to reliably fail in all cases where there is a conflict in the dependency graph. To be clear, when I say fail I only mean the expectation that an artifact can be resolved is baked into the product as it does not seem to have any awareness that some of these dependencies really should not be coming back from the ArtifactResolver's getAllDependecies method at all.

      I do not have any context on if this method has value in some other way, but I believe its likely a bug and at best is misleading.

              elguardian@gmail.com Enrique González Martínez (Inactive)
              danorris David Norris (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Created:
                Updated: