Before KIP-651 Kafka only supported file-based key and trust stores for SSL. After KIP-651 it's now also possible to use PEM, which can be passed as environment variables rather than files. This frees us from having to write files to the Docker images that run our application, and enables us to use build pack images.
Not sure if I've covered all corners in this initial implementation; please let me know if you would like me to make any changes. Tests had not covered Kafka SSL configuration before, and since we're merely passing values I'd not thought to add those now. Had also thought these changes are not notable enough to include my name as author at the top of the file, but not sure if it would be breaking convention to leave it out.
One thing to note is that the referenced properties were only added in Spring Kafka 2.7, so any users that both downgrade to Kafka 2.6 and use SSL might now pass unknown properties to Kafka, leading to a warning. (Provided the org.apache.kafka.common.config.SslConfigs constants are inlined by the compiler.)
Comment From: timtebeek
@garyrussell would you mind taking a look at this change? Thought it might help people switch from JKS to PEM, with the latter passed in through environment variables rather than files.
Comment From: timtebeek
We could also do with some tests for the new support. That would help us to make an informed decision around what to do when the type is and is not set and when it's set to a contradictory value. -- @wilkinsona
I'm struggling a bit with what type of test to add, as there's no conditionals yet in for instance KafkaProperties.Ssl.buildProperties().
You're right to point out that the new properties can only be used when a corresponding value of spring.kafka.ssl.key-store-type=PEM and/or spring.kafka.ssl.trust-store-type=PEM is provided, as the default is JKS.
For now all properties are passed as is to the relevant KafkaAdmin / KafkaConsumerFactory / KafkaProducerFactory / StreamsBuilderFactoryBean, without additional validation of what combination of properties is set.
So these properties would work
spring.kafka.ssl.key-store-type=PEM
spring.kafka.ssl.key-store-key=-----BEGIN
spring.kafka.ssl.key-store-certificate-chain=-----BEGIN
spring.kafka.ssl.trust-store-type=PEM
spring.kafka.ssl.trust-store-certificates=-----BEGIN
As would these properties, as taken from existing: @Test KafkaAutoConfigurationTests.consumerProperties()
spring.kafka.ssl.key-store-location=classpath:ksLoc
spring.kafka.ssl.key-store-password=p2
spring.kafka.ssl.key-store-type=PKCS12 // Overrides Kafka default of JKS as per SslConfigs
spring.kafka.ssl.trust-store-location=classpath:tsLoc
spring.kafka.ssl.trust-store-password=p3
spring.kafka.ssl.trust-store-type=PKCS12 // Overrides Kafka default of JKS as per SslConfigs
But at present invalid combinations of store-type and properties could also be passed, with interpretation and validation up to Apache Kafka itself.
Would you like me to add conditionals that guarantee that the new properties are only passed with the proper corresponding key type?
And if so, would KafkaProperties.Ssl.buildProperties() be the best place for that?
Otherwise it becomes hard to write tests that will actually break when envisioned/needed.
Comment From: timtebeek
After thinking it over some more I'm still somewhat hesitant in adding validation that mimics the validation done by Kafka; since we'd always be trying to keep up with any validation changes there.
Perhaps we could as a minimum require that key-store-type and trust-store-type are non-null when the corresponding PEM properties are passed? That should decouple us from hardcoding Kafka knowledge, allow for future changes, while still preventing potentially common misconfiguration.
Comment From: wilkinsona
I was more thinking that writing some additional tests that use the new properties would help us to learn about how Kafka behaves. From what you've described above, @timtebeek, it sounds like you already knew or have already learned that.
We recently added support for reporting an error when configuration properties that are mutually exclusive are used together. I wonder if that may be useful here as I suspect it doesn't make sense to configure key-store-location and key-store-certificate-chain at the same time, particularly if Kafka would just ignore one in favour of the other.
I'll flag this for team attention to see what everyone else thinks. I suspect we may have to do some experimentation to get a feel for how things work and what the best thing to do will be.
Comment From: timtebeek
Ah perfect thanks that MutuallyExclusiveConfigurationPropertiesException.throwIfMultipleNonNullValuesIn does indeed sound applicable here; I'll try to add that after work! :)
Comment From: mbhave
Thanks for contributing to Spring Boot, @timtebeek. This has been merged into main along with some tests.
Comment From: timtebeek
Thanks a lot, both for the added tests as well as the final review & merge!