Add guardrail for ALTER TABLE ADD / DROP / REMOVE column operations

Patch by Josh McKenzie; reviewed by Jon Meredith for CASSANDRA-17495

CHANGES.txt

@@ -1,4 +1,5 @@
 4.2
+ * Add guardrail for ALTER TABLE ADD / DROP / REMOVE column operations (CASSANDRA-17495)
  * Rename DisableFlag class to EnableFlag on guardrails (CASSANDRA-17544)
 Merged from 4.1:
 Merged from 4.0:

NEWS.txt

@@ -57,6 +57,8 @@ using the provided 'sstableupgrade' tool.
 
 New features
 ------------
+    - New Guardrails added:
+      - Whether ALTER TABLE commands are allowed to mutate columns
 
 Upgrading
 ---------

conf/cassandra.yaml

@@ -1601,58 +1601,75 @@ drop_compact_storage_enabled: false
 # The two thresholds default to -1 to disable.
 # keyspaces_warn_threshold: -1
 # keyspaces_fail_threshold: -1
+#
 # Guardrail to warn or fail when creating more user tables than threshold.
 # The two thresholds default to -1 to disable.
 # tables_warn_threshold: -1
 # tables_fail_threshold: -1
+#
 # Guardrail to enable or disable the ability to create uncompressed tables
 # uncompressed_tables_enabled: true
+#
 # Guardrail to warn or fail when creating/altering a table with more columns per table than threshold.
 # The two thresholds default to -1 to disable.
 # columns_per_table_warn_threshold: -1
 # columns_per_table_fail_threshold: -1
+#
 # Guardrail to warn or fail when creating more secondary indexes per table than threshold.
 # The two thresholds default to -1 to disable.
 # secondary_indexes_per_table_warn_threshold: -1
 # secondary_indexes_per_table_fail_threshold: -1
+#
 # Guardrail to enable or disable the creation of secondary indexes
 # secondary_indexes_enabled: true
+#
 # Guardrail to warn or fail when creating more materialized views per table than threshold.
 # The two thresholds default to -1 to disable.
 # materialized_views_per_table_warn_threshold: -1
 # materialized_views_per_table_fail_threshold: -1
+#
 # Guardrail to warn about, ignore or reject properties when creating tables. By default all properties are allowed.
 # table_properties_warned: []
 # table_properties_ignored: []
 # table_properties_disallowed: []
+#
 # Guardrail to allow/disallow user-provided timestamps. Defaults to true.
 # user_timestamps_enabled: true
+#
 # Guardrail to allow/disallow GROUP BY functionality.
 # group_by_enabled: true
+#
 # Guardrail to allow/disallow TRUNCATE and DROP TABLE statements
 # drop_truncate_table_enabled: true
+#
 # Guardrail to warn or fail when using a page size greater than threshold.
 # The two thresholds default to -1 to disable.
 # page_size_warn_threshold: -1
 # page_size_fail_threshold: -1
+#
 # Guardrail to allow/disallow list operations that require read before write, i.e. setting list element by index and
 # removing list elements by either index or value. Defaults to true.
 # read_before_write_list_operations_enabled: true
+#
 # Guardrail to warn or fail when querying with an IN restriction selecting more partition keys than threshold.
 # The two thresholds default to -1 to disable.
 # partition_keys_in_select_warn_threshold: -1
 # partition_keys_in_select_fail_threshold: -1
+#
 # Guardrail to warn or fail when an IN query creates a cartesian product with a size exceeding threshold,
 # eg. "a in (1,2,...10) and b in (1,2...10)" results in cartesian product of 100.
 # The two thresholds default to -1 to disable.
 # in_select_cartesian_product_warn_threshold: -1
 # in_select_cartesian_product_fail_threshold: -1
+#
 # Guardrail to warn about or reject read consistency levels. By default, all consistency levels are allowed.
 # read_consistency_levels_warned: []
 # read_consistency_levels_disallowed: []
+#
 # Guardrail to warn about or reject write consistency levels. By default, all consistency levels are allowed.
 # write_consistency_levels_warned: []
 # write_consistency_levels_disallowed: []
+#
 # Guardrail to warn or fail when encountering larger size of collection data than threshold.
 # At query time this guardrail is applied only to the collection fragment that is being writen, even though in the case
 # of non-frozen collections there could be unaccounted parts of the collection on the sstables. This is done this way to
