From 49b6c3007bc1f6acb3e12a6f5d7ae7a61bec6f92 Mon Sep 17 00:00:00 2001 From: Ali Beyad Date: Tue, 11 Oct 2016 10:36:31 -0400 Subject: [PATCH] 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. --- .../elasticsearch/repositories/s3/S3Repository.java | 9 +++++---- .../repositories/s3/S3RepositoryTests.java | 13 +++++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java index b1471e417f9cf..34e4d78f8cf31 100644 --- a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java +++ b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java @@ -306,11 +306,12 @@ public S3Repository(RepositoryMetaData metadata, Settings settings, AwsS3Service 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(); } diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3RepositoryTests.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3RepositoryTests.java index f8940c6158c8f..915183888b6da 100644 --- a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3RepositoryTests.java +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3RepositoryTests.java @@ -104,4 +104,17 @@ private void assertInvalidBuffer(int bufferMB, int chunkMB, Class 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 + } + }