You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/01/18 12:19:31 UTC
[GitHub] [rocketmq] caigy commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory
caigy commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r786523576
##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +41,74 @@
import org.apache.rocketmq.common.constant.LoggerName;
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;
+import org.apache.rocketmq.srvutil.AclFileWatchService;
public class PlainPermissionManager {
private static final InternalLogger log = InternalLoggerFactory.getLogger(LoggerName.COMMON_LOGGER_NAME);
- private static final String DEFAULT_PLAIN_ACL_FILE = "/conf/plain_acl.yml";
-
private String fileHome = System.getProperty(MixAll.ROCKETMQ_HOME_PROPERTY,
System.getenv(MixAll.ROCKETMQ_HOME_ENV));
- private String fileName = System.getProperty("rocketmq.acl.plain.file", DEFAULT_PLAIN_ACL_FILE);
+ private String defaultAclDir = fileHome + File.separator
+ + System.getProperty("rocketmq.acl.dir", "/conf/acl");
+
+ private String defaultAclFile = fileHome + File.separator
+ + System.getProperty("rocketmq.acl.dir", "/conf/acl") + File.separator + "plain_acl.yml";
- private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+ private Map<String/** aclFileName **/, Map<String/** AccessKey **/, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();
+
+ private Map<String/** AccessKey **/, String/** aclFileName **/> accessKeyTable = new HashMap<>();
private List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
private RemoteAddressStrategyFactory remoteAddressStrategyFactory = new RemoteAddressStrategyFactory();
private boolean isWatchStart;
- private final DataVersion dataVersion = new DataVersion();
+ private Map<String/** aclFileName **/, DataVersion> dataVersionMap = new HashMap<>();
public PlainPermissionManager() {
load();
watch();
}
public void load() {
+ if (fileHome == null || fileHome.isEmpty()) {
+ throw new AclException(String.format("%s file is empty", fileHome));
+ }
+ File aclDir = new File(defaultAclDir);
+ File[] aclFiles = aclDir.listFiles();
+ if (aclFiles == null || aclFiles.length == 0)
+ return;
+ if (aclPlainAccessResourceMap.size() != 0 && accessKeyTable.size() != 0) {
+ aclPlainAccessResourceMap.clear();
+ accessKeyTable.clear();
+ }
+ List<String> fileList = new ArrayList<>();
+ for (File aclFile : aclFiles) {
+ String aclFileAbsolutePath = aclFile.getAbsolutePath();
+ load(aclFileAbsolutePath);
Review comment:
Should we surround `load` by try-catch?
##########
File path: acl/src/main/java/org/apache/rocketmq/acl/AccessValidator.java
##########
@@ -60,16 +63,18 @@
*
* @return
*/
- String getAclConfigVersion();
+ Map<String, DataVersion> getAclConfigVersion();
Review comment:
It makes the interface incompatible to older versions, you'd better add a new method and deprecate the original ones.
##########
File path: common/src/main/java/org/apache/rocketmq/common/protocol/header/GetBrokerAclConfigResponseHeader.java
##########
@@ -16,14 +16,17 @@
*/
package org.apache.rocketmq.common.protocol.header;
+import org.apache.rocketmq.common.DataVersion;
import org.apache.rocketmq.remoting.CommandCustomHeader;
import org.apache.rocketmq.remoting.annotation.CFNotNull;
import org.apache.rocketmq.remoting.exception.RemotingCommandException;
+import java.util.Map;
+
public class GetBrokerAclConfigResponseHeader implements CommandCustomHeader {
@CFNotNull
- private String version;
+ private Map<String, DataVersion> version;
Review comment:
It changes the protocol, some users may already use it as String.
##########
File path: srvutil/src/main/java/org/apache/rocketmq/srvutil/AclFileWatchService.java
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.srvutil;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.rocketmq.common.ServiceThread;
+import org.apache.rocketmq.common.UtilAll;
+import org.apache.rocketmq.common.constant.LoggerName;
+import org.apache.rocketmq.logging.InternalLogger;
+import org.apache.rocketmq.logging.InternalLoggerFactory;
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.security.MessageDigest;
+import java.util.HashMap;
+import java.util.Map;
+
+public class AclFileWatchService extends ServiceThread {
+ private static final InternalLogger log = InternalLoggerFactory.getLogger(LoggerName.COMMON_LOGGER_NAME);
+
+ private final String aclPath;
+ private int aclFilesNum;
+ private final Map<String, String> fileCurrentHash;
+ private final AclFileWatchService.Listener listener;
+ private static final int WATCH_INTERVAL = 500;
Review comment:
Is it too often?
##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +41,74 @@
import org.apache.rocketmq.common.constant.LoggerName;
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;
+import org.apache.rocketmq.srvutil.AclFileWatchService;
public class PlainPermissionManager {
private static final InternalLogger log = InternalLoggerFactory.getLogger(LoggerName.COMMON_LOGGER_NAME);
- private static final String DEFAULT_PLAIN_ACL_FILE = "/conf/plain_acl.yml";
-
private String fileHome = System.getProperty(MixAll.ROCKETMQ_HOME_PROPERTY,
System.getenv(MixAll.ROCKETMQ_HOME_ENV));
- private String fileName = System.getProperty("rocketmq.acl.plain.file", DEFAULT_PLAIN_ACL_FILE);
Review comment:
Users using `DEFAULT_PLAIN_ACL_FILE` can't load ACL config file after this change, it requires more compatible considerations.
##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +41,74 @@
import org.apache.rocketmq.common.constant.LoggerName;
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;
+import org.apache.rocketmq.srvutil.AclFileWatchService;
public class PlainPermissionManager {
private static final InternalLogger log = InternalLoggerFactory.getLogger(LoggerName.COMMON_LOGGER_NAME);
- private static final String DEFAULT_PLAIN_ACL_FILE = "/conf/plain_acl.yml";
-
private String fileHome = System.getProperty(MixAll.ROCKETMQ_HOME_PROPERTY,
System.getenv(MixAll.ROCKETMQ_HOME_ENV));
- private String fileName = System.getProperty("rocketmq.acl.plain.file", DEFAULT_PLAIN_ACL_FILE);
+ private String defaultAclDir = fileHome + File.separator
+ + System.getProperty("rocketmq.acl.dir", "/conf/acl");
+
+ private String defaultAclFile = fileHome + File.separator
+ + System.getProperty("rocketmq.acl.dir", "/conf/acl") + File.separator + "plain_acl.yml";
- private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+ private Map<String/** aclFileName **/, Map<String/** AccessKey **/, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();
+
+ private Map<String/** AccessKey **/, String/** aclFileName **/> accessKeyTable = new HashMap<>();
private List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
private RemoteAddressStrategyFactory remoteAddressStrategyFactory = new RemoteAddressStrategyFactory();
private boolean isWatchStart;
- private final DataVersion dataVersion = new DataVersion();
+ private Map<String/** aclFileName **/, DataVersion> dataVersionMap = new HashMap<>();
public PlainPermissionManager() {
load();
watch();
}
public void load() {
+ if (fileHome == null || fileHome.isEmpty()) {
+ throw new AclException(String.format("%s file is empty", fileHome));
+ }
+ File aclDir = new File(defaultAclDir);
+ File[] aclFiles = aclDir.listFiles();
Review comment:
Array of `File` objects is created here but only path string is used(`org.apache.rocketmq.acl.common.AclUtils#getYamlDataObject` uses file name to read this file again), it will use less resource if using `java.nio.file.Files#list`, or add overloaded method for `org.apache.rocketmq.acl.common.AclUtils#getYamlDataObject` accepting file object.
##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -91,37 +125,51 @@ public void load() {
for (PlainAccessConfig plainAccessConfig : plainAccessConfigList) {
PlainAccessResource plainAccessResource = buildPlainAccessResource(plainAccessConfig);
plainAccessResourceMap.put(plainAccessResource.getAccessKey(), plainAccessResource);
+ this.accessKeyTable.put(plainAccessResource.getAccessKey(), aclFilePath);
}
}
// For loading dataversion part just
JSONArray tempDataVersion = plainAclConfData.getJSONArray(AclConstants.CONFIG_DATA_VERSION);
+ DataVersion dataVersion = new DataVersion();
if (tempDataVersion != null && !tempDataVersion.isEmpty()) {
- List<DataVersion> dataVersion = tempDataVersion.toJavaList(DataVersion.class);
- DataVersion firstElement = dataVersion.get(0);
- this.dataVersion.assignNewOne(firstElement);
+ List<DataVersion> dataVersions = tempDataVersion.toJavaList(DataVersion.class);
+ DataVersion firstElement = dataVersions.get(0);
+ dataVersion.assignNewOne(firstElement);
}
this.globalWhiteRemoteAddressStrategy = globalWhiteRemoteAddressStrategy;
- this.plainAccessResourceMap = plainAccessResourceMap;
+ this.aclPlainAccessResourceMap.put(aclFilePath, plainAccessResourceMap);
+ this.dataVersionMap.put(aclFilePath, dataVersion);
}
- public String getAclConfigDataVersion() {
Review comment:
Keep `public String getAclConfigDataVersion()` and mark it deprecated for compatibility where only one config file is used.
##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +41,74 @@
import org.apache.rocketmq.common.constant.LoggerName;
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;
+import org.apache.rocketmq.srvutil.AclFileWatchService;
public class PlainPermissionManager {
private static final InternalLogger log = InternalLoggerFactory.getLogger(LoggerName.COMMON_LOGGER_NAME);
- private static final String DEFAULT_PLAIN_ACL_FILE = "/conf/plain_acl.yml";
-
private String fileHome = System.getProperty(MixAll.ROCKETMQ_HOME_PROPERTY,
System.getenv(MixAll.ROCKETMQ_HOME_ENV));
- private String fileName = System.getProperty("rocketmq.acl.plain.file", DEFAULT_PLAIN_ACL_FILE);
+ private String defaultAclDir = fileHome + File.separator
+ + System.getProperty("rocketmq.acl.dir", "/conf/acl");
+
+ private String defaultAclFile = fileHome + File.separator
+ + System.getProperty("rocketmq.acl.dir", "/conf/acl") + File.separator + "plain_acl.yml";
- private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+ private Map<String/** aclFileName **/, Map<String/** AccessKey **/, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();
+
+ private Map<String/** AccessKey **/, String/** aclFileName **/> accessKeyTable = new HashMap<>();
private List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
private RemoteAddressStrategyFactory remoteAddressStrategyFactory = new RemoteAddressStrategyFactory();
private boolean isWatchStart;
- private final DataVersion dataVersion = new DataVersion();
+ private Map<String/** aclFileName **/, DataVersion> dataVersionMap = new HashMap<>();
public PlainPermissionManager() {
load();
watch();
}
public void load() {
+ if (fileHome == null || fileHome.isEmpty()) {
+ throw new AclException(String.format("%s file is empty", fileHome));
+ }
Review comment:
Is it more appropriate to just returning than throwing exception?
##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -91,37 +125,51 @@ public void load() {
for (PlainAccessConfig plainAccessConfig : plainAccessConfigList) {
PlainAccessResource plainAccessResource = buildPlainAccessResource(plainAccessConfig);
plainAccessResourceMap.put(plainAccessResource.getAccessKey(), plainAccessResource);
+ this.accessKeyTable.put(plainAccessResource.getAccessKey(), aclFilePath);
Review comment:
Can one access key be defined in multiple files? If not so, instructions should be added in documents and checks should be added.
##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -136,42 +184,46 @@ public boolean updateAccessConfig(PlainAccessConfig plainAccessConfig) {
Permission.checkResourcePerms(plainAccessConfig.getTopicPerms());
Permission.checkResourcePerms(plainAccessConfig.getGroupPerms());
- Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(fileHome + File.separator + fileName,
- Map.class);
- if (aclAccessConfigMap == null || aclAccessConfigMap.isEmpty()) {
- throw new AclException(String.format("the %s file is not found or empty", fileHome + File.separator + fileName));
- }
- List<Map<String, Object>> accounts = (List<Map<String, Object>>) aclAccessConfigMap.get(AclConstants.CONFIG_ACCOUNTS);
- Map<String, Object> updateAccountMap = null;
- if (accounts != null) {
Review comment:
Just keep the null check
##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -91,37 +125,51 @@ public void load() {
for (PlainAccessConfig plainAccessConfig : plainAccessConfigList) {
PlainAccessResource plainAccessResource = buildPlainAccessResource(plainAccessConfig);
plainAccessResourceMap.put(plainAccessResource.getAccessKey(), plainAccessResource);
+ this.accessKeyTable.put(plainAccessResource.getAccessKey(), aclFilePath);
}
}
// For loading dataversion part just
JSONArray tempDataVersion = plainAclConfData.getJSONArray(AclConstants.CONFIG_DATA_VERSION);
+ DataVersion dataVersion = new DataVersion();
if (tempDataVersion != null && !tempDataVersion.isEmpty()) {
- List<DataVersion> dataVersion = tempDataVersion.toJavaList(DataVersion.class);
- DataVersion firstElement = dataVersion.get(0);
- this.dataVersion.assignNewOne(firstElement);
+ List<DataVersion> dataVersions = tempDataVersion.toJavaList(DataVersion.class);
+ DataVersion firstElement = dataVersions.get(0);
+ dataVersion.assignNewOne(firstElement);
}
this.globalWhiteRemoteAddressStrategy = globalWhiteRemoteAddressStrategy;
- this.plainAccessResourceMap = plainAccessResourceMap;
+ this.aclPlainAccessResourceMap.put(aclFilePath, plainAccessResourceMap);
+ this.dataVersionMap.put(aclFilePath, dataVersion);
}
- public String getAclConfigDataVersion() {
- return this.dataVersion.toJson();
+ public Map<String, DataVersion> getAclConfigDataVersion() {
+ return this.dataVersionMap;
}
- private Map<String, Object> updateAclConfigFileVersion(Map<String, Object> updateAclConfigMap) {
+ public Map<String, Object> updateAclConfigFileVersion(Map<String, Object> updateAclConfigMap) {
+ Object dataVersions = updateAclConfigMap.get(AclConstants.CONFIG_DATA_VERSION);
+ DataVersion dataVersion = new DataVersion();
+ List<Map<String, Object>> dataVersionList = new ArrayList<Map<String, Object>>();
Review comment:
It seems unnecessary to create a new ArrayList.
##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -267,54 +318,61 @@ public boolean updateGlobalWhiteAddrsConfig(List<String> globalWhiteAddrsList) {
}
// Update globalWhiteRemoteAddr element in memory map firstly
aclAccessConfigMap.put(AclConstants.CONFIG_GLOBAL_WHITE_ADDRS, globalWhiteRemoteAddrList);
- if (AclUtils.writeDataObject(fileHome + File.separator + fileName, updateAclConfigFileVersion(aclAccessConfigMap))) {
- return true;
- }
- return false;
+ return AclUtils.writeDataObject(defaultAclFile, updateAclConfigFileVersion(aclAccessConfigMap));
}
- log.error("Users must ensure that the acl yaml config file has globalWhiteRemoteAddresses flag firstly");
+ log.error("Users must ensure that the acl yaml config file has globalWhiteRemoteAddresses flag in the %s firstly", defaultAclFile);
Review comment:
It seem that the placeholder is `{}`.
##########
File path: srvutil/src/main/java/org/apache/rocketmq/srvutil/AclFileWatchService.java
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.srvutil;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.rocketmq.common.ServiceThread;
+import org.apache.rocketmq.common.UtilAll;
+import org.apache.rocketmq.common.constant.LoggerName;
+import org.apache.rocketmq.logging.InternalLogger;
+import org.apache.rocketmq.logging.InternalLoggerFactory;
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.security.MessageDigest;
+import java.util.HashMap;
+import java.util.Map;
+
+public class AclFileWatchService extends ServiceThread {
+ private static final InternalLogger log = InternalLoggerFactory.getLogger(LoggerName.COMMON_LOGGER_NAME);
+
+ private final String aclPath;
+ private int aclFilesNum;
+ private final Map<String, String> fileCurrentHash;
+ private final AclFileWatchService.Listener listener;
+ private static final int WATCH_INTERVAL = 500;
+ private MessageDigest md = MessageDigest.getInstance("MD5");
+
+ public AclFileWatchService(String path, final AclFileWatchService.Listener listener) throws Exception {
+ this.aclPath = path;
+ this.fileCurrentHash = new HashMap<>();
+ this.listener = listener;
+
+ File aclDir = new File(path);
+ String[] aclFileNames = aclDir.list();
+ this.aclFilesNum = aclFileNames.length;
+ for (int i = 0; i < aclFilesNum; i++) {
+ String aclFilePath = this.aclPath + aclFileNames[i];
+ if (StringUtils.isNotEmpty(aclFileNames[i]) && new File(aclFilePath).exists()) {
+ this.fileCurrentHash.put(aclFilePath, hash(aclFilePath));
+ }
+ }
+
+ }
+
+ @Override
+ public String getServiceName() {
+ return "AclFileWatchService";
+ }
+
+ @Override
+ public void run() {
+ log.info(this.getServiceName() + " service started");
+
+ while (!this.isStopped()) {
+ try {
+ this.waitForRunning(WATCH_INTERVAL);
+
+ File aclDir = new File(aclPath);
+ File[] aclFiles = aclDir.listFiles();
+ int realAclFilesNum = aclFiles.length;
+
+ if (aclFilesNum != realAclFilesNum) {
+ log.info("aclFilesNum: " + aclFilesNum + " realAclFilesNum: " + realAclFilesNum);
+ aclFilesNum = realAclFilesNum;
+ log.info("aclFilesNum: " + aclFilesNum + " realAclFilesNum: " + realAclFilesNum);
+ listener.onFileNumChanged(aclPath);
+ } else {
+ for (int i = 0; i < aclFilesNum; i++) {
+ String fileName = aclFiles[i].getAbsolutePath();
+ String newHash = hash(fileName);
+ if (!newHash.equals(fileCurrentHash.get(i))) {
Review comment:
Should it be `fileCurrentHash.get(fileName)`?
##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -91,37 +125,51 @@ public void load() {
for (PlainAccessConfig plainAccessConfig : plainAccessConfigList) {
PlainAccessResource plainAccessResource = buildPlainAccessResource(plainAccessConfig);
plainAccessResourceMap.put(plainAccessResource.getAccessKey(), plainAccessResource);
+ this.accessKeyTable.put(plainAccessResource.getAccessKey(), aclFilePath);
}
}
// For loading dataversion part just
JSONArray tempDataVersion = plainAclConfData.getJSONArray(AclConstants.CONFIG_DATA_VERSION);
+ DataVersion dataVersion = new DataVersion();
if (tempDataVersion != null && !tempDataVersion.isEmpty()) {
- List<DataVersion> dataVersion = tempDataVersion.toJavaList(DataVersion.class);
- DataVersion firstElement = dataVersion.get(0);
- this.dataVersion.assignNewOne(firstElement);
+ List<DataVersion> dataVersions = tempDataVersion.toJavaList(DataVersion.class);
+ DataVersion firstElement = dataVersions.get(0);
+ dataVersion.assignNewOne(firstElement);
}
this.globalWhiteRemoteAddressStrategy = globalWhiteRemoteAddressStrategy;
- this.plainAccessResourceMap = plainAccessResourceMap;
+ this.aclPlainAccessResourceMap.put(aclFilePath, plainAccessResourceMap);
+ this.dataVersionMap.put(aclFilePath, dataVersion);
}
- public String getAclConfigDataVersion() {
- return this.dataVersion.toJson();
+ public Map<String, DataVersion> getAclConfigDataVersion() {
+ return this.dataVersionMap;
}
- private Map<String, Object> updateAclConfigFileVersion(Map<String, Object> updateAclConfigMap) {
+ public Map<String, Object> updateAclConfigFileVersion(Map<String, Object> updateAclConfigMap) {
+ Object dataVersions = updateAclConfigMap.get(AclConstants.CONFIG_DATA_VERSION);
+ DataVersion dataVersion = new DataVersion();
+ List<Map<String, Object>> dataVersionList = new ArrayList<Map<String, Object>>();
+ if (dataVersions != null) {
+ dataVersionList = (List<Map<String, Object>>) dataVersions;
+ dataVersion.setTimestamp((long) dataVersionList.get(0).get("timestamp"));
Review comment:
It would be more robust to check if dataVersionList is empty
##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +41,74 @@
import org.apache.rocketmq.common.constant.LoggerName;
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;
+import org.apache.rocketmq.srvutil.AclFileWatchService;
public class PlainPermissionManager {
private static final InternalLogger log = InternalLoggerFactory.getLogger(LoggerName.COMMON_LOGGER_NAME);
- private static final String DEFAULT_PLAIN_ACL_FILE = "/conf/plain_acl.yml";
-
private String fileHome = System.getProperty(MixAll.ROCKETMQ_HOME_PROPERTY,
System.getenv(MixAll.ROCKETMQ_HOME_ENV));
- private String fileName = System.getProperty("rocketmq.acl.plain.file", DEFAULT_PLAIN_ACL_FILE);
+ private String defaultAclDir = fileHome + File.separator
+ + System.getProperty("rocketmq.acl.dir", "/conf/acl");
+
+ private String defaultAclFile = fileHome + File.separator
+ + System.getProperty("rocketmq.acl.dir", "/conf/acl") + File.separator + "plain_acl.yml";
- private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+ private Map<String/** aclFileName **/, Map<String/** AccessKey **/, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();
+
+ private Map<String/** AccessKey **/, String/** aclFileName **/> accessKeyTable = new HashMap<>();
private List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
private RemoteAddressStrategyFactory remoteAddressStrategyFactory = new RemoteAddressStrategyFactory();
private boolean isWatchStart;
- private final DataVersion dataVersion = new DataVersion();
+ private Map<String/** aclFileName **/, DataVersion> dataVersionMap = new HashMap<>();
public PlainPermissionManager() {
load();
watch();
}
public void load() {
+ if (fileHome == null || fileHome.isEmpty()) {
+ throw new AclException(String.format("%s file is empty", fileHome));
+ }
+ File aclDir = new File(defaultAclDir);
+ File[] aclFiles = aclDir.listFiles();
+ if (aclFiles == null || aclFiles.length == 0)
+ return;
+ if (aclPlainAccessResourceMap.size() != 0 && accessKeyTable.size() != 0) {
+ aclPlainAccessResourceMap.clear();
+ accessKeyTable.clear();
+ }
+ List<String> fileList = new ArrayList<>();
Review comment:
Specifying expected size for ArrayList avoiding expansion.
##########
File path: srvutil/src/main/java/org/apache/rocketmq/srvutil/AclFileWatchService.java
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.srvutil;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.rocketmq.common.ServiceThread;
+import org.apache.rocketmq.common.UtilAll;
+import org.apache.rocketmq.common.constant.LoggerName;
+import org.apache.rocketmq.logging.InternalLogger;
+import org.apache.rocketmq.logging.InternalLoggerFactory;
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.security.MessageDigest;
+import java.util.HashMap;
+import java.util.Map;
+
+public class AclFileWatchService extends ServiceThread {
+ private static final InternalLogger log = InternalLoggerFactory.getLogger(LoggerName.COMMON_LOGGER_NAME);
+
+ private final String aclPath;
+ private int aclFilesNum;
+ private final Map<String, String> fileCurrentHash;
+ private final AclFileWatchService.Listener listener;
+ private static final int WATCH_INTERVAL = 500;
+ private MessageDigest md = MessageDigest.getInstance("MD5");
+
+ public AclFileWatchService(String path, final AclFileWatchService.Listener listener) throws Exception {
+ this.aclPath = path;
+ this.fileCurrentHash = new HashMap<>();
+ this.listener = listener;
+
+ File aclDir = new File(path);
+ String[] aclFileNames = aclDir.list();
+ this.aclFilesNum = aclFileNames.length;
+ for (int i = 0; i < aclFilesNum; i++) {
+ String aclFilePath = this.aclPath + aclFileNames[i];
+ if (StringUtils.isNotEmpty(aclFileNames[i]) && new File(aclFilePath).exists()) {
+ this.fileCurrentHash.put(aclFilePath, hash(aclFilePath));
+ }
+ }
+
+ }
+
+ @Override
+ public String getServiceName() {
+ return "AclFileWatchService";
+ }
+
+ @Override
+ public void run() {
+ log.info(this.getServiceName() + " service started");
+
+ while (!this.isStopped()) {
+ try {
+ this.waitForRunning(WATCH_INTERVAL);
+
+ File aclDir = new File(aclPath);
+ File[] aclFiles = aclDir.listFiles();
+ int realAclFilesNum = aclFiles.length;
+
+ if (aclFilesNum != realAclFilesNum) {
+ log.info("aclFilesNum: " + aclFilesNum + " realAclFilesNum: " + realAclFilesNum);
+ aclFilesNum = realAclFilesNum;
+ log.info("aclFilesNum: " + aclFilesNum + " realAclFilesNum: " + realAclFilesNum);
+ listener.onFileNumChanged(aclPath);
+ } else {
+ for (int i = 0; i < aclFilesNum; i++) {
+ String fileName = aclFiles[i].getAbsolutePath();
+ String newHash = hash(fileName);
Review comment:
Calculating hash of a file consumes a lot of system resources, can file modified time and file size be used instead?
##########
File path: common/src/main/java/org/apache/rocketmq/common/protocol/body/ClusterAclVersionInfo.java
##########
@@ -19,13 +19,15 @@
import org.apache.rocketmq.common.DataVersion;
import org.apache.rocketmq.remoting.protocol.RemotingSerializable;
+import java.util.Map;
+
public class ClusterAclVersionInfo extends RemotingSerializable {
private String brokerName;
private String brokerAddr;
- private DataVersion aclConfigDataVersion;
Review comment:
Add a new field other than changing it for compatibility.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org