loustler opened a new pull request, #11673:
URL: https://github.com/apache/seatunnel/pull/11673
### Purpose of this pull request
Closes #11655.
`seatunnel-hadoop-aws` is shaded without a Jackson relocation, while the
`seatunnel-hadoop3-*-uber` jar it resolves against relocates Jackson. The
two disagree,
so every S3A code path that goes through
`org.apache.hadoop.util.JsonSerialization` fails
at runtime.
### The mechanism
`seatunnel-hadoop-aws` bundles **no Jackson classes of its own**. It only
*references*
them, and it reaches the implementation through `hadoop-common`, which this
jar does not
bundle either — the uber jar supplies it.
The uber jar relocates Jackson, so the method it ships is declared as:
```
$ javap -p seatunnel-hadoop3-3.1.4-uber.jar →
org.apache.hadoop.util.JsonSerialization
public
org.apache.seatunnel.shade.hadoop.com.fasterxml.jackson.databind.ObjectMapper
getMapper();
```
But `hadoop-aws`, shaded without the matching relocation, keeps the original
descriptor at
its call site:
```
$ javap -p -c org/apache/hadoop/fs/s3a/auth/RoleModel.class # before
23: invokevirtual #58 // Method
org/apache/hadoop/util/JsonSerialization.getMapper:()Lcom/fasterxml/jackson/databind/ObjectMapper;
```
The JVM matches methods by **name + descriptor**. `()Lcom/fasterxml/...;` and
`()Lorg/apache/seatunnel/shade/hadoop/com/fasterxml/...;` are different
descriptors, so
this resolves against nothing:
```
java.lang.NoSuchMethodError:
org.apache.hadoop.util.JsonSerialization.getMapper()
Lcom/fasterxml/jackson/databind/ObjectMapper;
```
Any `fs.s3a.assumed.role.*` configuration hits this.
**Why it went unnoticed.** This is a link-time descriptor mismatch, not a
missing class.
Both jars build cleanly, `mvn dependency:tree` shows nothing unusual, and no
test that
avoids assumed-role touches the path. It only appears the first time a real
job uses an
assumed role.
### The fix
```xml
<relocation>
<pattern>com.fasterxml.jackson</pattern>
<shadedPattern>${seatunnel.shade.package}.hadoop.com.fasterxml.jackson</shadedPattern>
</relocation>
```
Two things about this are deliberate and worth stating, because both look
wrong at a
glance:
1. **It uses the uber jar's shaded pattern
(`…shade.hadoop.com.fasterxml.jackson`), not
this module's own prefix.** The goal is to agree with the class that
*supplies* the
method, not to be internally self-consistent. Relocating to a
module-specific prefix
would leave the descriptor just as mismatched, only differently.
2. **Nothing is added to this jar.** `maven-shade-plugin` rewrites
*references* even when
it bundles none of the target classes. The jar's class count is unchanged
(194 under
`org/apache/hadoop/` before and after); only constant-pool entries differ.
This also answers the option (b) raised in the issue — dropping the
relocation from the
uber jar instead. That would fix this call site but changes the contract for
every other
consumer of the uber jar, so option (a) is the contained change.
### Verification
Same module, same Hadoop version (3.1.4), same 194 classes under
`org/apache/hadoop/`,
built twice — once with the relocation removed, once with it applied:
| build | classes scanned | classes still referencing original Jackson |
|---|---|---|
| relocation removed | 194 | **3** — `RoleModel`, `RoleModel$Policy`,
`RoleModel$Statement` |
| relocation applied | 194 | **0** |
And the rewritten call site now matches the uber jar's declared return type
exactly:
```
$ javap -p -c org/apache/hadoop/fs/s3a/auth/RoleModel.class # after
23: invokevirtual #58 // Method
org/apache/hadoop/util/JsonSerialization.getMapper:()Lorg/apache/seatunnel/shade/hadoop/com/fasterxml/jackson/databind/ObjectMapper;
```
That descriptor was compared against a freshly built uber jar, not against
the pom — the
two agree.
### A second, silent failure mode on newer Hadoop
Running the same check against **hadoop-aws 3.4.3** flags three more classes:
```
org/apache/hadoop/fs/s3a/commit/files/PendingSet.class
org/apache/hadoop/fs/s3a/commit/files/SinglePendingCommit.class
org/apache/hadoop/fs/s3a/commit/files/SuccessData.class
```
These fail *differently*. Their references are annotations —
`Lcom/fasterxml/jackson/annotation/JsonProperty;` — and the relocated
Jackson looks for
*relocated* annotations. It does not find them, so it does not throw: it
silently falls
back to default property naming and inclusion when writing the S3A
committer's `_SUCCESS`
and `.pending` files. Wrong output, no exception.
3.1.4 does not carry these annotations, so this mode is not reachable on
`dev` today. It
is the reason the check below is worth keeping rather than being a one-off.
### Regression test
The issue suggested a localstack-based e2e case exercising assumed-role. I
went with a
jar-level check instead, in
`tools/dependencies/check_shaded_jackson_refs.py`, wired into
the existing `dependency-license` job — which already runs a full `install`,
so the jar is
there and the step costs seconds.
The reasoning: the defect is a property of the *artifact*, so checking the
artifact catches
it deterministically, with no container, no AWS credentials, and no
flakiness. An e2e test
would only cover the one code path someone thought to write; this covers
every class in the
jar, including the annotation-carrying ones above that no assumed-role test
would reach.
The check scans every `CONSTANT_Utf8` entry of every class under
`org/apache/hadoop/` —
class references, descriptors, signatures, annotations and string constants
alike. Two
details that make it trustworthy rather than decorative:
- Correctly relocated names *contain* the original package as a suffix, so a
naive
substring search reports every fixed reference as a violation. The check
masks relocated
names before searching.
- A jar with zero classes under the scanned prefix **fails** rather than
passing vacuously.
That is the case where a layout change would otherwise silently disarm the
check.
Scope is limited to `org/apache/hadoop/` on purpose: the invariant is that
this jar's
Hadoop classes agree with the uber jar's. The AWS SDK vendors its own
Jackson under
`com.amazonaws.thirdparty` / `software.amazon.awssdk.thirdparty` and
resolves those among
its own classes; flagging them would make the check permanently red for
something nobody
can act on.
Controls, so the check is known to discriminate rather than merely pass:
| artifact | result |
|---|---|
| unpatched, hadoop-aws 3.1.4 | FAIL — 3 of 194 |
| unpatched, hadoop-aws 3.4.3 | FAIL — 6 of 466 |
| **patched, hadoop-aws 3.1.4** | **PASS — 194 scanned** |
| jar with no `org/apache/hadoop/` classes | FAIL — nothing checked |
### What this PR does not do
- It does not change the uber jar.
- It does not upgrade Hadoop or the AWS SDK.
- It does not add an assumed-role integration test; see the reasoning above.
### Check list
* [x] Code changed are covered with tests, or it does not need tests
* [x] If any new Jar binary package adding in your PR, please add License
Notice according
[New License
Guide](https://github.com/apache/seatunnel/blob/dev/docs/en/contribution/new-license.md)
— no new jars are added; only existing references are rewritten
* [x] If necessary, please update the documentation to describe the new
feature.
https://github.com/apache/seatunnel/tree/dev/docs
* [x] If you are contributing the connector code, please check that the
following files are updated:
— not a connector change
* [x] Update the `docs/en/seatunnel-engine/download-seatunnel.md` if the
change is related to the release
— not release-related
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
