You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2020/07/30 08:38:51 UTC
[GitHub] [hadoop] sguggilam opened a new pull request #2178: HADOOP-17164 UGI loginUserFromKeytab doesn't set the last login time
sguggilam opened a new pull request #2178:
URL: https://github.com/apache/hadoop/pull/2178
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #2178: HADOOP-17164 UGI loginUserFromKeytab doesn't set the last login time
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2178:
URL: https://github.com/apache/hadoop/pull/2178#issuecomment-666961481
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 20m 5s | trunk passed |
| +1 :green_heart: | compile | 21m 50s | trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 |
| +1 :green_heart: | compile | 18m 11s | trunk passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 |
| +1 :green_heart: | checkstyle | 0m 49s | trunk passed |
| +1 :green_heart: | mvnsite | 1m 26s | trunk passed |
| +1 :green_heart: | shadedclient | 16m 21s | branch has no errors when building and testing our client artifacts. |
| -1 :x: | javadoc | 0m 38s | hadoop-common in trunk failed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1. |
| +1 :green_heart: | javadoc | 0m 57s | trunk passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 |
| +0 :ok: | spotbugs | 2m 17s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 2m 14s | trunk passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 54s | the patch passed |
| +1 :green_heart: | compile | 20m 57s | the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 |
| +1 :green_heart: | javac | 20m 57s | the patch passed |
| +1 :green_heart: | compile | 18m 2s | the patch passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 |
| +1 :green_heart: | javac | 18m 2s | the patch passed |
| -0 :warning: | checkstyle | 0m 48s | hadoop-common-project/hadoop-common: The patch generated 2 new + 83 unchanged - 0 fixed = 85 total (was 83) |
| +1 :green_heart: | mvnsite | 1m 24s | the patch passed |
| -1 :x: | whitespace | 0m 0s | The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| +1 :green_heart: | shadedclient | 14m 31s | patch has no errors when building and testing our client artifacts. |
| -1 :x: | javadoc | 0m 39s | hadoop-common in the patch failed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1. |
| +1 :green_heart: | javadoc | 0m 59s | the patch passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 |
| +1 :green_heart: | findbugs | 2m 24s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 10m 1s | hadoop-common in the patch passed. |
| +1 :green_heart: | asflicense | 0m 47s | The patch does not generate ASF License warnings. |
| | | 156m 41s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2178/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/2178 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux f5be6c9452a0 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/hadoop.sh |
| git revision | trunk / 05b3337a460 |
| Default Java | Private Build-1.8.0_252-8u252-b09-1~18.04-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_252-8u252-b09-1~18.04-b09 |
| javadoc | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2178/1/artifact/out/branch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1.txt |
| checkstyle | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2178/1/artifact/out/diff-checkstyle-hadoop-common-project_hadoop-common.txt |
| whitespace | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2178/1/artifact/out/whitespace-eol.txt |
| javadoc | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2178/1/artifact/out/patch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1.txt |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2178/1/testReport/ |
| Max. process+thread count | 2718 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2178/1/console |
| versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] sguggilam commented on a change in pull request #2178: HADOOP-17164 UGI loginUserFromKeytab doesn't set the last login time
Posted by GitBox <gi...@apache.org>.
sguggilam commented on a change in pull request #2178:
URL: https://github.com/apache/hadoop/pull/2178#discussion_r463345150
##########
File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUGILoginFromKeytab.java
##########
@@ -100,13 +101,34 @@ public void stopMiniKdc() {
executor.shutdownNow();
}
}
+
+ /**
+ * Login from keytab using the MiniKDC
+ */
+ @Test
+ public void testUGILoginFromKeytab() throws Exception {
+ long beforeLogin = Time.now();
+ String principal = "foo";
+ File keytab = new File(workDir, "foo.keytab");
+ kdc.createPrincipal(keytab, principal);
+
+ UserGroupInformation.loginUserFromKeytab(principal, keytab.getPath());
+ UserGroupInformation ugi = UserGroupInformation.getLoginUser();
+ Assert.assertTrue("UGI should be configured to login from keytab",
+ ugi.isFromKeytab());
+
+ User user = getUser(ugi.getSubject());
+ Assert.assertNotNull(user.getLogin());
+ Assert.assertTrue("User last login time is not set correctly",
Review comment:
Agreed, updated the code
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] liuml07 commented on a change in pull request #2178: HADOOP-17164 UGI loginUserFromKeytab doesn't set the last login time
Posted by GitBox <gi...@apache.org>.
liuml07 commented on a change in pull request #2178:
URL: https://github.com/apache/hadoop/pull/2178#discussion_r463304087
##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
##########
@@ -528,6 +528,10 @@ private HadoopLoginContext getLogin() {
private void setLogin(LoginContext login) {
user.setLogin(login);
}
+
+ private void setLastLogin(long now) {
Review comment:
Make `@param time` and `long now` the same variable name? e.g. name it `loginTime`
##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
##########
@@ -1968,6 +1976,7 @@ private static UserGroupInformation doSubjectLogin(
if (subject == null) {
params.put(LoginParam.PRINCIPAL, ugi.getUserName());
ugi.setLogin(login);
+ ugi.setLastLogin(Time.now());
Review comment:
the `user::lastLogin` was the last attempt to login. I'm not sure if we should put this before the `try` clause (aka before L1969)?
CC: @steveloughran
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] sguggilam commented on a change in pull request #2178: HADOOP-17164 UGI loginUserFromKeytab doesn't set the last login time
Posted by GitBox <gi...@apache.org>.
sguggilam commented on a change in pull request #2178:
URL: https://github.com/apache/hadoop/pull/2178#discussion_r463312777
##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
##########
@@ -1968,6 +1976,7 @@ private static UserGroupInformation doSubjectLogin(
if (subject == null) {
params.put(LoginParam.PRINCIPAL, ugi.getUserName());
ugi.setLogin(login);
+ ugi.setLastLogin(Time.now());
Review comment:
@liuml07 We should set this only after the login is successful right ? If we add it before the try, then we might update it even in case of unsuccessful login in which case a retry should be permitted immediately. Please let me know your thoughts
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] sguggilam commented on a change in pull request #2178: HADOOP-17164 UGI loginUserFromKeytab doesn't set the last login time
Posted by GitBox <gi...@apache.org>.
sguggilam commented on a change in pull request #2178:
URL: https://github.com/apache/hadoop/pull/2178#discussion_r463345248
##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
##########
@@ -528,6 +528,10 @@ private HadoopLoginContext getLogin() {
private void setLogin(LoginContext login) {
user.setLogin(login);
}
+
+ private void setLastLogin(long now) {
Review comment:
Yes agreed, made the change
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] liuml07 commented on a change in pull request #2178: HADOOP-17164 UGI loginUserFromKeytab doesn't set the last login time
Posted by GitBox <gi...@apache.org>.
liuml07 commented on a change in pull request #2178:
URL: https://github.com/apache/hadoop/pull/2178#discussion_r463308364
##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
##########
@@ -1968,6 +1976,7 @@ private static UserGroupInformation doSubjectLogin(
if (subject == null) {
params.put(LoginParam.PRINCIPAL, ugi.getUserName());
ugi.setLogin(login);
+ ugi.setLastLogin(Time.now());
Review comment:
the `user::lastLogin` was the last attempt to login. I'm not sure if we should put this before the `try` clause (aka before new L1969)?
CC: @steveloughran
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] sguggilam commented on a change in pull request #2178: HADOOP-17164 UGI loginUserFromKeytab doesn't set the last login time
Posted by GitBox <gi...@apache.org>.
sguggilam commented on a change in pull request #2178:
URL: https://github.com/apache/hadoop/pull/2178#discussion_r463345184
##########
File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUGILoginFromKeytab.java
##########
@@ -121,6 +143,9 @@ public void testUGILoginFromKeytab() throws Exception {
final long firstLogin = user.getLastLogin();
final LoginContext login1 = user.getLogin();
Assert.assertNotNull(login1);
+
+ // Sleep for 1 sec to have a difference between first and second login
+ Thread.sleep(1000);
Review comment:
Makes sense, changed it to 2 sec
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a change in pull request #2178: HADOOP-17164 UGI loginUserFromKeytab doesn't set the last login time
Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #2178:
URL: https://github.com/apache/hadoop/pull/2178#discussion_r463232170
##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
##########
@@ -528,6 +528,10 @@ private HadoopLoginContext getLogin() {
private void setLogin(LoginContext login) {
user.setLogin(login);
}
+
+ private void setLastLogin(long now) {
Review comment:
add javadoc, note unit of time. Milliseconds, right?
##########
File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUGILoginFromKeytab.java
##########
@@ -100,13 +101,34 @@ public void stopMiniKdc() {
executor.shutdownNow();
}
}
+
+ /**
+ * Login from keytab using the MiniKDC
+ */
+ @Test
+ public void testUGILoginFromKeytab() throws Exception {
+ long beforeLogin = Time.now();
+ String principal = "foo";
+ File keytab = new File(workDir, "foo.keytab");
+ kdc.createPrincipal(keytab, principal);
+
+ UserGroupInformation.loginUserFromKeytab(principal, keytab.getPath());
+ UserGroupInformation ugi = UserGroupInformation.getLoginUser();
+ Assert.assertTrue("UGI should be configured to login from keytab",
+ ugi.isFromKeytab());
+
+ User user = getUser(ugi.getSubject());
+ Assert.assertNotNull(user.getLogin());
+ Assert.assertTrue("User last login time is not set correctly",
Review comment:
error text should include the values at fault, so we can debug better from nothing but a junit report
##########
File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUGILoginFromKeytab.java
##########
@@ -121,6 +143,9 @@ public void testUGILoginFromKeytab() throws Exception {
final long firstLogin = user.getLastLogin();
final LoginContext login1 = user.getLogin();
Assert.assertNotNull(login1);
+
+ // Sleep for 1 sec to have a difference between first and second login
+ Thread.sleep(1000);
Review comment:
if you really want >1s of interval, make it a 2s, delay. There's too much risk of clock/sleep granularity issues
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] liuml07 merged pull request #2178: HADOOP-17164 UGI loginUserFromKeytab doesn't set the last login time
Posted by GitBox <gi...@apache.org>.
liuml07 merged pull request #2178:
URL: https://github.com/apache/hadoop/pull/2178
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org