From 38836ec64e4b4b94fb89240d93ed61c4d8b693f4 Mon Sep 17 00:00:00 2001 From: Roman Leventov Date: Tue, 27 Nov 2018 15:30:55 +0100 Subject: [PATCH 1/3] Fix a race in FileSessionCredentialsProvider --- .../aws/FileSessionCredentialsProvider.java | 81 +++++++++++-------- 1 file changed, 47 insertions(+), 34 deletions(-) diff --git a/aws-common/src/main/java/org/apache/druid/common/aws/FileSessionCredentialsProvider.java b/aws-common/src/main/java/org/apache/druid/common/aws/FileSessionCredentialsProvider.java index 3dbe64b678f4..4e2a75304d8b 100644 --- a/aws-common/src/main/java/org/apache/druid/common/aws/FileSessionCredentialsProvider.java +++ b/aws-common/src/main/java/org/apache/druid/common/aws/FileSessionCredentialsProvider.java @@ -24,27 +24,25 @@ import com.amazonaws.auth.AWSSessionCredentials; import org.apache.druid.java.util.common.concurrent.Execs; -import java.io.File; -import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Paths; import java.util.Properties; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; public class FileSessionCredentialsProvider implements AWSCredentialsProvider { - private final String sessionCredentials; - private volatile String sessionToken; - private volatile String accessKey; - private volatile String secretKey; + private final String sessionCredentialsFile; + private AWSSessionCredentials awsSessionCredentials; private final ScheduledExecutorService scheduler = Execs.scheduledSingleThreaded("FileSessionCredentialsProviderRefresh-%d"); - public FileSessionCredentialsProvider(String sessionCredentials) + public FileSessionCredentialsProvider(String sessionCredentialsFile) { - this.sessionCredentials = sessionCredentials; + this.sessionCredentialsFile = sessionCredentialsFile; refresh(); scheduler.scheduleAtFixedRate(this::refresh, 1, 1, TimeUnit.HOURS); // refresh every hour @@ -53,26 +51,7 @@ public FileSessionCredentialsProvider(String sessionCredentials) @Override public AWSCredentials getCredentials() { - return new AWSSessionCredentials() - { - @Override - public String getSessionToken() - { - return sessionToken; - } - - @Override - public String getAWSAccessKeyId() - { - return accessKey; - } - - @Override - public String getAWSSecretKey() - { - return secretKey; - } - }; + return awsSessionCredentials; } @Override @@ -80,16 +59,50 @@ public void refresh() { try { Properties props = new Properties(); - InputStream is = new FileInputStream(new File(sessionCredentials)); - props.load(is); - is.close(); + try (InputStream is = Files.newInputStream(Paths.get(sessionCredentialsFile))) { + props.load(is); + } + + String sessionToken = props.getProperty("sessionToken"); + String accessKey = props.getProperty("accessKey"); + String secretKey = props.getProperty("secretKey"); - sessionToken = props.getProperty("sessionToken"); - accessKey = props.getProperty("accessKey"); - secretKey = props.getProperty("secretKey"); + awsSessionCredentials = new Credentials(sessionToken, accessKey, secretKey); } catch (IOException e) { throw new RuntimeException("cannot refresh AWS credentials", e); } } + + private static class Credentials implements AWSSessionCredentials + { + private final String sessionToken; + private final String accessKey; + private final String secretKey; + + private Credentials(String sessionToken, String accessKey, String secretKey) + { + this.sessionToken = sessionToken; + this.accessKey = accessKey; + this.secretKey = secretKey; + } + + @Override + public String getSessionToken() + { + return sessionToken; + } + + @Override + public String getAWSAccessKeyId() + { + return accessKey; + } + + @Override + public String getAWSSecretKey() + { + return secretKey; + } + } } From d1685dc71ec6a0bd6a2ae4d8e2eb9c3f39a48bb8 Mon Sep 17 00:00:00 2001 From: Roman Leventov Date: Tue, 27 Nov 2018 19:17:16 +0100 Subject: [PATCH 2/3] Comment --- .../common/aws/FileSessionCredentialsProvider.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/aws-common/src/main/java/org/apache/druid/common/aws/FileSessionCredentialsProvider.java b/aws-common/src/main/java/org/apache/druid/common/aws/FileSessionCredentialsProvider.java index 4e2a75304d8b..4f74f164d76b 100644 --- a/aws-common/src/main/java/org/apache/druid/common/aws/FileSessionCredentialsProvider.java +++ b/aws-common/src/main/java/org/apache/druid/common/aws/FileSessionCredentialsProvider.java @@ -34,11 +34,15 @@ public class FileSessionCredentialsProvider implements AWSCredentialsProvider { - private final String sessionCredentialsFile; - private AWSSessionCredentials awsSessionCredentials; - private final ScheduledExecutorService scheduler = Execs.scheduledSingleThreaded("FileSessionCredentialsProviderRefresh-%d"); + private final String sessionCredentialsFile; + + /** + * This field doesn't need to be volatile. From the Java Memory Model point of view, volatile on this field changes + * anything and doesn't provide any extra guarantees. + */ + private AWSSessionCredentials awsSessionCredentials; public FileSessionCredentialsProvider(String sessionCredentialsFile) { From 3a5b3e96c1bd931e3ddc092ad7a76e556eebbebf Mon Sep 17 00:00:00 2001 From: Roman Leventov Date: Tue, 27 Nov 2018 19:18:04 +0100 Subject: [PATCH 3/3] word choice --- .../apache/druid/common/aws/FileSessionCredentialsProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws-common/src/main/java/org/apache/druid/common/aws/FileSessionCredentialsProvider.java b/aws-common/src/main/java/org/apache/druid/common/aws/FileSessionCredentialsProvider.java index 4f74f164d76b..ad4000ecc111 100644 --- a/aws-common/src/main/java/org/apache/druid/common/aws/FileSessionCredentialsProvider.java +++ b/aws-common/src/main/java/org/apache/druid/common/aws/FileSessionCredentialsProvider.java @@ -40,7 +40,7 @@ public class FileSessionCredentialsProvider implements AWSCredentialsProvider /** * This field doesn't need to be volatile. From the Java Memory Model point of view, volatile on this field changes - * anything and doesn't provide any extra guarantees. + * nothing and doesn't provide any extra guarantees. */ private AWSSessionCredentials awsSessionCredentials;