Browse Source

Fixes leading forward slash in S3 repository base_path (#20861)

In 2.x, the S3 repository accepted a `/` (forward slash) to start
the repositories.s3.base_path, and it used a different string splitting
method that removed the forward slash from the base path, so there
were no issues.

In 5.x, we removed this custom string splitting method in favor of
the JDK's string splitting method, which preserved the leading `/`.
The AWS SDK does not like the leading `/` in the key path after the
bucket name, and so it could not find any objects in the S3 repository.

This commit fixes the issue by removing the leading `/` if it exists
and adding a deprecation notice that leading `/` will not be supported
in the future in S3 repository's base_path.
Ali Beyad 9 years ago
parent
commit
bbf6e6d0bd

+ 5 - 4
plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java

@@ -306,11 +306,12 @@ public class S3Repository extends BlobStoreRepository {
 
         String basePath = getValue(metadata.settings(), settings, Repository.BASE_PATH_SETTING, Repositories.BASE_PATH_SETTING);
         if (Strings.hasLength(basePath)) {
-            BlobPath path = new BlobPath();
-            for(String elem : basePath.split("/")) {
-                path = path.add(elem);
+            if (basePath.startsWith("/")) {
+                basePath = basePath.substring(1);
+                deprecationLogger.deprecated("S3 repository base_path trimming the leading `/`, and " +
+                                                 "leading `/` will not be supported for the S3 repository in future releases");
             }
-            this.basePath = path;
+            this.basePath = new BlobPath().add(basePath);
         } else {
             this.basePath = BlobPath.cleanPath();
         }

+ 13 - 0
plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3RepositoryTests.java

@@ -104,4 +104,17 @@ public class S3RepositoryTests extends ESTestCase {
         Exception e = expectThrows(clazz, () -> new S3Repository(metadata, Settings.EMPTY, new DummyS3Service()));
         assertThat(e.getMessage(), containsString(msg));
     }
+
+    public void testBasePathSetting() throws IOException {
+        RepositoryMetaData metadata = new RepositoryMetaData("dummy-repo", "mock", Settings.builder()
+            .put(Repository.BASE_PATH_SETTING.getKey(), "/foo/bar").build());
+        S3Repository s3repo = new S3Repository(metadata, Settings.EMPTY, new DummyS3Service());
+        assertEquals("foo/bar/", s3repo.basePath().buildAsString()); // make sure leading `/` is removed and trailing is added
+
+        metadata = new RepositoryMetaData("dummy-repo", "mock", Settings.EMPTY);
+        Settings settings = Settings.builder().put(Repositories.BASE_PATH_SETTING.getKey(), "/foo/bar").build();
+        s3repo = new S3Repository(metadata, settings, new DummyS3Service());
+        assertEquals("foo/bar/", s3repo.basePath().buildAsString()); // make sure leading `/` is removed and trailing is added
+    }
+
 }