Browse Source

S3 Repository: Remove bucket auto create (#22846)

closes #22761
Ryan Ernst 8 years ago
parent
commit
fe4043c8ff

+ 3 - 0
docs/reference/migration/migrate_6_0/plugins.asciidoc

@@ -8,6 +8,9 @@ You can use {plugins}/ingest-attachment.html[ingest attachment plugin] instead.
 
 ==== S3 Repository plugin
 
+* The bucket an s3 repository is configured with will no longer be created automatically.
+It must exist before the s3 repository is created.
+
 * Support for specifying s3 credentials through environment variables and
 system properties has been removed. Use the `elasticsearch-keystore` tool
 to securely store the credentials.

+ 2 - 9
plugins/repository-s3/src/main/java/org/elasticsearch/cloud/aws/InternalAwsS3Service.java

@@ -162,7 +162,8 @@ public class InternalAwsS3Service extends AbstractLifecycleComponent implements
     // pkg private for tests
     /** Returns the endpoint the client should use, based on the available endpoint settings found. */
     static String findEndpoint(Logger logger, Settings repositorySettings, Settings settings, String clientName) {
-        String region = getRegion(repositorySettings, settings);
+        String region = getConfigValue(repositorySettings, settings, CLIENT_NAME.get(repositorySettings), S3Repository.REGION_SETTING,
+                                       S3Repository.Repository.REGION_SETTING, S3Repository.Repositories.REGION_SETTING);
         String endpoint = getConfigValue(repositorySettings, settings, clientName, S3Repository.ENDPOINT_SETTING,
                                          S3Repository.Repository.ENDPOINT_SETTING, S3Repository.Repositories.ENDPOINT_SETTING);
         if (Strings.isNullOrEmpty(endpoint)) {
@@ -188,14 +189,6 @@ public class InternalAwsS3Service extends AbstractLifecycleComponent implements
         return endpoint;
     }
 
-    /**
-     * Return the region configured, or empty string.
-     * TODO: remove after https://github.com/elastic/elasticsearch/issues/22761 */
-    public static String getRegion(Settings repositorySettings, Settings settings) {
-        return getConfigValue(repositorySettings, settings, CLIENT_NAME.get(repositorySettings), S3Repository.REGION_SETTING,
-                              S3Repository.Repository.REGION_SETTING, S3Repository.Repositories.REGION_SETTING);
-    }
-
     private static String getEndpoint(String region) {
         final String endpoint;
         switch (region) {

+ 5 - 27
plugins/repository-s3/src/main/java/org/elasticsearch/cloud/aws/blobstore/S3BlobStore.java

@@ -50,8 +50,6 @@ public class S3BlobStore extends AbstractComponent implements BlobStore {
 
     private final String bucket;
 
-    private final String region;
-
     private final ByteSizeValue bufferSize;
 
     private final boolean serverSideEncryption;
@@ -62,12 +60,11 @@ public class S3BlobStore extends AbstractComponent implements BlobStore {
 
     private final StorageClass storageClass;
 
-    public S3BlobStore(Settings settings, AmazonS3 client, String bucket, @Nullable String region, boolean serverSideEncryption,
+    public S3BlobStore(Settings settings, AmazonS3 client, String bucket, boolean serverSideEncryption,
                        ByteSizeValue bufferSize, int maxRetries, String cannedACL, String storageClass) {
         super(settings);
         this.client = client;
         this.bucket = bucket;
-        this.region = region;
         this.serverSideEncryption = serverSideEncryption;
         this.bufferSize = bufferSize;
         this.cannedACL = initCannedACL(cannedACL);
@@ -80,35 +77,16 @@ public class S3BlobStore extends AbstractComponent implements BlobStore {
         // client is not able to distinguish between bucket permission errors and
         // invalid credential errors, and this method could return an incorrect result.
         SocketAccess.doPrivilegedVoid(() -> {
-            int retry = 0;
-            while (retry <= maxRetries) {
-                try {
-                    if (!client.doesBucketExist(bucket)) {
-                        CreateBucketRequest request;
-                        if (region != null) {
-                            request = new CreateBucketRequest(bucket, region);
-                        } else {
-                            request = new CreateBucketRequest(bucket);
-                        }
-                        request.setCannedAcl(this.cannedACL);
-                        client.createBucket(request);
-                    }
-                    break;
-                } catch (AmazonClientException e) {
-                    if (shouldRetry(e) && retry < maxRetries) {
-                        retry++;
-                    } else {
-                        logger.debug("S3 client create bucket failed");
-                        throw e;
-                    }
-                }
+            if (client.doesBucketExist(bucket) == false) {
+                throw new IllegalArgumentException("The bucket [" + bucket + "] does not exist. Please create it before " +
+                                                   " creating an s3 snapshot repository backed by it.");
             }
         });
     }
 
     @Override
     public String toString() {
-        return (region == null ? "" : region + "/") + bucket;
+        return bucket;
     }
 
     public AmazonS3 client() {

+ 1 - 3
plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java

@@ -359,9 +359,7 @@ public class S3Repository extends BlobStoreRepository {
             storageClass, pathStyleAccess);
 
         AmazonS3 client = s3Service.client(metadata.settings(), maxRetries, useThrottleRetries, pathStyleAccess);
-        String region = InternalAwsS3Service.getRegion(metadata.settings(), settings);
-        blobStore = new S3BlobStore(settings, client,
-                bucket, region, serverSideEncryption, bufferSize, maxRetries, cannedACL, storageClass);
+        blobStore = new S3BlobStore(settings, client, bucket, serverSideEncryption, bufferSize, maxRetries, cannedACL, storageClass);
 
         String basePath = getValue(metadata.settings(), settings, Repository.BASE_PATH_SETTING, Repositories.BASE_PATH_SETTING);
         if (Strings.hasLength(basePath)) {

+ 1 - 1
plugins/repository-s3/src/test/java/org/elasticsearch/cloud/aws/blobstore/S3BlobStoreContainerTests.java

@@ -33,7 +33,7 @@ public class S3BlobStoreContainerTests extends ESBlobStoreContainerTestCase {
         MockAmazonS3 client = new MockAmazonS3();
         String bucket = randomAsciiOfLength(randomIntBetween(1, 10)).toLowerCase(Locale.ROOT);
 
-        return new S3BlobStore(Settings.EMPTY, client, bucket, null, false,
+        return new S3BlobStore(Settings.EMPTY, client, bucket, false,
             new ByteSizeValue(10, ByteSizeUnit.MB), 5, "public-read-write", "standard");
     }
 }