@@ -1661,6 +1678,7 @@ drop_compact_storage_enabled: false
 # The two thresholds default to null to disable.
 # collection_size_warn_threshold:
 # collection_size_fail_threshold:
+#
 # Guardrail to warn or fail when encountering more elements in collection than threshold.
 # At query time this guardrail is applied only to the collection fragment that is being writen, even though in the case
 # of non-frozen collections there could be unaccounted parts of the collection on the sstables. This is done this way to
@@ -1669,12 +1687,18 @@ drop_compact_storage_enabled: false
 # The two thresholds default to -1 to disable.
 # items_per_collection_warn_threshold: -1
 # items_per_collection_fail_threshold: -1
+#
 # Guardrail to allow/disallow querying with ALLOW FILTERING. Defaults to true.
 # allow_filtering_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
 # fields_per_udt_fail_threshold: -1
+#
+# Guardrail to indicate whether or not users are allowed to use ALTER TABLE commands to make column changes to tables
+# alter_table_enabled: true
+#
 # Guardrail to warn or fail when local data disk usage percentage exceeds threshold. Valid values are in [1, 100].
 # This is only used for the disks storing data directories, so it won't count any separate disks used for storing
 # the commitlog, hints nor saved caches. The disk usage is the ratio between the amount of space used by the data
@@ -1686,13 +1710,15 @@ drop_compact_storage_enabled: false
 # The two thresholds default to -1 to disable.
 # data_disk_usage_percentage_warn_threshold: -1
 # data_disk_usage_percentage_fail_threshold: -1
-# Allows defining the max disk size of the data directories when calculating thresholds for
+#
+# Guardrail that allows users to define the max disk size of the data directories when calculating thresholds for
 # disk_usage_percentage_warn_threshold and disk_usage_percentage_fail_threshold, so if this is greater than zero they
 # become percentages of a fixed size on disk instead of percentages of the physically available disk size. This should
 # be useful when we have a large disk and we only want to use a part of it for Cassandra's data directories.
 # Valid values are in [1, max available disk size of all data directories].
 # Defaults to null to disable and use the physically available disk size of data directories during calculations.
 # data_disk_usage_max_disk_size:
+#
 # Guardrail to warn or fail when the minimum replication factor is lesser than threshold.
 # This would also apply to system keyspaces.
 # Suggested value for use in production: 2 or higher

src/java/org/apache/cassandra/config/Config.java

@@ -825,6 +825,7 @@ public class Config
     public volatile Set<ConsistencyLevel> write_consistency_levels_warned = Collections.emptySet();
     public volatile Set<ConsistencyLevel> write_consistency_levels_disallowed = Collections.emptySet();
     public volatile boolean user_timestamps_enabled = true;
+    public volatile boolean alter_table_enabled = true;
     public volatile boolean group_by_enabled = true;
     public volatile boolean drop_truncate_table_enabled = true;
     public volatile boolean secondary_indexes_enabled = true;

src/java/org/apache/cassandra/config/GuardrailsOptions.java

@@ -385,6 +385,20 @@ public class GuardrailsOptions implements GuardrailsConfig
                                   x -> config.compact_tables_enabled = x);
     }
 
