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/14 16:13:34 UTC

[GitHub] [rocketmq] sunxi92 opened a new pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

sunxi92 opened a new pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761


   **Make sure set the target branch to `develop`**
   
   ## What is the purpose of the change
   
   Support for multiple ACL files in a fixed directory
   #2986
   
   ## Brief changelog
   
   XX
   
   ## Verifying this change
   
   XXXX
   
   Follow this checklist to help us incorporate your contribution quickly and easily. Notice, `it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR`.
   
   - [x] Make sure there is a [Github issue](https://github.com/apache/rocketmq/issues) filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue. 
   - [x] Format the pull request title like `[ISSUE #123] Fix UnknownException when host config not exist`. Each commit in the pull request should have a meaningful subject line and body.
   - [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
   - [x] Write necessary unit-test(over 80% coverage) to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in [test module](https://github.com/apache/rocketmq/tree/master/test).
   - [x] Run `mvn -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyle` to make sure basic checks pass. Run `mvn clean install -DskipITs` to make sure unit-test pass. Run `mvn clean test-compile failsafe:integration-test`  to make sure integration-test pass.
   - [ ] If this contribution is large, please file an [Apache Individual Contributor License Agreement](http://www.apache.org/licenses/#clas).
   


-- 
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



[GitHub] [rocketmq] dongeforever commented on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
dongeforever commented on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1018150824


   IMO.
   The default access validator should keep simple. If it has difficulty supporting the requirements, it is better to supply a new validator implemntation, instead of changing the default one.
   
   


-- 
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



[GitHub] [rocketmq] dongeforever commented on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
dongeforever commented on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1018143519


   The multi ACL files are not compatible with the default ACL file naturally.
   If it is necessary to support multi ACL files, a detailed design with compatibility is needed.
   
   


-- 
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



[GitHub] [rocketmq] sunxi92 commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
sunxi92 commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r806394508



##########
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";
-

Review comment:
       Default acl file is added.




-- 
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



[GitHub] [rocketmq] codecov-commenter edited a comment on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1013280235


   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#3761](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e6e3548) into [develop](https://codecov.io/gh/apache/rocketmq/commit/6dff04fcb6405fc53a90730cf36ed9a79335bef5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6dff04f) will **increase** coverage by `0.24%`.
   > The diff coverage is `61.92%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/rocketmq/pull/3761/graphs/tree.svg?width=650&height=150&src=pr&token=4w0sxP1wZv&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             develop    #3761      +/-   ##
   =============================================
   + Coverage      46.85%   47.09%   +0.24%     
   - Complexity      4835     4895      +60     
   =============================================
     Files            636      636              
     Lines          42251    42593     +342     
     Branches        5523     5577      +54     
   =============================================
   + Hits           19795    20060     +265     
   - Misses         19973    20018      +45     
   - Partials        2483     2515      +32     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ocketmq/broker/processor/AdminBrokerProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvcHJvY2Vzc29yL0FkbWluQnJva2VyUHJvY2Vzc29yLmphdmE=) | `34.34% <0.00%> (-0.04%)` | :arrow_down: |
   | [...mq/common/protocol/body/ClusterAclVersionInfo.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvYm9keS9DbHVzdGVyQWNsVmVyc2lvbkluZm8uamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...tocol/header/GetBrokerAclConfigResponseHeader.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvaGVhZGVyL0dldEJyb2tlckFjbENvbmZpZ1Jlc3BvbnNlSGVhZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...and/acl/ClusterAclConfigVersionListSubCommand.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2NvbW1hbmQvYWNsL0NsdXN0ZXJBY2xDb25maWdWZXJzaW9uTGlzdFN1YkNvbW1hbmQuamF2YQ==) | `20.83% <0.00%> (-1.39%)` | :arrow_down: |
   | [...g/apache/rocketmq/client/impl/MQClientAPIImpl.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9NUUNsaWVudEFQSUltcGwuamF2YQ==) | `13.93% <16.66%> (-0.05%)` | :arrow_down: |
   | [...pache/rocketmq/acl/plain/PlainAccessValidator.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvcGxhaW4vUGxhaW5BY2Nlc3NWYWxpZGF0b3IuamF2YQ==) | `89.70% <50.00%> (-2.61%)` | :arrow_down: |
   | [...che/rocketmq/acl/plain/PlainPermissionManager.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvcGxhaW4vUGxhaW5QZXJtaXNzaW9uTWFuYWdlci5qYXZh) | `70.57% <69.10%> (-1.31%)` | :arrow_down: |
   | [...ocketmq/store/schedule/ScheduleMessageService.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL3NjaGVkdWxlL1NjaGVkdWxlTWVzc2FnZVNlcnZpY2UuamF2YQ==) | `66.75% <0.00%> (-7.09%)` | :arrow_down: |
   | [.../java/org/apache/rocketmq/acl/common/AclUtils.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvY29tbW9uL0FjbFV0aWxzLmphdmE=) | `76.25% <0.00%> (-3.60%)` | :arrow_down: |
   | [...g/apache/rocketmq/broker/util/ServiceProvider.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvdXRpbC9TZXJ2aWNlUHJvdmlkZXIuamF2YQ==) | `46.42% <0.00%> (-3.58%)` | :arrow_down: |
   | ... and [25 more](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6dff04f...e6e3548](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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



[GitHub] [rocketmq] sunxi92 commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
sunxi92 commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r794980389



##########
File path: tools/src/main/java/org/apache/rocketmq/tools/command/acl/ClusterAclConfigVersionListSubCommand.java
##########
@@ -112,20 +112,19 @@ private void printClusterBaseInfo(
         final DefaultMQAdminExt defaultMQAdminExt, final String addr) throws
         InterruptedException, MQBrokerException, RemotingException, MQClientException {
 
-
         ClusterAclVersionInfo clusterAclVersionInfo = defaultMQAdminExt.examineBrokerClusterAclVersionInfo(addr);
-        DataVersion aclDataVersion = clusterAclVersionInfo.getAclConfigDataVersion();
-        String versionNum = String.valueOf(aclDataVersion.getCounter());
-
+        Map<String, DataVersion> aclDataVersion = clusterAclVersionInfo.getAllAclConfigDataVersion();
         DateFormat sdf = new SimpleDateFormat(UtilAll.YYYY_MM_DD_HH_MM_SS);
-        String timeStampStr = sdf.format(new Timestamp(aclDataVersion.getTimestamp()));
-
-        System.out.printf("%-16s  %-22s  %-22s  %-20s  %-22s%n",
-            clusterAclVersionInfo.getClusterName(),
-            clusterAclVersionInfo.getBrokerName(),
-            clusterAclVersionInfo.getBrokerAddr(),
-            versionNum,
-            timeStampStr
-        );
+        for (Map.Entry<String, DataVersion> entry : aclDataVersion.entrySet()) {

Review comment:
       done.




-- 
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



[GitHub] [rocketmq] lizhiboo commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
lizhiboo commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r808714712



##########
File path: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java
##########
@@ -387,6 +388,12 @@ public ClusterAclVersionInfo getBrokerClusterAclInfo(final String addr,
                 clusterAclVersionInfo.setBrokerName(responseHeader.getBrokerName());
                 clusterAclVersionInfo.setBrokerAddr(responseHeader.getBrokerAddr());
                 clusterAclVersionInfo.setAclConfigDataVersion(DataVersion.fromJson(responseHeader.getVersion(), DataVersion.class));
+                HashMap<String, Object> dataVersionMap = JSON.parseObject(responseHeader.getAllAclFileVersion(), HashMap.class);
+                Map<String, DataVersion> allAclConfigDataVersion = new HashMap<>();

Review comment:
       IMO, specify the concrete type for HashMap for java 1.6.




-- 
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



[GitHub] [rocketmq] sunxi92 commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
sunxi92 commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r806393539



##########
File path: srvutil/src/main/java/org/apache/rocketmq/srvutil/AclFileWatchService.java
##########
@@ -40,19 +40,24 @@
     private int aclFilesNum;
     @Deprecated
     private final Map<String, String> fileCurrentHash;
-    private final Map<String, Long> fileLastModifiedTime;
+    private Map<String, Long> fileLastModifiedTime;
     private List<String/**absolute pathname **/> fileList = new ArrayList<>();
     private final AclFileWatchService.Listener listener;
     private static final int WATCH_INTERVAL = 5000;
     private MessageDigest md = MessageDigest.getInstance("MD5");
+    private String defaultAclFile;
 
     public AclFileWatchService(String path, final AclFileWatchService.Listener listener) throws Exception {
         this.aclPath = path;
+        this.defaultAclFile = path.substring(0, path.length() - 4) + System.getProperty("rocketmq.acl.plain.file", "conf/plain_acl.yml");

Review comment:
       done

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,145 @@
 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 + "conf";
+
+    //private String defaultAclFile = fileHome + File.separator
+      //  + System.getProperty("rocketmq.acl.dir", "/conf") + File.separator + "plain_acl.yml";
+    private String defaultAclFile = fileHome + File.separator + System.getProperty("rocketmq.acl.plain.file", "conf/plain_acl.yml");
+
+    private Map<String/** fileFullPath **/, Map<String/** AccessKey **/, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();
 
-    private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+    private Map<String/** AccessKey **/, String/** fileFullPath **/> accessKeyTable = new HashMap<>();
 
     private List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
 
     private RemoteAddressStrategyFactory remoteAddressStrategyFactory = new RemoteAddressStrategyFactory();
 
+    private Map<String/** fileFullPath **/, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+
     private boolean isWatchStart;
 
+    private Map<String/** fileFullPath **/, DataVersion> dataVersionMap = new HashMap<>();
+
+    @Deprecated
     private final DataVersion dataVersion = new DataVersion();
 
+    private List<String> fileList = new ArrayList<>();
+
     public PlainPermissionManager() {
         load();
         watch();
     }
 
+    public List<String> getAllAclFiles(String path) {
+        List<String>  allAclFileFullPath = new ArrayList<>();
+        File file = new File(path);
+        File[] files = file.listFiles();
+        for (int i = 0; i < files.length; i++) {
+            String fileName = files[i].getAbsolutePath();
+            File f = new File(fileName);
+            if (fileName.equals(fileHome + MixAll.ACL_CONF_TOOLS_FILE)) {
+                continue;
+            } else if (fileName.endsWith(".yml") || fileName.endsWith(".yaml")) {
+                allAclFileFullPath.add(fileName);
+            } else if (f.isDirectory()) {
+                allAclFileFullPath.addAll(getAllAclFiles(fileName));
+            }
+        }
+        return allAclFileFullPath;
+    }
+
     public void load() {
+        if (fileHome == null || fileHome.isEmpty()) {
+            return;
+        }
+
+        Map<String, Map<String, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();
+        Map<String, String> accessKeyTable = new HashMap<>();
+        List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
+        Map<String, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+        Map<String, DataVersion> dataVersionMap = new HashMap<>();
 
+        fileList = getAllAclFiles(defaultAclDir);
+        if (new File(defaultAclFile).exists() && !fileList.contains(defaultAclFile)) {
+            fileList.add(defaultAclFile);
+        }
+
+        for (int i = 0; i < fileList.size(); i++) {
+            JSONObject plainAclConfData = AclUtils.getYamlDataObject(fileList.get(i),
+                JSONObject.class);
+            if (plainAclConfData == null || plainAclConfData.isEmpty()) {
+                throw new AclException(String.format("%s file is not data", fileList.get(i)));
+            }
+            log.info("Broker plain acl conf data is : ", plainAclConfData.toString());
+
+            List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategyList = new ArrayList<>();
+            JSONArray globalWhiteRemoteAddressesList = plainAclConfData.getJSONArray("globalWhiteRemoteAddresses");
+            if (globalWhiteRemoteAddressesList != null && !globalWhiteRemoteAddressesList.isEmpty()) {
+                for (int j = 0; j < globalWhiteRemoteAddressesList.size(); j++) {
+                    globalWhiteRemoteAddressStrategyList.add(remoteAddressStrategyFactory.
+                        getRemoteAddressStrategy(globalWhiteRemoteAddressesList.getString(j)));
+                }
+            }
+            if (globalWhiteRemoteAddressStrategyList.size() > 0) {
+                globalWhiteRemoteAddressStrategyMap.put(fileList.get(i), globalWhiteRemoteAddressStrategyList);
+                globalWhiteRemoteAddressStrategy.addAll(globalWhiteRemoteAddressStrategyList);
+            }
+
+            JSONArray accounts = plainAclConfData.getJSONArray(AclConstants.CONFIG_ACCOUNTS);
+            Map<String, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+            if (accounts != null && !accounts.isEmpty()) {
+                List<PlainAccessConfig> plainAccessConfigList = accounts.toJavaList(PlainAccessConfig.class);
+                for (PlainAccessConfig plainAccessConfig : plainAccessConfigList) {
+                    PlainAccessResource plainAccessResource = buildPlainAccessResource(plainAccessConfig);
+                    //AccessKey can not be defined in multiple ACL files
+                    if (accessKeyTable.get(plainAccessResource.getAccessKey()) == null) {

Review comment:
       done

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,145 @@
 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 + "conf";
+
+    //private String defaultAclFile = fileHome + File.separator
+      //  + System.getProperty("rocketmq.acl.dir", "/conf") + File.separator + "plain_acl.yml";
+    private String defaultAclFile = fileHome + File.separator + System.getProperty("rocketmq.acl.plain.file", "conf/plain_acl.yml");
+
+    private Map<String/** fileFullPath **/, Map<String/** AccessKey **/, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();
 
-    private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+    private Map<String/** AccessKey **/, String/** fileFullPath **/> accessKeyTable = new HashMap<>();
 
     private List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
 
     private RemoteAddressStrategyFactory remoteAddressStrategyFactory = new RemoteAddressStrategyFactory();
 
+    private Map<String/** fileFullPath **/, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+
     private boolean isWatchStart;
 
+    private Map<String/** fileFullPath **/, DataVersion> dataVersionMap = new HashMap<>();
+
+    @Deprecated
     private final DataVersion dataVersion = new DataVersion();
 
+    private List<String> fileList = new ArrayList<>();
+
     public PlainPermissionManager() {
         load();
         watch();
     }
 
+    public List<String> getAllAclFiles(String path) {
+        List<String>  allAclFileFullPath = new ArrayList<>();
+        File file = new File(path);
+        File[] files = file.listFiles();

Review comment:
       done




-- 
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



[GitHub] [rocketmq] sunxi92 commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
sunxi92 commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r790222784



##########
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:
       done

##########
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:
       done




-- 
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



[GitHub] [rocketmq] sunxi92 commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
sunxi92 commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r790222796



##########
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:
       done

##########
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:
       done




-- 
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



[GitHub] [rocketmq] sunxi92 commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
sunxi92 commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r790222823



##########
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:
       done

##########
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:
       done

##########
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:
       done




-- 
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



[GitHub] [rocketmq] caigy commented on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
caigy commented on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1020745066


   @sunxi92 Could you post your detailed design so that more people can join this pr? BTW, it should be well documented about how to use this new feature and how it effects the old implementation, not only in comments but also in user guide.


-- 
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



[GitHub] [rocketmq] coveralls edited a comment on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1013279991


   
   [![Coverage Status](https://coveralls.io/builds/46543788/badge)](https://coveralls.io/builds/46543788)
   
   Coverage increased (+0.5%) to 51.403% when pulling **2d877e6d0a0c2156c17589278214f3992f6a10e0 on sunxi92:acl_update** into **6dff04fcb6405fc53a90730cf36ed9a79335bef5 on apache:develop**.
   


-- 
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



[GitHub] [rocketmq] dongeforever commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
dongeforever commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r789338611



##########
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:
       +1.
   This pr need to supply a detailed design.




-- 
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



[GitHub] [rocketmq] coveralls commented on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1013279991


   
   [![Coverage Status](https://coveralls.io/builds/45638649/badge)](https://coveralls.io/builds/45638649)
   
   Coverage increased (+0.2%) to 51.09% when pulling **2e848e97022fb0c46f467da2549d1b0a46c3ab9e on sunxi92:acl_update** into **6dff04fcb6405fc53a90730cf36ed9a79335bef5 on apache:develop**.
   


-- 
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



[GitHub] [rocketmq] codecov-commenter commented on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1013280235


   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#3761](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2e848e9) into [develop](https://codecov.io/gh/apache/rocketmq/commit/6dff04fcb6405fc53a90730cf36ed9a79335bef5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6dff04f) will **increase** coverage by `0.20%`.
   > The diff coverage is `71.64%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/rocketmq/pull/3761/graphs/tree.svg?width=650&height=150&src=pr&token=4w0sxP1wZv&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             develop    #3761      +/-   ##
   =============================================
   + Coverage      46.85%   47.05%   +0.20%     
   - Complexity      4835     4867      +32     
   =============================================
     Files            636      636              
     Lines          42251    42306      +55     
     Branches        5523     5529       +6     
   =============================================
   + Hits           19795    19909     +114     
   + Misses         19973    19911      -62     
   - Partials        2483     2486       +3     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...mq/common/protocol/body/ClusterAclVersionInfo.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvYm9keS9DbHVzdGVyQWNsVmVyc2lvbkluZm8uamF2YQ==) | `0.00% <ø> (ø)` | |
   | [...tocol/header/GetBrokerAclConfigResponseHeader.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvaGVhZGVyL0dldEJyb2tlckFjbENvbmZpZ1Jlc3BvbnNlSGVhZGVyLmphdmE=) | `0.00% <ø> (ø)` | |
   | [...and/acl/ClusterAclConfigVersionListSubCommand.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2NvbW1hbmQvYWNsL0NsdXN0ZXJBY2xDb25maWdWZXJzaW9uTGlzdFN1YkNvbW1hbmQuamF2YQ==) | `20.83% <0.00%> (-1.39%)` | :arrow_down: |
   | [...pache/rocketmq/acl/plain/PlainAccessValidator.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvcGxhaW4vUGxhaW5BY2Nlc3NWYWxpZGF0b3IuamF2YQ==) | `91.04% <33.33%> (-1.27%)` | :arrow_down: |
   | [...g/apache/rocketmq/client/impl/MQClientAPIImpl.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9NUUNsaWVudEFQSUltcGwuamF2YQ==) | `13.98% <66.66%> (ø)` | |
   | [...che/rocketmq/acl/plain/PlainPermissionManager.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvcGxhaW4vUGxhaW5QZXJtaXNzaW9uTWFuYWdlci5qYXZh) | `75.91% <79.13%> (+4.03%)` | :arrow_up: |
   | [...ketmq/common/protocol/body/ConsumerConnection.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvYm9keS9Db25zdW1lckNvbm5lY3Rpb24uamF2YQ==) | `95.83% <0.00%> (-4.17%)` | :arrow_down: |
   | [...rg/apache/rocketmq/remoting/netty/NettyLogger.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL25ldHR5L05ldHR5TG9nZ2VyLmphdmE=) | `14.96% <0.00%> (-1.37%)` | :arrow_down: |
   | [...ent/impl/consumer/DefaultLitePullConsumerImpl.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9jb25zdW1lci9EZWZhdWx0TGl0ZVB1bGxDb25zdW1lckltcGwuamF2YQ==) | `67.99% <0.00%> (-0.52%)` | :arrow_down: |
   | ... and [19 more](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6dff04f...2e848e9](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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



[GitHub] [rocketmq] caigy commented on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
caigy commented on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1020745066


   @sunxi92 Could you post your detailed design so that more people can join this pr? BTW, it should be well documented about how to use this new feature and how it effects the old implementation, not only in comments but also in user guide.


-- 
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



[GitHub] [rocketmq] sunxi92 commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
sunxi92 commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r808752710



##########
File path: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java
##########
@@ -387,6 +388,12 @@ public ClusterAclVersionInfo getBrokerClusterAclInfo(final String addr,
                 clusterAclVersionInfo.setBrokerName(responseHeader.getBrokerName());
                 clusterAclVersionInfo.setBrokerAddr(responseHeader.getBrokerAddr());
                 clusterAclVersionInfo.setAclConfigDataVersion(DataVersion.fromJson(responseHeader.getVersion(), DataVersion.class));
+                HashMap<String, Object> dataVersionMap = JSON.parseObject(responseHeader.getAllAclFileVersion(), HashMap.class);
+                Map<String, DataVersion> allAclConfigDataVersion = new HashMap<>();

Review comment:
       done




-- 
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



[GitHub] [rocketmq] codecov-commenter edited a comment on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1013280235


   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#3761](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a5691ab) into [develop](https://codecov.io/gh/apache/rocketmq/commit/6dff04fcb6405fc53a90730cf36ed9a79335bef5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6dff04f) will **increase** coverage by `0.52%`.
   > The diff coverage is `63.15%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/rocketmq/pull/3761/graphs/tree.svg?width=650&height=150&src=pr&token=4w0sxP1wZv&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             develop    #3761      +/-   ##
   =============================================
   + Coverage      46.85%   47.37%   +0.52%     
   - Complexity      4835     4930      +95     
   =============================================
     Files            636      636              
     Lines          42251    42650     +399     
     Branches        5523     5593      +70     
   =============================================
   + Hits           19795    20204     +409     
   + Misses         19973    19926      -47     
   - Partials        2483     2520      +37     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ocketmq/broker/processor/AdminBrokerProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvcHJvY2Vzc29yL0FkbWluQnJva2VyUHJvY2Vzc29yLmphdmE=) | `34.34% <0.00%> (-0.04%)` | :arrow_down: |
   | [...mq/common/protocol/body/ClusterAclVersionInfo.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvYm9keS9DbHVzdGVyQWNsVmVyc2lvbkluZm8uamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...tocol/header/GetBrokerAclConfigResponseHeader.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvaGVhZGVyL0dldEJyb2tlckFjbENvbmZpZ1Jlc3BvbnNlSGVhZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...and/acl/ClusterAclConfigVersionListSubCommand.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2NvbW1hbmQvYWNsL0NsdXN0ZXJBY2xDb25maWdWZXJzaW9uTGlzdFN1YkNvbW1hbmQuamF2YQ==) | `20.40% <0.00%> (-1.82%)` | :arrow_down: |
   | [...g/apache/rocketmq/client/impl/MQClientAPIImpl.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9NUUNsaWVudEFQSUltcGwuamF2YQ==) | `13.93% <16.66%> (-0.05%)` | :arrow_down: |
   | [...pache/rocketmq/acl/plain/PlainAccessValidator.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvcGxhaW4vUGxhaW5BY2Nlc3NWYWxpZGF0b3IuamF2YQ==) | `89.70% <50.00%> (-2.61%)` | :arrow_down: |
   | [...che/rocketmq/acl/plain/PlainPermissionManager.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvcGxhaW4vUGxhaW5QZXJtaXNzaW9uTWFuYWdlci5qYXZh) | `70.89% <69.86%> (-0.98%)` | :arrow_down: |
   | [...tmq/broker/longpolling/PullRequestHoldService.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvbG9uZ3BvbGxpbmcvUHVsbFJlcXVlc3RIb2xkU2VydmljZS5qYXZh) | `12.04% <0.00%> (-8.44%)` | :arrow_down: |
   | [...ocketmq/store/schedule/ScheduleMessageService.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL3NjaGVkdWxlL1NjaGVkdWxlTWVzc2FnZVNlcnZpY2UuamF2YQ==) | `66.75% <0.00%> (-7.09%)` | :arrow_down: |
   | [...ketmq/common/protocol/body/ConsumerConnection.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvYm9keS9Db25zdW1lckNvbm5lY3Rpb24uamF2YQ==) | `95.83% <0.00%> (-4.17%)` | :arrow_down: |
   | ... and [44 more](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6dff04f...a5691ab](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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



[GitHub] [rocketmq] coveralls edited a comment on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1013279991






-- 
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



[GitHub] [rocketmq] sunxi92 commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
sunxi92 commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r790222836



##########
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:
       done

##########
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:
       done

##########
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:
       done

##########
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:
       done




-- 
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



[GitHub] [rocketmq] codecov-commenter edited a comment on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1013280235


   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#3761](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6248b47) into [develop](https://codecov.io/gh/apache/rocketmq/commit/6dff04fcb6405fc53a90730cf36ed9a79335bef5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6dff04f) will **increase** coverage by `0.37%`.
   > The diff coverage is `64.19%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/rocketmq/pull/3761/graphs/tree.svg?width=650&height=150&src=pr&token=4w0sxP1wZv&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             develop    #3761      +/-   ##
   =============================================
   + Coverage      46.85%   47.22%   +0.37%     
   - Complexity      4835     4909      +74     
   =============================================
     Files            636      636              
     Lines          42251    42626     +375     
     Branches        5523     5583      +60     
   =============================================
   + Hits           19795    20130     +335     
   - Misses         19973    19982       +9     
   - Partials        2483     2514      +31     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ocketmq/broker/processor/AdminBrokerProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvcHJvY2Vzc29yL0FkbWluQnJva2VyUHJvY2Vzc29yLmphdmE=) | `34.34% <0.00%> (-0.04%)` | :arrow_down: |
   | [...mq/common/protocol/body/ClusterAclVersionInfo.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvYm9keS9DbHVzdGVyQWNsVmVyc2lvbkluZm8uamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...tocol/header/GetBrokerAclConfigResponseHeader.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvaGVhZGVyL0dldEJyb2tlckFjbENvbmZpZ1Jlc3BvbnNlSGVhZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...and/acl/ClusterAclConfigVersionListSubCommand.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2NvbW1hbmQvYWNsL0NsdXN0ZXJBY2xDb25maWdWZXJzaW9uTGlzdFN1YkNvbW1hbmQuamF2YQ==) | `20.40% <0.00%> (-1.82%)` | :arrow_down: |
   | [...g/apache/rocketmq/client/impl/MQClientAPIImpl.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9NUUNsaWVudEFQSUltcGwuamF2YQ==) | `13.93% <16.66%> (-0.05%)` | :arrow_down: |
   | [...pache/rocketmq/acl/plain/PlainAccessValidator.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvcGxhaW4vUGxhaW5BY2Nlc3NWYWxpZGF0b3IuamF2YQ==) | `89.70% <50.00%> (-2.61%)` | :arrow_down: |
   | [...che/rocketmq/acl/plain/PlainPermissionManager.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvcGxhaW4vUGxhaW5QZXJtaXNzaW9uTWFuYWdlci5qYXZh) | `71.65% <71.16%> (-0.22%)` | :arrow_down: |
   | [...ocketmq/store/schedule/ScheduleMessageService.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL3NjaGVkdWxlL1NjaGVkdWxlTWVzc2FnZVNlcnZpY2UuamF2YQ==) | `66.75% <0.00%> (-7.09%)` | :arrow_down: |
   | [...sumer/rebalance/AllocateMessageQueueAveragely.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvY29uc3VtZXIvcmViYWxhbmNlL0FsbG9jYXRlTWVzc2FnZVF1ZXVlQXZlcmFnZWx5LmphdmE=) | `56.52% <0.00%> (-4.35%)` | :arrow_down: |
   | [.../java/org/apache/rocketmq/acl/common/AclUtils.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvY29tbW9uL0FjbFV0aWxzLmphdmE=) | `76.25% <0.00%> (-3.60%)` | :arrow_down: |
   | ... and [33 more](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6dff04f...6248b47](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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



[GitHub] [rocketmq] coveralls edited a comment on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1013279991


   
   [![Coverage Status](https://coveralls.io/builds/46546552/badge)](https://coveralls.io/builds/46546552)
   
   Coverage increased (+0.5%) to 51.427% when pulling **b323e8ea5251052cf8435d297f5c8f95e16d0a11 on sunxi92:acl_update** into **6dff04fcb6405fc53a90730cf36ed9a79335bef5 on apache:develop**.
   


-- 
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



[GitHub] [rocketmq] duhenglucky merged pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
duhenglucky merged pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761


   


-- 
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



[GitHub] [rocketmq] caigy commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
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



[GitHub] [rocketmq] sunxi92 commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
sunxi92 commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r790222769



##########
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:
       done

##########
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:
       done




-- 
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



[GitHub] [rocketmq] coveralls edited a comment on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1013279991


   
   [![Coverage Status](https://coveralls.io/builds/45865113/badge)](https://coveralls.io/builds/45865113)
   
   Coverage increased (+0.3%) to 51.177% when pulling **e6e3548f05105644a66230e0d1cb62e99405c56d on sunxi92:acl_update** into **6dff04fcb6405fc53a90730cf36ed9a79335bef5 on apache:develop**.
   


-- 
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



[GitHub] [rocketmq] caigy commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
caigy commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r805465656



##########
File path: srvutil/src/main/java/org/apache/rocketmq/srvutil/AclFileWatchService.java
##########
@@ -40,19 +40,24 @@
     private int aclFilesNum;
     @Deprecated
     private final Map<String, String> fileCurrentHash;
-    private final Map<String, Long> fileLastModifiedTime;
+    private Map<String, Long> fileLastModifiedTime;
     private List<String/**absolute pathname **/> fileList = new ArrayList<>();
     private final AclFileWatchService.Listener listener;
     private static final int WATCH_INTERVAL = 5000;
     private MessageDigest md = MessageDigest.getInstance("MD5");
+    private String defaultAclFile;
 
     public AclFileWatchService(String path, final AclFileWatchService.Listener listener) throws Exception {
         this.aclPath = path;
+        this.defaultAclFile = path.substring(0, path.length() - 4) + System.getProperty("rocketmq.acl.plain.file", "conf/plain_acl.yml");

Review comment:
       Just get parent directory instead of string truncation, or make `path` as a `List`.




-- 
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



[GitHub] [rocketmq] codecov-commenter edited a comment on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1013280235


   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#3761](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2d877e6) into [develop](https://codecov.io/gh/apache/rocketmq/commit/6dff04fcb6405fc53a90730cf36ed9a79335bef5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6dff04f) will **increase** coverage by `0.46%`.
   > The diff coverage is `60.56%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/rocketmq/pull/3761/graphs/tree.svg?width=650&height=150&src=pr&token=4w0sxP1wZv&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             develop    #3761      +/-   ##
   =============================================
   + Coverage      46.85%   47.31%   +0.46%     
   - Complexity      4835     4914      +79     
   =============================================
     Files            636      636              
     Lines          42251    42649     +398     
     Branches        5523     5593      +70     
   =============================================
   + Hits           19795    20178     +383     
   + Misses         19973    19945      -28     
   - Partials        2483     2526      +43     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ocketmq/broker/processor/AdminBrokerProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvcHJvY2Vzc29yL0FkbWluQnJva2VyUHJvY2Vzc29yLmphdmE=) | `34.34% <0.00%> (-0.04%)` | :arrow_down: |
   | [...mq/common/protocol/body/ClusterAclVersionInfo.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvYm9keS9DbHVzdGVyQWNsVmVyc2lvbkluZm8uamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...tocol/header/GetBrokerAclConfigResponseHeader.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvaGVhZGVyL0dldEJyb2tlckFjbENvbmZpZ1Jlc3BvbnNlSGVhZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...and/acl/ClusterAclConfigVersionListSubCommand.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2NvbW1hbmQvYWNsL0NsdXN0ZXJBY2xDb25maWdWZXJzaW9uTGlzdFN1YkNvbW1hbmQuamF2YQ==) | `20.40% <0.00%> (-1.82%)` | :arrow_down: |
   | [...g/apache/rocketmq/client/impl/MQClientAPIImpl.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9NUUNsaWVudEFQSUltcGwuamF2YQ==) | `13.93% <16.66%> (-0.05%)` | :arrow_down: |
   | [...pache/rocketmq/acl/plain/PlainAccessValidator.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvcGxhaW4vUGxhaW5BY2Nlc3NWYWxpZGF0b3IuamF2YQ==) | `89.70% <50.00%> (-2.61%)` | :arrow_down: |
   | [...che/rocketmq/acl/plain/PlainPermissionManager.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvcGxhaW4vUGxhaW5QZXJtaXNzaW9uTWFuYWdlci5qYXZh) | `68.43% <66.97%> (-3.44%)` | :arrow_down: |
   | [...ocketmq/store/schedule/ScheduleMessageService.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL3NjaGVkdWxlL1NjaGVkdWxlTWVzc2FnZVNlcnZpY2UuamF2YQ==) | `66.75% <0.00%> (-7.09%)` | :arrow_down: |
   | [.../java/org/apache/rocketmq/acl/common/AclUtils.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvY29tbW9uL0FjbFV0aWxzLmphdmE=) | `76.25% <0.00%> (-3.60%)` | :arrow_down: |
   | [...g/apache/rocketmq/broker/util/ServiceProvider.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvdXRpbC9TZXJ2aWNlUHJvdmlkZXIuamF2YQ==) | `46.42% <0.00%> (-3.58%)` | :arrow_down: |
   | ... and [44 more](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6dff04f...2d877e6](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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



[GitHub] [rocketmq] codecov-commenter edited a comment on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1013280235






-- 
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



[GitHub] [rocketmq] sunxi92 commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
sunxi92 commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r794980193



##########
File path: common/src/main/java/org/apache/rocketmq/common/protocol/header/GetBrokerAclConfigResponseHeader.java
##########
@@ -16,15 +16,28 @@
  */
 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> versions;

Review comment:
       done.

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -267,54 +394,107 @@ 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 AclUtils.writeDataObject(defaultAclFile, updateAclConfigFileVersion(aclAccessConfigMap));
+        }
+
+        log.error("Users must ensure that the acl yaml config file has globalWhiteRemoteAddresses flag in the {} firstly", defaultAclFile);
+        return false;
+    }
+
+    public boolean updateGlobalWhiteAddrsConfig(List<String> globalWhiteAddrsList, String fileName) {
+        if (globalWhiteAddrsList == null) {
+            log.error("Parameter value globalWhiteAddrsList is null,Please check your parameter");
             return false;
         }
 
-        log.error("Users must ensure that the acl yaml config file has globalWhiteRemoteAddresses flag firstly");
+        File file = new File(fileName);
+        if (!file.exists() || file.isDirectory()) {
+            log.error("Parameter value fileName is not exist or is a directory,Please check your parameter");
+            return false;
+        }
+
+        Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(fileName, Map.class);
+        if (aclAccessConfigMap == null || aclAccessConfigMap.isEmpty()) {
+            throw new AclException(String.format("the %s file is not found or empty", fileName));
+        }
+        List<String> globalWhiteRemoteAddrList = (List<String>) aclAccessConfigMap.get(AclConstants.CONFIG_GLOBAL_WHITE_ADDRS);
+        if (globalWhiteRemoteAddrList != null) {
+            globalWhiteRemoteAddrList.clear();
+            if (globalWhiteAddrsList != null) {
+                globalWhiteRemoteAddrList.addAll(globalWhiteAddrsList);

Review comment:
       done.

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,108 @@
 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");
+
+    private String defaultAclFile = fileHome + File.separator
+        + System.getProperty("rocketmq.acl.dir", "/conf") + 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 Map<String/** aclFileName **/, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+
     private boolean isWatchStart;
 
+    private Map<String/** aclFileName **/, DataVersion> dataVersionMap = new HashMap<>();
+
+    @Deprecated
     private final DataVersion dataVersion = new DataVersion();
 
+    private List<String> fileList = new ArrayList<>();
+
     public PlainPermissionManager() {
         load();
         watch();
     }
 
+    public void getAllAclFiles(String path) {
+        File file = new File(path);
+        File[] files = file.listFiles();
+        for (int i = 0; i < files.length; i++) {
+            String fileName = files[i].getAbsolutePath();
+            File f = new File(fileName);
+            if (fileName.endsWith(".yml")) {
+                fileList.add(fileName);
+            } else if (!f.isFile()) {
+                getAllAclFiles(fileName);
+            }
+        }
+    }
+
     public void load() {
+        if (fileHome == null || fileHome.isEmpty()) {
+            //throw new AclException(String.format("%s file is empty", fileHome));
+            return;
+        }
+        if (fileList.size() > 0) {
+            fileList.clear();
+        }
+        getAllAclFiles(defaultAclDir);
+        if (fileList == null || fileList.size() == 0)
+            return;
+
+        if (aclPlainAccessResourceMap.size() != 0 && accessKeyTable.size() != 0) {
+            aclPlainAccessResourceMap.clear();
+            accessKeyTable.clear();
+        }
+        if (globalWhiteRemoteAddressStrategy.size() != 0) {
+            globalWhiteRemoteAddressStrategy.clear();
+        }
+        if (globalWhiteRemoteAddressStrategyMap.size() != 0) {
+            globalWhiteRemoteAddressStrategyMap.clear();
+        }

Review comment:
       done.




-- 
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



[GitHub] [rocketmq] caigy commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
caigy commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r791329030



##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,108 @@
 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");
+
+    private String defaultAclFile = fileHome + File.separator
+        + System.getProperty("rocketmq.acl.dir", "/conf") + File.separator + "plain_acl.yml";
 
-    private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+    private Map<String/** aclFileName **/, Map<String/** AccessKey **/, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();

Review comment:
       Is 'file full path' more precise than 'aclFileName'?

##########
File path: tools/src/main/java/org/apache/rocketmq/tools/command/acl/ClusterAclConfigVersionListSubCommand.java
##########
@@ -112,20 +112,19 @@ private void printClusterBaseInfo(
         final DefaultMQAdminExt defaultMQAdminExt, final String addr) throws
         InterruptedException, MQBrokerException, RemotingException, MQClientException {
 
-
         ClusterAclVersionInfo clusterAclVersionInfo = defaultMQAdminExt.examineBrokerClusterAclVersionInfo(addr);
-        DataVersion aclDataVersion = clusterAclVersionInfo.getAclConfigDataVersion();
-        String versionNum = String.valueOf(aclDataVersion.getCounter());
-
+        Map<String, DataVersion> aclDataVersion = clusterAclVersionInfo.getAllAclConfigDataVersion();
         DateFormat sdf = new SimpleDateFormat(UtilAll.YYYY_MM_DD_HH_MM_SS);
-        String timeStampStr = sdf.format(new Timestamp(aclDataVersion.getTimestamp()));
-
-        System.out.printf("%-16s  %-22s  %-22s  %-20s  %-22s%n",
-            clusterAclVersionInfo.getClusterName(),
-            clusterAclVersionInfo.getBrokerName(),
-            clusterAclVersionInfo.getBrokerAddr(),
-            versionNum,
-            timeStampStr
-        );
+        for (Map.Entry<String, DataVersion> entry : aclDataVersion.entrySet()) {

Review comment:
       Would `AccessValidator.getAllAclConfigVersion()` return an empty set? 

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,108 @@
 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");
+
+    private String defaultAclFile = fileHome + File.separator
+        + System.getProperty("rocketmq.acl.dir", "/conf") + 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 Map<String/** aclFileName **/, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+
     private boolean isWatchStart;
 
+    private Map<String/** aclFileName **/, DataVersion> dataVersionMap = new HashMap<>();
+
+    @Deprecated
     private final DataVersion dataVersion = new DataVersion();
 
+    private List<String> fileList = new ArrayList<>();
+
     public PlainPermissionManager() {
         load();
         watch();
     }
 
+    public void getAllAclFiles(String path) {
+        File file = new File(path);
+        File[] files = file.listFiles();
+        for (int i = 0; i < files.length; i++) {
+            String fileName = files[i].getAbsolutePath();
+            File f = new File(fileName);
+            if (fileName.endsWith(".yml")) {

Review comment:
       '.yaml' is also a suffix to yaml files

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,108 @@
 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");
+
+    private String defaultAclFile = fileHome + File.separator
+        + System.getProperty("rocketmq.acl.dir", "/conf") + 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 Map<String/** aclFileName **/, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+
     private boolean isWatchStart;
 
+    private Map<String/** aclFileName **/, DataVersion> dataVersionMap = new HashMap<>();
+
+    @Deprecated
     private final DataVersion dataVersion = new DataVersion();
 
+    private List<String> fileList = new ArrayList<>();
+
     public PlainPermissionManager() {
         load();
         watch();
     }
 
+    public void getAllAclFiles(String path) {
+        File file = new File(path);
+        File[] files = file.listFiles();
+        for (int i = 0; i < files.length; i++) {
+            String fileName = files[i].getAbsolutePath();
+            File f = new File(fileName);
+            if (fileName.endsWith(".yml")) {
+                fileList.add(fileName);
+            } else if (!f.isFile()) {
+                getAllAclFiles(fileName);
+            }
+        }
+    }
+
     public void load() {
+        if (fileHome == null || fileHome.isEmpty()) {
+            //throw new AclException(String.format("%s file is empty", fileHome));
+            return;
+        }
+        if (fileList.size() > 0) {
+            fileList.clear();
+        }
+        getAllAclFiles(defaultAclDir);
+        if (fileList == null || fileList.size() == 0)
+            return;
+
+        if (aclPlainAccessResourceMap.size() != 0 && accessKeyTable.size() != 0) {
+            aclPlainAccessResourceMap.clear();
+            accessKeyTable.clear();
+        }
+        if (globalWhiteRemoteAddressStrategy.size() != 0) {
+            globalWhiteRemoteAddressStrategy.clear();
+        }
+        if (globalWhiteRemoteAddressStrategyMap.size() != 0) {
+            globalWhiteRemoteAddressStrategyMap.clear();
+        }
+        for (int i = 0; i < fileList.size(); i++) {
+            load(fileList.get(i));
+        }
 
+        if (dataVersionMap.size() != fileList.size()) {
+            Iterator<Map.Entry<String, DataVersion>> it = dataVersionMap.entrySet().iterator();
+            while (it.hasNext()) {
+                Map.Entry<String, DataVersion> entry = it.next();
+                if (!fileList.contains(entry.getKey()))
+                    it.remove();
+            }
+        }
+        if (dataVersionMap.size() == 1) {
+            for (Map.Entry<String, DataVersion> entry : dataVersionMap.entrySet()) {
+                dataVersion.assignNewOne(entry.getValue());
+            }
+        }

Review comment:
       Would dataVersion be empty or updated if there are multiple ACL config files?

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/AccessValidator.java
##########
@@ -60,17 +63,27 @@
      *
      * @return
      */
+    @Deprecated
     String getAclConfigVersion();

Review comment:
       Pls add more description about:
   - the reason to deprecate `String getAclConfigVersion()` and replace it with `Map<String, DataVersion> getAllAclConfigVersion()`
   - What is the object or scope that **version** describe? In previous implementation, **version** is used to mark the whole config. In this multi-filed feature, each config file has its own **version**. And if ACL config are stored in DB, each record may have its own **version**. We should define **version** precisely.
   -  In `Map<String, DataVersion> getAllAclConfigVersion()`, what is the key of Map? Users may curious of it. If this is the full path of config files, it brings implementation to interface.

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -85,43 +155,76 @@ public void load() {
             }
         }
 
+        if (this.globalWhiteRemoteAddressStrategyMap.get(aclFilePath) != null) {
+            List<RemoteAddressStrategy> remoteAddressStrategyList = this.globalWhiteRemoteAddressStrategyMap.get(aclFilePath);
+            for (int i = 0; i < remoteAddressStrategyList.size(); i++) {
+                this.globalWhiteRemoteAddressStrategy.remove(remoteAddressStrategyList.get(i));
+            }
+            this.globalWhiteRemoteAddressStrategyMap.remove(aclFilePath);

Review comment:
       Removing items in `globalWhiteRemoteAddressStrategy` may cause ACL check failures.

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,108 @@
 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");
+
+    private String defaultAclFile = fileHome + File.separator
+        + System.getProperty("rocketmq.acl.dir", "/conf") + 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 Map<String/** aclFileName **/, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+
     private boolean isWatchStart;
 
+    private Map<String/** aclFileName **/, DataVersion> dataVersionMap = new HashMap<>();
+
+    @Deprecated
     private final DataVersion dataVersion = new DataVersion();
 
+    private List<String> fileList = new ArrayList<>();
+
     public PlainPermissionManager() {
         load();
         watch();
     }
 
+    public void getAllAclFiles(String path) {
+        File file = new File(path);
+        File[] files = file.listFiles();
+        for (int i = 0; i < files.length; i++) {
+            String fileName = files[i].getAbsolutePath();
+            File f = new File(fileName);
+            if (fileName.endsWith(".yml")) {
+                fileList.add(fileName);
+            } else if (!f.isFile()) {

Review comment:
       Maybe it should be `file.isDirectory()`?

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -267,54 +394,107 @@ 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 AclUtils.writeDataObject(defaultAclFile, updateAclConfigFileVersion(aclAccessConfigMap));
+        }
+
+        log.error("Users must ensure that the acl yaml config file has globalWhiteRemoteAddresses flag in the {} firstly", defaultAclFile);
+        return false;
+    }
+
+    public boolean updateGlobalWhiteAddrsConfig(List<String> globalWhiteAddrsList, String fileName) {
+        if (globalWhiteAddrsList == null) {
+            log.error("Parameter value globalWhiteAddrsList is null,Please check your parameter");
             return false;
         }
 
-        log.error("Users must ensure that the acl yaml config file has globalWhiteRemoteAddresses flag firstly");
+        File file = new File(fileName);
+        if (!file.exists() || file.isDirectory()) {
+            log.error("Parameter value fileName is not exist or is a directory,Please check your parameter");
+            return false;
+        }
+
+        Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(fileName, Map.class);
+        if (aclAccessConfigMap == null || aclAccessConfigMap.isEmpty()) {
+            throw new AclException(String.format("the %s file is not found or empty", fileName));
+        }
+        List<String> globalWhiteRemoteAddrList = (List<String>) aclAccessConfigMap.get(AclConstants.CONFIG_GLOBAL_WHITE_ADDRS);
+        if (globalWhiteRemoteAddrList != null) {
+            globalWhiteRemoteAddrList.clear();
+            if (globalWhiteAddrsList != null) {
+                globalWhiteRemoteAddrList.addAll(globalWhiteAddrsList);

Review comment:
       Combination of `clear()` and `addAll()` is not atomic, which may cause ACL failure in this process.

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,108 @@
 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");
+
+    private String defaultAclFile = fileHome + File.separator
+        + System.getProperty("rocketmq.acl.dir", "/conf") + 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 Map<String/** aclFileName **/, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+
     private boolean isWatchStart;
 
+    private Map<String/** aclFileName **/, DataVersion> dataVersionMap = new HashMap<>();
+
+    @Deprecated
     private final DataVersion dataVersion = new DataVersion();
 
+    private List<String> fileList = new ArrayList<>();
+
     public PlainPermissionManager() {
         load();
         watch();
     }
 
+    public void getAllAclFiles(String path) {
+        File file = new File(path);
+        File[] files = file.listFiles();
+        for (int i = 0; i < files.length; i++) {
+            String fileName = files[i].getAbsolutePath();
+            File f = new File(fileName);
+            if (fileName.endsWith(".yml")) {
+                fileList.add(fileName);
+            } else if (!f.isFile()) {
+                getAllAclFiles(fileName);
+            }
+        }
+    }
+
     public void load() {
+        if (fileHome == null || fileHome.isEmpty()) {
+            //throw new AclException(String.format("%s file is empty", fileHome));
+            return;
+        }
+        if (fileList.size() > 0) {
+            fileList.clear();
+        }
+        getAllAclFiles(defaultAclDir);
+        if (fileList == null || fileList.size() == 0)
+            return;
+
+        if (aclPlainAccessResourceMap.size() != 0 && accessKeyTable.size() != 0) {
+            aclPlainAccessResourceMap.clear();
+            accessKeyTable.clear();
+        }
+        if (globalWhiteRemoteAddressStrategy.size() != 0) {
+            globalWhiteRemoteAddressStrategy.clear();
+        }
+        if (globalWhiteRemoteAddressStrategyMap.size() != 0) {
+            globalWhiteRemoteAddressStrategyMap.clear();
+        }

Review comment:
       Clearing globalWhiteRemoteAddressStrategy or globalWhiteRemoteAddressStrategyMap may cause ACL check failures before load() finished.

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,108 @@
 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

Review comment:
       Can users specify their own acl dir?

##########
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:
       It seems users can't specify config file by property `rocketmq.acl.plain.file`?

##########
File path: common/src/main/java/org/apache/rocketmq/common/protocol/header/GetBrokerAclConfigResponseHeader.java
##########
@@ -16,15 +16,28 @@
  */
 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> versions;

Review comment:
       `version` and `versions` are too similar and confusing, maybe we should name them more clearly.




-- 
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



[GitHub] [rocketmq] coveralls edited a comment on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1013279991


   
   [![Coverage Status](https://coveralls.io/builds/46630052/badge)](https://coveralls.io/builds/46630052)
   
   Coverage increased (+0.5%) to 51.399% when pulling **4dae014e4a82a2e5452e3d45128251776fef6151 on sunxi92:acl_update** into **6dff04fcb6405fc53a90730cf36ed9a79335bef5 on apache:develop**.
   


-- 
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



[GitHub] [rocketmq] codecov-commenter edited a comment on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1013280235


   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#3761](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d4629ac) into [develop](https://codecov.io/gh/apache/rocketmq/commit/6dff04fcb6405fc53a90730cf36ed9a79335bef5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6dff04f) will **increase** coverage by `0.39%`.
   > The diff coverage is `63.93%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/rocketmq/pull/3761/graphs/tree.svg?width=650&height=150&src=pr&token=4w0sxP1wZv&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             develop    #3761      +/-   ##
   =============================================
   + Coverage      46.85%   47.24%   +0.39%     
   - Complexity      4835     4913      +78     
   =============================================
     Files            636      636              
     Lines          42251    42647     +396     
     Branches        5523     5593      +70     
   =============================================
   + Hits           19795    20148     +353     
     Misses         19973    19973              
   - Partials        2483     2526      +43     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ocketmq/broker/processor/AdminBrokerProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvcHJvY2Vzc29yL0FkbWluQnJva2VyUHJvY2Vzc29yLmphdmE=) | `34.34% <0.00%> (-0.04%)` | :arrow_down: |
   | [...mq/common/protocol/body/ClusterAclVersionInfo.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvYm9keS9DbHVzdGVyQWNsVmVyc2lvbkluZm8uamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...tocol/header/GetBrokerAclConfigResponseHeader.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvaGVhZGVyL0dldEJyb2tlckFjbENvbmZpZ1Jlc3BvbnNlSGVhZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...and/acl/ClusterAclConfigVersionListSubCommand.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2NvbW1hbmQvYWNsL0NsdXN0ZXJBY2xDb25maWdWZXJzaW9uTGlzdFN1YkNvbW1hbmQuamF2YQ==) | `20.40% <0.00%> (-1.82%)` | :arrow_down: |
   | [...g/apache/rocketmq/client/impl/MQClientAPIImpl.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9NUUNsaWVudEFQSUltcGwuamF2YQ==) | `13.93% <16.66%> (-0.05%)` | :arrow_down: |
   | [...pache/rocketmq/acl/plain/PlainAccessValidator.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvcGxhaW4vUGxhaW5BY2Nlc3NWYWxpZGF0b3IuamF2YQ==) | `89.70% <50.00%> (-2.61%)` | :arrow_down: |
   | [...che/rocketmq/acl/plain/PlainPermissionManager.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvcGxhaW4vUGxhaW5QZXJtaXNzaW9uTWFuYWdlci5qYXZh) | `71.46% <70.83%> (-0.41%)` | :arrow_down: |
   | [...ocketmq/store/schedule/ScheduleMessageService.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL3NjaGVkdWxlL1NjaGVkdWxlTWVzc2FnZVNlcnZpY2UuamF2YQ==) | `66.75% <0.00%> (-7.09%)` | :arrow_down: |
   | [...ketmq/common/protocol/body/ConsumerConnection.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvYm9keS9Db25zdW1lckNvbm5lY3Rpb24uamF2YQ==) | `95.83% <0.00%> (-4.17%)` | :arrow_down: |
   | [.../java/org/apache/rocketmq/acl/common/AclUtils.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvY29tbW9uL0FjbFV0aWxzLmphdmE=) | `76.25% <0.00%> (-3.60%)` | :arrow_down: |
   | ... and [42 more](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6dff04f...d4629ac](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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



[GitHub] [rocketmq] coveralls edited a comment on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1013279991


   
   [![Coverage Status](https://coveralls.io/builds/46482324/badge)](https://coveralls.io/builds/46482324)
   
   Coverage increased (+0.5%) to 51.345% when pulling **d4629ac4168ba11cc4c6330f7a7eae59c6904c5c on sunxi92:acl_update** into **6dff04fcb6405fc53a90730cf36ed9a79335bef5 on apache:develop**.
   


-- 
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



[GitHub] [rocketmq] codecov-commenter edited a comment on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1013280235


   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#3761](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b323e8e) into [develop](https://codecov.io/gh/apache/rocketmq/commit/6dff04fcb6405fc53a90730cf36ed9a79335bef5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6dff04f) will **increase** coverage by `0.47%`.
   > The diff coverage is `63.82%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/rocketmq/pull/3761/graphs/tree.svg?width=650&height=150&src=pr&token=4w0sxP1wZv&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             develop    #3761      +/-   ##
   =============================================
   + Coverage      46.85%   47.32%   +0.47%     
   - Complexity      4835     4925      +90     
   =============================================
     Files            636      636              
     Lines          42251    42649     +398     
     Branches        5523     5593      +70     
   =============================================
   + Hits           19795    20185     +390     
   + Misses         19973    19939      -34     
   - Partials        2483     2525      +42     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ocketmq/broker/processor/AdminBrokerProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvcHJvY2Vzc29yL0FkbWluQnJva2VyUHJvY2Vzc29yLmphdmE=) | `34.34% <0.00%> (-0.04%)` | :arrow_down: |
   | [...mq/common/protocol/body/ClusterAclVersionInfo.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvYm9keS9DbHVzdGVyQWNsVmVyc2lvbkluZm8uamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...tocol/header/GetBrokerAclConfigResponseHeader.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvaGVhZGVyL0dldEJyb2tlckFjbENvbmZpZ1Jlc3BvbnNlSGVhZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...and/acl/ClusterAclConfigVersionListSubCommand.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2NvbW1hbmQvYWNsL0NsdXN0ZXJBY2xDb25maWdWZXJzaW9uTGlzdFN1YkNvbW1hbmQuamF2YQ==) | `20.40% <0.00%> (-1.82%)` | :arrow_down: |
   | [...g/apache/rocketmq/client/impl/MQClientAPIImpl.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9NUUNsaWVudEFQSUltcGwuamF2YQ==) | `13.93% <16.66%> (-0.05%)` | :arrow_down: |
   | [...pache/rocketmq/acl/plain/PlainAccessValidator.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvcGxhaW4vUGxhaW5BY2Nlc3NWYWxpZGF0b3IuamF2YQ==) | `89.70% <50.00%> (-2.61%)` | :arrow_down: |
   | [...che/rocketmq/acl/plain/PlainPermissionManager.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvcGxhaW4vUGxhaW5QZXJtaXNzaW9uTWFuYWdlci5qYXZh) | `71.35% <70.64%> (-0.53%)` | :arrow_down: |
   | [...ocketmq/store/schedule/ScheduleMessageService.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL3NjaGVkdWxlL1NjaGVkdWxlTWVzc2FnZVNlcnZpY2UuamF2YQ==) | `66.75% <0.00%> (-7.09%)` | :arrow_down: |
   | [...ketmq/common/protocol/body/ConsumerConnection.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvYm9keS9Db25zdW1lckNvbm5lY3Rpb24uamF2YQ==) | `95.83% <0.00%> (-4.17%)` | :arrow_down: |
   | [.../java/org/apache/rocketmq/acl/common/AclUtils.java](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvY29tbW9uL0FjbFV0aWxzLmphdmE=) | `76.25% <0.00%> (-3.60%)` | :arrow_down: |
   | ... and [42 more](https://codecov.io/gh/apache/rocketmq/pull/3761/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6dff04f...b323e8e](https://codecov.io/gh/apache/rocketmq/pull/3761?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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



[GitHub] [rocketmq] sunxi92 commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
sunxi92 commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r806393539



##########
File path: srvutil/src/main/java/org/apache/rocketmq/srvutil/AclFileWatchService.java
##########
@@ -40,19 +40,24 @@
     private int aclFilesNum;
     @Deprecated
     private final Map<String, String> fileCurrentHash;
-    private final Map<String, Long> fileLastModifiedTime;
+    private Map<String, Long> fileLastModifiedTime;
     private List<String/**absolute pathname **/> fileList = new ArrayList<>();
     private final AclFileWatchService.Listener listener;
     private static final int WATCH_INTERVAL = 5000;
     private MessageDigest md = MessageDigest.getInstance("MD5");
+    private String defaultAclFile;
 
     public AclFileWatchService(String path, final AclFileWatchService.Listener listener) throws Exception {
         this.aclPath = path;
+        this.defaultAclFile = path.substring(0, path.length() - 4) + System.getProperty("rocketmq.acl.plain.file", "conf/plain_acl.yml");

Review comment:
       done

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,145 @@
 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 + "conf";
+
+    //private String defaultAclFile = fileHome + File.separator
+      //  + System.getProperty("rocketmq.acl.dir", "/conf") + File.separator + "plain_acl.yml";
+    private String defaultAclFile = fileHome + File.separator + System.getProperty("rocketmq.acl.plain.file", "conf/plain_acl.yml");
+
+    private Map<String/** fileFullPath **/, Map<String/** AccessKey **/, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();
 
-    private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+    private Map<String/** AccessKey **/, String/** fileFullPath **/> accessKeyTable = new HashMap<>();
 
     private List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
 
     private RemoteAddressStrategyFactory remoteAddressStrategyFactory = new RemoteAddressStrategyFactory();
 
+    private Map<String/** fileFullPath **/, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+
     private boolean isWatchStart;
 
+    private Map<String/** fileFullPath **/, DataVersion> dataVersionMap = new HashMap<>();
+
+    @Deprecated
     private final DataVersion dataVersion = new DataVersion();
 
+    private List<String> fileList = new ArrayList<>();
+
     public PlainPermissionManager() {
         load();
         watch();
     }
 
+    public List<String> getAllAclFiles(String path) {
+        List<String>  allAclFileFullPath = new ArrayList<>();
+        File file = new File(path);
+        File[] files = file.listFiles();
+        for (int i = 0; i < files.length; i++) {
+            String fileName = files[i].getAbsolutePath();
+            File f = new File(fileName);
+            if (fileName.equals(fileHome + MixAll.ACL_CONF_TOOLS_FILE)) {
+                continue;
+            } else if (fileName.endsWith(".yml") || fileName.endsWith(".yaml")) {
+                allAclFileFullPath.add(fileName);
+            } else if (f.isDirectory()) {
+                allAclFileFullPath.addAll(getAllAclFiles(fileName));
+            }
+        }
+        return allAclFileFullPath;
+    }
+
     public void load() {
+        if (fileHome == null || fileHome.isEmpty()) {
+            return;
+        }
+
+        Map<String, Map<String, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();
+        Map<String, String> accessKeyTable = new HashMap<>();
+        List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
+        Map<String, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+        Map<String, DataVersion> dataVersionMap = new HashMap<>();
 
+        fileList = getAllAclFiles(defaultAclDir);
+        if (new File(defaultAclFile).exists() && !fileList.contains(defaultAclFile)) {
+            fileList.add(defaultAclFile);
+        }
+
+        for (int i = 0; i < fileList.size(); i++) {
+            JSONObject plainAclConfData = AclUtils.getYamlDataObject(fileList.get(i),
+                JSONObject.class);
+            if (plainAclConfData == null || plainAclConfData.isEmpty()) {
+                throw new AclException(String.format("%s file is not data", fileList.get(i)));
+            }
+            log.info("Broker plain acl conf data is : ", plainAclConfData.toString());
+
+            List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategyList = new ArrayList<>();
+            JSONArray globalWhiteRemoteAddressesList = plainAclConfData.getJSONArray("globalWhiteRemoteAddresses");
+            if (globalWhiteRemoteAddressesList != null && !globalWhiteRemoteAddressesList.isEmpty()) {
+                for (int j = 0; j < globalWhiteRemoteAddressesList.size(); j++) {
+                    globalWhiteRemoteAddressStrategyList.add(remoteAddressStrategyFactory.
+                        getRemoteAddressStrategy(globalWhiteRemoteAddressesList.getString(j)));
+                }
+            }
+            if (globalWhiteRemoteAddressStrategyList.size() > 0) {
+                globalWhiteRemoteAddressStrategyMap.put(fileList.get(i), globalWhiteRemoteAddressStrategyList);
+                globalWhiteRemoteAddressStrategy.addAll(globalWhiteRemoteAddressStrategyList);
+            }
+
+            JSONArray accounts = plainAclConfData.getJSONArray(AclConstants.CONFIG_ACCOUNTS);
+            Map<String, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+            if (accounts != null && !accounts.isEmpty()) {
+                List<PlainAccessConfig> plainAccessConfigList = accounts.toJavaList(PlainAccessConfig.class);
+                for (PlainAccessConfig plainAccessConfig : plainAccessConfigList) {
+                    PlainAccessResource plainAccessResource = buildPlainAccessResource(plainAccessConfig);
+                    //AccessKey can not be defined in multiple ACL files
+                    if (accessKeyTable.get(plainAccessResource.getAccessKey()) == null) {

Review comment:
       done

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,145 @@
 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 + "conf";
+
+    //private String defaultAclFile = fileHome + File.separator
+      //  + System.getProperty("rocketmq.acl.dir", "/conf") + File.separator + "plain_acl.yml";
+    private String defaultAclFile = fileHome + File.separator + System.getProperty("rocketmq.acl.plain.file", "conf/plain_acl.yml");
+
+    private Map<String/** fileFullPath **/, Map<String/** AccessKey **/, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();
 
-    private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+    private Map<String/** AccessKey **/, String/** fileFullPath **/> accessKeyTable = new HashMap<>();
 
     private List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
 
     private RemoteAddressStrategyFactory remoteAddressStrategyFactory = new RemoteAddressStrategyFactory();
 
+    private Map<String/** fileFullPath **/, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+
     private boolean isWatchStart;
 
+    private Map<String/** fileFullPath **/, DataVersion> dataVersionMap = new HashMap<>();
+
+    @Deprecated
     private final DataVersion dataVersion = new DataVersion();
 
+    private List<String> fileList = new ArrayList<>();
+
     public PlainPermissionManager() {
         load();
         watch();
     }
 
+    public List<String> getAllAclFiles(String path) {
+        List<String>  allAclFileFullPath = new ArrayList<>();
+        File file = new File(path);
+        File[] files = file.listFiles();

Review comment:
       done

##########
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";
-

Review comment:
       Default acl file is added.




-- 
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



[GitHub] [rocketmq] coveralls edited a comment on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1013279991


   
   [![Coverage Status](https://coveralls.io/builds/46568034/badge)](https://coveralls.io/builds/46568034)
   
   Coverage increased (+0.6%) to 51.449% when pulling **a5691ab0e6623e55c14c311d93fda2df8a8bd751 on sunxi92:acl_update** into **6dff04fcb6405fc53a90730cf36ed9a79335bef5 on apache:develop**.
   


-- 
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



[GitHub] [rocketmq] lizhiboo commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
lizhiboo commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r786686890



##########
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";
-

Review comment:
       remove old acl file directory incompatible with old version. Can old acl file and new acl dir coexist?

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,145 @@
 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 + "conf";
+
+    //private String defaultAclFile = fileHome + File.separator
+      //  + System.getProperty("rocketmq.acl.dir", "/conf") + File.separator + "plain_acl.yml";
+    private String defaultAclFile = fileHome + File.separator + System.getProperty("rocketmq.acl.plain.file", "conf/plain_acl.yml");
+
+    private Map<String/** fileFullPath **/, Map<String/** AccessKey **/, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();
 
-    private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+    private Map<String/** AccessKey **/, String/** fileFullPath **/> accessKeyTable = new HashMap<>();
 
     private List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
 
     private RemoteAddressStrategyFactory remoteAddressStrategyFactory = new RemoteAddressStrategyFactory();
 
+    private Map<String/** fileFullPath **/, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+
     private boolean isWatchStart;
 
+    private Map<String/** fileFullPath **/, DataVersion> dataVersionMap = new HashMap<>();
+
+    @Deprecated
     private final DataVersion dataVersion = new DataVersion();
 
+    private List<String> fileList = new ArrayList<>();
+
     public PlainPermissionManager() {
         load();
         watch();
     }
 
+    public List<String> getAllAclFiles(String path) {
+        List<String>  allAclFileFullPath = new ArrayList<>();
+        File file = new File(path);
+        File[] files = file.listFiles();
+        for (int i = 0; i < files.length; i++) {
+            String fileName = files[i].getAbsolutePath();
+            File f = new File(fileName);
+            if (fileName.equals(fileHome + MixAll.ACL_CONF_TOOLS_FILE)) {
+                continue;
+            } else if (fileName.endsWith(".yml") || fileName.endsWith(".yaml")) {
+                allAclFileFullPath.add(fileName);
+            } else if (f.isDirectory()) {
+                allAclFileFullPath.addAll(getAllAclFiles(fileName));
+            }
+        }
+        return allAclFileFullPath;
+    }
+
     public void load() {
+        if (fileHome == null || fileHome.isEmpty()) {
+            return;
+        }
+
+        Map<String, Map<String, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();
+        Map<String, String> accessKeyTable = new HashMap<>();
+        List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
+        Map<String, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+        Map<String, DataVersion> dataVersionMap = new HashMap<>();
 
+        fileList = getAllAclFiles(defaultAclDir);
+        if (new File(defaultAclFile).exists() && !fileList.contains(defaultAclFile)) {
+            fileList.add(defaultAclFile);
+        }
+
+        for (int i = 0; i < fileList.size(); i++) {
+            JSONObject plainAclConfData = AclUtils.getYamlDataObject(fileList.get(i),
+                JSONObject.class);
+            if (plainAclConfData == null || plainAclConfData.isEmpty()) {
+                throw new AclException(String.format("%s file is not data", fileList.get(i)));
+            }
+            log.info("Broker plain acl conf data is : ", plainAclConfData.toString());
+
+            List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategyList = new ArrayList<>();
+            JSONArray globalWhiteRemoteAddressesList = plainAclConfData.getJSONArray("globalWhiteRemoteAddresses");
+            if (globalWhiteRemoteAddressesList != null && !globalWhiteRemoteAddressesList.isEmpty()) {
+                for (int j = 0; j < globalWhiteRemoteAddressesList.size(); j++) {
+                    globalWhiteRemoteAddressStrategyList.add(remoteAddressStrategyFactory.
+                        getRemoteAddressStrategy(globalWhiteRemoteAddressesList.getString(j)));
+                }
+            }
+            if (globalWhiteRemoteAddressStrategyList.size() > 0) {
+                globalWhiteRemoteAddressStrategyMap.put(fileList.get(i), globalWhiteRemoteAddressStrategyList);
+                globalWhiteRemoteAddressStrategy.addAll(globalWhiteRemoteAddressStrategyList);
+            }
+
+            JSONArray accounts = plainAclConfData.getJSONArray(AclConstants.CONFIG_ACCOUNTS);
+            Map<String, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+            if (accounts != null && !accounts.isEmpty()) {
+                List<PlainAccessConfig> plainAccessConfigList = accounts.toJavaList(PlainAccessConfig.class);
+                for (PlainAccessConfig plainAccessConfig : plainAccessConfigList) {
+                    PlainAccessResource plainAccessResource = buildPlainAccessResource(plainAccessConfig);
+                    //AccessKey can not be defined in multiple ACL files
+                    if (accessKeyTable.get(plainAccessResource.getAccessKey()) == null) {

Review comment:
       IMO, if accessKeyTable contains aplainAccessResource.getAccessKey(), print warn log will be better.

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,145 @@
 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 + "conf";
+
+    //private String defaultAclFile = fileHome + File.separator
+      //  + System.getProperty("rocketmq.acl.dir", "/conf") + File.separator + "plain_acl.yml";
+    private String defaultAclFile = fileHome + File.separator + System.getProperty("rocketmq.acl.plain.file", "conf/plain_acl.yml");
+
+    private Map<String/** fileFullPath **/, Map<String/** AccessKey **/, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();
 
-    private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+    private Map<String/** AccessKey **/, String/** fileFullPath **/> accessKeyTable = new HashMap<>();
 
     private List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
 
     private RemoteAddressStrategyFactory remoteAddressStrategyFactory = new RemoteAddressStrategyFactory();
 
+    private Map<String/** fileFullPath **/, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+
     private boolean isWatchStart;
 
+    private Map<String/** fileFullPath **/, DataVersion> dataVersionMap = new HashMap<>();
+
+    @Deprecated
     private final DataVersion dataVersion = new DataVersion();
 
+    private List<String> fileList = new ArrayList<>();
+
     public PlainPermissionManager() {
         load();
         watch();
     }
 
+    public List<String> getAllAclFiles(String path) {
+        List<String>  allAclFileFullPath = new ArrayList<>();
+        File file = new File(path);
+        File[] files = file.listFiles();

Review comment:
       IMO, check file is directory will be better




-- 
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



[GitHub] [rocketmq] lizhiboo commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
lizhiboo commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r786686890



##########
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";
-

Review comment:
       remove old acl file directory incompatible with old version. Can old acl file and new acl dir coexist?

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,145 @@
 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 + "conf";
+
+    //private String defaultAclFile = fileHome + File.separator
+      //  + System.getProperty("rocketmq.acl.dir", "/conf") + File.separator + "plain_acl.yml";
+    private String defaultAclFile = fileHome + File.separator + System.getProperty("rocketmq.acl.plain.file", "conf/plain_acl.yml");
+
+    private Map<String/** fileFullPath **/, Map<String/** AccessKey **/, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();
 
-    private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+    private Map<String/** AccessKey **/, String/** fileFullPath **/> accessKeyTable = new HashMap<>();
 
     private List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
 
     private RemoteAddressStrategyFactory remoteAddressStrategyFactory = new RemoteAddressStrategyFactory();
 
+    private Map<String/** fileFullPath **/, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+
     private boolean isWatchStart;
 
+    private Map<String/** fileFullPath **/, DataVersion> dataVersionMap = new HashMap<>();
+
+    @Deprecated
     private final DataVersion dataVersion = new DataVersion();
 
+    private List<String> fileList = new ArrayList<>();
+
     public PlainPermissionManager() {
         load();
         watch();
     }
 
+    public List<String> getAllAclFiles(String path) {
+        List<String>  allAclFileFullPath = new ArrayList<>();
+        File file = new File(path);
+        File[] files = file.listFiles();
+        for (int i = 0; i < files.length; i++) {
+            String fileName = files[i].getAbsolutePath();
+            File f = new File(fileName);
+            if (fileName.equals(fileHome + MixAll.ACL_CONF_TOOLS_FILE)) {
+                continue;
+            } else if (fileName.endsWith(".yml") || fileName.endsWith(".yaml")) {
+                allAclFileFullPath.add(fileName);
+            } else if (f.isDirectory()) {
+                allAclFileFullPath.addAll(getAllAclFiles(fileName));
+            }
+        }
+        return allAclFileFullPath;
+    }
+
     public void load() {
+        if (fileHome == null || fileHome.isEmpty()) {
+            return;
+        }
+
+        Map<String, Map<String, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();
+        Map<String, String> accessKeyTable = new HashMap<>();
+        List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
+        Map<String, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+        Map<String, DataVersion> dataVersionMap = new HashMap<>();
 
+        fileList = getAllAclFiles(defaultAclDir);
+        if (new File(defaultAclFile).exists() && !fileList.contains(defaultAclFile)) {
+            fileList.add(defaultAclFile);
+        }
+
+        for (int i = 0; i < fileList.size(); i++) {
+            JSONObject plainAclConfData = AclUtils.getYamlDataObject(fileList.get(i),
+                JSONObject.class);
+            if (plainAclConfData == null || plainAclConfData.isEmpty()) {
+                throw new AclException(String.format("%s file is not data", fileList.get(i)));
+            }
+            log.info("Broker plain acl conf data is : ", plainAclConfData.toString());
+
+            List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategyList = new ArrayList<>();
+            JSONArray globalWhiteRemoteAddressesList = plainAclConfData.getJSONArray("globalWhiteRemoteAddresses");
+            if (globalWhiteRemoteAddressesList != null && !globalWhiteRemoteAddressesList.isEmpty()) {
+                for (int j = 0; j < globalWhiteRemoteAddressesList.size(); j++) {
+                    globalWhiteRemoteAddressStrategyList.add(remoteAddressStrategyFactory.
+                        getRemoteAddressStrategy(globalWhiteRemoteAddressesList.getString(j)));
+                }
+            }
+            if (globalWhiteRemoteAddressStrategyList.size() > 0) {
+                globalWhiteRemoteAddressStrategyMap.put(fileList.get(i), globalWhiteRemoteAddressStrategyList);
+                globalWhiteRemoteAddressStrategy.addAll(globalWhiteRemoteAddressStrategyList);
+            }
+
+            JSONArray accounts = plainAclConfData.getJSONArray(AclConstants.CONFIG_ACCOUNTS);
+            Map<String, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+            if (accounts != null && !accounts.isEmpty()) {
+                List<PlainAccessConfig> plainAccessConfigList = accounts.toJavaList(PlainAccessConfig.class);
+                for (PlainAccessConfig plainAccessConfig : plainAccessConfigList) {
+                    PlainAccessResource plainAccessResource = buildPlainAccessResource(plainAccessConfig);
+                    //AccessKey can not be defined in multiple ACL files
+                    if (accessKeyTable.get(plainAccessResource.getAccessKey()) == null) {

Review comment:
       IMO, if accessKeyTable contains aplainAccessResource.getAccessKey(), print warn log will be better.

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,145 @@
 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 + "conf";
+
+    //private String defaultAclFile = fileHome + File.separator
+      //  + System.getProperty("rocketmq.acl.dir", "/conf") + File.separator + "plain_acl.yml";
+    private String defaultAclFile = fileHome + File.separator + System.getProperty("rocketmq.acl.plain.file", "conf/plain_acl.yml");
+
+    private Map<String/** fileFullPath **/, Map<String/** AccessKey **/, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();
 
-    private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+    private Map<String/** AccessKey **/, String/** fileFullPath **/> accessKeyTable = new HashMap<>();
 
     private List<RemoteAddressStrategy> globalWhiteRemoteAddressStrategy = new ArrayList<>();
 
     private RemoteAddressStrategyFactory remoteAddressStrategyFactory = new RemoteAddressStrategyFactory();
 
+    private Map<String/** fileFullPath **/, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+
     private boolean isWatchStart;
 
+    private Map<String/** fileFullPath **/, DataVersion> dataVersionMap = new HashMap<>();
+
+    @Deprecated
     private final DataVersion dataVersion = new DataVersion();
 
+    private List<String> fileList = new ArrayList<>();
+
     public PlainPermissionManager() {
         load();
         watch();
     }
 
+    public List<String> getAllAclFiles(String path) {
+        List<String>  allAclFileFullPath = new ArrayList<>();
+        File file = new File(path);
+        File[] files = file.listFiles();

Review comment:
       IMO, check file is directory will be better




-- 
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



[GitHub] [rocketmq] sunxi92 commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
sunxi92 commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r790222748



##########
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:
       done




-- 
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



[GitHub] [rocketmq] sunxi92 commented on a change in pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
sunxi92 commented on a change in pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#discussion_r794980224



##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,108 @@
 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");
+
+    private String defaultAclFile = fileHome + File.separator
+        + System.getProperty("rocketmq.acl.dir", "/conf") + 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 Map<String/** aclFileName **/, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+
     private boolean isWatchStart;
 
+    private Map<String/** aclFileName **/, DataVersion> dataVersionMap = new HashMap<>();
+
+    @Deprecated
     private final DataVersion dataVersion = new DataVersion();
 
+    private List<String> fileList = new ArrayList<>();
+
     public PlainPermissionManager() {
         load();
         watch();
     }
 
+    public void getAllAclFiles(String path) {
+        File file = new File(path);
+        File[] files = file.listFiles();
+        for (int i = 0; i < files.length; i++) {
+            String fileName = files[i].getAbsolutePath();
+            File f = new File(fileName);
+            if (fileName.endsWith(".yml")) {
+                fileList.add(fileName);
+            } else if (!f.isFile()) {
+                getAllAclFiles(fileName);
+            }
+        }
+    }
+
     public void load() {
+        if (fileHome == null || fileHome.isEmpty()) {
+            //throw new AclException(String.format("%s file is empty", fileHome));
+            return;
+        }
+        if (fileList.size() > 0) {
+            fileList.clear();
+        }
+        getAllAclFiles(defaultAclDir);
+        if (fileList == null || fileList.size() == 0)
+            return;
+
+        if (aclPlainAccessResourceMap.size() != 0 && accessKeyTable.size() != 0) {
+            aclPlainAccessResourceMap.clear();
+            accessKeyTable.clear();
+        }
+        if (globalWhiteRemoteAddressStrategy.size() != 0) {
+            globalWhiteRemoteAddressStrategy.clear();
+        }
+        if (globalWhiteRemoteAddressStrategyMap.size() != 0) {
+            globalWhiteRemoteAddressStrategyMap.clear();
+        }
+        for (int i = 0; i < fileList.size(); i++) {
+            load(fileList.get(i));
+        }
 
+        if (dataVersionMap.size() != fileList.size()) {
+            Iterator<Map.Entry<String, DataVersion>> it = dataVersionMap.entrySet().iterator();
+            while (it.hasNext()) {
+                Map.Entry<String, DataVersion> entry = it.next();
+                if (!fileList.contains(entry.getKey()))
+                    it.remove();
+            }
+        }
+        if (dataVersionMap.size() == 1) {
+            for (Map.Entry<String, DataVersion> entry : dataVersionMap.entrySet()) {
+                dataVersion.assignNewOne(entry.getValue());
+            }
+        }

Review comment:
       done.

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -85,43 +155,76 @@ public void load() {
             }
         }
 
+        if (this.globalWhiteRemoteAddressStrategyMap.get(aclFilePath) != null) {
+            List<RemoteAddressStrategy> remoteAddressStrategyList = this.globalWhiteRemoteAddressStrategyMap.get(aclFilePath);
+            for (int i = 0; i < remoteAddressStrategyList.size(); i++) {
+                this.globalWhiteRemoteAddressStrategy.remove(remoteAddressStrategyList.get(i));
+            }
+            this.globalWhiteRemoteAddressStrategyMap.remove(aclFilePath);

Review comment:
       done.

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,108 @@
 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");
+
+    private String defaultAclFile = fileHome + File.separator
+        + System.getProperty("rocketmq.acl.dir", "/conf") + 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 Map<String/** aclFileName **/, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+
     private boolean isWatchStart;
 
+    private Map<String/** aclFileName **/, DataVersion> dataVersionMap = new HashMap<>();
+
+    @Deprecated
     private final DataVersion dataVersion = new DataVersion();
 
+    private List<String> fileList = new ArrayList<>();
+
     public PlainPermissionManager() {
         load();
         watch();
     }
 
+    public void getAllAclFiles(String path) {
+        File file = new File(path);
+        File[] files = file.listFiles();
+        for (int i = 0; i < files.length; i++) {
+            String fileName = files[i].getAbsolutePath();
+            File f = new File(fileName);
+            if (fileName.endsWith(".yml")) {

Review comment:
       done.

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,108 @@
 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");
+
+    private String defaultAclFile = fileHome + File.separator
+        + System.getProperty("rocketmq.acl.dir", "/conf") + 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 Map<String/** aclFileName **/, List<RemoteAddressStrategy>> globalWhiteRemoteAddressStrategyMap = new HashMap<>();
+
     private boolean isWatchStart;
 
+    private Map<String/** aclFileName **/, DataVersion> dataVersionMap = new HashMap<>();
+
+    @Deprecated
     private final DataVersion dataVersion = new DataVersion();
 
+    private List<String> fileList = new ArrayList<>();
+
     public PlainPermissionManager() {
         load();
         watch();
     }
 
+    public void getAllAclFiles(String path) {
+        File file = new File(path);
+        File[] files = file.listFiles();
+        for (int i = 0; i < files.length; i++) {
+            String fileName = files[i].getAbsolutePath();
+            File f = new File(fileName);
+            if (fileName.endsWith(".yml")) {
+                fileList.add(fileName);
+            } else if (!f.isFile()) {

Review comment:
       done.

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
##########
@@ -30,51 +43,108 @@
 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");
+
+    private String defaultAclFile = fileHome + File.separator
+        + System.getProperty("rocketmq.acl.dir", "/conf") + File.separator + "plain_acl.yml";
 
-    private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
+    private Map<String/** aclFileName **/, Map<String/** AccessKey **/, PlainAccessResource>> aclPlainAccessResourceMap = new HashMap<>();

Review comment:
       done.

##########
File path: acl/src/main/java/org/apache/rocketmq/acl/AccessValidator.java
##########
@@ -60,17 +63,27 @@
      *
      * @return
      */
+    @Deprecated
     String getAclConfigVersion();

Review comment:
       done.




-- 
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



[GitHub] [rocketmq] coveralls edited a comment on pull request #3761: [ISSUE #2986] Support for multiple ACL files in a fixed directory

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3761:
URL: https://github.com/apache/rocketmq/pull/3761#issuecomment-1013279991


   
   [![Coverage Status](https://coveralls.io/builds/46030228/badge)](https://coveralls.io/builds/46030228)
   
   Coverage increased (+0.4%) to 51.3% when pulling **6248b47f03402c03934cca4a195a0c88a167ea37 on sunxi92:acl_update** into **6dff04fcb6405fc53a90730cf36ed9a79335bef5 on apache:develop**.
   


-- 
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