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

Update Samples for 3.1.0 with removing cacert.pem from Kafka Extensions #168

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

TsuyoshiUshio
Copy link
Contributor

@TsuyoshiUshio TsuyoshiUshio commented Aug 28, 2020

Update the samples to the version 3.1.0

What I did

  • Upgrade sample from 3.0.0 to 3.1.0
  • Remove cacert.pem since It no longer needed
  • Remove cacert.pem from Kafka extension
  • Remove EvenHubs with cacert validation
  • Tested EventHubs with Azure Functions sample and it works.

Hi @fbeltrao
Could you have a look at Microsoft.Azure.WebJobs.Extensions.Kafka.csproj , please?
Also, the KafkaListner's validation of the EventHubs with cacert validation.
I'd like to make sure if it is no problem for the customers.

@TsuyoshiUshio TsuyoshiUshio changed the title Update Samples for 3.1.0 Update Samples for 3.1.0 with removing cacert.pem from Kafka Extensions Aug 28, 2020
@@ -214,36 +214,14 @@ private ConsumerConfig GetConsumerConfiguration()
conf.SaslMechanism = SaslMechanism.Plain;
conf.SaslUsername = EventHubsSaslUsername;
conf.SaslPassword = this.listenerConfiguration.EventHubConnectionString;
conf.SslCaLocation= this.EnsureValidEventHubsCertificateLocation(this.listenerConfiguration.SslCaLocation);
conf.SslCaLocation= this.listenerConfiguration.SslCaLocation;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to provide a CA for event hubs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @fbeltrao
Thank you for the review. I think we don't need CA anymore. not only EventHubs but others.
We might be able to remove the SslCaLocation, however, there is leagacy code that cause breaking change by it and in some usecase still might use it. That is why I keep the SslCaLocation It's self. Do you have any suggestion?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Since this is inside the block for event hubs only and we don't need a CA we could remove the line, don't we?

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