You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2022/07/09 00:12:32 UTC

[GitHub] [cassandra-sidecar] frankgh opened a new pull request, #32: CASSANDRAC-39: Allow Cassandra input validation to be configurable

frankgh opened a new pull request, #32:
URL: https://github.com/apache/cassandra-sidecar/pull/32

   It is sometimes desirable to allow configuring the Cassandra input validation in case
   we want to further restrict the existing validations. In this commit, we introduce the
   ability to inject the ValidationConfiguration. A default YamlValidationConfiguration is
   provided and the sidecar.yaml is updated to reflect the existing default validation.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


[GitHub] [cassandra-sidecar] frankgh commented on a diff in pull request #32: CASSANDRAC-39: Allow Cassandra input validation to be configurable

Posted by GitBox <gi...@apache.org>.
frankgh commented on code in PR #32:
URL: https://github.com/apache/cassandra-sidecar/pull/32#discussion_r918384102


##########
src/main/java/org/apache/cassandra/sidecar/MainModule.java:
##########
@@ -166,90 +198,84 @@ public Router vertxRouter(Vertx vertx,
 
     @Provides
     @Singleton
-    public Configuration configuration(InstancesConfig instancesConfig) throws ConfigurationException, IOException
+    public Configuration configuration(YAMLConfiguration yamlConf, InstancesConfig instancesConfig,
+                                       ValidationConfiguration validationConfiguration)
+    throws IOException
     {
-        final String confPath = System.getProperty("sidecar.config", "file://./conf/config.yaml");
-        logger.info("Reading configuration from {}", confPath);
-        try
-        {
-            URL url = new URL(confPath);
-
-            YAMLConfiguration yamlConf = new YAMLConfiguration();
-            InputStream stream = url.openStream();
-            yamlConf.read(stream);
-
-            return new Configuration.Builder()
-                    .setInstancesConfig(instancesConfig)
-                    .setHost(yamlConf.get(String.class, YAMLKeyConstants.HOST))
-                    .setPort(yamlConf.get(Integer.class, YAMLKeyConstants.PORT))
-                    .setHealthCheckFrequency(yamlConf.get(Integer.class, YAMLKeyConstants.HEALTH_CHECK_FREQ))
-                    .setKeyStorePath(yamlConf.get(String.class, YAMLKeyConstants.KEYSTORE_PATH, null))
-                    .setKeyStorePassword(yamlConf.get(String.class, YAMLKeyConstants.KEYSTORE_PASSWORD, null))
-                    .setTrustStorePath(yamlConf.get(String.class, YAMLKeyConstants.TRUSTSTORE_PATH, null))
-                    .setTrustStorePassword(yamlConf.get(String.class, YAMLKeyConstants.TRUSTSTORE_PASSWORD, null))
-                    .setSslEnabled(yamlConf.get(Boolean.class, YAMLKeyConstants.SSL_ENABLED, false))
-                    .setRateLimitStreamRequestsPerSecond(yamlConf.getLong(YAMLKeyConstants.STREAM_REQUESTS_PER_SEC))
-                    .setThrottleTimeoutInSeconds(yamlConf.getLong(YAMLKeyConstants.THROTTLE_TIMEOUT_SEC))
-                    .setThrottleDelayInSeconds(yamlConf.getLong(YAMLKeyConstants.THROTTLE_DELAY_SEC))
-                    .build();
-        }
-        catch (MalformedURLException e)
-        {
-            throw new ConfigurationException("Failed reading from sidebar.config path: " + confPath, e);
-        }
+        return new Configuration.Builder()
+               .setInstancesConfig(instancesConfig)
+               .setHost(yamlConf.get(String.class, HOST))
+               .setPort(yamlConf.get(Integer.class, PORT))
+               .setHealthCheckFrequency(yamlConf.get(Integer.class, HEALTH_CHECK_FREQ))
+               .setKeyStorePath(yamlConf.get(String.class, KEYSTORE_PATH, null))
+               .setKeyStorePassword(yamlConf.get(String.class, KEYSTORE_PASSWORD, null))
+               .setTrustStorePath(yamlConf.get(String.class, TRUSTSTORE_PATH, null))
+               .setTrustStorePassword(yamlConf.get(String.class, TRUSTSTORE_PASSWORD, null))
+               .setSslEnabled(yamlConf.get(Boolean.class, SSL_ENABLED, false))
+               .setRateLimitStreamRequestsPerSecond(yamlConf.getLong(STREAM_REQUESTS_PER_SEC))
+               .setThrottleTimeoutInSeconds(yamlConf.getLong(THROTTLE_TIMEOUT_SEC))
+               .setThrottleDelayInSeconds(yamlConf.getLong(THROTTLE_DELAY_SEC))
+               .setValidationConfiguration(validationConfiguration)
+               .build();
     }
 
     @Provides
     @Singleton
-    public InstancesConfig getInstancesConfig(CassandraVersionProvider versionProvider)
-        throws ConfigurationException, IOException
+    public InstancesConfig getInstancesConfig(YAMLConfiguration yamlConf, CassandraVersionProvider versionProvider)
     {
-        final String confPath = System.getProperty("sidecar.config", "file://./conf/config.yaml");
-        URL url = new URL(confPath);
+        return readInstancesConfig(yamlConf, versionProvider);
+    }
 
-        try
-        {
-            YAMLConfiguration yamlConf = new YAMLConfiguration();
-            InputStream stream = url.openStream();
-            yamlConf.read(stream);
-            return readInstancesConfig(yamlConf, versionProvider);
-        }
-        catch (MalformedURLException e)
-        {
-            throw new ConfigurationException(String.format("Unable to parse cluster information from '%s'", url));
-        }
+    @Provides
+    @Singleton
+    public ValidationConfiguration validationConfiguration(YAMLConfiguration yamlConf)
+    {
+        org.apache.commons.configuration2.Configuration validationConfiguration
+        = yamlConf.subset(CASSANDRA_INPUT_VALIDATION);
+        Set<String> forbiddenKeyspaces
+        = new HashSet<>(validationConfiguration.getCollection(String.class, CASSANDRA_FORBIDDEN_KEYSPACES, null));
+        String allowedPatternForDirectory
+        = validationConfiguration.get(String.class,
+                                                                        CASSANDRA_ALLOWED_CHARS_FOR_DIRECTORY);
+        String allowedPatternForComponentName
+        = validationConfiguration.get(String.class, CASSANDRA_ALLOWED_CHARS_FOR_COMPONENT_NAME);
+        String allowedPatternForRestrictedComponentName
+        = validationConfiguration.get(String.class, CASSANDRA_ALLOWED_CHARS_FOR_RESTRICTED_COMPONENT_NAME);

Review Comment:
   Thanks for the feedback, `var` is not available in Java 8. I have pushed a new commit



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


[GitHub] [cassandra-sidecar] frankgh commented on pull request #32: CASSANDRAC-39: Allow Cassandra input validation to be configurable

Posted by GitBox <gi...@apache.org>.
frankgh commented on PR #32:
URL: https://github.com/apache/cassandra-sidecar/pull/32#issuecomment-1182276003

   Closed via https://github.com/apache/cassandra-sidecar/commit/a7a2c29e990acb2363eb7a15cc4b970ebdc04753


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


[GitHub] [cassandra-sidecar] yifan-c commented on a diff in pull request #32: CASSANDRAC-39: Allow Cassandra input validation to be configurable

Posted by GitBox <gi...@apache.org>.
yifan-c commented on code in PR #32:
URL: https://github.com/apache/cassandra-sidecar/pull/32#discussion_r918366623


##########
src/main/java/org/apache/cassandra/sidecar/MainModule.java:
##########
@@ -166,90 +198,84 @@ public Router vertxRouter(Vertx vertx,
 
     @Provides
     @Singleton
-    public Configuration configuration(InstancesConfig instancesConfig) throws ConfigurationException, IOException
+    public Configuration configuration(YAMLConfiguration yamlConf, InstancesConfig instancesConfig,
+                                       ValidationConfiguration validationConfiguration)
+    throws IOException
     {
-        final String confPath = System.getProperty("sidecar.config", "file://./conf/config.yaml");
-        logger.info("Reading configuration from {}", confPath);
-        try
-        {
-            URL url = new URL(confPath);
-
-            YAMLConfiguration yamlConf = new YAMLConfiguration();
-            InputStream stream = url.openStream();
-            yamlConf.read(stream);
-
-            return new Configuration.Builder()
-                    .setInstancesConfig(instancesConfig)
-                    .setHost(yamlConf.get(String.class, YAMLKeyConstants.HOST))
-                    .setPort(yamlConf.get(Integer.class, YAMLKeyConstants.PORT))
-                    .setHealthCheckFrequency(yamlConf.get(Integer.class, YAMLKeyConstants.HEALTH_CHECK_FREQ))
-                    .setKeyStorePath(yamlConf.get(String.class, YAMLKeyConstants.KEYSTORE_PATH, null))
-                    .setKeyStorePassword(yamlConf.get(String.class, YAMLKeyConstants.KEYSTORE_PASSWORD, null))
-                    .setTrustStorePath(yamlConf.get(String.class, YAMLKeyConstants.TRUSTSTORE_PATH, null))
-                    .setTrustStorePassword(yamlConf.get(String.class, YAMLKeyConstants.TRUSTSTORE_PASSWORD, null))
-                    .setSslEnabled(yamlConf.get(Boolean.class, YAMLKeyConstants.SSL_ENABLED, false))
-                    .setRateLimitStreamRequestsPerSecond(yamlConf.getLong(YAMLKeyConstants.STREAM_REQUESTS_PER_SEC))
-                    .setThrottleTimeoutInSeconds(yamlConf.getLong(YAMLKeyConstants.THROTTLE_TIMEOUT_SEC))
-                    .setThrottleDelayInSeconds(yamlConf.getLong(YAMLKeyConstants.THROTTLE_DELAY_SEC))
-                    .build();
-        }
-        catch (MalformedURLException e)
-        {
-            throw new ConfigurationException("Failed reading from sidebar.config path: " + confPath, e);
-        }
+        return new Configuration.Builder()
+               .setInstancesConfig(instancesConfig)
+               .setHost(yamlConf.get(String.class, HOST))
+               .setPort(yamlConf.get(Integer.class, PORT))
+               .setHealthCheckFrequency(yamlConf.get(Integer.class, HEALTH_CHECK_FREQ))
+               .setKeyStorePath(yamlConf.get(String.class, KEYSTORE_PATH, null))
+               .setKeyStorePassword(yamlConf.get(String.class, KEYSTORE_PASSWORD, null))
+               .setTrustStorePath(yamlConf.get(String.class, TRUSTSTORE_PATH, null))
+               .setTrustStorePassword(yamlConf.get(String.class, TRUSTSTORE_PASSWORD, null))
+               .setSslEnabled(yamlConf.get(Boolean.class, SSL_ENABLED, false))
+               .setRateLimitStreamRequestsPerSecond(yamlConf.getLong(STREAM_REQUESTS_PER_SEC))
+               .setThrottleTimeoutInSeconds(yamlConf.getLong(THROTTLE_TIMEOUT_SEC))
+               .setThrottleDelayInSeconds(yamlConf.getLong(THROTTLE_DELAY_SEC))
+               .setValidationConfiguration(validationConfiguration)
+               .build();
     }
 
     @Provides
     @Singleton
-    public InstancesConfig getInstancesConfig(CassandraVersionProvider versionProvider)
-        throws ConfigurationException, IOException
+    public InstancesConfig getInstancesConfig(YAMLConfiguration yamlConf, CassandraVersionProvider versionProvider)
     {
-        final String confPath = System.getProperty("sidecar.config", "file://./conf/config.yaml");
-        URL url = new URL(confPath);
+        return readInstancesConfig(yamlConf, versionProvider);
+    }
 
-        try
-        {
-            YAMLConfiguration yamlConf = new YAMLConfiguration();
-            InputStream stream = url.openStream();
-            yamlConf.read(stream);
-            return readInstancesConfig(yamlConf, versionProvider);
-        }
-        catch (MalformedURLException e)
-        {
-            throw new ConfigurationException(String.format("Unable to parse cluster information from '%s'", url));
-        }
+    @Provides
+    @Singleton
+    public ValidationConfiguration validationConfiguration(YAMLConfiguration yamlConf)
+    {
+        org.apache.commons.configuration2.Configuration validationConfiguration
+        = yamlConf.subset(CASSANDRA_INPUT_VALIDATION);
+        Set<String> forbiddenKeyspaces
+        = new HashSet<>(validationConfiguration.getCollection(String.class, CASSANDRA_FORBIDDEN_KEYSPACES, null));
+        String allowedPatternForDirectory
+        = validationConfiguration.get(String.class,
+                                                                        CASSANDRA_ALLOWED_CHARS_FOR_DIRECTORY);
+        String allowedPatternForComponentName
+        = validationConfiguration.get(String.class, CASSANDRA_ALLOWED_CHARS_FOR_COMPONENT_NAME);
+        String allowedPatternForRestrictedComponentName
+        = validationConfiguration.get(String.class, CASSANDRA_ALLOWED_CHARS_FOR_RESTRICTED_COMPONENT_NAME);

Review Comment:
   The format could be improved... 
   And `new HashSet<>(validationConfiguration.getCollection(String.class, CASSANDRA_FORBIDDEN_KEYSPACES, null)` will throw NPE if the forbidden keyspaces is not defined. The collection returned with default value `null` (not the `null` passed in the line. Then hash set new with `null` throws. 
   
   Proposed change
   ```suggestion
           var validation = yamlConf.subset(CASSANDRA_INPUT_VALIDATION);
           Set<String> forbiddenKeyspaces = new HashSet<>(validation.getList(String.class,
                                                                             CASSANDRA_FORBIDDEN_KEYSPACES,
                                                                             Collections.emptyList()));
           UnaryOperator<String> readString = key -> validation.get(String.class, key);
           String allowedPatternForDirectory = readString.apply(CASSANDRA_ALLOWED_CHARS_FOR_DIRECTORY);
           String allowedPatternForComponentName = readString.apply(CASSANDRA_ALLOWED_CHARS_FOR_COMPONENT_NAME);
           String allowedPatternForRestrictedComponentName = readString.apply(
               CASSANDRA_ALLOWED_CHARS_FOR_RESTRICTED_COMPONENT_NAME);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


[GitHub] [cassandra-sidecar] frankgh closed pull request #32: CASSANDRAC-39: Allow Cassandra input validation to be configurable

Posted by GitBox <gi...@apache.org>.
frankgh closed pull request #32: CASSANDRAC-39: Allow Cassandra input validation to be configurable
URL: https://github.com/apache/cassandra-sidecar/pull/32


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org