Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 14, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
efd1f04
fix fail to remove topic/group permissions and global white address o…
yuz10 Aug 19, 2021
ef6d54a
update acl config for all brokers
yuz10 Aug 20, 2021
49358c7
refactor rename UtilAll.string2List and list2String
yuz10 Sep 4, 2021
f9f432a
fix fail to remove whiteRemoteAddress of acl config
yuz10 Sep 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,6 @@

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;
Expand All @@ -39,6 +32,14 @@
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);
Expand Down Expand Up @@ -194,9 +195,9 @@ private Map<String, Object> createAclAccessConfigMap(Map<String, Object> existed
"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()))) {
Expand All @@ -208,10 +209,10 @@ private Map<String, Object> createAclAccessConfigMap(Map<String, Object> existed
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());
}

Expand Down
43 changes: 42 additions & 1 deletion acl/src/test/java/org/apache/rocketmq/acl/plain/PlainAccessValidatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -524,7 +525,7 @@ public void deleteAccessAclYamlConfigNormalTest() {
// 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);
}
Expand Down Expand Up @@ -616,4 +617,44 @@ public void getAllAclConfigTest(){
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);
}
}
6 changes: 3 additions & 3 deletions broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,8 +316,8 @@ private synchronized RemotingCommand updateAndCreateAccessConfig(ChannelHandlerC
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 {

Expand Down Expand Up @@ -390,7 +390,7 @@ private synchronized RemotingCommand updateGlobalWhiteAddrsConfig(ChannelHandler

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();
Expand Down
4 changes: 2 additions & 2 deletions client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,8 @@ public void createPlainAccessConfig(final String addr, final PlainAccessConfig p
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);

Expand Down
16 changes: 8 additions & 8 deletions common/src/main/java/org/apache/rocketmq/common/UtilAll.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
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;
Expand Down Expand Up @@ -461,7 +460,7 @@ private static boolean ipCheck(byte[] ip) {
if (ip.length != 4) {
throw new RuntimeException("illegal ipv4 bytes");
}

InetAddressValidator validator = InetAddressValidator.getInstance();
return validator.isValidInet4Address(ipToIPv4Str(ip));
}
Expand Down Expand Up @@ -561,27 +560,28 @@ public static void deleteFile(File file) {
}
}

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);
}
}
8 changes: 4 additions & 4 deletions common/src/test/java/org/apache/rocketmq/common/UtilAllTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,12 @@ public void testIPv6Check() throws UnknownHostException {
}

@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 {
Expand Down
6 changes: 3 additions & 3 deletions tools/src/main/java/org/apache/rocketmq/tools/command/acl/DeleteAccessConfigSubCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ public Options buildCommandlineOptions(Options options) {

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);
}
Expand Down
6 changes: 3 additions & 3 deletions .../src/main/java/org/apache/rocketmq/tools/command/acl/UpdateGlobalWhiteAddrSubCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down