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]