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: enable TLS roots for flight CLI client #6640

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

crepererum
Copy link
Contributor

Which issue does this PR close?

-

Rationale for this change

Otherwise you get:

Error: setup client

Caused by:
    0: connect to endpoint
    1: transport error
    2: invalid peer certificate: UnknownIssuer
    3: invalid peer certificate: UnknownIssuer

Also see hyperium/tonic#1904 .

What changes are included in this PR?

Just call the method that registers the roots that we include via feature flags anyways.

Are there any user-facing changes?

TLS works again.

Otherwise you get:

```text
Error: setup client

Caused by:
    0: connect to endpoint
    1: transport error
    2: invalid peer certificate: UnknownIssuer
    3: invalid peer certificate: UnknownIssuer
```

Also see hyperium/tonic#1904 .
@github-actions github-actions bot added arrow Changes to the arrow crate arrow-flight Changes to the arrow-flight crate labels Oct 29, 2024
@tustvold
Copy link
Contributor

Ideally we'd have a test of this, but I don't feel strongly and it is probably difficult to setup

@crepererum
Copy link
Contributor Author

Ideally we'd have a test of this, but I don't feel strongly and it is probably difficult to setup

I thought about that, but that would require that we have a server that has a certificate that is accepted by the Mozilla CA bundle, so without additional infrastructure, I don't think that's feasible.

Or we do something "stupid" like point the client to https://apache.org and check the error message that should say something like "not found" (because the gRPC endpoint isn't implemented) but that kinda proofs that the TLS connection worked. WDYT?

@tustvold
Copy link
Contributor

That sounds like it could very well end up being flaky, I think let's leave it for now and we can always revisit

@tustvold tustvold merged commit 2983dc1 into apache:master Oct 29, 2024
12 checks passed
alamb added a commit to influxdata/arrow-rs that referenced this pull request Dec 2, 2024
wiedld pushed a commit to influxdata/arrow-rs that referenced this pull request Jan 27, 2025
wiedld pushed a commit to influxdata/arrow-rs that referenced this pull request Jan 31, 2025
alamb added a commit to influxdata/arrow-rs that referenced this pull request Feb 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate arrow-flight Changes to the arrow-flight crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants