Details
-
Improvement
-
Status: Resolved
-
Major
-
Resolution: Fixed
-
None
-
None
-
MSI Change applies only to Hadoop running in an Azure VM
Description
This change is to upgrade the Hadoop ADLS connector to enable new auth features exposed by the ADLS Java SDK.
Specifically:
MSI Tokens: MSI (Managed Service Identity) is a way to provide an identity to an Azure Service. In the case of VMs, they can be used to give an identity to a VM deployment. This simplifies managing Service Principals, since the creds don’t have to be managed in core-site files anymore. The way this works is that during VM deployment, the ARM (Azure Resource Manager) template needs to be modified to enable MSI. Once deployed, the MSI extension runs a service on the VM that exposes a token endpoint to http://localhost at a port specified in the template. The SDK has a new TokenProvider to fetch the token from this local endpoint. This change would expose that TokenProvider as an auth option.
DeviceCode auth: This enables a token to be obtained from an interactive login. The user is given a URL and a token to use on the login screen. User can use the token to login from any device. Once the login is done, the token that is obtained is in the name of the user who logged in. Note that because of the interactive login involved, this is not very suitable for job scenarios, but can work for ad-hoc scenarios like running “hdfs dfs” commands.
Attachments
Attachments
- HADOOP-14627.002.patch
- 15 kB
- Atul Sikaria
- HADOOP-14627.003.patch
- 15 kB
- John Zhuge
- HADOOP-14627.004.patch
- 15 kB
- John Zhuge
- HADOOP-14627-001.patch
- 10 kB
- Atul Sikaria
Issue Links
- is duplicated by
-
HADOOP-14438 Make ADLS doc of setting up client key up to date
- Resolved
Activity
If there are multiple changes that are separable, we can make this one a Uber JIRA and create subtasks for it. Small and individual changes are better for review, test and release. Thanks,
Thanks liuml07, I thought about that too. However, I went with one Jira for this time because 1) The combined change is small (~30-40 lines of code) so it was small enough already. 2) The changes for the two auth methods were very similar, so I thought it would make it easier to review them together. 3) The biggest change is to the doc file (index.md), which would be easier to see as a final doc containing both, rather than individual isolated changes for each.
Having said that, this is my perspective (from the patch creator's side), so I am only guessing at your usability in reading the change. Let me know if the current change is not as easy to review as I thought; if so, I can break it up.
- A test would still be good, if just to verify that attempting to use the new auth mechanism fails if the configuration is missing any required property.
- New fs.adl.oauth2.msi.TenantGuid should be all lower case, for consistency with (nearly) everything else
- Is this property a secret which should be stored in hadoop credentials files & retrieved with Configuration.getPassword()?
Thanks stevel@apache.org for reviewing. Will do a test, and fix the casing on the property name. Note on the test that it is only meaningful if run from within an Azure VM - MSI service will not be present anywhere else.
The properties are not secrets - ok to have in cleartext.
ASikaria It does not hurt to call ```getPaswordString``` for non-secret properties. It is nice to group all related properties together in one place, the cred store. Of course, the downside is that non-secret properties are placed into cred store which can be confusing.
steve_l What do you think?
- Name the patch in the format of
HADOOP-14627.00X.patch. - Add all new properties and their default values to core-default.xml.
- Update SDK version whenever it is GA.
- Add unit tests for the new token providers to TestAzureADTokenProvider
jzhuge, steve_l: addressed all your concerns in the comments above.
Also updating patch to not be dependent on the preview version of SDK (since released version 2.2.1 is now available).
+chris.douglas as FYI
Thanks ASikaria for the new rev. Looking great except a few minors:
- Use all upper case MSI except in config property names.
- Convert fs.adl.oauth2.devicecode.ClientAppId to all lower case.
- At AdlConfKeys.java#61, need a space after "//" in "//DeviceCode Auth configuration".
- AdlFileSystem#getAdlStoreClient is unused.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 15s | Docker mode activated. |
Prechecks | |||
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
trunk Compile Tests | |||
0 | mvndep | 1m 26s | Maven dependency ordering for branch |
+1 | mvninstall | 20m 20s | trunk passed |
+1 | compile | 20m 54s | trunk passed |
+1 | checkstyle | 2m 38s | trunk passed |
+1 | mvnsite | 2m 34s | trunk passed |
+1 | findbugs | 2m 49s | trunk passed |
+1 | javadoc | 1m 33s | trunk passed |
Patch Compile Tests | |||
0 | mvndep | 0m 27s | Maven dependency ordering for patch |
+1 | mvninstall | 1m 28s | the patch passed |
+1 | compile | 16m 12s | the patch passed |
+1 | javac | 16m 11s | the patch passed |
-0 | checkstyle | 2m 38s | root: The patch generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0) |
+1 | mvnsite | 2m 47s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | xml | 0m 3s | The patch has no ill-formed XML file. |
+1 | findbugs | 2m 52s | the patch passed |
+1 | javadoc | 1m 30s | the patch passed |
Other Tests | |||
-1 | unit | 9m 33s | hadoop-common in the patch failed. |
-1 | unit | 3m 57s | hadoop-azure-datalake in the patch failed. |
+1 | asflicense | 0m 40s | The patch does not generate ASF License warnings. |
117m 24s |
Reason | Tests |
---|---|
Failed junit tests | hadoop.fs.shell.TestCopyFromLocal |
hadoop.fs.adl.TestGetFileStatus | |
hadoop.fs.adl.TestAzureADTokenProvider |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:14b5c93 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12880330/HADOOP-14627.002.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle |
uname | Linux af35720f8a10 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / 024c3ec |
Default Java | 1.8.0_131 |
findbugs | v3.1.0-RC1 |
checkstyle | https://builds.apache.org/job/PreCommit-HADOOP-Build/12966/artifact/patchprocess/diff-checkstyle-root.txt |
unit | https://builds.apache.org/job/PreCommit-HADOOP-Build/12966/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt |
unit | https://builds.apache.org/job/PreCommit-HADOOP-Build/12966/artifact/patchprocess/patch-unit-hadoop-tools_hadoop-azure-datalake.txt |
Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/12966/testReport/ |
modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-azure-datalake U: . |
Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/12966/console |
Powered by | Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
Patch 003
- Fix TestAzureADTokenProvider.testMSITokenProvider failure
- My last review comments
- Formatting in core-default.xml and index.md
Testing Done
- Passed live unit tests
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 13s | Docker mode activated. |
Prechecks | |||
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
trunk Compile Tests | |||
0 | mvndep | 0m 15s | Maven dependency ordering for branch |
+1 | mvninstall | 14m 14s | trunk passed |
+1 | compile | 14m 47s | trunk passed |
+1 | checkstyle | 1m 57s | trunk passed |
+1 | mvnsite | 1m 55s | trunk passed |
+1 | findbugs | 1m 59s | trunk passed |
+1 | javadoc | 1m 14s | trunk passed |
Patch Compile Tests | |||
0 | mvndep | 0m 18s | Maven dependency ordering for patch |
+1 | mvninstall | 0m 59s | the patch passed |
+1 | compile | 11m 15s | the patch passed |
+1 | javac | 11m 15s | the patch passed |
+1 | checkstyle | 2m 0s | the patch passed |
+1 | mvnsite | 1m 55s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | xml | 0m 3s | The patch has no ill-formed XML file. |
+1 | findbugs | 2m 23s | the patch passed |
+1 | javadoc | 1m 18s | the patch passed |
Other Tests | |||
-1 | unit | 8m 15s | hadoop-common in the patch failed. |
+1 | unit | 3m 44s | hadoop-azure-datalake in the patch passed. |
+1 | asflicense | 0m 33s | The patch does not generate ASF License warnings. |
91m 16s |
Reason | Tests |
---|---|
Failed junit tests | hadoop.security.TestKDiag |
hadoop.net.TestDNS |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:14b5c93 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12881161/HADOOP-14627.003.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle |
uname | Linux 17e81f374afd 3.13.0-117-generic #164-Ubuntu SMP Fri Apr 7 11:05:26 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / 8d953c2 |
Default Java | 1.8.0_131 |
findbugs | v3.1.0-RC1 |
unit | https://builds.apache.org/job/PreCommit-HADOOP-Build/12999/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt |
Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/12999/testReport/ |
modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-azure-datalake U: . |
Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/12999/console |
Powered by | Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
The change looks good to me overall, but I need more time to give another +1. As I am working on something else recently, please go ahead w/o me. Thanks jzhuge.
+1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 15s | Docker mode activated. |
Prechecks | |||
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
trunk Compile Tests | |||
0 | mvndep | 1m 27s | Maven dependency ordering for branch |
+1 | mvninstall | 17m 30s | trunk passed |
+1 | compile | 18m 31s | trunk passed |
+1 | checkstyle | 2m 25s | trunk passed |
+1 | mvnsite | 2m 16s | trunk passed |
+1 | findbugs | 2m 36s | trunk passed |
+1 | javadoc | 1m 27s | trunk passed |
Patch Compile Tests | |||
0 | mvndep | 0m 22s | Maven dependency ordering for patch |
+1 | mvninstall | 1m 15s | the patch passed |
+1 | compile | 13m 42s | the patch passed |
+1 | javac | 13m 42s | the patch passed |
+1 | checkstyle | 2m 24s | the patch passed |
+1 | mvnsite | 2m 16s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | xml | 0m 3s | The patch has no ill-formed XML file. |
+1 | findbugs | 2m 51s | the patch passed |
+1 | javadoc | 1m 30s | the patch passed |
Other Tests | |||
+1 | unit | 9m 32s | hadoop-common in the patch passed. |
+1 | unit | 3m 56s | hadoop-azure-datalake in the patch passed. |
+1 | asflicense | 0m 41s | The patch does not generate ASF License warnings. |
107m 7s |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:14b5c93 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12881595/HADOOP-14627.004.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle |
uname | Linux f6d77ee875f3 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / 28d97b7 |
Default Java | 1.8.0_144 |
findbugs | v3.1.0-RC1 |
Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/13013/testReport/ |
modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-azure-datalake U: . |
Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/13013/console |
Powered by | Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
Committed to trunk, branch-2, and branch-2.8.
Thanks ASikaria for the contribution!
SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #12175 (See https://builds.apache.org/job/Hadoop-trunk-Commit/12175/)
HADOOP-14627. Support MSI and DeviceCode token provider in ADLS. (jzhuge: rev 7769e9614956283a86eda9e4e69aaa592c0ca960)
- (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/TestAzureADTokenProvider.java
- (edit) hadoop-tools/hadoop-azure-datalake/src/site/markdown/index.md
- (edit) hadoop-tools/hadoop-azure-datalake/src/main/java/org/apache/hadoop/fs/adl/AdlConfKeys.java
- (edit) hadoop-tools/hadoop-azure-datalake/src/main/java/org/apache/hadoop/fs/adl/AdlFileSystem.java
- (edit) hadoop-tools/hadoop-azure-datalake/src/main/java/org/apache/hadoop/fs/adl/TokenProviderType.java
- (edit) hadoop-tools/hadoop-azure-datalake/pom.xml
- (edit) hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
Attached patch with the current code. This is to provide early opportunity to review, it should not be checked in until the SDK version it is based on removes the preview label.