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

Fix TLS grpc connection on windows #2551

Closed
wants to merge 1 commit into from

Conversation

wscheele
Copy link

@wscheele wscheele commented Oct 11, 2020

Resolves #2550
Borrows workaround from Sensu:
sensu/sensu-go#4018

Signed-off-by: Wouter Scheele wscheele@gmail.com

@wscheele wscheele requested a review from a team as a code owner October 11, 2020 11:52
@wscheele wscheele requested a review from pavolloffay October 11, 2020 11:52
Resolves jaegertracing#2550
Borrows workaround from Sensu:
sensu/sensu-go#4018

Signed-off-by: Wouter Scheele <wscheele@sf.local>
Signed-off-by: Wouter Scheele <wscheele@gmail.com>
@wscheele wscheele force-pushed the bugfix/windows-certs branch from 9fa7938 to fe336d4 Compare October 11, 2020 11:58
@jpkrohling
Copy link
Contributor

CI restarted.

Copy link
Contributor

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

Is it possible to test this at all?

@wscheele
Copy link
Author

For what it's worth, I have deployed, and am using, the branch built windows agent; and it runs happily and reports to tls enabled collector. Without the workaround it just immediately bails out. running the v1.20.1 windows build with --reporter.grpc.tls.enabled=true (pointing to a tls endpoint) should reproduce it easily.

I noticed I introduced some lint and test failures in the build, but am not too familiar with Go in general, and not at all familiar with this codebase and test setup specifically. I would be happy to enrich the PR with further changes to get a green build, but would appreciate some pointers in that case.

@jpkrohling
Copy link
Contributor

I noticed I introduced some lint and test failures in the build, but am not too familiar with Go in general, and not at all familiar with this codebase and test setup specifically. I would be happy to enrich the PR with further changes to get a green build, but would appreciate some pointers in that case.

The problem is that the Linux version of syscall doesn't have a CertOpenSystemStore method, for instance. The typical way of handling those cases in Go is to have the same function in different sources, each with a specific platform build tag. In the sensu-go PR you linked, you can see that they have two sources for the "fetcher": asset/fetcher_unix.go and asset/fetcher_windows.go. The first line in the Unix version looks like this:

 // +build !windows

And the first line for the Windows version:

 // +build windows

You'll need to do the same for this PR here. You can use the same approach for tests: create a Windows-specific test in a separate source file, requiring // +build windows. I don't think we are testing things in Windows at all, though.

@pere3
Copy link

pere3 commented Nov 13, 2020

Currently facing the same issue, so we are very eager for this merge - @wscheele would you kindly fix tests issues or you about to abandon it?

@yurishkuro
Copy link
Member

I think this has been fixed by #2756

@yurishkuro yurishkuro closed this Feb 25, 2021
@ghost
Copy link

ghost commented Apr 8, 2021

I'm running version 1.22.0 but I still get failed to load TLS config: failed to load CA CertPool: failed to load SystemCertPool: crypto/x509: system root pool is not available on Window. Specifying --reporter.grpc.tls.ca with an appropriate .pem file fixes it

@jpkrohling jpkrohling added this to the Release 1.23.0 milestone Jun 4, 2021
@Sepisak
Copy link

Sepisak commented Apr 1, 2022

I am now running jaeger-all-in-one.exe version 1.32.0 under Windows 10 and still get could not start gRPC collector failed to load CA CertPool: failed to load SystemCertPool: crypto/x509: system root pool is not available on Windows when I run commad jaeger-all-in-one --collector.zipkin.host-port=:9411 --collector.grpc.tls.enabled=true

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.

jaeger-agent --reporter.grpc.tls.enabled=true fails on windows
5 participants