diff --git a/CHANGES.txt b/CHANGES.txt index 77b725fa3c..baf11ee6c0 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -17,6 +17,7 @@ Merged from 4.0: Merged from 3.11: * Fix the capital P usage in the CQL parser (CASSANDRA-17919) Merged from 3.0: + * Validate the existence of a datacenter in nodetool rebuild (CASSANDRA-14319) * Suppress CVE-2023-2251 (CASSANDRA-18497) * Do not remove SSTables when cause of FSReadError is OutOfMemoryError while using best_effort disk failure policy (CASSANDRA-18336) * Do not remove truncated_at entry in system.local while dropping an index (CASSANDRA-18105) diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index 92c31c1f7a..87cfbac046 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -1363,10 +1363,29 @@ public class StorageService extends NotificationBroadcasterSupport implements IE public void rebuild(String sourceDc, String keyspace, String tokens, String specificSources) { - // check ongoing rebuild - if (!isRebuilding.compareAndSet(false, true)) + try { - throw new IllegalStateException("Node is still rebuilding. Check nodetool netstats."); + // check ongoing rebuild + if (!isRebuilding.compareAndSet(false, true)) + { + throw new IllegalStateException("Node is still rebuilding. Check nodetool netstats."); + } + + if (sourceDc != null) + { + TokenMetadata.Topology topology = getTokenMetadata().cloneOnlyTokenMap().getTopology(); + Set availableDCs = topology.getDatacenterEndpoints().keySet(); + if (!availableDCs.contains(sourceDc)) + { + throw new IllegalArgumentException(String.format("Provided datacenter '%s' is not a valid datacenter, available datacenters are: %s", + sourceDc, String.join(",", availableDCs))); + } + } + } + catch (Throwable ex) + { + isRebuilding.set(false); + throw ex; } try diff --git a/test/unit/org/apache/cassandra/service/StorageServiceTest.java b/test/unit/org/apache/cassandra/service/StorageServiceTest.java index 694c876732..e4589d66e0 100644 --- a/test/unit/org/apache/cassandra/service/StorageServiceTest.java +++ b/test/unit/org/apache/cassandra/service/StorageServiceTest.java @@ -21,6 +21,7 @@ package org.apache.cassandra.service; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.atomic.AtomicInteger; +import com.google.common.collect.ImmutableMultimap; import org.junit.Assert; import org.junit.Before; import org.junit.BeforeClass; @@ -40,12 +41,15 @@ import org.apache.cassandra.locator.InetAddressAndPort; import org.apache.cassandra.locator.Replica; import org.apache.cassandra.locator.ReplicaCollection; import org.apache.cassandra.locator.ReplicaMultimap; +import org.apache.cassandra.locator.SimpleSnitch; import org.apache.cassandra.locator.SimpleStrategy; import org.apache.cassandra.locator.TokenMetadata; +import org.mockito.Mockito; import static java.util.concurrent.TimeUnit.MINUTES; import static java.util.concurrent.TimeUnit.SECONDS; import static org.awaitility.Awaitility.await; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -198,4 +202,57 @@ public class StorageServiceTest // fast tasks are shut down as part of the Runtime shutdown hook. assertFalse(ScheduledExecutors.scheduledFastTasks.isTerminated()); } + + @Test + public void testRebuildFailOnNonExistingDatacenter() + { + String nonExistentDC = "NON_EXISTENT_DC"; + + try + { + getStorageService().rebuild(nonExistentDC, "StorageServiceTest", null, null); + fail(); + } + catch (IllegalArgumentException ex) + { + Assert.assertEquals(String.format("Provided datacenter '%s' is not a valid datacenter, available datacenters are: %s", + nonExistentDC, + SimpleSnitch.DATA_CENTER_NAME), + ex.getMessage()); + } + } + + @Test + public void testRebuildingWithTokensWithoutKeyspace() throws Exception + { + try + { + getStorageService().rebuild("datacenter1", null, "123", null); + fail(); + } + catch (IllegalArgumentException ex) + { + assertEquals("Cannot specify tokens without keyspace.", ex.getMessage()); + } + } + + private StorageService getStorageService() + { + ImmutableMultimap.Builder builder = ImmutableMultimap.builder(); + builder.put(SimpleSnitch.DATA_CENTER_NAME, aAddress); + + TokenMetadata.Topology tokenMetadataTopology = Mockito.mock(TokenMetadata.Topology.class); + Mockito.when(tokenMetadataTopology.getDatacenterEndpoints()).thenReturn(builder.build()); + + TokenMetadata metadata = new TokenMetadata(new SimpleSnitch()); + TokenMetadata spiedMetadata = Mockito.spy(metadata); + + Mockito.when(spiedMetadata.getTopology()).thenReturn(tokenMetadataTopology); + + StorageService spiedStorageService = Mockito.spy(StorageService.instance); + Mockito.when(spiedStorageService.getTokenMetadata()).thenReturn(spiedMetadata); + Mockito.when(spiedMetadata.cloneOnlyTokenMap()).thenReturn(spiedMetadata); + + return spiedStorageService; + } }