Details
-
Bug
-
Status: Resolved
-
Major
-
Resolution: Fixed
-
None
-
None
-
None
-
Reviewed
Description
There's a race in the globals. The non-global APIs from ZOOKEEPER-2139 are not available yet in a stable ZK version and there's no timeline for availability, so for now it would help to make SM aware of other users of the global config.
Attachments
Attachments
- HADOOP-13422.patch
- 1 kB
- Sergey Shelukhin
- HADOOP-13422.01.patch
- 3 kB
- Sergey Shelukhin
Activity
private final javax.security.auth.login.Configuration baseConfig = javax.security.auth.login.Configuration .getConfiguration();
I expect pre-commit will flag a nitpick about indentation and line length exceeding 80 characters here, so we'll need one more patch revision.
I'm in favor of the approach though. This will help avoid some bugs until we can implement a long-term fix that makes use of ZOOKEEPER-2139. There is already similar working code in Hive. (See the LlapZookeeperRegistryImpl class.) I know Sergey was able to demonstrate that this fix works through manual testing.
asuresh, are you interested in reviewing this? I'll give it some time before I consider committing.
Thanks for the patch sershe.. it looks straighforward and I understand unit testing this is non-trivial.
Can you also modify ZKSignerSecretProvider to include this fix as well ? since ZKDTSM and the signerSecretProvider are generally used in conjunction.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 22s | Docker mode activated. |
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
-1 | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
+1 | mvninstall | 6m 53s | trunk passed |
+1 | compile | 7m 2s | trunk passed |
+1 | checkstyle | 0m 22s | trunk passed |
+1 | mvnsite | 0m 58s | trunk passed |
+1 | mvneclipse | 0m 13s | trunk passed |
+1 | findbugs | 1m 20s | trunk passed |
+1 | javadoc | 0m 45s | trunk passed |
+1 | mvninstall | 0m 38s | the patch passed |
+1 | compile | 6m 42s | the patch passed |
+1 | javac | 6m 42s | the patch passed |
-0 | checkstyle | 0m 24s | hadoop-common-project/hadoop-common: The patch generated 4 new + 10 unchanged - 0 fixed = 14 total (was 10) |
+1 | mvnsite | 0m 53s | the patch passed |
+1 | mvneclipse | 0m 13s | the patch passed |
-1 | whitespace | 0m 0s | The patch 3 line(s) with tabs. |
+1 | findbugs | 1m 33s | the patch passed |
+1 | javadoc | 0m 48s | the patch passed |
+1 | unit | 8m 21s | hadoop-common in the patch passed. |
+1 | asflicense | 0m 21s | The patch does not generate ASF License warnings. |
39m 12s |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:9560f25 |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12820041/HADOOP-13422.patch |
JIRA Issue | |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux 208b135301e2 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / 703fdf8 |
Default Java | 1.8.0_101 |
findbugs | v3.0.0 |
checkstyle | https://builds.apache.org/job/PreCommit-HADOOP-Build/10079/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt |
whitespace | https://builds.apache.org/job/PreCommit-HADOOP-Build/10079/artifact/patchprocess/whitespace-tabs.txt |
Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/10079/testReport/ |
modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/10079/console |
Powered by | Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 22s | Docker mode activated. |
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
-1 | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
0 | mvndep | 0m 13s | Maven dependency ordering for branch |
+1 | mvninstall | 14m 4s | trunk passed |
+1 | compile | 14m 23s | trunk passed |
+1 | checkstyle | 0m 46s | trunk passed |
+1 | mvnsite | 2m 14s | trunk passed |
+1 | mvneclipse | 0m 40s | trunk passed |
+1 | findbugs | 3m 5s | trunk passed |
+1 | javadoc | 1m 33s | trunk passed |
0 | mvndep | 0m 10s | Maven dependency ordering for patch |
+1 | mvninstall | 1m 10s | the patch passed |
+1 | compile | 9m 59s | the patch passed |
+1 | javac | 9m 59s | the patch passed |
-0 | checkstyle | 0m 35s | hadoop-common-project: The patch generated 2 new + 24 unchanged - 0 fixed = 26 total (was 24) |
+1 | mvnsite | 1m 43s | the patch passed |
+1 | mvneclipse | 0m 41s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | findbugs | 3m 51s | the patch passed |
+1 | javadoc | 1m 15s | the patch passed |
+1 | unit | 3m 37s | hadoop-auth in the patch passed. |
-1 | unit | 11m 37s | hadoop-common in the patch failed. |
+1 | asflicense | 0m 31s | The patch does not generate ASF License warnings. |
77m 18s |
Reason | Tests |
---|---|
Failed junit tests | hadoop.security.TestGroupsCaching |
hadoop.metrics2.impl.TestGangliaMetrics |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:9560f25 |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12820274/HADOOP-13422.01.patch |
JIRA Issue | |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux 0455f7d213d2 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / d2cf8b5 |
Default Java | 1.8.0_101 |
findbugs | v3.0.0 |
checkstyle | https://builds.apache.org/job/PreCommit-HADOOP-Build/10090/artifact/patchprocess/diff-checkstyle-hadoop-common-project.txt |
unit | https://builds.apache.org/job/PreCommit-HADOOP-Build/10090/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt |
Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/10090/testReport/ |
modules | C: hadoop-common-project/hadoop-auth hadoop-common-project/hadoop-common U: hadoop-common-project |
Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/10090/console |
Powered by | Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
+1 for the patch. The test failures from the last pre-commit run were unrelated. Checkstyle flagged some warnings about indentation level, which I can clean up trivially at commit time. I will commit this soon.
I have committed this to trunk, branch-2 and branch-2.8. sershe, thank you for the patch. asuresh, thank you for reviewing and pointing out the similar issue in ZKSignerSecretProvider.
SUCCESS: Integrated in Hadoop-trunk-Commit #10157 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10157/)
HADOOP-13422. ZKDelegationTokenSecretManager JaasConfig does not work (cnauroth: rev 255ea45e50e102505ee61eb0ba45ea93035abe3c)
- hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/ZKDelegationTokenSecretManager.java
- hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java
The initial patch.