dclim commented on a change in pull request #6285: Securing passwords used for 
SSL connections to Kafka
URL: https://github.com/apache/incubator-druid/pull/6285#discussion_r224192478
 
 

 ##########
 File path: 
extensions-core/kafka-indexing-service/src/test/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisorIOConfigTest.java
 ##########
 @@ -119,6 +122,42 @@ public void testSerdeWithNonDefaults() throws Exception
     Assert.assertTrue("skipOffsetGaps", config.isSkipOffsetGaps());
   }
 
+  @Test
+  public void testSerdeForConsumerPropertiesWithPasswords() throws Exception
 
 Review comment:
   I don't think this is actually testing what you want to be testing. It is 
basically testing that string entries in `consumerProperties` get serde'd 
correctly and (somewhat separately) that DefaultPasswordProvider works as 
intended.
   
   What you might want to test is that a) `KafkaSupervisorIOConfig` gets 
deserialized properly when there is a `consumerProperties` entry that is _not_ 
a string (i.e. that is a map representing a `PasswordProvider` object) and b) 
that `KafkaIndexTask.addConsumerPropertiesFromConfig()` does what you expect it 
to do (i.e. ultimately converts this into a string that it can pass to the 
KafkaConsumer). Something along these lines:
   
   ```
       String jsonStr = "{\n"
                        + "  \"type\": \"kafka\",\n"
                        + "  \"topic\": \"my-topic\",\n"
                        + "  \"consumerProperties\": {\n"
                        + "    \"bootstrap.servers\": \"localhost:9092\",\n"
                        + "    \"ssl.truststore.password\": {\"type\": 
\"default\", \"password\": \"mytruststorepassword\"},\n"
                        + "    \"ssl.keystore.password\": {\"type\": 
\"default\", \"password\": \"mykeystorepassword\"},\n"
                        + "    \"ssl.key.password\": \"mykeypassword\"\n"
                        + "  }\n"
                        + "}";
   
       KafkaSupervisorIOConfig ioConfig = mapper.readValue(jsonStr, 
KafkaSupervisorIOConfig.class);
   
       Properties props = new Properties();
       KafkaIndexTask.addConsumerPropertiesFromConfig(props, mapper, 
ioConfig.getConsumerProperties());
   ```
   (and then run your assertions). Bonus if you can test a PasswordProvider 
that is not the default one (I understand the 
EnvironmentVariablePasswordProvider is somewhat hard to test without some 
serious hacks).

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to