[
https://issues.apache.org/jira/browse/HADOOP-19384?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17926094#comment-17926094
]
ASF GitHub Bot commented on HADOOP-19384:
-----------------------------------------
steveloughran commented on code in PR #7284:
URL: https://github.com/apache/hadoop/pull/7284#discussion_r1951317234
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/ProfileAWSCredentialsProvider.java:
##########
@@ -0,0 +1,56 @@
+package org.apache.hadoop.fs.s3a.auth;
Review Comment:
need the license...just copy from another class
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AAWSCredentialsProvider.java:
##########
@@ -18,8 +18,7 @@
package org.apache.hadoop.fs.s3a;
-import java.io.IOException;
-import java.io.InterruptedIOException;
+import java.io.*;
Review Comment:
revert; .* is not allowed in the hadoop code as it is too brittle
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/ProfileAWSCredentialsProvider.java:
##########
@@ -0,0 +1,56 @@
+package org.apache.hadoop.fs.s3a.auth;
+
+import software.amazon.awssdk.auth.credentials.AwsCredentials;
+import software.amazon.awssdk.auth.credentials.ProfileCredentialsProvider;
+import software.amazon.awssdk.profiles.ProfileFile;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+
+import org.apache.commons.lang3.SystemUtils;
+import org.apache.hadoop.conf.Configuration;
+
+import java.net.URI;
+import java.nio.file.FileSystems;
+import java.nio.file.Path;
+
[email protected]
[email protected]
+public class ProfileAWSCredentialsProvider extends
AbstractAWSCredentialProvider {
+ public static final String NAME
+ = "org.apache.hadoop.fs.s3a.auth.ProfileAWSCredentialsProvider";
+ public static final String PROFILE_FILE = "fs.s3a.auth.profile.file";
Review Comment:
add javadocs for these and the field at L25
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/ProfileAWSCredentialsProvider.java:
##########
@@ -0,0 +1,56 @@
+package org.apache.hadoop.fs.s3a.auth;
+
+import software.amazon.awssdk.auth.credentials.AwsCredentials;
+import software.amazon.awssdk.auth.credentials.ProfileCredentialsProvider;
+import software.amazon.awssdk.profiles.ProfileFile;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+
+import org.apache.commons.lang3.SystemUtils;
+import org.apache.hadoop.conf.Configuration;
+
+import java.net.URI;
+import java.nio.file.FileSystems;
+import java.nio.file.Path;
+
[email protected]
[email protected]
+public class ProfileAWSCredentialsProvider extends
AbstractAWSCredentialProvider {
+ public static final String NAME
+ = "org.apache.hadoop.fs.s3a.auth.ProfileAWSCredentialsProvider";
+ public static final String PROFILE_FILE = "fs.s3a.auth.profile.file";
+ public static final String PROFILE_NAME = "fs.s3a.auth.profile.name";
+
+ private final ProfileCredentialsProvider pcp;
+
+ private static Path getCredentialsPath(Configuration conf) {
+ String credentialsFile = conf.get(PROFILE_FILE, null);
+ if (credentialsFile == null) {
+ credentialsFile =
SystemUtils.getEnvironmentVariable("AWS_SHARED_CREDENTIALS_FILE", null);
+ }
+ Path path = (credentialsFile == null) ?
+
FileSystems.getDefault().getPath(SystemUtils.getUserHome().getPath(),".aws","credentials")
Review Comment:
nit: can you add a space after the commas in the arguments
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AAWSCredentialsProvider.java:
##########
@@ -139,6 +134,32 @@ public void testInstantiationChain() throws Throwable {
assertCredentialProviders(expectedClasses, list);
}
+ @Test
+ public void testProfileAWSCredentialsProvider() throws Throwable {
+ Configuration conf = new Configuration(false);
+ conf.set(AWS_CREDENTIALS_PROVIDER, ProfileAWSCredentialsProvider.NAME);
+ try (FileWriter fileWriter = new FileWriter("testcred"); BufferedWriter
bufferedWriter = new BufferedWriter(fileWriter)) {
Review Comment:
the test files have to go into target/, whether run on the command line or
IDE.
use `File.createTempFile()` to get one in the temp dir
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/ProfileAWSCredentialsProvider.java:
##########
@@ -0,0 +1,56 @@
+package org.apache.hadoop.fs.s3a.auth;
+
+import software.amazon.awssdk.auth.credentials.AwsCredentials;
Review Comment:
import ordering (should be)
```
java.
javax.
non-apache-stuff.
org.apache.
static everything
---
it's close but a bit mixed up...please fix
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AAWSCredentialsProvider.java:
##########
@@ -35,6 +34,7 @@
import java.util.stream.Collectors;
import javax.annotation.Nullable;
+import org.apache.hadoop.fs.s3a.auth.*;
Review Comment:
revert
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/ProfileAWSCredentialsProvider.java:
##########
@@ -0,0 +1,56 @@
+package org.apache.hadoop.fs.s3a.auth;
+
+import software.amazon.awssdk.auth.credentials.AwsCredentials;
+import software.amazon.awssdk.auth.credentials.ProfileCredentialsProvider;
+import software.amazon.awssdk.profiles.ProfileFile;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+
+import org.apache.commons.lang3.SystemUtils;
+import org.apache.hadoop.conf.Configuration;
+
+import java.net.URI;
+import java.nio.file.FileSystems;
+import java.nio.file.Path;
+
[email protected]
[email protected]
+public class ProfileAWSCredentialsProvider extends
AbstractAWSCredentialProvider {
Review Comment:
I wonder if we should be logging at debug when the env vars are being used?
Nothing secret will be logged and it could be useful.
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AAWSCredentialsProvider.java:
##########
@@ -139,6 +134,32 @@ public void testInstantiationChain() throws Throwable {
assertCredentialProviders(expectedClasses, list);
}
+ @Test
+ public void testProfileAWSCredentialsProvider() throws Throwable {
+ Configuration conf = new Configuration(false);
+ conf.set(AWS_CREDENTIALS_PROVIDER, ProfileAWSCredentialsProvider.NAME);
+ try (FileWriter fileWriter = new FileWriter("testcred"); BufferedWriter
bufferedWriter = new BufferedWriter(fileWriter)) {
+ bufferedWriter.write("[default]\n"
+ + "aws_access_key_id = defaultaccesskeyid\n"
+ + "aws_secret_access_key = defaultsecretkeyid\n");
+ bufferedWriter.write("[nondefault]\n"
+ + "aws_access_key_id = nondefaultaccesskeyid\n"
+ + "aws_secret_access_key = nondefaultsecretkeyid\n");
+ }
+ conf.set(ProfileAWSCredentialsProvider.PROFILE_FILE, "testcred");
+ URI testUri = new URI("s3a://bucket1");
+ AWSCredentialProviderList list = createAWSCredentialProviderList(testUri,
conf);
+
assertCredentialProviders(Collections.singletonList(ProfileAWSCredentialsProvider.class),
list);
+ AwsCredentials credentials = list.resolveCredentials();
+ assertEquals("defaultaccesskeyid", credentials.accessKeyId());
Review Comment:
Use AssertJ assertions. We need this to ease our migration to junit5
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/ProfileAWSCredentialsProvider.java:
##########
@@ -0,0 +1,56 @@
+package org.apache.hadoop.fs.s3a.auth;
+
+import software.amazon.awssdk.auth.credentials.AwsCredentials;
+import software.amazon.awssdk.auth.credentials.ProfileCredentialsProvider;
+import software.amazon.awssdk.profiles.ProfileFile;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+
+import org.apache.commons.lang3.SystemUtils;
+import org.apache.hadoop.conf.Configuration;
+
+import java.net.URI;
+import java.nio.file.FileSystems;
+import java.nio.file.Path;
+
[email protected]
[email protected]
+public class ProfileAWSCredentialsProvider extends
AbstractAWSCredentialProvider {
+ public static final String NAME
+ = "org.apache.hadoop.fs.s3a.auth.ProfileAWSCredentialsProvider";
+ public static final String PROFILE_FILE = "fs.s3a.auth.profile.file";
+ public static final String PROFILE_NAME = "fs.s3a.auth.profile.name";
+
+ private final ProfileCredentialsProvider pcp;
+
+ private static Path getCredentialsPath(Configuration conf) {
+ String credentialsFile = conf.get(PROFILE_FILE, null);
+ if (credentialsFile == null) {
+ credentialsFile =
SystemUtils.getEnvironmentVariable("AWS_SHARED_CREDENTIALS_FILE", null);
Review Comment:
move all env vars to string constants; add javadocs for each of these
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/ProfileAWSCredentialsProvider.java:
##########
@@ -0,0 +1,56 @@
+package org.apache.hadoop.fs.s3a.auth;
+
+import software.amazon.awssdk.auth.credentials.AwsCredentials;
+import software.amazon.awssdk.auth.credentials.ProfileCredentialsProvider;
+import software.amazon.awssdk.profiles.ProfileFile;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+
+import org.apache.commons.lang3.SystemUtils;
+import org.apache.hadoop.conf.Configuration;
+
+import java.net.URI;
+import java.nio.file.FileSystems;
+import java.nio.file.Path;
+
[email protected]
[email protected]
+public class ProfileAWSCredentialsProvider extends
AbstractAWSCredentialProvider {
+ public static final String NAME
+ = "org.apache.hadoop.fs.s3a.auth.ProfileAWSCredentialsProvider";
+ public static final String PROFILE_FILE = "fs.s3a.auth.profile.file";
+ public static final String PROFILE_NAME = "fs.s3a.auth.profile.name";
+
+ private final ProfileCredentialsProvider pcp;
+
+ private static Path getCredentialsPath(Configuration conf) {
+ String credentialsFile = conf.get(PROFILE_FILE, null);
+ if (credentialsFile == null) {
+ credentialsFile =
SystemUtils.getEnvironmentVariable("AWS_SHARED_CREDENTIALS_FILE", null);
+ }
+ Path path = (credentialsFile == null) ?
+
FileSystems.getDefault().getPath(SystemUtils.getUserHome().getPath(),".aws","credentials")
+ : FileSystems.getDefault().getPath(credentialsFile);
+ return path;
+ }
+
+ private static String getCredentialsName(Configuration conf) {
+ String profileName = conf.get(PROFILE_NAME, null);
+ if (profileName == null) {
+ profileName = SystemUtils.getEnvironmentVariable("AWS_PROFILE",
"default");
+ }
+ return profileName;
+ }
+
+ public ProfileAWSCredentialsProvider(URI uri, Configuration conf) {
+ super(uri, conf);
+ ProfileCredentialsProvider.Builder builder =
ProfileCredentialsProvider.builder();
+
builder.profileName(getCredentialsName(conf)).profileFile(ProfileFile.builder().content(getCredentialsPath(conf)).type(ProfileFile.Type.CREDENTIALS).build());
Review Comment:
should be split across lines, one per build attribute
> Add support for ProfileCredentialsProvider
> ------------------------------------------
>
> Key: HADOOP-19384
> URL: https://issues.apache.org/jira/browse/HADOOP-19384
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs/s3
> Affects Versions: 3.4.1
> Reporter: Venkatasubrahmanian Narayanan
> Assignee: Venkatasubrahmanian Narayanan
> Priority: Minor
> Labels: pull-request-available
>
> Hadoop currently doesn't support AWS' ProfileCredentialsProvider. A thin
> wrapper is sufficient to get it to work, since it just needs us to fetch the
> credentials file.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
