[GitHub] [activemq-artemis] brusdev commented on a change in pull request #3741: ARTEMIS-3474 - replace whitelist with allowlist in management.xml

[GitBox <gi...@apache.org>]

brusdev commented on a change in pull request #3741:
URL: https://github.com/apache/activemq-artemis/pull/3741#discussion_r707089490



##########
File path: artemis-cli/src/main/java/org/apache/activemq/artemis/cli/factory/jmx/ManagementFactory.java
##########
@@ -67,9 +69,19 @@ public static ManagementContext create(ManagementContextDTO config, ActiveMQSecu
       if (config.getAuthorisation() != null) {
          AuthorisationDTO authorisation = config.getAuthorisation();
          JMXAccessControlList accessControlList = new JMXAccessControlList();
-         List<EntryDTO> entries = authorisation.getWhiteList().getEntries();
-         for (EntryDTO entry : entries) {
-            accessControlList.addToWhiteList(entry.domain, entry.key);
+         //deprecated but here for backward compatibility
+         WhiteListDTO whiteList = authorisation.getWhiteList();
+         if (whiteList != null) {
+            for (EntryDTO entry : whiteList.getEntries()) {
+               accessControlList.addToAllowList(entry.domain, entry.key);

Review comment:
       A warning could help soft migration to allowList before the removal

##########
File path: artemis-cli/src/main/java/org/apache/activemq/artemis/cli/factory/jmx/ManagementFactory.java
##########
@@ -67,9 +69,19 @@ public static ManagementContext create(ManagementContextDTO config, ActiveMQSecu
       if (config.getAuthorisation() != null) {
          AuthorisationDTO authorisation = config.getAuthorisation();
          JMXAccessControlList accessControlList = new JMXAccessControlList();
-         List<EntryDTO> entries = authorisation.getWhiteList().getEntries();
-         for (EntryDTO entry : entries) {
-            accessControlList.addToWhiteList(entry.domain, entry.key);
+         //deprecated but here for backward compatibility
+         WhiteListDTO whiteList = authorisation.getWhiteList();
+         if (whiteList != null) {
+            for (EntryDTO entry : whiteList.getEntries()) {
+               accessControlList.addToAllowList(entry.domain, entry.key);
+            }
+         }
+
+         AllowListDTO allowList = authorisation.getAllowList();
+         if (allowList != null) {

Review comment:
       An error if both whiteList and allowList are not null could help to detect wrong configs

##########
File path: docs/user-manual/en/management.md
##########
@@ -305,26 +305,26 @@ the broker's JAAS plugin support.  This is configured via the `authorisation`
 element in the `management.xml` configuration file and can be used to restrict
 access to attributes and methods on MBeans.
 
-There are 3 elements within the `authorisation` element, `whitelist`,
+There are 3 elements within the `authorisation` element, `allowlist`,

Review comment:
       Maybe, a comment on the deprecation could help to manage previous configs




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org