Skip to content

Remove buildConsumerProperties(SslBundles) from KafkaProperties #45722

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Renboo opened this issue May 29, 2025 · 6 comments
Closed

Remove buildConsumerProperties(SslBundles) from KafkaProperties #45722

Renboo opened this issue May 29, 2025 · 6 comments
Assignees
Labels
status: superseded An issue that has been superseded by another type: task A general task

Comments

@Renboo
Copy link

Renboo commented May 29, 2025

Spring boot 3.5.0

I found that KafkaProperties.java lost this part:

private Map<String, Object> buildPropertiesForSslBundle(SslBundles sslBundles, String name) {
	Properties properties = new Properties();
	properties.in(SslConfigs.SSL_ENGINE_FACTORY_CLASS_CONFIG).accept(SslBundleSslEngineFactory.class);
	properties.in(SslBundle.class.getName()).accept(sslBundles.getBundle(name));
	return properties;
}

And after this logged consumer config values will contain
ssl.engine.factory.class = null
and SSL Handshake exception.

In previous 3.4.x all work greate.

To understand the whole story I use buildConsumerProperties(sslBundles) method to create Kafkfa config.

Is it bug or feature?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 29, 2025
@wilkinsona
Copy link
Member

The first thing to note here is that the methods on Spring Boot's @ConfigurationProperties classes are not considered to be public API:

The properties that map to @ConfigurationProperties classes available in Spring Boot, which are configured through properties files, YAML files, environment variables, and other mechanisms, are public API but the accessors (getters/setters) of the class itself are not meant to be used directly.

As you're using methods on KafkaProperties, you are being affected by the change made in dae891f. Applying the properties for the SSL bundle is now done in KafkaAutoConfiguration. You will have to adapt your own code to match.

@wilkinsona wilkinsona closed this as not planned Won't fix, can't repro, duplicate, stale May 29, 2025
@wilkinsona wilkinsona added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged labels May 29, 2025
@Renboo
Copy link
Author

Renboo commented May 29, 2025

@wilkinsona thank you!

But to be honest buildConsumerProperties(sslBundles) should be deleted from KafkaProperties, am i right?
It is useless without
properties.in(SslConfigs.SSL_ENGINE_FACTORY_CLASS_CONFIG).accept(SslBundleSslEngineFactory.class);

@wilkinsona
Copy link
Member

Yes, I think you're right. We can use this issue to do that.

@wilkinsona wilkinsona reopened this May 29, 2025
@wilkinsona wilkinsona changed the title KafkaProperties lost buildPropertiesForSslBundle Remove buildConsumerProperties(SslBundles) from KafkaProperties May 29, 2025
@wilkinsona wilkinsona added type: task A general task and removed status: invalid An issue that we don't feel is valid labels May 29, 2025
@wilkinsona wilkinsona added this to the 4.0.x milestone May 29, 2025
@Torres-09
Copy link

Hi, I’d like to work on this issue. Let me know if it's okay to proceed.

@wilkinsona
Copy link
Member

It's all yours, @Torres-09. Thanks!

@wilkinsona
Copy link
Member

Closing in favor of #45727. Thanks for the PR, @Torres-09.

@wilkinsona wilkinsona closed this as not planned Won't fix, can't repro, duplicate, stale May 30, 2025
@wilkinsona wilkinsona added the status: superseded An issue that has been superseded by another label May 30, 2025
@wilkinsona wilkinsona removed this from the 4.0.x milestone May 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another type: task A general task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants