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: set h2 protocol identifier to comply with TLS-ALPN #5413

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

sgarcez
Copy link
Contributor

@sgarcez sgarcez commented Oct 9, 2024

In order to bump grpc-go (in this repo or in external clients) we need to specify h2 as the application protocol in buildkitd's TLS config.

The enforcement was introduced in grpc-go v1.67.0.

buildkit and buildx are currently on v1.66.2 so this patch is needed for the next update. I didn't update grpc-go in this patch, but can do if you prefer.

Testing with buildctl on v1.67.1

◊ cat go.mod | grep "google.golang.org/grpc\ "
        google.golang.org/grpc v1.67.1

Before this patch

◊ ./buildctl --addr tcp://localhost:1234 --tlsdir /tmp/certs/client/ build --local context=. --local dockerfile=. --frontend dockerfile.v0
[+] Building 0.0s (0/0)
error: listing workers for Build: failed to list workers: Unavailable: connection error: desc = "transport: authentication handshake failed: credentials: cannot check peer: missing selected ALPN property"

After patch

◊ ./buildctl --addr tcp://localhost:1234 --tlsdir /tmp/certs/client/ build --local context=. --local dockerfile=. --frontend dockerfile.v0
[+] Building 44.3s (71/71) FINISHED

Context

@sgarcez sgarcez force-pushed the grpc-alpn-fix branch 2 times, most recently from 7fdfd2b to f2a2866 Compare October 9, 2024 16:03
@thompson-shaun thompson-shaun added this to the v0.19.0 milestone Oct 9, 2024
@tonistiigi tonistiigi merged commit 2534310 into moby:master Oct 10, 2024
91 checks passed
@crazy-max crazy-max modified the milestones: v0.19.0, v0.17.0 Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants