Skip to content

CAMEL-11682: Add sasl.jaas.config setting#1897

Closed
snowch wants to merge 1 commit intoapache:masterfrom
snowch:CAMEL-11682
Closed

CAMEL-11682: Add sasl.jaas.config setting#1897
snowch wants to merge 1 commit intoapache:masterfrom
snowch:CAMEL-11682

Conversation

@snowch
Copy link
Contributor

@snowch snowch commented Aug 22, 2017

I have tested the PR against IBM MessageHub and it worked with the following routes:

final String brokers = "kafka04-prod01.messagehub.services.eu-de.bluemix.net:9093,kafka05-prod01.messagehub.services.eu-de.bluemix.net:9093";

final String saslJaasConfig = "org.apache.kafka.common.security.plain.PlainLoginModule required username=\"USERNAME\" password=\"PASSWORD\";";
					
from("direct:in")
.to("kafka:test?" 
    + "brokers=" + brokers
    + "&saslMechanism=PLAIN"  
    + "&securityProtocol=SASL_SSL"
    + "&sslProtocol=TLSv1.2"
    + "&sslEnabledProtocols=TLSv1.2" 
    + "&sslEndpointAlgorithm=HTTPS"
    + "&saslJaasConfig=" + saslJaasConfig );

and KafkaConsumer

final String brokers = "kafka04-prod01.messagehub.services.eu-de.bluemix.net:9093,kafka05-prod01.messagehub.services.eu-de.bluemix.net:9093";

final String saslJaasConfig = "org.apache.kafka.common.security.plain.PlainLoginModule required username=\"USERNAME\" password=\"PASSWORD\";";
					
from("kafka:test?" 
    + "brokers=" + brokers
    + "&saslMechanism=PLAIN"  
    + "&securityProtocol=SASL_SSL"
    + "&sslProtocol=TLSv1.2"
    + "&sslEnabledProtocols=TLSv1.2" 
    + "&sslEndpointAlgorithm=HTTPS"
    + "&saslJaasConfig=" + saslJaasConfig
    + "&groupId=mygroup")
.to("stream:out");

The camel-kafka tests were failing on master before I made my changes. The failures were:

[ERROR] Failures:
[ERROR] org.apache.camel.component.kafka.KafkaConsumerOffsetRepositoryEmptyTest.shouldStartFromBeginningWithEmptyOffsetRepository(org.apache.camel.component.kafka.KafkaConsumerOffsetRepositoryEmptyTest)
[ERROR]   Run 1: KafkaConsumerOffsetRepositoryEmptyTest.shouldStartFromBeginningWithEmptyOffsetRepository:74 mock://result Received message count. Expected: <10> but was: <0>
[ERROR]   Run 2: KafkaConsumerOffsetRepositoryEmptyTest.shouldStartFromBeginningWithEmptyOffsetRepository:74 mock://result Received message count. Expected: <10> but was: <0>
[ERROR]   Run 3: KafkaConsumerOffsetRepositoryEmptyTest.shouldStartFromBeginningWithEmptyOffsetRepository:74 mock://result Received message count. Expected: <10> but was: <0>
[INFO]
[ERROR] org.apache.camel.component.kafka.KafkaConsumerOffsetRepositoryResumeTest.shouldResumeFromAnyParticularOffset(org.apache.camel.component.kafka.KafkaConsumerOffsetRepositoryResumeTest)
[ERROR]   Run 1: KafkaConsumerOffsetRepositoryResumeTest.shouldResumeFromAnyParticularOffset:75 mock://result Received message count. Expected: <3> but was: <0>
[ERROR]   Run 2: KafkaConsumerOffsetRepositoryResumeTest.shouldResumeFromAnyParticularOffset:75 mock://result Received message count. Expected: <3> but was: <0>
[ERROR]   Run 3: KafkaConsumerOffsetRepositoryResumeTest.shouldResumeFromAnyParticularOffset:75 mock://result Received message count. Expected: <3> but was: <0>
[INFO]
[INFO]
[ERROR] Tests run: 43, Failures: 2, Errors: 0, Skipped: 2

@oscerd
Copy link
Contributor

oscerd commented Aug 22, 2017

LGTM. Will merge soon.

@oscerd
Copy link
Contributor

oscerd commented Aug 22, 2017

@davsclaus @lburgazzoli @zregvart may I have your feedback too?

@zregvart
Copy link
Member

@oscerd looks good to me, not sure about the failing tests. I think for future proofing we could add a Map<String, String> additionalProperties or similar (but that's another issue).

@oscerd
Copy link
Contributor

oscerd commented Aug 22, 2017

Thanks for the PR. It has been merged. Can you close this?

@snowch snowch closed this Aug 22, 2017
@snowch snowch reopened this Aug 22, 2017
@oscerd
Copy link
Contributor

oscerd commented Aug 22, 2017

Is there something missing? I see you reopened the PR..

@snowch
Copy link
Contributor Author

snowch commented Aug 22, 2017

Sorry, for closing and re-opening. I misread the GITHUB BOT feedback on the Jira ticket and thought I needed to do close the ticket another way.

Thanks for reviewing and merging.

@snowch snowch closed this Aug 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants