unit tests in rocketmq-acl may fail due to data conflict

[caigy]

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:

rocketmq/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java

Lines 238 to 249 in ead6274

	if (accounts != null && !accounts.isEmpty()) {
	List<PlainAccessConfig> plainAccessConfigList = accounts.toJavaList(PlainAccessConfig.class);
	for (PlainAccessConfig plainAccessConfig : plainAccessConfigList) {
	PlainAccessResource plainAccessResource = buildPlainAccessResource(plainAccessConfig);
	//AccessKey can not be defined in multiple ACL files
	String oldPath = this.accessKeyTable.get(plainAccessResource.getAccessKey());
	if (oldPath == null || aclFilePath.equals(oldPath)) {
	plainAccessResourceMap.put(plainAccessResource.getAccessKey(), plainAccessResource);
	this.accessKeyTable.put(plainAccessResource.getAccessKey(), aclFilePath);
	}
	}
	}

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:

rocketmq/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainAccessValidatorTest.java

Lines 908 to 917 in ead6274

	public void deleteAccessAclToEmptyTest() {
	System.setProperty("rocketmq.acl.plain.file", "/conf/empty.yml");
	PlainAccessConfig plainAccessConfig = new PlainAccessConfig();
	plainAccessConfig.setAccessKey("deleteAccessAclToEmpty");
	plainAccessConfig.setSecretKey("12345678");
	PlainAccessValidator plainAccessValidator = new PlainAccessValidator();
	plainAccessValidator.updateAccessConfig(plainAccessConfig);
	boolean success = plainAccessValidator.deleteAccessConfig("deleteAccessAclToEmpty");
	System.setProperty("rocketmq.acl.plain.file", "/conf/plain_acl.yml");

In PlainPermissionManager, File.separator was prepended to the value of property rocketmq.acl.plain.file:

rocketmq/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java

Line 63 in ead6274

private String defaultAclFile = fileHome + File.separator + System.getProperty("rocketmq.acl.plain.file", "conf/plain_acl.yml");

rocketmq/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java

Lines 124 to 127 in ead6274

	fileList = getAllAclFiles(defaultAclDir);
	if (new File(defaultAclFile).exists() && !fileList.contains(defaultAclFile)) {
	fileList.add(defaultAclFile);
	}

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.