Skip to content

Conversation

@alvarowolfx
Copy link
Collaborator

Add core logic for generating signed URLs for GCS. Tested against GCS conformance tests.

Towards #3645

@alvarowolfx alvarowolfx requested a review from a team as a code owner January 2, 2026 15:56
@product-auto-label product-auto-label bot added the api: storage Issues related to the Cloud Storage API. label Jan 2, 2026
@codecov
Copy link

codecov bot commented Jan 2, 2026

Codecov Report

❌ Patch coverage is 98.83721% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.78%. Comparing base (1830c77) to head (1b13f40).
⚠️ Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
src/storage/src/storage/signed_url.rs 98.83% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4141      +/-   ##
==========================================
+ Coverage   94.72%   94.78%   +0.06%     
==========================================
  Files         184      184              
  Lines        6933     7002      +69     
==========================================
+ Hits         6567     6637      +70     
+ Misses        366      365       -1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@dbolduc dbolduc left a comment

Choose a reason for hiding this comment

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

You need to convert the keys to lowercase on insert to the maps and revisit this logic.

And note that we are using BTreeMaps as the data structure, so we get sorting for free. You should be suspicious of any additional sorting. It probably means the code could be simpler.

@alvarowolfx
Copy link
Collaborator Author

@dbolduc I managed to get rid of the confusing code around sorting and lowercasing headers. In the end the only part that needs to keep ordering and casing is for Query Params, so for headers we can lowercase on insert and keep sort of BTreeMap.

@alvarowolfx alvarowolfx requested a review from dbolduc January 5, 2026 15:32
dbolduc
dbolduc previously approved these changes Jan 5, 2026
Copy link
Member

@dbolduc dbolduc left a comment

Choose a reason for hiding this comment

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

This is much easier on the eyes. Thanks.

dbolduc
dbolduc previously approved these changes Jan 5, 2026
@alvarowolfx
Copy link
Collaborator Author

This is much easier on the eyes. Thanks.

thanks for the help on getting this into a much better state. Just pushed some updates to address the final nits.

@alvarowolfx alvarowolfx requested a review from dbolduc January 5, 2026 20:05
@alvarowolfx alvarowolfx merged commit 99dcfeb into googleapis:main Jan 5, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants