diff --git a/CHANGES.txt b/CHANGES.txt index b132527d5c..4a7492079f 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,6 @@ 3.11.16 Merged from 3.0: + * Validate the existence of a datacenter in nodetool rebuild (CASSANDRA-14319) * Suppress CVE-2023-2251 (CASSANDRA-18497) 3.11.15 diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index 701dd26da2..ba5ebc0dd0 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -1358,16 +1358,35 @@ 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."); + } - // check the arguments - if (keyspace == null && tokens != null) + 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))); + } + } + + // check the arguments + if (keyspace == null && tokens != null) + { + throw new IllegalArgumentException("Cannot specify tokens without keyspace."); + } + } + catch (Throwable ex) { - throw new IllegalArgumentException("Cannot specify tokens without keyspace."); + isRebuilding.set(false); + throw ex; } logger.info("rebuild from dc: {}, {}, {}", sourceDc == null ? "(any dc)" : sourceDc, diff --git a/test/unit/org/apache/cassandra/service/StorageServiceServerTest.java b/test/unit/org/apache/cassandra/service/StorageServiceServerTest.java index d5ab214340..33bdcebf70 100644 --- a/test/unit/org/apache/cassandra/service/StorageServiceServerTest.java +++ b/test/unit/org/apache/cassandra/service/StorageServiceServerTest.java @@ -33,6 +33,7 @@ import java.util.Random; import java.util.UUID; import com.google.common.collect.HashMultimap; +import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.Multimap; import org.junit.BeforeClass; @@ -60,9 +61,11 @@ import org.apache.cassandra.schema.KeyspaceParams; import org.apache.cassandra.schema.ReplicationParams; import org.apache.cassandra.schema.SchemaKeyspaceTables; import org.apache.cassandra.utils.FBUtilities; +import org.mockito.Mockito; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.junit.Assume.assumeTrue; @RunWith(OrderedJUnit4ClassRunner.class) @@ -599,4 +602,57 @@ public class StorageServiceServerTest repairRangeFrom = StorageService.instance.createRepairRangeFrom("2000", "2000"); assert repairRangeFrom.size() == 0; } + + @Test + public void testRebuildFailOnNonExistingDatacenter() throws Exception + { + String nonExistentDC = "NON_EXISTENT_DC"; + + try + { + getStorageService().rebuild(nonExistentDC, "StorageServiceServerTest", null, null); + fail(); + } + catch (IllegalArgumentException ex) + { + assertEquals(String.format("Provided datacenter '%s' is not a valid datacenter, available datacenters are: %s", + nonExistentDC, + "datacenter1"), + 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() throws Exception + { + ImmutableMultimap.Builder builder = ImmutableMultimap.builder(); + builder.put("datacenter1", InetAddress.getByName("127.0.0.1")); + + TokenMetadata.Topology tokenMetadataTopology = Mockito.mock(TokenMetadata.Topology.class); + Mockito.when(tokenMetadataTopology.getDatacenterEndpoints()).thenReturn(builder.build()); + + TokenMetadata metadata = new TokenMetadata(); + 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; + } }