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/08/03 15:50:42 UTC
[GitHub] [rocketmq] sunxi92 opened a new pull request, #4769: [ISSUE #4688]acl module support namespace.
sunxi92 opened a new pull request, #4769:
URL: https://github.com/apache/rocketmq/pull/4769
#4688
--
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 pull request #4769: [ISSUE #4688]acl module support namespace.
Posted by GitBox <gi...@apache.org>.
sunxi92 commented on PR #4769:
URL: https://github.com/apache/rocketmq/pull/4769#issuecomment-1272312410
> @sunxi92 Pls add detailed design document previously discussed.
The detailed design document link is https://shimo.im/docs/47kgJM5eE7inm8qV/
--
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
Re: [PR] [ISSUE #4688]acl module support namespace. [rocketmq]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4769:
URL: https://github.com/apache/rocketmq/pull/4769#issuecomment-1817683672
This PR is stale because it has been open for 365 days with no activity. It will be closed in 3 days if no further activity occurs. If you wish not to mark it as stale, please leave a comment in this PR.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] caigy commented on pull request #4769: [ISSUE #4688]acl module support namespace.
Posted by GitBox <gi...@apache.org>.
caigy commented on PR #4769:
URL: https://github.com/apache/rocketmq/pull/4769#issuecomment-1272192770
@sunxi92 Pls add detailed design document previously discussed.
--
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
Re: [PR] [ISSUE #4688]acl module support namespace. [rocketmq]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4769:
URL: https://github.com/apache/rocketmq/pull/4769#issuecomment-1823679203
This PR was closed because it has been inactive for 3 days since being marked as stale.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] francisoliverlee commented on pull request #4769: [ISSUE #4688]acl module support namespace.
Posted by GitBox <gi...@apache.org>.
francisoliverlee commented on PR #4769:
URL: https://github.com/apache/rocketmq/pull/4769#issuecomment-1231086582
a README.md to show how-to-use would 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] codecov-commenter commented on pull request #4769: [ISSUE #4688]acl module support namespace.
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #4769:
URL: https://github.com/apache/rocketmq/pull/4769#issuecomment-1204644710
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/4769?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 [#4769](https://codecov.io/gh/apache/rocketmq/pull/4769?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (80332da) into [develop](https://codecov.io/gh/apache/rocketmq/commit/8504abfafcf763be6fc593183edb1a2f8f061e51?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8504abf) will **increase** coverage by `0.26%`.
> The diff coverage is `27.27%`.
```diff
@@ Coverage Diff @@
## develop #4769 +/- ##
=============================================
+ Coverage 44.68% 44.95% +0.26%
- Complexity 7540 7596 +56
=============================================
Files 977 977
Lines 67594 67834 +240
Branches 8927 8979 +52
=============================================
+ Hits 30207 30494 +287
+ Misses 33669 33550 -119
- Partials 3718 3790 +72
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/4769?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/4769/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=) | `26.56% <0.00%> (+0.12%)` | :arrow_up: |
| [...ools/command/acl/UpdateAccessConfigSubCommand.java](https://codecov.io/gh/apache/rocketmq/pull/4769/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-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2NvbW1hbmQvYWNsL1VwZGF0ZUFjY2Vzc0NvbmZpZ1N1YkNvbW1hbmQuamF2YQ==) | `38.54% <23.07%> (-1.00%)` | :arrow_down: |
| [...apache/rocketmq/acl/plain/PlainAccessResource.java](https://codecov.io/gh/apache/rocketmq/pull/4769/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-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvcGxhaW4vUGxhaW5BY2Nlc3NSZXNvdXJjZS5qYXZh) | `50.85% <27.27%> (-6.80%)` | :arrow_down: |
| [...ools/command/acl/DeleteAccessConfigSubCommand.java](https://codecov.io/gh/apache/rocketmq/pull/4769/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-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2NvbW1hbmQvYWNsL0RlbGV0ZUFjY2Vzc0NvbmZpZ1N1YkNvbW1hbmQuamF2YQ==) | `33.33% <37.50%> (+1.62%)` | :arrow_up: |
| [...rg/apache/rocketmq/common/stats/StatsSnapshot.java](https://codecov.io/gh/apache/rocketmq/pull/4769/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vc3RhdHMvU3RhdHNTbmFwc2hvdC5qYXZh) | `84.61% <0.00%> (-15.39%)` | :arrow_down: |
| [...apache/rocketmq/broker/longpolling/PopRequest.java](https://codecov.io/gh/apache/rocketmq/pull/4769/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvbG9uZ3BvbGxpbmcvUG9wUmVxdWVzdC5qYXZh) | `31.03% <0.00%> (-13.80%)` | :arrow_down: |
| [.../apache/rocketmq/common/stats/MomentStatsItem.java](https://codecov.io/gh/apache/rocketmq/pull/4769/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vc3RhdHMvTW9tZW50U3RhdHNJdGVtLmphdmE=) | `38.09% <0.00%> (-9.53%)` | :arrow_down: |
| [...q/namesrv/routeinfo/BrokerHousekeepingService.java](https://codecov.io/gh/apache/rocketmq/pull/4769/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-bmFtZXNydi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvbmFtZXNydi9yb3V0ZWluZm8vQnJva2VySG91c2VrZWVwaW5nU2VydmljZS5qYXZh) | `72.72% <0.00%> (-9.10%)` | :arrow_down: |
| [...ache/rocketmq/common/stats/MomentStatsItemSet.java](https://codecov.io/gh/apache/rocketmq/pull/4769/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vc3RhdHMvTW9tZW50U3RhdHNJdGVtU2V0LmphdmE=) | `39.13% <0.00%> (-8.70%)` | :arrow_down: |
| [...a/org/apache/rocketmq/logging/inner/SysLogger.java](https://codecov.io/gh/apache/rocketmq/pull/4769/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-bG9nZ2luZy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvbG9nZ2luZy9pbm5lci9TeXNMb2dnZXIuamF2YQ==) | `28.57% <0.00%> (-5.72%)` | :arrow_down: |
| ... and [123 more](https://codecov.io/gh/apache/rocketmq/pull/4769/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) | |
:mega: Codecov can now indicate which changes are the most critical in Pull Requests. [Learn more](https://about.codecov.io/product/feature/runtime-insights/?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 a diff in pull request #4769: [ISSUE #4688]acl module support namespace.
Posted by GitBox <gi...@apache.org>.
caigy commented on code in PR #4769:
URL: https://github.com/apache/rocketmq/pull/4769#discussion_r997694705
##########
acl/src/main/java/org/apache/rocketmq/acl/common/Permission.java:
##########
@@ -111,4 +121,73 @@ public static void checkResourcePerms(List<String> resources) {
public static boolean needAdminPerm(Integer code) {
return ADMIN_CODE.contains(code);
}
+
+ public static void parseResourcePermsAndNamespacePerms(PlainAccessResource plainAccessResource, PlainAccessConfig plainAccessConfig) {
+ List<ResourceAndPerm> resourcePerms = plainAccessConfig.getResourcePerms();
+ if (resourcePerms != null && !resourcePerms.isEmpty()) {
+ for (ResourceAndPerm resource : resourcePerms) {
+ ResourceType type = resource.getType();
+ String namespace = resource.getNamespace();
+ String perm = resource.getPerm();
+ String resourceName = namespace == null ? resource.getResource() : namespace + NAMESPACE_SEPARATOR + resource.getResource();
Review Comment:
Wrapping namespace by `NamespaceUtil` is more appropriate.
##########
acl/src/main/java/org/apache/rocketmq/acl/common/Permission.java:
##########
@@ -111,4 +121,73 @@ public static void checkResourcePerms(List<String> resources) {
public static boolean needAdminPerm(Integer code) {
return ADMIN_CODE.contains(code);
}
+
+ public static void parseResourcePermsAndNamespacePerms(PlainAccessResource plainAccessResource, PlainAccessConfig plainAccessConfig) {
+ List<ResourceAndPerm> resourcePerms = plainAccessConfig.getResourcePerms();
+ if (resourcePerms != null && !resourcePerms.isEmpty()) {
+ for (ResourceAndPerm resource : resourcePerms) {
+ ResourceType type = resource.getType();
+ String namespace = resource.getNamespace();
+ String perm = resource.getPerm();
+ String resourceName = namespace == null ? resource.getResource() : namespace + NAMESPACE_SEPARATOR + resource.getResource();
+ if (type == ResourceType.GROUP) {
+ resourceName = PlainAccessResource.getRetryTopic(resourceName);
+ }
+ plainAccessResource.addResourceAndPerm(resourceName, Permission.parsePermFromString(perm));
+ }
+ }
+ List<NamespaceAndPerm> namespacePerms = plainAccessConfig.getNamespacePerms();
+ if (namespacePerms != null && !namespacePerms.isEmpty()) {
+ Map<String, Map<String, Byte>> namespacePermMap = new HashMap<>();
+ for (NamespaceAndPerm namespace : namespacePerms) {
+ String namespaceName = namespace.getNamespace();
+ String topicPerm = namespace.getTopicPerm();
+ String groupPerm = namespace.getGroupPerm();
+ Map<String, Byte> permMap = Maps.newHashMapWithExpectedSize(2);
+ if (topicPerm != null && !topicPerm.isEmpty()) {
+ permMap.put(AclConstants.CONFIG_TOPIC_PERM, Permission.parsePermFromString(topicPerm));
+ }
+ if (groupPerm != null && !groupPerm.isEmpty()) {
+ permMap.put(AclConstants.CONFIG_GROUP_PERM, Permission.parsePermFromString(groupPerm));
+ }
+ namespacePermMap.put(namespaceName, permMap);
+ }
+ plainAccessResource.setNamespacePermMap(namespacePermMap);
+ }
+ }
+
+ public static void parseResourcePermsAndNamespacePerms(PlainAccessResource plainAccessResource, Map<String, Object> accountMap) {
Review Comment:
Is it almost the same as `parseResourcePermsAndNamespacePerms(org.apache.rocketmq.acl.plain.PlainAccessResource, org.apache.rocketmq.common.PlainAccessConfig)`? If so, just build `PlainAccessConfig` from `Map` and call it.
--
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 diff in pull request #4769: [ISSUE #4688]acl module support namespace.
Posted by GitBox <gi...@apache.org>.
caigy commented on code in PR #4769:
URL: https://github.com/apache/rocketmq/pull/4769#discussion_r990716859
##########
acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java:
##########
@@ -393,6 +399,401 @@ public boolean updateAccessConfig(PlainAccessConfig plainAccessConfig) {
}
}
+ public boolean updateAclAccount(PlainAccessConfig plainAccessConfig) {
Review Comment:
Add `@Overrride`
##########
acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java:
##########
@@ -393,6 +399,401 @@ public boolean updateAccessConfig(PlainAccessConfig plainAccessConfig) {
}
}
+ public boolean updateAclAccount(PlainAccessConfig plainAccessConfig) {
+ if (plainAccessConfig == null) {
+ log.error("Parameter value plainAccessConfig is null,Please check your parameter");
+ throw new AclException("Parameter value plainAccessConfig is null, Please check your parameter");
+ }
+ checkPlainAccessConfig(plainAccessConfig);
+
+ //Permission.checkResourcePerms(plainAccessConfig.getTopicPerms());
+ //Permission.checkResourcePerms(plainAccessConfig.getGroupPerms());
+ if (accessKeyTable.containsKey(plainAccessConfig.getAccessKey())) {
+ Map<String, Object> updateAccountMap = null;
+ String aclFileName = accessKeyTable.get(plainAccessConfig.getAccessKey());
+ Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(aclFileName, Map.class);
+ List<Map<String, Object>> accounts = (List<Map<String, Object>>) aclAccessConfigMap.get(AclConstants.CONFIG_ACCOUNTS);
+ if (null != accounts) {
+ for (Map<String, Object> account : accounts) {
+ if (account.get(AclConstants.CONFIG_ACCESS_KEY).equals(plainAccessConfig.getAccessKey())) {
+ // Update acl access config elements
+ accounts.remove(account);
+ updateAccountMap = createAclAccessConfigMap(account, plainAccessConfig);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ break;
+ }
+ }
+ } else {
+ // Maybe deleted in file, add it back
+ accounts = new LinkedList<>();
+ updateAccountMap = createAclAccessConfigMap(null, plainAccessConfig);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ }
+ Map<String, PlainAccessResource> accountMap = aclPlainAccessResourceMap.get(aclFileName);
+ PlainAccessResource updatePlainAccessResource = updatePlainAccessResource(updateAccountMap);
+ if (accountMap == null) {
+ accountMap = new HashMap<String, PlainAccessResource>(1);
+ accountMap.put(plainAccessConfig.getAccessKey(), updatePlainAccessResource);
+ } else if (accountMap.size() == 0) {
+ accountMap.put(plainAccessConfig.getAccessKey(), updatePlainAccessResource);
+ } else {
+ for (Map.Entry<String, PlainAccessResource> entry : accountMap.entrySet()) {
+ if (entry.getValue().getAccessKey().equals(plainAccessConfig.getAccessKey())) {
+ accountMap.put(entry.getKey(), updatePlainAccessResource);
+ break;
+ }
+ }
+ }
+ aclPlainAccessResourceMap.put(aclFileName, accountMap);
+ return AclUtils.writeDataObject(aclFileName, updateAclConfigFileVersion(aclFileName, aclAccessConfigMap));
+ } else {
+ String fileName = MixAll.dealFilePath(defaultAclFile);
+ //Create acl access config elements on the default acl file
+ if (aclPlainAccessResourceMap.get(defaultAclFile) == null || aclPlainAccessResourceMap.get(defaultAclFile).size() == 0) {
+ try {
+ File defaultAclFile = new File(fileName);
+ if (!defaultAclFile.exists()) {
+ defaultAclFile.createNewFile();
+ }
+ } catch (IOException e) {
+ log.warn("create default acl file has exception when update accessConfig. ", e);
+ }
+ }
+ Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(defaultAclFile, Map.class);
+ if (aclAccessConfigMap == null) {
+ aclAccessConfigMap = new HashMap<>();
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, new ArrayList<>());
+ }
+ List<Map<String, Object>> accounts = (List<Map<String, Object>>) aclAccessConfigMap.get(AclConstants.CONFIG_ACCOUNTS);
+ // When no accounts defined
+ if (null == accounts) {
+ accounts = new ArrayList<>();
+ }
+ accounts.add(createAclAccessConfigMap(null, plainAccessConfig));
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ accessKeyTable.put(plainAccessConfig.getAccessKey(), fileName);
+ if (aclPlainAccessResourceMap.get(fileName) == null) {
+ Map<String, PlainAccessResource> plainAccessResourceMap = new HashMap<>(1);
+ plainAccessResourceMap.put(plainAccessConfig.getAccessKey(), buildPlainAccessResource(plainAccessConfig));
+ aclPlainAccessResourceMap.put(fileName, plainAccessResourceMap);
+ } else {
+ Map<String, PlainAccessResource> plainAccessResourceMap = aclPlainAccessResourceMap.get(fileName);
+ plainAccessResourceMap.put(plainAccessConfig.getAccessKey(), buildPlainAccessResource(plainAccessConfig));
+ aclPlainAccessResourceMap.put(fileName, plainAccessResourceMap);
+ }
+ return AclUtils.writeDataObject(defaultAclFile, updateAclConfigFileVersion(defaultAclFile, aclAccessConfigMap));
+ }
+ }
+
+ public PlainAccessResource updatePlainAccessResource(Map<String, Object> updateAccountMap) {
+ PlainAccessResource updatePlainAccessResource = new PlainAccessResource();
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_ACCESS_KEY)) {
+ updatePlainAccessResource.setAccessKey(updateAccountMap.get(AclConstants.CONFIG_ACCESS_KEY).toString());
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_SECRET_KEY)) {
+ updatePlainAccessResource.setSecretKey(updateAccountMap.get(AclConstants.CONFIG_SECRET_KEY).toString());
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_WHITE_ADDR)) {
+ updatePlainAccessResource.setRemoteAddressStrategy(remoteAddressStrategyFactory.
+ getRemoteAddressStrategy(updateAccountMap.get(AclConstants.CONFIG_WHITE_ADDR).toString()));
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_ADMIN_ROLE)) {
+ updatePlainAccessResource.setAdmin(Boolean.parseBoolean(updateAccountMap.get(AclConstants.CONFIG_ADMIN_ROLE).toString()));
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_DEFAULT_TOPIC_PERM)) {
+ updatePlainAccessResource.setDefaultTopicPerm(Permission.
+ parsePermFromString(updateAccountMap.get(AclConstants.CONFIG_DEFAULT_TOPIC_PERM).toString()));
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_DEFAULT_GROUP_PERM)) {
+ updatePlainAccessResource.setDefaultGroupPerm(Permission.
+ parsePermFromString(updateAccountMap.get(AclConstants.CONFIG_DEFAULT_GROUP_PERM).toString()));
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_RESOURCE_PERMS)) {
+ List<LinkedHashMap<String, Object>> resourceAndPermList = (List<LinkedHashMap<String, Object>>)updateAccountMap.get(AclConstants.CONFIG_RESOURCE_PERMS);
+ for (LinkedHashMap resourceAndPerm : resourceAndPermList) {
+ String resource = resourceAndPerm.get("resource").toString();
+ String namespace = resourceAndPerm.get("namespace").toString();
+ String type = resourceAndPerm.get("type").toString();
+ String perm = resourceAndPerm.get("perm").toString();
+ if (namespace == null || namespace.isEmpty()) {
+ if (type.equals(ResourceType.GROUP.toString())) {
Review Comment:
Use `java.lang.Enum.name()` instead of `toString()`
##########
acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java:
##########
@@ -393,6 +399,401 @@ public boolean updateAccessConfig(PlainAccessConfig plainAccessConfig) {
}
}
+ public boolean updateAclAccount(PlainAccessConfig plainAccessConfig) {
+ if (plainAccessConfig == null) {
+ log.error("Parameter value plainAccessConfig is null,Please check your parameter");
+ throw new AclException("Parameter value plainAccessConfig is null, Please check your parameter");
+ }
+ checkPlainAccessConfig(plainAccessConfig);
+
+ //Permission.checkResourcePerms(plainAccessConfig.getTopicPerms());
+ //Permission.checkResourcePerms(plainAccessConfig.getGroupPerms());
+ if (accessKeyTable.containsKey(plainAccessConfig.getAccessKey())) {
+ Map<String, Object> updateAccountMap = null;
+ String aclFileName = accessKeyTable.get(plainAccessConfig.getAccessKey());
+ Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(aclFileName, Map.class);
+ List<Map<String, Object>> accounts = (List<Map<String, Object>>) aclAccessConfigMap.get(AclConstants.CONFIG_ACCOUNTS);
+ if (null != accounts) {
+ for (Map<String, Object> account : accounts) {
+ if (account.get(AclConstants.CONFIG_ACCESS_KEY).equals(plainAccessConfig.getAccessKey())) {
+ // Update acl access config elements
+ accounts.remove(account);
+ updateAccountMap = createAclAccessConfigMap(account, plainAccessConfig);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ break;
+ }
+ }
+ } else {
+ // Maybe deleted in file, add it back
+ accounts = new LinkedList<>();
+ updateAccountMap = createAclAccessConfigMap(null, plainAccessConfig);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ }
+ Map<String, PlainAccessResource> accountMap = aclPlainAccessResourceMap.get(aclFileName);
+ PlainAccessResource updatePlainAccessResource = updatePlainAccessResource(updateAccountMap);
+ if (accountMap == null) {
+ accountMap = new HashMap<String, PlainAccessResource>(1);
Review Comment:
Use `Maps.newHashMapWithExpectedSize()` to prevent resizing.
##########
acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java:
##########
@@ -45,13 +45,19 @@
import org.apache.rocketmq.common.AclConfig;
import org.apache.rocketmq.common.DataVersion;
import org.apache.rocketmq.common.MixAll;
+import org.apache.rocketmq.common.NamespaceAndPerm;
+import org.apache.rocketmq.common.OperationType;
import org.apache.rocketmq.common.PlainAccessConfig;
+import org.apache.rocketmq.common.ResourceAndPerm;
+import org.apache.rocketmq.common.ResourceType;
import org.apache.rocketmq.common.constant.LoggerName;
import org.apache.rocketmq.common.topic.TopicValidator;
import org.apache.rocketmq.logging.InternalLogger;
import org.apache.rocketmq.logging.InternalLoggerFactory;
import org.apache.rocketmq.srvutil.AclFileWatchService;
+import static org.apache.rocketmq.common.protocol.NamespaceUtil.NAMESPACE_SEPARATOR;
+
public class PlainPermissionManager {
Review Comment:
Methods changed in this class are a bit complicated, you'd better do some refactoring work, like extracting common parts, reducing indentation, etc.
##########
acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java:
##########
@@ -393,6 +399,401 @@ public boolean updateAccessConfig(PlainAccessConfig plainAccessConfig) {
}
}
+ public boolean updateAclAccount(PlainAccessConfig plainAccessConfig) {
+ if (plainAccessConfig == null) {
+ log.error("Parameter value plainAccessConfig is null,Please check your parameter");
+ throw new AclException("Parameter value plainAccessConfig is null, Please check your parameter");
+ }
+ checkPlainAccessConfig(plainAccessConfig);
+
+ //Permission.checkResourcePerms(plainAccessConfig.getTopicPerms());
+ //Permission.checkResourcePerms(plainAccessConfig.getGroupPerms());
+ if (accessKeyTable.containsKey(plainAccessConfig.getAccessKey())) {
+ Map<String, Object> updateAccountMap = null;
+ String aclFileName = accessKeyTable.get(plainAccessConfig.getAccessKey());
+ Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(aclFileName, Map.class);
+ List<Map<String, Object>> accounts = (List<Map<String, Object>>) aclAccessConfigMap.get(AclConstants.CONFIG_ACCOUNTS);
+ if (null != accounts) {
+ for (Map<String, Object> account : accounts) {
+ if (account.get(AclConstants.CONFIG_ACCESS_KEY).equals(plainAccessConfig.getAccessKey())) {
+ // Update acl access config elements
+ accounts.remove(account);
+ updateAccountMap = createAclAccessConfigMap(account, plainAccessConfig);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ break;
+ }
+ }
+ } else {
+ // Maybe deleted in file, add it back
+ accounts = new LinkedList<>();
+ updateAccountMap = createAclAccessConfigMap(null, plainAccessConfig);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ }
+ Map<String, PlainAccessResource> accountMap = aclPlainAccessResourceMap.get(aclFileName);
+ PlainAccessResource updatePlainAccessResource = updatePlainAccessResource(updateAccountMap);
+ if (accountMap == null) {
+ accountMap = new HashMap<String, PlainAccessResource>(1);
+ accountMap.put(plainAccessConfig.getAccessKey(), updatePlainAccessResource);
+ } else if (accountMap.size() == 0) {
+ accountMap.put(plainAccessConfig.getAccessKey(), updatePlainAccessResource);
+ } else {
+ for (Map.Entry<String, PlainAccessResource> entry : accountMap.entrySet()) {
+ if (entry.getValue().getAccessKey().equals(plainAccessConfig.getAccessKey())) {
+ accountMap.put(entry.getKey(), updatePlainAccessResource);
+ break;
+ }
+ }
+ }
+ aclPlainAccessResourceMap.put(aclFileName, accountMap);
+ return AclUtils.writeDataObject(aclFileName, updateAclConfigFileVersion(aclFileName, aclAccessConfigMap));
+ } else {
+ String fileName = MixAll.dealFilePath(defaultAclFile);
+ //Create acl access config elements on the default acl file
+ if (aclPlainAccessResourceMap.get(defaultAclFile) == null || aclPlainAccessResourceMap.get(defaultAclFile).size() == 0) {
+ try {
+ File defaultAclFile = new File(fileName);
+ if (!defaultAclFile.exists()) {
+ defaultAclFile.createNewFile();
+ }
+ } catch (IOException e) {
+ log.warn("create default acl file has exception when update accessConfig. ", e);
+ }
+ }
+ Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(defaultAclFile, Map.class);
+ if (aclAccessConfigMap == null) {
+ aclAccessConfigMap = new HashMap<>();
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, new ArrayList<>());
+ }
+ List<Map<String, Object>> accounts = (List<Map<String, Object>>) aclAccessConfigMap.get(AclConstants.CONFIG_ACCOUNTS);
+ // When no accounts defined
+ if (null == accounts) {
+ accounts = new ArrayList<>();
+ }
+ accounts.add(createAclAccessConfigMap(null, plainAccessConfig));
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ accessKeyTable.put(plainAccessConfig.getAccessKey(), fileName);
+ if (aclPlainAccessResourceMap.get(fileName) == null) {
+ Map<String, PlainAccessResource> plainAccessResourceMap = new HashMap<>(1);
+ plainAccessResourceMap.put(plainAccessConfig.getAccessKey(), buildPlainAccessResource(plainAccessConfig));
+ aclPlainAccessResourceMap.put(fileName, plainAccessResourceMap);
+ } else {
+ Map<String, PlainAccessResource> plainAccessResourceMap = aclPlainAccessResourceMap.get(fileName);
+ plainAccessResourceMap.put(plainAccessConfig.getAccessKey(), buildPlainAccessResource(plainAccessConfig));
+ aclPlainAccessResourceMap.put(fileName, plainAccessResourceMap);
+ }
+ return AclUtils.writeDataObject(defaultAclFile, updateAclConfigFileVersion(defaultAclFile, aclAccessConfigMap));
+ }
+ }
+
+ public PlainAccessResource updatePlainAccessResource(Map<String, Object> updateAccountMap) {
+ PlainAccessResource updatePlainAccessResource = new PlainAccessResource();
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_ACCESS_KEY)) {
+ updatePlainAccessResource.setAccessKey(updateAccountMap.get(AclConstants.CONFIG_ACCESS_KEY).toString());
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_SECRET_KEY)) {
+ updatePlainAccessResource.setSecretKey(updateAccountMap.get(AclConstants.CONFIG_SECRET_KEY).toString());
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_WHITE_ADDR)) {
+ updatePlainAccessResource.setRemoteAddressStrategy(remoteAddressStrategyFactory.
+ getRemoteAddressStrategy(updateAccountMap.get(AclConstants.CONFIG_WHITE_ADDR).toString()));
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_ADMIN_ROLE)) {
+ updatePlainAccessResource.setAdmin(Boolean.parseBoolean(updateAccountMap.get(AclConstants.CONFIG_ADMIN_ROLE).toString()));
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_DEFAULT_TOPIC_PERM)) {
+ updatePlainAccessResource.setDefaultTopicPerm(Permission.
+ parsePermFromString(updateAccountMap.get(AclConstants.CONFIG_DEFAULT_TOPIC_PERM).toString()));
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_DEFAULT_GROUP_PERM)) {
+ updatePlainAccessResource.setDefaultGroupPerm(Permission.
+ parsePermFromString(updateAccountMap.get(AclConstants.CONFIG_DEFAULT_GROUP_PERM).toString()));
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_RESOURCE_PERMS)) {
+ List<LinkedHashMap<String, Object>> resourceAndPermList = (List<LinkedHashMap<String, Object>>)updateAccountMap.get(AclConstants.CONFIG_RESOURCE_PERMS);
+ for (LinkedHashMap resourceAndPerm : resourceAndPermList) {
+ String resource = resourceAndPerm.get("resource").toString();
+ String namespace = resourceAndPerm.get("namespace").toString();
+ String type = resourceAndPerm.get("type").toString();
+ String perm = resourceAndPerm.get("perm").toString();
+ if (namespace == null || namespace.isEmpty()) {
+ if (type.equals(ResourceType.GROUP.toString())) {
+ updatePlainAccessResource.addResourceAndPerm(MixAll.getRetryTopic(resource), Permission.parsePermFromString(perm));
+ } else {
+ updatePlainAccessResource.addResourceAndPerm(resource, Permission.parsePermFromString(perm));
+ }
+ } else {
+ if (type.equals(ResourceType.GROUP.toString())) {
+ updatePlainAccessResource.addResourceAndPerm(
+ MixAll.RETRY_GROUP_TOPIC_PREFIX + namespace + NAMESPACE_SEPARATOR + resource, Permission.parsePermFromString(perm));
+ } else {
+ updatePlainAccessResource.addResourceAndPerm(namespace + resource, Permission.parsePermFromString(perm));
+ }
+ }
+
+ }
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_NAMESPACE_PERMS)) {
+ List<LinkedHashMap<String, String>> namespaceAndPermList = (List<LinkedHashMap<String, String>>)updateAccountMap.get(AclConstants.CONFIG_NAMESPACE_PERMS);
+ Map<String, Map<String, Byte>> namespacePermMap = new HashMap<>();
+ for (LinkedHashMap<String, String> namespaceAndPerm : namespaceAndPermList) {
+ String namespace = namespaceAndPerm.get("namespace");
+ String topicPerm = namespaceAndPerm.get("topicPerm");
+ String groupPerm = namespaceAndPerm.get("groupPerm");
+ Map<String, Byte> permMap = new HashMap<>(2);
+ if (topicPerm != null && !topicPerm.isEmpty()) {
+ permMap.put(AclConstants.CONFIG_TOPIC_PERM, Permission.parsePermFromString(topicPerm));
+ }
+ if (groupPerm != null && !groupPerm.isEmpty()) {
+ permMap.put(AclConstants.CONFIG_GROUP_PERM, Permission.parsePermFromString(groupPerm));
+ }
+ namespacePermMap.put(namespace, permMap);
+ }
+ updatePlainAccessResource.setNamespacePermMap(namespacePermMap);
+ }
+ return updatePlainAccessResource;
+ }
+
+ public boolean updateAclResourcePerms(String accesskey, ResourceAndPerm resourceAndPerm, OperationType operationType) {
+ if (accesskey == null || resourceAndPerm == null || operationType == null) {
+ log.error("Parameter: accesskey or resourceAndPerm or operationType is null, Please check your parameter");
+ throw new AclException("The parameter of updateAclResourcePerms command: accesskey or resourceAndPerm or operationType is null, Please check your parameter");
+ }
+ if (operationType == OperationType.ADD || operationType == OperationType.UPDATE) {
+ checkResourceAndPerm(resourceAndPerm);
+ }
+
+ if (accessKeyTable.containsKey(accesskey)) {
+ Map<String, Object> updateAccountMap = null;
+ String aclFileName = accessKeyTable.get(accesskey);
+ Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(aclFileName, Map.class);
+ List<Map<String, Object>> accounts = (List<Map<String, Object>>) aclAccessConfigMap.get(AclConstants.CONFIG_ACCOUNTS);
+ if (null != accounts) {
+ for (Map<String, Object> account : accounts) {
+ if (account.get(AclConstants.CONFIG_ACCESS_KEY).equals(accesskey)) {
+ // Update acl access config elements
+ accounts.remove(account);
+ updateAccountMap = updateAclAccessConfigMap(account, resourceAndPerm, operationType);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ break;
+ }
+ }
+ } else {
+ // Maybe deleted in file, add it back
+ accounts = new LinkedList<>();
+ updateAccountMap = updateAclAccessConfigMap(null, resourceAndPerm, operationType);
+ updateAccountMap.put(AclConstants.CONFIG_ACCESS_KEY, accesskey);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ }
+ Map<String, PlainAccessResource> accountMap = aclPlainAccessResourceMap.get(aclFileName);
+ PlainAccessResource updatePlainAccessResource = updatePlainAccessResource(updateAccountMap);
+ if (accountMap == null) {
+ accountMap = new HashMap<String, PlainAccessResource>(1);
+ accountMap.put(accesskey, updatePlainAccessResource);
+ } else if (accountMap.size() == 0) {
+ accountMap.put(accesskey, updatePlainAccessResource);
+ } else {
+ for (Map.Entry<String, PlainAccessResource> entry : accountMap.entrySet()) {
+ if (entry.getValue().getAccessKey().equals(accesskey)) {
+ accountMap.put(entry.getKey(), updatePlainAccessResource);
+ break;
+ }
+ }
+ }
+ aclPlainAccessResourceMap.put(aclFileName, accountMap);
+ return AclUtils.writeDataObject(aclFileName, updateAclConfigFileVersion(aclFileName, aclAccessConfigMap));
+ } else {
+ throw new AclException("The accesskey for update acl resource perms is not exist in the acl config.");
+ }
+
+ }
+
+ public Map<String, Object> updateAclAccessConfigMap(Map<String, Object> existedAccountMap,
+ ResourceAndPerm resourceAndPerm, OperationType operationType) {
+ Map<String, Object> newAccountsMap = null;
+ if (existedAccountMap == null) {
+ newAccountsMap = new LinkedHashMap<String, Object>();
+ } else {
+ newAccountsMap = existedAccountMap;
+ }
+
+ List<LinkedHashMap> resourceAndPerms = (List<LinkedHashMap>)newAccountsMap.get(AclConstants.CONFIG_RESOURCE_PERMS);
+ if (resourceAndPerms == null) {
+ resourceAndPerms = new ArrayList<>();
+ }
+
+ if (operationType == OperationType.ADD) {
+ LinkedHashMap<String, Object> addResourceAndPerm = new LinkedHashMap<String, Object>(4);
+ addResourceAndPerm.put("resource", resourceAndPerm.getResource());
+ addResourceAndPerm.put("type", resourceAndPerm.getType());
+ addResourceAndPerm.put("namespace", resourceAndPerm.getNamespace());
+ addResourceAndPerm.put("perm", resourceAndPerm.getPerm());
+ resourceAndPerms.add(addResourceAndPerm);
+ } else {
+ Iterator iterator = resourceAndPerms.iterator();
+ while (iterator.hasNext()) {
+ LinkedHashMap<String, Object> curResourceAndPerm = (LinkedHashMap)iterator.next();
+ if (curResourceAndPerm.get("resource").toString().equals(resourceAndPerm.getResource()) && curResourceAndPerm.get("type").toString().equals(resourceAndPerm.getType().toString())) {
Review Comment:
Is `toString()` necessary here?
##########
common/src/main/java/org/apache/rocketmq/common/PlainAccessConfig.java:
##########
@@ -110,6 +132,8 @@ public String toString() {
", defaultGroupPerm='" + defaultGroupPerm + '\'' +
", topicPerms=" + topicPerms +
", groupPerms=" + groupPerms +
+ ", resourcePerms=" + resourcePerms +
+ ", namespacePerms=" + namespacePerms +
Review Comment:
Implement `toString()` of `ResourceAndPerm` and `NamespaceAndPerm`.
##########
acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java:
##########
@@ -393,6 +399,401 @@ public boolean updateAccessConfig(PlainAccessConfig plainAccessConfig) {
}
}
+ public boolean updateAclAccount(PlainAccessConfig plainAccessConfig) {
+ if (plainAccessConfig == null) {
+ log.error("Parameter value plainAccessConfig is null,Please check your parameter");
+ throw new AclException("Parameter value plainAccessConfig is null, Please check your parameter");
+ }
+ checkPlainAccessConfig(plainAccessConfig);
+
+ //Permission.checkResourcePerms(plainAccessConfig.getTopicPerms());
+ //Permission.checkResourcePerms(plainAccessConfig.getGroupPerms());
+ if (accessKeyTable.containsKey(plainAccessConfig.getAccessKey())) {
+ Map<String, Object> updateAccountMap = null;
+ String aclFileName = accessKeyTable.get(plainAccessConfig.getAccessKey());
+ Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(aclFileName, Map.class);
+ List<Map<String, Object>> accounts = (List<Map<String, Object>>) aclAccessConfigMap.get(AclConstants.CONFIG_ACCOUNTS);
+ if (null != accounts) {
+ for (Map<String, Object> account : accounts) {
+ if (account.get(AclConstants.CONFIG_ACCESS_KEY).equals(plainAccessConfig.getAccessKey())) {
+ // Update acl access config elements
+ accounts.remove(account);
+ updateAccountMap = createAclAccessConfigMap(account, plainAccessConfig);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ break;
+ }
+ }
+ } else {
+ // Maybe deleted in file, add it back
+ accounts = new LinkedList<>();
+ updateAccountMap = createAclAccessConfigMap(null, plainAccessConfig);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ }
+ Map<String, PlainAccessResource> accountMap = aclPlainAccessResourceMap.get(aclFileName);
+ PlainAccessResource updatePlainAccessResource = updatePlainAccessResource(updateAccountMap);
+ if (accountMap == null) {
+ accountMap = new HashMap<String, PlainAccessResource>(1);
+ accountMap.put(plainAccessConfig.getAccessKey(), updatePlainAccessResource);
+ } else if (accountMap.size() == 0) {
+ accountMap.put(plainAccessConfig.getAccessKey(), updatePlainAccessResource);
+ } else {
+ for (Map.Entry<String, PlainAccessResource> entry : accountMap.entrySet()) {
+ if (entry.getValue().getAccessKey().equals(plainAccessConfig.getAccessKey())) {
+ accountMap.put(entry.getKey(), updatePlainAccessResource);
+ break;
+ }
+ }
+ }
+ aclPlainAccessResourceMap.put(aclFileName, accountMap);
+ return AclUtils.writeDataObject(aclFileName, updateAclConfigFileVersion(aclFileName, aclAccessConfigMap));
+ } else {
+ String fileName = MixAll.dealFilePath(defaultAclFile);
+ //Create acl access config elements on the default acl file
+ if (aclPlainAccessResourceMap.get(defaultAclFile) == null || aclPlainAccessResourceMap.get(defaultAclFile).size() == 0) {
+ try {
+ File defaultAclFile = new File(fileName);
+ if (!defaultAclFile.exists()) {
+ defaultAclFile.createNewFile();
+ }
+ } catch (IOException e) {
+ log.warn("create default acl file has exception when update accessConfig. ", e);
+ }
+ }
+ Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(defaultAclFile, Map.class);
+ if (aclAccessConfigMap == null) {
+ aclAccessConfigMap = new HashMap<>();
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, new ArrayList<>());
+ }
+ List<Map<String, Object>> accounts = (List<Map<String, Object>>) aclAccessConfigMap.get(AclConstants.CONFIG_ACCOUNTS);
+ // When no accounts defined
+ if (null == accounts) {
+ accounts = new ArrayList<>();
+ }
+ accounts.add(createAclAccessConfigMap(null, plainAccessConfig));
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ accessKeyTable.put(plainAccessConfig.getAccessKey(), fileName);
+ if (aclPlainAccessResourceMap.get(fileName) == null) {
+ Map<String, PlainAccessResource> plainAccessResourceMap = new HashMap<>(1);
+ plainAccessResourceMap.put(plainAccessConfig.getAccessKey(), buildPlainAccessResource(plainAccessConfig));
+ aclPlainAccessResourceMap.put(fileName, plainAccessResourceMap);
+ } else {
+ Map<String, PlainAccessResource> plainAccessResourceMap = aclPlainAccessResourceMap.get(fileName);
+ plainAccessResourceMap.put(plainAccessConfig.getAccessKey(), buildPlainAccessResource(plainAccessConfig));
+ aclPlainAccessResourceMap.put(fileName, plainAccessResourceMap);
+ }
+ return AclUtils.writeDataObject(defaultAclFile, updateAclConfigFileVersion(defaultAclFile, aclAccessConfigMap));
+ }
+ }
+
+ public PlainAccessResource updatePlainAccessResource(Map<String, Object> updateAccountMap) {
Review Comment:
In this method, `PlainAccessResource` is **created** but not **updated.** The signature of methods should be more descriptive.
How about modifying `org.apache.rocketmq.acl.plain.PlainPermissionManager#buildPlainAccessResource` to support new config format?
##########
acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java:
##########
@@ -393,6 +399,401 @@ public boolean updateAccessConfig(PlainAccessConfig plainAccessConfig) {
}
}
+ public boolean updateAclAccount(PlainAccessConfig plainAccessConfig) {
+ if (plainAccessConfig == null) {
+ log.error("Parameter value plainAccessConfig is null,Please check your parameter");
+ throw new AclException("Parameter value plainAccessConfig is null, Please check your parameter");
+ }
+ checkPlainAccessConfig(plainAccessConfig);
+
+ //Permission.checkResourcePerms(plainAccessConfig.getTopicPerms());
+ //Permission.checkResourcePerms(plainAccessConfig.getGroupPerms());
+ if (accessKeyTable.containsKey(plainAccessConfig.getAccessKey())) {
+ Map<String, Object> updateAccountMap = null;
+ String aclFileName = accessKeyTable.get(plainAccessConfig.getAccessKey());
+ Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(aclFileName, Map.class);
+ List<Map<String, Object>> accounts = (List<Map<String, Object>>) aclAccessConfigMap.get(AclConstants.CONFIG_ACCOUNTS);
+ if (null != accounts) {
+ for (Map<String, Object> account : accounts) {
+ if (account.get(AclConstants.CONFIG_ACCESS_KEY).equals(plainAccessConfig.getAccessKey())) {
+ // Update acl access config elements
+ accounts.remove(account);
+ updateAccountMap = createAclAccessConfigMap(account, plainAccessConfig);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ break;
+ }
+ }
+ } else {
+ // Maybe deleted in file, add it back
+ accounts = new LinkedList<>();
+ updateAccountMap = createAclAccessConfigMap(null, plainAccessConfig);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ }
+ Map<String, PlainAccessResource> accountMap = aclPlainAccessResourceMap.get(aclFileName);
+ PlainAccessResource updatePlainAccessResource = updatePlainAccessResource(updateAccountMap);
+ if (accountMap == null) {
+ accountMap = new HashMap<String, PlainAccessResource>(1);
+ accountMap.put(plainAccessConfig.getAccessKey(), updatePlainAccessResource);
+ } else if (accountMap.size() == 0) {
+ accountMap.put(plainAccessConfig.getAccessKey(), updatePlainAccessResource);
+ } else {
+ for (Map.Entry<String, PlainAccessResource> entry : accountMap.entrySet()) {
+ if (entry.getValue().getAccessKey().equals(plainAccessConfig.getAccessKey())) {
+ accountMap.put(entry.getKey(), updatePlainAccessResource);
+ break;
+ }
+ }
+ }
+ aclPlainAccessResourceMap.put(aclFileName, accountMap);
+ return AclUtils.writeDataObject(aclFileName, updateAclConfigFileVersion(aclFileName, aclAccessConfigMap));
+ } else {
+ String fileName = MixAll.dealFilePath(defaultAclFile);
+ //Create acl access config elements on the default acl file
+ if (aclPlainAccessResourceMap.get(defaultAclFile) == null || aclPlainAccessResourceMap.get(defaultAclFile).size() == 0) {
+ try {
+ File defaultAclFile = new File(fileName);
+ if (!defaultAclFile.exists()) {
+ defaultAclFile.createNewFile();
+ }
+ } catch (IOException e) {
+ log.warn("create default acl file has exception when update accessConfig. ", e);
+ }
+ }
+ Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(defaultAclFile, Map.class);
+ if (aclAccessConfigMap == null) {
+ aclAccessConfigMap = new HashMap<>();
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, new ArrayList<>());
+ }
+ List<Map<String, Object>> accounts = (List<Map<String, Object>>) aclAccessConfigMap.get(AclConstants.CONFIG_ACCOUNTS);
+ // When no accounts defined
+ if (null == accounts) {
+ accounts = new ArrayList<>();
+ }
+ accounts.add(createAclAccessConfigMap(null, plainAccessConfig));
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ accessKeyTable.put(plainAccessConfig.getAccessKey(), fileName);
+ if (aclPlainAccessResourceMap.get(fileName) == null) {
+ Map<String, PlainAccessResource> plainAccessResourceMap = new HashMap<>(1);
+ plainAccessResourceMap.put(plainAccessConfig.getAccessKey(), buildPlainAccessResource(plainAccessConfig));
+ aclPlainAccessResourceMap.put(fileName, plainAccessResourceMap);
+ } else {
+ Map<String, PlainAccessResource> plainAccessResourceMap = aclPlainAccessResourceMap.get(fileName);
+ plainAccessResourceMap.put(plainAccessConfig.getAccessKey(), buildPlainAccessResource(plainAccessConfig));
+ aclPlainAccessResourceMap.put(fileName, plainAccessResourceMap);
+ }
+ return AclUtils.writeDataObject(defaultAclFile, updateAclConfigFileVersion(defaultAclFile, aclAccessConfigMap));
+ }
+ }
+
+ public PlainAccessResource updatePlainAccessResource(Map<String, Object> updateAccountMap) {
+ PlainAccessResource updatePlainAccessResource = new PlainAccessResource();
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_ACCESS_KEY)) {
+ updatePlainAccessResource.setAccessKey(updateAccountMap.get(AclConstants.CONFIG_ACCESS_KEY).toString());
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_SECRET_KEY)) {
+ updatePlainAccessResource.setSecretKey(updateAccountMap.get(AclConstants.CONFIG_SECRET_KEY).toString());
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_WHITE_ADDR)) {
+ updatePlainAccessResource.setRemoteAddressStrategy(remoteAddressStrategyFactory.
+ getRemoteAddressStrategy(updateAccountMap.get(AclConstants.CONFIG_WHITE_ADDR).toString()));
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_ADMIN_ROLE)) {
+ updatePlainAccessResource.setAdmin(Boolean.parseBoolean(updateAccountMap.get(AclConstants.CONFIG_ADMIN_ROLE).toString()));
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_DEFAULT_TOPIC_PERM)) {
+ updatePlainAccessResource.setDefaultTopicPerm(Permission.
+ parsePermFromString(updateAccountMap.get(AclConstants.CONFIG_DEFAULT_TOPIC_PERM).toString()));
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_DEFAULT_GROUP_PERM)) {
+ updatePlainAccessResource.setDefaultGroupPerm(Permission.
+ parsePermFromString(updateAccountMap.get(AclConstants.CONFIG_DEFAULT_GROUP_PERM).toString()));
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_RESOURCE_PERMS)) {
+ List<LinkedHashMap<String, Object>> resourceAndPermList = (List<LinkedHashMap<String, Object>>)updateAccountMap.get(AclConstants.CONFIG_RESOURCE_PERMS);
+ for (LinkedHashMap resourceAndPerm : resourceAndPermList) {
+ String resource = resourceAndPerm.get("resource").toString();
+ String namespace = resourceAndPerm.get("namespace").toString();
+ String type = resourceAndPerm.get("type").toString();
+ String perm = resourceAndPerm.get("perm").toString();
+ if (namespace == null || namespace.isEmpty()) {
+ if (type.equals(ResourceType.GROUP.toString())) {
+ updatePlainAccessResource.addResourceAndPerm(MixAll.getRetryTopic(resource), Permission.parsePermFromString(perm));
+ } else {
+ updatePlainAccessResource.addResourceAndPerm(resource, Permission.parsePermFromString(perm));
+ }
+ } else {
+ if (type.equals(ResourceType.GROUP.toString())) {
+ updatePlainAccessResource.addResourceAndPerm(
+ MixAll.RETRY_GROUP_TOPIC_PREFIX + namespace + NAMESPACE_SEPARATOR + resource, Permission.parsePermFromString(perm));
Review Comment:
Make use of methods provided in `NamespaceUtil` to construct the resource. Do not import the concrete implementation of namespace formate in ACL module.
##########
acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java:
##########
@@ -393,6 +399,401 @@ public boolean updateAccessConfig(PlainAccessConfig plainAccessConfig) {
}
}
+ public boolean updateAclAccount(PlainAccessConfig plainAccessConfig) {
+ if (plainAccessConfig == null) {
+ log.error("Parameter value plainAccessConfig is null,Please check your parameter");
+ throw new AclException("Parameter value plainAccessConfig is null, Please check your parameter");
+ }
+ checkPlainAccessConfig(plainAccessConfig);
+
+ //Permission.checkResourcePerms(plainAccessConfig.getTopicPerms());
+ //Permission.checkResourcePerms(plainAccessConfig.getGroupPerms());
+ if (accessKeyTable.containsKey(plainAccessConfig.getAccessKey())) {
+ Map<String, Object> updateAccountMap = null;
+ String aclFileName = accessKeyTable.get(plainAccessConfig.getAccessKey());
+ Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(aclFileName, Map.class);
+ List<Map<String, Object>> accounts = (List<Map<String, Object>>) aclAccessConfigMap.get(AclConstants.CONFIG_ACCOUNTS);
+ if (null != accounts) {
+ for (Map<String, Object> account : accounts) {
+ if (account.get(AclConstants.CONFIG_ACCESS_KEY).equals(plainAccessConfig.getAccessKey())) {
+ // Update acl access config elements
+ accounts.remove(account);
+ updateAccountMap = createAclAccessConfigMap(account, plainAccessConfig);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ break;
+ }
+ }
+ } else {
+ // Maybe deleted in file, add it back
+ accounts = new LinkedList<>();
+ updateAccountMap = createAclAccessConfigMap(null, plainAccessConfig);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ }
+ Map<String, PlainAccessResource> accountMap = aclPlainAccessResourceMap.get(aclFileName);
+ PlainAccessResource updatePlainAccessResource = updatePlainAccessResource(updateAccountMap);
+ if (accountMap == null) {
+ accountMap = new HashMap<String, PlainAccessResource>(1);
+ accountMap.put(plainAccessConfig.getAccessKey(), updatePlainAccessResource);
+ } else if (accountMap.size() == 0) {
+ accountMap.put(plainAccessConfig.getAccessKey(), updatePlainAccessResource);
+ } else {
+ for (Map.Entry<String, PlainAccessResource> entry : accountMap.entrySet()) {
+ if (entry.getValue().getAccessKey().equals(plainAccessConfig.getAccessKey())) {
+ accountMap.put(entry.getKey(), updatePlainAccessResource);
+ break;
+ }
+ }
+ }
+ aclPlainAccessResourceMap.put(aclFileName, accountMap);
+ return AclUtils.writeDataObject(aclFileName, updateAclConfigFileVersion(aclFileName, aclAccessConfigMap));
+ } else {
+ String fileName = MixAll.dealFilePath(defaultAclFile);
+ //Create acl access config elements on the default acl file
+ if (aclPlainAccessResourceMap.get(defaultAclFile) == null || aclPlainAccessResourceMap.get(defaultAclFile).size() == 0) {
+ try {
+ File defaultAclFile = new File(fileName);
+ if (!defaultAclFile.exists()) {
+ defaultAclFile.createNewFile();
+ }
+ } catch (IOException e) {
+ log.warn("create default acl file has exception when update accessConfig. ", e);
+ }
+ }
+ Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(defaultAclFile, Map.class);
+ if (aclAccessConfigMap == null) {
+ aclAccessConfigMap = new HashMap<>();
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, new ArrayList<>());
+ }
+ List<Map<String, Object>> accounts = (List<Map<String, Object>>) aclAccessConfigMap.get(AclConstants.CONFIG_ACCOUNTS);
+ // When no accounts defined
+ if (null == accounts) {
+ accounts = new ArrayList<>();
+ }
+ accounts.add(createAclAccessConfigMap(null, plainAccessConfig));
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ accessKeyTable.put(plainAccessConfig.getAccessKey(), fileName);
+ if (aclPlainAccessResourceMap.get(fileName) == null) {
+ Map<String, PlainAccessResource> plainAccessResourceMap = new HashMap<>(1);
+ plainAccessResourceMap.put(plainAccessConfig.getAccessKey(), buildPlainAccessResource(plainAccessConfig));
+ aclPlainAccessResourceMap.put(fileName, plainAccessResourceMap);
+ } else {
+ Map<String, PlainAccessResource> plainAccessResourceMap = aclPlainAccessResourceMap.get(fileName);
+ plainAccessResourceMap.put(plainAccessConfig.getAccessKey(), buildPlainAccessResource(plainAccessConfig));
+ aclPlainAccessResourceMap.put(fileName, plainAccessResourceMap);
+ }
+ return AclUtils.writeDataObject(defaultAclFile, updateAclConfigFileVersion(defaultAclFile, aclAccessConfigMap));
+ }
+ }
+
+ public PlainAccessResource updatePlainAccessResource(Map<String, Object> updateAccountMap) {
+ PlainAccessResource updatePlainAccessResource = new PlainAccessResource();
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_ACCESS_KEY)) {
+ updatePlainAccessResource.setAccessKey(updateAccountMap.get(AclConstants.CONFIG_ACCESS_KEY).toString());
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_SECRET_KEY)) {
+ updatePlainAccessResource.setSecretKey(updateAccountMap.get(AclConstants.CONFIG_SECRET_KEY).toString());
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_WHITE_ADDR)) {
+ updatePlainAccessResource.setRemoteAddressStrategy(remoteAddressStrategyFactory.
+ getRemoteAddressStrategy(updateAccountMap.get(AclConstants.CONFIG_WHITE_ADDR).toString()));
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_ADMIN_ROLE)) {
+ updatePlainAccessResource.setAdmin(Boolean.parseBoolean(updateAccountMap.get(AclConstants.CONFIG_ADMIN_ROLE).toString()));
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_DEFAULT_TOPIC_PERM)) {
+ updatePlainAccessResource.setDefaultTopicPerm(Permission.
+ parsePermFromString(updateAccountMap.get(AclConstants.CONFIG_DEFAULT_TOPIC_PERM).toString()));
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_DEFAULT_GROUP_PERM)) {
+ updatePlainAccessResource.setDefaultGroupPerm(Permission.
+ parsePermFromString(updateAccountMap.get(AclConstants.CONFIG_DEFAULT_GROUP_PERM).toString()));
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_RESOURCE_PERMS)) {
+ List<LinkedHashMap<String, Object>> resourceAndPermList = (List<LinkedHashMap<String, Object>>)updateAccountMap.get(AclConstants.CONFIG_RESOURCE_PERMS);
+ for (LinkedHashMap resourceAndPerm : resourceAndPermList) {
+ String resource = resourceAndPerm.get("resource").toString();
+ String namespace = resourceAndPerm.get("namespace").toString();
+ String type = resourceAndPerm.get("type").toString();
+ String perm = resourceAndPerm.get("perm").toString();
+ if (namespace == null || namespace.isEmpty()) {
+ if (type.equals(ResourceType.GROUP.toString())) {
+ updatePlainAccessResource.addResourceAndPerm(MixAll.getRetryTopic(resource), Permission.parsePermFromString(perm));
+ } else {
+ updatePlainAccessResource.addResourceAndPerm(resource, Permission.parsePermFromString(perm));
+ }
+ } else {
+ if (type.equals(ResourceType.GROUP.toString())) {
+ updatePlainAccessResource.addResourceAndPerm(
+ MixAll.RETRY_GROUP_TOPIC_PREFIX + namespace + NAMESPACE_SEPARATOR + resource, Permission.parsePermFromString(perm));
+ } else {
+ updatePlainAccessResource.addResourceAndPerm(namespace + resource, Permission.parsePermFromString(perm));
+ }
+ }
+
+ }
+ }
+ if (updateAccountMap.containsKey(AclConstants.CONFIG_NAMESPACE_PERMS)) {
+ List<LinkedHashMap<String, String>> namespaceAndPermList = (List<LinkedHashMap<String, String>>)updateAccountMap.get(AclConstants.CONFIG_NAMESPACE_PERMS);
+ Map<String, Map<String, Byte>> namespacePermMap = new HashMap<>();
+ for (LinkedHashMap<String, String> namespaceAndPerm : namespaceAndPermList) {
+ String namespace = namespaceAndPerm.get("namespace");
+ String topicPerm = namespaceAndPerm.get("topicPerm");
+ String groupPerm = namespaceAndPerm.get("groupPerm");
+ Map<String, Byte> permMap = new HashMap<>(2);
+ if (topicPerm != null && !topicPerm.isEmpty()) {
+ permMap.put(AclConstants.CONFIG_TOPIC_PERM, Permission.parsePermFromString(topicPerm));
+ }
+ if (groupPerm != null && !groupPerm.isEmpty()) {
+ permMap.put(AclConstants.CONFIG_GROUP_PERM, Permission.parsePermFromString(groupPerm));
+ }
+ namespacePermMap.put(namespace, permMap);
+ }
+ updatePlainAccessResource.setNamespacePermMap(namespacePermMap);
+ }
+ return updatePlainAccessResource;
+ }
+
+ public boolean updateAclResourcePerms(String accesskey, ResourceAndPerm resourceAndPerm, OperationType operationType) {
+ if (accesskey == null || resourceAndPerm == null || operationType == null) {
+ log.error("Parameter: accesskey or resourceAndPerm or operationType is null, Please check your parameter");
+ throw new AclException("The parameter of updateAclResourcePerms command: accesskey or resourceAndPerm or operationType is null, Please check your parameter");
+ }
+ if (operationType == OperationType.ADD || operationType == OperationType.UPDATE) {
+ checkResourceAndPerm(resourceAndPerm);
+ }
+
+ if (accessKeyTable.containsKey(accesskey)) {
+ Map<String, Object> updateAccountMap = null;
+ String aclFileName = accessKeyTable.get(accesskey);
+ Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(aclFileName, Map.class);
+ List<Map<String, Object>> accounts = (List<Map<String, Object>>) aclAccessConfigMap.get(AclConstants.CONFIG_ACCOUNTS);
+ if (null != accounts) {
+ for (Map<String, Object> account : accounts) {
+ if (account.get(AclConstants.CONFIG_ACCESS_KEY).equals(accesskey)) {
+ // Update acl access config elements
+ accounts.remove(account);
+ updateAccountMap = updateAclAccessConfigMap(account, resourceAndPerm, operationType);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ break;
+ }
+ }
+ } else {
+ // Maybe deleted in file, add it back
+ accounts = new LinkedList<>();
+ updateAccountMap = updateAclAccessConfigMap(null, resourceAndPerm, operationType);
+ updateAccountMap.put(AclConstants.CONFIG_ACCESS_KEY, accesskey);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ }
+ Map<String, PlainAccessResource> accountMap = aclPlainAccessResourceMap.get(aclFileName);
+ PlainAccessResource updatePlainAccessResource = updatePlainAccessResource(updateAccountMap);
+ if (accountMap == null) {
+ accountMap = new HashMap<String, PlainAccessResource>(1);
+ accountMap.put(accesskey, updatePlainAccessResource);
+ } else if (accountMap.size() == 0) {
+ accountMap.put(accesskey, updatePlainAccessResource);
+ } else {
+ for (Map.Entry<String, PlainAccessResource> entry : accountMap.entrySet()) {
+ if (entry.getValue().getAccessKey().equals(accesskey)) {
+ accountMap.put(entry.getKey(), updatePlainAccessResource);
+ break;
+ }
+ }
+ }
+ aclPlainAccessResourceMap.put(aclFileName, accountMap);
+ return AclUtils.writeDataObject(aclFileName, updateAclConfigFileVersion(aclFileName, aclAccessConfigMap));
+ } else {
+ throw new AclException("The accesskey for update acl resource perms is not exist in the acl config.");
+ }
+
+ }
+
+ public Map<String, Object> updateAclAccessConfigMap(Map<String, Object> existedAccountMap,
+ ResourceAndPerm resourceAndPerm, OperationType operationType) {
+ Map<String, Object> newAccountsMap = null;
+ if (existedAccountMap == null) {
+ newAccountsMap = new LinkedHashMap<String, Object>();
+ } else {
+ newAccountsMap = existedAccountMap;
+ }
+
+ List<LinkedHashMap> resourceAndPerms = (List<LinkedHashMap>)newAccountsMap.get(AclConstants.CONFIG_RESOURCE_PERMS);
+ if (resourceAndPerms == null) {
+ resourceAndPerms = new ArrayList<>();
+ }
+
+ if (operationType == OperationType.ADD) {
+ LinkedHashMap<String, Object> addResourceAndPerm = new LinkedHashMap<String, Object>(4);
+ addResourceAndPerm.put("resource", resourceAndPerm.getResource());
+ addResourceAndPerm.put("type", resourceAndPerm.getType());
+ addResourceAndPerm.put("namespace", resourceAndPerm.getNamespace());
+ addResourceAndPerm.put("perm", resourceAndPerm.getPerm());
+ resourceAndPerms.add(addResourceAndPerm);
+ } else {
+ Iterator iterator = resourceAndPerms.iterator();
+ while (iterator.hasNext()) {
+ LinkedHashMap<String, Object> curResourceAndPerm = (LinkedHashMap)iterator.next();
+ if (curResourceAndPerm.get("resource").toString().equals(resourceAndPerm.getResource()) && curResourceAndPerm.get("type").toString().equals(resourceAndPerm.getType().toString())) {
+ if (curResourceAndPerm.get("namespace") != null && resourceAndPerm.getNamespace() != null
+ && !resourceAndPerm.getNamespace().isEmpty() && curResourceAndPerm.get("namespace").toString().equals(resourceAndPerm.getNamespace()) ||
+ curResourceAndPerm.get("namespace") == null && resourceAndPerm.getNamespace() == null) {
+ iterator.remove();
+ break;
+ }
+ }
+ }
+ if (operationType == OperationType.UPDATE) {
+ LinkedHashMap addResourceAndPerm = new LinkedHashMap<String, Object>(4);
+ addResourceAndPerm.put("resource", resourceAndPerm.getResource());
+ addResourceAndPerm.put("type", resourceAndPerm.getType());
+ addResourceAndPerm.put("namespace", resourceAndPerm.getNamespace());
+ addResourceAndPerm.put("perm", resourceAndPerm.getPerm());
+ resourceAndPerms.add(addResourceAndPerm);
+ }
+ }
+
+ newAccountsMap.put(AclConstants.CONFIG_RESOURCE_PERMS, resourceAndPerms);
+
+ return newAccountsMap;
+ }
+
+ public boolean updateAclNamespacePerms(String accesskey, List<NamespaceAndPerm> namespaceAndPerms, OperationType operationType) {
+ if (accesskey == null || operationType == null || namespaceAndPerms == null) {
+ log.error("Parameter: accesskey or operationType is null, Please check your parameter");
+ throw new AclException("The parameter of updateAclResourcePerms command: accesskey or operationType is null, Please check your parameter");
+ }
+
+ if (accessKeyTable.containsKey(accesskey)) {
+ Map<String, Object> updateAccountMap = null;
+ String aclFileName = accessKeyTable.get(accesskey);
+ Map<String, Object> aclAccessConfigMap = AclUtils.getYamlDataObject(aclFileName, Map.class);
+ List<Map<String, Object>> accounts = (List<Map<String, Object>>) aclAccessConfigMap.get(AclConstants.CONFIG_ACCOUNTS);
+ if (null != accounts) {
+ for (Map<String, Object> account : accounts) {
+ if (account.get(AclConstants.CONFIG_ACCESS_KEY).equals(accesskey)) {
+ // Update acl access config elements
+ accounts.remove(account);
+ updateAccountMap = updateAclAccessConfigMap(account, namespaceAndPerms, operationType);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ break;
+ }
+ }
+ } else {
+ // Maybe deleted in file, add it back
+ accounts = new LinkedList<>();
+ updateAccountMap = updateAclAccessConfigMap(null, namespaceAndPerms, operationType);
+ updateAccountMap.put(AclConstants.CONFIG_ACCESS_KEY, accesskey);
+ accounts.add(updateAccountMap);
+ aclAccessConfigMap.put(AclConstants.CONFIG_ACCOUNTS, accounts);
+ }
+ Map<String, PlainAccessResource> accountMap = aclPlainAccessResourceMap.get(aclFileName);
+ PlainAccessResource updatePlainAccessResource = updatePlainAccessResource(updateAccountMap);
+ if (accountMap == null) {
+ accountMap = new HashMap<String, PlainAccessResource>(1);
+ accountMap.put(accesskey, updatePlainAccessResource);
+ } else if (accountMap.size() == 0) {
+ accountMap.put(accesskey, updatePlainAccessResource);
+ } else {
+ for (Map.Entry<String, PlainAccessResource> entry : accountMap.entrySet()) {
+ if (entry.getValue().getAccessKey().equals(accesskey)) {
+ accountMap.put(entry.getKey(), updatePlainAccessResource);
+ break;
+ }
+ }
+ }
+ aclPlainAccessResourceMap.put(aclFileName, accountMap);
+ return AclUtils.writeDataObject(aclFileName, updateAclConfigFileVersion(aclFileName, aclAccessConfigMap));
+ } else {
+ throw new AclException("The accesskey for update acl namespace perms is not exist in the acl config.");
+ }
+
+
+ }
+
+ public Map<String, Object> updateAclAccessConfigMap(Map<String, Object> existedAccountMap,
+ List<NamespaceAndPerm> namespaceAndPerms, OperationType operationType) {
+ Map<String, Object> newAccountsMap = null;
+ if (existedAccountMap == null) {
+ newAccountsMap = new LinkedHashMap<String, Object>();
+ } else {
+ newAccountsMap = existedAccountMap;
+ }
+
+ List<LinkedHashMap<String, String>> namespaceAndPermList = (List<LinkedHashMap<String, String>>)newAccountsMap.get(AclConstants.CONFIG_NAMESPACE_PERMS);
+ if (namespaceAndPermList == null) {
+ namespaceAndPermList = new ArrayList<>();
+ }
+
+ if (operationType == OperationType.ADD) {
+ for (NamespaceAndPerm namespaceAndPerm : namespaceAndPerms) {
+ LinkedHashMap<String, String> linkedHashMap = new LinkedHashMap<>(3);
+ linkedHashMap.put("namespace", namespaceAndPerm.getNamespace());
+ linkedHashMap.put("topicPerm", namespaceAndPerm.getTopicPerm());
+ linkedHashMap.put("groupPerm", namespaceAndPerm.getGroupPerm());
+ namespaceAndPermList.add(linkedHashMap);
+ }
+ //namespaceAndPermList.addAll(namespaceAndPerms);
+ /*
+ for (NamespaceAndPerm namespaceAndPerm : namespaceAndPerms) {
+ namespaceAndPermList.add(namespaceAndPerm);
+ }
+ */
+ } else {
+ Iterator iterator = namespaceAndPermList.iterator();
+ while (iterator.hasNext()) {
+ LinkedHashMap<String, String> curNamespaceAndPerm = (LinkedHashMap<String, String>)iterator.next();
+ for (NamespaceAndPerm namespaceAndPerm : namespaceAndPerms) {
+ if (curNamespaceAndPerm.get("namespace").equals(namespaceAndPerm.getNamespace())) {
+ iterator.remove();
+ }
+ }
+ }
+ if (operationType == OperationType.UPDATE) {
+ for (NamespaceAndPerm namespaceAndPerm : namespaceAndPerms) {
+ LinkedHashMap<String, String> linkedHashMap = new LinkedHashMap<>(3);
+ linkedHashMap.put("namespace", namespaceAndPerm.getNamespace());
+ linkedHashMap.put("topicPerm", namespaceAndPerm.getTopicPerm());
+ linkedHashMap.put("groupPerm", namespaceAndPerm.getGroupPerm());
+ namespaceAndPermList.add(linkedHashMap);
+ }
+ }
+ }
+
+ newAccountsMap.put(AclConstants.CONFIG_NAMESPACE_PERMS, namespaceAndPermList);
+
+ return newAccountsMap;
+ }
+
+ public PlainAccessConfig getAccesskeyConfg(String accesskey) {
Review Comment:
`getConfigByAccessKey` would be more accurate.
--
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
Re: [PR] [ISSUE #4688]acl module support namespace. [rocketmq]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #4769: [ISSUE #4688]acl module support namespace.
URL: https://github.com/apache/rocketmq/pull/4769
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] caigy commented on a diff in pull request #4769: [ISSUE #4688]acl module support namespace.
Posted by GitBox <gi...@apache.org>.
caigy commented on code in PR #4769:
URL: https://github.com/apache/rocketmq/pull/4769#discussion_r991754880
##########
docs/cn/acl/user_guide.md:
##########
@@ -30,6 +30,26 @@ ACL客户端可以参考：**org.apache.rocketmq.example.simple**包下面的**A
| defaultGroupPerm | DENY;PUB;SUB;PUB\|SUB | 默认的ConsumerGroup权限 |
| topicPerms | topic=权限 | 各个Topic的权限 |
| groupPerms | group=权限 | 各个ConsumerGroup的权限 |
Review Comment:
Add notes about deprecation of `topicPerms` and `groupPerms`, and their successor `resourcePerms`.
##########
acl/src/main/java/org/apache/rocketmq/acl/AccessValidator.java:
##########
@@ -60,6 +63,38 @@ public interface AccessValidator {
*/
boolean updateAccessConfig(PlainAccessConfig plainAccessConfig);
+ /**
+ * Update the accessKey's secretKey whiteRemoteAddress admin defaultTopicPerm and defaultGroupPerm
+ * @param plainAccessConfig
+ * @return
+ */
+ boolean updateAclAccount(PlainAccessConfig plainAccessConfig);
+
+ /**
+ * update the accessKey's resourcePerms
+ * @param accesskey
+ * @param resourceAndPerm
+ * @param operationType
+ * @return
+ */
+ boolean updateAclResourcePerms(String accesskey, ResourceAndPerm resourceAndPerm, OperationType operationType);
+
+ /**
+ * update the accessKey's namespacePerms
+ * @param accesskey
+ * @param namespaceAndPerms
+ * @param operationType
+ * @return
+ */
+ boolean updateAclNamespacePerms(String accesskey, List<NamespaceAndPerm> namespaceAndPerms, OperationType operationType);
+
+ /**
+ * get the accessKey's config
+ * @param accesskey
+ * @return
+ */
Review Comment:
IMO `PlainAccessConfig` is an implementation of ACL, it should be separated from interface `AccessValidator`.
--
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 #4769: [ISSUE #4688]acl module support namespace.
Posted by GitBox <gi...@apache.org>.
coveralls commented on PR #4769:
URL: https://github.com/apache/rocketmq/pull/4769#issuecomment-1204654892
[](https://coveralls.io/builds/51402679)
Coverage increased (+0.09%) to 49.264% when pulling **80332da69dc0e936b1abfeeb1c7a8d7d0f20d92c on sunxi92:acl_namespace** into **c6b3a66ecf5e6bdeffb01093e5216bb10ee64a59 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 diff in pull request #4769: [ISSUE #4688]acl module support namespace.
Posted by GitBox <gi...@apache.org>.
caigy commented on code in PR #4769:
URL: https://github.com/apache/rocketmq/pull/4769#discussion_r990571364
##########
acl/src/main/java/org/apache/rocketmq/acl/common/AclUtils.java:
##########
@@ -256,6 +260,10 @@ public static <T> T getYamlDataObject(InputStream fis, Class<T> clazz) {
public static boolean writeDataObject(String path, Map<String, Object> dataMap) {
Yaml yaml = new Yaml();
+ if (new File(path).exists()) {
Review Comment:
May be better to use `Files.exists()` to check existence of path.
##########
acl/src/main/java/org/apache/rocketmq/acl/plain/PlainAccessResource.java:
##########
@@ -281,6 +380,43 @@ public static PlainAccessResource build(PlainAccessConfig plainAccessConfig, Rem
Permission.parseResourcePerms(plainAccessResource, false, plainAccessConfig.getGroupPerms());
Permission.parseResourcePerms(plainAccessResource, true, plainAccessConfig.getTopicPerms());
+ List<ResourceAndPerm> resourcePerms = plainAccessConfig.getResourcePerms();
+ if (resourcePerms != null && !resourcePerms.isEmpty()) {
+ for (ResourceAndPerm resource : resourcePerms) {
+ String resourceName = resource.getResource();
+ ResourceType type = resource.getType();
+ String namespace = resource.getNamespace();
+ String perm = resource.getPerm();
+ if (type == ResourceType.GROUP) {
+ String key = namespace == null ? PlainAccessResource.getRetryTopic(resourceName) :
+ MixAll.RETRY_GROUP_TOPIC_PREFIX + namespace + NAMESPACE_SEPARATOR + resourceName;
Review Comment:
Build `resourceName` for resource with namespace, then call `PlainAccessResource.getRetryTopic()`.
##########
acl/src/main/java/org/apache/rocketmq/acl/plain/PlainAccessResource.java:
##########
@@ -109,38 +116,92 @@ public static PlainAccessResource parse(RemotingCommand request, String remoteAd
accessResource.setSignature(request.getExtFields().get(SessionCredentials.SIGNATURE));
accessResource.setSecretToken(request.getExtFields().get(SessionCredentials.SECURITY_TOKEN));
+ String namespace = new String();
+ String resource = new String();
try {
switch (request.getCode()) {
case RequestCode.SEND_MESSAGE:
final String topic = request.getExtFields().get("topic");
if (PlainAccessResource.isRetryTopic(topic)) {
- accessResource.addResourceAndPerm(getRetryTopic(request.getExtFields().get("group")), Permission.SUB);
+ String group = request.getExtFields().get("group");
+ if (group != null && group.contains(String.valueOf(NAMESPACE_SEPARATOR))) {
+ namespace = group.substring(0, group.indexOf(NAMESPACE_SEPARATOR));
+ group = group.substring(group.indexOf(NAMESPACE_SEPARATOR) + 1);
+ }
+ if (!namespace.isEmpty()) {
+ resource = MixAll.RETRY_GROUP_TOPIC_PREFIX + namespace + NAMESPACE_SEPARATOR + group;
+ } else {
+ resource = getRetryTopic(group);
+ }
+ accessResource.addResourceAndPerm(resource, Permission.SUB);
Review Comment:
Is it necessary? It seems that `getRetryTopic()` will get the same result.
##########
acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionChecker.java:
##########
@@ -30,36 +31,82 @@ public void check(AccessResource checkedAccess, AccessResource ownedAccess) {
if (Permission.needAdminPerm(checkedPlainAccess.getRequestCode()) && !ownedPlainAccess.isAdmin()) {
throw new AclException(String.format("Need admin permission for request code=%d, but accessKey=%s is not", checkedPlainAccess.getRequestCode(), ownedPlainAccess.getAccessKey()));
}
- Map<String, Byte> needCheckedPermMap = checkedPlainAccess.getResourcePermMap();
- Map<String, Byte> ownedPermMap = ownedPlainAccess.getResourcePermMap();
+ Map<String, Byte> needCheckedResourcePermMap = checkedPlainAccess.getResourcePermMap();
+ Map<String, Byte> ownedResourcePermMap = ownedPlainAccess.getResourcePermMap();
+ Map<String, Map<String, Byte>> ownedNamespacePermMap = ownedPlainAccess.getNamespacePermMap();
- if (needCheckedPermMap == null) {
+ if (needCheckedResourcePermMap == null) {
// If the needCheckedPermMap is null,then return
return;
}
- if (ownedPermMap == null && ownedPlainAccess.isAdmin()) {
+ if (ownedResourcePermMap == null && ownedPlainAccess.isAdmin()) {
// If the ownedPermMap is null and it is an admin user, then return
return;
}
- for (Map.Entry<String, Byte> needCheckedEntry : needCheckedPermMap.entrySet()) {
+ for (Map.Entry<String, Byte> needCheckedEntry : needCheckedResourcePermMap.entrySet()) {
String resource = needCheckedEntry.getKey();
Byte neededPerm = needCheckedEntry.getValue();
boolean isGroup = PlainAccessResource.isRetryTopic(resource);
+ boolean isResourceContainsNamespace = PlainAccessResource.isContainNamespace(resource);
- if (ownedPermMap == null || !ownedPermMap.containsKey(resource)) {
- // Check the default perm
- byte ownedPerm = isGroup ? ownedPlainAccess.getDefaultGroupPerm() :
- ownedPlainAccess.getDefaultTopicPerm();
- if (!Permission.checkPermission(neededPerm, ownedPerm)) {
+ //the resource perm that ak owned is null or doesn't contain the resource
+ if (ownedResourcePermMap == null || !ownedResourcePermMap.containsKey(resource)) {
+ //check the namespace perm and the default perm
+ if (isMatchNamespaceAndDeafultPerm(isResourceContainsNamespace, resource, ownedNamespacePermMap, isGroup, neededPerm, ownedPlainAccess)) {
+ continue;
+ } else {
throw new AclException(String.format("No default permission for %s", PlainAccessResource.printStr(resource, isGroup)));
}
+ } else {
+ //check whether the resource perm that the ak owned is match the needed
+ if (!Permission.checkPermission(neededPerm, ownedResourcePermMap.get(resource))) {
+ //check the namespace perm and the default perm
+ if (isMatchNamespaceAndDeafultPerm(isResourceContainsNamespace, resource, ownedNamespacePermMap, isGroup, neededPerm, ownedPlainAccess)) {
+ continue;
+ } else {
+ throw new AclException(String.format("No default permission for %s", PlainAccessResource.printStr(resource, isGroup)));
+ }
+ }
continue;
}
- if (!Permission.checkPermission(neededPerm, ownedPermMap.get(resource))) {
- throw new AclException(String.format("No default permission for %s", PlainAccessResource.printStr(resource, isGroup)));
+ }
+ }
+
+ public boolean isMatchNamespaceAndDeafultPerm(boolean isResourceContainsNamespace, String resource, Map<String, Map<String, Byte>> ownedNamespacePermMap,
Review Comment:
`isMatchNamespaceOrDeafultPerm()` may be more accurate.
##########
acl/src/main/java/org/apache/rocketmq/acl/plain/PlainAccessResource.java:
##########
@@ -281,6 +380,43 @@ public static PlainAccessResource build(PlainAccessConfig plainAccessConfig, Rem
Permission.parseResourcePerms(plainAccessResource, false, plainAccessConfig.getGroupPerms());
Permission.parseResourcePerms(plainAccessResource, true, plainAccessConfig.getTopicPerms());
+ List<ResourceAndPerm> resourcePerms = plainAccessConfig.getResourcePerms();
+ if (resourcePerms != null && !resourcePerms.isEmpty()) {
+ for (ResourceAndPerm resource : resourcePerms) {
+ String resourceName = resource.getResource();
+ ResourceType type = resource.getType();
+ String namespace = resource.getNamespace();
+ String perm = resource.getPerm();
+ if (type == ResourceType.GROUP) {
+ String key = namespace == null ? PlainAccessResource.getRetryTopic(resourceName) :
+ MixAll.RETRY_GROUP_TOPIC_PREFIX + namespace + NAMESPACE_SEPARATOR + resourceName;
+ plainAccessResource.addResourceAndPerm(key, Permission.parsePermFromString(perm));
+ } else if (type == ResourceType.TOPIC) {
+ String key = namespace == null ? resourceName : namespace + NAMESPACE_SEPARATOR + resourceName;
+ plainAccessResource.addResourceAndPerm(key, Permission.parsePermFromString(perm));
Review Comment:
use `org.apache.rocketmq.common.protocol.NamespaceUtil#wrapNamespace` to build resource with namespace.
##########
acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionChecker.java:
##########
@@ -30,36 +31,82 @@ public void check(AccessResource checkedAccess, AccessResource ownedAccess) {
if (Permission.needAdminPerm(checkedPlainAccess.getRequestCode()) && !ownedPlainAccess.isAdmin()) {
throw new AclException(String.format("Need admin permission for request code=%d, but accessKey=%s is not", checkedPlainAccess.getRequestCode(), ownedPlainAccess.getAccessKey()));
}
- Map<String, Byte> needCheckedPermMap = checkedPlainAccess.getResourcePermMap();
- Map<String, Byte> ownedPermMap = ownedPlainAccess.getResourcePermMap();
+ Map<String, Byte> needCheckedResourcePermMap = checkedPlainAccess.getResourcePermMap();
+ Map<String, Byte> ownedResourcePermMap = ownedPlainAccess.getResourcePermMap();
+ Map<String, Map<String, Byte>> ownedNamespacePermMap = ownedPlainAccess.getNamespacePermMap();
- if (needCheckedPermMap == null) {
+ if (needCheckedResourcePermMap == null) {
// If the needCheckedPermMap is null,then return
return;
}
- if (ownedPermMap == null && ownedPlainAccess.isAdmin()) {
+ if (ownedResourcePermMap == null && ownedPlainAccess.isAdmin()) {
// If the ownedPermMap is null and it is an admin user, then return
return;
}
- for (Map.Entry<String, Byte> needCheckedEntry : needCheckedPermMap.entrySet()) {
+ for (Map.Entry<String, Byte> needCheckedEntry : needCheckedResourcePermMap.entrySet()) {
String resource = needCheckedEntry.getKey();
Byte neededPerm = needCheckedEntry.getValue();
boolean isGroup = PlainAccessResource.isRetryTopic(resource);
+ boolean isResourceContainsNamespace = PlainAccessResource.isContainNamespace(resource);
- if (ownedPermMap == null || !ownedPermMap.containsKey(resource)) {
- // Check the default perm
- byte ownedPerm = isGroup ? ownedPlainAccess.getDefaultGroupPerm() :
- ownedPlainAccess.getDefaultTopicPerm();
- if (!Permission.checkPermission(neededPerm, ownedPerm)) {
+ //the resource perm that ak owned is null or doesn't contain the resource
+ if (ownedResourcePermMap == null || !ownedResourcePermMap.containsKey(resource)) {
+ //check the namespace perm and the default perm
+ if (isMatchNamespaceAndDeafultPerm(isResourceContainsNamespace, resource, ownedNamespacePermMap, isGroup, neededPerm, ownedPlainAccess)) {
+ continue;
+ } else {
throw new AclException(String.format("No default permission for %s", PlainAccessResource.printStr(resource, isGroup)));
}
+ } else {
+ //check whether the resource perm that the ak owned is match the needed
+ if (!Permission.checkPermission(neededPerm, ownedResourcePermMap.get(resource))) {
+ //check the namespace perm and the default perm
+ if (isMatchNamespaceAndDeafultPerm(isResourceContainsNamespace, resource, ownedNamespacePermMap, isGroup, neededPerm, ownedPlainAccess)) {
+ continue;
Review Comment:
It seem that we should not check namespace or default permission here, e.g.: If ak owns `DENY` permission, it is not allowed to access the resource even namespace or default permission is `PUB` or `SUB`.
##########
acl/src/main/java/org/apache/rocketmq/acl/plain/PlainAccessResource.java:
##########
@@ -281,6 +380,43 @@ public static PlainAccessResource build(PlainAccessConfig plainAccessConfig, Rem
Permission.parseResourcePerms(plainAccessResource, false, plainAccessConfig.getGroupPerms());
Permission.parseResourcePerms(plainAccessResource, true, plainAccessConfig.getTopicPerms());
+ List<ResourceAndPerm> resourcePerms = plainAccessConfig.getResourcePerms();
+ if (resourcePerms != null && !resourcePerms.isEmpty()) {
+ for (ResourceAndPerm resource : resourcePerms) {
+ String resourceName = resource.getResource();
+ ResourceType type = resource.getType();
+ String namespace = resource.getNamespace();
+ String perm = resource.getPerm();
+ if (type == ResourceType.GROUP) {
+ String key = namespace == null ? PlainAccessResource.getRetryTopic(resourceName) :
+ MixAll.RETRY_GROUP_TOPIC_PREFIX + namespace + NAMESPACE_SEPARATOR + resourceName;
+ plainAccessResource.addResourceAndPerm(key, Permission.parsePermFromString(perm));
+ } else if (type == ResourceType.TOPIC) {
+ String key = namespace == null ? resourceName : namespace + NAMESPACE_SEPARATOR + resourceName;
+ plainAccessResource.addResourceAndPerm(key, Permission.parsePermFromString(perm));
+ }
+ }
+ }
+
+ List<NamespaceAndPerm> namespacePerms = plainAccessConfig.getNamespacePerms();
+ if (namespacePerms != null && !namespacePerms.isEmpty()) {
+ Map<String, Map<String, Byte>> namespacePermMap = new HashMap<>();
+ for (NamespaceAndPerm namespace : namespacePerms) {
+ String namespaceName = namespace.getNamespace();
+ String topicPerm = namespace.getTopicPerm();
+ String groupPerm = namespace.getGroupPerm();
+ Map<String, Byte> permMap = new HashMap<>(2);
Review Comment:
Use `com.google.common.collect.Maps#newHashMapWithExpectedSize` to prevent resizing, or `java.util.Collections#singletonMap` if only one entry in this map.
##########
acl/src/main/java/org/apache/rocketmq/acl/plain/PlainAccessResource.java:
##########
@@ -289,6 +425,42 @@ public static boolean isRetryTopic(String topic) {
return null != topic && topic.startsWith(MixAll.RETRY_GROUP_TOPIC_PREFIX);
}
+ public static boolean isContainNamespace(String resource) {
Review Comment:
Use methods provided in `NamespaceUtil` if possible, the logic of processing namespace should not be included in ACL module.
##########
acl/src/main/java/org/apache/rocketmq/acl/common/AclUtils.java:
##########
@@ -256,6 +260,10 @@ public static <T> T getYamlDataObject(InputStream fis, Class<T> clazz) {
public static boolean writeDataObject(String path, Map<String, Object> dataMap) {
Yaml yaml = new Yaml();
+ if (new File(path).exists()) {
+ String pathBak = path + ".bak";
+ copyFile(path, pathBak);
Review Comment:
use `Files.copy()` instead of recreating one.
##########
acl/src/main/java/org/apache/rocketmq/acl/plain/PlainAccessResource.java:
##########
@@ -281,6 +380,43 @@ public static PlainAccessResource build(PlainAccessConfig plainAccessConfig, Rem
Permission.parseResourcePerms(plainAccessResource, false, plainAccessConfig.getGroupPerms());
Permission.parseResourcePerms(plainAccessResource, true, plainAccessConfig.getTopicPerms());
+ List<ResourceAndPerm> resourcePerms = plainAccessConfig.getResourcePerms();
Review Comment:
It may be better to wrap the process of converting new resource and namespace perm in `Permission`.
--
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] lmove commented on pull request #4769: [ISSUE #4688]acl module support namespace.
Posted by GitBox <gi...@apache.org>.
lmove commented on PR #4769:
URL: https://github.com/apache/rocketmq/pull/4769#issuecomment-1319792326
Hi! Our tool [BreakBot](https://github.com/alien-tools/breakbot) found that this pull request introduces **9 breaking changes** and they appear to impact **9 of your clients**. You can find the full BreakBot report in our fork repository: [report for PR#4769](https://github.com/breakbot-playground/rocketmq/pull/1/checks).
We hope this information is valuable to you, and apologize otherwise. If you're willing to help, we would kindly ask for your help to fill in a [5-minutes survey](https://survey.breakbot.net/index.php/418865/newtest/Y?ID=apache-rocketmq-4769) about the report. Your feedback will help us improve the tool and provide a better experience for users in the future!
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] coveralls commented on pull request #4769: [ISSUE #4688]acl module support namespace.
Posted by GitBox <gi...@apache.org>.
coveralls commented on PR #4769:
URL: https://github.com/apache/rocketmq/pull/4769#issuecomment-1204654895
[](https://coveralls.io/builds/51402679)
Coverage increased (+0.09%) to 49.264% when pulling **80332da69dc0e936b1abfeeb1c7a8d7d0f20d92c on sunxi92:acl_namespace** into **c6b3a66ecf5e6bdeffb01093e5216bb10ee64a59 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