[ISSUE #3281 ][acl] fix fail to delete topic perm list and global white address(#3128) (#3280)

* fix fail to remove topic/group permissions and global white address of acl config

* update acl config for all brokers

* refactor rename UtilAll.string2List and list2String

* fix fail to remove whiteRemoteAddress of acl config

acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java

@@ -18,13 +18,6 @@ package org.apache.rocketmq.acl.plain;
 
 import com.alibaba.fastjson.JSONArray;
 import com.alibaba.fastjson.JSONObject;
-import java.io.File;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.rocketmq.acl.common.AclConstants;
 import org.apache.rocketmq.acl.common.AclException;
@@ -39,6 +32,14 @@ import org.apache.rocketmq.logging.InternalLogger;
 import org.apache.rocketmq.logging.InternalLoggerFactory;
 import org.apache.rocketmq.srvutil.FileWatchService;
 
+import java.io.File;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
 public class PlainPermissionManager {
 
     private static final InternalLogger log = InternalLoggerFactory.getLogger(LoggerName.COMMON_LOGGER_NAME);
@@ -194,9 +195,9 @@ public class PlainPermissionManager {
                     "The secretKey=%s value length should longer than 6",
                     plainAccessConfig.getSecretKey()));
             }
-            newAccountsMap.put(AclConstants.CONFIG_SECRET_KEY, (String) plainAccessConfig.getSecretKey());
+            newAccountsMap.put(AclConstants.CONFIG_SECRET_KEY, plainAccessConfig.getSecretKey());
         }
