[CAMPAIGN] Remove jackson-databind and jackson-annotations from build.gradle since these dependencies are now coming from core #191

ryanbogan · 2022-12-13T20:07:01Z

jackson-databind and jackson-annotations were added as api dependencies in server of opensearch core. Therefore, all plugins now have these as transitive dependencies. Plugins that currently consume these dependencies directly will break while building unless the direct dependencies are removed. If your plugin does not consume jackson-databind or jackson-annotations directly, feel free to close the related issue in your plugin's repository.

Example of solution in security plugin: opensearch-project/security#2325

The text was updated successfully, but these errors were encountered:

dbwiddis · 2022-12-14T00:46:17Z

Some projects have other Jackson dependencies that are not one of these two. Most extensions may have, for example, jackson-datatype-jsr310 or jackson-datatype-guava. To maximize compatibility, projects should use the same version as OpenSearch core.

Is there any way for a separate project's build.gradle to reference the version number in OpenSearch?

See here for a potential idea.

dbwiddis · 2022-12-14T16:48:45Z

Is there any way for a separate project's build.gradle to reference the version number in OpenSearch?

Another option for synchronizing versions is dependencyManagement. This could/would declare the dependencies in OpenSeach core with the same version, but not actually load them. Only if dependent projects requested that dependency would it be loaded, but since it is managed, the correct version would be used.

penghuo · 2022-12-14T18:02:16Z

Could you explain more about the reason? SQL plugin using versions defined in OpenSearch. Is there any issue with this solution?

Plugins that currently consume these dependencies directly will break while building unless the direct dependencies are removed.

What do you mean break? The proposed solution does not work if plugin has subproject which does not depend on org.opensearch. e.g. https://github.com/opensearch-project/sql/blob/main/core/build.gradle. core is the subpoject include analyzer, planner, expression which depend on jackson, but not OpenSearch.

ryanbogan · 2022-12-14T20:28:06Z

Could you explain more about the reason? SQL plugin using versions defined in OpenSearch. Is there any issue with this solution?

Plugins that currently consume these dependencies directly will break while building unless the direct dependencies are removed.

What do you mean break? The proposed solution does not work if plugin has subproject which does not depend on org.opensearch. e.g. https://github.com/opensearch-project/sql/blob/main/core/build.gradle. core is the subpoject include analyzer, planner, expression which depend on jackson, but not OpenSearch.

@penghuo The security plugin and all native plugins/modules that used jackson-databind and jackson-annotations would fail ./gradlew build after these dependencies were moved to server. If your plugin is still functioning properly, then you can close the issue out, but I wanted to draw attention to a potential error that plugins might be facing.

penghuo · 2022-12-14T20:54:53Z

@ryanbogan got you. thanks.

ryanbogan · 2022-12-15T18:06:07Z

Is there any way for a separate project's build.gradle to reference the version number in OpenSearch?

@dbwiddis I believe this is already happening in some of the plugins. In the core repo, we have a version.properties file that contains all of the dependency versions for core.

For example, this is the line that security was using before to directly depend on jackson.
implementation "com.fasterxml.jackson.core:jackson-annotations:${versions.jackson}"

The ${versions.jackson} part of the code references the version.properties version number associated with jackson in that file.

dbwiddis · 2022-12-15T18:31:55Z

I believe this is already happening in some of the plugins.

Indeed it does look very widespread.

For example, the alerting plugin does what I suggested here:

 force "com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:${versions.jackson}"

Would it make sense to expand this campaign to recommend all repos switch to this versioning to prevent future breakage? I commented in a PR elsewhere for slf4j version that would also benefit from this format.

ryanbogan · 2022-12-15T18:55:49Z

Would it make sense to expand this campaign to recommend all repos switch to this versioning to prevent future breakage?

@dbwiddis Given the 2.5 deadline for this campaign, I think it would be better to do a separate campaign if we want to make that big of a change.

penghuo · 2022-12-15T18:56:04Z

@ryanbogan correct me if i am wrong.

I am not sure why security plugin failed, but based on my test on security 2.x branch, security.zip do not include jackson-databind.zip which means it should not have jarhell issue.
I also test on SQL main and 2.x branch, not issue found so far.
As i explained below, currently, OpenSearch exclude resolveableCompileOnly depedencies from runtimeClasspath when zip plugin, if your change breaking this, I suggest to add the exclude logic in PluginBuildPlugin instead.

OpenSearch exclude resolveableCompileOnly depedencies from runtimeClasspath when zip plugin

In OpenSearch PluginBuildPlugin, resolveableCompileOnly depedencies are removed from runtimeClasspath.

            from project.configurations.runtimeClasspath - project.configurations.getByName(
                    CompileOnlyResolvePlugin.RESOLVEABLE_COMPILE_ONLY_CONFIGURATION_NAME
            )

