From aafb4d19448f12ce600dc4e84a5b181308825b32 Mon Sep 17 00:00:00 2001 From: Marcus Eriksson Date: Wed, 12 Apr 2023 09:17:50 +0200 Subject: [PATCH] Improve nodetool enable{audit,fullquery}log Patch by marcuse; reviewed by Dinesh Joshi and Mick Semb Wever for CASSANDRA-18550 --- CHANGES.txt | 1 + conf/cassandra.yaml | 2 ++ .../cassandra/config/DatabaseDescriptor.java | 5 ++++ .../cassandra/service/StorageService.java | 2 ++ .../tools/nodetool/EnableFullQueryLog.java | 3 +- .../cassandra/utils/binlog/BinLogOptions.java | 7 +++++ .../cassandra/fql/FullQueryLoggerTest.java | 28 ++++++++++++++++++- 7 files changed, 46 insertions(+), 2 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index b057a07f2a13..4f873aa71cd0 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0.10 + * Improve nodetool enable{audit,fullquery}log (CASSANDRA-18550) * Report network cache info in nodetool (CASSANDRa-18400) * Partial compaction can resurrect deleted data (CASSANDRA-18507) * Allow internal address to change with reconnecting snitches (CASSANDRA-16718) diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index 1b0871fcf180..3e4da52f5f6a 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -1360,6 +1360,8 @@ audit_logging_options: # max_log_size: 17179869184 # 16 GiB ## archive command is "/path/to/script.sh %path" where %path is replaced with the file being rolled: # archive_command: + ## note that enabling this allows anyone with JMX/nodetool access to run local shell commands as the user running cassandra + # allow_nodetool_archive_command: false # max_archive_retries: 10 # validate tombstones on reads and compaction diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java index f78a5b668a60..8893f58e560c 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -3140,6 +3140,11 @@ public static FullQueryLoggerOptions getFullQueryLogOptions() return conf.full_query_logging_options; } + public static void setFullQueryLogOptions(FullQueryLoggerOptions options) + { + conf.full_query_logging_options = options; + } + public static boolean getBlockForPeersInRemoteDatacenters() { return conf.block_for_peers_in_remote_dcs; diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index 6a02b405af19..b53467e81ba0 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -6081,6 +6081,8 @@ public void enableFullQueryLogger(String path, String rollCycle, Boolean blockin blocking = blocking != null ? blocking : fqlOptions.block; maxQueueWeight = maxQueueWeight != Integer.MIN_VALUE ? maxQueueWeight : fqlOptions.max_queue_weight; maxLogSize = maxLogSize != Long.MIN_VALUE ? maxLogSize : fqlOptions.max_log_size; + if (archiveCommand != null && !fqlOptions.allow_nodetool_archive_command) + throw new ConfigurationException("Can't enable full query log archiving via nodetool unless full_query_logging_options.allow_nodetool_archive_command is set to true"); archiveCommand = archiveCommand != null ? archiveCommand : fqlOptions.archive_command; maxArchiveRetries = maxArchiveRetries != Integer.MIN_VALUE ? maxArchiveRetries : fqlOptions.max_archive_retries; diff --git a/src/java/org/apache/cassandra/tools/nodetool/EnableFullQueryLog.java b/src/java/org/apache/cassandra/tools/nodetool/EnableFullQueryLog.java index 9873e5a01aed..d78d5ae80f41 100644 --- a/src/java/org/apache/cassandra/tools/nodetool/EnableFullQueryLog.java +++ b/src/java/org/apache/cassandra/tools/nodetool/EnableFullQueryLog.java @@ -42,7 +42,8 @@ public class EnableFullQueryLog extends NodeToolCmd private String path = null; @Option(title = "archive_command", name = {"--archive-command"}, description = "Command that will handle archiving rolled full query log files." + - " Format is \"/path/to/script.sh %path\" where %path will be replaced with the file to archive") + " Format is \"/path/to/script.sh %path\" where %path will be replaced with the file to archive" + + " Enable this by setting the full_query_logging_options.allow_nodetool_archive_command: true in the config.") private String archiveCommand = null; @Option(title = "archive_retries", name = {"--max-archive-retries"}, description = "Max number of archive retries.") diff --git a/src/java/org/apache/cassandra/utils/binlog/BinLogOptions.java b/src/java/org/apache/cassandra/utils/binlog/BinLogOptions.java index 8005ca38efa7..614d19031c9f 100644 --- a/src/java/org/apache/cassandra/utils/binlog/BinLogOptions.java +++ b/src/java/org/apache/cassandra/utils/binlog/BinLogOptions.java @@ -23,6 +23,13 @@ public class BinLogOptions { public String archive_command = StringUtils.EMPTY; + + /** + * enable if a user should be able to set the archive command via nodetool/jmx + * + * do not make this a hotprop. + */ + public boolean allow_nodetool_archive_command = false; /** * How often to roll BinLog segments so they can potentially be reclaimed. Available options are: * MINUTELY, HOURLY, DAILY, LARGE_DAILY, XLARGE_DAILY, HUGE_DAILY. diff --git a/test/unit/org/apache/cassandra/fql/FullQueryLoggerTest.java b/test/unit/org/apache/cassandra/fql/FullQueryLoggerTest.java index 73be0b493e9c..adb3ab67fd59 100644 --- a/test/unit/org/apache/cassandra/fql/FullQueryLoggerTest.java +++ b/test/unit/org/apache/cassandra/fql/FullQueryLoggerTest.java @@ -32,7 +32,6 @@ import org.apache.commons.lang3.StringUtils; import org.junit.After; -import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; @@ -44,15 +43,18 @@ import net.openhft.chronicle.wire.ValueIn; import net.openhft.chronicle.wire.WireOut; import org.apache.cassandra.Util; +import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.cql3.CQLTester; import org.apache.cassandra.cql3.QueryOptions; import org.apache.cassandra.cql3.statements.BatchStatement; +import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.fql.FullQueryLogger.Query; import org.apache.cassandra.fql.FullQueryLogger.Batch; import org.apache.cassandra.cql3.statements.BatchStatement.Type; import org.apache.cassandra.io.util.FileUtils; import org.apache.cassandra.service.ClientState; import org.apache.cassandra.service.QueryState; +import org.apache.cassandra.service.StorageService; import org.apache.cassandra.transport.ProtocolVersion; import org.apache.cassandra.utils.ObjectSizes; import org.apache.cassandra.utils.binlog.BinLogTest; @@ -74,6 +76,7 @@ import static org.apache.cassandra.fql.FullQueryLogger.TYPE; import static org.apache.cassandra.fql.FullQueryLogger.VALUES; import static org.apache.cassandra.fql.FullQueryLogger.VERSION; +import static org.junit.Assert.fail; public class FullQueryLoggerTest extends CQLTester { @@ -670,6 +673,29 @@ public void testLogQueryNegativeTime() throws Exception logQuery("", QueryOptions.DEFAULT, queryState(), -1); } + @Test + public void testJMXArchiveCommand() + { + FullQueryLoggerOptions options = new FullQueryLoggerOptions(); + + try + { + StorageService.instance.enableFullQueryLogger(options.log_dir, options.roll_cycle, false, 1000, 1000, "/xyz/not/null", 0); + fail("not allowed"); + } + catch (ConfigurationException e) + { + assertTrue(e.getMessage().contains("Can't enable full query log archiving via nodetool")); + } + + options.archive_command = "/xyz/not/null"; + options.log_dir = "/tmp/abc"; + DatabaseDescriptor.setFullQueryLogOptions(options); + StorageService.instance.enableFullQueryLogger(options.log_dir, options.roll_cycle, false, 1000, 1000, null, 0); + assertTrue(FullQueryLogger.instance.isEnabled()); + assertEquals("/xyz/not/null", FullQueryLogger.instance.getFullQueryLoggerOptions().archive_command); + } + private static void compareQueryOptions(QueryOptions a, QueryOptions b) { assertEquals(a.getClass(), b.getClass());