-        if (!StringUtils.isEmpty(plainAccessConfig.getWhiteRemoteAddress())) {
+        if (plainAccessConfig.getWhiteRemoteAddress() != null) {
             newAccountsMap.put(AclConstants.CONFIG_WHITE_ADDR, plainAccessConfig.getWhiteRemoteAddress());
         }
         if (!StringUtils.isEmpty(String.valueOf(plainAccessConfig.isAdmin()))) {
@@ -208,10 +209,10 @@ public class PlainPermissionManager {
         if (!StringUtils.isEmpty(plainAccessConfig.getDefaultGroupPerm())) {
             newAccountsMap.put(AclConstants.CONFIG_DEFAULT_GROUP_PERM, plainAccessConfig.getDefaultGroupPerm());
         }
-        if (plainAccessConfig.getTopicPerms() != null && !plainAccessConfig.getTopicPerms().isEmpty()) {
+        if (plainAccessConfig.getTopicPerms() != null) {
             newAccountsMap.put(AclConstants.CONFIG_TOPIC_PERMS, plainAccessConfig.getTopicPerms());
         }
-        if (plainAccessConfig.getGroupPerms() != null && !plainAccessConfig.getGroupPerms().isEmpty()) {
+        if (plainAccessConfig.getGroupPerms() != null) {
             newAccountsMap.put(AclConstants.CONFIG_GROUP_PERMS, plainAccessConfig.getGroupPerms());
         }
 

acl/src/test/java/org/apache/rocketmq/acl/plain/PlainAccessValidatorTest.java

@@ -19,6 +19,7 @@ package org.apache.rocketmq.acl.plain;
 
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -524,7 +525,7 @@ public class PlainAccessValidatorTest {
         // Verify the dateversion element is correct or not
         List<Map<String, Object>> dataVersions = (List<Map<String, Object>>) readableMap.get(AclConstants.CONFIG_DATA_VERSION);
         Assert.assertEquals(1,dataVersions.get(0).get(AclConstants.CONFIG_COUNTER));
-        
+
         // Restore the backup file and flush to yaml file
         AclUtils.writeDataObject(targetFileName, backUpAclConfigMap);
     }
@@ -616,4 +617,44 @@ public class PlainAccessValidatorTest {
         Assert.assertEquals(aclConfig.getPlainAccessConfigs().size(), 2);
     }
 
+
+    @Test
+    public void updateAccessConfigEmptyPermListTest(){
+        PlainAccessValidator plainAccessValidator = new PlainAccessValidator();
+        PlainAccessConfig plainAccessConfig = new PlainAccessConfig();
+        String accessKey = "updateAccessConfigEmptyPerm";
+        plainAccessConfig.setAccessKey(accessKey);
+        plainAccessConfig.setSecretKey("123456789111");
+        plainAccessConfig.setTopicPerms(Collections.singletonList("topicB=PUB"));
+        plainAccessValidator.updateAccessConfig(plainAccessConfig);
+
+        plainAccessConfig.setTopicPerms(new ArrayList<>());
+        plainAccessValidator.updateAccessConfig(plainAccessConfig);
+
+        PlainAccessConfig result = plainAccessValidator.getAllAclConfig().getPlainAccessConfigs()
+                .stream().filter(c->c.getAccessKey().equals(accessKey)).findFirst().orElse(null);
+        Assert.assertEquals(0, result.getTopicPerms().size());
+
+        plainAccessValidator.deleteAccessConfig(accessKey);
+    }
+
+    @Test
+    public void updateAccessConfigEmptyWhiteRemoteAddressTest(){
+        PlainAccessValidator plainAccessValidator = new PlainAccessValidator();
+        PlainAccessConfig plainAccessConfig = new PlainAccessConfig();
+        String accessKey = "updateAccessConfigEmptyWhiteRemoteAddress";
+        plainAccessConfig.setAccessKey(accessKey);
+        plainAccessConfig.setSecretKey("123456789111");
+        plainAccessConfig.setWhiteRemoteAddress("127.0.0.1");
+        plainAccessValidator.updateAccessConfig(plainAccessConfig);
+
+        plainAccessConfig.setWhiteRemoteAddress("");
+        plainAccessValidator.updateAccessConfig(plainAccessConfig);
+
+        PlainAccessConfig result = plainAccessValidator.getAllAclConfig().getPlainAccessConfigs()
+                .stream().filter(c->c.getAccessKey().equals(accessKey)).findFirst().orElse(null);
+        Assert.assertEquals("", result.getWhiteRemoteAddress());
+
+        plainAccessValidator.deleteAccessConfig(accessKey);
+    }
 }

broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java

@@ -316,8 +316,8 @@ public class AdminBrokerProcessor extends AsyncNettyRequestProcessor implements
         accessConfig.setWhiteRemoteAddress(requestHeader.getWhiteRemoteAddress());
         accessConfig.setDefaultTopicPerm(requestHeader.getDefaultTopicPerm());
         accessConfig.setDefaultGroupPerm(requestHeader.getDefaultGroupPerm());
-        accessConfig.setTopicPerms(UtilAll.string2List(requestHeader.getTopicPerms(), ","));
-        accessConfig.setGroupPerms(UtilAll.string2List(requestHeader.getGroupPerms(), ","));
+        accessConfig.setTopicPerms(UtilAll.split(requestHeader.getTopicPerms(), ","));
+        accessConfig.setGroupPerms(UtilAll.split(requestHeader.getGroupPerms(), ","));
         accessConfig.setAdmin(requestHeader.isAdmin());
         try {
 
@@ -390,7 +390,7 @@ public class AdminBrokerProcessor extends AsyncNettyRequestProcessor implements
 
         try {
             AccessValidator accessValidator = this.brokerController.getAccessValidatorMap().get(PlainAccessValidator.class);
-            if (accessValidator.updateGlobalWhiteAddrsConfig(UtilAll.string2List(requestHeader.getGlobalWhiteAddrs(), ","))) {
+            if (accessValidator.updateGlobalWhiteAddrsConfig(UtilAll.split(requestHeader.getGlobalWhiteAddrs(), ","))) {
                 response.setCode(ResponseCode.SUCCESS);
                 response.setOpaque(request.getOpaque());
                 response.markResponseType();

client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java

@@ -307,8 +307,8 @@ public class MQClientAPIImpl {
         requestHeader.setDefaultGroupPerm(plainAccessConfig.getDefaultGroupPerm());
         requestHeader.setDefaultTopicPerm(plainAccessConfig.getDefaultTopicPerm());
         requestHeader.setWhiteRemoteAddress(plainAccessConfig.getWhiteRemoteAddress());
-        requestHeader.setTopicPerms(UtilAll.list2String(plainAccessConfig.getTopicPerms(), ","));
-        requestHeader.setGroupPerms(UtilAll.list2String(plainAccessConfig.getGroupPerms(), ","));
+        requestHeader.setTopicPerms(UtilAll.join(plainAccessConfig.getTopicPerms(), ","));
+        requestHeader.setGroupPerms(UtilAll.join(plainAccessConfig.getGroupPerms(), ","));
 
         RemotingCommand request = RemotingCommand.createRequestCommand(RequestCode.UPDATE_AND_CREATE_ACL_CONFIG, requestHeader);
 

common/src/main/java/org/apache/rocketmq/common/UtilAll.java

@@ -39,7 +39,6 @@ import java.util.Map;
 import java.util.zip.CRC32;
 import java.util.zip.DeflaterOutputStream;
 import java.util.zip.InflaterInputStream;
-import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.validator.routines.InetAddressValidator;
 import org.apache.rocketmq.common.constant.LoggerName;
 import org.apache.rocketmq.logging.InternalLogger;
@@ -466,7 +465,7 @@ public class UtilAll {
         if (ip.length != 4) {
             throw new RuntimeException("illegal ipv4 bytes");
         }
-    
+
         InetAddressValidator validator = InetAddressValidator.getInstance();
         return validator.isValidInet4Address(ipToIPv4Str(ip));
     }
@@ -566,27 +565,28 @@ public class UtilAll {
         }
     }
 
-    public static String list2String(List<String> list, String splitor) {
-        if (list == null || list.size() == 0) {
+    public static String join(List<String> list, String splitter) {
+        if (list == null) {
             return null;
         }
+
         StringBuilder str = new StringBuilder();
         for (int i = 0; i < list.size(); i++) {
             str.append(list.get(i));
             if (i == list.size() - 1) {
                 break;
             }
-            str.append(splitor);
+            str.append(splitter);
         }
         return str.toString();
     }
 
-    public static List<String> string2List(String str, String splitor) {
-        if (StringUtils.isEmpty(str)) {
+    public static List<String> split(String str, String splitter) {
+        if (str == null) {
             return null;
         }
 
-        String[] addrArray = str.split(splitor);
+        String[] addrArray = str.split(splitter);
         return Arrays.asList(addrArray);
     }
 }

common/src/test/java/org/apache/rocketmq/common/UtilAllTest.java

@@ -114,12 +114,12 @@ public class UtilAllTest {
     }
 
     @Test
-    public void testList2String() {
+    public void testJoin() {
         List<String> list = Arrays.asList("groupA=DENY", "groupB=PUB|SUB", "groupC=SUB");
         String comma = ",";
-        assertEquals("groupA=DENY,groupB=PUB|SUB,groupC=SUB", UtilAll.list2String(list, comma));
-        assertEquals(null, UtilAll.list2String(null, comma));
-        assertEquals(null, UtilAll.list2String(Collections.emptyList(), comma));
+        assertEquals("groupA=DENY,groupB=PUB|SUB,groupC=SUB", UtilAll.join(list, comma));
+        assertEquals(null, UtilAll.join(null, comma));
+        assertEquals("", UtilAll.join(Collections.emptyList(), comma));
     }
 
     static class DemoConfig {

tools/src/main/java/org/apache/rocketmq/tools/command/acl/DeleteAccessConfigSubCommand.java

@@ -85,9 +85,9 @@ public class DeleteAccessConfigSubCommand implements SubCommand {
 
                 defaultMQAdminExt.start();
 
-                Set<String> masterSet =
-                    CommandUtil.fetchMasterAddrByClusterName(defaultMQAdminExt, clusterName);
-                for (String addr : masterSet) {
+                Set<String> brokerAddrSet =
+                    CommandUtil.fetchMasterAndSlaveAddrByClusterName(defaultMQAdminExt, clusterName);
+                for (String addr : brokerAddrSet) {
                     defaultMQAdminExt.deletePlainAccessConfig(addr, accessKey);
                     System.out.printf("delete plain access config account from %s success.%n", addr);
                 }

tools/src/main/java/org/apache/rocketmq/tools/command/acl/UpdateGlobalWhiteAddrSubCommand.java

@@ -82,9 +82,9 @@ public class UpdateGlobalWhiteAddrSubCommand implements SubCommand {
                 String clusterName = commandLine.getOptionValue('c').trim();
 
                 defaultMQAdminExt.start();
-                Set<String> masterSet =
-                    CommandUtil.fetchMasterAddrByClusterName(defaultMQAdminExt, clusterName);
-                for (String addr : masterSet) {
+                Set<String> brokerAddrSet =
+                    CommandUtil.fetchMasterAndSlaveAddrByClusterName(defaultMQAdminExt, clusterName);
+                for (String addr : brokerAddrSet) {
                     defaultMQAdminExt.updateGlobalWhiteAddrConfig(addr, globalWhiteRemoteAddresses);
                     System.out.printf("update global white remote addresses to %s success.%n", addr);
                 }