honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Extract AWS Key from KeyChain instead of using potential null value 

While I was working on #18703, I discovered a bad behavior when people don't provide AWS key/secret as part as their `elasticsearch.yml` but rely on SysProps or env. variables...

In [`InternalAwsS3Service#getClient(...)`](d4366f8493/plugins/repository-s3/src/main/java/org/elasticsearch/cloud/aws/InternalAwsS3Service.java (L76-L141)), we have:

```java
        Tuple<String, String> clientDescriptor = new Tuple<>(endpoint, account);
        AmazonS3Client client = clients.get(clientDescriptor);
```

But if people don't provide credentials, `account` is `null`.

Even if it actually could work, I think that we should use the `AWSCredentialsProvider` we create later on and extract from it the `account` (AWS KEY actually) and then use it as the second value of the tuple.

Closes #19557.
This commit is contained in:
David Pilato 2016-07-23 01:17:23 +02:00
parent fef0ffc49e
commit f8e0557be5
7 changed files with 34 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
plugins/repository-s3/src/main/java/org/elasticsearch/cloud/aws/AwsS3Service.java
View File

@ -24,6 +24,7 @@ import com.amazonaws.services.s3.AmazonS3;
import org.elasticsearch.common.component.LifecycleComponent;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.common.settings.Settings;
import java.util.Locale;
import java.util.function.Function;
@ -154,6 +155,6 @@ public interface AwsS3Service extends LifecycleComponent {
Setting<String> ENDPOINT_SETTING = Setting.simpleString("cloud.aws.s3.endpoint", Property.NodeScope);
}
AmazonS3 client(String endpoint, Protocol protocol, String region, String account, String key, Integer maxRetries,
AmazonS3 client(Settings repositorySettings, String endpoint, Protocol protocol, String region, Integer maxRetries,
boolean useThrottleRetries, Boolean pathStyleAccess);
}

19
plugins/repository-s3/src/main/java/org/elasticsearch/cloud/aws/InternalAwsS3Service.java
View File

