Skip to content

Commit fe3c4ce

Browse files
committed
Throw better exception if verifying empty repo
Today if you attempt to verify the integrity of a brand-new repository (no `index-${N}` blob) then it will fail because the repository generation is `-1` which cannot be sent over the wire. But it makes no sense to verify the integrity of such a repository anyway, so with this commit we fail such requests up-front with a more helpful error message. Backport of #131677 to `9.0`
1 parent 1bb21cb commit fe3c4ce

File tree

4 files changed

+72
-0
lines changed

4 files changed

+72
-0
lines changed

docs/changelog/131677.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 131677
2+
summary: Throw better exception if verifying empty repo
3+
area: Snapshot/Restore
4+
type: bug
5+
issues: []

x-pack/plugin/snapshot-repo-test-kit/src/internalClusterTest/java/org/elasticsearch/repositories/blobstore/testkit/integrity/RepositoryVerifyIntegrityIT.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.elasticsearch.action.support.SubscribableListener;
1515
import org.elasticsearch.client.Request;
1616
import org.elasticsearch.client.Response;
17+
import org.elasticsearch.client.ResponseException;
1718
import org.elasticsearch.common.bytes.BytesReference;
1819
import org.elasticsearch.common.settings.Settings;
1920
import org.elasticsearch.common.unit.ByteSizeValue;
@@ -35,6 +36,7 @@
3536
import org.elasticsearch.repositories.blobstore.RepositoryFileType;
3637
import org.elasticsearch.repositories.blobstore.testkit.SnapshotRepositoryTestKit;
3738
import org.elasticsearch.repositories.fs.FsRepository;
39+
import org.elasticsearch.rest.RestStatus;
3840
import org.elasticsearch.snapshots.AbstractSnapshotIntegTestCase;
3941
import org.elasticsearch.snapshots.SnapshotId;
4042
import org.elasticsearch.snapshots.SnapshotInfo;
@@ -73,6 +75,7 @@
7375
import static org.elasticsearch.repositories.blobstore.BlobStoreRepository.SNAPSHOT_FORMAT;
7476
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
7577
import static org.hamcrest.Matchers.allOf;
78+
import static org.hamcrest.Matchers.containsString;
7679
import static org.hamcrest.Matchers.empty;
7780
import static org.hamcrest.Matchers.equalTo;
7881
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
@@ -713,6 +716,21 @@ public void testBlobInSnapshotNotShardGeneration() throws IOException {
713716
}, "blob in snapshot but not shard generation");
714717
}
715718

719+
public void testFreshRepository() {
720+
final var repositoryName = randomIdentifier();
721+
final var repositoryRootPath = randomRepoPath();
722+
723+
createRepository(repositoryName, FsRepository.TYPE, repositoryRootPath);
724+
try {
725+
final var request = new Request("POST", "/_snapshot/" + repositoryName + "/_verify_integrity");
726+
final var responseException = expectThrows(ResponseException.class, () -> getRestClient().performRequest(request));
727+
assertEquals(RestStatus.BAD_REQUEST.getStatus(), responseException.getResponse().getStatusLine().getStatusCode());
728+
assertThat(responseException.getMessage(), containsString("repository is empty, cannot verify its integrity"));
729+
} finally {
730+
deleteRepository(repositoryName);
731+
}
732+
}
733+
716734
private void runInconsistentShardGenerationBlobTest(
717735
TestContext testContext,
718736
UnaryOperator<BlobStoreIndexShardSnapshots> shardGenerationUpdater,

x-pack/plugin/snapshot-repo-test-kit/src/main/java/org/elasticsearch/repositories/blobstore/testkit/integrity/TransportRepositoryVerifyIntegrityAction.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ public void writeResponseChunk(RepositoryVerifyIntegrityResponseChunk responseCh
132132

133133
.<RepositoryData>newForked(l -> repository.getRepositoryData(executor, l))
134134
.andThenApply(repositoryData -> {
135+
ensureValidGenId(repositoryData.getGenId());
135136
final var cancellableThreads = new CancellableThreads();
136137
task.addListener(() -> cancellableThreads.cancel("task cancelled"));
137138
final var verifier = new RepositoryIntegrityVerifier(
@@ -155,4 +156,17 @@ public void writeResponseChunk(RepositoryVerifyIntegrityResponseChunk responseCh
155156
.<RepositoryVerifyIntegrityResponse>andThen((l, repositoryIntegrityVerifier) -> repositoryIntegrityVerifier.start(l))
156157
.addListener(listener);
157158
}
159+
160+
static void ensureValidGenId(long repositoryGenId) {
161+
if (repositoryGenId == RepositoryData.EMPTY_REPO_GEN) {
162+
throw new IllegalArgumentException("repository is empty, cannot verify its integrity");
163+
}
164+
if (repositoryGenId < 0) {
165+
final var exception = new IllegalStateException(
166+
"repository is in an unexpected state [" + repositoryGenId + "], cannot verify its integrity"
167+
);
168+
assert false : exception; // cannot be unknown, and if corrupt we throw a corruptedStateException from getRepositoryData
169+
throw exception;
170+
}
171+
}
158172
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.repositories.blobstore.testkit.integrity;
9+
10+
import org.elasticsearch.repositories.RepositoryData;
11+
import org.elasticsearch.test.ESTestCase;
12+
13+
import static org.hamcrest.Matchers.equalTo;
14+
15+
public class TransportRepositoryVerifyIntegrityActionTests extends ESTestCase {
16+
public void testEnsureValidGenId() {
17+
TransportRepositoryVerifyIntegrityAction.ensureValidGenId(0);
18+
TransportRepositoryVerifyIntegrityAction.ensureValidGenId(randomNonNegativeLong());
19+
assertThat(
20+
expectThrows(
21+
IllegalArgumentException.class,
22+
() -> TransportRepositoryVerifyIntegrityAction.ensureValidGenId(RepositoryData.EMPTY_REPO_GEN)
23+
).getMessage(),
24+
equalTo("repository is empty, cannot verify its integrity")
25+
);
26+
assertThat(expectThrows(IllegalStateException.class, () -> {
27+
try {
28+
TransportRepositoryVerifyIntegrityAction.ensureValidGenId(RepositoryData.CORRUPTED_REPO_GEN);
29+
} catch (AssertionError e) {
30+
// if assertions disabled, we throw the cause directly
31+
throw e.getCause();
32+
}
33+
}).getMessage(), equalTo("repository is in an unexpected state [-3], cannot verify its integrity"));
34+
}
35+
}

0 commit comments

Comments
 (0)