From 5ce2c008762ce231e50d819fbb2b14db59a31126 Mon Sep 17 00:00:00 2001 From: David Turner Date: Tue, 22 Jul 2025 08:51:51 +0100 Subject: [PATCH 1/2] 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. --- .../RepositoryVerifyIntegrityIT.java | 18 ++++++++++ ...nsportRepositoryVerifyIntegrityAction.java | 14 ++++++++ ...tRepositoryVerifyIntegrityActionTests.java | 35 +++++++++++++++++++ 3 files changed, 67 insertions(+) create mode 100644 x-pack/plugin/snapshot-repo-test-kit/src/test/java/org/elasticsearch/repositories/blobstore/testkit/integrity/TransportRepositoryVerifyIntegrityActionTests.java diff --git a/x-pack/plugin/snapshot-repo-test-kit/src/internalClusterTest/java/org/elasticsearch/repositories/blobstore/testkit/integrity/RepositoryVerifyIntegrityIT.java b/x-pack/plugin/snapshot-repo-test-kit/src/internalClusterTest/java/org/elasticsearch/repositories/blobstore/testkit/integrity/RepositoryVerifyIntegrityIT.java index e8c23d0741712..b0150555d1e40 100644 --- a/x-pack/plugin/snapshot-repo-test-kit/src/internalClusterTest/java/org/elasticsearch/repositories/blobstore/testkit/integrity/RepositoryVerifyIntegrityIT.java +++ b/x-pack/plugin/snapshot-repo-test-kit/src/internalClusterTest/java/org/elasticsearch/repositories/blobstore/testkit/integrity/RepositoryVerifyIntegrityIT.java @@ -14,6 +14,7 @@ import org.elasticsearch.action.support.SubscribableListener; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; +import org.elasticsearch.client.ResponseException; import org.elasticsearch.cluster.metadata.ProjectId; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; @@ -37,6 +38,7 @@ import org.elasticsearch.repositories.blobstore.RepositoryFileType; import org.elasticsearch.repositories.blobstore.testkit.SnapshotRepositoryTestKit; import org.elasticsearch.repositories.fs.FsRepository; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.snapshots.AbstractSnapshotIntegTestCase; import org.elasticsearch.snapshots.SnapshotId; import org.elasticsearch.snapshots.SnapshotInfo; @@ -75,6 +77,7 @@ import static org.elasticsearch.repositories.blobstore.BlobStoreRepository.SNAPSHOT_FORMAT; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; @@ -719,6 +722,21 @@ public void testBlobInSnapshotNotShardGeneration() throws IOException { }, "blob in snapshot but not shard generation"); } + public void testFreshRepository() { + final var repositoryName = randomIdentifier(); + final var repositoryRootPath = randomRepoPath(); + + createRepository(repositoryName, FsRepository.TYPE, repositoryRootPath); + try { + final var request = new Request("POST", "/_snapshot/" + repositoryName + "/_verify_integrity"); + final var responseException = expectThrows(ResponseException.class, () -> getRestClient().performRequest(request)); + assertEquals(RestStatus.BAD_REQUEST.getStatus(), responseException.getResponse().getStatusLine().getStatusCode()); + assertThat(responseException.getMessage(), containsString("repository is empty, cannot verify its integrity")); + } finally { + deleteRepository(repositoryName); + } + } + private void runInconsistentShardGenerationBlobTest( TestContext testContext, UnaryOperator shardGenerationUpdater, diff --git a/x-pack/plugin/snapshot-repo-test-kit/src/main/java/org/elasticsearch/repositories/blobstore/testkit/integrity/TransportRepositoryVerifyIntegrityAction.java b/x-pack/plugin/snapshot-repo-test-kit/src/main/java/org/elasticsearch/repositories/blobstore/testkit/integrity/TransportRepositoryVerifyIntegrityAction.java index 57f77cb8441d7..b2cfd7e4f91bb 100644 --- a/x-pack/plugin/snapshot-repo-test-kit/src/main/java/org/elasticsearch/repositories/blobstore/testkit/integrity/TransportRepositoryVerifyIntegrityAction.java +++ b/x-pack/plugin/snapshot-repo-test-kit/src/main/java/org/elasticsearch/repositories/blobstore/testkit/integrity/TransportRepositoryVerifyIntegrityAction.java @@ -132,6 +132,7 @@ public void writeResponseChunk(RepositoryVerifyIntegrityResponseChunk responseCh .newForked(l -> repository.getRepositoryData(executor, l)) .andThenApply(repositoryData -> { + ensureValidGenId(repositoryData.getGenId()); final var cancellableThreads = new CancellableThreads(); task.addListener(() -> cancellableThreads.cancel("task cancelled")); final var verifier = new RepositoryIntegrityVerifier( @@ -155,4 +156,17 @@ public void writeResponseChunk(RepositoryVerifyIntegrityResponseChunk responseCh .andThen((l, repositoryIntegrityVerifier) -> repositoryIntegrityVerifier.start(l)) .addListener(listener); } + + static void ensureValidGenId(long repositoryGenId) { + if (repositoryGenId == RepositoryData.EMPTY_REPO_GEN) { + throw new IllegalArgumentException("repository is empty, cannot verify its integrity"); + } + if (repositoryGenId < 0) { + final var exception = new IllegalStateException( + "repository is in an unexpected state [" + repositoryGenId + "], cannot verify its integrity" + ); + assert false : exception; // cannot be unknown, and if corrupt we throw a corruptedStateException from getRepositoryData + throw exception; + } + } } diff --git a/x-pack/plugin/snapshot-repo-test-kit/src/test/java/org/elasticsearch/repositories/blobstore/testkit/integrity/TransportRepositoryVerifyIntegrityActionTests.java b/x-pack/plugin/snapshot-repo-test-kit/src/test/java/org/elasticsearch/repositories/blobstore/testkit/integrity/TransportRepositoryVerifyIntegrityActionTests.java new file mode 100644 index 0000000000000..9eeee8ba6e24b --- /dev/null +++ b/x-pack/plugin/snapshot-repo-test-kit/src/test/java/org/elasticsearch/repositories/blobstore/testkit/integrity/TransportRepositoryVerifyIntegrityActionTests.java @@ -0,0 +1,35 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.repositories.blobstore.testkit.integrity; + +import org.elasticsearch.repositories.RepositoryData; +import org.elasticsearch.test.ESTestCase; + +import static org.hamcrest.Matchers.equalTo; + +public class TransportRepositoryVerifyIntegrityActionTests extends ESTestCase { + public void testEnsureValidGenId() { + TransportRepositoryVerifyIntegrityAction.ensureValidGenId(0); + TransportRepositoryVerifyIntegrityAction.ensureValidGenId(randomNonNegativeLong()); + assertThat( + expectThrows( + IllegalArgumentException.class, + () -> TransportRepositoryVerifyIntegrityAction.ensureValidGenId(RepositoryData.EMPTY_REPO_GEN) + ).getMessage(), + equalTo("repository is empty, cannot verify its integrity") + ); + assertThat(expectThrows(IllegalStateException.class, () -> { + try { + TransportRepositoryVerifyIntegrityAction.ensureValidGenId(RepositoryData.CORRUPTED_REPO_GEN); + } catch (AssertionError e) { + // if assertions disabled, we throw the cause directly + throw e.getCause(); + } + }).getMessage(), equalTo("repository is in an unexpected state [-3], cannot verify its integrity")); + } +} From 5d4adadf62829313786dc25772e85e29571b914d Mon Sep 17 00:00:00 2001 From: David Turner Date: Tue, 22 Jul 2025 08:56:21 +0100 Subject: [PATCH 2/2] Update docs/changelog/131677.yaml --- docs/changelog/131677.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/131677.yaml diff --git a/docs/changelog/131677.yaml b/docs/changelog/131677.yaml new file mode 100644 index 0000000000000..5c707a44ebe0f --- /dev/null +++ b/docs/changelog/131677.yaml @@ -0,0 +1,5 @@ +pr: 131677 +summary: Throw better exception if verifying empty repo +area: Snapshot/Restore +type: bug +issues: []