@ -35,10 +35,13 @@ import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.component.AbstractLifecycleComponent;
import org.elasticsearch.common.logging.ESLogger;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.repositories.s3.S3Repository;
import java.util.HashMap;
import java.util.Map;
import static org.elasticsearch.repositories.s3.S3Repository.getValue;
/**
*
*/
@ -54,17 +57,20 @@ public class InternalAwsS3Service extends AbstractLifecycleComponent implements
}
@Override
public synchronized AmazonS3 client(String endpoint, Protocol protocol, String region, String key, String secret, Integer maxRetries,
public synchronized AmazonS3 client(Settings repositorySettings, String endpoint, Protocol protocol, String region, Integer maxRetries,
boolean useThrottleRetries, Boolean pathStyleAccess) {
String foundEndpoint = findEndpoint(logger, settings, endpoint, region);
Tuple<String, String> clientDescriptor = new Tuple<>(foundEndpoint, key);
AWSCredentialsProvider credentials = buildCredentials(logger, settings, repositorySettings);
Tuple<String, String> clientDescriptor = new Tuple<>(foundEndpoint, credentials.getCredentials().getAWSAccessKeyId());
AmazonS3Client client = clients.get(clientDescriptor);
if (client != null) {
return client;
}
client = new AmazonS3Client(
buildCredentials(logger, key, secret),
credentials,
buildConfiguration(logger, settings, protocol, maxRetries, foundEndpoint, useThrottleRetries));
if (pathStyleAccess != null) {
@ -116,8 +122,13 @@ public class InternalAwsS3Service extends AbstractLifecycleComponent implements
return clientConfiguration;
}
public static AWSCredentialsProvider buildCredentials(ESLogger logger, String key, String secret) {
public static AWSCredentialsProvider buildCredentials(ESLogger logger, Settings settings, Settings repositorySettings) {
AWSCredentialsProvider credentials;
String key = getValue(repositorySettings, settings,
S3Repository.Repository.KEY_SETTING, S3Repository.Repositories.KEY_SETTING);
String secret = getValue(repositorySettings, settings,
S3Repository.Repository.SECRET_SETTING, S3Repository.Repositories.SECRET_SETTING);
if (key.isEmpty() && secret.isEmpty()) {
logger.debug("Using either environment variables, system properties or instance profile credentials");
credentials = new DefaultAWSCredentialsProviderChain();

5
plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java
View File

@ -301,11 +301,8 @@ public class S3Repository extends BlobStoreRepository {
bucket, region, endpoint, protocol, chunkSize, serverSideEncryption, bufferSize, maxRetries, useThrottleRetries, cannedACL,
storageClass, pathStyleAccess);
String key = getValue(metadata.settings(), settings, Repository.KEY_SETTING, Repositories.KEY_SETTING);
String secret = getValue(metadata.settings(), settings, Repository.SECRET_SETTING, Repositories.SECRET_SETTING);
blobStore = new S3BlobStore(settings,
s3Service.client(endpoint, protocol, region, key, secret, maxRetries, useThrottleRetries, pathStyleAccess),
s3Service.client(metadata.settings(), endpoint, protocol, region, maxRetries, useThrottleRetries, pathStyleAccess),
bucket, region, serverSideEncryption, bufferSize, maxRetries, cannedACL, storageClass);
String basePath = getValue(metadata.settings(), settings, Repository.BASE_PATH_SETTING, Repositories.BASE_PATH_SETTING);

9
plugins/repository-s3/src/test/java/org/elasticsearch/cloud/aws/AwsS3ServiceImplTests.java
View File

@ -35,7 +35,7 @@ import static org.hamcrest.Matchers.is;
public class AwsS3ServiceImplTests extends ESTestCase {
public void testAWSCredentialsWithSystemProviders() {
AWSCredentialsProvider credentialsProvider = InternalAwsS3Service.buildCredentials(logger, "", "");
AWSCredentialsProvider credentialsProvider = InternalAwsS3Service.buildCredentials(logger, Settings.EMPTY, Settings.EMPTY);
assertThat(credentialsProvider, instanceOf(DefaultAWSCredentialsProviderChain.class));
}
@ -136,12 +136,7 @@ public class AwsS3ServiceImplTests extends ESTestCase {
protected void launchAWSCredentialsWithElasticsearchSettingsTest(Settings singleRepositorySettings, Settings settings,
String expectedKey, String expectedSecret) {
String key = S3Repository.getValue(singleRepositorySettings, settings,
S3Repository.Repository.KEY_SETTING, S3Repository.Repositories.KEY_SETTING);
String secret = S3Repository.getValue(singleRepositorySettings, settings,
S3Repository.Repository.SECRET_SETTING, S3Repository.Repositories.SECRET_SETTING);
AWSCredentials credentials = InternalAwsS3Service.buildCredentials(logger, key, secret).getCredentials();
AWSCredentials credentials = InternalAwsS3Service.buildCredentials(logger, settings, singleRepositorySettings).getCredentials();
assertThat(credentials.getAWSAccessKeyId(), is(expectedKey));
assertThat(credentials.getAWSSecretKey(), is(expectedSecret));
}

6
plugins/repository-s3/src/test/java/org/elasticsearch/cloud/aws/TestAwsS3Service.java
View File

@ -21,10 +21,8 @@ package org.elasticsearch.cloud.aws;
import com.amazonaws.Protocol;
import com.amazonaws.services.s3.AmazonS3;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.plugin.repository.s3.S3RepositoryPlugin;
import org.elasticsearch.plugins.Plugin;
import java.util.IdentityHashMap;
@ -44,9 +42,9 @@ public class TestAwsS3Service extends InternalAwsS3Service {
@Override
public synchronized AmazonS3 client(String endpoint, Protocol protocol, String region, String account, String key, Integer maxRetries,
public synchronized AmazonS3 client(Settings repositorySettings, String endpoint, Protocol protocol, String region, Integer maxRetries,
boolean useThrottleRetries, Boolean pathStyleAccess) {
return cachedWrapper(super.client(endpoint, protocol, region, account, key, maxRetries, useThrottleRetries, pathStyleAccess));
return cachedWrapper(super.client(repositorySettings, endpoint, protocol, region, maxRetries, useThrottleRetries, pathStyleAccess));
}
private AmazonS3 cachedWrapper(AmazonS3 client) {

18
plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/AbstractS3SnapshotRestoreTest.java
View File

@ -161,12 +161,15 @@ public abstract class AbstractS3SnapshotRestoreTest extends AbstractAwsTestCase
public void testEncryption() {
Client client = client();
logger.info("--> creating s3 repository with bucket[{}] and path [{}]", internalCluster().getInstance(Settings.class).get("repositories.s3.bucket"), basePath);
PutRepositoryResponse putRepositoryResponse = client.admin().cluster().preparePutRepository("test-repo")
.setType("s3").setSettings(Settings.builder()
Settings repositorySettings = Settings.builder()
.put(S3Repository.Repository.BASE_PATH_SETTING.getKey(), basePath)
.put(S3Repository.Repository.CHUNK_SIZE_SETTING.getKey(), randomIntBetween(1000, 10000))
.put(S3Repository.Repository.SERVER_SIDE_ENCRYPTION_SETTING.getKey(), true)
).get();
.build();
PutRepositoryResponse putRepositoryResponse = client.admin().cluster().preparePutRepository("test-repo")
.setType("s3").setSettings(repositorySettings).get();
assertThat(putRepositoryResponse.isAcknowledged(), equalTo(true));
createIndex("test-idx-1", "test-idx-2", "test-idx-3");
@ -193,11 +196,10 @@ public abstract class AbstractS3SnapshotRestoreTest extends AbstractAwsTestCase
Settings settings = internalCluster().getInstance(Settings.class);
Settings bucket = settings.getByPrefix("repositories.s3.");
AmazonS3 s3Client = internalCluster().getInstance(AwsS3Service.class).client(
repositorySettings,
null,
S3Repository.Repositories.PROTOCOL_SETTING.get(settings),
S3Repository.Repositories.REGION_SETTING.get(settings),
S3Repository.Repositories.KEY_SETTING.get(settings),
S3Repository.Repositories.SECRET_SETTING.get(settings),
null, randomBoolean(), null);
String bucketName = bucket.get("bucket");
@ -466,15 +468,13 @@ public abstract class AbstractS3SnapshotRestoreTest extends AbstractAwsTestCase
String endpoint = bucket.get("endpoint", S3Repository.Repositories.ENDPOINT_SETTING.get(settings));
Protocol protocol = S3Repository.Repositories.PROTOCOL_SETTING.get(settings);
String region = bucket.get("region", S3Repository.Repositories.REGION_SETTING.get(settings));
String accessKey = bucket.get("access_key", S3Repository.Repositories.KEY_SETTING.get(settings));
String secretKey = bucket.get("secret_key", S3Repository.Repositories.SECRET_SETTING.get(settings));
String bucketName = bucket.get("bucket");
// We check that settings has been set in elasticsearch.yml integration test file
// as described in README
assertThat("Your settings in elasticsearch.yml are incorrects. Check README file.", bucketName, notNullValue());
AmazonS3 client = internalCluster().getInstance(AwsS3Service.class).client(endpoint, protocol, region, accessKey, secretKey,
null, randomBoolean(), null);
AmazonS3 client = internalCluster().getInstance(AwsS3Service.class).client(Settings.EMPTY, endpoint, protocol, region, null,
randomBoolean(), null);
try {
ObjectListing prevListing = null;
//From http://docs.amazonwebservices.com/AmazonS3/latest/dev/DeletingMultipleObjectsUsingJava.html

2
plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3RepositoryTests.java
View File

@ -58,7 +58,7 @@ public class S3RepositoryTests extends ESTestCase {
@Override
protected void doClose() {}
@Override
public AmazonS3 client(String endpoint, Protocol protocol, String region, String account, String key, Integer maxRetries,
public AmazonS3 client(Settings repositorySettings, String endpoint, Protocol protocol, String region, Integer maxRetries,
boolean useThrottleRetries, Boolean pathStyleAccess) {
return new DummyS3Client();
}