[ISSUE #1147]Broker will report Exception if open the aclEnable and enableDLegerCommitLog flag at the same time. #1149
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… copy the tool.yml file to their related fold and AclEnable flag is closed.
…ableDLegerCommitLog flag at the same time.
…ableDLegerCommitLog flag at the same time.
zongtanghu
changed the title
| } | ||
|
|
||
| accessResource.setRequestCode(request.getCode()); | ||
| accessResource.setAccessKey(request.getExtFields().get(SessionCredentials.ACCESS_KEY)); |
There was a problem hiding this comment.
Okay,I will adjust my codes.
… aclEnable and use Master/Slave or Dledger the same time.
| JSONObject.class); | ||
| } catch (Exception e) { | ||
| e.printStackTrace(); | ||
| return null; |
There was a problem hiding this comment.
use logger instead of print
There was a problem hiding this comment.
Okay,I will adjust my codes in this pr again.
| if (request.getExtFields() == null) { | ||
| throw new AclException("request's extFields value is null"); | ||
| //If request's extFields is null,then return accessResource directly(users can use whiteAddress pattern) | ||
| return accessResource; | ||
| } |
There was a problem hiding this comment.
It is vulnerable to return fast here, for it may miss something if adding some logic after the return, the "accessResource.setRequestCode(request.getCode())" is the case, but not the only case.
As the following code relies on that the getExtFields is not null, then use if statement is suggested, just as:
if (request.getExtFields() != null) {
accessResource.setAccessKey(request.getExtFields().get(SessionCredentials.ACCESS_KEY));
XXX
}
There was a problem hiding this comment.
Okay,I will return the variable later in my next commits.
There was a problem hiding this comment.
Hi, @dongeforever I return back my pr's codes to original ones.
|
|
||
|
|
||
| (2)如果ACL与高可用部署(多副本Dledger架构)同时启用，由于出现节点宕机时，Dledger Group组内会自动选主，那么就需要将Dledger Group组 | ||
| 内所有Broker节点的plain_acl.yml配置文件的白名单设置所有Broker节点的ip地址。 |
There was a problem hiding this comment.
Remind the user to notice the bug denoted by this PR?
Currently, it is not ok even though configuring the white IP.
There was a problem hiding this comment.
Okay.I write this notice.
…an't open the aclEnable and use Master/Slave or Dledger the same time.
zongtanghu
changed the title
…an't open the aclEnable and use Master/Slave or Dledger the same time.
vongosling
changed the title
wlliqipeng
reviewed
Apr 26, 2019
docs/cn/acl/user_guide.md
Outdated
| @@ -75,8 +75,10 @@ Broker端对权限的校验逻辑主要分为以下几步： | |||
| ## 5. 热加载修改后权限控制定义 | |||
| RocketrMQ的权限控制存储的默认实现是基于yml配置文件。用户可以动态修改权限控制定义的属性，而不需重新启动Broker服务节点。 | |||
rocketmq version
rocketmq start model #debug initialAcl |
Hi，you could have a try for using rocketmq 4.5.1 version.And you can tell use the steps to show you issue again. |
JiaMingLiu93
pushed a commit
to JiaMingLiu93/rocketmq
that referenced
this pull request
May 28, 2020
… and enableDLegerCommitLog flag at the same time. (apache#1149) * [issue#1078]fix User can't use mqadmin command normally if they don't copy the tool.yml file to their related fold and AclEnable flag is closed. * [issue#1147]Broker will report Exception if open the aclEnable and enableDLegerCommitLog flag at the same time. * [issue#1147]Broker will report Exception if open the aclEnable and enableDLegerCommitLog flag at the same time. * [issue#1147]Add the restriction of Acl in the acl's user guide. * [issue#1147]Adjust some codes for acl issue that users can't open the aclEnable and use Master/Slave or Dledger the same time. * [issue#1147]Adjust and optimize some codes for acl issue that users can't open the aclEnable and use Master/Slave or Dledger the same time. * [issue#1147]return back for original codes for acl issue that users can't open the aclEnable and use Master/Slave or Dledger the same time.
pulllock
pushed a commit
to pulllock/rocketmq
that referenced
this pull request
Oct 19, 2023
… and enableDLegerCommitLog flag at the same time. (apache#1149) * [issue#1078]fix User can't use mqadmin command normally if they don't copy the tool.yml file to their related fold and AclEnable flag is closed. * [issue#1147]Broker will report Exception if open the aclEnable and enableDLegerCommitLog flag at the same time. * [issue#1147]Broker will report Exception if open the aclEnable and enableDLegerCommitLog flag at the same time. * [issue#1147]Add the restriction of Acl in the acl's user guide. * [issue#1147]Adjust some codes for acl issue that users can't open the aclEnable and use Master/Slave or Dledger the same time. * [issue#1147]Adjust and optimize some codes for acl issue that users can't open the aclEnable and use Master/Slave or Dledger the same time. * [issue#1147]return back for original codes for acl issue that users can't open the aclEnable and use Master/Slave or Dledger the same time.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the purpose of the change
（1）Move the getAclRPCHook funtion to the Acl module.
（2）Optimize some codes.
Brief changelog
（1）I have writes some test cases for my optimize codes.
Verifying this change
（1）Move the getAclRPCHook funtion to the Acl module.
（2）Optimize some codes.
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.[ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyleto make sure basic checks pass. Runmvn clean install -DskipITsto make sure unit-test pass. Runmvn clean test-compile failsafe:integration-testto make sure integration-test pass.