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.

Sign up for GitHub

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

Jump to bottom

[fix][client] Shade com.fasterxml.jackson.datatype.* to prevent ClassNotFoundException #19458

Merged
merged 2 commits into from
Feb 16, 2023
Merged

[fix][client] Shade com.fasterxml.jackson.datatype.* to prevent ClassNotFoundException #19458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6ee6f96
Shade com.fasterxml.jackson.datatype.* to prevent ClassNotFoundException
Feb 8, 2023
42c72d1
Exclude jackson-annotations from shading
Feb 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions pulsar-client-admin-shaded/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@
<include>com.google.protobuf:protobuf-java</include>
<include>com.google.guava:guava</include>
<include>com.google.code.gson:gson</include>
<include>com.fasterxml.jackson.core</include>
<include>com.fasterxml.jackson.*:*</include>
<include>io.netty:*</include>
<include>io.netty.incubator:*</include>
<include>org.apache.pulsar:pulsar-common</include>
Expand All @@ -133,7 +133,6 @@
<include>javax.ws.rs:*</include>
<include>jakarta.annotation:*</include>
<include>org.glassfish.hk2*:*</include>
<include>com.fasterxml.jackson.*:*</include>
<include>io.grpc:*</include>
<include>io.perfmark:*</include>
<include>com.yahoo.datasketches:*</include>
Expand All @@ -150,6 +149,9 @@
<!-- Issue #6834, Since Netty ByteBuf shaded, we need also shade this module -->
<include>org.apache.pulsar:pulsar-client-messagecrypto-bc</include>
</includes>
<excludes>
<exclude>com.fasterxml.jackson.core:jackson-annotations</exclude>
</excludes>
</artifactSet>
<filters>
<filter>
Expand Down Expand Up @@ -192,6 +194,9 @@
<relocation>
<pattern>com.fasterxml.jackson</pattern>
<shadedPattern>org.apache.pulsar.shade.com.fasterxml.jackson</shadedPattern>
<excludes>
<exclude>com.fasterxml.jackson.annotation.*</exclude>
</excludes>
</relocation>
<relocation>
<pattern>io.netty</pattern>
Expand Down
12 changes: 7 additions & 5 deletions pulsar-client-all/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,7 @@
<include>com.google.errorprone:*</include>
<include>com.google.j2objc:*</include>
<include>com.google.code.gson:gson</include>
<include>com.fasterxml.jackson.core</include>
<include>com.fasterxml.jackson.module</include>
<include>com.fasterxml.jackson.core:jackson-core</include>
<include>com.fasterxml.jackson.dataformat</include>
<include>com.fasterxml.jackson.*:*</include>
<include>io.netty:netty</include>
<include>io.netty:netty-all</include>
<include>io.netty:netty-tcnative-boringssl-static</include>
Expand All @@ -171,7 +168,6 @@
<include>javax.ws.rs:*</include>
<include>jakarta.annotation:*</include>
<include>org.glassfish.hk2*:*</include>
<include>com.fasterxml.jackson.*:*</include>
<include>io.grpc:*</include>
<include>io.perfmark:*</include>
<include>com.yahoo.datasketches:*</include>
Expand All @@ -194,6 +190,9 @@
<!-- Issue #6834, Since Netty ByteBuf shaded, we need also shade this module -->
<include>org.apache.pulsar:pulsar-client-messagecrypto-bc</include>
</includes>
<excludes>
<exclude>com.fasterxml.jackson.core:jackson-annotations</exclude>
</excludes>
</artifactSet>
<filters>
<filter>
Expand Down Expand Up @@ -230,6 +229,9 @@
<relocation>
<pattern>com.fasterxml.jackson</pattern>
<shadedPattern>org.apache.pulsar.shade.com.fasterxml.jackson</shadedPattern>
<excludes>
<exclude>com.fasterxml.jackson.annotation.*</exclude>
</excludes>
</relocation>
<relocation>
<pattern>io.netty</pattern>
Expand Down
11 changes: 7 additions & 4 deletions pulsar-client-shaded/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,7 @@
<include>com.google.errorprone:*</include>
<include>com.google.j2objc:*</include>
<include>com.google.code.gson:gson</include>
<include>com.fasterxml.jackson.core</include>
<include>com.fasterxml.jackson.module</include>
<include>com.fasterxml.jackson.core:jackson-core</include>
<include>com.fasterxml.jackson.dataformat</include>
<include>com.fasterxml.jackson.*:*</include>
Copy link
Member

Choose a reason for hiding this comment

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

I think that this breaks the usage of Jackson annotations in user compiled classes. The Jackson annotations are part of com.fasterxml.jackson.core:jackson-annotations dependency.
That's perhaps the reason why the wildcard rule was missing before. @massakam do you have a chance to check? An integration test would be nice.

Copy link
Member

Choose a reason for hiding this comment

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

Instead, revert the changes in this pom.xml and add

      <include>com.fasterxml.jackson.module:jackson-module-parameter-names</include>
      <include>com.fasterxml.jackson.datatype:jackson-datatype-jsr310</include>
      <include>com.fasterxml.jackson.datatype:jackson-datatype-jdk8</include>

Copy link
Member

@lhotari lhotari Feb 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That would ensure that Jackson annotations are processed in the same way as before.

