fix: per-topic Kafka builder methods drop SASL_SSL credentials from ConsumerConfig/ProducerConfig#3344
Merged
jeremydmiller merged 1 commit intoJul 8, 2026
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a Kafka transport configuration inheritance bug where per-topic builder methods (that create sparse ConsumerConfig/ProducerConfig instances) could unintentionally drop transport-level SASL/TLS-related settings, leading to failed authentication/handshake against secured brokers.
Changes:
- Backfill
SecurityProtocol,SaslMechanism,SaslUsername, andSaslPasswordfrom the parent transport into topic-levelConsumerConfig/ProducerConfigwhen those values are unset. - Add unit tests covering SASL_SSL inheritance behavior for the affected listener and subscriber builder methods.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/Transports/Kafka/Wolverine.Kafka/KafkaTopic.cs |
Extends effective config inheritance to include SASL/security properties for both consumer and producer configs. |
src/Transports/Kafka/Wolverine.Kafka.Tests/KafkaTransportTests.cs |
Adds tests verifying consumer-side SASL_SSL inheritance across multiple per-topic builder methods. |
src/Transports/Kafka/Wolverine.Kafka.Tests/kafka_eos_building_blocks.cs |
Adds a test verifying producer-side SASL_SSL inheritance when using UseIdempotentProducer(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+141
to
+142
| /// Gets the effective ConsumerConfig for this topic, ensuring BootstrapServers, GroupId, and the | ||
| /// SASL_SSL security settings are inherited from parent if not set |
Comment on lines
+180
to
+181
| /// Gets the effective ProducerConfig for this topic, ensuring BootstrapServers and the SASL_SSL | ||
| /// security settings are inherited from parent if not set |
Comment on lines
+156
to
+164
| if (ConsumerConfig != null && ConsumerConfig.SecurityProtocol == null) | ||
| { | ||
| ConsumerConfig.SecurityProtocol = Parent.ConsumerConfig.SecurityProtocol; | ||
| } | ||
|
|
||
| if (ConsumerConfig != null && ConsumerConfig.SaslMechanism == null) | ||
| { | ||
| ConsumerConfig.SaslMechanism = Parent.ConsumerConfig.SaslMechanism; | ||
| } |
…onsumerConfig/ProducerConfig GetEffectiveConsumerConfig() already inherited BootstrapServers and GroupId from the parent transport config (the latter fixed in JasperFx#3174), but not SecurityProtocol/SaslMechanism/SaslUsername/ SaslPassword. BeginAtEarliest()/BeginAtLatest()/UseReadCommitted()/UseCooperativeStickyAssignment() (listener-level)/UseStaticMembership()/TailFromLatest() all build a fresh per-topic ConsumerConfig containing only the one property each sets. A listener using any of these without a trailing ExtendConsumerConfiguration() call therefore connects to a SASL_SSL broker without credentials and is disconnected during the initial handshake (APIVERSION_QUERY) -- a real production incident, not a theoretical gap. The identical pattern exists on the producer side: UseIdempotentProducer() builds a fresh ProducerConfig containing only EnableIdempotence, and GetEffectiveProducerConfig() only backfilled BootstrapServers. Add SecurityProtocol/SaslMechanism/SaslUsername/SaslPassword backfill to both GetEffectiveConsumerConfig() and GetEffectiveProducerConfig(), using the same pattern already used for BootstrapServers/GroupId. Add tests covering every affected builder method on both the consumer and producer side.
lahma
force-pushed
the
kafka-consumer-config-sasl-inheritance
branch
from
July 8, 2026 18:24
7ea165e to
9a7b8de
Compare
This was referenced Jul 9, 2026
Merged
This was referenced Jul 14, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
GetEffectiveConsumerConfig()already inheritedBootstrapServersandGroupIdfrom the parent transport config (the latter fixed in #3174), but notSecurityProtocol/SaslMechanism/SaslUsername/SaslPassword.BeginAtEarliest()/BeginAtLatest()/UseReadCommitted()/UseCooperativeStickyAssignment()(listener-level)/UseStaticMembership()/TailFromLatest()all build a fresh per-topicConsumerConfigcontaining only the one property each sets (e.g.topic.ConsumerConfig ??= new ConsumerConfig(); topic.ConsumerConfig.AutoOffsetReset = ...). A listener that uses any of these without a trailingExtendConsumerConfiguration(...)call therefore ends up with aConsumerConfigmissing the transport's security settings entirely.This is a real production incident, not a theoretical gap. Against a SASL_SSL-secured broker (Confluent Cloud, in our case), the resulting client silently attempts a plaintext connection. The broker can't parse the plaintext bytes as TLS and closes the connection during the very first, pre-auth step (
APIVERSION_QUERY) — logged as aLocal_AllBrokersDown/"1/1 brokers are down"cycle that repeats indefinitely since the bootstrap connection never progresses past the handshake. This ran undetected in our deployment for about two weeks: outbound publishing was unaffected (producers use the fully-configured parentProducerConfig), the admin-client-based health check uses a separate, fully-explicit config path, and downstream consumers just kept serving stale data instead of erroring.The identical pattern exists on the producer side:
UseIdempotentProducer()builds a freshProducerConfigcontaining onlyEnableIdempotence, andGetEffectiveProducerConfig()only backfilledBootstrapServers.Fix
Add
SecurityProtocol/SaslMechanism/SaslUsername/SaslPasswordbackfill to bothGetEffectiveConsumerConfig()andGetEffectiveProducerConfig(), using the exact same pattern already used forBootstrapServers/GroupId(only backfill when the topic-level value is unset, so explicit per-topic overrides still win).Tests
Added coverage for every affected builder method on both the consumer and producer side, following the existing
begin_at_earliest_inherits_group_id_from_parent_transport-style pattern inKafkaTransportTests.cs, plus a producer-side case inkafka_eos_building_blocks.csalongside the existingsubscriber_idempotent_producertest.Test plan
dotnet testonWolverine.Kafka.Tests(net10.0) — all tests pass, including the new SASL_SSL inheritance casesAPIVERSION_QUERY/Local_AllBrokersDownfailure signature with the bug present, confirmed clean connection with the fix