From 147d23e3ed0a817f8ebb427168f35ba9fe946026 Mon Sep 17 00:00:00 2001 From: yuz10 <845238369@qq.com> Date: Tue, 14 Sep 2021 15:33:38 +0800 Subject: [PATCH] [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/plain/PlainPermissionManager.java | 23 +++++----- .../acl/plain/PlainAccessValidatorTest.java | 43 ++++++++++++++++++- .../processor/AdminBrokerProcessor.java | 6 +-- .../rocketmq/client/impl/MQClientAPIImpl.java | 4 +- .../org/apache/rocketmq/common/UtilAll.java | 16 +++---- .../apache/rocketmq/common/UtilAllTest.java | 8 ++-- .../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 809cc759..f7af586d 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 056f0119..a0eb567b 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> dataVersions = (List>) 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 c481d14d..86aab634 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 3bbdd84f..b7694295 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 6318be07..457deb8d 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 list, String splitor) { - if (list == null || list.size() == 0) { + public static String join(List 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 string2List(String str, String splitor) { - if (StringUtils.isEmpty(str)) { + public static List 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 e1942b40..ffbcd33e 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 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 d82453dc..4e7cd934 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 masterSet = - CommandUtil.fetchMasterAddrByClusterName(defaultMQAdminExt, clusterName); - for (String addr : masterSet) { + Set 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 ef9d9407..efd39e9f 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 masterSet = - CommandUtil.fetchMasterAddrByClusterName(defaultMQAdminExt, clusterName); - for (String addr : masterSet) { + Set 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); } -- GitLab