Add integ tests for ftp:// URL repository (#126757)

We document support for snapshot repositories using `ftp://` URLs but it
seems this functionality has not worked for many years because of
security-manager restrictions, although nobody noticed because it was
not covered by any tests. The migration to the Entitlements framework
means that this functionality now works again, so this commit adds tests
to make sure we do not break it again in future.

build-tools-internal/version.properties

@@ -41,6 +41,7 @@ junit             = 4.13.2
 junit5            = 5.12.1
 hamcrest          = 3.0
 mocksocket        = 1.2
+apache_mina       = 2.2.4
 
 # test container dependencies
 testcontainer     = 1.19.2

gradle/verification-metadata.xml

@@ -2388,6 +2388,16 @@
             <sha256 value="24c0b2d8081cbc9624e60a1c19f1dd0d104e014cdba4038d9b1aed0ab63721c6" origin="Generated by Gradle"/>
          </artifact>
       </component>
+      <component group="org.apache.ftpserver" name="ftplet-api" version="1.2.1">
+         <artifact name="ftplet-api-1.2.1.jar">
+            <sha256 value="36e47834dfa4225448aaaa5460b304f26e99e837c4a80ef5b41d090735be9b07" origin="Generated by Gradle"/>
+         </artifact>
+      </component>
+      <component group="org.apache.ftpserver" name="ftpserver-core" version="1.2.1">
+         <artifact name="ftpserver-core-1.2.1.jar">
+            <sha256 value="18279abb21abd74e10bb60d5ac6fa044044cc40f1b34d62418e906f50d4212ec" origin="Generated by Gradle"/>
+         </artifact>
+      </component>
       <component group="org.apache.geronimo.specs" name="geronimo-jcache_1.0_spec" version="1.0-alpha-1">
          <artifact name="geronimo-jcache_1.0_spec-1.0-alpha-1.jar">
             <sha256 value="0070a12e58f491b95719391325299a6294530ee6c3ce25e50bdc98b0b700966c" origin="Generated by Gradle"/>
@@ -3196,9 +3206,9 @@
             <sha256 value="44a60c610f4e31524b03d81a698b1ecceba116320ea510babf859575b2ea7233" origin="Generated by Gradle"/>
          </artifact>
       </component>
-      <component group="org.apache.mina" name="mina-core" version="2.0.17">
+      <component group="org.apache.mina" name="mina-core" version="2.2.4">
-         <artifact name="mina-core-2.0.17.jar">
+         <artifact name="mina-core-2.2.4.jar">
-            <sha256 value="08316826fa2b9357b061e52fa8f19ccae75420c949ebe29e28759d2bddd9b39b" origin="Generated by Gradle"/>
+            <sha256 value="39b2dfc8e84380bf7adab657d3d5e1625cb6592a885ebdb854ec5c6f7a3ec88d" origin="Generated by Gradle"/>
          </artifact>
       </component>
       <component group="org.apache.pdfbox" name="fontbox" version="2.0.31">

modules/repository-url/build.gradle

@@ -10,7 +10,6 @@
 import org.elasticsearch.gradle.PropertyNormalization
 
 apply plugin: 'elasticsearch.internal-yaml-rest-test'
-apply plugin: 'elasticsearch.yaml-rest-compat-test'
 apply plugin: 'elasticsearch.internal-cluster-test'
 
 esplugin {
@@ -31,7 +30,7 @@ dependencies {
   api "commons-codec:commons-codec:${versions.commonscodec}"
   api "org.apache.logging.log4j:log4j-1.2-api:${versions.log4j}"
   yamlRestTestImplementation project(':test:fixtures:url-fixture')
-  internalClusterTestImplementation project(':test:fixtures:url-fixture')
+  yamlRestTestImplementation project(':modules:repository-url')
 }
 
 tasks.named("thirdPartyAudit").configure {

modules/repository-url/src/yamlRestTest/java/org/elasticsearch/repositories/url/RepositoryURLClientYamlTestSuiteIT.java

@@ -10,42 +10,35 @@
 package org.elasticsearch.repositories.url;
 
 import fixture.url.URLFixture;
+import io.netty.handler.codec.http.HttpMethod;
 
 import com.carrotsearch.randomizedtesting.annotations.Name;
 import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 
-import org.apache.http.HttpEntity;
-import org.apache.http.entity.ContentType;
-import org.apache.http.nio.entity.NStringEntity;
 import org.elasticsearch.client.Request;
 import org.elasticsearch.client.Response;
-import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.core.PathUtils;
 import org.elasticsearch.repositories.fs.FsRepository;
-import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.test.cluster.ElasticsearchCluster;
+import org.elasticsearch.test.rest.ESRestTestCase;
 import org.elasticsearch.test.rest.yaml.ClientYamlTestCandidate;
 import org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase;
-import org.elasticsearch.xcontent.ToXContent;
-import org.elasticsearch.xcontent.XContentBuilder;
 import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.rules.RuleChain;
 import org.junit.rules.TestRule;
 
 import java.io.IOException;
-import java.net.InetAddress;
 import java.net.URI;
-import java.net.URL;
 import java.util.List;
 import java.util.Map;
+import java.util.function.UnaryOperator;
 
-import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
-import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.startsWith;
 
 public class RepositoryURLClientYamlTestSuiteIT extends ESClientYamlSuiteTestCase {
 
@@ -54,7 +47,7 @@ public class RepositoryURLClientYamlTestSuiteIT extends ESClientYamlSuiteTestCas
     public static ElasticsearchCluster cluster = ElasticsearchCluster.local()
         .module("repository-url")
         .setting("path.repo", urlFixture::getRepositoryDir)
-        .setting("repositories.url.allowed_urls", () -> "http://snapshot.test*, " + urlFixture.getAddress())
+        .setting("repositories.url.allowed_urls", () -> "http://snapshot.test*, " + urlFixture.getAddress() + "," + urlFixture.getFtpUrl())
         .build();
 
     @ClassRule
@@ -75,11 +68,16 @@ public class RepositoryURLClientYamlTestSuiteIT extends ESClientYamlSuiteTestCas
     }
 
     /**
-     * This method registers 3 snapshot/restore repositories:
+     * This method registers 4 snapshot/restore repositories:
-     * - repository-fs: this FS repository is used to create snapshots.
+     * <ol>
-     * - repository-url: this URL repository is used to restore snapshots created using the previous repository. It uses
+     * <li>{@code repository-fs}: this FS repository is used to create snapshots.</li>
-     * the URLFixture to restore snapshots over HTTP.
+     * <li>{@code repository-url-http}: this URL repository is used to restore snapshots created using the previous repository. It uses
-     * - repository-file: similar as the previous repository but using a file:// prefix instead of http://.
+     *     the {@link URLFixture} to restore snapshots over HTTP.</li>
+     * <li>{@code repository-url-file}: similar as the previous repository but using the {@code file://} scheme instead of
+     *     {@code http://}.</li>
+     * <li>{@code repository-url-ftp}: similar as the previous repository but using the {@code ftp://} scheme instead of
+     *     {@code http://}.</li>
+     * </ol>
      **/
     @Before
     public void registerRepositories() throws IOException {
@@ -97,54 +95,25 @@ public class RepositoryURLClientYamlTestSuiteIT extends ESClientYamlSuiteTestCas
         final String pathRepo = pathRepos.get(0);
         final URI pathRepoUri = PathUtils.get(pathRepo).toUri().normalize();
 
-        // Create a FS repository using the path.repo location
+        createRepository("repository-fs", FsRepository.TYPE, b -> b.put("location", pathRepo));
-        Request createFsRepositoryRequest = new Request("PUT", "/_snapshot/repository-fs");
+        createUrlRepository("file", pathRepoUri.toString());
-        createFsRepositoryRequest.setEntity(
+        createUrlRepository("http", urlFixture.getAddress());
-            buildRepositorySettings(FsRepository.TYPE, Settings.builder().put("location", pathRepo).build())
+        createUrlRepository("ftp", urlFixture.getFtpUrl());
-        );
-        Response createFsRepositoryResponse = client().performRequest(createFsRepositoryRequest);
-        assertThat(createFsRepositoryResponse.getStatusLine().getStatusCode(), equalTo(RestStatus.OK.getStatus()));
-
-        // Create a URL repository using the file://{path.repo} URL
-        Request createFileRepositoryRequest = new Request("PUT", "/_snapshot/repository-file");
-        createFileRepositoryRequest.setEntity(
-            buildRepositorySettings("url", Settings.builder().put("url", pathRepoUri.toString()).build())
-        );
-        Response createFileRepositoryResponse = client().performRequest(createFileRepositoryRequest);
-        assertThat(createFileRepositoryResponse.getStatusLine().getStatusCode(), equalTo(RestStatus.OK.getStatus()));
-
-        // Create a URL repository using the http://{fixture} URL
-        @SuppressWarnings("unchecked")
-        List<String> allowedUrls = (List<String>) XContentMapValues.extractValue("defaults.repositories.url.allowed_urls", clusterSettings);
-        for (String allowedUrl : allowedUrls) {
-            try {
-                InetAddress inetAddress = InetAddress.getByName(new URL(allowedUrl).getHost());
-                if (inetAddress.isAnyLocalAddress() || inetAddress.isLoopbackAddress()) {
-                    Request createUrlRepositoryRequest = new Request("PUT", "/_snapshot/repository-url");
-                    createUrlRepositoryRequest.setEntity(buildRepositorySettings("url", Settings.builder().put("url", allowedUrl).build()));
-                    Response createUrlRepositoryResponse = client().performRequest(createUrlRepositoryRequest);
-                    assertThat(createUrlRepositoryResponse.getStatusLine().getStatusCode(), equalTo(RestStatus.OK.getStatus()));
-                    break;
-                }
-            } catch (Exception e) {
-                logger.debug("Failed to resolve inet address for allowed URL [{}], skipping", allowedUrl);
-            }
-        }
     }
 
-    private static HttpEntity buildRepositorySettings(final String type, final Settings settings) throws IOException {
+    private static void createUrlRepository(final String nameSuffix, final String url) throws IOException {
-        try (XContentBuilder builder = jsonBuilder()) {
+        assertThat(url, startsWith(nameSuffix + "://"));
-            builder.startObject();
+        createRepository("repository-url-" + nameSuffix, URLRepository.TYPE, b -> b.put("url", url));
-            {
+    }
-                builder.field("type", type);
+
-                builder.startObject("settings");
+    private static void createRepository(final String name, final String type, final UnaryOperator<Settings.Builder> settings)
-                {
+        throws IOException {
-                    settings.toXContent(builder, ToXContent.EMPTY_PARAMS);
+        assertOK(client().performRequest(ESRestTestCase.newXContentRequest(HttpMethod.PUT, "/_snapshot/" + name, (builder, params) -> {
-                }
+            builder.field("type", type);
-                builder.endObject();
+            builder.startObject("settings");
-            }
+            settings.apply(Settings.builder()).build().toXContent(builder, params);
             builder.endObject();
-            return new NStringEntity(Strings.toString(builder), ContentType.APPLICATION_JSON);
+            return builder;
-        }
+        })));
     }
 }

modules/repository-url/src/yamlRestTest/resources/rest-api-spec/test/repository_url/10_basic.yml

@@ -116,14 +116,14 @@ teardown:
   # Ensure that the URL repository is registered
   - do:
       snapshot.get_repository:
-        repository: repository-url
+        repository: repository-url-http
 
-  - match: { repository-url.type : "url" }
+  - match: { repository-url-http.type : "url" }
-  - match: { repository-url.settings.url: '/http://(.+):\d+/' }
+  - match: { repository-url-http.settings.url: '/http://(.+):\d+/' }
 
   - do:
       snapshot.get:
-        repository: repository-url
+        repository: repository-url-http
         snapshot: snapshot-one,snapshot-two
 
   - is_true: snapshots
@@ -138,7 +138,7 @@ teardown:
   # Restore the second snapshot
   - do:
       snapshot.restore:
-        repository: repository-url
+        repository: repository-url-http
         snapshot: snapshot-two
         wait_for_completion: true
 
@@ -156,7 +156,7 @@ teardown:
   # Restore the first snapshot
   - do:
       snapshot.restore:
-        repository: repository-url
+        repository: repository-url-http
         snapshot: snapshot-one
         wait_for_completion: true
 
@@ -169,7 +169,7 @@ teardown:
   - do:
       catch: /repository is readonly/
       snapshot.delete:
-        repository: repository-url
+        repository: repository-url-http
         snapshot: snapshot-two
 
 ---
@@ -178,14 +178,14 @@ teardown:
   # Ensure that the URL repository is registered
   - do:
       snapshot.get_repository:
-        repository: repository-file
+        repository: repository-url-file
 
-  - match: { repository-file.type : "url" }
+  - match: { repository-url-file.type : "url" }
-  - match: { repository-file.settings.url: '/file://(.+)/' }
+  - match: { repository-url-file.settings.url: '/file://(.+)/' }
 
   - do:
       snapshot.get:
-        repository: repository-file
+        repository: repository-url-file
         snapshot: snapshot-one,snapshot-two
 
   - is_true: snapshots
@@ -200,7 +200,7 @@ teardown:
   # Restore the second snapshot
   - do:
       snapshot.restore:
-        repository: repository-file
+        repository: repository-url-file
         snapshot: snapshot-two
         wait_for_completion: true
 
@@ -218,7 +218,7 @@ teardown:
   # Restore the first snapshot
   - do:
       snapshot.restore:
-        repository: repository-file
+        repository: repository-url-file
         snapshot: snapshot-one
         wait_for_completion: true
 
@@ -231,7 +231,69 @@ teardown:
   - do:
       catch: /repository is readonly/
       snapshot.delete:
-        repository: repository-file
+        repository: repository-url-file
+        snapshot: snapshot-one
+
+---
+"Restore with repository-url using ftp://":
+
+  # Ensure that the ftp:// URL repository is registered
+  - do:
+      snapshot.get_repository:
+        repository: repository-url-ftp
+
+  - match: { repository-url-ftp.type : "url" }
+  - match: { repository-url-ftp.settings.url: '/ftp://(.+)/' }
+
+  - do:
+      snapshot.get:
+        repository: repository-url-ftp
+        snapshot: snapshot-one,snapshot-two
+
+  - is_true: snapshots
+  - match: { snapshots.0.state : SUCCESS }
+  - match: { snapshots.1.state : SUCCESS }
+
+  # Delete the index
+  - do:
+      indices.delete:
+        index: docs
+
+  # Restore the second snapshot
+  - do:
+      snapshot.restore:
+        repository: repository-url-ftp
+        snapshot: snapshot-two
+        wait_for_completion: true
+
+  - do:
+      count:
+        index: docs
+
+  - match: {count: 7}
+
+  # Delete the index again
+  - do:
+      indices.delete:
+        index: docs
+
+  # Restore the first snapshot
+  - do:
+      snapshot.restore:
+        repository: repository-url-ftp
+        snapshot: snapshot-one
+        wait_for_completion: true
+
+  - do:
+      count:
+        index: docs
+
+  - match: {count: 3}
+
+  - do:
+      catch: /repository is readonly/
+      snapshot.delete:
+        repository: repository-url-ftp
         snapshot: snapshot-one
 
 ---
@@ -240,7 +302,7 @@ teardown:
   - do:
       catch: /snapshot_missing_exception/
       snapshot.get:
-        repository: repository-url
+        repository: repository-url-http
         snapshot: missing
 
 ---
@@ -249,7 +311,7 @@ teardown:
   - do:
       catch: /snapshot_missing_exception/
       snapshot.delete:
-        repository: repository-url
+        repository: repository-url-http
         snapshot: missing
 
 ---
@@ -258,6 +320,6 @@ teardown:
   - do:
       catch: /snapshot_restore_exception/
       snapshot.restore:
-        repository: repository-url
+        repository: repository-url-http
         snapshot: missing
         wait_for_completion: true

test/fixtures/url-fixture/build.gradle

@@ -12,4 +12,7 @@ description = 'Fixture for URL external service'
 dependencies {
   api project(':server')
   api project(':test:framework')
+  api "org.apache.ftpserver:ftpserver-core:1.2.1"
+  api "org.apache.ftpserver:ftplet-api:1.2.1"
+  api "org.apache.mina:mina-core:${versions.apache_mina}"
 }

test/fixtures/url-fixture/src/main/java/fixture/url/URLFixture.java

@@ -8,7 +8,15 @@
  */
 package fixture.url;
 
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.listener.Listener;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+import org.elasticsearch.core.Nullable;
 import org.elasticsearch.rest.RestStatus;
+import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.fixture.AbstractHttpFixture;
 import org.elasticsearch.test.fixture.HttpHeaderParser;
 import org.junit.rules.TemporaryFolder;
@@ -17,7 +25,6 @@ import org.junit.rules.TestRule;
 import java.io.IOException;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
-import java.net.UnknownHostException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.HashMap;
@@ -97,8 +104,43 @@ public class URLFixture extends AbstractHttpFixture implements TestRule {
     protected void before() throws Throwable {
         this.temporaryFolder.create();
         this.repositoryDir = temporaryFolder.newFolder("repoDir").toPath();
-        InetSocketAddress inetSocketAddress = resolveAddress("0.0.0.0", 0);
+        InetSocketAddress inetSocketAddress = new InetSocketAddress(InetAddress.getLoopbackAddress(), 0);
         listen(inetSocketAddress, false);
+
+        startFtpServer();
+    }
+
+    @Nullable // if not started
+    private FtpServer ftpServer;
+
+    @Nullable // if not started
+    private String ftpUrl;
+
+    private void startFtpServer() throws FtpException {
+        final var listenerFactory = new ListenerFactory();
+        listenerFactory.setServerAddress(InetAddress.getLoopbackAddress().getHostAddress());
+        listenerFactory.setPort(0);
+        final var listener = listenerFactory.createListener();
+        final var listenersMap = new HashMap<String, Listener>();
+        listenersMap.put("default", listener);
+
+        final var user = new BaseUser();
+        user.setName(ESTestCase.randomIdentifier());
+        user.setPassword(ESTestCase.randomSecretKey());
+        user.setEnabled(true);
+        user.setHomeDirectory(getRepositoryDir());
+        user.setMaxIdleTime(0);
+
+        final var ftpServerFactory = new FtpServerFactory();
+        ftpServerFactory.setListeners(listenersMap);
+        ftpServerFactory.getUserManager().save(user);
+        ftpServer = ftpServerFactory.createServer();
+        ftpServer.start();
+        ftpUrl = "ftp://" + user.getName() + ":" + user.getPassword() + "@" + listener.getServerAddress() + ":" + listener.getPort();
+    }
+
+    public String getFtpUrl() {
+        return ftpUrl;
     }
 
     public String getRepositoryDir() {
@@ -108,16 +150,11 @@ public class URLFixture extends AbstractHttpFixture implements TestRule {
         return repositoryDir.toFile().getAbsolutePath();
     }
 
-    private static InetSocketAddress resolveAddress(String address, int port) {
-        try {
-            return new InetSocketAddress(InetAddress.getByName(address), port);
-        } catch (UnknownHostException e) {
-            throw new RuntimeException(e);
-        }
-    }
-
     @Override
     protected void after() {
+        if (ftpServer != null) {
+            ftpServer.stop();
+        }
         super.stop();
         this.temporaryFolder.delete();
     }

x-pack/plugin/security/build.gradle

@@ -128,7 +128,7 @@ dependencies {
   testImplementation('org.apache.directory.api:api-ldap-extras-codec-api:1.0.0')
   testImplementation('commons-pool:commons-pool:1.6')
   testImplementation('commons-collections:commons-collections:3.2.2')
-  testImplementation('org.apache.mina:mina-core:2.0.17')
+  testImplementation("org.apache.mina:mina-core:${versions.apache_mina}")
   testImplementation('org.apache.directory.api:api-util:1.0.1')
   testImplementation('org.apache.directory.api:api-i18n:1.0.1')
   testImplementation('org.apache.directory.api:api-ldap-model:1.0.1')