Skip to content
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

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

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