Adding missing configs in system_views.settings to be backward compatible

patch by Sudipta Laha; reviewed by Stefan Miklosovic, Tiago L. Alves, Francisco Guerrero, Yifan Cai for CASSANDRA-20863

Author: shruti-p-s

CHANGES.txt

@@ -1,4 +1,5 @@
 5.0.7
+ * Adding missing configs in system_views.settings to be backward compatible (CASSANDRA-20863)
  * Heap dump should not be generated on handled exceptions (CASSANDRA-20974)
 Merged from 4.1:
  * ReadCommandController should close fast to avoid deadlock when building secondary index (CASSANDRA-19564)

src/java/org/apache/cassandra/db/virtual/SettingsTable.java

@@ -42,12 +42,13 @@
 import org.apache.cassandra.service.ClientWarn;
 import org.yaml.snakeyaml.introspector.Property;
 
-final class SettingsTable extends AbstractVirtualTable
+@VisibleForTesting
+public final class SettingsTable extends AbstractVirtualTable
 {
     private static final String NAME = "name";
     private static final String VALUE = "value";
 
-    private static final Map<String, String> BACKWARDS_COMPATABLE_NAMES = ImmutableMap.copyOf(getBackwardsCompatableNames());
+    public static final Map<String, String> BACKWARDS_COMPATIBLE_NAMES = ImmutableMap.copyOf(getBackwardsCompatibleNames());
     protected static final Map<String, Property> PROPERTIES = ImmutableMap.copyOf(getProperties());
 
     private final Config config;
@@ -76,8 +77,8 @@ public DataSet data(DecoratedKey partitionKey)
     {
         SimpleDataSet result = new SimpleDataSet(metadata());
         String name = UTF8Type.instance.compose(partitionKey.getKey());
-        if (BACKWARDS_COMPATABLE_NAMES.containsKey(name))
-            ClientWarn.instance.warn("key '" + name + "' is deprecated; should switch to '" + BACKWARDS_COMPATABLE_NAMES.get(name) + "'");
+        if (BACKWARDS_COMPATIBLE_NAMES.containsKey(name))
+            ClientWarn.instance.warn("key '" + name + "' is deprecated; should switch to '" + BACKWARDS_COMPATIBLE_NAMES.get(name) + "'");
         if (PROPERTIES.containsKey(name))
             result.row(name).column(VALUE, getValue(PROPERTIES.get(name)));
         return result;
@@ -163,7 +164,7 @@ private static Map<String, Property> getProperties()
                 assert conflict == null || r.oldName.equals(r.newName) : String.format("New property %s attempted to replace %s, but this property already exists", latest.getName(), conflict.getName());
             }
         }
