From 42b2cf26a43a8a79e98a7721b1dc29fab9472901 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 25 Aug 2020 11:35:45 +0200 Subject: [PATCH 1/3] Prevent snapshots to be mounted as system indices --- ...ransportMountSearchableSnapshotAction.java | 8 +- ...hableSnapshotsSystemIndicesIntegTests.java | 127 ++++++++++++++++++ 2 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsSystemIndicesIntegTests.java diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java index ced0c03cf642a..483c852325c30 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java @@ -24,6 +24,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexNotFoundException; +import org.elasticsearch.indices.InvalidIndexNameException; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.repositories.IndexId; import org.elasticsearch.repositories.RepositoriesService; @@ -132,6 +133,11 @@ protected void masterOperation( ) { SearchableSnapshots.ensureValidLicense(licenseState); + final String mountedIndexName = request.mountedIndexName(); + if (mountedIndexName.charAt(0) == '.') { + throw new InvalidIndexNameException(mountedIndexName, "system indices cannot be mounted as searchable snapshots"); + } + final String repoName = request.repositoryName(); final String snapName = request.snapshotName(); final String indexName = request.snapshotIndexName(); @@ -168,7 +174,7 @@ protected void masterOperation( .indices(indexName) // Always rename it to the desired mounted index name .renamePattern(".+") - .renameReplacement(request.mountedIndexName()) + .renameReplacement(mountedIndexName) // Pass through index settings, adding the index-level settings required to use searchable snapshots .indexSettings( Settings.builder() diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsSystemIndicesIntegTests.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsSystemIndicesIntegTests.java new file mode 100644 index 0000000000000..de62d09a313d2 --- /dev/null +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsSystemIndicesIntegTests.java @@ -0,0 +1,127 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.searchablesnapshots; + +import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; +import org.elasticsearch.action.bulk.BulkRequest; +import org.elasticsearch.action.bulk.BulkResponse; +import org.elasticsearch.action.index.IndexRequest; +import org.elasticsearch.client.Client; +import org.elasticsearch.client.OriginSettingClient; +import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.indices.InvalidIndexNameException; +import org.elasticsearch.indices.SystemIndexDescriptor; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.plugins.SystemIndexPlugin; +import org.elasticsearch.xpack.core.ClientHelper; +import org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotAction; +import org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotRequest; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Locale; + +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; + +public class SearchableSnapshotsSystemIndicesIntegTests extends BaseSearchableSnapshotsIntegTestCase { + + @Override + protected Collection> nodePlugins() { + final List> plugins = new ArrayList<>(super.nodePlugins()); + plugins.add(TestSystemIndexPlugin.class); + return plugins; + } + + public void testCannotMountSystemIndex() { + final String systemIndexName = '.' + randomAlphaOfLength(10).toLowerCase(Locale.ROOT); + executeTest(systemIndexName, client()); + } + + public void testCannotMountSystemIndexWithDescriptor() { + // TODO replace STACK_ORIGIN with searchable snapshot origin + executeTest(TestSystemIndexPlugin.INDEX_NAME, new OriginSettingClient(client(), ClientHelper.STACK_ORIGIN)); + } + + private void executeTest(final String indexName, final Client client) { + final boolean isHidden = randomBoolean(); + assertAcked( + client.admin() + .indices() + .prepareCreate(indexName) + .setSettings(Settings.builder().put(IndexMetadata.SETTING_INDEX_HIDDEN, isHidden).build()) + ); + + final int nbDocs = scaledRandomIntBetween(0, 100); + if (nbDocs > 0) { + final BulkRequest bulkRequest = new BulkRequest(); + for (int i = 0; i < nbDocs; i++) { + IndexRequest indexRequest = new IndexRequest(indexName); + indexRequest.source("value", i); + bulkRequest.add(indexRequest); + } + final BulkResponse bulkResponse = client.bulk(bulkRequest).actionGet(); + assertThat(bulkResponse.hasFailures(), is(false)); + } + flushAndRefresh(indexName); + assertHitCount(client.prepareSearch(indexName).get(), nbDocs); + + final String repositoryName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); + createRepo(repositoryName); + + final String snapshotName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); + final CreateSnapshotResponse snapshotResponse = client.admin() + .cluster() + .prepareCreateSnapshot(repositoryName, snapshotName) + .setIndices(indexName) + .setWaitForCompletion(true) + .get(); + + final int numPrimaries = getNumShards(indexName).numPrimaries; + assertThat(snapshotResponse.getSnapshotInfo().successfulShards(), equalTo(numPrimaries)); + assertThat(snapshotResponse.getSnapshotInfo().failedShards(), equalTo(0)); + + if (randomBoolean()) { + assertAcked(client.admin().indices().prepareClose(indexName)); + } else { + assertAcked(client.admin().indices().prepareDelete(indexName)); + } + + final MountSearchableSnapshotRequest mountRequest = new MountSearchableSnapshotRequest( + indexName, + repositoryName, + snapshotName, + indexName, + Settings.builder().put(IndexMetadata.SETTING_INDEX_HIDDEN, randomBoolean()).build(), + Strings.EMPTY_ARRAY, + true + ); + + final InvalidIndexNameException exception = expectThrows( + InvalidIndexNameException.class, + () -> client.execute(MountSearchableSnapshotAction.INSTANCE, mountRequest).actionGet() + ); + assertThat(exception.getIndex().getName(), equalTo(indexName)); + assertThat(exception.getMessage(), containsString("system indices cannot be mounted as searchable snapshots")); + } + + public static class TestSystemIndexPlugin extends Plugin implements SystemIndexPlugin { + + static final String INDEX_NAME = ".test-system-index"; + + @Override + public Collection getSystemIndexDescriptors(Settings settings) { + return List.of(new SystemIndexDescriptor(INDEX_NAME, "System index for [" + getTestClass().getName() + ']')); + } + } +} From ae5843374684039e1bd580b53fe674d48f1e22bb Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Thu, 27 Aug 2020 12:23:35 +0200 Subject: [PATCH 2/3] Use systemIndices --- .../main/java/org/elasticsearch/node/Node.java | 1 + .../core/LocalStateCompositeXPackPlugin.java | 4 +++- ...TransportMountSearchableSnapshotAction.java | 11 +++++++---- .../LocalStateSearchableSnapshots.java | 18 +++++++++++++----- ...chableSnapshotsSystemIndicesIntegTests.java | 17 +++++++---------- 5 files changed, 31 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 569ba3ceb6c5b..ea7533a9ab1e7 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -650,6 +650,7 @@ protected Node(final Environment initialEnvironment, b.bind(RerouteService.class).toInstance(rerouteService); b.bind(ShardLimitValidator.class).toInstance(shardLimitValidator); b.bind(FsHealthService.class).toInstance(fsHealthService); + b.bind(SystemIndices.class).toInstance(systemIndices); } ); injector = modules.createInjector(); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java index 8da93eec756de..3c478d845ac68 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java @@ -63,6 +63,7 @@ import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.RepositoryPlugin; import org.elasticsearch.plugins.ScriptPlugin; +import org.elasticsearch.plugins.SystemIndexPlugin; import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.repositories.Repository; import org.elasticsearch.rest.RestController; @@ -100,7 +101,8 @@ import static java.util.stream.Collectors.toList; public class LocalStateCompositeXPackPlugin extends XPackPlugin implements ScriptPlugin, ActionPlugin, IngestPlugin, NetworkPlugin, - ClusterPlugin, DiscoveryPlugin, MapperPlugin, AnalysisPlugin, PersistentTaskPlugin, EnginePlugin, IndexStorePlugin { + ClusterPlugin, DiscoveryPlugin, MapperPlugin, AnalysisPlugin, PersistentTaskPlugin, EnginePlugin, IndexStorePlugin, + SystemIndexPlugin { private XPackLicenseState licenseState; private SSLService sslService; diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java index 483c852325c30..c2c06a1281682 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java @@ -24,7 +24,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexNotFoundException; -import org.elasticsearch.indices.InvalidIndexNameException; +import org.elasticsearch.indices.SystemIndices; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.repositories.IndexId; import org.elasticsearch.repositories.RepositoriesService; @@ -64,6 +64,7 @@ public class TransportMountSearchableSnapshotAction extends TransportMasterNodeA private final Client client; private final RepositoriesService repositoriesService; private final XPackLicenseState licenseState; + private final SystemIndices systemIndices; @Inject public TransportMountSearchableSnapshotAction( @@ -74,7 +75,8 @@ public TransportMountSearchableSnapshotAction( RepositoriesService repositoriesService, ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver, - XPackLicenseState licenseState + XPackLicenseState licenseState, + SystemIndices systemIndices ) { super( MountSearchableSnapshotAction.NAME, @@ -88,6 +90,7 @@ public TransportMountSearchableSnapshotAction( this.client = client; this.repositoriesService = repositoriesService; this.licenseState = Objects.requireNonNull(licenseState); + this.systemIndices = Objects.requireNonNull(systemIndices); } @Override @@ -134,8 +137,8 @@ protected void masterOperation( SearchableSnapshots.ensureValidLicense(licenseState); final String mountedIndexName = request.mountedIndexName(); - if (mountedIndexName.charAt(0) == '.') { - throw new InvalidIndexNameException(mountedIndexName, "system indices cannot be mounted as searchable snapshots"); + if (systemIndices.isSystemIndex(mountedIndexName)) { + throw new ElasticsearchException("system index [{}] cannot be mounted as searchable snapshots", mountedIndexName); } final String repoName = request.repositoryName(); diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/LocalStateSearchableSnapshots.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/LocalStateSearchableSnapshots.java index 88615d02ebb1a..8140663b3f856 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/LocalStateSearchableSnapshots.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/LocalStateSearchableSnapshots.java @@ -7,24 +7,32 @@ package org.elasticsearch.xpack.searchablesnapshots; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.indices.SystemIndexDescriptor; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.xpack.core.LocalStateCompositeXPackPlugin; import java.nio.file.Path; +import java.util.Collection; public class LocalStateSearchableSnapshots extends LocalStateCompositeXPackPlugin { + private final SearchableSnapshots plugin; + public LocalStateSearchableSnapshots(final Settings settings, final Path configPath) { super(settings, configPath); - LocalStateSearchableSnapshots thisVar = this; - - plugins.add(new SearchableSnapshots(settings) { + this.plugin = new SearchableSnapshots(settings) { @Override protected XPackLicenseState getLicenseState() { - return thisVar.getLicenseState(); + return LocalStateSearchableSnapshots.this.getLicenseState(); } - }); + }; + plugins.add(plugin); + } + + @Override + public Collection getSystemIndexDescriptors(Settings settings) { + return plugin.getSystemIndexDescriptors(settings); } } diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsSystemIndicesIntegTests.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsSystemIndicesIntegTests.java index de62d09a313d2..1eab6231f309e 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsSystemIndicesIntegTests.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsSystemIndicesIntegTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.searchablesnapshots; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; import org.elasticsearch.action.bulk.BulkRequest; import org.elasticsearch.action.bulk.BulkResponse; @@ -15,7 +16,6 @@ import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.indices.InvalidIndexNameException; import org.elasticsearch.indices.SystemIndexDescriptor; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.SystemIndexPlugin; @@ -44,13 +44,11 @@ protected Collection> nodePlugins() { } public void testCannotMountSystemIndex() { - final String systemIndexName = '.' + randomAlphaOfLength(10).toLowerCase(Locale.ROOT); - executeTest(systemIndexName, client()); + executeTest(TestSystemIndexPlugin.INDEX_NAME, new OriginSettingClient(client(), ClientHelper.SEARCHABLE_SNAPSHOTS_ORIGIN)); } - public void testCannotMountSystemIndexWithDescriptor() { - // TODO replace STACK_ORIGIN with searchable snapshot origin - executeTest(TestSystemIndexPlugin.INDEX_NAME, new OriginSettingClient(client(), ClientHelper.STACK_ORIGIN)); + public void testCannotMountSnapshotBlobCacheIndex() { + executeTest(SearchableSnapshotsConstants.SNAPSHOT_BLOB_CACHE_INDEX, client()); } private void executeTest(final String indexName, final Client client) { @@ -107,12 +105,11 @@ private void executeTest(final String indexName, final Client client) { true ); - final InvalidIndexNameException exception = expectThrows( - InvalidIndexNameException.class, + final ElasticsearchException exception = expectThrows( + ElasticsearchException.class, () -> client.execute(MountSearchableSnapshotAction.INSTANCE, mountRequest).actionGet() ); - assertThat(exception.getIndex().getName(), equalTo(indexName)); - assertThat(exception.getMessage(), containsString("system indices cannot be mounted as searchable snapshots")); + assertThat(exception.getMessage(), containsString("system index [" + indexName + "] cannot be mounted as searchable snapshots")); } public static class TestSystemIndexPlugin extends Plugin implements SystemIndexPlugin { From 3e173cbfc15feef3ae4b3904eb49e684e555d474 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Mon, 31 Aug 2020 14:55:20 +0200 Subject: [PATCH 3/3] use createAndPopulateIndex() --- ...hableSnapshotsSystemIndicesIntegTests.java | 32 +++---------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsSystemIndicesIntegTests.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsSystemIndicesIntegTests.java index 1eab6231f309e..8015e4ee1b892 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsSystemIndicesIntegTests.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsSystemIndicesIntegTests.java @@ -8,9 +8,6 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; -import org.elasticsearch.action.bulk.BulkRequest; -import org.elasticsearch.action.bulk.BulkResponse; -import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.client.Client; import org.elasticsearch.client.OriginSettingClient; import org.elasticsearch.cluster.metadata.IndexMetadata; @@ -29,10 +26,8 @@ import java.util.Locale; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; public class SearchableSnapshotsSystemIndicesIntegTests extends BaseSearchableSnapshotsIntegTestCase { @@ -43,36 +38,17 @@ protected Collection> nodePlugins() { return plugins; } - public void testCannotMountSystemIndex() { + public void testCannotMountSystemIndex() throws Exception { executeTest(TestSystemIndexPlugin.INDEX_NAME, new OriginSettingClient(client(), ClientHelper.SEARCHABLE_SNAPSHOTS_ORIGIN)); } - public void testCannotMountSnapshotBlobCacheIndex() { + public void testCannotMountSnapshotBlobCacheIndex() throws Exception { executeTest(SearchableSnapshotsConstants.SNAPSHOT_BLOB_CACHE_INDEX, client()); } - private void executeTest(final String indexName, final Client client) { + private void executeTest(final String indexName, final Client client) throws Exception { final boolean isHidden = randomBoolean(); - assertAcked( - client.admin() - .indices() - .prepareCreate(indexName) - .setSettings(Settings.builder().put(IndexMetadata.SETTING_INDEX_HIDDEN, isHidden).build()) - ); - - final int nbDocs = scaledRandomIntBetween(0, 100); - if (nbDocs > 0) { - final BulkRequest bulkRequest = new BulkRequest(); - for (int i = 0; i < nbDocs; i++) { - IndexRequest indexRequest = new IndexRequest(indexName); - indexRequest.source("value", i); - bulkRequest.add(indexRequest); - } - final BulkResponse bulkResponse = client.bulk(bulkRequest).actionGet(); - assertThat(bulkResponse.hasFailures(), is(false)); - } - flushAndRefresh(indexName); - assertHitCount(client.prepareSearch(indexName).get(), nbDocs); + createAndPopulateIndex(indexName, Settings.builder().put(IndexMetadata.SETTING_INDEX_HIDDEN, isHidden)); final String repositoryName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); createRepo(repositoryName);