From 6254c140e370a329e201e49122389eca89320ec3 Mon Sep 17 00:00:00 2001 From: Justin Bertram Date: Tue, 2 May 2023 14:25:21 -0500 Subject: [PATCH] ARTEMIS-4212 handful of updates - fix syntax used for 'addresses' CLI option - update release notes - enforce new semantics on parsing & add test --- .../activemq/artemis/cli/commands/Create.java | 18 ++++++++-- .../apache/activemq/cli/test/ArtemisTest.java | 7 ++-- .../impl/FileConfigurationParser.java | 4 +++ .../core/server/ActiveMQMessageBundle.java | 3 ++ .../impl/FileConfigurationParserTest.java | 14 ++++++++ ...urationParser-addressWithNoRoutingType.xml | 29 +++++++++++++++ docs/user-manual/en/versions.md | 36 +++++++++++++++++++ .../test/resources/reload-diverts-updated.xml | 16 ++++++--- .../src/test/resources/reload-diverts.xml | 16 ++++++--- 9 files changed, 130 insertions(+), 13 deletions(-) create mode 100644 artemis-server/src/test/resources/FileConfigurationParser-addressWithNoRoutingType.xml diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java index afbf809b93..d5af376dc0 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java @@ -218,10 +218,10 @@ public class Create extends InstallAbstract { @Option(name = "--no-web", description = "Whether to omit the web-server definition from bootstrap.xml.") private boolean noWeb; - @Option(name = "--queues", description = "A comma separated list of queues with the option to specify a routing type, e.g. --queues myqueue,mytopic:multicast.") + @Option(name = "--queues", description = "A comma separated list of queues with the option to specify a routing type, e.g. --queues myQueue1,myQueue2:multicast. Routing-type default: anycast.") private String queues; - @Option(name = "--addresses", description = "A comma separated list of addresses.") + @Option(name = "--addresses", description = "A comma separated list of addresses with the option to specify a routing type, e.g. --addresses myAddress1,myAddress2:anycast. Routing-type default: multicast.") private String addresses; @Option(name = "--aio", description = "Set the journal as asyncio.") @@ -1001,7 +1001,19 @@ public class Create extends InstallAbstract { printWriter.println(" "); } for (String str : getAddressList()) { - printWriter.println("
"); + String[] seg = str.split(":"); + String name = seg[0].trim(); + // default routing type to multicast if not specified + String routingType = (seg.length == 2 ? seg[1].trim() : "multicast"); + try { + RoutingType.valueOf(routingType.toUpperCase()); + } catch (Exception e) { + e.printStackTrace(); + System.err.println("Invalid routing type: " + routingType); + } + printWriter.println("
"); + printWriter.println(" <" + routingType + "/>"); + printWriter.println("
"); } filters.put("${address-queue.settings}", writer.toString()); } diff --git a/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java b/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java index a582b9eedb..9d62df9e53 100644 --- a/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java +++ b/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java @@ -1369,7 +1369,7 @@ public class ArtemisTest extends CliTestBase { setupAuth(instanceFolder); String queues = "q1,q2:multicast"; - String addresses = "a1,a2"; + String addresses = "a1,a2:anycast"; // This is usually set when run from the command line via artemis.profile @@ -1393,8 +1393,11 @@ public class ArtemisTest extends CliTestBase { assertEquals(routingType, queryResult.getRoutingType()); } for (String str : addresses.split(",")) { - ClientSession.AddressQuery queryResult = coreSession.addressQuery(SimpleString.toSimpleString(str)); + String[] seg = str.split(":"); + RoutingType routingType = RoutingType.valueOf((seg.length == 2 ? seg[1] : "multicast").toUpperCase()); + ClientSession.AddressQuery queryResult = coreSession.addressQuery(SimpleString.toSimpleString(seg[0])); assertTrue("Couldn't find address " + str, queryResult.isExists()); + assertTrue(routingType == RoutingType.ANYCAST ? queryResult.isSupportsAnycast() : queryResult.isSupportsMulticast()); } } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java index c8dbf6e692..82e083cced 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java @@ -85,6 +85,7 @@ import org.apache.activemq.artemis.core.config.storage.DatabaseStorageConfigurat import org.apache.activemq.artemis.core.config.storage.FileStorageConfiguration; import org.apache.activemq.artemis.core.io.aio.AIOSequentialFileFactory; import org.apache.activemq.artemis.core.security.Role; +import org.apache.activemq.artemis.core.server.ActiveMQMessageBundle; import org.apache.activemq.artemis.core.server.ActiveMQServerLogger; import org.apache.activemq.artemis.core.server.ComponentConfigurationRoutingType; import org.apache.activemq.artemis.core.server.JournalType; @@ -1575,6 +1576,9 @@ public final class FileConfigurationParser extends XMLConfigurationUtil { List queueConfigurations = new ArrayList<>(); NodeList children = node.getChildNodes(); + if (children.getLength() == 0) { + throw ActiveMQMessageBundle.BUNDLE.addressWithNoRoutingType(name); + } for (int j = 0; j < children.getLength(); j++) { Node child = children.item(j); if (child.getNodeName().equals("multicast")) { diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQMessageBundle.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQMessageBundle.java index 05625fd3af..886595ff5e 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQMessageBundle.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQMessageBundle.java @@ -530,4 +530,7 @@ public interface ActiveMQMessageBundle { @Message(id = 229246, value = "Invalid page full message policy type {}") IllegalArgumentException invalidPageFullPolicyType(String val); + + @Message(id = 229247, value = "Invalid address configuration for '{}'. Address must support multicast and/or anycast.") + IllegalArgumentException addressWithNoRoutingType(String address); } diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationParserTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationParserTest.java index 1cf36fca22..f100623800 100644 --- a/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationParserTest.java +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationParserTest.java @@ -96,6 +96,20 @@ public class FileConfigurationParserTest extends ActiveMQTestBase { assertEquals(0, server.locateQueue(SimpleString.toSimpleString("q")).getMaxConsumers()); } + @Test + public void testAddressWithNoRoutingType() throws Exception { + String filename = "FileConfigurationParser-addressWithNoRoutingType.xml"; + FileConfiguration fc = new FileConfiguration(); + FileDeploymentManager deploymentManager = new FileDeploymentManager(filename); + deploymentManager.addDeployable(fc); + try { + deploymentManager.readConfiguration(); + fail(); + } catch (IllegalArgumentException e) { + // expected exception when address has no routing type configured + } + } + @Test public void testDuplicateAddressSettings() throws Exception { FileConfigurationParser parser = new FileConfigurationParser(); diff --git a/artemis-server/src/test/resources/FileConfigurationParser-addressWithNoRoutingType.xml b/artemis-server/src/test/resources/FileConfigurationParser-addressWithNoRoutingType.xml new file mode 100644 index 0000000000..fa2385c8b5 --- /dev/null +++ b/artemis-server/src/test/resources/FileConfigurationParser-addressWithNoRoutingType.xml @@ -0,0 +1,29 @@ + + + + + + false + +
+ + + + diff --git a/docs/user-manual/en/versions.md b/docs/user-manual/en/versions.md index 7c2029f28a..2a6668b3c0 100644 --- a/docs/user-manual/en/versions.md +++ b/docs/user-manual/en/versions.md @@ -22,6 +22,42 @@ Highlights: operational access to these MBeans will now have to be manually enabled in `management.xml` either by changing the `default-access` (not recommended) or specifically configuring a `role-access` for the particular MBean in question. Note: this applies to all MBean access including directly via JMX and via the Jolokia JMX-HTTP bridge. +* Due to [ARTEMIS-4212](https://issues.apache.org/jira/browse/ARTEMIS-4212) the broker will reject address definitions + in `broker.xml` which don't specify a routing type, e.g.: + ```xml +
+ ``` + Such configurations will need to be changed to specify a routing-type, e.g.: + ```xml +
+ +
+ ``` + Or + ```xml +
+ +
+ ``` + If an address without a routing type is configured the broker will throw an exception like this and fail to start: + ``` + java.lang.IllegalArgumentException: AMQ229247: Invalid address configuration for 'myAddress'. Address must support multicast and/or anycast. + at org.apache.activemq.artemis.core.deployers.impl.FileConfigurationParser.parseAddressConfiguration(FileConfigurationParser.java:1580) + at org.apache.activemq.artemis.core.deployers.impl.FileConfigurationParser.parseAddresses(FileConfigurationParser.java:1038) + at org.apache.activemq.artemis.core.deployers.impl.FileConfigurationParser.parseMainConfig(FileConfigurationParser.java:804) + at org.apache.activemq.artemis.core.config.impl.FileConfiguration.parse(FileConfiguration.java:56) + at org.apache.activemq.artemis.core.config.FileDeploymentManager.readConfiguration(FileDeploymentManager.java:81) + at org.apache.activemq.artemis.integration.FileBroker.createComponents(FileBroker.java:120) + at org.apache.activemq.artemis.cli.commands.Run.execute(Run.java:119) + at org.apache.activemq.artemis.cli.Artemis.internalExecute(Artemis.java:212) + at org.apache.activemq.artemis.cli.Artemis.execute(Artemis.java:162) + at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) + at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) + at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) + at java.base/java.lang.reflect.Method.invoke(Method.java:566) + at org.apache.activemq.artemis.boot.Artemis.execute(Artemis.java:144) + at org.apache.activemq.artemis.boot.Artemis.main(Artemis.java:61) + ``` ## 2.28.0 [Full release notes](https://issues.apache.org/jira/secure/ReleaseNote.jspa?version=12352523&projectId=12315920) diff --git a/tests/integration-tests/src/test/resources/reload-diverts-updated.xml b/tests/integration-tests/src/test/resources/reload-diverts-updated.xml index 4afc44f320..a36f556cde 100644 --- a/tests/integration-tests/src/test/resources/reload-diverts-updated.xml +++ b/tests/integration-tests/src/test/resources/reload-diverts-updated.xml @@ -48,10 +48,18 @@ under the License. -
-
-
-
+
+ +
+
+ +
+
+ +
+
+ +
diff --git a/tests/integration-tests/src/test/resources/reload-diverts.xml b/tests/integration-tests/src/test/resources/reload-diverts.xml index 6b18f62573..98eee5d0a3 100644 --- a/tests/integration-tests/src/test/resources/reload-diverts.xml +++ b/tests/integration-tests/src/test/resources/reload-diverts.xml @@ -48,10 +48,18 @@ under the License. -
-
-
-
+
+ +
+
+ +
+
+ +
+
+ +