From 1d34f6b8b3d5d4ea2a1d020f481e492cc3a07e41 Mon Sep 17 00:00:00 2001 From: Brian Stansberry Date: Sun, 22 Dec 2024 20:13:52 -0500 Subject: [PATCH] [WFCORE-7116] Remove ModuleIdentifier from Extension handling --- .../java/org/jboss/as/cli/impl/ExtensionsLoader.java | 11 +++++++---- .../as/controller/extension/ExtensionAddHandler.java | 10 +++++++++- .../jboss/as/controller/logging/ControllerLogger.java | 3 +++ .../controller/parsing/DeferredExtensionContext.java | 3 +-- .../org/jboss/as/controller/parsing/ExtensionXml.java | 11 ++++++++++- .../as/controller/transform/TransformerRegistry.java | 3 +-- .../controller/operations/ApplyExtensionsHandler.java | 5 +++-- 7 files changed, 34 insertions(+), 12 deletions(-) diff --git a/cli/src/main/java/org/jboss/as/cli/impl/ExtensionsLoader.java b/cli/src/main/java/org/jboss/as/cli/impl/ExtensionsLoader.java index e75ba00ef64..012533c9578 100644 --- a/cli/src/main/java/org/jboss/as/cli/impl/ExtensionsLoader.java +++ b/cli/src/main/java/org/jboss/as/cli/impl/ExtensionsLoader.java @@ -29,7 +29,6 @@ import org.jboss.dmr.ModelNode; import org.jboss.dmr.Property; import org.jboss.modules.ModuleClassLoader; -import org.jboss.modules.ModuleIdentifier; import org.jboss.modules.ModuleLoadException; import org.jboss.modules.ModuleLoader; @@ -102,9 +101,7 @@ List getExtensionsErrors() { * Using the client, iterates through the available domain management model extensions * and tries to load CLI command handlers from their modules. * - * @param registry * @param address - * @param client */ void loadHandlers(ControllerAddress address) throws CommandLineException, CommandLineParserException { @@ -150,7 +147,7 @@ void loadHandlers(ControllerAddress address) throws CommandLineException, Comman if(!module.isDefined()) { addError("Extension " + ext.getName() + " is missing module attribute"); } else { - final ModuleIdentifier moduleId = ModuleIdentifier.fromString(module.asString()); + final String moduleId = canonicalize(module.asString()); ModuleClassLoader cl; try { cl = moduleLoader.loadModule(moduleId).getClassLoader(); @@ -213,6 +210,12 @@ private void addError(String msg) { } } + private static String canonicalize(String moduleName) { + // This isn't a full canonicalization, but extension module names are unlikely to be non-canonical + // in any way beyond maybe a trailing ":main". So just check for and strip any trailing ":main" + return moduleName.endsWith(":main") ? moduleName.substring(0, moduleName.length() - 5) : moduleName; + } + class ExtensionCommandsHandler extends CommandHandlerWithHelp { private static final String NAME = "extension-commands"; diff --git a/controller/src/main/java/org/jboss/as/controller/extension/ExtensionAddHandler.java b/controller/src/main/java/org/jboss/as/controller/extension/ExtensionAddHandler.java index 6f5c6dde2e9..e1afdd46011 100644 --- a/controller/src/main/java/org/jboss/as/controller/extension/ExtensionAddHandler.java +++ b/controller/src/main/java/org/jboss/as/controller/extension/ExtensionAddHandler.java @@ -9,6 +9,7 @@ import java.util.Iterator; import org.jboss.as.controller.Extension; +import org.jboss.as.controller.ModuleIdentifierUtil; import org.jboss.as.controller.OperationContext; import org.jboss.as.controller.OperationFailedException; import org.jboss.as.controller.OperationStepHandler; @@ -58,7 +59,14 @@ public ExtensionAddHandler(final ExtensionRegistry extensionRegistry, final bool @Override public void execute(OperationContext context, ModelNode operation) throws OperationFailedException { - final String moduleName = context.getCurrentAddressValue(); + + // Require canonical module names + final String addressValue = context.getCurrentAddressValue(); + final String moduleName = ModuleIdentifierUtil.canonicalModuleIdentifier(addressValue); + if (!addressValue.equals(moduleName)) { + throw new OperationFailedException(ControllerLogger.MGMT_OP_LOGGER.nonCanonicalExtensionName(addressValue, moduleName)); + } + final ExtensionResource resource = new ExtensionResource(moduleName, extensionRegistry); context.addResource(PathAddress.EMPTY_ADDRESS, resource); diff --git a/controller/src/main/java/org/jboss/as/controller/logging/ControllerLogger.java b/controller/src/main/java/org/jboss/as/controller/logging/ControllerLogger.java index 3738d8b45dc..98e54581407 100644 --- a/controller/src/main/java/org/jboss/as/controller/logging/ControllerLogger.java +++ b/controller/src/main/java/org/jboss/as/controller/logging/ControllerLogger.java @@ -3809,4 +3809,7 @@ OperationFailedRuntimeException capabilityAlreadyRegisteredInContext(String capa @LogMessage(level = WARN) @Message(id = 517, value = "There are multiple Parallel Boot Operations.") void multipleParallelBootOperation(); + + @Message(id = 518, value = "Resource /extension=%s uses a non-canonical module name; use the canonical form %s") + String nonCanonicalExtensionName(String nonCanonical, String canonical); } diff --git a/controller/src/main/java/org/jboss/as/controller/parsing/DeferredExtensionContext.java b/controller/src/main/java/org/jboss/as/controller/parsing/DeferredExtensionContext.java index 0a8508ea0da..d58a83e9f08 100644 --- a/controller/src/main/java/org/jboss/as/controller/parsing/DeferredExtensionContext.java +++ b/controller/src/main/java/org/jboss/as/controller/parsing/DeferredExtensionContext.java @@ -20,7 +20,6 @@ import org.jboss.as.controller.extension.ExtensionRegistry; import org.jboss.as.controller.logging.ControllerLogger; import org.jboss.modules.Module; -import org.jboss.modules.ModuleIdentifier; import org.jboss.modules.ModuleLoadException; import org.jboss.modules.ModuleLoader; import org.jboss.staxmapper.XMLMapper; @@ -92,7 +91,7 @@ public Void call() throws Exception { private void loadModule(final String moduleName, final XMLMapper xmlMapper) throws XMLStreamException { // Register element handlers for this extension try { - final Module module = moduleLoader.loadModule(ModuleIdentifier.fromString(moduleName)); + final Module module = moduleLoader.loadModule(moduleName); boolean initialized = false; for (final Extension extension : module.loadService(Extension.class)) { ClassLoader oldTccl = WildFlySecurityManager.setCurrentContextClassLoaderPrivileged(extension.getClass()); diff --git a/controller/src/main/java/org/jboss/as/controller/parsing/ExtensionXml.java b/controller/src/main/java/org/jboss/as/controller/parsing/ExtensionXml.java index 7e27dbb34ef..98647cc14d3 100644 --- a/controller/src/main/java/org/jboss/as/controller/parsing/ExtensionXml.java +++ b/controller/src/main/java/org/jboss/as/controller/parsing/ExtensionXml.java @@ -24,6 +24,7 @@ import javax.xml.stream.XMLStreamException; +import org.jboss.as.controller.ModuleIdentifierUtil; import org.jboss.as.controller.extension.ExtensionRegistry; import org.jboss.as.controller.logging.ControllerLogger; import org.jboss.dmr.ModelNode; @@ -94,7 +95,15 @@ public void parseExtensions(final XMLExtendedStreamReader reader, final ModelNod } // One attribute && require no content - final String moduleName = readStringAttributeElement(reader, Attribute.MODULE.getLocalName()); + final String inputModuleName = readStringAttributeElement(reader, Attribute.MODULE.getLocalName()); + + // Require canonical module names + final String moduleName = ModuleIdentifierUtil.canonicalModuleIdentifier(inputModuleName); + if (!inputModuleName.equals(moduleName)) { + throw new XMLStreamException( + ControllerLogger.MGMT_OP_LOGGER.nonCanonicalExtensionName(inputModuleName, moduleName), + reader.getLocation()); + } if (!found.add(moduleName)) { // duplicate module name diff --git a/controller/src/main/java/org/jboss/as/controller/transform/TransformerRegistry.java b/controller/src/main/java/org/jboss/as/controller/transform/TransformerRegistry.java index f20bee6298a..d527f528370 100644 --- a/controller/src/main/java/org/jboss/as/controller/transform/TransformerRegistry.java +++ b/controller/src/main/java/org/jboss/as/controller/transform/TransformerRegistry.java @@ -25,7 +25,6 @@ import org.jboss.dmr.ModelNode; import org.jboss.dmr.Property; import org.jboss.modules.Module; -import org.jboss.modules.ModuleIdentifier; import org.jboss.modules.ModuleLoadException; /** @@ -61,7 +60,7 @@ public void loadAndRegisterTransformers(String name, ModelVersion subsystemVersi try { SubsystemTransformerRegistration transformerRegistration = new SubsystemTransformerRegistrationImpl(name, subsystemVersion); if (Module.getCallerModule() != null) { //only register when running in modular environment, testsuite does its own loading - for (ExtensionTransformerRegistration registration : Module.loadServiceFromCallerModuleLoader(ModuleIdentifier.fromString(extensionModuleName), ExtensionTransformerRegistration.class)) { + for (ExtensionTransformerRegistration registration : Module.loadServiceFromCallerModuleLoader(extensionModuleName, ExtensionTransformerRegistration.class)) { if (registration.getSubsystemName().equals(name)) { //to prevent registering transformers for different subsystems registration.registerTransformers(transformerRegistration); } diff --git a/host-controller/src/main/java/org/jboss/as/domain/controller/operations/ApplyExtensionsHandler.java b/host-controller/src/main/java/org/jboss/as/domain/controller/operations/ApplyExtensionsHandler.java index 238baea92ad..ae1a22a0d4a 100644 --- a/host-controller/src/main/java/org/jboss/as/domain/controller/operations/ApplyExtensionsHandler.java +++ b/host-controller/src/main/java/org/jboss/as/domain/controller/operations/ApplyExtensionsHandler.java @@ -46,7 +46,6 @@ import org.jboss.as.server.operations.ServerProcessStateHandler; import org.jboss.dmr.ModelNode; import org.jboss.modules.Module; -import org.jboss.modules.ModuleIdentifier; import org.jboss.modules.ModuleLoadException; /** @@ -116,6 +115,8 @@ public void execute(OperationContext context, ModelNode operation) throws Operat if (resourceAddress.size() == 1 && resourceAddress.getElement(0).getKey().equals(EXTENSION)) { final String module = resourceAddress.getElement(0).getValue(); if (appliedExtensions.add(module)) { + // We know 'module' is a canonical module name as the primary is our version or later + // and we require canonical names for extension resources initializeExtension(module, rootRegistration); installedExtensions.put(module, resource); } @@ -195,7 +196,7 @@ private Resource getResource(PathAddress resourceAddress, Resource rootResource, protected void initializeExtension(String module, ManagementResourceRegistration rootRegistration) throws OperationFailedException { try { - for (final Extension extension : Module.loadServiceFromCallerModuleLoader(ModuleIdentifier.fromString(module), Extension.class)) { + for (final Extension extension : Module.loadServiceFromCallerModuleLoader(module, Extension.class)) { if (rootRegistration.enables(extension)) { ClassLoader oldTccl = SecurityActions.setThreadContextClassLoader(extension.getClass()); try {