[WFCORE-7116] Remove ModuleIdentifier from Extension handling

wildfly · Jan 14, 2025 · 1d34f6b · 1d34f6b
1 parent 61d6404
commit 1d34f6b
Show file tree

Hide file tree

Showing 7 changed files with 34 additions and 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
@@ -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<String> 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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/...oller/src/main/java/org/jboss/as/domain/controller/operations/ApplyExtensionsHandler.java b/...oller/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 {