e.g. for SQL 2.x, jackson-core:2.4.1, jackson-databind:2.4.1 are removed from class path, and excluded from plugin.zip.

➜  sql git:(2.x) ./gradlew opensearch-sql:dependencies

resolveableCompileOnly
+--- org.opensearch:opensearch:2.5.0-SNAPSHOT
|    +--- org.opensearch:opensearch-core:2.5.0-SNAPSHOT
|    +--- org.opensearch:opensearch-secure-sm:2.5.0-SNAPSHOT
|    +--- org.opensearch:opensearch-x-content:2.5.0-SNAPSHOT
|    |    +--- org.opensearch:opensearch-core:2.5.0-SNAPSHOT
|    |    +--- org.yaml:snakeyaml:1.32
|    |    +--- com.fasterxml.jackson.core:jackson-core:2.14.1
|    |    +--- com.fasterxml.jackson.dataformat:jackson-dataformat-smile:2.14.1
|    |    +--- com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.14.1
|    |    \--- com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:2.14.1
|    +--- org.opensearch:opensearch-geo:2.5.0-SNAPSHOT
|    +--- org.apache.lucene:lucene-core:9.4.2
|    +--- org.apache.lucene:lucene-analysis-common:9.4.2
|    +--- org.apache.lucene:lucene-backward-codecs:9.4.2
|    +--- org.apache.lucene:lucene-grouping:9.4.2
|    +--- org.apache.lucene:lucene-highlighter:9.4.2
|    +--- org.apache.lucene:lucene-join:9.4.2
|    +--- org.apache.lucene:lucene-memory:9.4.2
|    +--- org.apache.lucene:lucene-misc:9.4.2
|    +--- org.apache.lucene:lucene-queries:9.4.2
|    +--- org.apache.lucene:lucene-queryparser:9.4.2
|    +--- org.apache.lucene:lucene-sandbox:9.4.2
|    +--- org.apache.lucene:lucene-spatial-extras:9.4.2
|    +--- org.apache.lucene:lucene-spatial3d:9.4.2
|    +--- org.apache.lucene:lucene-suggest:9.4.2
|    +--- org.opensearch:opensearch-cli:2.5.0-SNAPSHOT
|    |    +--- net.sf.jopt-simple:jopt-simple:5.0.4
|    |    \--- org.opensearch:opensearch-core:2.5.0-SNAPSHOT
|    +--- com.carrotsearch:hppc:0.8.1
|    +--- joda-time:joda-time:2.10.12
|    +--- com.tdunning:t-digest:3.2
|    +--- org.hdrhistogram:HdrHistogram:2.1.12
|    +--- org.apache.logging.log4j:log4j-api:2.17.1
|    +--- org.apache.logging.log4j:log4j-jul:2.17.1
|    +--- net.java.dev.jna:jna:5.5.0
|    +--- com.fasterxml.jackson.core:jackson-databind:2.14.1
|    \--- com.fasterxml.jackson.core:jackson-annotations:2.14.1
+--- org.locationtech.spatial4j:spatial4j:0.7
+--- org.locationtech.jts:jts-core:1.15.0
+--- org.apache.logging.log4j:log4j-api:2.17.1
+--- org.apache.logging.log4j:log4j-core:2.17.1
+--- net.java.dev.jna:jna:5.5.0
\--- org.projectlombok:lombok:1.18.22

ryanbogan · 2022-12-15T19:00:16Z

I'm not experienced with the inner workings of the security plugin, but there was an issue raised by them and this solution fixed it. @cwperks you raised the PR to fix the issue, thoughts on this?

cwperks · 2022-12-15T19:41:18Z

@penghuo @ryanbogan Thank you for sharing the resolveableCompileOnly dependency workflow. We experienced jar hell when the plugin install github workflow was run in CI. See this issue with details: opensearch-project/security#2324

Its possible that when ./gradlew assemble was run to create the snapshot security zip that it was building with stale artifacts from core without jackson-databind and jackson-annotations as dependencies which resulted in the security zip including those jars.

The workflow downloads minified opensearch using https://artifacts.opensearch.org/snapshots/core/opensearch/${{ inputs.opensearch-version }}-SNAPSHOT/opensearch-min-${{ inputs.opensearch-version }}-SNAPSHOT-linux-x64-latest.tar.gz and installs the security plugin zip that is output from ./gradlew assemble

See: https://github.com/opensearch-project/security/blob/2.x/.github/actions/start-opensearch-with-one-plugin/action.yml#L39

I removed the implementation dependencies on jackson-databind and jackson-annotations in security's build.gradle to explicitly remove them as direct dependencies.

