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

experimental/credentials: Add experimental credentials that don't enforce ALPN #7980

Merged
merged 7 commits into from
Jan 15, 2025

Conversation

arjan-bal
Copy link
Contributor

@arjan-bal arjan-bal commented Jan 6, 2025

Fixes: #7922
Related Issue: #434

This change copies the existing TLS credentials, removing the code for ALPN enforcement. These credentials will be removed in a couple of releases and users who still need to disable ALPN can copy the credentials as is. Changes made:

  • Copy credentials/tls.go to experimental/credentials/tls.go. Add suffix WithALPNDisabled to constructors.
  • Copy required packages from internal/credentials to experimental/credentials/internal.
  • Improve error message when connection fails due to ALPN.

RELEASE NOTES:

  • experimental/credentials: Experimental transport credentials are added which don't enforce ALPN. These will be removed in upcoming grpc-go releases. Users must not rely on these credentials directly. Instead, they should either vendor a specific version of gRPC or copy the relevant credentials into their own codebase if absolutely necessary.

@arjan-bal arjan-bal added the Type: Internal Cleanup Refactors, etc label Jan 6, 2025
@arjan-bal arjan-bal added this to the 1.70 Release milestone Jan 6, 2025
@arjan-bal arjan-bal requested a review from dfawley January 6, 2025 07:45
Copy link

codecov bot commented Jan 6, 2025

Codecov Report

Attention: Patch coverage is 81.92771% with 30 lines in your changes missing coverage. Please review.

Project coverage is 82.29%. Comparing base (724f450) to head (e179f32).
Report is 11 commits behind head on master.

Files with missing lines Patch % Lines
experimental/credentials/tls.go 75.80% 26 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7980      +/-   ##
==========================================
+ Coverage   82.28%   82.29%   +0.01%     
==========================================
  Files         381      384       +3     
  Lines       38539    38745     +206     
==========================================
+ Hits        31712    31887     +175     
- Misses       5535     5559      +24     
- Partials     1292     1299       +7     
Files with missing lines Coverage Δ
credentials/tls.go 86.57% <100.00%> (ø)
experimental/credentials/internal/spiffe.go 100.00% <100.00%> (ø)
experimental/credentials/internal/syscallconn.go 100.00% <100.00%> (ø)
experimental/credentials/tls.go 75.80% <75.80%> (ø)

... and 27 files with indirect coverage changes

@arjan-bal arjan-bal assigned dfawley and unassigned arjan-bal Jan 13, 2025
@arjan-bal arjan-bal requested a review from dfawley January 13, 2025 06:08
// Package credentials provides experimental TLS credentials.
// The use of this package is strongly discouraged. These credentials exist
// solely to maintain compatibility for users interacting with clients that
// violate the HTTP/2 specification. This package is slated for removal in
Copy link
Member

Choose a reason for hiding this comment

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

Maybe be more explicit here about the way in which they violate the spec? E.g.

...violate the HTTP/2 specification by not advertising support for "h2" in ALPN.?

@dfawley dfawley assigned arjan-bal and unassigned dfawley Jan 13, 2025
@arjan-bal arjan-bal merged commit eb1adde into grpc:master Jan 15, 2025
14 of 15 checks passed
@arjan-bal arjan-bal deleted the experiments-tls branch January 15, 2025 11:55
@arjan-bal arjan-bal changed the title credentials: Add experimental credentials that don't enforce ALPN experimental/credentials: Add experimental credentials that don't enforce ALPN Jan 16, 2025
arjan-bal added a commit to arjan-bal/grpc-go that referenced this pull request Jan 16, 2025
janardhanvissa pushed a commit to janardhanvissa/grpc-go that referenced this pull request Jan 26, 2025
janardhanvissa pushed a commit to janardhanvissa/grpc-go that referenced this pull request Feb 13, 2025
janardhanvissa pushed a commit to janardhanvissa/grpc-go that referenced this pull request Feb 13, 2025
janardhanvissa pushed a commit to janardhanvissa/grpc-go that referenced this pull request Feb 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updating from go-grpc v1.64.0 to v1.68.1 broke our AWS deployments, migration path is unclear
2 participants