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/04/12 12:21:07 UTC
[GitHub] [rocketmq] caigy opened a new issue, #4155: unit tests in rocketmq-acl may fail due to data conflict
caigy opened a new issue, #4155:
URL: https://github.com/apache/rocketmq/issues/4155
**BUG REPORT**
1. Please describe the issue you observed:
- What did you do (The steps to reproduce)?
Check out `develop` branch and run `mvn test` under `acl` directory.
- What is expected to see?
All tests should be successful.
- What did you see instead?
Tests may fail with results below:
```
Results :
Failed tests:
PlainAccessControlFlowTest.testOnlyAclFolderCase:170->testValidationAfterConfigFileChanged:228 Message should not be consumed after account deleted
```
2. Please tell us about your environment:
- Java version: 1.8.0_292
- maven: 3.6.3
3. Other information (e.g. detailed explanation, logs, related issues, suggestions on how to fix, etc):
Reproduction conditions:
- Running tests with `mvn test` can reproduce the problem, but running test in GUI of Intelij IDE will not.
- Running tests in rocketmq-acl together can reproduce the problem, but running all tests only in `PlainAccessControlFlowTest` will not.
**Cause analysis**
In `org.apache.rocketmq.acl.plain.PlainAccessControlFlowTest#testValidationAfterConfigFileChanged`, consumer access key (ak22222) is deleted in config file and then `org.apache.rocketmq.acl.plain.PlainPermissionManager#load(aclFilePath)` is called to load the content of config file previously updated.
The loading logic is as follows:
https://github.com/apache/rocketmq/blob/ead6274b3e8016ee2fa75cf0dc201b5581ee7a34/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java#L238-L249
After I printed the content of `org.apache.rocketmq.acl.plain.PlainPermissionManager#accessKeyTable`, I found something strange: the file path of `ak22222` is `...//conf/plain_acl.yml` (pay attention to the double slashes), but `PlainPermissionManager#load(aclFilePath)` was called with `/Users/caigy/git/rocketmq/acl/src/test/resources/only_acl_folder_conf/conf/plain_acl.yml` as parameter.
```
{ak22222=/Users/caigy/git/rocketmq/acl/src/test/resources/only_acl_folder_conf//conf/plain_acl.yml, rocketmq2=/Users/caigy/git/rocketmq/acl/src/test/resources/only_acl_folder_conf/conf/acl/plain_acl.yml, ak11111=/Users/caigy/git/rocketmq/acl/src/test/resources/only_acl_folder_conf//conf/plain_acl.yml, RocketMQ=/Users/caigy/git/rocketmq/acl/src/test/resources/only_acl_folder_conf/conf/acl/plain_acl.yml}
```
According to the loading logic above, access resource will not be updated.
So where does the strange `//` come from?
In `org.apache.rocketmq.acl.plain.PlainAccessValidatorTest#deleteAccessAclToEmptyTest`, the system property `rocketmq.acl.plain.file` was set:
https://github.com/apache/rocketmq/blob/ead6274b3e8016ee2fa75cf0dc201b5581ee7a34/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainAccessValidatorTest.java#L908-L917
In `PlainPermissionManager`, `File.separator` was prepended to the value of property `rocketmq.acl.plain.file`:
https://github.com/apache/rocketmq/blob/ead6274b3e8016ee2fa75cf0dc201b5581ee7a34/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java#L63
https://github.com/apache/rocketmq/blob/ead6274b3e8016ee2fa75cf0dc201b5581ee7a34/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java#L124-L127
**Fix**
- In real scenario, `rocketmq.acl.plain.file` would not be set multiple times in one process, so I would fix this issue in a simple and direct way: removing redundant file separator in `PlainAccessValidatorTest`.
- Using file path as key in `PlainPermissionManager#load(aclFilePath)` is still risky, I would open another issue to normalize file paths used as key.
--
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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] yuz10 closed issue #4155: unit tests in rocketmq-acl may fail due to data conflict
Posted by GitBox <gi...@apache.org>.
yuz10 closed issue #4155: unit tests in rocketmq-acl may fail due to data conflict
URL: https://github.com/apache/rocketmq/issues/4155
--
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