diff --git a/CHANGES.txt b/CHANGES.txt index a96a3ab097..ffaa1b5d00 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -268,6 +268,7 @@ Merged from 5.0: * Prioritize built indexes in IndexStatusManager (CASSANDRA-19400) * Add java.base/java.lang.reflect among opens for jvm11-client.options (CASSANDRA-19780) Merged from 4.1: + * Rework / simplification of nodetool get/setguardrailsconfig commands (CASSANDRA-20778) * IntrusiveStack.accumulate is not accumulating correctly (CASSANDRA-20670) * Add nodetool get/setguardrailsconfig commands (CASSANDRA-19552) * Fix mixed mode paxos ttl commit hang (CASSANDRA-20514) diff --git a/src/java/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommand.java b/src/java/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommand.java index 935e538788..e71713641c 100644 --- a/src/java/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommand.java +++ b/src/java/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommand.java @@ -22,9 +22,13 @@ import java.io.PrintStream; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.function.Function; import java.util.regex.Pattern; @@ -57,7 +61,11 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd allowedValues = { "values", "thresholds", "flags", "others" }) private String guardrailCategory; - @Arguments(description = "Specific names or guardrails to get configuration of.") + @Option(name = { "--expand" }, + description = "Expand all guardrail names so they reflect their counterparts in cassandra.yaml") + private boolean expand = false; + + @Arguments(description = "Specific name of a guardrail to get configuration of.") private List args = new ArrayList<>(); @Override @@ -65,47 +73,104 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd { GuardrailCategory categoryEnum = GuardrailCategory.parseCategory(guardrailCategory, probe.output().out); - if (!args.isEmpty() && categoryEnum != null) + if (args.size() > 1) + throw new IllegalStateException("Specify only one guardrail name to get the configuration of or no name to get the configuration of all of them."); + + String guardrailName = !args.isEmpty() ? args.get(0) : null; + + if (guardrailName != null && categoryEnum != null) throw new IllegalStateException("Do not specify additional arguments when --category/-c is set."); - List allGetters = stream(probe.getGuardrailsMBean().getClass().getDeclaredMethods()) - .filter(method -> method.getName().startsWith("get") - && !method.getName().endsWith("CSV")) - .filter(method -> args.isEmpty() || args.contains(toSnakeCase(method.getName().substring(3)))) - .sorted(comparing(Method::getName)) - .collect(toList()); + Map> allGetters = parseGuardrailNames(probe.getGuardrailsMBean().getClass().getDeclaredMethods(), guardrailName); - display(probe, allGetters, categoryEnum); + if (allGetters.isEmpty()) + { + assert guardrailName != null; + throw new IllegalStateException(format("Guardrail %s not found.", guardrailName)); + } + + display(probe, allGetters, categoryEnum, expand); + } + + @VisibleForTesting + public static Map> parseGuardrailNames(Method[] guardrailsMethods, String guardrailName) + { + Map> allGetters = stream(guardrailsMethods) + .filter(method -> method.getName().startsWith("get") + && !method.getName().endsWith("CSV") + && !(method.getName().endsWith("WarnThreshold") || method.getName().endsWith("FailThreshold"))) + .filter(method -> guardrailName == null || guardrailName.equals(toSnakeCase(method.getName().substring(3)))) + .collect(Collectors.groupingBy(method -> toSnakeCase(method.getName().substring(3)))); + + // TODO for now remove custom guardrails + for (String ignore : ignored) + allGetters.remove(ignore); + + Map> thresholds = stream(guardrailsMethods) + .filter(method -> method.getName().startsWith("get") + && !method.getName().endsWith("CSV") + && (method.getName().endsWith("WarnThreshold") || method.getName().endsWith("FailThreshold"))) + .filter(method -> { + if (guardrailName == null) + return true; + + String snakeCase = toSnakeCase(method.getName().substring(3)); + String snakeCaseSuccinct = snakeCase.replace("_warn_", "_") + .replace("_fail_", "_"); + + return guardrailName.equals(snakeCase) || guardrailName.equals(snakeCaseSuccinct); + }) + .sorted(comparing(Method::getName)) + .collect(Collectors.groupingBy(method -> { + String methodName = method.getName().substring(3); + String snakeCase = toSnakeCase(methodName); + if (snakeCase.endsWith("warn_threshold")) + return snakeCase.replaceAll("_warn_", "_"); + else + return snakeCase.replaceAll("_fail_", "_"); + })); + + allGetters.putAll(thresholds); + + return allGetters.entrySet() + .stream() + .sorted(Map.Entry.comparingByKey()) + .collect(Collectors.toMap(Map.Entry::getKey, + Map.Entry::getValue, + (e1, e2) -> e1, + LinkedHashMap::new)); } @Override - public void addRow(List bucket, GuardrailsMBean mBean, Method method, String guardrailName) throws Throwable + public void addRow(List bucket, GuardrailsMBean mBean, List methods, String guardrailName) throws Throwable { - Class returnType = method.getReturnType(); - Object value = method.invoke(mBean); + List values = new ArrayList<>(); + for (Method method : methods) + { + Class returnType = method.getReturnType(); + Object value = method.invoke(mBean); - if (returnType.equals(int.class) || returnType.equals(Integer.class) - || returnType.equals(long.class) || returnType.equals(Long.class) - || returnType.equals(boolean.class) || returnType.equals(Boolean.class) - || returnType.equals(Set.class)) - { - constructRow(bucket, guardrailName, value.toString()); - } - else if (returnType.equals(String.class)) - { - if (value == null || value.toString().isEmpty()) - constructRow(bucket, guardrailName, "null"); + if (returnType.equals(int.class) || returnType.equals(Integer.class) + || returnType.equals(long.class) || returnType.equals(Long.class) + || returnType.equals(boolean.class) || returnType.equals(Boolean.class) + || returnType.equals(Set.class)) + { + values.add(value.toString()); + } + else if (returnType.equals(String.class)) + { + if (value == null || value.toString().isEmpty()) + values.add("null"); + else + values.add(value.toString()); + } else - constructRow(bucket, guardrailName, value.toString()); - } - else if (returnType.equals(Map.class)) - { - // TODO for now skipping, only custom guardrails are configured by a map - } - else - { - throw new RuntimeException("unhandled return type: " + returnType.getTypeName()); + { + throw new RuntimeException("Unhandled return type: " + returnType.getTypeName()); + } } + + constructRow(bucket, guardrailName, values.size() == 1 ? values.get(0) : values.toString()); } } @@ -114,54 +179,76 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd { private static final Pattern SETTER_PATTERN = Pattern.compile("^set"); - @Option(name = { "--list", "-l" }, - description = "List all available guardrails setters") - private boolean list; - - @Option(name = { "--category", "-c" }, - description = "Category of guardrails to filter, can be one of 'values', 'thresholds', 'flags', 'others'.", - allowedValues = { "values", "thresholds", "flags", "others" }) - private String guardrailCategory; - @Arguments(usage = "[ ...]", description = "For flags, possible values are 'true' or 'false'. " + - "For thresholds, two values are expected, first for warning, second for failure. " + - "For values, one value is expected, multiple values separated by comma.") + "For thresholds, two values are expected, first for failure, second for warning. " + + "For values, enumeration of values expected or one value where multiple items are separated by comma. " + + "Setting for thresholds accepting strings and value guardrails are reset by specifying 'null' or '[]' value. " + + "For thresholds accepting integers, the reset value is -1.") private final List args = new ArrayList<>(); @Override public void execute(NodeProbe probe) { - if (!list && guardrailCategory != null) - throw new IllegalStateException("--category/-c can be specified only together with --list/-l"); - - GuardrailCategory categoryEnum = GuardrailCategory.parseCategory(guardrailCategory, probe.output().out); - - if (args.isEmpty() && !list) + if (args.isEmpty()) throw new IllegalStateException("No arguments."); - if (list) - display(probe, getAllSetters(probe), categoryEnum); - else - executeSetter(probe); + String snakeCaseName = args.get(0); + + Method setter = getAllSetters(probe).entrySet().stream() + .findFirst() + .map(o -> o.getValue().get(0)) + .orElseThrow(() -> new IllegalStateException(format("Guardrail %s not found.", snakeCaseName))); + + sanitizeArguments(setter, args); + validateArguments(setter, snakeCaseName, args); + + List methodArgs = args.subList(1, args.size()); + try + { + setter.invoke(probe.getGuardrailsMBean(), prepareArguments(methodArgs, setter)); + } + catch (Exception ex) + { + String reason; + if (ex.getCause() != null && ex.getCause().getMessage() != null) + reason = ex.getCause().getMessage(); + else + reason = ex.getMessage(); + + throw new IllegalStateException(format("Error occured when setting the config for setter %s with arguments %s: %s", + snakeCaseName, methodArgs, reason)); + } } @Override - public void addRow(List bucket, GuardrailsMBean mBean, Method method, String guardrailName) throws Throwable + public void addRow(List bucket, GuardrailsMBean mBean, List methods, String guardrailName) throws Throwable { - if (method.getParameterTypes().length == 1) - constructRow(bucket, sanitizeSetterName(method), method.getParameterTypes()[0].getName()); - else - constructRow(bucket, sanitizeSetterName(method), stream(method.getParameterTypes()).map(Class::getName).collect(toList()).toString()); + if (methods.size() == 1) + { + Method method = methods.get(0); + if (method.getParameterTypes().length == 1) + constructRow(bucket, sanitizeSetterName(method), method.getParameterTypes()[0].getName()); + else + constructRow(bucket, sanitizeSetterName(method), stream(method.getParameterTypes()).map(Class::getName).collect(toList()).toString()); + } } - private List getAllSetters(NodeProbe probe) + private Map> getAllSetters(NodeProbe probe) { return stream(probe.getGuardrailsMBean().getClass().getDeclaredMethods()) .filter(method -> method.getName().startsWith("set") && !method.getName().endsWith("CSV")) .filter(method -> args.isEmpty() || args.contains(toSnakeCase(method.getName().substring(3)))) .sorted(comparing(Method::getName)) - .collect(toList()); + .collect(Collectors.groupingBy(method -> toSnakeCase(method.getName().substring(3)))) + .entrySet() + .stream() + .filter(p -> !ignored.contains(p.getKey())) + .sorted(Map.Entry.comparingByKey()) + .collect(Collectors.toMap(Map.Entry::getKey, + Map.Entry::getValue, + (e1, e2) -> e1, + LinkedHashMap::new)); } private String sanitizeSetterName(Method setter) @@ -169,34 +256,20 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd return toSnakeCase(SETTER_PATTERN.matcher(setter.getName()).replaceAll("")); } - private void executeSetter(NodeProbe nodeProbe) + private void sanitizeArguments(Method setter, List args) { - String snakeCaseName = args.get(0); - String setterName = toCamelCase(args.get(0).startsWith("set_") ? args.get(0) : "set_" + args.get(0)); - - Method setter = getAllSetters(nodeProbe).stream() - .findFirst() - .orElseThrow(() -> new IllegalStateException(format("Setter method %s not found. " + - "Run nodetool setguardrailsconfig --list " + - "to see available setters", setterName))); - - validateArguments(setter, snakeCaseName, args); - - List methodArgs = args.subList(1, args.size()); - try + Class[] parameterTypes = setter.getParameterTypes(); + if (parameterTypes.length == 1 && parameterTypes[0] == Set.class) { - setter.invoke(nodeProbe.getGuardrailsMBean(), prepareArguments(methodArgs, setter)); - } - catch (Exception ex) - { - String reason; - if (ex.getCause() != null) - reason = ex.getCause().getMessage(); - else - reason = ex.getMessage(); - - throw new IllegalStateException(format("Error occured when setting the config for setter %s with arguments %s: %s", - snakeCaseName, methodArgs, reason)); + if (args.size() > 2) + { + String guardrail = args.get(0); + // replace multiple arguments with one which is separated by a single comma + String collectedArguments = String.join(",", args.subList(1, args.size())); + args.clear(); + args.add(guardrail); + args.add(collectedArguments); + } } } @@ -219,6 +292,13 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd for (int i = 0; i < args.size(); i++) arguments[i] = castType(parameterTypes[i], args.get(i)); + if (method.getName().endsWith("Threshold")) + { + List thresholdArgs = Arrays.asList(arguments); + Collections.reverse(thresholdArgs); + arguments = thresholdArgs.toArray(); + } + return arguments; } @@ -241,10 +321,10 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd } else if (targetType == Set.class) { - if (value == null || value.equals("null")) - return Set.of(); + if (value == null || value.equals("null") || value.equals("[]")) + return new HashSet<>(); else - return Set.of(value.split(",")); + return new LinkedHashSet<>(Arrays.asList(value.split(","))); } else { @@ -278,11 +358,16 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd "ZeroTTLOnTWCSWarned", "zero_ttl_on_twcs_warned", "FieldsPerUDTFailThreshold", "fields_per_udt_fail_threshold", "FieldsPerUDTWarnThreshold", "fields_per_udt_warn_threshold", - "FieldsPerUDTThreshold", "fields_per_udt_threshold"); + "FieldsPerUDTThreshold", "fields_per_udt_threshold", + "SimpleStrategyEnabled", "simplestrategy_enabled", + "NonPartitionRestrictedQueryEnabled", "non_partition_restricted_index_query_enabled"); - private static final Map toCamelCaseTranslationMap = Map.of("set_zero_ttl_on_twcs_enabled", "setZeroTTLOnTWCSEnabled", - "set_zero_ttl_on_twcs_warned", "setZeroTTLOnTWCSWarned", - "set_fields_per_udt_threshold", "setFieldsPerUDTThreshold"); + private static final Set ignored = Set.of("password_validator_config"); + + /** + * Set of guardrails which are flags, even though their suffix would suggest they are part of "values" which have warned, ignored, and disallowed sub-categories + */ + private static final Set specialFlags = Set.of("intersect_filtering_query_warned", "zero_ttl_on_twcs_warned"); @VisibleForTesting public enum GuardrailCategory @@ -314,12 +399,7 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd } } - /** - * Set of guardrails which are flags, even though their suffix would suggest they are part of "values" which have warned, ignored, and disallowed sub-categories - */ - private static final Set specialFlags = Set.of("intersect_filtering_query_warned", "zero_ttl_on_twcs_warned"); - - void display(NodeProbe probe, List methods, GuardrailCategory userCategory) + void display(NodeProbe probe, Map> methods, GuardrailCategory userCategory, boolean verbose) { try { @@ -328,22 +408,35 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd List values = new ArrayList<>(); List others = new ArrayList<>(); - for (Method method : methods) + for (Map.Entry> entry : methods.entrySet()) { - String guardrailName = toSnakeCase(method.getName().substring(3)); - + String key = entry.getKey(); List bucket; - if (guardrailName.endsWith("_enabled")) + if (key.endsWith("_enabled")) bucket = flags; - else if (guardrailName.endsWith("_threshold")) - bucket = thresholds; - else if (guardrailName.endsWith("_disallowed") || - guardrailName.endsWith("_ignored")) - bucket = values; - else if (guardrailName.endsWith("_warned")) + else if (key.endsWith("_threshold")) { - if (specialFlags.contains(guardrailName)) + if (!verbose) + { + addRow(thresholds, probe.getGuardrailsMBean(), entry.getValue(), entry.getKey()); + } + else + { + for (Method method : entry.getValue()) + { + String guardrailName = toSnakeCase(method.getName().substring(3)); + addRow(thresholds, probe.getGuardrailsMBean(), method, guardrailName); + } + } + continue; + } + else if (key.endsWith("_disallowed") || + key.endsWith("_ignored")) + bucket = values; + else if (key.endsWith("_warned")) + { + if (specialFlags.contains(key)) bucket = flags; else bucket = values; @@ -351,7 +444,7 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd else bucket = others; - addRow(bucket, probe.getGuardrailsMBean(), method, guardrailName); + addRow(bucket, probe.getGuardrailsMBean(), entry.getValue().get(0), key); } TableBuilder tb = new TableBuilder(); @@ -368,7 +461,7 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd } else { - if (holder.values().stream().flatMap(list -> Stream.of(list.toArray(InternalRow[]::new))).count() == 1) + if (holder.values().stream().flatMap(list -> Stream.of(list.toArray(new InternalRow[0]))).count() == 1) { for (Map.Entry> entry : holder.entrySet()) populateOne(tb, entry.getValue()); @@ -405,7 +498,14 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd bucket.add(new InternalRow(guardrailName, value)); } - abstract void addRow(List bucket, GuardrailsMBean mBean, Method method, String guardrailName) throws Throwable; + void addRow(List bucket, GuardrailsMBean mBean, Method method, String guardrailName) throws Throwable + { + List methods = new ArrayList<>(); + methods.add(method); + addRow(bucket, mBean, methods, guardrailName); + } + + abstract void addRow(List bucket, GuardrailsMBean mBean, List method, String guardrailName) throws Throwable; public static class InternalRow { @@ -417,9 +517,34 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd this.name = name; this.value = value; } + + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + InternalRow that = (InternalRow) o; + return Objects.equals(name, that.name) && Objects.equals(value, that.value); + } + + @Override + public int hashCode() + { + return Objects.hash(name, value); + } + + @Override + public String toString() + { + return "InternalRow{" + + "name='" + name + '\'' + + ", value='" + value + '\'' + + '}'; + } } - private static String toSnakeCase(String camelCase) + @VisibleForTesting + public static String toSnakeCase(String camelCase) { if (camelCase == null || camelCase.isEmpty()) return camelCase; @@ -432,33 +557,4 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd return LocalizeString.toLowerCaseLocalized(CAMEL_PATTERN.matcher(camelCase).replaceAll("$1_$2")); } } - - private static String toCamelCase(String snakeCase) - { - if (snakeCase == null || snakeCase.isEmpty()) - return snakeCase; - - String maybeCamelCase = toCamelCaseTranslationMap.get(snakeCase); - if (maybeCamelCase != null) - return maybeCamelCase; - - StringBuilder result = new StringBuilder(); - boolean toUpper = false; - - for (int i = 0; i < snakeCase.length(); i++) - { - char c = snakeCase.charAt(i); - if (c == '_') - { - toUpper = true; - } - else - { - result.append(toUpper ? LocalizeString.toUpperCaseLocalized(Character.toString(c)) : c); - toUpper = false; - } - } - - return result.toString(); - } } diff --git a/test/unit/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommandsTest.java b/test/unit/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommandsTest.java index af32d82011..60965b41d9 100644 --- a/test/unit/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommandsTest.java +++ b/test/unit/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommandsTest.java @@ -18,23 +18,29 @@ package org.apache.cassandra.tools.nodetool; -import java.util.ArrayList; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Set; import org.junit.BeforeClass; import org.junit.Test; +import org.apache.cassandra.config.Config; import org.apache.cassandra.cql3.CQLTester; +import org.apache.cassandra.db.guardrails.GuardrailsMBean; import org.apache.cassandra.tools.ToolRunner.ToolResult; -import org.apache.cassandra.tools.nodetool.GuardrailsConfigCommand.GuardrailCategory; +import org.apache.cassandra.tools.nodetool.GuardrailsConfigCommand.GetGuardrailsConfig; import static org.apache.cassandra.tools.ToolRunner.invokeNodetool; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; public class GuardrailsConfigCommandsTest extends CQLTester { @@ -52,6 +58,10 @@ public class GuardrailsConfigCommandsTest extends CQLTester getResult.asserts().success(); assertEquals(removeMultipleSpaces(ALL_GUARDRAILS_GETTER_OUTPUT), getOutput(getResult)); + ToolResult getResultVerbose = invokeNodetool("getguardrailsconfig", "--expand"); + getResultVerbose.asserts().success(); + assertEquals(removeMultipleSpaces(ALL_GUARDRAILS_GETTER_VERBOSE_OUTPUT), getOutput(getResultVerbose)); + ToolResult getFlagsResult = invokeNodetool("getguardrailsconfig", "-c", "flags"); getFlagsResult.asserts().success(); assertEquals(removeMultipleSpaces(ALL_FLAGS_GETTER_OUTPUT), getOutput(getFlagsResult)); @@ -61,7 +71,7 @@ public class GuardrailsConfigCommandsTest extends CQLTester assertEquals(removeMultipleSpaces(ALL_VALUES_GETTER_OUTPUT), getOutput(getValuesResult)); ToolResult getThresholdsResult = invokeNodetool("getguardrailsconfig", "-c", "thresholds"); - getValuesResult.asserts().success(); + getThresholdsResult.asserts().success(); assertEquals(removeMultipleSpaces(ALL_THRESHOLDS_GETTER_OUTPUT), getOutput(getThresholdsResult)); ToolResult wrongCategory = invokeNodetool("getguardrailsconfig", "-c", "nonsense"); @@ -70,84 +80,26 @@ public class GuardrailsConfigCommandsTest extends CQLTester // individual guardrail ToolResult individualResult = invokeNodetool("getguardrailsconfig", "group_by_enabled"); - getResult.asserts().success(); + individualResult.asserts().success(); assertEquals("true\n", getOutput(individualResult)); - // more than one result + // more than one guardrail ToolResult multipleResult = invokeNodetool("getguardrailsconfig", "group_by_enabled", "keyspaces_fail_threshold"); - getResult.asserts().success(); - assertEquals("group_by_enabled true\nkeyspaces_fail_threshold -1\n", getOutput(multipleResult)); + multipleResult.asserts().failure(); + assertTrue(getOutput(multipleResult).contains("Specify only one guardrail name to get the configuration of or no name to get the configuration of all of them.")); // category with individual ToolResult categoryWithIndividualResult = invokeNodetool("getguardrailsconfig", "-c", "values", "group_by_enabled"); categoryWithIndividualResult.asserts().failure(); assertTrue(categoryWithIndividualResult.getStdout().contains("Do not specify additional arguments when --category/-c is set.")); - // get config of all guardrails enumerated on command line - String[] allLines = ALL_GUARDRAILS_GETTER_OUTPUT.split("\n"); - - List argsForAllGuardrails = new ArrayList<>(); - argsForAllGuardrails.add("getguardrailsconfig"); - - for (String line : allLines) - argsForAllGuardrails.add(line.split(" ")[0]); - - ToolResult allGuardrails = invokeNodetool(argsForAllGuardrails); - allGuardrails.asserts().success(); - assertEquals(ALL_GUARDRAILS_GETTER_OUTPUT, removeMultipleSpaces(allGuardrails.getStdout())); - // set ToolResult setResultNoArgs = invokeNodetool("setguardrailsconfig"); setResultNoArgs.asserts().failure(); assertTrue(getOutput(setResultNoArgs).contains("No arguments.")); - ToolResult setResultList = invokeNodetool("setguardrailsconfig", "--list"); - setResultList.asserts().success(); - setResultList = invokeNodetool("setguardrailsconfig", "-l"); - setResultList.asserts().success(); - assertEquals(removeMultipleSpaces(ALL_GUARDRAILS_SETTER_OUTPUT), getOutput(setResultList)); - - ToolResult categoryList = invokeNodetool("setguardrailsconfig", "--list", "--category", "values"); - categoryList.asserts().success(); - - for (GuardrailCategory category : GuardrailCategory.values()) - { - categoryList = invokeNodetool("setguardrailsconfig", "-l", "-c", category.name()); - categoryList.asserts().success(); - - String expectedOutput = null; - switch (category) - { - case flags: - expectedOutput = ALL_FLAGS_SETTER_OUTPUT; - break; - case values: - expectedOutput = ALL_VALUES_SETTER_OUTPUT; - break; - case thresholds: - expectedOutput = ALL_THRESHOLDS_SETTER_OUTPUT; - break; - case others: - expectedOutput = ALL_OTHER_SETTER_OUTPUT; - break; - default: - fail("Untested category " + category); - } - assertEquals(removeMultipleSpaces(expectedOutput), getOutput(categoryList)); - } - - // test -c without -l does not make sense - ToolResult emptyCategoryListing = invokeNodetool("setguardrailsconfig", "-l", "-c"); - emptyCategoryListing.asserts().failure(); - assertTrue(getOutput(emptyCategoryListing).contains("Required values for option 'guardrailCategory' not provided")); - - // test -c alone does not make sense - ToolResult onlyCategory = invokeNodetool("setguardrailsconfig", "-c", "values"); - onlyCategory.asserts().failure(); - assertTrue(getOutput(onlyCategory).contains("--category/-c can be specified only together with --list/-l")); - - // it would be quite cumbersome to test all guardrails are settable so we will + // it would be quite cumbersome to test all guardrails are settable, so we will // set one from each category to prove the point // flag @@ -161,9 +113,13 @@ public class GuardrailsConfigCommandsTest extends CQLTester assertArrayEquals(new String[]{ "comment", "cdc" }, getValues("table_properties_warned")); setValues("table_properties_warned", "null"); assertArrayEquals(new String[0], getValues("table_properties_warned")); + setValues("table_properties_warned", "comment", "cdc"); + assertArrayEquals(new String[]{ "comment", "cdc" }, getValues("table_properties_warned")); + setValues("table_properties_warned", "[]"); + assertArrayEquals(new String[0], getValues("table_properties_warned")); // threshold - setThresholds("keyspaces_threshold", "10", "20"); + setThresholds("keyspaces_threshold", "20", "10"); assertEquals("20", getThreshold("keyspaces_fail_threshold")); assertEquals("10", getThreshold("keyspaces_warn_threshold")); setThresholds("keyspaces_threshold", "-1", "-1"); @@ -175,10 +131,13 @@ public class GuardrailsConfigCommandsTest extends CQLTester invalidNumberOfArgsForThreshold.asserts().failure(); assertTrue(invalidNumberOfArgsForThreshold.getStdout().contains("keyspaces_threshold is expecting 2 argument values. Getting 3 instead.")); - // not separated by comma - ToolResult invalidNumberOfArgsForValues = invokeNodetool("setguardrailsconfig", "table_properties_warned", "comment", "cdc"); - invalidNumberOfArgsForValues.asserts().failure(); - assertTrue(invalidNumberOfArgsForValues.getStdout().contains("table_properties_warned is expecting 1 argument values. Getting 2 instead.")); + // separated by comma + ToolResult argumentsForValuesSeparatedByComma = invokeNodetool("setguardrailsconfig", "table_properties_warned", "comment,cdc"); + argumentsForValuesSeparatedByComma.asserts().success(); + + // enumerated + ToolResult argumentsForValuesEnumerated = invokeNodetool("setguardrailsconfig", "table_properties_warned", "comment", "cdc"); + argumentsForValuesEnumerated.asserts().success(); // invalid boolean ToolResult invalidBooleanForFlags = invokeNodetool("setguardrailsconfig", "allow_filtering_enabled", "nonsense"); @@ -189,30 +148,108 @@ public class GuardrailsConfigCommandsTest extends CQLTester ToolResult nonsenseSetterArgs = invokeNodetool("setguardrailsconfig", "keyspaces_threshold", "-10", "-20"); nonsenseSetterArgs.asserts().failure(); assertTrue(nonsenseSetterArgs.getStdout().contains("Error occured when setting the config for setter keyspaces_threshold with arguments [-10, -20]: " + - "Invalid value -10 for keyspaces_warn_threshold: negative values are not allowed, outside of -1 which disables the guardrail")); + "Invalid value -20 for keyspaces_warn_threshold: negative values are not allowed, outside of -1 which disables the guardrail")); + + // invalid set name + ToolResult setInvalidName = invokeNodetool("setguardrailsconfig", "non_sense", "10"); + setInvalidName.asserts().failure(); + assertTrue(setInvalidName.getStdout().contains("Guardrail non_sense not found.")); + + // invalid get name + ToolResult getInvalidName = invokeNodetool("getguardrailsconfig", "non_sense"); + getInvalidName.asserts().failure(); + assertTrue(getInvalidName.getStdout().contains("Guardrail non_sense not found.")); } + @Test + public void testParsedGuardrailNamesFromMBeanExistInCassandraYaml() + { + Set configFieldNames = getConfigFieldNames(); + Map> snakeCaseGuardrailsMap = GetGuardrailsConfig.parseGuardrailNames(GuardrailsMBean.class.getDeclaredMethods(), null); + + for (Map.Entry> entry : snakeCaseGuardrailsMap.entrySet()) + { + for (Method method : entry.getValue()) + { + String guardrailName = GuardrailsConfigCommand.toSnakeCase(method.getName().substring(3)); + if (entry.getValue().size() == 1) + assertEquals(entry.getKey(), guardrailName); + // else it is threshold, so it does not match the key + + // assert converted snake-case guardrail name is actually in Config / cassandra.yaml + assertTrue(configFieldNames.contains(guardrailName)); + } + } + } + + private Set getConfigFieldNames() + { + Set variableNames = new HashSet<>(); + for (Field field : Config.class.getFields()) + { + // ignore the constants + if (Modifier.isFinal(field.getModifiers())) + continue; + variableNames.add(field.getName()); + } + + return variableNames; + } + + private static final String ALL_FLAGS_GETTER_OUTPUT = - "allow_filtering_enabled true \n" + - "alter_table_enabled true \n" + - "bulk_load_enabled true \n" + - "compact_tables_enabled true \n" + - "drop_keyspace_enabled true \n" + - "drop_truncate_table_enabled true \n" + - "group_by_enabled true \n" + - "intersect_filtering_query_enabled true \n" + - "intersect_filtering_query_warned true \n" + - "non_partition_restricted_query_enabled true \n" + - "read_before_write_list_operations_enabled true \n" + - "secondary_indexes_enabled true \n" + - "simple_strategy_enabled true \n" + - "uncompressed_tables_enabled true \n" + - "user_timestamps_enabled true \n" + - "vector_type_enabled true \n" + - "zero_ttl_on_twcs_enabled true \n" + - "zero_ttl_on_twcs_warned true \n"; + "allow_filtering_enabled true\n" + + "alter_table_enabled true\n" + + "bulk_load_enabled true\n" + + "compact_tables_enabled true\n" + + "drop_keyspace_enabled true\n" + + "drop_truncate_table_enabled true\n" + + "group_by_enabled true\n" + + "intersect_filtering_query_enabled true\n" + + "intersect_filtering_query_warned true\n" + + "non_partition_restricted_index_query_enabled true\n" + + "read_before_write_list_operations_enabled true\n" + + "secondary_indexes_enabled true\n" + + "simplestrategy_enabled true\n" + + "uncompressed_tables_enabled true\n" + + "user_timestamps_enabled true\n" + + "vector_type_enabled true\n" + + "zero_ttl_on_twcs_enabled true\n" + + "zero_ttl_on_twcs_warned true\n"; private static final String ALL_THRESHOLDS_GETTER_OUTPUT = + "collection_list_size_threshold [null, null] \n" + + "collection_map_size_threshold [null, null] \n" + + "collection_set_size_threshold [null, null] \n" + + "collection_size_threshold [null, null] \n" + + "column_ascii_value_size_threshold [null, null] \n" + + "column_blob_value_size_threshold [null, null] \n" + + "column_text_and_varchar_value_size_threshold [null, null] \n" + + "column_value_size_threshold [null, null] \n" + + "columns_per_table_threshold [-1, -1] \n" + + "data_disk_usage_percentage_threshold [-1, -1] \n" + + "fields_per_udt_threshold [-1, -1] \n" + + "in_select_cartesian_product_threshold [-1, -1] \n" + + "items_per_collection_threshold [-1, -1] \n" + + "keyspaces_threshold [-1, -1] \n" + + "materialized_views_per_table_threshold [-1, -1] \n" + + "maximum_replication_factor_threshold [-1, -1] \n" + + "maximum_timestamp_threshold [null, null] \n" + + "minimum_replication_factor_threshold [-1, -1] \n" + + "minimum_timestamp_threshold [null, null] \n" + + "page_size_threshold [-1, -1] \n" + + "partition_keys_in_select_threshold [-1, -1] \n" + + "partition_size_threshold [null, null] \n" + + "partition_tombstones_threshold [-1, -1] \n" + + "sai_frozen_term_size_threshold [8KiB, 1KiB] \n" + + "sai_sstable_indexes_per_query_threshold [-1, 32] \n" + + "sai_string_term_size_threshold [8KiB, 1KiB] \n" + + "sai_vector_term_size_threshold [32KiB, 16KiB]\n" + + "secondary_indexes_per_table_threshold [-1, -1] \n" + + "tables_threshold [-1, -1] \n" + + "vector_dimensions_threshold [-1, -1] \n"; + + private static final String ALL_THRESHOLDS_GETTER_VERBOSE_OUTPUT = "collection_list_size_fail_threshold null \n" + "collection_list_size_warn_threshold null \n" + "collection_map_size_fail_threshold null \n" + @@ -286,80 +323,16 @@ public class GuardrailsConfigCommandsTest extends CQLTester private static final String ALL_OTHER_GETTER_OUTPUT = "data_disk_usage_max_disk_size null \n"; - private static final String ALL_FLAGS_SETTER_OUTPUT = - "allow_filtering_enabled boolean \n" + - "alter_table_enabled boolean \n" + - "bulk_load_enabled boolean \n" + - "compact_tables_enabled boolean \n" + - "drop_keyspace_enabled boolean \n" + - "drop_truncate_table_enabled boolean \n" + - "group_by_enabled boolean \n" + - "intersect_filtering_query_enabled boolean \n" + - "intersect_filtering_query_warned boolean \n" + - "non_partition_restricted_query_enabled boolean \n" + - "read_before_write_list_operations_enabled boolean \n" + - "secondary_indexes_enabled boolean \n" + - "simple_strategy_enabled boolean \n" + - "uncompressed_tables_enabled boolean \n" + - "user_timestamps_enabled boolean \n" + - "vector_type_enabled boolean \n" + - "zero_ttl_on_twcs_enabled boolean \n" + - "zero_ttl_on_twcs_warned boolean \n"; - - private static final String ALL_THRESHOLDS_SETTER_OUTPUT = - "collection_list_size_threshold [java.lang.String, java.lang.String]\n" + - "collection_map_size_threshold [java.lang.String, java.lang.String]\n" + - "collection_set_size_threshold [java.lang.String, java.lang.String]\n" + - "collection_size_threshold [java.lang.String, java.lang.String]\n" + - "column_ascii_value_size_threshold [java.lang.String, java.lang.String]\n" + - "column_blob_value_size_threshold [java.lang.String, java.lang.String]\n" + - "column_text_and_varchar_value_size_threshold [java.lang.String, java.lang.String]\n" + - "column_value_size_threshold [java.lang.String, java.lang.String]\n" + - "columns_per_table_threshold [int, int] \n" + - "data_disk_usage_percentage_threshold [int, int] \n" + - "fields_per_udt_threshold [int, int] \n" + - "in_select_cartesian_product_threshold [int, int] \n" + - "items_per_collection_threshold [int, int] \n" + - "keyspaces_threshold [int, int] \n" + - "materialized_views_per_table_threshold [int, int] \n" + - "maximum_replication_factor_threshold [int, int] \n" + - "maximum_timestamp_threshold [java.lang.String, java.lang.String]\n" + - "minimum_replication_factor_threshold [int, int] \n" + - "minimum_timestamp_threshold [java.lang.String, java.lang.String]\n" + - "page_size_threshold [int, int] \n" + - "partition_keys_in_select_threshold [int, int] \n" + - "partition_size_threshold [java.lang.String, java.lang.String]\n" + - "partition_tombstones_threshold [long, long] \n" + - "sai_frozen_term_size_threshold [java.lang.String, java.lang.String]\n" + - "sai_sstable_indexes_per_query_threshold [int, int] \n" + - "sai_string_term_size_threshold [java.lang.String, java.lang.String]\n" + - "sai_vector_term_size_threshold [java.lang.String, java.lang.String]\n" + - "secondary_indexes_per_table_threshold [int, int] \n" + - "tables_threshold [int, int] \n" + - "vector_dimensions_threshold [int, int] \n"; - - private static final String ALL_VALUES_SETTER_OUTPUT = - "read_consistency_levels_disallowed java.util.Set \n" + - "read_consistency_levels_warned java.util.Set \n" + - "table_properties_disallowed java.util.Set \n" + - "table_properties_ignored java.util.Set \n" + - "table_properties_warned java.util.Set \n" + - "write_consistency_levels_disallowed java.util.Set \n" + - "write_consistency_levels_warned java.util.Set \n"; - - private static final String ALL_OTHER_SETTER_OUTPUT = - "data_disk_usage_max_disk_size java.lang.String \n"; - private static final String ALL_GUARDRAILS_GETTER_OUTPUT = removeMultipleSpaces(ALL_FLAGS_GETTER_OUTPUT + ALL_THRESHOLDS_GETTER_OUTPUT + ALL_VALUES_GETTER_OUTPUT + ALL_OTHER_GETTER_OUTPUT); - private static final String ALL_GUARDRAILS_SETTER_OUTPUT = removeMultipleSpaces(ALL_FLAGS_SETTER_OUTPUT + - ALL_THRESHOLDS_SETTER_OUTPUT + - ALL_VALUES_SETTER_OUTPUT + - ALL_OTHER_SETTER_OUTPUT); + private static final String ALL_GUARDRAILS_GETTER_VERBOSE_OUTPUT = removeMultipleSpaces(ALL_FLAGS_GETTER_OUTPUT + + ALL_THRESHOLDS_GETTER_VERBOSE_OUTPUT + + ALL_VALUES_GETTER_OUTPUT + + ALL_OTHER_GETTER_OUTPUT); private static String removeMultipleSpaces(String input) { @@ -376,9 +349,9 @@ public class GuardrailsConfigCommandsTest extends CQLTester return invokeNodetool("setguardrailsconfig", name, flag.toString()); } - private ToolResult setThresholds(String name, String warn, String fail) + private ToolResult setThresholds(String name, String fail, String warn) { - return invokeNodetool("setguardrailsconfig", name, warn, fail); + return invokeNodetool("setguardrailsconfig", name, fail, warn); } private ToolResult setValues(String name, String... values)