Edit: I tested to see if security zip would include jackson-databind and jackson-annotations in the zip if the dependency was removed from the server by removing the dependency in the server and running ./gradlew publishToMavenLocal and producing the security zip. The issue we faced with jar hell was likely due to stale core artifacts.

ryanbogan · 2022-12-15T19:56:46Z

@cwperks Thank you for the additional insight. What is the best way for plugins to know if they need to make this change?

cwperks · 2022-12-15T21:12:44Z

If they encounter the same jarHell issue then removing the direct dependencies is a workaround. Since updated core artifacts are available, the workaround to remove the direct dependencies in plugins may not be necessary since the OpenSearch build system will prevent the jar from being included in the plugin zip on assembly.

ryanbogan · 2022-12-22T06:31:43Z

After further discussion with @cwperks , there can still be issues when the version of the plugin does not match the version from core. Therefore, all plugins still need to remove any dependency they have in build.gradle files on jackson-databind and jackson-annotations.

minalsha · 2023-04-27T16:15:07Z

We will create a new campaign to to remove jackson-databind and jackson-annotations completely from every plugin repo instead of only from build.gradle (as called out in this issue). @ryanbogan please create a new campaign issue once you are ready. thanks

ryanbogan added bug Something isn't working campaign Parent issues of OpenSearch release campaigns. v2.5.0 'Issues and PRs related to version v2.5.0' labels Dec 13, 2022

minalsha changed the title ~~[CAMPAIGN] Remove jackson-databind and jackson-annotations dependencies now coming from core~~ [CAMPAIGN] Remove jackson-databind and jackson-annotations from build.gradle since these dependencies are now coming from core Dec 13, 2022

gaiksaya mentioned this issue Dec 13, 2022

[RELEASE] Release version 2.5.0 opensearch-project/opensearch-build#2908

Closed

47 tasks

penghuo mentioned this issue Dec 14, 2022

Remove jackson-databind and jackson-annotations dependencies now coming from core opensearch-project/sql#1167

Closed

This was referenced Dec 28, 2022

Remove jackson-databind and jackson-annotations dependencies now coming from core opensearch-project/ml-commons#651

Closed

Remove jackson-databind and jackson-annotations dependencies now coming from core opensearch-project/ml-commons#652

Merged

priyatsh mentioned this issue Jan 6, 2023

Removed jackson-databind dependency from build-gradle that now comes … opensearch-project/cross-cluster-replication#675

Merged

5 tasks

ryanbogan closed this as completed Jan 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CAMPAIGN] Remove jackson-databind and jackson-annotations from build.gradle since these dependencies are now coming from core #191

[CAMPAIGN] Remove jackson-databind and jackson-annotations from build.gradle since these dependencies are now coming from core #191

ryanbogan commented Dec 13, 2022 •

edited

Loading

dbwiddis commented Dec 14, 2022 •

edited

Loading

dbwiddis commented Dec 14, 2022

penghuo commented Dec 14, 2022

ryanbogan commented Dec 14, 2022 •

edited

Loading

penghuo commented Dec 14, 2022

ryanbogan commented Dec 15, 2022

dbwiddis commented Dec 15, 2022

ryanbogan commented Dec 15, 2022

penghuo commented Dec 15, 2022

ryanbogan commented Dec 15, 2022

cwperks commented Dec 15, 2022 •

edited

Loading

ryanbogan commented Dec 15, 2022

cwperks commented Dec 15, 2022

ryanbogan commented Dec 22, 2022

minalsha commented Apr 27, 2023

[CAMPAIGN] Remove jackson-databind and jackson-annotations from build.gradle since these dependencies are now coming from core #191

[CAMPAIGN] Remove jackson-databind and jackson-annotations from build.gradle since these dependencies are now coming from core #191

Comments

ryanbogan commented Dec 13, 2022 • edited Loading

dbwiddis commented Dec 14, 2022 • edited Loading

dbwiddis commented Dec 14, 2022

penghuo commented Dec 14, 2022

ryanbogan commented Dec 14, 2022 • edited Loading

penghuo commented Dec 14, 2022

ryanbogan commented Dec 15, 2022

dbwiddis commented Dec 15, 2022

ryanbogan commented Dec 15, 2022

penghuo commented Dec 15, 2022

OpenSearch exclude resolveableCompileOnly depedencies from runtimeClasspath when zip plugin

ryanbogan commented Dec 15, 2022

cwperks commented Dec 15, 2022 • edited Loading

ryanbogan commented Dec 15, 2022

cwperks commented Dec 15, 2022

ryanbogan commented Dec 22, 2022

minalsha commented Apr 27, 2023

ryanbogan commented Dec 13, 2022 •

edited

Loading

dbwiddis commented Dec 14, 2022 •

edited

Loading

ryanbogan commented Dec 14, 2022 •

edited

Loading

cwperks commented Dec 15, 2022 •

edited

Loading