ARTEMIS-4212 handful of updates

- fix syntax used for 'addresses' CLI option - update release notes - enforce new semantics on parsing & add test
apache · May 4, 2023 · 6254c14 · 6254c14
1 parent 9a7302e
commit 6254c14
Show file tree

Hide file tree

Showing 9 changed files with 130 additions and 13 deletions.
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
@@ -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 @@ private void applyAddressesAndQueues(HashMap<String, String> filters) {
          printWriter.println("         </address>");
       }
       for (String str : getAddressList()) {
-         printWriter.println("         <address name=\"" + str + "\"/>");
+         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("         <address name=\"" + name + "\">");
+         printWriter.println("            <" + routingType + "/>");
+         printWriter.println("         </address>");
       }
       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
@@ -1369,7 +1369,7 @@ public void testSimpleRun(String folderName, int acceptorPort) throws Exception
 
       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 void testSimpleRun(String folderName, int acceptorPort) throws Exception
                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/...rc/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java b/...rc/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java
@@ -85,6 +85,7 @@
 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 @@ protected CoreAddressConfiguration parseAddressConfiguration(final Node node) {
 
       List<QueueConfiguration> 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/...s-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQMessageBundle.java b/...s-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQMessageBundle.java
@@ -530,4 +530,7 @@ IllegalStateException invalidRoutingTypeUpdate(String queueName,
 
    @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/...c/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationParserTest.java b/...c/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationParserTest.java
@@ -96,6 +96,20 @@ public void testDuplicateQueue() throws Exception {
       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
@@ -0,0 +1,29 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<configuration xmlns="urn:activemq"
+               xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+               xsi:schemaLocation="urn:activemq /schema/artemis-configuration.xsd">
+
+
+   <core xmlns="urn:activemq:core">
+      <persistence-enabled>false</persistence-enabled>
+      <addresses>
+         <address name="a"/>
+      </addresses>
+   </core>
+
+</configuration>
diff --git 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
+  <address name="myAddress"/>
+  ```
+  Such configurations will need to be changed to specify a routing-type, e.g.:
+  ```xml
+  <address name="myAddress">
+     <anycast/>
+  </address>
+  ```
+  Or
+  ```xml
+  <address name="myAddress">
+     <multicast/>
+  </address>
+  ```
+  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
@@ -48,10 +48,18 @@ under the License.
       </acceptors>
 
       <addresses>
-         <address name="a"/>
-         <address name="b"/>
-         <address name="c"/>
-         <address name="target"/>
+         <address name="a">
+            <multicast/>
+         </address>
+         <address name="b">
+            <multicast/>
+         </address>
+         <address name="c">
+            <multicast/>
+         </address>
+         <address name="target">
+            <multicast/>
+         </address>
       </addresses>
 
       <diverts>

diff --git 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.
       </acceptors>
 
       <addresses>
-         <address name="a"/>
-         <address name="b"/>
-         <address name="c"/>
-         <address name="target"/>
+         <address name="a">
+            <multicast/>
+         </address>
+         <address name="b">
+            <multicast/>
+         </address>
+         <address name="c">
+            <multicast/>
+         </address>
+         <address name="target">
+            <multicast/>
+         </address>
       </addresses>
 
       <diverts>