Skip to content

Dep: Pin grpcio in flytekit-identity-aware-proxy to version that allo…#2212

Merged
fg91 merged 1 commit into
masterfrom
fg91/dep/flytekit-identity-aware-proxy-pin-grpcio
Mar 1, 2024
Merged

Dep: Pin grpcio in flytekit-identity-aware-proxy to version that allo…#2212
fg91 merged 1 commit into
masterfrom
fg91/dep/flytekit-identity-aware-proxy-pin-grpcio

Conversation

@fg91

@fg91 fg91 commented Feb 23, 2024

Copy link
Copy Markdown
Member

There was a regression in grpcio which made a channel fail if the server responded with a content type other than application/grpc.

During flytekit's lazy authentication, GCP Identity Aware Proxy (IAP) responds with 401 and content type text/html to the first unauthenticated request. For recent grpcio versions, lazy authentication thus failed completely.

I described this in detail in grpc/grpc#35323.


In #2156 I previously pinned grpcio in the flytekit IAP plugin to versions without this regression.


The grpc team now fixed the issue I reported. In this PR I therefore pin grpcio>=1.62.0.

I exclude the previously allowed versions because the error message

E1215 14:41:40.919736000 4303291776 hpack_parser.cc:853]               Error parsing metadata: error=invalid value key=content-type value=text/html; charset=UTF-8

used to be logged during lazy authentication even though it did actually work.

With the newest version, no more error message is logged.

@codecov

codecov Bot commented Feb 23, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.85%. Comparing base (90324e0) to head (a4512aa).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2212      +/-   ##
==========================================
- Coverage   85.86%   85.85%   -0.01%     
==========================================
  Files         314      314              
  Lines       24042    24042              
  Branches     3643     3643              
==========================================
- Hits        20643    20641       -2     
- Misses       2759     2760       +1     
- Partials      640      641       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fg91 fg91 marked this pull request as ready for review February 23, 2024 17:40
@dosubot dosubot Bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Feb 23, 2024
@fg91 fg91 requested a review from jeevb February 23, 2024 17:41
@dosubot dosubot Bot added the lgtm This PR has been approved by maintainer label Feb 23, 2024
…ws non 'application/grpc' content type

Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
@fg91 fg91 force-pushed the fg91/dep/flytekit-identity-aware-proxy-pin-grpcio branch from 58b3eca to a4512aa Compare February 29, 2024 21:26
@fg91 fg91 merged commit ea3c02d into master Mar 1, 2024
@fg91 fg91 deleted the fg91/dep/flytekit-identity-aware-proxy-pin-grpcio branch March 1, 2024 07:42
fiedlerNr9 pushed a commit that referenced this pull request Jul 25, 2024
…ws non 'application/grpc' content type (#2212)

Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
Co-authored-by: Fabio Grätz <fabiogratz@googlemail.com>
Signed-off-by: Jan Fiedler <jan@union.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by maintainer size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants