Add repository-url module and move URLRepository

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy

@@ -276,8 +276,7 @@ class ClusterFormationTasks {
                 'path.repo'                    : "${node.sharedDir}/repo",
                 'path.shared_data'             : "${node.sharedDir}/",
                 // Define a node attribute so we can test that it exists
-                'node.attr.testattr'           : 'test',
-                'repositories.url.allowed_urls': 'http://snapshot.test*'
+                'node.attr.testattr'           : 'test'
         ]
         // we set min master nodes to the total number of nodes in the cluster and
         // basically skip initial state recovery to allow the cluster to form using a realistic master election

buildSrc/src/main/resources/checkstyle_suppressions.xml

@@ -256,7 +256,6 @@
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]Base64.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]Numbers.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]blobstore[/\\]fs[/\\]FsBlobStore.java" checks="LineLength" />
-  <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]blobstore[/\\]url[/\\]URLBlobStore.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]bytes[/\\]BytesArray.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]bytes[/\\]PagedBytesReference.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]cache[/\\]Cache.java" checks="LineLength" />
@@ -437,7 +436,6 @@
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]repositories[/\\]blobstore[/\\]ChecksumBlobStoreFormat.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]repositories[/\\]fs[/\\]FsRepository.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]repositories[/\\]uri[/\\]URLIndexShardRepository.java" checks="LineLength" />
-  <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]repositories[/\\]uri[/\\]URLRepository.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]rest[/\\]RestController.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]rest[/\\]action[/\\]cat[/\\]RestCountAction.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]rest[/\\]action[/\\]cat[/\\]RestIndicesAction.java" checks="LineLength" />

core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java

@@ -81,7 +81,6 @@
 import org.elasticsearch.node.Node;
 import org.elasticsearch.plugins.PluginsService;
 import org.elasticsearch.repositories.fs.FsRepository;
-import org.elasticsearch.repositories.uri.URLRepository;
 import org.elasticsearch.rest.BaseRestHandler;
 import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.search.SearchModule;
@@ -348,9 +347,6 @@ public void apply(Settings value, Settings current, Settings previous) {
                     Node.NODE_INGEST_SETTING,
                     Node.NODE_ATTRIBUTES,
                     Node.NODE_LOCAL_STORAGE_SETTING,
-                    URLRepository.ALLOWED_URLS_SETTING,
-                    URLRepository.REPOSITORIES_URL_SETTING,
-                    URLRepository.SUPPORTED_PROTOCOLS_SETTING,
                     TransportMasterNodeReadAction.FORCE_LOCAL_SETTING,
                     AutoCreateIndex.AUTO_CREATE_INDEX_SETTING,
                     BaseRestHandler.MULTI_ALLOW_EXPLICIT_INDEX,

core/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java

@@ -19,24 +19,22 @@
 
 package org.elasticsearch.repositories;
 
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
 import org.elasticsearch.action.admin.cluster.snapshots.status.TransportNodesSnapshotsStatus;
 import org.elasticsearch.common.inject.AbstractModule;
-import org.elasticsearch.common.inject.binder.LinkedBindingBuilder;
 import org.elasticsearch.common.inject.multibindings.MapBinder;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.plugins.RepositoryPlugin;
 import org.elasticsearch.repositories.fs.FsRepository;
-import org.elasticsearch.repositories.uri.URLRepository;
 import org.elasticsearch.snapshots.RestoreService;
 import org.elasticsearch.snapshots.SnapshotShardsService;
 import org.elasticsearch.snapshots.SnapshotsService;
 
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 /**
  * Sets up classes for Snapshot/Restore.
  */
@@ -47,7 +45,6 @@ public class RepositoriesModule extends AbstractModule {
     public RepositoriesModule(Environment env, List<RepositoryPlugin> repoPlugins, NamedXContentRegistry namedXContentRegistry) {
         Map<String, Repository.Factory> factories = new HashMap<>();
         factories.put(FsRepository.TYPE, (metadata) -> new FsRepository(metadata, env, namedXContentRegistry));
-        factories.put(URLRepository.TYPE, (metadata) -> new URLRepository(metadata, env, namedXContentRegistry));
 
         for (RepositoryPlugin repoPlugin : repoPlugins) {
             Map<String, Repository.Factory> newRepoTypes = repoPlugin.getRepositories(env, namedXContentRegistry);

core/src/test/java/org/elasticsearch/bwcompat/RestoreBackwardsCompatIT.java

@@ -26,21 +26,22 @@
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.cluster.metadata.IndexTemplateMetaData;
 import org.elasticsearch.cluster.routing.allocation.decider.FilterAllocationDecider;
+import org.elasticsearch.common.io.FileTestUtils;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
-import org.elasticsearch.repositories.uri.URLRepository;
+import org.elasticsearch.repositories.fs.FsRepository;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.snapshots.AbstractSnapshotIntegTestCase;
 import org.elasticsearch.snapshots.RestoreInfo;
 import org.elasticsearch.snapshots.SnapshotInfo;
 import org.elasticsearch.snapshots.SnapshotRestoreException;
+import org.elasticsearch.snapshots.mockstore.MockRepository;
 import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
 import org.elasticsearch.test.ESIntegTestCase.Scope;
 import org.elasticsearch.test.VersionUtils;
+import org.junit.BeforeClass;
 
 import java.io.IOException;
-import java.net.URI;
-import java.net.URISyntaxException;
 import java.nio.file.DirectoryStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -61,27 +62,19 @@
 @ClusterScope(scope = Scope.TEST)
 public class RestoreBackwardsCompatIT extends AbstractSnapshotIntegTestCase {
 
+    private static Path repoPath;
+
     @Override
     protected Settings nodeSettings(int nodeOrdinal) {
-        if (randomBoolean()) {
-            // Configure using path.repo
-            return Settings.builder()
-                    .put(super.nodeSettings(nodeOrdinal))
-                    .put(Environment.PATH_REPO_SETTING.getKey(), getBwcIndicesPath())
-                    .build();
-        } else {
-            // Configure using url white list
-            try {
-                URI repoJarPatternUri = new URI("jar:" + getBwcIndicesPath().toUri().toString() + "*.zip!/repo/");
-                return Settings.builder()
-                        .put(super.nodeSettings(nodeOrdinal))
-                        .putArray(URLRepository.ALLOWED_URLS_SETTING.getKey(), repoJarPatternUri.toString())
-                        .build();
-            } catch (URISyntaxException ex) {
-                throw new IllegalArgumentException(ex);
-            }
+        return Settings.builder()
+            .put(super.nodeSettings(nodeOrdinal))
+            .put(Environment.PATH_REPO_SETTING.getKey(), repoPath)
+            .build();
+    }
 
-        }
+    @BeforeClass
+    public static void repoSetup() throws IOException {
+        repoPath = createTempDir("repositories");
     }
 
     public void testRestoreOldSnapshots() throws Exception {
@@ -151,13 +144,14 @@ private List<String> listRepoVersions(String prefix) throws Exception {
     }
 
     private void createRepo(String prefix, String version, String repo) throws Exception {
-        Path repoFile = getBwcIndicesPath().resolve(prefix + "-" + version + ".zip");
-        URI repoFileUri = repoFile.toUri();
-        URI repoJarUri = new URI("jar:" + repoFileUri.toString() + "!/repo/");
+        Path repoFileFromBuild = getBwcIndicesPath().resolve(prefix + "-" + version + ".zip");
+        String repoFileName = repoFileFromBuild.getFileName().toString().split(".zip")[0];
+        Path fsRepoPath = repoPath.resolve(repoFileName);
+        FileTestUtils.unzip(repoFileFromBuild, fsRepoPath, null);
         logger.info("-->  creating repository [{}] for version [{}]", repo, version);
         assertAcked(client().admin().cluster().preparePutRepository(repo)
-                .setType("url").setSettings(Settings.builder()
-                        .put("url", repoJarUri.toString())));
+            .setType(MockRepository.TYPE).setSettings(Settings.builder()
+                .put(FsRepository.REPOSITORIES_LOCATION_SETTING.getKey(), fsRepoPath.getParent().relativize(fsRepoPath).resolve("repo").toString())));
     }
 
     private void testOldSnapshot(String version, String repo, String snapshot) throws IOException {
@@ -198,7 +192,7 @@ private void testOldSnapshot(String version, String repo, String snapshot) throw
                 equalTo("{\"type1\":{\"_source\":{\"enabled\":0}}}"),
                 equalTo("{\"type1\":{\"_source\":{\"enabled\":\"off\"}}}"),
                 equalTo("{\"type1\":{\"_source\":{\"enabled\":\"no\"}}}")
-        ));
+            ));
         assertThat(template.aliases().size(), equalTo(3));
         assertThat(template.aliases().get("alias1"), notNullValue());
         assertThat(template.aliases().get("alias2").filter().string(), containsString(version));

core/src/test/java/org/elasticsearch/snapshots/RepositoriesIT.java

@@ -142,30 +142,6 @@ public void testMisconfiguredRepository() throws Exception {
         } catch (RepositoryException ex) {
             assertThat(ex.toString(), containsString("location [" + location + "] doesn't match any of the locations specified by path.repo"));
         }
-
-        String repoUrl = invalidRepoPath.toAbsolutePath().toUri().toURL().toString();
-        String unsupportedUrl = repoUrl.replace("file:/", "netdoc:/");
-        logger.info("--> trying creating url repository with unsupported url protocol");
-        try {
-            client().admin().cluster().preparePutRepository("test-repo")
-                    .setType("url").setSettings(Settings.builder().put("url", unsupportedUrl))
-                    .get();
-            fail("Shouldn't be here");
-        } catch (RepositoryException ex) {
-            assertThat(ex.toString(),
-                either(containsString("unsupported url protocol [netdoc]"))
-                    .or(containsString("unknown protocol: netdoc"))); // newer versions of JDK 9
-        }
-
-        logger.info("--> trying creating url repository with location that is not registered in path.repo setting");
-        try {
-            client().admin().cluster().preparePutRepository("test-repo")
-                    .setType("url").setSettings(Settings.builder().put("url", invalidRepoPath.toUri().toURL()))
-                    .get();
-            fail("Shouldn't be here");
-        } catch (RepositoryException ex) {
-            assertThat(ex.toString(), containsString("doesn't match any of the locations specified by path.repo"));
-        }
     }
 
     public void testRepositoryAckTimeout() throws Exception {

core/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java

@@ -1442,63 +1442,6 @@ public void testDeleteRepositoryWhileSnapshotting() throws Exception {
         assertThat(client.prepareSearch("test-idx").setSize(0).get().getHits().totalHits(), equalTo(100L));
     }
 
-    public void testUrlRepository() throws Exception {
-        Client client = client();
-
-        logger.info("-->  creating repository");
-        Path repositoryLocation = randomRepoPath();
-        assertAcked(client.admin().cluster().preparePutRepository("test-repo")
-                .setType("fs").setSettings(Settings.builder()
-                        .put("location", repositoryLocation)
-                        .put("compress", randomBoolean())
-                        .put("chunk_size", randomIntBetween(100, 1000), ByteSizeUnit.BYTES)));
-
-        createIndex("test-idx");
-        ensureGreen();
-
-        logger.info("--> indexing some data");
-        for (int i = 0; i < 100; i++) {
-            index("test-idx", "doc", Integer.toString(i), "foo", "bar" + i);
-        }
-        refresh();
-        assertThat(client.prepareSearch("test-idx").setSize(0).get().getHits().totalHits(), equalTo(100L));
-
-        logger.info("--> snapshot");
-        CreateSnapshotResponse createSnapshotResponse = client.admin().cluster().prepareCreateSnapshot("test-repo", "test-snap").setWaitForCompletion(true).setIndices("test-idx").get();
-        assertThat(createSnapshotResponse.getSnapshotInfo().successfulShards(), greaterThan(0));
-        assertThat(createSnapshotResponse.getSnapshotInfo().successfulShards(), equalTo(createSnapshotResponse.getSnapshotInfo().totalShards()));
-
-        assertThat(client.admin().cluster().prepareGetSnapshots("test-repo").setSnapshots("test-snap").get().getSnapshots().get(0).state(), equalTo(SnapshotState.SUCCESS));
-
-        logger.info("--> delete index");
-        cluster().wipeIndices("test-idx");
-
-        logger.info("--> create read-only URL repository");
-        assertAcked(client.admin().cluster().preparePutRepository("url-repo")
-                .setType("url").setSettings(Settings.builder()
-                        .put("url", repositoryLocation.toUri().toURL())
-                        .put("list_directories", randomBoolean())));
-        logger.info("--> restore index after deletion");
-        RestoreSnapshotResponse restoreSnapshotResponse = client.admin().cluster().prepareRestoreSnapshot("url-repo", "test-snap").setWaitForCompletion(true).setIndices("test-idx").execute().actionGet();
-        assertThat(restoreSnapshotResponse.getRestoreInfo().totalShards(), greaterThan(0));
-
-        assertThat(client.prepareSearch("test-idx").setSize(0).get().getHits().totalHits(), equalTo(100L));
-
-        logger.info("--> list available shapshots");
-        GetSnapshotsResponse getSnapshotsResponse = client.admin().cluster().prepareGetSnapshots("url-repo").get();
-        assertThat(getSnapshotsResponse.getSnapshots(), notNullValue());
-        assertThat(getSnapshotsResponse.getSnapshots().size(), equalTo(1));
-
-        logger.info("--> delete snapshot");
-        DeleteSnapshotResponse deleteSnapshotResponse = client.admin().cluster().prepareDeleteSnapshot("test-repo", "test-snap").get();
-        assertAcked(deleteSnapshotResponse);
-
-        logger.info("--> list available shapshot again, no snapshots should be returned");
-        getSnapshotsResponse = client.admin().cluster().prepareGetSnapshots("url-repo").get();
-        assertThat(getSnapshotsResponse.getSnapshots(), notNullValue());
-        assertThat(getSnapshotsResponse.getSnapshots().size(), equalTo(0));
-    }
-
     @TestLogging("_root:DEBUG")  // this fails every now and then: https://github.com/elastic/elasticsearch/issues/18121 but without
     // more logs we cannot find out why
     public void testReadonlyRepository() throws Exception {

modules/repository-url/build.gradle

@@ -0,0 +1,32 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+esplugin {
+    description 'Module for generic repositories'
+    classname 'org.elasticsearch.plugin.repository.url.URLRepositoryPlugin'
+}
+
+compileJava.options.compilerArgs << "-Xlint:-unchecked,-rawtypes"
+compileTestJava.options.compilerArgs << "-Xlint:-unchecked,-rawtypes"
+
+integTest {
+    cluster {
+        setting 'repositories.url.allowed_urls', 'http://snapshot.test*'
+    }
+}

core/src/main/java/org/elasticsearch/common/blobstore/url/URLBlobStore.java → modules/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/URLBlobStore.java

@@ -55,7 +55,8 @@ public class URLBlobStore extends AbstractComponent implements BlobStore {
     public URLBlobStore(Settings settings, URL path) {
         super(settings);
         this.path = path;
-        this.bufferSizeInBytes = (int) settings.getAsBytesSize("repositories.uri.buffer_size", new ByteSizeValue(100, ByteSizeUnit.KB)).getBytes();
+        this.bufferSizeInBytes = (int) settings.getAsBytesSize("repositories.uri.buffer_size",
+            new ByteSizeValue(100, ByteSizeUnit.KB)).getBytes();
     }
 
     /**

modules/repository-url/src/main/java/org/elasticsearch/plugin/repository/url/URLRepositoryPlugin.java

@@ -0,0 +1,50 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.plugin.repository.url;
+
+import org.elasticsearch.common.settings.Setting;
+import org.elasticsearch.common.xcontent.NamedXContentRegistry;
+import org.elasticsearch.env.Environment;
+import org.elasticsearch.plugins.Plugin;
+import org.elasticsearch.plugins.RepositoryPlugin;
+import org.elasticsearch.repositories.Repository;
+import org.elasticsearch.repositories.url.URLRepository;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+public class URLRepositoryPlugin extends Plugin implements RepositoryPlugin {
+
+    @Override
+    public List<Setting<?>> getSettings() {
+        return Arrays.asList(
+            URLRepository.ALLOWED_URLS_SETTING,
+            URLRepository.REPOSITORIES_URL_SETTING,
+            URLRepository.SUPPORTED_PROTOCOLS_SETTING
+        );
+    }
+
+    @Override
+    public Map<String, Repository.Factory> getRepositories(Environment env, NamedXContentRegistry namedXContentRegistry) {
+        return Collections.singletonMap(URLRepository.TYPE, metadata -> new URLRepository(metadata, env, namedXContentRegistry));
+    }
+}