+    @Override
+    public boolean getAlterTableEnabled()
+    {
+        return config.alter_table_enabled;
+    }
+
+    public void setAlterTableEnabled(boolean enabled)
+    {
+        updatePropertyWithLogging("alter_table_enabled",
+                                  enabled,
+                                  () -> config.alter_table_enabled,
+                                  x -> config.alter_table_enabled = x);
+    }
+
     @Override
     public boolean getReadBeforeWriteListOperationsEnabled()
     {

src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java

@@ -79,6 +79,7 @@ public abstract class AlterTableStatement extends AlterSchemaStatement
 {
     protected final String tableName;
     private final boolean ifExists;
+    protected ClientState state;
 
     public AlterTableStatement(String keyspaceName, String tableName, boolean ifExists)
     {
@@ -87,6 +88,15 @@ public abstract class AlterTableStatement extends AlterSchemaStatement
         this.ifExists = ifExists;
     }
 
+    @Override
+    public void validate(ClientState state)
+    {
+        super.validate(state);
+
+        // save the query state to use it for guardrails validation in #apply
+        this.state = state;
+    }
+
     public Keyspaces apply(Keyspaces schema)
     {
         KeyspaceMetadata keyspace = schema.getNullable(keyspaceName);
@@ -187,6 +197,7 @@ public abstract class AlterTableStatement extends AlterSchemaStatement
 
         public KeyspaceMetadata apply(KeyspaceMetadata keyspace, TableMetadata table)
         {
+            Guardrails.alterTableEnabled.ensureEnabled("ALTER TABLE changing columns", state);
             TableMetadata.Builder tableBuilder = table.unbuild();
             Views.Builder viewsBuilder = keyspace.views.unbuild();
             newColumns.forEach(c -> addColumn(keyspace, table, c, ifColumnNotExists, tableBuilder, viewsBuilder));
@@ -289,6 +300,7 @@ public abstract class AlterTableStatement extends AlterSchemaStatement
 
         public KeyspaceMetadata apply(KeyspaceMetadata keyspace, TableMetadata table)
         {
+            Guardrails.alterTableEnabled.ensureEnabled("ALTER TABLE changing columns", state);
             TableMetadata.Builder builder = table.unbuild();
             removedColumns.forEach(c -> dropColumn(keyspace, table, c, ifColumnExists, builder));
             return keyspace.withSwapped(keyspace.tables.withSwapped(builder.build()));
@@ -356,6 +368,7 @@ public abstract class AlterTableStatement extends AlterSchemaStatement
 
         public KeyspaceMetadata apply(KeyspaceMetadata keyspace, TableMetadata table)
         {
+            Guardrails.alterTableEnabled.ensureEnabled("ALTER TABLE changing columns", state);
             TableMetadata.Builder tableBuilder = table.unbuild();
             Views.Builder viewsBuilder = keyspace.views.unbuild();
             renamedColumns.forEach((o, n) -> renameColumn(keyspace, table, o, n, ifColumnsExists, tableBuilder, viewsBuilder));

src/java/org/apache/cassandra/db/guardrails/Guardrails.java

@@ -145,6 +145,14 @@ public final class Guardrails implements GuardrailsMBean
                    state -> CONFIG_PROVIDER.getOrCreate(state).getGroupByEnabled(),
                    "GROUP BY functionality");
 
+    /**
+     * Guardrail disabling ALTER TABLE column mutation access.
+     */
+    public static final EnableFlag alterTableEnabled =
+    new EnableFlag("alter_table",
+                    state -> CONFIG_PROVIDER.getOrCreate(state).getAlterTableEnabled(),
+                    "User access to ALTER TABLE statement for column mutation");
+
     public static final EnableFlag dropTruncateTableEnabled =
     new EnableFlag("drop_truncate_table_enabled",
                    state -> CONFIG_PROVIDER.getOrCreate(state).getDropTruncateTableEnabled(),
@@ -539,6 +547,18 @@ public final class Guardrails implements GuardrailsMBean
         DEFAULT_CONFIG.setUserTimestampsEnabled(enabled);
     }
 
+    @Override
+    public boolean getAlterTableEnabled()
+    {
+        return DEFAULT_CONFIG.getAlterTableEnabled();
+    }
+
+    @Override
+    public void setAlterTableEnabled(boolean enabled)
+    {
+        DEFAULT_CONFIG.setAlterTableEnabled(enabled);
+    }
+
     @Override
     public boolean getAllowFilteringEnabled()
     {

src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java

@@ -132,6 +132,13 @@ public interface GuardrailsConfig
      */
     boolean getUserTimestampsEnabled();
 
+    /**
+     * Returns whether users are allowed access to the ALTER TABLE statement to mutate columns or not
+     *
+     * @return {@code true} if ALTER TABLE ADD/REMOVE/RENAME is allowed, {@code false} otherwise.
+     */
+    boolean getAlterTableEnabled();
+
     /**
      * Returns whether tables can be uncompressed
      *

src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java

@@ -250,6 +250,20 @@ public interface GuardrailsMBean
      */
     void setCompactTablesEnabled(boolean enabled);
 
+    /**
+     * Gets whether users can use the ALTER TABLE statement to change columns
+     *
+     * @return {@code true} if ALTER TABLE is allowed, {@code false} otherwise.
+     */
+    boolean getAlterTableEnabled();
+
+    /**
+     * Sets whether users can use the ALTER TABLE statement to change columns
+     *
+     * @param enabled {@code true} if changing columns is allowed, {@code false} otherwise.
+     */
+    void setAlterTableEnabled(boolean enabled);
+
     /**
      * Returns whether GROUP BY queries are allowed.
      *

src/java/org/apache/cassandra/service/StorageService.java

@@ -17,7 +17,6 @@
  */
 package org.apache.cassandra.service;
 
-
 import java.io.ByteArrayInputStream;
 import java.io.DataInputStream;
 import java.io.IOError;

test/unit/org/apache/cassandra/db/guardrails/GuardrailAlterTableTest.java

@@ -0,0 +1,133 @@
+/*
+ * 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.Before;
+import org.junit.Test;
+
+
+/**
+ * Tests the guardrail for disabling user access to the ALTER TABLE statement, {@link Guardrails#alterTableEnabled}.
+ *
+ * NOTE: This test class depends on {@link #currentTable()} method for setup, cleanup, and execution of tests. You'll
+ * need to refactor this if you add tests that make changes to the current table as the test teardown will no longer match
+ * setup.
+ */
+public class GuardrailAlterTableTest extends GuardrailTester
+{
+    public GuardrailAlterTableTest()
+    {
+        super(Guardrails.alterTableEnabled);
+    }
+
+    @Before
+    public void setupTest() throws Throwable
+    {
+        createTable("CREATE TABLE IF NOT EXISTS %s (k INT, c INT, v TEXT, PRIMARY KEY(k, c))");
+    }
+
+    @After
+    public void afterTest() throws Throwable
+    {
+        dropTable("DROP TABLE %s");
+        setGuardrail(true);
+    }
+
+    private void setGuardrail(boolean alterTableEnabled)
+    {
+        guardrails().setAlterTableEnabled(alterTableEnabled);
+    }
+
+    /**
+     * Confirm that ALTER TABLE queries either work (guardrail enabled) or fail (guardrail disabled) appropriately
+     * @throws Throwable
+     */
+    @Test
+    public void testGuardrailEnabledAndDisabled() throws Throwable
+    {
+        setGuardrail(false);
+        assertFails("ALTER TABLE %s ADD test_one text;", "changing columns");
+
+        setGuardrail(true);
+        assertValid("ALTER TABLE %s ADD test_two text;");
+
+        setGuardrail(false);
+        assertFails("ALTER TABLE %s ADD test_three text;", "changing columns");
+    }
+
+    /**
+     * Confirm the guardrail appropriately catches the ALTER DROP case on a column
+     * @throws Throwable
+     */
+    @Test
+    public void testAppliesToAlterDropColumn() throws Throwable
+    {
+        setGuardrail(true);
+        assertValid("ALTER TABLE %s ADD test_one text;");
+
+        setGuardrail(false);
+        assertFails("ALTER TABLE %s DROP test_one", "changing columns");
+
+        setGuardrail(true);
+        assertValid("ALTER TABLE %s DROP test_one");
+    }
+
+    /**
+     * Confirm the guardrail appropriately catches the ALTER RENAME case on a column
+     * @throws Throwable
+     */
+    @Test
+    public void testAppliesToAlterRenameColumn() throws Throwable
+    {
+        setGuardrail(false);
+        assertFails("ALTER TABLE %s RENAME c TO renamed_c", "changing columns");
+
+        setGuardrail(true);
+        assertValid("ALTER TABLE %s RENAME c TO renamed_c");
+    }
+
+    /**
+     * Confirm we can always alter properties via the options map regardless of guardrail state
+     * @throws Throwable
+     */
+    @Test
+    public void testAlterViaMapAlwaysWorks() throws Throwable
+    {
+        setGuardrail(false);
+        assertValid("ALTER TABLE %s WITH compression = { 'class' : 'SnappyCompressor', 'chunk_length_in_kb' : 32 };");
+
+        setGuardrail(true);
+        assertValid("ALTER TABLE %s WITH compression = { 'class' : 'SnappyCompressor', 'chunk_length_in_kb' : 32 };");
+    }
+
+    /**
+     * Confirm the other form of ALTER TABLE property map changing always works regardless of guardrail state
+     * @throws Throwable
+     */
+    @Test
+    public void testAlterOptionsAlwaysWorks() throws Throwable
+    {
+        setGuardrail(true);
+        assertValid("ALTER TABLE %s WITH GC_GRACE_SECONDS = 456; ");
+
+        setGuardrail(false);
+        assertValid("ALTER TABLE %s WITH GC_GRACE_SECONDS = 123; ");
+    }
+}