rocketmq-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From vongosl...@apache.org
Subject [rocketmq] branch develop updated: [ISSUE #3281 ][acl] fix fail to delete topic perm list and global white address(#3128) (#3280)
Date Tue, 14 Sep 2021 07:33:47 GMT
This is an automated email from the ASF dual-hosted git repository.

vongosling pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/rocketmq.git


The following commit(s) were added to refs/heads/develop by this push:
     new 147d23e  [ISSUE #3281 ][acl] fix fail to delete topic perm list and global white
address(#3128) (#3280)
147d23e is described below

commit 147d23e3ed0a817f8ebb427168f35ba9fe946026
Author: yuz10 <845238369@qq.com>
AuthorDate: Tue Sep 14 15:33:38 2021 +0800

    [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
---
 .../rocketmq/acl/plain/PlainPermissionManager.java | 23 ++++++------
 .../acl/plain/PlainAccessValidatorTest.java        | 43 +++++++++++++++++++++-
 .../broker/processor/AdminBrokerProcessor.java     |  6 +--
 .../rocketmq/client/impl/MQClientAPIImpl.java      |  4 +-
 .../java/org/apache/rocketmq/common/UtilAll.java   | 16 ++++----
 .../org/apache/rocketmq/common/UtilAllTest.java    |  8 ++--
 .../command/acl/DeleteAccessConfigSubCommand.java  |  6 +--
 .../acl/UpdateGlobalWhiteAddrSubCommand.java       |  6 +--
 8 files changed, 77 insertions(+), 35 deletions(-)

diff --git a/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java b/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
index 809cc75..f7af586 100644
--- a/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
+++ b/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());
         }
 
diff --git a/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainAccessValidatorTest.java
b/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainAccessValidatorTest.java
index 056f011..a0eb567 100644
--- a/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainAccessValidatorTest.java
+++ b/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);
+    }
 }
diff --git a/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
b/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
index c481d14..86aab63 100644
--- a/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
+++ b/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();
diff --git a/client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java b/client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java
index 3bbdd84..b769429 100644
--- a/client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java
+++ b/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);
 
diff --git a/common/src/main/java/org/apache/rocketmq/common/UtilAll.java b/common/src/main/java/org/apache/rocketmq/common/UtilAll.java
index 6318be0..457deb8 100644
--- a/common/src/main/java/org/apache/rocketmq/common/UtilAll.java
+++ b/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);
     }
 }
diff --git a/common/src/test/java/org/apache/rocketmq/common/UtilAllTest.java b/common/src/test/java/org/apache/rocketmq/common/UtilAllTest.java
index e1942b4..ffbcd33 100644
--- a/common/src/test/java/org/apache/rocketmq/common/UtilAllTest.java
+++ b/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 {
diff --git a/tools/src/main/java/org/apache/rocketmq/tools/command/acl/DeleteAccessConfigSubCommand.java
b/tools/src/main/java/org/apache/rocketmq/tools/command/acl/DeleteAccessConfigSubCommand.java
index d82453d..4e7cd93 100644
--- a/tools/src/main/java/org/apache/rocketmq/tools/command/acl/DeleteAccessConfigSubCommand.java
+++ b/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);
                 }
diff --git a/tools/src/main/java/org/apache/rocketmq/tools/command/acl/UpdateGlobalWhiteAddrSubCommand.java
b/tools/src/main/java/org/apache/rocketmq/tools/command/acl/UpdateGlobalWhiteAddrSubCommand.java
index ef9d940..efd39e9 100644
--- a/tools/src/main/java/org/apache/rocketmq/tools/command/acl/UpdateGlobalWhiteAddrSubCommand.java
+++ b/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);
                 }

Mime
View raw message