This is an automated email from the ASF dual-hosted git repository.
jmckenzie pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push:
new 14fbab15bd Add guardrail to allow disabling SimpleStrategy
14fbab15bd is described below
commit 14fbab15bd264dd1cf894bf48170cf4f30ada8a0
Author: Josh McKenzie <[email protected]>
AuthorDate: Thu May 19 15:48:20 2022 -0400
Add guardrail to allow disabling SimpleStrategy
Patch by Josh McKenzie; reviewed by Aleksey Yeschenko for CASSANDRA-17647
---
CHANGES.txt | 1 +
NEWS.txt | 1 +
conf/cassandra.yaml | 3 +
src/java/org/apache/cassandra/config/Config.java | 1 +
.../apache/cassandra/config/GuardrailsOptions.java | 14 ++++
.../statements/schema/AlterKeyspaceStatement.java | 5 ++
.../statements/schema/CreateKeyspaceStatement.java | 4 +
.../cql3/statements/schema/KeyspaceAttributes.java | 2 +-
.../apache/cassandra/db/guardrails/Guardrails.java | 20 +++++
.../cassandra/db/guardrails/GuardrailsConfig.java | 7 ++
.../cassandra/db/guardrails/GuardrailsMBean.java | 14 ++++
.../db/guardrails/GuardrailSimpleStrategyTest.java | 89 ++++++++++++++++++++++
12 files changed, 160 insertions(+), 1 deletion(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index c38906747d..6d9feb52ff 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
4.2
+ * Add guardrail to allow disabling of SimpleStrategy (CASSANDRA-17647)
* Change default directory permission to 750 in packaging (CASSANDRA-17470)
* Adding support for TLS client authentication for internode communication
(CASSANDRA-17513)
* Add new CQL function maxWritetime (CASSANDRA-17425)
diff --git a/NEWS.txt b/NEWS.txt
index e44b0e4fe8..2b81a284ad 100644
--- a/NEWS.txt
+++ b/NEWS.txt
@@ -62,6 +62,7 @@ New features
non-frozen collections and UDT, and returns the largest timestamp. One
should not to use it when upgrading to 4.2.
- New Guardrails added:
- Whether ALTER TABLE commands are allowed to mutate columns
+ - Whether SimpleStrategy is allowed on keyspace creation or alteration
Upgrading
---------
diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml
index 409110a024..491740f012 100644
--- a/conf/cassandra.yaml
+++ b/conf/cassandra.yaml
@@ -1763,6 +1763,9 @@ drop_compact_storage_enabled: false
# Guardrail to allow/disallow querying with ALLOW FILTERING. Defaults to true.
# allow_filtering_enabled: true
#
+# Guardrail to allow/disallow setting SimpleStrategy via keyspace creation or
alteration. Defaults to true.
+# simplestrategy_enabled: true
+#
# Guardrail to warn or fail when creating a user-defined-type with more fields
in than threshold.
# Default -1 to disable.
# fields_per_udt_warn_threshold: -1
diff --git a/src/java/org/apache/cassandra/config/Config.java
b/src/java/org/apache/cassandra/config/Config.java
index 9f75a277bc..c3c5b3582c 100644
--- a/src/java/org/apache/cassandra/config/Config.java
+++ b/src/java/org/apache/cassandra/config/Config.java
@@ -833,6 +833,7 @@ public class Config
public volatile boolean compact_tables_enabled = true;
public volatile boolean read_before_write_list_operations_enabled = true;
public volatile boolean allow_filtering_enabled = true;
+ public volatile boolean simplestrategy_enabled = true;
public volatile DataStorageSpec.LongBytesBound
collection_size_warn_threshold = null;
public volatile DataStorageSpec.LongBytesBound
collection_size_fail_threshold = null;
public volatile int items_per_collection_warn_threshold = -1;
diff --git a/src/java/org/apache/cassandra/config/GuardrailsOptions.java
b/src/java/org/apache/cassandra/config/GuardrailsOptions.java
index b14f428ca4..e8d7bda77a 100644
--- a/src/java/org/apache/cassandra/config/GuardrailsOptions.java
+++ b/src/java/org/apache/cassandra/config/GuardrailsOptions.java
@@ -427,6 +427,20 @@ public class GuardrailsOptions implements GuardrailsConfig
x -> config.allow_filtering_enabled = x);
}
+ @Override
+ public boolean getSimpleStrategyEnabled()
+ {
+ return config.simplestrategy_enabled;
+ }
+
+ public void setSimpleStrategyEnabled(boolean enabled)
+ {
+ updatePropertyWithLogging("simplestrategy_enabled",
+ enabled,
+ () -> config.simplestrategy_enabled,
+ x -> config.simplestrategy_enabled = x);
+ }
+
@Override
public int getInSelectCartesianProductWarnThreshold()
{
diff --git
a/src/java/org/apache/cassandra/cql3/statements/schema/AlterKeyspaceStatement.java
b/src/java/org/apache/cassandra/cql3/statements/schema/AlterKeyspaceStatement.java
index 87377d70ec..dec0655e74 100644
---
a/src/java/org/apache/cassandra/cql3/statements/schema/AlterKeyspaceStatement.java
+++
b/src/java/org/apache/cassandra/cql3/statements/schema/AlterKeyspaceStatement.java
@@ -31,12 +31,14 @@ import org.apache.cassandra.config.DatabaseDescriptor;
import org.apache.cassandra.cql3.CQLStatement;
import org.apache.cassandra.db.ColumnFamilyStore;
import org.apache.cassandra.db.Keyspace;
+import org.apache.cassandra.db.guardrails.Guardrails;
import org.apache.cassandra.exceptions.ConfigurationException;
import org.apache.cassandra.gms.Gossiper;
import org.apache.cassandra.locator.AbstractReplicationStrategy;
import org.apache.cassandra.locator.InetAddressAndPort;
import org.apache.cassandra.locator.LocalStrategy;
import org.apache.cassandra.locator.ReplicationFactor;
+import org.apache.cassandra.locator.SimpleStrategy;
import org.apache.cassandra.schema.KeyspaceMetadata;
import org.apache.cassandra.schema.KeyspaceMetadata.KeyspaceDiff;
import org.apache.cassandra.schema.Keyspaces;
@@ -76,6 +78,9 @@ public final class AlterKeyspaceStatement extends
AlterSchemaStatement
KeyspaceMetadata newKeyspace =
keyspace.withSwapped(attrs.asAlteredKeyspaceParams(keyspace.params));
+ if (attrs.getReplicationStrategyClass() != null &&
attrs.getReplicationStrategyClass().equals(SimpleStrategy.class.getSimpleName()))
+ Guardrails.simpleStrategyEnabled.ensureEnabled(state);
+
if (newKeyspace.params.replication.klass.equals(LocalStrategy.class))
throw ire("Unable to use given strategy class: LocalStrategy is
reserved for internal use.");
diff --git
a/src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java
b/src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java
index dc82f93a10..ad6bcc472d 100644
---
a/src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java
+++
b/src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java
@@ -36,6 +36,7 @@ import org.apache.cassandra.cql3.CQLStatement;
import org.apache.cassandra.db.guardrails.Guardrails;
import org.apache.cassandra.exceptions.AlreadyExistsException;
import org.apache.cassandra.locator.LocalStrategy;
+import org.apache.cassandra.locator.SimpleStrategy;
import org.apache.cassandra.schema.KeyspaceMetadata;
import org.apache.cassandra.schema.KeyspaceParams.Option;
import org.apache.cassandra.schema.Keyspaces;
@@ -67,6 +68,9 @@ public final class CreateKeyspaceStatement extends
AlterSchemaStatement
if (!attrs.hasOption(Option.REPLICATION))
throw ire("Missing mandatory option '%s'", Option.REPLICATION);
+ if (attrs.getReplicationStrategyClass() != null &&
attrs.getReplicationStrategyClass().equals(SimpleStrategy.class.getSimpleName()))
+ Guardrails.simpleStrategyEnabled.ensureEnabled("SimpleStrategy",
state);
+
if (schema.containsKeyspace(keyspaceName))
{
if (ifNotExists)
diff --git
a/src/java/org/apache/cassandra/cql3/statements/schema/KeyspaceAttributes.java
b/src/java/org/apache/cassandra/cql3/statements/schema/KeyspaceAttributes.java
index 42fcaf4e69..d4d5b984b3 100644
---
a/src/java/org/apache/cassandra/cql3/statements/schema/KeyspaceAttributes.java
+++
b/src/java/org/apache/cassandra/cql3/statements/schema/KeyspaceAttributes.java
@@ -50,7 +50,7 @@ public final class KeyspaceAttributes extends
PropertyDefinitions
throw new ConfigurationException("Missing replication strategy
class");
}
- private String getReplicationStrategyClass()
+ public String getReplicationStrategyClass()
{
return getAllReplicationOptions().get(ReplicationParams.CLASS);
}
diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
index 57dd2af5ce..16146fec87 100644
--- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
+++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
@@ -218,6 +218,14 @@ public final class Guardrails implements GuardrailsMBean
state ->
CONFIG_PROVIDER.getOrCreate(state).getAllowFilteringEnabled(),
"Querying with ALLOW FILTERING");
+ /**
+ * Guardrail disabling setting SimpleStrategy via keyspace creation or
alteration
+ */
+ public static final EnableFlag simpleStrategyEnabled =
+ new EnableFlag("simplestrategy",
+ state ->
CONFIG_PROVIDER.getOrCreate(state).getSimpleStrategyEnabled(),
+ "SimpleStrategy");
+
/**
* Guardrail on the number of restrictions created by a cartesian product
of a CQL's {@code IN} query.
*/
@@ -571,6 +579,18 @@ public final class Guardrails implements GuardrailsMBean
DEFAULT_CONFIG.setAllowFilteringEnabled(enabled);
}
+ @Override
+ public boolean getSimpleStrategyEnabled()
+ {
+ return DEFAULT_CONFIG.getSimpleStrategyEnabled();
+ }
+
+ @Override
+ public void setSimpleStrategyEnabled(boolean enabled)
+ {
+ DEFAULT_CONFIG.setSimpleStrategyEnabled(enabled);
+ }
+
@Override
public boolean getUncompressedTablesEnabled()
{
diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java
b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java
index 7f38e80ec3..72eaaa5b48 100644
--- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java
+++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java
@@ -191,6 +191,13 @@ public interface GuardrailsConfig
*/
boolean getAllowFilteringEnabled();
+ /**
+ * Returns whether setting SimpleStrategy via keyspace creation or
alteration is enabled
+ *
+ * @return {@code true} if SimpleStrategy is allowed, {@code false}
otherwise.
+ */
+ boolean getSimpleStrategyEnabled();
+
/**
* @return The threshold to warn when an IN query creates a cartesian
product with a size exceeding threshold.
* -1 means disabled.
diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java
b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java
index 30be464d02..47db91a6fa 100644
--- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java
+++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java
@@ -222,6 +222,20 @@ public interface GuardrailsMBean
*/
void setAllowFilteringEnabled(boolean enabled);
+ /**
+ * Returns whether SimpleStrategy is allowed on keyspace creation or
alteration
+ *
+ * @return {@code true} if SimpleStrategy is allowed; {@code false}
otherwise
+ */
+ boolean getSimpleStrategyEnabled();
+
+ /**
+ * Sets whether SimpleStrategy is allowed on keyspace creation or
alteration
+ *
+ * @param enabled {@code true} if SimpleStrategy is allowed, {@code false}
otherwise.
+ */
+ void setSimpleStrategyEnabled(boolean enabled);
+
/**
* Returns whether users can disable compression on tables
*
diff --git
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailSimpleStrategyTest.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailSimpleStrategyTest.java
new file mode 100644
index 0000000000..3cc6bc747d
--- /dev/null
+++
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailSimpleStrategyTest.java
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.db.guardrails;
+
+import org.junit.After;
+import org.junit.Test;
+
+public class GuardrailSimpleStrategyTest extends GuardrailTester
+{
+ public static String ERROR_MSG = "SimpleStrategy is not allowed";
+
+ public GuardrailSimpleStrategyTest()
+ {
+ super(Guardrails.simpleStrategyEnabled);
+ }
+
+ private void setGuardrail(boolean enabled)
+ {
+ guardrails().setSimpleStrategyEnabled(enabled);
+ }
+
+ @After
+ public void afterTest() throws Throwable
+ {
+ setGuardrail(true);
+ execute("DROP KEYSPACE IF EXISTS test_ss;");
+ }
+
+ @Test
+ public void testCanCreateWithGuardrailEnabled() throws Throwable
+ {
+ assertValid("CREATE KEYSPACE test_ss WITH replication = {'class':
'SimpleStrategy'};");
+ }
+
+ @Test
+ public void testCanAlterWithGuardrailEnabled() throws Throwable
+ {
+ execute("CREATE KEYSPACE test_ss WITH replication = {'class':
'NetworkTopologyStrategy', 'datacenter1':2, 'datacenter2':0};");
+ assertValid("ALTER KEYSPACE test_ss WITH replication = {'class':
'SimpleStrategy'};");
+ }
+
+ @Test
+ public void testGuardrailBlocksCreate() throws Throwable
+ {
+ setGuardrail(false);
+ assertFails("CREATE KEYSPACE test_ss WITH replication = {'class':
'SimpleStrategy'};", ERROR_MSG);
+ }
+
+ @Test
+ public void testGuardrailBlocksAlter() throws Throwable
+ {
+ setGuardrail(false);
+ execute("CREATE KEYSPACE test_ss WITH replication = {'class':
'NetworkTopologyStrategy', 'datacenter1':2, 'datacenter2':0};");
+ assertFails("ALTER KEYSPACE test_ss WITH replication = {'class':
'SimpleStrategy'};", ERROR_MSG);
+ }
+
+ @Test
+ public void testToggle() throws Throwable
+ {
+ setGuardrail(false);
+ assertFails("CREATE KEYSPACE test_ss WITH replication = {'class':
'SimpleStrategy'};", ERROR_MSG);
+
+ setGuardrail(true);
+ assertValid("CREATE KEYSPACE test_ss WITH replication = {'class':
'SimpleStrategy'};");
+ execute("ALTER KEYSPACE test_ss WITH replication = {'class':
'NetworkTopologyStrategy', 'datacenter1':2, 'datacenter2':0};");
+
+ setGuardrail(false);
+ assertFails("ALTER KEYSPACE test_ss WITH replication = {'class':
'SimpleStrategy'};", ERROR_MSG);
+
+ setGuardrail(true);
+ assertValid("ALTER KEYSPACE test_ss WITH replication = {'class':
'SimpleStrategy'};");
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
