honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Validate snapshot UUID during restore (#59601) 

Today when mounting a searchable snapshot we obtain the snapshot/index
UUIDs and then assume that these are the UUIDs used during the
subsequent restore. If you concurrently delete the snapshot and replace
it with one with the same name then this assumption is violated, with
chaotic consequences.

This commit introduces a check that ensures that the snapshot UUID does
not change during the mount process. If the snapshot remains in place
then the index UUID necessarily does not change either.

Relates #50999
This commit is contained in:
David Turner 2020-07-15 16:23:20 +01:00 committed by GitHub
parent 6b75525efb
commit 691759fb1f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 201 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java
View File

@ -25,6 +25,7 @@ import org.elasticsearch.Version;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.master.MasterNodeRequest;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
@ -68,6 +69,9 @@ public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotReq
private Settings indexSettings = EMPTY_SETTINGS;
private String[] ignoreIndexSettings = Strings.EMPTY_ARRAY;
@Nullable // if any snapshot UUID will do
private String snapshotUuid;
public RestoreSnapshotRequest() {
}
@ -99,6 +103,9 @@ public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotReq
}
indexSettings = readSettingsFromStream(in);
ignoreIndexSettings = in.readStringArray();
if (in.getVersion().onOrAfter(Version.V_7_10_0)) {
snapshotUuid = in.readOptionalString();
}
}
@Override
@ -119,6 +126,12 @@ public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotReq
}
writeSettingsToStream(indexSettings, out);
out.writeStringArray(ignoreIndexSettings);
if (out.getVersion().onOrAfter(Version.V_7_10_0)) {
out.writeOptionalString(snapshotUuid);
} else if (snapshotUuid != null) {
throw new IllegalStateException(
"restricting the snapshot UUID is forbidden in a cluster with version [" + out.getVersion() + "] nodes");
}
}
@Override
@ -439,6 +452,28 @@ public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotReq
return this.indexSettings;
}
/**
* Sometimes a client has identified precisely which snapshot is to be restored via a separate mechanism and wishes to guarantee that
* this is the snapshot that this request restores. If the client can only identify a snapshot by its name then there is a risk that the
* desired snapshot may be deleted and replaced by a new snapshot with the same name which is inconsistent with the original one. This
* method lets us fail the restore if the precise snapshot we want is not available.
*
* This is for internal use only and is not exposed in the REST layer.
*/
public RestoreSnapshotRequest snapshotUuid(String snapshotUuid) {
this.snapshotUuid = snapshotUuid;
return this;
}
/**
* @return the UUID that identifies the specific snapshot in the repository to be restored, or {@code null} if the snapshot name is
* a sufficient identifier.
*/
@Nullable
public String snapshotUuid() {
return snapshotUuid;
}
/**
* Parses restore definition
*
@ -561,13 +596,14 @@ public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotReq
Objects.equals(renamePattern, that.renamePattern) &&
Objects.equals(renameReplacement, that.renameReplacement) &&
Objects.equals(indexSettings, that.indexSettings) &&
Arrays.equals(ignoreIndexSettings, that.ignoreIndexSettings);
Arrays.equals(ignoreIndexSettings, that.ignoreIndexSettings) &&
Objects.equals(snapshotUuid, that.snapshotUuid);
}
@Override
public int hashCode() {
int result = Objects.hash(snapshot, repository, indicesOptions, renamePattern, renameReplacement, waitForCompletion,
includeGlobalState, partial, includeAliases, indexSettings);
includeGlobalState, partial, includeAliases, indexSettings, snapshotUuid);
result = 31 * result + Arrays.hashCode(indices);
result = 31 * result + Arrays.hashCode(ignoreIndexSettings);
return result;

5
server/src/main/java/org/elasticsearch/snapshots/RestoreService.java
View File

@ -199,6 +199,11 @@ public class RestoreService implements ClusterStateApplier {
}
final SnapshotId snapshotId = matchingSnapshotId.get();
if (request.snapshotUuid() != null && request.snapshotUuid().equals(snapshotId.getUUID()) == false) {
throw new SnapshotRestoreException(repositoryName, snapshotName,
"snapshot UUID mismatch: expected [" + request.snapshotUuid() + "] but got [" + snapshotId.getUUID() + "]");
}
final SnapshotInfo snapshotInfo = repository.getSnapshotInfo(snapshotId);
final Snapshot snapshot = new Snapshot(repositoryName, snapshotId);

5
server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java
View File

@ -89,6 +89,11 @@ public class RestoreSnapshotRequestTests extends AbstractWireSerializingTestCase
if (randomBoolean()) {
instance.masterNodeTimeout(randomTimeValue());
}
if (randomBoolean()) {
instance.snapshotUuid(randomBoolean() ? null : randomAlphaOfLength(10));
}
return instance;
}

9
x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java
View File

@ -153,11 +153,6 @@ public class TransportMountSearchableSnapshotAction extends TransportMasterNodeA
}
final SnapshotId snapshotId = matchingSnapshotId.get();
// TODO validate IDs in the restore:
// We must fail the restore if it obtains different IDs from the ones we just obtained (e.g. the target snapshot was replaced
// by one with the same name while we are restoring it) or else the index metadata might bear no relation to the snapshot we're
// searching.
final String[] ignoreIndexSettings = Arrays.copyOf(request.ignoreIndexSettings(), request.ignoreIndexSettings().length + 1);
ignoreIndexSettings[ignoreIndexSettings.length - 1] = IndexMetadata.SETTING_DATA_PATH;
@ -188,7 +183,9 @@ public class TransportMountSearchableSnapshotAction extends TransportMasterNodeA
// Pass through the wait-for-completion flag
.waitForCompletion(request.waitForCompletion())
// Pass through the master-node timeout
.masterNodeTimeout(request.masterNodeTimeout()),
.masterNodeTimeout(request.masterNodeTimeout())
// Fail the restore if the snapshot found above is swapped out from under us before the restore happens
.snapshotUuid(snapshotId.getUUID()),
listener
);
}, listener::onFailure);

150
x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsUuidValidationIntegTests.java Normal file
View File

@ -0,0 +1,150 @@
/*
* 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.ActionFuture;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse;
import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotAction;
import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse;
import org.elasticsearch.action.support.ActionFilter;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.plugins.ActionPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.snapshots.SnapshotInfo;
import org.elasticsearch.snapshots.SnapshotRestoreException;
import org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotAction;
import org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotRequest;
import java.util.Collection;
import java.util.List;
import java.util.Locale;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import static java.util.Collections.singletonList;
import static org.elasticsearch.index.IndexSettings.INDEX_SOFT_DELETES_SETTING;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
public class SearchableSnapshotsUuidValidationIntegTests extends BaseSearchableSnapshotsIntegTestCase {
public static class TestPlugin extends Plugin implements ActionPlugin {
private final RestoreBlockingActionFilter restoreBlockingActionFilter;
public TestPlugin() {
restoreBlockingActionFilter = new RestoreBlockingActionFilter();
}
@Override
public List<ActionFilter> getActionFilters() {
return singletonList(restoreBlockingActionFilter);
}
}
public static class RestoreBlockingActionFilter extends org.elasticsearch.action.support.ActionFilter.Simple {
private final PlainActionFuture<Void> executed = new PlainActionFuture<>();
private final PlainActionFuture<Void> unblocked = new PlainActionFuture<>();
@Override
protected boolean apply(String action, ActionRequest request, ActionListener<?> listener) {
if (RestoreSnapshotAction.NAME.equals(action)) {
executed.onResponse(null);
unblocked.actionGet();
}
return true;
}
@Override
public int order() {
return 0;
}
public void unblock() {
unblocked.onResponse(null);
}
public void awaitExecution() {
executed.actionGet();
}
}
@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return Stream.concat(super.nodePlugins().stream(), Stream.of(TestPlugin.class)).collect(Collectors.toList());
}
public void testMountFailsIfSnapshotChanged() throws Exception {
final String fsRepoName = randomAlphaOfLength(10);
final String indexName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT);
final String restoredIndexName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT);
final String snapshotName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT);
createRepo(fsRepoName);
final Settings.Builder originalIndexSettings = Settings.builder().put(INDEX_SOFT_DELETES_SETTING.getKey(), true);
createAndPopulateIndex(indexName, originalIndexSettings);
createSnapshot(fsRepoName, snapshotName);
final MountSearchableSnapshotRequest req = new MountSearchableSnapshotRequest(
restoredIndexName,
fsRepoName,
snapshotName,
indexName,
Settings.EMPTY,
Strings.EMPTY_ARRAY,
true
);
final ActionFuture<RestoreSnapshotResponse> responseFuture = client().execute(MountSearchableSnapshotAction.INSTANCE, req);
final RestoreBlockingActionFilter restoreBlockingActionFilter = getBlockingActionFilter();
restoreBlockingActionFilter.awaitExecution();
assertAcked(client().admin().cluster().prepareDeleteSnapshot(fsRepoName, snapshotName).get());
createSnapshot(fsRepoName, snapshotName);
assertFalse(responseFuture.isDone());
restoreBlockingActionFilter.unblock();
assertThat(
expectThrows(SnapshotRestoreException.class, responseFuture::actionGet).getMessage(),
containsString("snapshot UUID mismatch")
);
assertAcked(client().admin().indices().prepareDelete(indexName));
}
private static void createSnapshot(String fsRepoName, String snapshotName) {
final CreateSnapshotResponse createSnapshotResponse = client().admin()
.cluster()
.prepareCreateSnapshot(fsRepoName, snapshotName)
.setWaitForCompletion(true)
.get();
final SnapshotInfo snapshotInfo = createSnapshotResponse.getSnapshotInfo();
assertThat(snapshotInfo.successfulShards(), greaterThan(0));
assertThat(snapshotInfo.successfulShards(), equalTo(snapshotInfo.totalShards()));
}
private static RestoreBlockingActionFilter getBlockingActionFilter() {
for (final ActionFilter filter : internalCluster().getCurrentMasterNodeInstance(ActionFilters.class).filters()) {
if (filter instanceof RestoreBlockingActionFilter) {
return (RestoreBlockingActionFilter) filter;
}
}
throw new AssertionError("did not find BlockingActionFilter");
}
}