Closed Shubhi-Jain98 closed 2 years ago
@Justin-Xiang could you help review the PR?
Thank you for the PR. Great work!
I have tested all of them and there is only one problem on my side:
In Ctest2 Param2: hadoop.user.group.static.mapping.overrides:
me=;
This should fail in this line of code:
assertThat(groups.getGroups("me").size()).isEqualTo(2);
You can check parseStaticMapping()
method in Groups.java
:
String user = userToGroupsArray[0];
List<String> groups = Collections.emptyList();
if (userToGroupsArray.length == 2) {
groups = (List<String>) StringUtils
.getStringCollection(userToGroupsArray[1]);
}
staticUserToGroupsMap.put(user, groups);
Here, the("me", []) will be put into the staticUserToGroupsMap
Besides, you can also take a closer look at the cacheGroupsAdd()
function.
Btw, I wonder how you test param like "user1=user2=group1" since this param has 2 "=", you may have to modify the run_single_ctest.py
to inject this successfully. You can also push changes in this file to this PR.
Thank you for reviewing my PR. Pardon me; this is a mistake on my end. I rechecked the workflow, and yes, the test will fail for me=; I have incorporated this change.
For the "user1=user2=user3" value, I found that since the python script finds the index of first "=" so the script injects the whole argument after it as a value; here, "user1=user2=user3". I am attaching the screenshot of the logs.
@Justin-Xiang can you review again and try to merge the PR if you think it's correct and ready?
@tianyin Yes, I think @Shubhi-Jain98 has corrected the mistake I mentioned so it's ok to merge this PR. But it seems that I don't have the authority to merge PR.
If you approve, I'm going to merge.
Thanks @Justin-Xiang for the review and congratulations @Shubhi-Jain98 for getting the PR merged!
Hi All, The below CTests include the rewritten tests and the tests validated for correctness and effectiveness.
CTest 1. org.apache.hadoop.net.TestNetUtils#testInvalidAddress
Param1: hadoop.security.dns.log-slow-lookups.threshold.ms
2147483647
GOOD | PASS2147483648
BAD | FAIL java.lang.NumberFormatException-1
GOOD | PASS0
GOOD | PASSParam2: hadoop.security.dns.log-slow-lookups.enabled
false
GOOD | PASStrue
GOOD | PASSParam3: hadoop.rpc.socket.factory.class.default
org.apache.hadoop.net.StandardSocketFactory
GOOD | PASSorg.apache.hadoop.net.NotStandardSocketFactory
BAD | FAIL with java.lang.ClassNotFoundException: Class org.apache.hadoop.net.NotStandardSocketFactory not foundorg.apache.hadoop.net.SocksSocketFactory
GOOD | PASSCTest 2. org.apache.hadoop.security.TestGroupsCaching# testExceptionCallingLoadWithoutBackgroundRefreshReturnsOldValue
Param1: hadoop.security.groups.cache.background.reload.threads
1
= GOOD | PASS90
= GOOD | PASShadoop.security.groups.cache.background.reload=false && -1
BAD | PASShadoop.security.groups.cache.background.reload=true && -1
BAD | FAIL with java.lang.IllegalArgumentExceptionhadoop.security.groups.cache.background.reload=false && 0
BAD | PASShadoop.security.groups.cache.background.reload=true && 0
BAD | FAIL with java.lang.IllegalArgumentException2147483648
BAD | FAIL with java.lang.NumberFormatExceptionThe CTest passes for bad values 0 and –1 as this configuration is not considered until hadoop.security.groups.cache.background.reload is set to true. On setting this hadoop.security.groups.cache.background.reload to true, the test gives expected behavior.
Param2: hadoop.user.group.static.mapping.overrides
dr.who=;
GOOD | PASSuser1=group1;
GOOD | PASSgroup1=group2;
GOOD | PASSme=;
BAD | FAILuser1=user2=group1
BAD | FAIL with org.apache.hadoop.HadoopIllegalArgumentException: Configuration hadoop.user.group.static.mapping.overrides is invalidParam3: hadoop.security.groups.negative-cache.secs
30
GOOD | PASS1000
GOOD | PASS1
GOOD | PASS99999999999999999999
BAD | FAIL java.lang.NumberFormatExceptionParam4: hadoop.security.groups.cache.warn.after.ms
5000
GOOD | PASS1
GOOD | PASS30
GOOD | PASS-1
BAD | PASS99999999999999999999
BAD | FAIL java.lang.NumberFormatExceptionHere, the test is passing on configuration values 0 and –1 because the value, warningDeltaMs, is used as a comparison in the code, if (deltaMs > warningDeltaMs)
Param5: hadoop.security.groups.shell.command.timeout
2s
GOOD | PASS10s
GOOD | PASS3m
GOOD | PASS2j
BAD | FAIL java.lang.NumberFormatExceptionBelow are the two parameters added in the test:
Added Param6: hadoop.security.groups.cache.secs
1
GOOD | PASS90
GOOD | PASS300
GOOD | PASS-1
BAD | FAIL java.lang.IllegalArgumentException: duration must be positive: -1000 MILLISECONDS0
BAD | FAIL java.lang.IllegalArgumentException: duration must be positive: 0 MILLISECONDSIn this case, conf.setLong was sending default parameter value 1. The test was passing for non-negative values as well. Since time is a positive quantity, the value of the "hadoop.security.groups.cache.secs" parameter should be greater than 0. Hence, removed the default conf.setLong to fix the case.
Added Param7: hadoop.security.groups.cache.background.reload
false
GOOD | PASStrue
GOOD | PASSCTest 3. org.apache.hadoop.io.compress.TestCodec#testGzipCodecWithParam
Param1: io.file.buffer.size
8
GOOD | PASS512
GOOD | PASS4096
GOOD | PASS-1
BAD | FAIL java.lang.IllegalArgumentException: Illegal bufferSizeCTest 4. org.apache.hadoop.crypto.key.kms.TestKMSClientProvider#testSelectDelegationToken
Param1: hadoop.security.dns.log-slow-lookups.threshold.ms
1
GOOD | PASS1000
GOOD | PASS2000
GOOD | PASS-1
BAD | PASS2147483648
BAD | FAIL java.lang.NumberFormatExceptionHere, the test is passing on configuration value –1 as well because here this value, slowLookupThresholdMs, is used a comparison in the code, if (elapsedMs >= slowLookupThresholdMs)
Param2: hadoop.security.dns.log-slow-lookups.enabled
false
GOOD | PASStrue
GOOD | PASSCTest 5. org.apache.hadoop.security.TestShellBasedUnixGroupsMapping#testFiniteGroupResolutionTime
Param1: hadoop.security.groups.cache.background.reload.threads
1
= GOOD | PASS90
= GOOD | PASShadoop.security.groups.cache.background.reload=false && -1
BAD | PASShadoop.security.groups.cache.background.reload=true && -1
BAD | FAIL with java.lang.IllegalArgumentExceptionhadoop.security.groups.cache.background.reload=false && 0
BAD | PASShadoop.security.groups.cache.background.reload=true && 0
BAD | FAIL with java.lang.IllegalArgumentException2147483648
BAD | FAIL with java.lang.NumberFormatExceptionParam2: hadoop.security.groups.cache.secs
1
GOOD | PASS90
GOOD | PASS300
GOOD | PASS-1
BAD | FAIL java.lang.IllegalArgumentException: duration must be positive: -1000 MILLISECONDS0
BAD | FAIL java.lang.IllegalArgumentException: duration must be positive: 0 MILLISECONDSParam3: hadoop.user.group.static.mapping.overrides
dr.who=;
GOOD | PASSuser1=group1;
GOOD | PASSme=;
GOOD | PASSuser1=user2=group1
BAD | FAIL with org.apache.hadoop.HadoopIllegalArgumentException: Configuration hadoop.user.group.static.mapping.overrides is invalidParam4: hadoop.security.groups.cache.background.reload
false
GOOD | PASStrue
GOOD | PASSParam5: hadoop.security.groups.negative-cache.secs
30
GOOD | PASS1000
GOOD | PASS1
GOOD | PASS9995696789999978889
BAD | FAIL java.lang.NumberFormatExceptionParam6: hadoop.security.groups.cache.warn.after.ms
5000
GOOD | PASS1
GOOD | PASS30
GOOD | PASS-1
BAD | PASS9995696789999978889
BAD | FAIL java.lang.NumberFormatExceptionHere, the test is passing on configuration values 0 and –1 as well because here this value, warningDeltaMs, is used a comparison in the code, if (deltaMs > warningDeltaMs)