-        for (Map.Entry<String, String> e : BACKWARDS_COMPATABLE_NAMES.entrySet())
+        for (Map.Entry<String, String> e : BACKWARDS_COMPATIBLE_NAMES.entrySet())
         {
             String oldName = e.getKey();
             if (properties.containsKey(oldName))
@@ -183,7 +184,7 @@ private static Map<String, Property> getProperties()
      * There were a handle full of properties which had custom names, names not present in the yaml, this map also
      * fixes this and returns the proper (what is accessable via yaml) names.
      */
-    private static Map<String, String> getBackwardsCompatableNames()
+    private static Map<String, String> getBackwardsCompatibleNames()
     {
         Map<String, String> names = new HashMap<>();
         // Names that dont match yaml
@@ -192,6 +193,12 @@ private static Map<String, String> getBackwardsCompatableNames()
         names.put("server_encryption_options_endpoint_verification", "server_encryption_options.require_endpoint_verification");
         names.put("server_encryption_options_legacy_ssl_storage_port", "server_encryption_options.legacy_ssl_storage_port_enabled");
         names.put("server_encryption_options_protocol", "server_encryption_options.accepted_protocols");
+        names.put("authenticator", "authenticator.class_name");
+        names.put("authorizer", "authorizer.class_name");
+        names.put("network_authorizer", "network_authorizer.class_name");
+        names.put("role_manager", "role_manager.class_name");
+        names.put("internode_authenticator", "internode_authenticator.class_name");
+
 
         // matching names
         names.put("audit_logging_options_audit_logs_dir", "audit_logging_options.audit_logs_dir");

test/unit/org/apache/cassandra/config/ConfigCompatibilityTest.java

@@ -54,6 +54,8 @@
 import org.apache.cassandra.distributed.upgrade.ConfigCompatibilityTestGenerate;
 import org.yaml.snakeyaml.introspector.Property;
 
+import static org.apache.cassandra.db.virtual.SettingsTable.BACKWARDS_COMPATIBLE_NAMES;
+
 /**
  * To create the test files used by this class, run {@link ConfigCompatibilityTestGenerate}.
  */
@@ -163,7 +165,9 @@ private void diff(String original, Set<String> ignore, Set<String> expectedError
         Map<Class<?>, Map<String, Replacement>> replacements = Replacements.getNameReplacements(type);
         Set<String> missing = new HashSet<>();
         Set<String> errors = new HashSet<>();
-        diff(loader, replacements, previous, type, "", missing, errors);
+        Map<String, String> backwardsCompatNames = BACKWARDS_COMPATIBLE_NAMES;
+
+        diff(loader, replacements, previous, type, "", missing, errors, backwardsCompatNames);
         missing = Sets.difference(missing, ignore);
         errors = Sets.difference(errors, expectedErrors);
         StringBuilder msg = new StringBuilder();
@@ -179,7 +183,7 @@ private void diff(String original, Set<String> ignore, Set<String> expectedError
             throw new AssertionError(msg);
     }
 
-    private void diff(Loader loader, Map<Class<?>, Map<String, Replacement>> replacements, ClassTree previous, Class<?> type, String prefix, Set<String> missing, Set<String> errors)
+    private void diff(Loader loader, Map<Class<?>, Map<String, Replacement>> replacements, ClassTree previous, Class<?> type, String prefix, Set<String> missing, Set<String> errors, Map<String, String> backwardsCompatNames)
     {
         Map<String, Replacement> replaces = replacements.getOrDefault(type, Collections.emptyMap());
         Map<String, Property> properties = loader.getProperties(type);
@@ -210,7 +214,7 @@ private void diff(Loader loader, Map<Class<?>, Map<String, Replacement>> replace
             if (node instanceof ClassTree)
             {
                 // current is nested type
-                diff(loader, replacements, (ClassTree) node, prop.getType(), prefix + name + ".", missing, errors);
+                diff(loader, replacements, (ClassTree) node, prop.getType(), prefix + name + ".", missing, errors, backwardsCompatNames);
             }
             else
             {
@@ -225,7 +229,19 @@ private void diff(Loader loader, Map<Class<?>, Map<String, Replacement>> replace
                     // previous is leaf, is current?
                     Map<String, Property> children = Properties.isPrimitive(prop) || Properties.isCollection(prop) ? Collections.emptyMap() : loader.getProperties(prop.getType());
                     if (!children.isEmpty())
+                    {
                         errors.add(String.format("Property %s used to be a value-type, but now is nested type %s", name, prop.getType()));
+
+                        // Verify SettingsTable maps old name to new nested path for backwards compatibility (e.g., "authenticator" -> "authenticator.class_name")
+                        if (!backwardsCompatNames.containsKey(name))
+                        {
+                            errors.add(String.format(
+                                    "Property %s changed to nested type but is missing from SettingsTable.BACKWARDS_COMPATIBLE_NAMES. " +
+                                            "Add mapping for '%s' to its new nested property path.",
+                                    name, name));
+                        }
+                    }
+
                     typeCheck(null, toString(prop.getType()), ((Leaf) node).type, name, errors);
                 }
             }

test/unit/org/apache/cassandra/db/virtual/SettingsTableTest.java

@@ -176,6 +176,16 @@ public void virtualTableBackwardCompatibility() throws Throwable
         assertRowsNet(executeNet(q), new Object[] {"credentials_update_interval", null});
         q = "SELECT * FROM vts.settings WHERE name = 'credentials_update_interval_in_ms';";
         assertRowsNet(executeNet(q), new Object[] {"credentials_update_interval_in_ms", "-1"});
+
+        // test non matching auth related properties
+        q = "SELECT * FROM vts.settings WHERE name = 'authenticator';";
+        assertRowsNet(executeNet(q), new Object[] {"authenticator", null});
+        q = "SELECT * FROM vts.settings WHERE name = 'authorizer';";
+        assertRowsNet(executeNet(q), new Object[] {"authorizer", null});
+        q = "SELECT * FROM vts.settings WHERE name = 'network_authorizer';";
+        assertRowsNet(executeNet(q), new Object[] {"network_authorizer", null});
+        q= "select * from vts.settings where name = 'role_manager';";
+        assertRowsNet(executeNet(q), new Object[] {"role_manager", null});
     }
 
     private void check(String keyspaceTable, String setting, String expected)