Remove SslBundles from KafkaProperties as it is no longer used

[Torres-09]

This PR removes the deprecated buildConsumerProperties(SslBundles) method from the KafkaProperties.

The removed test has been restored and updated to reflect the new logic. All tests and builds pass locally.

assertThatExceptionOfType(MutuallyExclusiveConfigurationPropertiesException.class).isThrownBy(properties::buildConsumerProperties);

gh-45722

[Torres-09]

I’d like to ask for guidance regarding the proper direction for this change.

Since buildConsumerProperties(SslBundles) has been removed, I'm deciding between two implementation options for buildConsumerProperties().

Option 1 – Minimal change (currently applied in PR):

public Map<String, Object> buildConsumerProperties() {
    Map<String, Object> properties = buildCommonProperties(null);
    properties.putAll(this.consumer.buildProperties(null));
    return properties;
}

This keeps the current method signature of buildProperties(SslBundles) and simply passes null, minimizing code changes.

Option 2 – Cleanup approach:

public Map<String, Object> buildConsumerProperties() {
    Map<String, Object> properties = buildCommonProperties(null);
    properties.putAll(this.consumer.buildProperties());
    return properties;
}

public static class Consumer {
    private final Ssl ssl = new Ssl();
    ...

    public Map<String, Object> buildProperties() {
        Properties properties = new Properties();
        PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull();
        map.from(this::getAutoCommitInterval)
            .asInt(Duration::toMillis)
            ...
            .to(properties.in(ConsumerConfig.MAX_POLL_INTERVAL_MS_CONFIG));
        return properties.with(this.ssl, this.security, this.properties, null);
    }
}

This removes the now-unnecessary parameter entirely and results in a cleaner method.

Functionally, both options appear to be equivalent.

I'd love to hear your thoughts. Thank you!

[wilkinsona]

Thanks for noticing this, @Torres-09. I think we should go for the second option. SslBundles appears in the signature of several methods in KafkaProperties but is never actually used. It looks like all references to SslBundles can be removed from KafkaProperties. This will require some small changes to its tests and a couple of configuration classes as well.

[Torres-09]

@wilkinsona

Thank you for the feedback!

I’ll proceed to remove all remaining references to SslBundles from KafkaProperties, as well as update the related classes and tests accordingly. I’ll revise the PR to reflect these changes.

[wilkinsona]

This test should be kept but should call .isThrownBy(properties::buildConsumerProperties);.

[wilkinsona]

This test should be kept but should call .isThrownBy(properties::buildConsumerProperties);.

[wilkinsona]

This test should be kept but should call .isThrownBy(properties::buildConsumerProperties);.

[wilkinsona]

This test should be kept but should call .isThrownBy(properties::buildConsumerProperties);.

[Torres-09]

@wilkinsona 👋
I accidentally marked this conversation as resolved and forgot to leave a comment — sorry about that.

Thanks for the review. I've restored the four KafkaPropertiesTests and updated them using .isThrownBy(properties::buildConsumerProperties).

All tests pass locally. Let me know if anything else needs to be adjusted!