diff --git a/fe/fe-authentication/fe-authentication-handler/src/main/java/org/apache/doris/authentication/handler/AuthenticationPluginManager.java b/fe/fe-authentication/fe-authentication-handler/src/main/java/org/apache/doris/authentication/handler/AuthenticationPluginManager.java index 3565013dca8740..202bef69339435 100644 --- a/fe/fe-authentication/fe-authentication-handler/src/main/java/org/apache/doris/authentication/handler/AuthenticationPluginManager.java +++ b/fe/fe-authentication/fe-authentication-handler/src/main/java/org/apache/doris/authentication/handler/AuthenticationPluginManager.java @@ -66,15 +66,19 @@ public class AuthenticationPluginManager { /** Family label in the process-wide {@link PluginRegistry}. */ private static final String PLUGIN_FAMILY = "AUTHENTICATION"; + private static final String LEGACY_UNVERSIONED_API_VERSION = "1.0"; /** * The authentication plugin API contract this FE serves. Built from the version filtered into * fe-authentication-spi at build time, anchored on {@link AuthenticationPluginFactory} so that it is read * from the very artifact carrying the SPI. A missing or malformed resource is a build defect and fails - * class initialization loudly rather than degrading into a check that admits everything. + * class initialization loudly rather than degrading into a check that admits everything. Authentication + * plugins shipped before the manifest contract are treated as API 1.0 for upgrade compatibility; explicit + * malformed or incompatible declarations remain rejected. */ private static final ApiVersionGate API_VERSION_GATE = - ApiVersionGate.forFamily("authentication", AuthenticationPluginFactory.class); + ApiVersionGate.forFamilyAllowingUnversioned( + "authentication", AuthenticationPluginFactory.class, LEGACY_UNVERSIONED_API_VERSION); /** Factories by plugin name (e.g., "ldap", "oidc", "password") */ private final Map factories = new ConcurrentHashMap<>(); diff --git a/fe/fe-authentication/fe-authentication-handler/src/test/java/org/apache/doris/authentication/handler/AuthenticationPluginManagerTest.java b/fe/fe-authentication/fe-authentication-handler/src/test/java/org/apache/doris/authentication/handler/AuthenticationPluginManagerTest.java index 10f929712eea3b..0c38463d220c9f 100644 --- a/fe/fe-authentication/fe-authentication-handler/src/test/java/org/apache/doris/authentication/handler/AuthenticationPluginManagerTest.java +++ b/fe/fe-authentication/fe-authentication-handler/src/test/java/org/apache/doris/authentication/handler/AuthenticationPluginManagerTest.java @@ -27,7 +27,6 @@ import org.apache.doris.extension.loader.ApiVersionGate; import org.apache.doris.extension.loader.ClassLoadingPolicy; import org.apache.doris.extension.loader.DirectoryPluginRuntimeManager; -import org.apache.doris.extension.loader.LoadFailure; import org.apache.doris.extension.loader.LoadReport; import org.apache.doris.extension.loader.PluginHandle; @@ -405,27 +404,22 @@ void testLoadAll_IncompatibleApiVersionIsRefusedWithDiagnosableReason() throws E } @Test - @DisplayName("UT-HANDLER-PM-022: Plugin declaring no API version is refused (fail-closed)") - void testLoadAll_UndeclaredApiVersionIsRefused() throws Exception { - // Given: a jar built with no awareness of this contract at all. + @DisplayName("UT-HANDLER-PM-022: Legacy plugin declaring no API version remains loadable") + void testLoadAll_UndeclaredLegacyApiVersionIsAccepted() throws Exception { + // Given: an authentication plugin built before the manifest version contract existed. Path root = Files.createTempDirectory("plugin-root-no-api-version"); createPluginJar( Files.createDirectories(root.resolve("external-dir-test")).resolve("external-dir-test.jar"), DirectoryPluginFactory.class.getName(), null); - // When: this is the only directory, so loadAll reports total failure... - AuthenticationException ex = Assertions.assertThrows( - AuthenticationException.class, - () -> pluginManager.loadAll( - Arrays.asList(root), - Thread.currentThread().getContextClassLoader())); + // When + pluginManager.loadAll(Arrays.asList(root), Thread.currentThread().getContextClassLoader()); - // Then: ...and the thrown message already carries the version reason, so the caller that wraps it - // does not have to. - Assertions.assertFalse(pluginManager.hasFactory("external-dir-test")); - Assertions.assertTrue(ex.getMessage().contains(LoadFailure.STAGE_API_VERSION), ex.getMessage()); - Assertions.assertTrue(ex.getMessage().contains(AUTH_GATE.getManifestAttribute()), ex.getMessage()); + // Then: absence maps to the documented legacy API 1.0, while explicitly incompatible versions are + // still covered by UT-HANDLER-PM-021. + Assertions.assertTrue(pluginManager.hasFactory("external-dir-test")); + Assertions.assertEquals("", pluginManager.apiVersionRejectionHint()); } @Test diff --git a/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/ApiVersionGate.java b/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/ApiVersionGate.java index 63040c303ac506..e0949edfa99330 100644 --- a/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/ApiVersionGate.java +++ b/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/ApiVersionGate.java @@ -64,7 +64,9 @@ * *

Major must match exactly; minor and patch are ignored in both directions. That is only sound because * "major" is defined as any change to the SPI surface — including additions — so a compatible minor - * can never change the set of API elements a plugin can see. + * can never change the set of API elements a plugin can see. Families that distributed plugins before this + * manifest contract existed may explicitly map an absent declaration to their documented legacy version; + * strict rejection remains the default, and explicit declarations always follow the same major rule. */ public final class ApiVersionGate { @@ -75,14 +77,19 @@ public final class ApiVersionGate { private final String manifestAttribute; private final String expectedVersion; private final int expectedMajor; + private final String legacyUnversionedVersion; + private final int legacyUnversionedMajor; private final ClassLoader kernelClassLoader; private ApiVersionGate(String familyLabel, String manifestAttribute, String expectedVersion, - int expectedMajor, ClassLoader kernelClassLoader) { + int expectedMajor, String legacyUnversionedVersion, int legacyUnversionedMajor, + ClassLoader kernelClassLoader) { this.familyLabel = familyLabel; this.manifestAttribute = manifestAttribute; this.expectedVersion = expectedVersion; this.expectedMajor = expectedMajor; + this.legacyUnversionedVersion = legacyUnversionedVersion; + this.legacyUnversionedMajor = legacyUnversionedMajor; this.kernelClassLoader = kernelClassLoader; } @@ -100,6 +107,29 @@ private ApiVersionGate(String familyLabel, String manifestAttribute, String expe * degrading into a check that admits everything. */ public static ApiVersionGate forFamily(String family, Class spiAnchor) { + return forFamily(family, spiAnchor, null); + } + + /** + * Builds a gate that maps a genuinely absent manifest declaration to a documented legacy API version. + * + *

This is only for a plugin family that shipped external plugins before the manifest contract existed. + * It preserves upgrade compatibility for those artifacts without weakening validation for malformed + * declarations or explicitly incompatible majors. If the FE later moves to another major, unversioned + * plugins are rejected automatically because their legacy major no longer matches. + * + * @param family lower-case single-token family name + * @param spiAnchor a type from the family's SPI artifact + * @param legacyUnversionedVersion API version implemented by artifacts that predate the manifest contract + */ + public static ApiVersionGate forFamilyAllowingUnversioned( + String family, Class spiAnchor, String legacyUnversionedVersion) { + Objects.requireNonNull(legacyUnversionedVersion, "legacyUnversionedVersion"); + return forFamily(family, spiAnchor, legacyUnversionedVersion); + } + + private static ApiVersionGate forFamily( + String family, Class spiAnchor, String legacyUnversionedVersion) { Objects.requireNonNull(family, "family"); Objects.requireNonNull(spiAnchor, "spiAnchor"); String resource = versionResourceOf(family); @@ -110,8 +140,19 @@ public static ApiVersionGate forFamily(String family, Class spiAnchor) { + " (from " + spiAnchor.getName() + "); expected major[.minor[.patch]]. This is a build" + " defect: the maven property feeding this resource is missing or was not filtered."); } + int legacyMajor = -1; + if (legacyUnversionedVersion != null) { + OptionalInt parsedLegacyMajor = parseMajor(legacyUnversionedVersion); + if (!parsedLegacyMajor.isPresent()) { + throw new IllegalArgumentException("Malformed legacy unversioned API version='" + + legacyUnversionedVersion + "' for " + family + "; expected major[.minor[.patch]]"); + } + legacyUnversionedVersion = legacyUnversionedVersion.trim(); + legacyMajor = parsedLegacyMajor.getAsInt(); + } return new ApiVersionGate(family.toUpperCase(Locale.ROOT), manifestAttributeOf(family), - declared.trim(), major.getAsInt(), spiAnchor.getClassLoader()); + declared.trim(), major.getAsInt(), legacyUnversionedVersion, legacyMajor, + spiAnchor.getClassLoader()); } /** Kernel-side resource path for a family, by convention. */ @@ -155,18 +196,28 @@ public int getExpectedMajor() { /** * Judges what a plugin jar declared. * - *

Fail-closed: a jar that declares nothing is rejected, so that a plugin built without any awareness of - * this contract cannot slip through. A third party can still get in by declaring a version it was not - * built against, but that is an active false claim rather than an omission. + *

Strict gates reject a jar that declares nothing. A family that explicitly configures a legacy + * unversioned version may accept an absent declaration when that legacy major matches the FE. Blank, + * malformed, and explicitly incompatible declarations always remain fail-closed. * * @param declaredVersion the value of {@link #getManifestAttribute()} in the plugin jar, or null when absent * @return null when the plugin may load, otherwise a diagnostic naming the declared and expected values */ public String rejectionReason(String declaredVersion) { - if (declaredVersion == null || declaredVersion.trim().isEmpty()) { - 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) { + if (legacyUnversionedVersion == null) { + return missingDeclarationReason(); + } + if (legacyUnversionedMajor != expectedMajor) { + return "unversioned legacy " + familyLabel + " plugin API " + legacyUnversionedVersion + + " is incompatible with this FE, which serves plugin API " + expectedVersion + + ". Rebuild the plugin against this Doris release."; + } + return null; + } + if (declaredVersion.trim().isEmpty()) { + return "empty " + manifestAttribute + " in the plugin jar MANIFEST; this FE serves " + familyLabel + + " plugin API " + expectedVersion + "."; } OptionalInt major = parseMajor(declaredVersion); if (!major.isPresent()) { @@ -183,21 +234,40 @@ public String rejectionReason(String declaredVersion) { return null; } + /** + * Returns an operator-facing warning when an accepted plugin relied on the legacy unversioned mapping. + */ + public String acceptanceWarning(String declaredVersion) { + if (declaredVersion == null && legacyUnversionedVersion != null + && legacyUnversionedMajor == expectedMajor) { + return "plugin jar has no " + manifestAttribute + "; treating it as legacy " + familyLabel + + " plugin API " + legacyUnversionedVersion + ". Rebuild the plugin with an explicit" + + " API version declaration."; + } + return null; + } + /** Applies the same manifest-major contract to a classpath-discovered provider. */ public String rejectionReasonForClass(Class pluginClass) { Path definingJar = ManifestVersions.jarOf(pluginClass); if (definingJar == null) { // Maven/IDE runs expose kernel-built providers as class directories, but an independently // loaded exploded provider has no immutable declaration and must remain fail-closed. - return pluginClass.getClassLoader() == kernelClassLoader ? null : rejectionReason((String) null); + return pluginClass.getClassLoader() == kernelClassLoader ? null : missingDeclarationReason(); } try (JarFile jar = new JarFile(definingJar.toFile())) { return rejectionReason(ManifestVersions.mainAttribute(jar, manifestAttribute)); } catch (IOException e) { - return rejectionReason((String) null); + return missingDeclarationReason(); } } + private String missingDeclarationReason() { + 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."; + } + /** * Major segment of {@code major[.minor[.patch]]}, or empty when the value is not that shape. * diff --git a/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/DirectoryPluginRuntimeManager.java b/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/DirectoryPluginRuntimeManager.java index 556242738c5c2f..03efc68bcff418 100644 --- a/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/DirectoryPluginRuntimeManager.java +++ b/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/DirectoryPluginRuntimeManager.java @@ -19,6 +19,9 @@ import org.apache.doris.extension.spi.PluginFactory; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + import java.io.Closeable; import java.io.IOException; import java.net.MalformedURLException; @@ -104,6 +107,8 @@ */ public class DirectoryPluginRuntimeManager { + private static final Logger LOG = LogManager.getLogger(DirectoryPluginRuntimeManager.class); + private final ConcurrentMap> handlesByName = new ConcurrentHashMap<>(); private final Object lifecycleLock = new Object(); @@ -288,6 +293,10 @@ private PluginHandle loadFromPluginDir(Path pluginDir, ClassLoader parent, Cl "Rejected plugin in " + normalizedDir + ": " + rejection, null); } + String acceptanceWarning = apiVersionGate.acceptanceWarning(declaredApiVersion); + if (acceptanceWarning != null) { + LOG.warn("Loading legacy unversioned plugin from {}: {}", normalizedDir, acceptanceWarning); + } try { @SuppressWarnings("unchecked") diff --git a/fe/fe-extension-loader/src/test/java/org/apache/doris/extension/loader/ApiVersionGateTest.java b/fe/fe-extension-loader/src/test/java/org/apache/doris/extension/loader/ApiVersionGateTest.java index fa4ccb24151929..06ed183f9264fc 100644 --- a/fe/fe-extension-loader/src/test/java/org/apache/doris/extension/loader/ApiVersionGateTest.java +++ b/fe/fe-extension-loader/src/test/java/org/apache/doris/extension/loader/ApiVersionGateTest.java @@ -21,8 +21,9 @@ import org.junit.jupiter.api.Test; /** - * The decision rule itself: major must match, minor and patch never matter, and anything a plugin did not - * clearly declare is refused. + * The decision rule itself: major must match and minor and patch never matter. Strict gates refuse missing + * declarations; an explicitly configured legacy gate accepts only a genuinely absent declaration for the + * documented legacy major. * *

Each case here encodes a promise made to plugin authors, not just current behavior. The two-way minor * cases are the load-bearing ones: they are only sound because "major" is defined to cover additions @@ -37,6 +38,8 @@ class ApiVersionGateTest { private static final ApiVersionGate GATE = ApiVersionGate.forFamily("test", ApiVersionGateTest.class); + private static final ApiVersionGate LEGACY_GATE = + ApiVersionGate.forFamilyAllowingUnversioned("test", ApiVersionGateTest.class, "7.0"); @Test void testKernelVersionComesFromTheFilteredResource() { @@ -116,6 +119,36 @@ void testMissingDeclarationIsRejectedFailClosed() { Assertions.assertNotNull(GATE.rejectionReason(" ")); } + @Test + void testDocumentedLegacyUnversionedPluginIsAcceptedWithWarning() { + Assertions.assertNull(LEGACY_GATE.rejectionReason(null)); + String warning = LEGACY_GATE.acceptanceWarning(null); + Assertions.assertNotNull(warning); + Assertions.assertTrue(warning.contains("legacy TEST plugin API 7.0"), warning); + + Assertions.assertNotNull(LEGACY_GATE.rejectionReason(""), + "an explicit empty declaration is malformed, not a legacy artifact"); + Assertions.assertNull(LEGACY_GATE.acceptanceWarning("7.0"), + "a versioned plugin must not be mislabeled as legacy"); + } + + @Test + void testLegacyUnversionedPluginIsRejectedAfterKernelMajorUpgrade() { + ApiVersionGate oldLegacy = + ApiVersionGate.forFamilyAllowingUnversioned("test", ApiVersionGateTest.class, "6.9"); + String reason = oldLegacy.rejectionReason(null); + Assertions.assertNotNull(reason); + Assertions.assertTrue(reason.contains("6.9") && reason.contains("7.2"), reason); + Assertions.assertNull(oldLegacy.acceptanceWarning(null)); + } + + @Test + void testMalformedLegacyVersionConfigurationFailsLoudly() { + Assertions.assertThrows(IllegalArgumentException.class, + () -> ApiVersionGate.forFamilyAllowingUnversioned( + "test", ApiVersionGateTest.class, "legacy-v7")); + } + @Test void testMalformedDeclarationIsRejectedRatherThanTruncated() { // "7.x" must not be read as major 7. Silently salvaging a major out of a typo would admit a plugin