Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[ISSUE #1156]Add new mqadmin API for ACL configuration #1257

Merged
merged 4 commits into from Jun 16, 2019

Conversation

@zongtanghu
Copy link
Contributor

zongtanghu commented Jun 14, 2019

What is the purpose of the change

(1)Adding data new version mechanism for every broker's acl config file in a cluster.
(2)Adding ClusterAclConfigVersionListSubCommand for querying acl config version;
(3)Adding DeleteAccessConfigSubCommand for deleting account attribute in acl config file.
(4)Adding UpdateAccessConfigSubCommand for updating account attribute in acl config file.
(5)Adding UpdateGlobalWhiteAddrSubCommand for updating global white addrs in acl config file.

Brief changelog

(1)Adding data new version mechanism for every broker's acl config file in a cluster.
(2)Adding ClusterAclConfigVersionListSubCommand for querying acl config version;
(3)Adding DeleteAccessConfigSubCommand for deleting account attribute in acl config file.
(4)Adding UpdateAccessConfigSubCommand for updating account attribute in acl config file.
(5)Adding UpdateGlobalWhiteAddrSubCommand for updating global white addrs in acl config file.

Verifying this change

I have already verified this changes in my local enviroment and passed acl uniting test cases.

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.

  • Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test(over 80% coverage) to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
  • Run mvn -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyle to make sure basic checks pass. Run mvn clean install -DskipITs to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.
zongtanghu added 2 commits Jun 14, 2019
* [issue#1164]return the codes to original reput method part.

* [issue#1164]fix issue that Consumer Instance can't consume message from slave when cluster is in the high level tps and master has been killed.

* [issue#1164]if the broker is a master node,then modify reputFromOffset correctly.

* [issue#1164]add some coding comments.

* [ISSUE#1156]new mqadmin API for ACL configuration.Add dataVersion and write data to acl yaml config file in the acl module.

* [ISSUE#1156]add unit test cases for new acl mqadmin API command which adding dataversion part.

* [ISSUE#1156]implement update,delete and query Acl config version in new acl mqadmin API command.

* [ISSUE#1156]polish and optimize the implementation for new acl mqadmin API command.

* [ISSUE#1156]fix the issues that topicPerms and groupPerms can't be updated and there is NPE when querying acl config version.

* [ISSUE#1156]fix small issue that specify wrong plainAccessConfig attribute,the correct one is AccessKey.

* [ISSUE#1156]add updateGlobalWhiteAddr subcommand codes for mqadmin acl command.

* [ISSUE#1156]adjust some codes for cluster acl config version list in the mqadmin acl commands.

* [ISSUE#1156]add acl mqadmin command part in the acl user_guide docs.

* [ISSUE#1156]polish and optimize some part of acl mqadmin command codes.

* [ISSUE#1156]fix code comments issue that the first letter needs to be capitalized and adjust the contents of acl user_guide.
@ShannonDing

This comment has been minimized.

Copy link
Member

ShannonDing commented Jun 14, 2019

It is better to rebase the commit log first, and could you please fix the CI build error in rocketmq-acl module.

@zongtanghu

This comment has been minimized.

Copy link
Contributor Author

zongtanghu commented Jun 14, 2019

It is better to rebase the commit log first, and could you please fix the CI build error in rocketmq-acl module.

Yes,I'm solving the rocketmq client failure test cases; @ShannonDing

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jun 14, 2019

Coverage Status

Coverage decreased (-0.2%) to 50.345% when pulling d1db4a1 on enchanced_acl into a7e77d2 on develop.

Copy link
Member

vongosling left a comment

great job

Yaml yaml = new Yaml();
PrintWriter pw = null;
try {
pw = new PrintWriter(new FileWriter(path));

This comment has been minimized.

Copy link
@ShannonDing

ShannonDing Jun 15, 2019

Member

Should this "new FileWriter(path)" handler be closed after use?

This comment has been minimized.

Copy link
@zongtanghu

zongtanghu Jun 15, 2019

Author Contributor

I think it will be ok if the printwriter object has been closed.

This comment has been minimized.

Copy link
@zongtanghu

zongtanghu Jun 15, 2019

Author Contributor

I look into the implementation of PrintWriter close method,it can close Writer object which is as constructor of paramter(FileWriter)!

@ShannonDing ShannonDing added this to the 4.5.2 milestone Jun 15, 2019
@duhenglucky duhenglucky merged commit a5d34dd into develop Jun 16, 2019
3 checks passed
3 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.02%) to 50.407%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.