diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java index c92760f98f275..f58bfa1c9b436 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java @@ -55,6 +55,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -140,7 +141,7 @@ public void registerRepository(final PutRepositoryRequest request, final ActionL // Trying to create the new repository on master to make sure it works try { - closeRepository(createRepository(newRepositoryMetadata, typesRegistry)); + closeRepository(createRepository(newRepositoryMetadata, typesRegistry, this::throwRepositoryTypeDoesNotExists)); } catch (Exception e) { listener.onFailure(e); return; @@ -490,7 +491,7 @@ public void applyClusterState(ClusterChangedEvent event) { archiveRepositoryStats(repository, state.version()); repository = null; try { - repository = createRepository(repositoryMetadata, typesRegistry); + repository = createRepository(repositoryMetadata, typesRegistry, this::createUnknownTypeRepository); } catch (RepositoryException ex) { // TODO: this catch is bogus, it means the old repo is already closed, // but we have nothing to replace it @@ -499,7 +500,7 @@ public void applyClusterState(ClusterChangedEvent event) { } } else { try { - repository = createRepository(repositoryMetadata, typesRegistry); + repository = createRepository(repositoryMetadata, typesRegistry, this::createUnknownTypeRepository); } catch (RepositoryException ex) { logger.warn(() -> new ParameterizedMessage("failed to create repository [{}]", repositoryMetadata.name()), ex); } @@ -592,7 +593,7 @@ public void registerInternalRepository(String name, String type) { RepositoryMetadata metadata = new RepositoryMetadata(name, type, Settings.EMPTY); Repository repository = internalRepositories.computeIfAbsent(name, (n) -> { logger.debug("put internal repository [{}][{}]", name, type); - return createRepository(metadata, internalTypesRegistry); + return createRepository(metadata, internalTypesRegistry, this::throwRepositoryTypeDoesNotExists); }); if (type.equals(repository.getMetadata().type()) == false) { logger.warn( @@ -647,11 +648,15 @@ private void archiveRepositoryStats(Repository repository, long clusterStateVers /** * Creates repository holder. This method starts the repository */ - private Repository createRepository(RepositoryMetadata repositoryMetadata, Map factories) { + private Repository createRepository( + RepositoryMetadata repositoryMetadata, + Map factories, + Function defaultFactory + ) { logger.debug("creating repository [{}][{}]", repositoryMetadata.type(), repositoryMetadata.name()); Repository.Factory factory = factories.get(repositoryMetadata.type()); if (factory == null) { - throw new RepositoryException(repositoryMetadata.name(), "repository type [" + repositoryMetadata.type() + "] does not exist"); + return defaultFactory.apply(repositoryMetadata); } Repository repository = null; try { @@ -668,6 +673,19 @@ private Repository createRepository(RepositoryMetadata repositoryMetadata, Map listener) { + listener.onFailure(createUnknownTypeException()); + } + + @Override + public void finalizeSnapshot(FinalizeSnapshotContext finalizeSnapshotContext) { + finalizeSnapshotContext.onFailure(createUnknownTypeException()); + } + + @Override + public void deleteSnapshots( + Collection snapshotIds, + long repositoryStateId, + Version repositoryMetaVersion, + ActionListener listener + ) { + listener.onFailure(createUnknownTypeException()); + } + + @Override + public long getSnapshotThrottleTimeInNanos() { + throw createUnknownTypeException(); + } + + @Override + public long getRestoreThrottleTimeInNanos() { + throw createUnknownTypeException(); + } + + @Override + public String startVerification() { + throw createUnknownTypeException(); + } + + @Override + public void endVerification(String verificationToken) { + throw createUnknownTypeException(); + } + + @Override + public void verify(String verificationToken, DiscoveryNode localNode) { + throw createUnknownTypeException(); + } + + @Override + public boolean isReadOnly() { + // this repository is assumed writable to bypass read-only check and fail with exception produced by this class + return false; + } + + @Override + public void snapshotShard(SnapshotShardContext snapshotShardContext) { + snapshotShardContext.onFailure(createUnknownTypeException()); + } + + @Override + public void restoreShard( + Store store, + SnapshotId snapshotId, + IndexId indexId, + ShardId snapshotShardId, + RecoveryState recoveryState, + ActionListener listener + ) { + listener.onFailure(createUnknownTypeException()); + } + + @Override + public IndexShardSnapshotStatus getShardSnapshotStatus(SnapshotId snapshotId, IndexId indexId, ShardId shardId) { + throw createUnknownTypeException(); + } + + @Override + public void updateState(ClusterState state) { + + } + + @Override + public void executeConsistentStateUpdate( + Function createUpdateTask, + String source, + Consumer onFailure + ) { + onFailure.accept(createUnknownTypeException()); + } + + @Override + public void cloneShardSnapshot( + SnapshotId source, + SnapshotId target, + RepositoryShardId shardId, + ShardGeneration shardGeneration, + ActionListener listener + ) { + listener.onFailure(createUnknownTypeException()); + } + + @Override + public void awaitIdle() { + + } + + @Override + protected void doStart() { + + } + + @Override + protected void doStop() { + + } + + @Override + protected void doClose() throws IOException { + + } +} diff --git a/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java b/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java index 308e1e495c18e..9525055b13b2d 100644 --- a/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java @@ -11,6 +11,7 @@ import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryRequest; +import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.cluster.ClusterChangedEvent; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; @@ -50,7 +51,10 @@ import java.util.function.Consumer; import java.util.function.Function; +import static org.elasticsearch.test.hamcrest.ThrowableAssertions.assertThatException; +import static org.elasticsearch.test.hamcrest.ThrowableAssertions.assertThatThrows; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.isA; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -162,6 +166,48 @@ public void testRepositoriesStatsCanHaveTheSameNameAndDifferentTypeOverTime() { assertThat(repositoryStatsTypeB.getRepositoryStats(), equalTo(MeteredRepositoryTypeB.STATS)); } + // this can happen when the repository plugin is removed, but repository is still exist + public void testHandlesUnknownRepositoryTypeWhenApplyingClusterState() { + var repoName = randomAlphaOfLengthBetween(10, 25); + + var clusterState = createClusterStateWithRepo(repoName, "unknown"); + repositoriesService.applyClusterState(new ClusterChangedEvent("starting", clusterState, emptyState())); + + var repo = repositoriesService.repository(repoName); + assertThat(repo, isA(UnknownTypeRepository.class)); + } + + public void testRemoveUnknownRepositoryTypeWhenApplyingClusterState() { + var repoName = randomAlphaOfLengthBetween(10, 25); + + var clusterState = createClusterStateWithRepo(repoName, "unknown"); + repositoriesService.applyClusterState(new ClusterChangedEvent("starting", clusterState, emptyState())); + repositoriesService.applyClusterState(new ClusterChangedEvent("removing repo", emptyState(), clusterState)); + + assertThatThrows( + () -> repositoriesService.repository(repoName), + RepositoryMissingException.class, + equalTo("[" + repoName + "] missing") + ); + } + + public void testRegisterRepositoryFailsForUnknownType() { + var repoName = randomAlphaOfLengthBetween(10, 25); + var request = new PutRepositoryRequest().name(repoName).type("unknown"); + + repositoriesService.registerRepository(request, new ActionListener<>() { + @Override + public void onResponse(AcknowledgedResponse acknowledgedResponse) { + fail("Should not register unknown repository type"); + } + + @Override + public void onFailure(Exception e) { + assertThatException(e, RepositoryException.class, equalTo("[" + repoName + "] repository type [unknown] does not exist")); + } + }); + } + private ClusterState createClusterStateWithRepo(String repoName, String repoType) { ClusterState.Builder state = ClusterState.builder(new ClusterName("test")); Metadata.Builder mdBuilder = Metadata.builder(); @@ -179,8 +225,7 @@ private ClusterState emptyState() { } private void assertThrowsOnRegister(String repoName) { - PutRepositoryRequest request = new PutRepositoryRequest(repoName); - expectThrows(RepositoryException.class, () -> repositoriesService.registerRepository(request, null)); + expectThrows(RepositoryException.class, () -> repositoriesService.registerRepository(new PutRepositoryRequest(repoName), null)); } private static class TestRepository implements Repository { diff --git a/server/src/test/java/org/elasticsearch/repositories/UnknownTypeRepositoryTests.java b/server/src/test/java/org/elasticsearch/repositories/UnknownTypeRepositoryTests.java new file mode 100644 index 0000000000000..6263be3a93235 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/repositories/UnknownTypeRepositoryTests.java @@ -0,0 +1,32 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.repositories; + +import org.elasticsearch.cluster.metadata.RepositoryMetadata; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.snapshots.SnapshotId; +import org.elasticsearch.test.ESTestCase; + +public class UnknownTypeRepositoryTests extends ESTestCase { + + private UnknownTypeRepository repository = new UnknownTypeRepository(new RepositoryMetadata("name", "type", Settings.EMPTY)); + + public void testShouldThrowWhenGettingMetadata() { + expectThrows(RepositoryException.class, () -> repository.getSnapshotGlobalMetadata(new SnapshotId("name", "uuid"))); + } + + public void testShouldNotThrowWhenApplyingLifecycleChanges() { + repository.start(); + repository.stop(); + } + + public void testShouldNotThrowWhenClosingToAllowRemovingRepo() { + repository.close(); + } +} diff --git a/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ThrowableAssertions.java b/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ThrowableAssertions.java new file mode 100644 index 0000000000000..4bad4d0754623 --- /dev/null +++ b/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ThrowableAssertions.java @@ -0,0 +1,37 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ +package org.elasticsearch.test.hamcrest; + +import org.apache.lucene.util.LuceneTestCase; +import org.hamcrest.Matcher; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.isA; + +/** + * Assertions for exceptions and their messages + */ +public class ThrowableAssertions { + + public static void assertThatThrows( + LuceneTestCase.ThrowingRunnable code, + Class exceptionType, + Matcher messageMatcher + ) { + try { + code.run(); + } catch (Throwable e) { + assertThatException(e, exceptionType, messageMatcher); + } + } + + public static void assertThatException(Throwable exception, Class exceptionType, Matcher messageMatcher) { + assertThat(exception, isA(exceptionType)); + assertThat(exception.getMessage(), messageMatcher); + } +}