Copy link
Member

Choose a reason for hiding this comment

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

For the other pom.xml's the wildcard was already there and it raises some questions how unshaded Jackson annotations would work with shaded Jackson. Perhaps it does?

Copy link
Member

Choose a reason for hiding this comment

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

Related to #6902 . It seems that annotations should be unshaded. For example a Testcontainers comment: testcontainers/testcontainers-java#3972 (comment) . Found it by googling. Another one: https://github.com/hazelcast/hazelcast/blob/master/hazelcast/pom.xml#L158-L162

There's a bug in pulsar-client-admin-shaded/pom.xml and pulsar-client-all/pom.xml since there's already the wildcard for jackson. That doesn't make sense since annotations didn't seem to be shaded for pulsar-client-shaded, but it's shaded for the other 2. I believe that the wildcard shouldn't be used unless there's an exclusion for com.fasterxml.jackson.core:jackson-annotations . That could be a better solution so that it wouldn't be necessary to list all jackson deps. @massakam could you try if that would be a an option.

Letting it be
<include>com.fasterxml.jackson.*:*</include>
but also adding
<exclude>com.fasterxml.jackson.core:jackson-annotations</exclude> to <excludes>. I don't know if that works with maven. Perhaps it shades it if it gets included. It would have to be checked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lhotari The group id of jackson-annotations is com.fasterxml.jackson.core, so it seems to be already shaded to pulsar-client, pulsar-client-admin and pulsar-client-all.

Copy link
Contributor Author

@massakam massakam Feb 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@lhotari Excluded jackson-annotation from shading (42c72d1). I confirmed that jars such as pulsar-client no longer include jackson-annotation classes:

click to expand 
$ diff -u <(jar tf BEFORE/pulsar-client-2.12.0-SNAPSHOT.jar | grep 'jackson.annotation') <(jar tf AFTER/pulsar-client-2.12.0-SNAPSHOT.jar | grep 'jackson.annotation')

--- /dev/fd/63  2023-02-13 16:26:33.652605755 +0900
+++ /dev/fd/62  2023-02-13 16:26:33.652605755 +0900
@@ -1,75 +0,0 @@
-META-INF/maven/com.fasterxml.jackson.core/jackson-annotations/
-META-INF/maven/com.fasterxml.jackson.core/jackson-annotations/pom.properties
-META-INF/maven/com.fasterxml.jackson.core/jackson-annotations/pom.xml
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JacksonAnnotation.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JacksonAnnotationValue.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JacksonAnnotationsInside.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JacksonInject$Value.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JacksonInject.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonAlias.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonAnyGetter.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonAnySetter.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonAutoDetect$1.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonAutoDetect$Value.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonAutoDetect$Visibility.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonAutoDetect.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonBackReference.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonClassDescription.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonCreator$Mode.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonCreator.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonEnumDefaultValue.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonFilter.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonFormat$Feature.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonFormat$Features.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonFormat$Shape.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonFormat$Value.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonFormat.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonGetter.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonIdentityInfo.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonIdentityReference.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonIgnore.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonIgnoreProperties$Value.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonIgnoreProperties.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonIgnoreType.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonInclude$Include.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonInclude$Value.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonInclude.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonIncludeProperties$Value.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonIncludeProperties.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonKey.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonManagedReference.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonMerge.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonProperty$Access.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonProperty.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonPropertyDescription.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonPropertyOrder.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonRawValue.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonRootName.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonSetter$Value.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonSetter.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonSubTypes$Type.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonSubTypes.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonTypeId.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonTypeInfo$As.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonTypeInfo$Id.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonTypeInfo$None.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonTypeInfo.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonTypeName.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonUnwrapped.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonValue.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonView.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/Nulls.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerator$IdKey.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerator.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerators$Base.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerators$IntSequenceGenerator.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerators$None.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerators$PropertyGenerator.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerators$StringIdGenerator.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerators$UUIDGenerator.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerators.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdResolver.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/OptBoolean.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/PropertyAccessor.class
-org/apache/pulsar/shade/com/fasterxml/jackson/annotation/SimpleObjectIdResolver.class

Copy link
Contributor

Choose a reason for hiding this comment

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

Excluded jackson-annotation from shading

This causes ConsumerBuilder#subscribe to get stuck in certain environments using jackson-annotation versions earlier than 2.12.0. See #21971 for details.

<include>io.netty:*</include>
<include>io.netty.incubator:*</include>
<include>io.perfmark:*</include>
Expand All @@ -171,6 +168,9 @@
<!-- Issue #6834, Since Netty ByteBuf shaded, we need also shade this module -->
<include>org.apache.pulsar:pulsar-client-messagecrypto-bc</include>
</includes>
<excludes>
<exclude>com.fasterxml.jackson.core:jackson-annotations</exclude>
</excludes>
</artifactSet>
<filters>
<filter>
Expand Down Expand Up @@ -207,6 +207,9 @@
<relocation>
<pattern>com.fasterxml.jackson</pattern>
<shadedPattern>org.apache.pulsar.shade.com.fasterxml.jackson</shadedPattern>
<excludes>
<exclude>com.fasterxml.jackson.annotation.*</exclude>
</excludes>
</relocation>
<relocation>
<pattern>io.netty</pattern>
Expand Down