Uh oh!
There was an error while loading. Please reload this page.
[fix](plugin) Allow legacy unversioned authentication plugins - #67286
[fix](plugin) Allow legacy unversioned authentication plugins#67286CalvinKirs wants to merge 1 commit into
Conversation
hello-stephen
commented
Aug 28, 2026
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
CalvinKirs
commented
Aug 28, 2026
run buildall |
hello-stephen
commented
Aug 28, 2026
TPC-H: Total hot run time: 16967 ms |
hello-stephen
commented
Aug 28, 2026
TPC-DS: Total hot run time: 82569 ms |
hello-stephen
commented
Aug 28, 2026
ClickBench: Total hot run time: 14.73 s |
morningman
commented
Aug 31, 2026
/review |
CalvinKirs
commented
Sep 3, 2026
run external |
CalvinKirs
commented
Sep 3, 2026
/review |
There was a problem hiding this comment.
Request changes: the legacy compatibility branch makes a directory-loader manifest lookup failure indistinguishable from a genuinely unversioned legacy artifact, so an unverifiable authentication plugin can be published.
Critical checkpoints:
- Goal and proof: readable legacy authentication JARs are now accepted with a warning, while blank, malformed, and incompatible declarations remain rejected. The goal is not fully met because unresolved or unreadable defining-JAR metadata also enters the legacy branch; the missing loader-level negative test is called out inline.
- Scope and clarity: the five-file change is focused, and the generic gate keeps legacy behavior opt-in.
- Concurrency and lifecycle: no new shared mutable state or lock order is introduced; the immutable gate and warning run inside the existing load lifecycle. External JAR mutation is not protected by that lock and exposes the accepted issue. Existing
PluginLoadExceptioncleanup would close the classloader once read failures are propagated. - Compatibility and parallel paths: repository history supports authentication API 1.0 as the pre-manifest contract, and the fixed legacy major is compared with the independently filtered current major, so a future 2.x FE rejects it. Connector, filesystem, and lineage remain strict. The classpath-provider path also remains fail-closed on JAR I/O errors; only the directory path has the inline inconsistency.
- Tests and results: the added tests correctly cover readable absence, warning selection, malformed legacy configuration, and legacy/current major mismatch. No result files changed. I did not run local builds or tests because this review environment forbids them; the live PR currently reports Compile, FE UT, and CheckStyle passing.
- Observability and performance: the warning is useful and negligible for true legacy loads, but becomes misleading on the accepted failure path. No other hot-path or observability concern was found.
- Configuration, persistence, transactions, data writes, and FE-BE protocol variables: not applicable to this change.
- User focus: no additional focus was provided.
Review completion: two bounded rounds completed with two full-coverage lanes and one separate risk-focused lane. Round 2 returned NO_NEW_VALUABLE_FINDINGS in all lanes; every candidate was accepted, deduplicated, or dismissed with evidence. One inline issue remains.
| return "no " + manifestAttribute + " in the plugin jar MANIFEST; this FE serves " + familyLabel | ||
| + " plugin API " + expectedVersion + ". Declare the attribute and rebuild the plugin" | ||
| + " against this Doris release."; | ||
| if (declaredVersion == null) { |
There was a problem hiding this comment.
[P2] Distinguish an absent declaration from a failed manifest lookup
On the directory-loading path, null does not prove that a readable defining JAR genuinely omitted this attribute. readFromDefiningJar also returns null after every JarFile/manifest IOException, or when the fallback scan cannot find the factory class. For example, lazy loading can define a self-contained factory and then race a JAR removal or permission/read failure before the separate manifest opens; this branch then admits and publishes the unverifiable plugin as legacy 1.0. Preserve an explicit successfully-read-but-absent result for this legacy case, reject unresolved/read failures at STAGE_API_VERSION, and add a loader-level negative test alongside the true-absence case.
What problem does this PR solve?
#66211
Problem Summary:
The authentication plugin API-version gate rejects every plugin jar that does not contain
Doris-Authentication-Plugin-Api-Version. Authentication plugins were distributed before this manifest contract existed, so upgrading FE breaks those legacy artifacts even when they implement the current v1 API.This change adds an opt-in legacy mapping to the generic API gate and configures only the authentication family to treat a genuinely absent declaration as API 1.0: