From 461764039591fb2caf1b7df6d044b3ded7a3dad1 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Tue, 22 Feb 2022 17:45:22 +0100 Subject: [PATCH] Remove extra repo flag to access archive indices (#84222) Removes the extra "allow_bwc_indices" flag on the repository to access older snapshots. --- .../plugins/RepositoryPlugin.java | 7 ++-- .../repositories/RepositoriesModule.java | 24 ++++++++++--- .../repositories/RepositoriesService.java | 10 +++--- .../snapshots/RestoreService.java | 34 +++++++------------ .../snapshots/RestoreServiceTests.java | 2 +- .../core/LocalStateCompositeXPackPlugin.java | 12 +++---- .../lucene/bwc/ArchiveLicenseIntegTests.java | 30 ++++++++++++---- .../xpack/lucene/bwc/OldLuceneVersions.java | 21 +++++++++--- .../oldrepos/DocValueOnlyFieldsIT.java | 2 +- .../oldrepos/OldRepositoryAccessIT.java | 1 - 10 files changed, 90 insertions(+), 53 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java b/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java index 476baf1c28f63..a61f0fc6721af 100644 --- a/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java @@ -8,17 +8,18 @@ package org.elasticsearch.plugins; -import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.Version; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.env.Environment; import org.elasticsearch.indices.recovery.RecoverySettings; import org.elasticsearch.repositories.Repository; +import org.elasticsearch.snapshots.Snapshot; import org.elasticsearch.xcontent.NamedXContentRegistry; import java.util.Collections; import java.util.Map; -import java.util.function.Consumer; +import java.util.function.BiConsumer; /** * An extension point for {@link Plugin} implementations to add custom snapshot repositories. @@ -66,7 +67,7 @@ default Map getInternalRepositories( * * returns null if no check is provided */ - default Consumer addPreRestoreCheck() { + default BiConsumer addPreRestoreVersionCheck() { return null; } diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java index 21de3f2f961c6..3436c929b02ed 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java @@ -8,7 +8,7 @@ package org.elasticsearch.repositories; -import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.Version; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.BigArrays; @@ -16,6 +16,8 @@ import org.elasticsearch.indices.recovery.RecoverySettings; import org.elasticsearch.plugins.RepositoryPlugin; import org.elasticsearch.repositories.fs.FsRepository; +import org.elasticsearch.snapshots.Snapshot; +import org.elasticsearch.snapshots.SnapshotRestoreException; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xcontent.NamedXContentRegistry; @@ -24,7 +26,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.function.Consumer; +import java.util.function.BiConsumer; /** * Sets up classes for Snapshot/Restore. @@ -83,13 +85,27 @@ public RepositoriesModule( } } - List> preRestoreChecks = new ArrayList<>(); + List> preRestoreChecks = new ArrayList<>(); for (RepositoryPlugin repoPlugin : repoPlugins) { - Consumer preRestoreCheck = repoPlugin.addPreRestoreCheck(); + BiConsumer preRestoreCheck = repoPlugin.addPreRestoreVersionCheck(); if (preRestoreCheck != null) { preRestoreChecks.add(preRestoreCheck); } } + if (preRestoreChecks.isEmpty()) { + preRestoreChecks.add((snapshot, version) -> { + if (version.before(Version.CURRENT.minimumIndexCompatibilityVersion())) { + throw new SnapshotRestoreException( + snapshot, + "the snapshot was created with Elasticsearch version [" + + version + + "] which is below the current versions minimum index compatibility version [" + + Version.CURRENT.minimumIndexCompatibilityVersion() + + "]" + ); + } + }); + } Settings settings = env.settings(); Map repositoryTypes = Collections.unmodifiableMap(factories); diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java index 6b837f20eb045..5680df7a9c0f7 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java @@ -11,6 +11,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRunnable; import org.elasticsearch.action.StepListener; @@ -44,6 +45,7 @@ import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.index.Index; import org.elasticsearch.repositories.blobstore.MeteredBlobStoreRepository; +import org.elasticsearch.snapshots.Snapshot; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; @@ -56,7 +58,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.function.Consumer; +import java.util.function.BiConsumer; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -98,7 +100,7 @@ public class RepositoriesService extends AbstractLifecycleComponent implements C private volatile Map repositories = Collections.emptyMap(); private final RepositoriesStatsArchive repositoriesStatsArchive; - private final List> preRestoreChecks; + private final List> preRestoreChecks; public RepositoriesService( Settings settings, @@ -107,7 +109,7 @@ public RepositoriesService( Map typesRegistry, Map internalTypesRegistry, ThreadPool threadPool, - List> preRestoreChecks + List> preRestoreChecks ) { this.typesRegistry = typesRegistry; this.internalTypesRegistry = internalTypesRegistry; @@ -781,7 +783,7 @@ private static RepositoryConflictException newRepositoryConflictException(String ); } - public List> getPreRestoreChecks() { + public List> getPreRestoreVersionChecks() { return preRestoreChecks; } diff --git a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java index de137cde1f331..5170622360bfa 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java @@ -298,7 +298,7 @@ private void startRestore( final Snapshot snapshot = new Snapshot(repositoryName, snapshotId); // Make sure that we can restore from this snapshot - validateSnapshotRestorable(request, repository.getMetadata(), snapshotInfo); + validateSnapshotRestorable(request, repository.getMetadata(), snapshotInfo, repositoriesService.getPreRestoreVersionChecks()); // Get the global state if necessary Metadata globalMetadata = null; @@ -957,10 +957,16 @@ private static String renameIndex(String index, RestoreSnapshotRequest request, /** * Checks that snapshots can be restored and have compatible version - * @param repository repository name + * @param repository repository name * @param snapshotInfo snapshot metadata + * @param preRestoreVersionChecks */ - static void validateSnapshotRestorable(RestoreSnapshotRequest request, RepositoryMetadata repository, SnapshotInfo snapshotInfo) { + static void validateSnapshotRestorable( + RestoreSnapshotRequest request, + RepositoryMetadata repository, + SnapshotInfo snapshotInfo, + List> preRestoreVersionChecks + ) { if (snapshotInfo.state().restorable() == false) { throw new SnapshotRestoreException( new Snapshot(repository.name(), snapshotInfo.snapshotId()), @@ -977,17 +983,8 @@ static void validateSnapshotRestorable(RestoreSnapshotRequest request, Repositor + "]" ); } - if (ALLOW_BWC_INDICES_SETTING.get(repository.settings()) == false - && snapshotInfo.version().before(Version.CURRENT.minimumIndexCompatibilityVersion())) { - throw new SnapshotRestoreException( - new Snapshot(repository.name(), snapshotInfo.snapshotId()), - "the snapshot was created with Elasticsearch version [" - + snapshotInfo.version() - + "] which is below the current versions minimum index compatibility version [" - + Version.CURRENT.minimumIndexCompatibilityVersion() - + "]" - ); - } + Snapshot snapshot = new Snapshot(repository.name(), snapshotInfo.snapshotId()); + preRestoreVersionChecks.forEach(c -> c.accept(snapshot, snapshotInfo.version())); if (request.includeGlobalState() && snapshotInfo.includeGlobalState() == Boolean.FALSE) { throw new SnapshotRestoreException( new Snapshot(repository.name(), snapshotInfo.snapshotId()), @@ -996,12 +993,6 @@ static void validateSnapshotRestorable(RestoreSnapshotRequest request, Repositor } } - public static final Setting ALLOW_BWC_INDICES_SETTING = Setting.boolSetting( - "allow_bwc_indices", - false, - Setting.Property.NodeScope - ); - public static boolean failed(SnapshotInfo snapshot, String index) { for (SnapshotShardFailure failure : snapshot.shardFailures()) { if (index.equals(failure.index())) { @@ -1277,7 +1268,8 @@ public ClusterState execute(ClusterState currentState) { for (Map.Entry indexEntry : indicesToRestore.entrySet()) { final IndexId index = indexEntry.getValue(); final IndexMetadata originalIndexMetadata = metadata.index(index.getName()); - repositoriesService.getPreRestoreChecks().forEach(check -> check.accept(originalIndexMetadata)); + repositoriesService.getPreRestoreVersionChecks() + .forEach(check -> check.accept(snapshot, originalIndexMetadata.getCreationVersion())); IndexMetadata snapshotIndexMetadata = updateIndexSettings( snapshot, originalIndexMetadata, diff --git a/server/src/test/java/org/elasticsearch/snapshots/RestoreServiceTests.java b/server/src/test/java/org/elasticsearch/snapshots/RestoreServiceTests.java index fb9ce48ad683b..58cff40cef378 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/RestoreServiceTests.java +++ b/server/src/test/java/org/elasticsearch/snapshots/RestoreServiceTests.java @@ -194,7 +194,7 @@ public void testNotAllowToRestoreGlobalStateFromSnapshotWithoutOne() { var exception = expectThrows( SnapshotRestoreException.class, - () -> RestoreService.validateSnapshotRestorable(request, repository, snapshotInfo) + () -> RestoreService.validateSnapshotRestorable(request, repository, snapshotInfo, List.of()) ); assertThat( exception.getMessage(), 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 35dccbb3ef9ed..7e37ebfc4d22e 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 @@ -6,6 +6,7 @@ */ package org.elasticsearch.xpack.core; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionResponse; @@ -20,7 +21,6 @@ import org.elasticsearch.client.internal.Client; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.coordination.ElectionStrategy; -import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.IndexTemplateMetadata; import org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata; @@ -84,6 +84,7 @@ import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptService; import org.elasticsearch.search.internal.ShardSearchRequest; +import org.elasticsearch.snapshots.Snapshot; import org.elasticsearch.threadpool.ExecutorBuilder; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.Transport; @@ -105,7 +106,6 @@ import java.util.Optional; import java.util.Set; import java.util.function.BiConsumer; -import java.util.function.Consumer; import java.util.function.Function; import java.util.function.LongSupplier; import java.util.function.Predicate; @@ -571,12 +571,12 @@ public Map getInternalRepositories( } @Override - public Consumer addPreRestoreCheck() { - List> checks = filterPlugins(RepositoryPlugin.class).stream() - .map(RepositoryPlugin::addPreRestoreCheck) + public BiConsumer addPreRestoreVersionCheck() { + List> checks = filterPlugins(RepositoryPlugin.class).stream() + .map(RepositoryPlugin::addPreRestoreVersionCheck) .filter(Objects::nonNull) .collect(Collectors.toList()); - return checks.isEmpty() ? null : imd -> checks.forEach(c -> c.accept(imd)); + return checks.isEmpty() ? null : (s, v) -> checks.forEach(c -> c.accept(s, v)); } @Override diff --git a/x-pack/plugin/old-lucene-versions/src/internalClusterTest/java/org/elasticsearch/xpack/lucene/bwc/ArchiveLicenseIntegTests.java b/x-pack/plugin/old-lucene-versions/src/internalClusterTest/java/org/elasticsearch/xpack/lucene/bwc/ArchiveLicenseIntegTests.java index 4d2c8113c02ba..fa09b63102626 100644 --- a/x-pack/plugin/old-lucene-versions/src/internalClusterTest/java/org/elasticsearch/xpack/lucene/bwc/ArchiveLicenseIntegTests.java +++ b/x-pack/plugin/old-lucene-versions/src/internalClusterTest/java/org/elasticsearch/xpack/lucene/bwc/ArchiveLicenseIntegTests.java @@ -38,8 +38,8 @@ import org.elasticsearch.repositories.RepositoryData; import org.elasticsearch.repositories.fs.FsRepository; import org.elasticsearch.snapshots.AbstractSnapshotIntegTestCase; -import org.elasticsearch.snapshots.RestoreService; import org.elasticsearch.snapshots.SnapshotId; +import org.elasticsearch.snapshots.SnapshotRestoreException; import org.elasticsearch.snapshots.mockstore.MockRepository; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.xcontent.NamedXContentRegistry; @@ -107,7 +107,8 @@ public IndexMetadata getSnapshotIndexMetaData(RepositoryData repositoryData, Sna .put(original.getSettings()) .put( IndexMetadata.SETTING_INDEX_VERSION_CREATED.getKey(), - randomBoolean() ? Version.fromString("5.0.0") : Version.fromString("6.0.0") + metadata.settings() + .getAsVersion("version", randomBoolean() ? Version.fromString("5.0.0") : Version.fromString("6.0.0")) ) ) .build(); @@ -121,11 +122,7 @@ public IndexMetadata getSnapshotIndexMetaData(RepositoryData repositoryData, Sna @Before public void createAndRestoreArchive() throws Exception { - createRepository( - repoName, - TestRepositoryPlugin.FAKE_VERSIONS_TYPE, - randomRepositorySettings().put(RestoreService.ALLOW_BWC_INDICES_SETTING.getKey(), true) - ); + createRepository(repoName, TestRepositoryPlugin.FAKE_VERSIONS_TYPE); createIndex(indexName); createFullSnapshot(repoName, snapshotName); @@ -168,6 +165,25 @@ public void testFailRestoreOnInvalidLicense() throws Exception { assertThat(e.getMessage(), containsString("current license is non-compliant for [archive]")); } + public void testFailRestoreOnTooOldVersion() { + createRepository( + repoName, + TestRepositoryPlugin.FAKE_VERSIONS_TYPE, + Settings.builder().put(getRepositoryOnMaster(repoName).getMetadata().settings()).put("version", Version.fromString("2.0.0").id) + ); + final RestoreSnapshotRequest req = new RestoreSnapshotRequest(repoName, snapshotName).indices(indexName).waitForCompletion(true); + SnapshotRestoreException e = expectThrows( + SnapshotRestoreException.class, + () -> client().admin().cluster().restoreSnapshot(req).actionGet() + ); + assertThat( + e.getMessage(), + containsString( + "the snapshot was created with Elasticsearch version [2.0.0] " + "which isn't supported by the archive functionality" + ) + ); + } + // checks that shards are failed if license becomes invalid after successful restore public void testShardAllocationOnInvalidLicense() throws Exception { final RestoreSnapshotRequest req = new RestoreSnapshotRequest(repoName, snapshotName).indices(indexName).waitForCompletion(true); diff --git a/x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/OldLuceneVersions.java b/x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/OldLuceneVersions.java index 69ac9777960de..20f5f27dc694e 100644 --- a/x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/OldLuceneVersions.java +++ b/x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/OldLuceneVersions.java @@ -15,7 +15,6 @@ import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.client.internal.Client; -import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.routing.allocation.decider.AllocationDecider; @@ -44,6 +43,8 @@ import org.elasticsearch.plugins.RepositoryPlugin; import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.script.ScriptService; +import org.elasticsearch.snapshots.Snapshot; +import org.elasticsearch.snapshots.SnapshotRestoreException; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.watcher.ResourceWatcherService; import org.elasticsearch.xcontent.NamedXContentRegistry; @@ -58,7 +59,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.function.Consumer; +import java.util.function.BiConsumer; import java.util.function.Supplier; public class OldLuceneVersions extends Plugin implements IndexStorePlugin, ClusterPlugin, RepositoryPlugin, ActionPlugin { @@ -69,6 +70,8 @@ public class OldLuceneVersions extends Plugin implements IndexStorePlugin, Clust License.OperationMode.ENTERPRISE ); + private static Version MINIMUM_ARCHIVE_VERSION = Version.fromString("5.0.0"); + public static boolean isArchiveIndex(Version version) { return version.before(Version.CURRENT.minimumIndexCompatibilityVersion()); } @@ -133,12 +136,20 @@ public void afterFilesRestoredFromRepository(IndexShard indexShard) { } @Override - public Consumer addPreRestoreCheck() { - return indexMetadata -> { - if (isArchiveIndex(indexMetadata.getCreationVersion())) { + public BiConsumer addPreRestoreVersionCheck() { + return (snapshot, version) -> { + if (isArchiveIndex(version)) { if (ARCHIVE_FEATURE.checkWithoutTracking(getLicenseState()) == false) { throw LicenseUtils.newComplianceException("archive"); } + if (version.before(MINIMUM_ARCHIVE_VERSION)) { + throw new SnapshotRestoreException( + snapshot, + "the snapshot was created with Elasticsearch version [" + + version + + "] which isn't supported by the archive functionality" + ); + } } }; } diff --git a/x-pack/qa/repository-old-versions/src/test/java/org/elasticsearch/oldrepos/DocValueOnlyFieldsIT.java b/x-pack/qa/repository-old-versions/src/test/java/org/elasticsearch/oldrepos/DocValueOnlyFieldsIT.java index ab1105d989ff1..80bee84736941 100644 --- a/x-pack/qa/repository-old-versions/src/test/java/org/elasticsearch/oldrepos/DocValueOnlyFieldsIT.java +++ b/x-pack/qa/repository-old-versions/src/test/java/org/elasticsearch/oldrepos/DocValueOnlyFieldsIT.java @@ -190,7 +190,7 @@ public void setupIndex() throws IOException { // register repo on new ES and restore snapshot Request createRepoRequest2 = new Request("PUT", "/_snapshot/" + repoName); createRepoRequest2.setJsonEntity(""" - {"type":"fs","settings":{"location":"%s","allow_bwc_indices":true}} + {"type":"fs","settings":{"location":"%s"}} """.formatted(repoLocation)); assertOK(client().performRequest(createRepoRequest2)); diff --git a/x-pack/qa/repository-old-versions/src/test/java/org/elasticsearch/oldrepos/OldRepositoryAccessIT.java b/x-pack/qa/repository-old-versions/src/test/java/org/elasticsearch/oldrepos/OldRepositoryAccessIT.java index 0f77bfb8ee964..8bb8cb6b08fe4 100644 --- a/x-pack/qa/repository-old-versions/src/test/java/org/elasticsearch/oldrepos/OldRepositoryAccessIT.java +++ b/x-pack/qa/repository-old-versions/src/test/java/org/elasticsearch/oldrepos/OldRepositoryAccessIT.java @@ -204,7 +204,6 @@ private void beforeRestart( if (sourceOnlyRepository) { repoSettingsBuilder.put("delegate_type", "fs"); } - repoSettingsBuilder.put("allow_bwc_indices", true); ElasticsearchAssertions.assertAcked( client.snapshot() .createRepository(