From f119d07b8c9de85720e703f6ce886bd6defca8d4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E8=90=A7=E6=98=93=E5=AE=A2?= <kmter@live.com>
Date: Mon, 18 Nov 2024 15:24:36 +0800
Subject: [PATCH] Fix potential jetcd-core shading problem (#4526)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

### Motivation

There is a potential jar shading issue introduced in #4426 that causes `NoClassDefFoundError` when connecting to an etcd metadata store.

The `jetcd-core-shaded` module was introduced in #4426 to address the compatibility issues between jetcd-core’s grpc-java dependency and Netty. You can find more details [here][1] and in the [grpc-java documentation][2].

[1]: https://github.com/apache/bookkeeper/pull/4426#issuecomment-2162348232
[2]: https://github.com/grpc/grpc-java/blob/master/SECURITY.md#netty

Currently, we use `unpack-shaded-jar` execution unpacks the shaded jar produced by `maven-shade-plugin:shade` into the `jetcd-core-shaded/target/classes` directory. However, the classes in this directory conflict with its dependencies. If the `maven-shade-plugin:shade` runs again without cleaning this directory, it can produce an incorrect shaded jar. You can replicate and verify this issue with the following commands:
```shell
# Step 1: Clean the build directory
mvn clean

# Step 2: Perform an install and unpack the shaded jar into a directory.
# Verify the import statement for `io.netty.handler.logging.ByteBufFormat` in
# `org/apache/pulsar/jetcd/shaded/io/vertx/core/net/NetClientOptions.class`.
# The correct import should be:
# `import io.grpc.netty.shaded.io.netty.handler.logging.ByteBufFormat;`.
mvn install
unzip $M2_REPO/org/apache/bookkeeper/metadata/drivers/jetcd-core-shaded/4.18.0-SNAPSHOT/jetcd-core-shaded-4.18.0-SNAPSHOT-shaded.jar \
  -d metadata-drivers/jetcd-core-shaded/target/first-classes

# Step 3: Run the install command again without cleaning.
# The unpacked jar from the previous step will persist in `target/classes`.
# Unpack the shaded jar into a different directory (e.g., second-classes) and check the import.
# The incorrect import will be:
# `import io.grpc.netty.shaded.io.grpc.netty.shaded.io.netty.handler.logging.ByteBufFormat;`.
mvn install
unzip $M2_REPO/org/apache/bookkeeper/metadata/drivers/jetcd-core-shaded/4.18.0-SNAPSHOT/jetcd-core-shaded-4.18.0-SNAPSHOT-shaded.jar \
  -d metadata-drivers/jetcd-core-shaded/target/second-classes

# Step 4: Use IntelliJ IDEA's "Compare Directories" tool to compare the `first-classes`
# and `second-classes` directories. The differences in imports should become apparent.
```

We can remove the attach and unpack configurations, and it should work fine.

This issue has already been [reported][3] in apache/pulsar, and a similar [patch][patch] has addressed it.

[3]: https://github.com/apache/pulsar/issues/23513
[patch]: https://github.com/apache/pulsar/pull/23604
---
 .github/workflows/bk-ci.yml                |  4 +-
 metadata-drivers/etcd/pom.xml              |  1 -
 metadata-drivers/jetcd-core-shaded/pom.xml | 48 ----------------------
 3 files changed, 3 insertions(+), 50 deletions(-)

diff --git a/.github/workflows/bk-ci.yml b/.github/workflows/bk-ci.yml
index 0106da05b33..6c40881bd54 100644
--- a/.github/workflows/bk-ci.yml
+++ b/.github/workflows/bk-ci.yml
@@ -242,7 +242,9 @@ jobs:
         run: mvn -B -nsu clean install -Pdocker -DskipTests
 
       - name: Run metadata driver tests
-        run: mvn -B -nsu -f metadata-drivers/pom.xml test -DintegrationTests
+        # Exclude jetcd-core-shaded from integration tests, as it’s a POM-only project used internally,
+        # and maven prioritizes workspace artifacts during testing.
+        run: mvn -B -nsu -f metadata-drivers/pom.xml -pl '!jetcd-core-shaded' test -DintegrationTests
 
       - name: Run all integration tests (except backward compatibility tests)
         run: |
diff --git a/metadata-drivers/etcd/pom.xml b/metadata-drivers/etcd/pom.xml
index 3dfbdcde1ed..bb03d5b853f 100644
--- a/metadata-drivers/etcd/pom.xml
+++ b/metadata-drivers/etcd/pom.xml
@@ -37,7 +37,6 @@
       <groupId>org.apache.bookkeeper.metadata.drivers</groupId>
       <artifactId>jetcd-core-shaded</artifactId>
       <version>${project.version}</version>
-      <classifier>shaded</classifier>
       <exclusions>
         <exclusion>
           <groupId>io.etcd</groupId>
diff --git a/metadata-drivers/jetcd-core-shaded/pom.xml b/metadata-drivers/jetcd-core-shaded/pom.xml
index 2b8e855aa22..cd995f66cfd 100644
--- a/metadata-drivers/jetcd-core-shaded/pom.xml
+++ b/metadata-drivers/jetcd-core-shaded/pom.xml
@@ -132,54 +132,6 @@
                   <file>${project.basedir}/dependency-reduced-pom.xml</file>
                 </transformer>
               </transformers>
-              <!-- required for IntelliJ support -->
-              <shadedArtifactAttached>true</shadedArtifactAttached>
-              <shadedClassifierName>shaded</shadedClassifierName>
-            </configuration>
-          </execution>
-        </executions>
-      </plugin>
-      <!-- required for IntelliJ support, for some reason shadedArtifactAttached isn't sufficient alone -->
-      <plugin>
-        <groupId>org.codehaus.mojo</groupId>
-        <artifactId>build-helper-maven-plugin</artifactId>
-        <executions>
-          <execution>
-            <id>attach-shaded-jar</id>
-            <phase>package</phase>
-            <goals>
-              <goal>attach-artifact</goal>
-            </goals>
-            <configuration>
-              <artifacts>
-                <artifact>
-                  <file>${project.build.directory}/${project.artifactId}-${project.version}-shaded.jar</file>
-                  <type>jar</type>
-                  <classifier>shaded</classifier>
-                </artifact>
-              </artifacts>
-            </configuration>
-          </execution>
-        </executions>
-      </plugin>
-      <!-- required for running tests with "mvn -f metadata-drivers/pom.xml test -DintegrationTests" -->
-      <plugin>
-        <groupId>org.apache.maven.plugins</groupId>
-        <artifactId>maven-antrun-plugin</artifactId>
-        <version>${maven-antrun-plugin.version}</version>
-        <executions>
-          <execution>
-            <id>unpack-shaded-jar</id>
-            <phase>package</phase>
-            <goals>
-              <goal>run</goal>
-            </goals>
-            <configuration>
-              <target>
-                <unzip src="${project.build.directory}/${project.artifactId}-${project.version}-shaded.jar"
-                       dest="${project.build.outputDirectory}"
-                       overwrite="true" />
-              </target>
             </configuration>
           </execution>
         </executions>