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

Use system SSL store for notifiers by default #541

Merged
merged 1 commit into from Jan 29, 2020

Conversation

danudey
Copy link
Contributor

@danudey danudey commented Jun 10, 2019

A fix to issue #540, using the system store by default (if possible), and adding any extra certificates to that (if applicable).

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 74.66% when pulling 1230c40 on athinkingape:master into a449cc4 on linkedin:master.


rootCAs, caError := x509.SystemCertPool()
if caError != nil {
log.Println("Unable to load system certs, using empty cert pool instead")

Choose a reason for hiding this comment

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

Nit: a period at the end.

@ghost
Copy link

ghost commented Jan 3, 2020

I've tested these changes and it fixes x509: certificate signed by unknown authority"} issue

@andrewchoi5 can we get this PR merged please

@richardwalsh
Copy link

+1

Running into this issue too when updating to the latest for kafka 2.4 support.

@seallison
Copy link

This issue is preventing us from upgrading.

@andrewchoi5 would be great to have this merged

@andrewchoi5
Copy link

This issue is preventing us from upgrading.

@andrewchoi5 would be great to have this merged

Merge permission isn't visible on my end.

@seallison
Copy link

This issue is preventing us from upgrading.
@andrewchoi5 would be great to have this merged

Merge permission isn't visible on my end.

thanks - @bai can you help out with getting this merged & released?

@bai bai merged commit c125a38 into linkedin:master Jan 29, 2020
@bai
Copy link
Collaborator

bai commented Jan 29, 2020

Thanks 💯I'll release this today.

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.

None yet

6 participants