What Problem Does This PR Solve?

Today publish-e2e-spark-images.yaml only builds the Spark E2E Docker images on manual workflow_dispatch, and it only did so successfully once (#11664). Nothing validates that a change to the Dockerfile or docker-bake.hcl still produces a working multi-arch (linux/amd64 + linux/arm64) image until someone remembers to run the workflow by hand, at which point it pushes straight to Docker Hub. This PR closes that gap by:

• adding a pull_request trigger, scoped to the two paths that actually affect the image (publish-e2e-spark-images.yaml itself and the docker/spark/** build context), that runs a credential-free docker buildx bake build with no Docker Hub login and no --push;
• keeping workflow_dispatch + docker/login-action + --push isolated to a separate publish job that only runs on apache/seatunnel itself, and letting contributors dry-run the same credential-free build manually on their own forks;
• pinning the downloaded Spark tarball to its official Apache SHA-512 checksum before extraction, so a corrupted or tampered download fails the build instead of being silently unpacked;
• renaming the Buildx bake targets from spark-2.4.6/spark-3.3.0 to spark-2-4-6/spark-3-3-0, since Buildx target names only accept [a-zA-Z0-9_-]+ and the dotted names silently broke the first real Buildx run — while keeping the published Docker tags dotted so downstream consumers see no change.

1. Code Change Review

1.1 Core Logic Analysis

I traced the full trigger matrix by diffing the workflow against its pre-PR version and the current PR HEAD:

• build job: if: github.event_name == 'pull_request' || github.repository != 'apache/seatunnel'. On a pull_request event this is always true regardless of github.repository (which is the base repo for pull_request events, i.e. apache/seatunnel, even for fork PRs), so it correctly runs for both same-repo and fork PRs. On workflow_dispatch it only runs when github.repository != 'apache/seatunnel', i.e. a contributor manually dispatching it from their own fork — matching the "credential-free manual fork build" claim in the PR description, and matching the linked evidence run on FenjuFu/seatunnel.
• publish job: if: github.event_name == 'workflow_dispatch' && github.repository == 'apache/seatunnel'. This can never be true at the same time as build's fork-dispatch branch, and a manual dispatch on the official repo skips build and goes straight to publish (which builds-and-pushes in one step, same as before this PR) — no behavior change there.
• The build job requests only permissions: contents: read and never references secrets.*, so a fork PR that touches this workflow does not need (and cannot get) Docker Hub credentials — the publish job is the only place secrets are used, and it is unreachable from pull_request events or fork workflow_dispatch.

I independently verified both SHA-512 values against archive.apache.org's published .sha512 files for spark-2.4.6-bin-hadoop2.7.tgz and spark-3.3.0-bin-hadoop3.tgz, and both match exactly.

I also verified the Dockerfile's RUN chain: SPARK_SHA512 is checked with test -n alongside the existing SPARK_VERSION/HADOOP_PROFILE guards, and echo "${SPARK_SHA512} /tmp/spark.tgz" | sha512sum --check --strict - runs after the curl --output and before tar --extract, so a checksum mismatch fails the build before anything is unpacked — a correct fail-closed placement.

Finally, I grepped the whole PR branch for any other reference to the old dotted bake target names (spark-2.4.6/spark-3.3.0 as target identifiers, not tags) and found none — the rename is self-contained to docker-bake.hcl, and the published tags (apache/seatunnel:e2e-spark-2.4.6/:e2e-spark-3.3.0) are untouched, so any downstream consumer of those tags is unaffected.

1.2 Compatibility Impact

No user-facing or packaging/dist artifact impact — this only touches internal E2E image build tooling. Published Docker tags are unchanged (only the internal Buildx target identifiers changed), and I could not find any other file in the repo referencing the old dotted target names, so nothing else needs updating in lockstep with this PR.

1.3 Performance / Side-Effect Analysis

One real side effect worth flagging: concurrency: group: publish-e2e-spark-images / cancel-in-progress: false is unchanged from before, but its blast radius just grew. Before this PR the group only ever had workflow_dispatch runs (rare, one at a time by construction). Now every PR touching docker/spark/** also lands in that same, un-suffixed group. Two PRs touching that path around the same time — or a PR build queued behind an in-flight manual publish run — will serialize rather than run in parallel, and with timeout-minutes: 60 a stuck run could block a second PR's validation feedback for up to an hour. See "Issue 1" below.

1.4 Error Handling and Logging

The new checksum check fails loudly and early (sha512sum --check --strict) rather than silently continuing with a possibly-corrupted archive — good fail-closed behavior. The ARG SPARK_SHA512 + test -n guard also protects against someone building the Dockerfile directly (bypassing docker-bake.hcl) and silently skipping verification. No secrets are echoed or logged anywhere in the new steps.

2. Code Quality Assessment

2.1 Coding Standards

The workflow YAML and HCL changes are minimal, readable, and consistent with the existing style in the file (job structure, step naming, timeout-minutes). The Dockerfile change follows the existing backslash-continued && chain pattern rather than introducing a new style.

2.2 Test Coverage and Test Stability

This is CI/build tooling, not application test code, so the usual flaky-test-pattern checklist doesn't directly apply — but the author did the equivalent groundwork: a real fork Buildx run (linked, 2m03s, both Spark versions x both architectures, both SHA-512 checks passing, publish job correctly skipped), actionlint validation of the workflow YAML, and manual confirmation that both archive URLs resolve and that eclipse-temurin:8-jre-jammy resolves for both architectures. That's solid, concrete evidence rather than a bare "should work."

2.3 Documentation Updates

None needed and none claimed — this is a purely internal CI/E2E change with no user-visible behavior, matching the PR checklist.

3. Architectural Soundness

3.1 Elegance of the Solution

Splitting build (validate-only, no secrets) from publish (push, secrets required, official repo only) is the right shape for this problem: it lets the credential-free path run automatically and even in forks, while keeping the credentialed path narrowly gated. The path-scoped trigger also avoids running an unrelated multi-arch Docker build on every PR.

3.2 Maintainability

Straightforward and easy to reason about — two small, clearly-named jobs with explicit if: conditions rather than one job with nested conditionals.

3.3 Extensibility

The same build/publish split pattern could be reused as-is for the Flink images mentioned as out-of-scope in the PR description, so this sets a reasonable precedent for that follow-up.

3.4 Historical-Version Compatibility

Not applicable in the usual checkpoint/serialization sense — this is CI tooling with no persisted state. The only "compatibility" surface (published Docker tags) is explicitly preserved.

4. Issue Summary

Raised by another reviewer: No (no prior reviews or comments exist on this PR).

5. Merge Recommendation

Conclusion: Ready to merge

1. Blockers — must be fixed

   • None.

2. Recommended fixes — non-blocking

   • Consider suffixing the concurrency group for the build job with something like ${{ github.event.pull_request.number || github.run_id }} (or splitting build and publish into separate concurrency groups) so that per-PR validation builds can run in parallel with each other and don't queue behind the credentialed publish job, which is the one that actually needs global serialization against Docker Hub pushes.

Overall assessment: this is a well-scoped, security-conscious hardening of the Spark E2E image pipeline. The trigger logic is correct for every combination of event/repo I traced (same-repo PR, fork PR, official manual dispatch, fork manual dispatch), the SHA-512 values check out against the official Apache archive, the Buildx target rename is self-contained and doesn't touch published tags, and the credential/permission boundary between build and publish is clean. The only thing worth improving is the shared concurrency group, which is a CI-latency nit rather than a correctness or security problem — happy to see this merged as-is, with the concurrency tweak as a quick follow-up if desired. Thanks for the thorough testing evidence in the PR description, it made this review much easier to verify independently.

+1 LGTM

+1
LGTM

Thanks for your contribution! This is a solid addition to the project, and your attention to detail really shows. Contributors like you are what keep this community moving forward. Feel free to keep the PRs coming — we're always glad to have you here!

Please ignore the following message if you have joined in the Slack~

To facilitate focused communication among contributors, we’ve created a brand-new dedicated #contributors channel in official Slack workspace.
Slack workspace link: https://s.apache.org/seatunnel-slack

This channel is designed for technical discussions, feature design brainstorming, roadmap sync and direct communication with project core Contributors. Everyone who contributes code, docs, test cases or community ideas is warmly invited to join, share thoughts and collaborate together under The Apache Way. Welcome to join 😄