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/03/02 22:20:53 UTC
[GitHub] [hadoop] bilaharith opened a new pull request #1872: Hadoop 16890:
Change in expiry calculation for MSI token provider
bilaharith opened a new pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872
## NOTICE
Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HADOOP-XXXXX. Fix a typo in YYY.)
For more details, please see https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r389574936
##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/AzureADAuthenticator.java
##########
@@ -408,17 +409,30 @@ private static AzureADToken parseTokenFromStream(InputStream httpResponseStream)
if (fieldName.equals("access_token")) {
token.setAccessToken(fieldValue);
}
+
if (fieldName.equals("expires_in")) {
- expiryPeriod = Integer.parseInt(fieldValue);
+ expiryPeriodInSecs = Integer.parseInt(fieldValue);
+ }
+
+ if (fieldName.equals("expires_on")) {
+ expiresOnInSecs = Long.parseLong(fieldValue);
}
+
}
jp.nextToken();
}
jp.close();
- long expiry = System.currentTimeMillis();
- expiry = expiry + expiryPeriod * 1000L; // convert expiryPeriod to milliseconds and add
- token.setExpiry(new Date(expiry));
- LOG.debug("AADToken: fetched token with expiry " + token.getExpiry().toString());
+ if (expiresOnInSecs > 0) {
+ token.setExpiry(new Date(expiresOnInSecs * 1000));
+ } else {
+ long expiry = System.currentTimeMillis();
+ expiry = expiry + expiryPeriodInSecs
+ * 1000L; // convert expiryPeriod to milliseconds and add
Review comment:
put expiryPeriod and * 1000 on same line. Also, feel free to use the new java time classes which are complex but powerful once you start to use them
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 removed a comment on issue #1872: Hadoop
16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus removed a comment on issue #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#issuecomment-593699941
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 36s | 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 2 new or modified test files. |
||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 1m 11s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 18m 46s | trunk passed |
| +1 :green_heart: | compile | 17m 0s | trunk passed |
| +1 :green_heart: | checkstyle | 2m 37s | trunk passed |
| +1 :green_heart: | mvnsite | 1m 21s | trunk passed |
| +1 :green_heart: | shadedclient | 19m 40s | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 13s | trunk passed |
| +0 :ok: | spotbugs | 1m 3s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +0 :ok: | findbugs | 0m 33s | branch/hadoop-project no findbugs output file (findbugsXml.xml) |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 30s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 41s | the patch passed |
| +1 :green_heart: | compile | 16m 14s | the patch passed |
| +1 :green_heart: | javac | 16m 14s | the patch passed |
| -0 :warning: | checkstyle | 2m 36s | root: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | mvnsite | 1m 19s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 3s | The patch has no ill-formed XML file. |
| +1 :green_heart: | shadedclient | 14m 15s | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 16s | the patch passed |
| +0 :ok: | findbugs | 0m 30s | hadoop-project has no data from findbugs |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 25s | hadoop-project in the patch passed. |
| +1 :green_heart: | unit | 1m 33s | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 49s | The patch does not generate ASF License warnings. |
| | | 106m 19s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/1872 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle |
| uname | Linux 9795022f8637 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 / edc2e9d |
| Default Java | 1.8.0_242 |
| checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/1/artifact/out/diff-checkstyle-root.txt |
| Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/1/testReport/ |
| Max. process+thread count | 452 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-tools/hadoop-azure U: . |
| Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/1/console |
| versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
| Powered by | Apache Yetus 0.11.1 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] snvijaya commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
snvijaya commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r386855466
##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsMsiTokenProvider.java
##########
@@ -0,0 +1,91 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.azurebfs;
+
Review comment:
Order the non-static imports in the order:
- java*
- any non org.apache imports
- org.apache imports
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r389575291
##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/MsiTokenProvider.java
##########
@@ -51,6 +55,35 @@ protected AzureADToken refreshToken() throws IOException {
LOG.debug("AADToken: refreshing token from MSI");
AzureADToken token = AzureADAuthenticator
.getTokenFromMsi(authEndpoint, tenantGuid, clientId, authority, false);
+ tokenFetchTime = System.currentTimeMillis();
return token;
}
+
+ /**
+ * Checks if the token is about to expire as per base expiry logic.
+ * Otherwise try to expire every 1 hour
+ *
+ * @return true if the token is expiring in next 1 hour
Review comment:
or if a token has never been fetched
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] DadanielZ commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
DadanielZ commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r387859502
##########
File path: hadoop-tools/hadoop-azure/pom.xml
##########
@@ -290,13 +290,13 @@
<groupId>org.powermock</groupId>
<artifactId>powermock-api-mockito2</artifactId>
<scope>test</scope>
- <version>${powermock.version}</version>
+ <!--<version>${powermock.version}</version>-->
Review comment:
same here
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] bilaharith commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
bilaharith commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r389902495
##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsMsiTokenProvider.java
##########
@@ -0,0 +1,93 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.azurebfs;
+
+import org.apache.commons.lang3.StringUtils;
Review comment:
Done :)
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] DadanielZ commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
DadanielZ commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r387399658
##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/AzureADAuthenticator.java
##########
@@ -408,17 +409,30 @@ private static AzureADToken parseTokenFromStream(InputStream httpResponseStream)
if (fieldName.equals("access_token")) {
token.setAccessToken(fieldValue);
}
+
if (fieldName.equals("expires_in")) {
- expiryPeriod = Integer.parseInt(fieldValue);
+ expiryPeriodInSecs = Integer.parseInt(fieldValue);
+ }
+
+ if (fieldName.equals("expires_on")) {
+ expiresOnInSecs = Long.parseLong(fieldValue);
}
+
}
jp.nextToken();
}
jp.close();
- long expiry = System.currentTimeMillis();
- expiry = expiry + expiryPeriod * 1000L; // convert expiryPeriod to milliseconds and add
- token.setExpiry(new Date(expiry));
- LOG.debug("AADToken: fetched token with expiry " + token.getExpiry().toString());
+ if (expiresOnInSecs > -1) {
Review comment:
I suppose 0 is not a valid value for expiresOnInSecs, do we need to add value check here?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] snvijaya commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
snvijaya commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r386848787
##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/AzureADAuthenticator.java
##########
@@ -408,17 +409,29 @@ private static AzureADToken parseTokenFromStream(InputStream httpResponseStream)
if (fieldName.equals("access_token")) {
token.setAccessToken(fieldValue);
}
+
if (fieldName.equals("expires_in")) {
- expiryPeriod = Integer.parseInt(fieldValue);
+ expiryPeriodInSecs = Integer.parseInt(fieldValue);
+ }
+
+ if (fieldName.equals("expires_on")) {
+ expiresOnInSecs = Long.parseLong(fieldValue);
}
+
}
jp.nextToken();
}
jp.close();
- long expiry = System.currentTimeMillis();
- expiry = expiry + expiryPeriod * 1000L; // convert expiryPeriod to milliseconds and add
- token.setExpiry(new Date(expiry));
- LOG.debug("AADToken: fetched token with expiry " + token.getExpiry().toString());
+ if (expiresOnInSecs > -1) {
+ token.setExpiry(new Date(expiresOnInSecs * 1000));
+ } else {
+ long expiry = System.currentTimeMillis();
Review comment:
If expiresOnInSecs is supposed to be the field to rely on and has been returned -1, why not error out in else ?
Why is it that we are defaulting to value in expires_in which is not reliable ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] snvijaya commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
snvijaya commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r390082763
##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsMsiTokenProvider.java
##########
@@ -0,0 +1,95 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.azurebfs;
+
+
Review comment:
Extra new line.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #1872: Hadoop 16890:
Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#issuecomment-593699941
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 36s | 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 2 new or modified test files. |
||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 1m 11s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 18m 46s | trunk passed |
| +1 :green_heart: | compile | 17m 0s | trunk passed |
| +1 :green_heart: | checkstyle | 2m 37s | trunk passed |
| +1 :green_heart: | mvnsite | 1m 21s | trunk passed |
| +1 :green_heart: | shadedclient | 19m 40s | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 13s | trunk passed |
| +0 :ok: | spotbugs | 1m 3s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +0 :ok: | findbugs | 0m 33s | branch/hadoop-project no findbugs output file (findbugsXml.xml) |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 30s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 41s | the patch passed |
| +1 :green_heart: | compile | 16m 14s | the patch passed |
| +1 :green_heart: | javac | 16m 14s | the patch passed |
| -0 :warning: | checkstyle | 2m 36s | root: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | mvnsite | 1m 19s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 3s | The patch has no ill-formed XML file. |
| +1 :green_heart: | shadedclient | 14m 15s | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 16s | the patch passed |
| +0 :ok: | findbugs | 0m 30s | hadoop-project has no data from findbugs |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 25s | hadoop-project in the patch passed. |
| +1 :green_heart: | unit | 1m 33s | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 49s | The patch does not generate ASF License warnings. |
| | | 106m 19s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/1872 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle |
| uname | Linux 9795022f8637 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 / edc2e9d |
| Default Java | 1.8.0_242 |
| checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/1/artifact/out/diff-checkstyle-root.txt |
| Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/1/testReport/ |
| Max. process+thread count | 452 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-tools/hadoop-azure U: . |
| Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/1/console |
| versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
| Powered by | Apache Yetus 0.11.1 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] DadanielZ commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
DadanielZ commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r387397768
##########
File path: hadoop-tools/hadoop-azure/pom.xml
##########
@@ -45,6 +45,7 @@
<fs.azure.scale.test.timeout>7200</fs.azure.scale.test.timeout>
<fs.azure.scale.test.list.performance.threads>10</fs.azure.scale.test.list.performance.threads>
<fs.azure.scale.test.list.performance.files>1000</fs.azure.scale.test.list.performance.files>
+ <powermock.version>2.0.4</powermock.version>
Review comment:
the version need to go to hadoop-project pom file
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] DadanielZ commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
DadanielZ commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r387407889
##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/MsiTokenProvider.java
##########
@@ -51,6 +55,35 @@ protected AzureADToken refreshToken() throws IOException {
LOG.debug("AADToken: refreshing token from MSI");
AzureADToken token = AzureADAuthenticator
.getTokenFromMsi(authEndpoint, tenantGuid, clientId, authority, false);
+ tokenFetchTime = System.currentTimeMillis();
return token;
}
+
+ /**
+ * Checks if the token is about to expire as per base expiry logic.
+ * Otherwise try to expire every 1 hour
+ *
+ * @return true if the token is expiring in next 5 minutes
Review comment:
need to update this `@return` documentation too.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] snvijaya commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
snvijaya commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r386859869
##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/unittests/TestMsiTokenProvider.java
##########
@@ -0,0 +1,102 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.fs.azurebfs.unittests;
+
+import org.apache.hadoop.fs.azurebfs.oauth2.AccessTokenProvider;
+import org.apache.hadoop.fs.azurebfs.oauth2.AzureADAuthenticator;
+import org.apache.hadoop.fs.azurebfs.oauth2.AzureADToken;
+import org.apache.hadoop.fs.azurebfs.oauth2.MsiTokenProvider;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import java.io.IOException;
+import java.util.Date;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.lessThanOrEqualTo;
+import static org.hamcrest.core.AllOf.allOf;
+import static org.hamcrest.core.Is.is;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.when;
+import static org.powermock.api.mockito.PowerMockito.mockStatic;
+import static org.powermock.reflect.Whitebox.getInternalState;
+import static org.powermock.reflect.Whitebox.invokeMethod;
+import static org.powermock.reflect.Whitebox.setInternalState;
+
+/**
+ * Unit test for MsiTokenProvider
+ */
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(AzureADAuthenticator.class)
+public class TestMsiTokenProvider {
+
+ private static final long ONE_HOUR = 3600 * 1000;
+ private static final long TWO_HOUR = ONE_HOUR * 2;
+
+ @Test
+ public void testMsiTokenProvider() throws Exception {
+ String testToken = "TEST_TOKEN1";
+ setMockAzureADAuthenticator(testToken,
+ System.currentTimeMillis() + TWO_HOUR);
+
+ AccessTokenProvider msiTokenProvider = new MsiTokenProvider("", "", "", "");
+ long tokenFetchTime = getInternalState(msiTokenProvider, "tokenFetchTime");
+ Assert.assertEquals(-1, tokenFetchTime);
+
+ long before = System.currentTimeMillis();
+ AzureADToken token = msiTokenProvider.getToken();
+ long after = System.currentTimeMillis();
+ long newTokenFetchTime = getInternalState(msiTokenProvider,
+ "tokenFetchTime");
+ assertThat(newTokenFetchTime,
+ is(allOf(greaterThan(before), lessThanOrEqualTo(after))));
+ assertThat(token.getAccessToken(), is(equalTo(testToken)));
+
+ assertThat(invokeMethod(msiTokenProvider, "isTokenAboutToExpire"),
+ is(false));
+
+ setInternalState(msiTokenProvider, "tokenFetchTime",
+ System.currentTimeMillis() - ONE_HOUR);
+ assertThat(invokeMethod(msiTokenProvider, "isTokenAboutToExpire"),
+ is(true));
+
+ setInternalState(msiTokenProvider, "tokenFetchTime",
+ System.currentTimeMillis() + 2 - ONE_HOUR);
+ assertThat(invokeMethod(msiTokenProvider, "isTokenAboutToExpire"),
+ is(false));
+ }
+
+ private AzureADToken setMockAzureADAuthenticator(String tokenStr, long expiry)
+ throws IOException {
+ AzureADToken token = new AzureADToken();
+ token.setAccessToken(tokenStr);
+ token.setExpiry(new Date(expiry));
+ mockStatic(AzureADAuthenticator.class);
+ when(AzureADAuthenticator
+ .getTokenFromMsi(Mockito.anyString(), Mockito.anyString(),
Review comment:
With this mock the code which parses the response and sets the expiry from expires_on will not be hit ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] snvijaya commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
snvijaya commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r386856010
##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsMsiTokenProvider.java
##########
@@ -0,0 +1,91 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.azurebfs;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.fs.azurebfs.oauth2.AccessTokenProvider;
+import org.apache.hadoop.fs.azurebfs.oauth2.AzureADToken;
+import org.apache.hadoop.fs.azurebfs.oauth2.MsiTokenProvider;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.Date;
+
+import static org.apache.hadoop.fs.azurebfs.constants.AuthConfigurations.DEFAULT_FS_AZURE_ACCOUNT_OAUTH_MSI_AUTHORITY;
+import static org.apache.hadoop.fs.azurebfs.constants.AuthConfigurations.DEFAULT_FS_AZURE_ACCOUNT_OAUTH_MSI_ENDPOINT;
+import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_OAUTH_CLIENT_ID;
+import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_OAUTH_MSI_AUTHORITY;
+import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_OAUTH_MSI_ENDPOINT;
+import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_OAUTH_MSI_TENANT;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.Matchers.isEmptyOrNullString;
+import static org.hamcrest.Matchers.isEmptyString;
+import static org.junit.Assume.assumeThat;
+
+/**
+ * Test MsiTokenProvider
Review comment:
It seems there are javadoc compilers issues with description comments without a dot at end. Please add here and at places such comments are added.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] bilaharith commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
bilaharith commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r389901643
##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/AzureADAuthenticator.java
##########
@@ -408,17 +409,30 @@ private static AzureADToken parseTokenFromStream(InputStream httpResponseStream)
if (fieldName.equals("access_token")) {
token.setAccessToken(fieldValue);
}
+
if (fieldName.equals("expires_in")) {
Review comment:
content type check is done at the method from where this method is called.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r391253397
##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/MsiTokenProvider.java
##########
@@ -36,6 +36,10 @@
private final String clientId;
+ private long tokenFetchTime = -1;
+
+ private static final long ONE_HOUR = 3600 * 1000;
Review comment:
better: 3600_1000
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #1872: Hadoop 16890:
Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#issuecomment-594653639
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 37s | 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 2 new or modified test files. |
||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 1m 14s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 21m 10s | trunk passed |
| +1 :green_heart: | compile | 17m 18s | trunk passed |
| +1 :green_heart: | checkstyle | 2m 38s | trunk passed |
| +1 :green_heart: | mvnsite | 1m 20s | trunk passed |
| +1 :green_heart: | shadedclient | 19m 13s | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 12s | trunk passed |
| +0 :ok: | spotbugs | 1m 3s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +0 :ok: | findbugs | 0m 34s | branch/hadoop-project no findbugs output file (findbugsXml.xml) |
| -0 :warning: | patch | 1m 29s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 31s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 42s | the patch passed |
| +1 :green_heart: | compile | 16m 9s | the patch passed |
| +1 :green_heart: | javac | 16m 9s | the patch passed |
| -0 :warning: | checkstyle | 2m 42s | root: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | mvnsite | 1m 22s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 3s | The patch has no ill-formed XML file. |
| +1 :green_heart: | shadedclient | 14m 13s | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 14s | the patch passed |
| +0 :ok: | findbugs | 0m 31s | hadoop-project has no data from findbugs |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 32s | hadoop-project in the patch passed. |
| +1 :green_heart: | unit | 1m 37s | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 54s | The patch does not generate ASF License warnings. |
| | | 109m 16s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.7 Server=19.03.7 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/1872 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle |
| uname | Linux 5a028e078956 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/hadoop.sh |
| git revision | trunk / bbd704b |
| Default Java | 1.8.0_242 |
| checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/4/artifact/out/diff-checkstyle-root.txt |
| Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/4/testReport/ |
| Max. process+thread count | 456 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-tools/hadoop-azure U: . |
| Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/4/console |
| versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
| Powered by | Apache Yetus 0.11.1 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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #1872: Hadoop 16890:
Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#issuecomment-593691330
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 26s | 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 2 new or modified test files. |
||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 21m 12s | trunk passed |
| +1 :green_heart: | compile | 0m 27s | trunk passed |
| +1 :green_heart: | checkstyle | 0m 18s | trunk passed |
| +1 :green_heart: | mvnsite | 0m 30s | trunk passed |
| +1 :green_heart: | shadedclient | 15m 57s | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | trunk passed |
| +0 :ok: | spotbugs | 0m 49s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 0m 48s | trunk passed |
| -0 :warning: | patch | 1m 5s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 26s | the patch passed |
| +1 :green_heart: | compile | 0m 21s | the patch passed |
| +1 :green_heart: | javac | 0m 21s | the patch passed |
| -0 :warning: | checkstyle | 0m 14s | hadoop-tools/hadoop-azure: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | mvnsite | 0m 24s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 1s | The patch has no ill-formed XML file. |
| +1 :green_heart: | shadedclient | 15m 17s | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 19s | the patch passed |
| +1 :green_heart: | findbugs | 0m 52s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 23s | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 27s | The patch does not generate ASF License warnings. |
| | | 61m 14s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/1872 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle |
| uname | Linux 6e61497f53c4 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/hadoop.sh |
| git revision | trunk / edc2e9d |
| Default Java | 1.8.0_242 |
| checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/2/artifact/out/diff-checkstyle-hadoop-tools_hadoop-azure.txt |
| Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/2/testReport/ |
| Max. process+thread count | 307 (vs. ulimit of 5500) |
| modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
| Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/2/console |
| versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
| Powered by | Apache Yetus 0.11.1 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] snvijaya commented on issue #1872: Hadoop 16890: Change in
expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
snvijaya commented on issue #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#issuecomment-593827526
In the test results pasted, there is failure for a testcase which is not related to this PR. Create a JIRA to track and fix test issue.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] bilaharith commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
bilaharith commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r389390848
##########
File path: hadoop-project/pom.xml
##########
@@ -204,6 +204,7 @@
<assertj.version>3.12.2</assertj.version>
<jline.version>3.9.0</jline.version>
<powermock.version>1.5.6</powermock.version>
Review comment:
Removing powermock for now, will raise another PR for powermock upgrade and then one for the mocktest
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #1872: Hadoop 16890:
Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#issuecomment-594120524
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 35m 13s | 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 2 new or modified test files. |
||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 25m 54s | trunk passed |
| +1 :green_heart: | compile | 0m 37s | trunk passed |
| +1 :green_heart: | checkstyle | 0m 26s | trunk passed |
| +1 :green_heart: | mvnsite | 0m 42s | trunk passed |
| +1 :green_heart: | shadedclient | 20m 41s | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 32s | trunk passed |
| +0 :ok: | spotbugs | 1m 9s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 1m 7s | trunk passed |
| -0 :warning: | patch | 1m 29s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 40s | the patch passed |
| +1 :green_heart: | compile | 0m 28s | the patch passed |
| +1 :green_heart: | javac | 0m 28s | the patch passed |
| -0 :warning: | checkstyle | 0m 19s | hadoop-tools/hadoop-azure: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | mvnsite | 0m 32s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 1s | The patch has no ill-formed XML file. |
| +1 :green_heart: | shadedclient | 18m 50s | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 27s | the patch passed |
| +1 :green_heart: | findbugs | 1m 7s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 37s | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 44s | The patch does not generate ASF License warnings. |
| | | 111m 33s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/1872 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle |
| uname | Linux 296de404716a 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/hadoop.sh |
| git revision | trunk / d0a7c79 |
| Default Java | 1.8.0_242 |
| checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/3/artifact/out/diff-checkstyle-hadoop-tools_hadoop-azure.txt |
| Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/3/testReport/ |
| Max. process+thread count | 308 (vs. ulimit of 5500) |
| modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
| Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/3/console |
| versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
| Powered by | Apache Yetus 0.11.1 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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r391253178
##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/AzureADAuthenticator.java
##########
@@ -408,17 +409,29 @@ private static AzureADToken parseTokenFromStream(InputStream httpResponseStream)
if (fieldName.equals("access_token")) {
token.setAccessToken(fieldValue);
}
+
if (fieldName.equals("expires_in")) {
- expiryPeriod = Integer.parseInt(fieldValue);
+ expiryPeriodInSecs = Integer.parseInt(fieldValue);
+ }
+
+ if (fieldName.equals("expires_on")) {
+ expiresOnInSecs = Long.parseLong(fieldValue);
}
+
}
jp.nextToken();
}
jp.close();
- long expiry = System.currentTimeMillis();
- expiry = expiry + expiryPeriod * 1000L; // convert expiryPeriod to milliseconds and add
- token.setExpiry(new Date(expiry));
- LOG.debug("AADToken: fetched token with expiry " + token.getExpiry().toString());
+ if (expiresOnInSecs > -1) {
+ token.setExpiry(new Date(expiresOnInSecs * 1000));
+ } else {
+ long expiry = System.currentTimeMillis();
Review comment:
it'd be good to have some test JSONs here for real-world responses
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran merged pull request #1872: Hadoop 16890:
Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
steveloughran merged pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #1872: Hadoop 16890:
Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#issuecomment-595988510
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | 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 2 new or modified test files. |
||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 1m 19s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 23m 31s | trunk passed |
| +1 :green_heart: | compile | 20m 12s | trunk passed |
| +1 :green_heart: | checkstyle | 2m 50s | trunk passed |
| +1 :green_heart: | mvnsite | 1m 1s | trunk passed |
| +1 :green_heart: | shadedclient | 20m 29s | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 52s | trunk passed |
| +0 :ok: | spotbugs | 1m 3s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +0 :ok: | findbugs | 0m 25s | branch/hadoop-project no findbugs output file (findbugsXml.xml) |
| -0 :warning: | patch | 1m 23s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 25s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 39s | the patch passed |
| -1 :x: | compile | 11m 0s | root in the patch failed. |
| -1 :x: | javac | 11m 0s | root in the patch failed. |
| -0 :warning: | checkstyle | 2m 38s | root: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | mvnsite | 0m 49s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 3s | The patch has no ill-formed XML file. |
| +1 :green_heart: | shadedclient | 16m 57s | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 13s | the patch passed |
| +0 :ok: | findbugs | 0m 29s | hadoop-project has no data from findbugs |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 23s | hadoop-project in the patch passed. |
| +1 :green_heart: | unit | 1m 31s | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 48s | The patch does not generate ASF License warnings. |
| | | 111m 20s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.7 Server=19.03.7 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/6/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/1872 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle |
| uname | Linux 16fce474ffaf 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/hadoop.sh |
| git revision | trunk / 4062217 |
| Default Java | 1.8.0_242 |
| compile | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/6/artifact/out/patch-compile-root.txt |
| javac | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/6/artifact/out/patch-compile-root.txt |
| checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/6/artifact/out/diff-checkstyle-root.txt |
| Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/6/testReport/ |
| Max. process+thread count | 313 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-tools/hadoop-azure U: . |
| Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/6/console |
| versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
| Powered by | Apache Yetus 0.11.1 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] snvijaya commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
snvijaya commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r386856316
##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/unittests/TestMsiTokenProvider.java
##########
@@ -0,0 +1,102 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.fs.azurebfs.unittests;
+
+import org.apache.hadoop.fs.azurebfs.oauth2.AccessTokenProvider;
Review comment:
Please refer to non-static import order mentioned in above comment.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] snvijaya commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
snvijaya commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r386848909
##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/AzureADAuthenticator.java
##########
@@ -408,17 +409,29 @@ private static AzureADToken parseTokenFromStream(InputStream httpResponseStream)
if (fieldName.equals("access_token")) {
token.setAccessToken(fieldValue);
}
+
if (fieldName.equals("expires_in")) {
- expiryPeriod = Integer.parseInt(fieldValue);
+ expiryPeriodInSecs = Integer.parseInt(fieldValue);
+ }
+
+ if (fieldName.equals("expires_on")) {
+ expiresOnInSecs = Long.parseLong(fieldValue);
}
+
}
jp.nextToken();
}
jp.close();
- long expiry = System.currentTimeMillis();
- expiry = expiry + expiryPeriod * 1000L; // convert expiryPeriod to milliseconds and add
- token.setExpiry(new Date(expiry));
- LOG.debug("AADToken: fetched token with expiry " + token.getExpiry().toString());
+ if (expiresOnInSecs > -1) {
+ token.setExpiry(new Date(expiresOnInSecs * 1000));
+ } else {
+ long expiry = System.currentTimeMillis();
+ expiry = expiry + expiryPeriodInSecs
+ * 1000L; // convert expiryPeriod to milliseconds and add
+ token.setExpiry(new Date(expiry));
+ }
+ LOG.debug("AADToken: fetched token with expiry {}, expiresOn passed: {}",
Review comment:
new line after if check block needed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] snvijaya commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
snvijaya commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r390082876
##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsMsiTokenProvider.java
##########
@@ -0,0 +1,95 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.azurebfs;
+
+
+import java.io.IOException;
+import java.util.Date;
+
+import org.junit.Test;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.fs.azurebfs.oauth2.AccessTokenProvider;
+import org.apache.hadoop.fs.azurebfs.oauth2.AzureADToken;
+import org.apache.hadoop.fs.azurebfs.oauth2.MsiTokenProvider;
+
+import static org.junit.Assume.assumeThat;
Review comment:
import ordering still not as expected.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] bilaharith commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
bilaharith commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r389904377
##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/AzureADAuthenticator.java
##########
@@ -408,17 +409,30 @@ private static AzureADToken parseTokenFromStream(InputStream httpResponseStream)
if (fieldName.equals("access_token")) {
token.setAccessToken(fieldValue);
}
+
if (fieldName.equals("expires_in")) {
- expiryPeriod = Integer.parseInt(fieldValue);
+ expiryPeriodInSecs = Integer.parseInt(fieldValue);
+ }
+
+ if (fieldName.equals("expires_on")) {
+ expiresOnInSecs = Long.parseLong(fieldValue);
}
+
}
jp.nextToken();
}
jp.close();
- long expiry = System.currentTimeMillis();
- expiry = expiry + expiryPeriod * 1000L; // convert expiryPeriod to milliseconds and add
- token.setExpiry(new Date(expiry));
- LOG.debug("AADToken: fetched token with expiry " + token.getExpiry().toString());
+ if (expiresOnInSecs > -1) {
Review comment:
Changed to expiresOnInSecs > 0, good to have check, for a safer side
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] bilaharith commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
bilaharith commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r389470135
##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/AzureADAuthenticator.java
##########
@@ -408,17 +409,29 @@ private static AzureADToken parseTokenFromStream(InputStream httpResponseStream)
if (fieldName.equals("access_token")) {
token.setAccessToken(fieldValue);
}
+
if (fieldName.equals("expires_in")) {
- expiryPeriod = Integer.parseInt(fieldValue);
+ expiryPeriodInSecs = Integer.parseInt(fieldValue);
+ }
+
+ if (fieldName.equals("expires_on")) {
+ expiresOnInSecs = Long.parseLong(fieldValue);
}
+
}
jp.nextToken();
}
jp.close();
- long expiry = System.currentTimeMillis();
- expiry = expiry + expiryPeriod * 1000L; // convert expiryPeriod to milliseconds and add
- token.setExpiry(new Date(expiry));
- LOG.debug("AADToken: fetched token with expiry " + token.getExpiry().toString());
+ if (expiresOnInSecs > -1) {
+ token.setExpiry(new Date(expiresOnInSecs * 1000));
+ } else {
+ long expiry = System.currentTimeMillis();
Review comment:
This seemed a safer approach. Also its not unreliable when you are retrieving from AAD service itself, it becomes unreliable for cache scenario like msi.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] bilaharith commented on issue #1872: Hadoop 16890: Change
in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
bilaharith commented on issue #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#issuecomment-596340351
> In the test results pasted, there is failure for a testcase which is not related to this PR. Create a JIRA to track and fix test issue.
Done
https://issues.apache.org/jira/browse/HADOOP-16915
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r389573794
##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/AzureADAuthenticator.java
##########
@@ -408,17 +409,30 @@ private static AzureADToken parseTokenFromStream(InputStream httpResponseStream)
if (fieldName.equals("access_token")) {
token.setAccessToken(fieldValue);
}
+
if (fieldName.equals("expires_in")) {
Review comment:
I'd like some reporting of parse failures here, especially which field isn't parsing.
Also, make sure that the code is validating content type before JSON parsing. We've done that for OAuth, needs to be done here so that a bad URL or proxy returns HTML and everything fails for no obvious reason
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #1872: Hadoop 16890:
Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#issuecomment-596077067
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 34s | 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 _ |
| +0 :ok: | mvndep | 1m 16s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 19m 0s | trunk passed |
| +1 :green_heart: | compile | 16m 54s | trunk passed |
| +1 :green_heart: | checkstyle | 2m 38s | trunk passed |
| +1 :green_heart: | mvnsite | 1m 20s | trunk passed |
| +1 :green_heart: | shadedclient | 19m 27s | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 12s | trunk passed |
| +0 :ok: | spotbugs | 1m 4s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +0 :ok: | findbugs | 0m 33s | branch/hadoop-project no findbugs output file (findbugsXml.xml) |
| -0 :warning: | patch | 1m 30s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 24s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 42s | the patch passed |
| +1 :green_heart: | compile | 16m 18s | the patch passed |
| +1 :green_heart: | javac | 16m 18s | the patch passed |
| +1 :green_heart: | checkstyle | 2m 36s | the patch passed |
| +1 :green_heart: | mvnsite | 1m 22s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 3s | The patch has no ill-formed XML file. |
| +1 :green_heart: | shadedclient | 14m 21s | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 12s | the patch passed |
| +0 :ok: | findbugs | 0m 33s | hadoop-project has no data from findbugs |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 34s | hadoop-project in the patch passed. |
| +1 :green_heart: | unit | 1m 36s | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 51s | The patch does not generate ASF License warnings. |
| | | 106m 47s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.7 Server=19.03.7 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/7/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/1872 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle |
| uname | Linux aaf892ffd9ea 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/hadoop.sh |
| git revision | trunk / 3859fa7 |
| Default Java | 1.8.0_242 |
| Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/7/testReport/ |
| Max. process+thread count | 414 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-tools/hadoop-azure U: . |
| Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/7/console |
| versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
| Powered by | Apache Yetus 0.11.1 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] DadanielZ commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
DadanielZ commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r387853747
##########
File path: hadoop-project/pom.xml
##########
@@ -1680,6 +1680,20 @@
<artifactId>jna</artifactId>
<version>${jna.version}</version>
</dependency>
+
+ <dependency>
+ <groupId>org.powermock</groupId>
+ <artifactId>powermock-api-mockito2</artifactId>
+ <scope>test</scope>
+ <version>2.0.4</version>
+ </dependency>
+ <dependency>
+ <groupId>org.powermock</groupId>
+ <artifactId>powermock-module-junit4</artifactId>
+ <scope>test</scope>
+ <version>2.0.4</version>
Review comment:
same here
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r390958751
##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/AzureADAuthenticator.java
##########
@@ -258,8 +258,13 @@ public UnexpectedResponseException(final int httpErrorCode,
}
private static AzureADToken getTokenCall(String authEndpoint, String body,
- Hashtable<String, String> headers, String httpMethod)
- throws IOException {
+ Hashtable<String, String> headers, String httpMethod) throws IOException {
+ return getTokenCall(authEndpoint, body, headers, httpMethod, false);
+ }
+
+ private static AzureADToken getTokenCall(String authEndpoint, String body,
+ Hashtable<String, String> headers, String httpMethod, boolean isMsi)
Review comment:
Not something needing changing in this PR, but this should really be Map<> and the code to move to a HashMap; all of Hashtable's methods are synchronized and it underperforms.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r389574087
##########
File path: hadoop-project/pom.xml
##########
@@ -1680,6 +1680,20 @@
<artifactId>jna</artifactId>
<version>${jna.version}</version>
</dependency>
+
Review comment:
this is going to be a separate PR, isn't it?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #1872: Hadoop 16890:
Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#issuecomment-594902805
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 37s | 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 2 new or modified test files. |
||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 1m 27s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 26m 52s | trunk passed |
| +1 :green_heart: | compile | 23m 0s | trunk passed |
| +1 :green_heart: | checkstyle | 3m 22s | trunk passed |
| +1 :green_heart: | mvnsite | 1m 15s | trunk passed |
| +1 :green_heart: | shadedclient | 23m 18s | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 59s | trunk passed |
| +0 :ok: | spotbugs | 1m 14s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +0 :ok: | findbugs | 0m 27s | branch/hadoop-project no findbugs output file (findbugsXml.xml) |
| -0 :warning: | patch | 1m 36s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 28s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 44s | the patch passed |
| +1 :green_heart: | compile | 20m 31s | the patch passed |
| +1 :green_heart: | javac | 20m 31s | the patch passed |
| -0 :warning: | checkstyle | 3m 26s | root: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | mvnsite | 1m 11s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 3s | The patch has no ill-formed XML file. |
| +1 :green_heart: | shadedclient | 16m 53s | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 54s | the patch passed |
| +0 :ok: | findbugs | 0m 29s | hadoop-project has no data from findbugs |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 23s | hadoop-project in the patch passed. |
| +1 :green_heart: | unit | 1m 22s | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 45s | The patch does not generate ASF License warnings. |
| | | 131m 15s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.7 Server=19.03.7 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/1872 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle |
| uname | Linux 64625decbc8e 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/hadoop.sh |
| git revision | trunk / 3afd4cb |
| Default Java | 1.8.0_242 |
| checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/5/artifact/out/diff-checkstyle-root.txt |
| Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/5/testReport/ |
| Max. process+thread count | 309 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-tools/hadoop-azure U: . |
| Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/5/console |
| versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
| Powered by | Apache Yetus 0.11.1 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] DadanielZ commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
DadanielZ commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r387859258
##########
File path: hadoop-tools/hadoop-azure/pom.xml
##########
@@ -45,7 +45,7 @@
<fs.azure.scale.test.timeout>7200</fs.azure.scale.test.timeout>
<fs.azure.scale.test.list.performance.threads>10</fs.azure.scale.test.list.performance.threads>
<fs.azure.scale.test.list.performance.files>1000</fs.azure.scale.test.list.performance.files>
- <powermock.version>2.0.4</powermock.version>
+ <!--<powermock.version>2.0.4</powermock.version>-->
Review comment:
remove it if not being used
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] DadanielZ commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
DadanielZ commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r387859688
##########
File path: hadoop-tools/hadoop-azure/pom.xml
##########
@@ -290,13 +290,13 @@
<groupId>org.powermock</groupId>
<artifactId>powermock-api-mockito2</artifactId>
<scope>test</scope>
- <version>${powermock.version}</version>
+ <!--<version>${powermock.version}</version>-->
</dependency>
<dependency>
<groupId>org.powermock</groupId>
<artifactId>powermock-module-junit4</artifactId>
<scope>test</scope>
- <version>${powermock.version}</version>
+ <!--<version>${powermock.version}</version>-->
Review comment:
same here
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r389575563
##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsMsiTokenProvider.java
##########
@@ -0,0 +1,93 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.azurebfs;
+
+import org.apache.commons.lang3.StringUtils;
Review comment:
check import ordering. you know what I'll complain about
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] bilaharith commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
bilaharith commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r389901242
##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/AzureADAuthenticator.java
##########
@@ -408,17 +409,30 @@ private static AzureADToken parseTokenFromStream(InputStream httpResponseStream)
if (fieldName.equals("access_token")) {
token.setAccessToken(fieldValue);
}
+
if (fieldName.equals("expires_in")) {
Review comment:
Added log to indicate based on which field exactly the expiry is calculated
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] DadanielZ commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
DadanielZ commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r387853665
##########
File path: hadoop-project/pom.xml
##########
@@ -1680,6 +1680,20 @@
<artifactId>jna</artifactId>
<version>${jna.version}</version>
</dependency>
+
+ <dependency>
+ <groupId>org.powermock</groupId>
+ <artifactId>powermock-api-mockito2</artifactId>
+ <scope>test</scope>
+ <version>2.0.4</version>
Review comment:
define a variable for the version
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] DadanielZ commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
DadanielZ commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r388115706
##########
File path: hadoop-project/pom.xml
##########
@@ -204,6 +204,7 @@
<assertj.version>3.12.2</assertj.version>
<jline.version>3.9.0</jline.version>
<powermock.version>1.5.6</powermock.version>
Review comment:
Hey @bilaharith @steveloughran , I'm still not sure about this change. Simply updating the existing "`powermock.version`" will cause build issues for some other projects, so new version variable "`powermock-api.version`" is introduced.
Some other projects already override "`powermock.version`" for `powermock-api-mockito2 `, such as Hadoop-Yarn [here](https://github.com/apache/hadoop/blob/4a70a0d81601f20ba8cecb37fae12b6a8be327b4/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice-documentstore/pom.xml#L61).
Maybe we can override its version in hadoop-azure's pom file too, instead of introducing the new version variable?
Other changes look good to me.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 removed a comment on issue #1872: Hadoop
16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus removed a comment on issue #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#issuecomment-593691330
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 26s | 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 2 new or modified test files. |
||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 21m 12s | trunk passed |
| +1 :green_heart: | compile | 0m 27s | trunk passed |
| +1 :green_heart: | checkstyle | 0m 18s | trunk passed |
| +1 :green_heart: | mvnsite | 0m 30s | trunk passed |
| +1 :green_heart: | shadedclient | 15m 57s | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | trunk passed |
| +0 :ok: | spotbugs | 0m 49s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 0m 48s | trunk passed |
| -0 :warning: | patch | 1m 5s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 26s | the patch passed |
| +1 :green_heart: | compile | 0m 21s | the patch passed |
| +1 :green_heart: | javac | 0m 21s | the patch passed |
| -0 :warning: | checkstyle | 0m 14s | hadoop-tools/hadoop-azure: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | mvnsite | 0m 24s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 1s | The patch has no ill-formed XML file. |
| +1 :green_heart: | shadedclient | 15m 17s | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 19s | the patch passed |
| +1 :green_heart: | findbugs | 0m 52s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 23s | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 27s | The patch does not generate ASF License warnings. |
| | | 61m 14s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/1872 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle |
| uname | Linux 6e61497f53c4 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/hadoop.sh |
| git revision | trunk / edc2e9d |
| Default Java | 1.8.0_242 |
| checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/2/artifact/out/diff-checkstyle-hadoop-tools_hadoop-azure.txt |
| Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/2/testReport/ |
| Max. process+thread count | 307 (vs. ulimit of 5500) |
| modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
| Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/2/console |
| versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
| Powered by | Apache Yetus 0.11.1 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] bilaharith commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
bilaharith commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r389902365
##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/AzureADAuthenticator.java
##########
@@ -408,17 +409,29 @@ private static AzureADToken parseTokenFromStream(InputStream httpResponseStream)
if (fieldName.equals("access_token")) {
token.setAccessToken(fieldValue);
}
+
if (fieldName.equals("expires_in")) {
- expiryPeriod = Integer.parseInt(fieldValue);
+ expiryPeriodInSecs = Integer.parseInt(fieldValue);
+ }
+
+ if (fieldName.equals("expires_on")) {
+ expiresOnInSecs = Long.parseLong(fieldValue);
}
+
}
jp.nextToken();
}
jp.close();
- long expiry = System.currentTimeMillis();
- expiry = expiry + expiryPeriod * 1000L; // convert expiryPeriod to milliseconds and add
- token.setExpiry(new Date(expiry));
- LOG.debug("AADToken: fetched token with expiry " + token.getExpiry().toString());
+ if (expiresOnInSecs > -1) {
+ token.setExpiry(new Date(expiresOnInSecs * 1000));
+ } else {
+ long expiry = System.currentTimeMillis();
Review comment:
Though MSI team confirmed faulty expires_in it still is valid for other flows and MSI response will defenitly contain the expires_in field
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] bilaharith commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
bilaharith commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r389903688
##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/AzureADAuthenticator.java
##########
@@ -408,17 +409,30 @@ private static AzureADToken parseTokenFromStream(InputStream httpResponseStream)
if (fieldName.equals("access_token")) {
token.setAccessToken(fieldValue);
}
+
if (fieldName.equals("expires_in")) {
- expiryPeriod = Integer.parseInt(fieldValue);
+ expiryPeriodInSecs = Integer.parseInt(fieldValue);
+ }
+
+ if (fieldName.equals("expires_on")) {
+ expiresOnInSecs = Long.parseLong(fieldValue);
}
+
}
jp.nextToken();
}
jp.close();
- long expiry = System.currentTimeMillis();
- expiry = expiry + expiryPeriod * 1000L; // convert expiryPeriod to milliseconds and add
- token.setExpiry(new Date(expiry));
- LOG.debug("AADToken: fetched token with expiry " + token.getExpiry().toString());
+ if (expiresOnInSecs > 0) {
+ token.setExpiry(new Date(expiresOnInSecs * 1000));
+ } else {
+ long expiry = System.currentTimeMillis();
+ expiry = expiry + expiryPeriodInSecs
+ * 1000L; // convert expiryPeriod to milliseconds and add
Review comment:
Not using Java 8 features as we keep a backport to an older Java version branch.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 removed a comment on issue #1872: Hadoop
16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus removed a comment on issue #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#issuecomment-594653639
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 37s | 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 2 new or modified test files. |
||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 1m 14s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 21m 10s | trunk passed |
| +1 :green_heart: | compile | 17m 18s | trunk passed |
| +1 :green_heart: | checkstyle | 2m 38s | trunk passed |
| +1 :green_heart: | mvnsite | 1m 20s | trunk passed |
| +1 :green_heart: | shadedclient | 19m 13s | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 12s | trunk passed |
| +0 :ok: | spotbugs | 1m 3s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +0 :ok: | findbugs | 0m 34s | branch/hadoop-project no findbugs output file (findbugsXml.xml) |
| -0 :warning: | patch | 1m 29s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 31s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 42s | the patch passed |
| +1 :green_heart: | compile | 16m 9s | the patch passed |
| +1 :green_heart: | javac | 16m 9s | the patch passed |
| -0 :warning: | checkstyle | 2m 42s | root: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | mvnsite | 1m 22s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 3s | The patch has no ill-formed XML file. |
| +1 :green_heart: | shadedclient | 14m 13s | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 14s | the patch passed |
| +0 :ok: | findbugs | 0m 31s | hadoop-project has no data from findbugs |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 32s | hadoop-project in the patch passed. |
| +1 :green_heart: | unit | 1m 37s | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 54s | The patch does not generate ASF License warnings. |
| | | 109m 16s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.7 Server=19.03.7 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/1872 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle |
| uname | Linux 5a028e078956 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/hadoop.sh |
| git revision | trunk / bbd704b |
| Default Java | 1.8.0_242 |
| checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/4/artifact/out/diff-checkstyle-root.txt |
| Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/4/testReport/ |
| Max. process+thread count | 456 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-tools/hadoop-azure U: . |
| Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/4/console |
| versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
| Powered by | Apache Yetus 0.11.1 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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 removed a comment on issue #1872: Hadoop
16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus removed a comment on issue #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#issuecomment-594120524
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 35m 13s | 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 2 new or modified test files. |
||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 25m 54s | trunk passed |
| +1 :green_heart: | compile | 0m 37s | trunk passed |
| +1 :green_heart: | checkstyle | 0m 26s | trunk passed |
| +1 :green_heart: | mvnsite | 0m 42s | trunk passed |
| +1 :green_heart: | shadedclient | 20m 41s | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 32s | trunk passed |
| +0 :ok: | spotbugs | 1m 9s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 1m 7s | trunk passed |
| -0 :warning: | patch | 1m 29s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 40s | the patch passed |
| +1 :green_heart: | compile | 0m 28s | the patch passed |
| +1 :green_heart: | javac | 0m 28s | the patch passed |
| -0 :warning: | checkstyle | 0m 19s | hadoop-tools/hadoop-azure: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | mvnsite | 0m 32s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 1s | The patch has no ill-formed XML file. |
| +1 :green_heart: | shadedclient | 18m 50s | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 27s | the patch passed |
| +1 :green_heart: | findbugs | 1m 7s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 37s | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 44s | The patch does not generate ASF License warnings. |
| | | 111m 33s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/1872 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle |
| uname | Linux 296de404716a 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/hadoop.sh |
| git revision | trunk / d0a7c79 |
| Default Java | 1.8.0_242 |
| checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/3/artifact/out/diff-checkstyle-hadoop-tools_hadoop-azure.txt |
| Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/3/testReport/ |
| Max. process+thread count | 308 (vs. ulimit of 5500) |
| modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
| Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/3/console |
| versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
| Powered by | Apache Yetus 0.11.1 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] DadanielZ commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
DadanielZ commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r387859258
##########
File path: hadoop-tools/hadoop-azure/pom.xml
##########
@@ -45,7 +45,7 @@
<fs.azure.scale.test.timeout>7200</fs.azure.scale.test.timeout>
<fs.azure.scale.test.list.performance.threads>10</fs.azure.scale.test.list.performance.threads>
<fs.azure.scale.test.list.performance.files>1000</fs.azure.scale.test.list.performance.files>
- <powermock.version>2.0.4</powermock.version>
+ <!--<powermock.version>2.0.4</powermock.version>-->
Review comment:
remove if not being used
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] bilaharith commented on a change in pull request #1872:
Hadoop 16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
bilaharith commented on a change in pull request #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#discussion_r387170783
##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/unittests/TestMsiTokenProvider.java
##########
@@ -0,0 +1,102 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.fs.azurebfs.unittests;
+
+import org.apache.hadoop.fs.azurebfs.oauth2.AccessTokenProvider;
+import org.apache.hadoop.fs.azurebfs.oauth2.AzureADAuthenticator;
+import org.apache.hadoop.fs.azurebfs.oauth2.AzureADToken;
+import org.apache.hadoop.fs.azurebfs.oauth2.MsiTokenProvider;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import java.io.IOException;
+import java.util.Date;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.lessThanOrEqualTo;
+import static org.hamcrest.core.AllOf.allOf;
+import static org.hamcrest.core.Is.is;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.when;
+import static org.powermock.api.mockito.PowerMockito.mockStatic;
+import static org.powermock.reflect.Whitebox.getInternalState;
+import static org.powermock.reflect.Whitebox.invokeMethod;
+import static org.powermock.reflect.Whitebox.setInternalState;
+
+/**
+ * Unit test for MsiTokenProvider
+ */
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(AzureADAuthenticator.class)
+public class TestMsiTokenProvider {
+
+ private static final long ONE_HOUR = 3600 * 1000;
+ private static final long TWO_HOUR = ONE_HOUR * 2;
+
+ @Test
+ public void testMsiTokenProvider() throws Exception {
+ String testToken = "TEST_TOKEN1";
+ setMockAzureADAuthenticator(testToken,
+ System.currentTimeMillis() + TWO_HOUR);
+
+ AccessTokenProvider msiTokenProvider = new MsiTokenProvider("", "", "", "");
+ long tokenFetchTime = getInternalState(msiTokenProvider, "tokenFetchTime");
+ Assert.assertEquals(-1, tokenFetchTime);
+
+ long before = System.currentTimeMillis();
+ AzureADToken token = msiTokenProvider.getToken();
+ long after = System.currentTimeMillis();
+ long newTokenFetchTime = getInternalState(msiTokenProvider,
+ "tokenFetchTime");
+ assertThat(newTokenFetchTime,
+ is(allOf(greaterThan(before), lessThanOrEqualTo(after))));
+ assertThat(token.getAccessToken(), is(equalTo(testToken)));
+
+ assertThat(invokeMethod(msiTokenProvider, "isTokenAboutToExpire"),
+ is(false));
+
+ setInternalState(msiTokenProvider, "tokenFetchTime",
+ System.currentTimeMillis() - ONE_HOUR);
+ assertThat(invokeMethod(msiTokenProvider, "isTokenAboutToExpire"),
+ is(true));
+
+ setInternalState(msiTokenProvider, "tokenFetchTime",
+ System.currentTimeMillis() + 2 - ONE_HOUR);
+ assertThat(invokeMethod(msiTokenProvider, "isTokenAboutToExpire"),
+ is(false));
+ }
+
+ private AzureADToken setMockAzureADAuthenticator(String tokenStr, long expiry)
+ throws IOException {
+ AzureADToken token = new AzureADToken();
+ token.setAccessToken(tokenStr);
+ token.setExpiry(new Date(expiry));
+ mockStatic(AzureADAuthenticator.class);
+ when(AzureADAuthenticator
+ .getTokenFromMsi(Mockito.anyString(), Mockito.anyString(),
Review comment:
No. Planning to do that as a separate task.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 removed a comment on issue #1872: Hadoop
16890: Change in expiry calculation for MSI token provider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus removed a comment on issue #1872: Hadoop 16890: Change in expiry calculation for MSI token provider
URL: https://github.com/apache/hadoop/pull/1872#issuecomment-594902805
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 37s | 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 2 new or modified test files. |
||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 1m 27s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 26m 52s | trunk passed |
| +1 :green_heart: | compile | 23m 0s | trunk passed |
| +1 :green_heart: | checkstyle | 3m 22s | trunk passed |
| +1 :green_heart: | mvnsite | 1m 15s | trunk passed |
| +1 :green_heart: | shadedclient | 23m 18s | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 59s | trunk passed |
| +0 :ok: | spotbugs | 1m 14s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +0 :ok: | findbugs | 0m 27s | branch/hadoop-project no findbugs output file (findbugsXml.xml) |
| -0 :warning: | patch | 1m 36s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 28s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 44s | the patch passed |
| +1 :green_heart: | compile | 20m 31s | the patch passed |
| +1 :green_heart: | javac | 20m 31s | the patch passed |
| -0 :warning: | checkstyle | 3m 26s | root: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | mvnsite | 1m 11s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 3s | The patch has no ill-formed XML file. |
| +1 :green_heart: | shadedclient | 16m 53s | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 54s | the patch passed |
| +0 :ok: | findbugs | 0m 29s | hadoop-project has no data from findbugs |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 23s | hadoop-project in the patch passed. |
| +1 :green_heart: | unit | 1m 22s | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 45s | The patch does not generate ASF License warnings. |
| | | 131m 15s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.7 Server=19.03.7 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/1872 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle |
| uname | Linux 64625decbc8e 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/hadoop.sh |
| git revision | trunk / 3afd4cb |
| Default Java | 1.8.0_242 |
| checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/5/artifact/out/diff-checkstyle-root.txt |
| Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/5/testReport/ |
| Max. process+thread count | 309 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-tools/hadoop-azure U: . |
| Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1872/5/console |
| versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
| Powered by | Apache Yetus 0.11.1 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org