Skip to content

fix: honor GCS skip_signature during uploads - #861

Open
RanaPriyansh wants to merge 2 commits into
apache:mainfrom
RanaPriyansh:fix/gcp-skip-signature-520
Open

RanaPriyansh wants to merge 2 commits into
apache:mainfrom
RanaPriyansh:fix/gcp-skip-signature-520

Conversation

@RanaPriyansh

@RanaPriyansh RanaPriyansh commented Sep 25, 2026 •

Copy link
Copy Markdown

Which issue does this PR close?

Rationale for this change

GCS upload requests fetched credentials and added a Bearer header even when with_skip_signature(true) was set. Request::send bypassed the existing skip-aware credential helper.

What changes are included in this PR?

Route upload requests through GoogleCloudStorageConfig::get_credential() and the optional Bearer-auth helper. Add a mock-server regression that checks credential calls and Authorization headers with signing enabled and disabled.

Are there any user-facing changes?

GCS uploads with skip_signature=true no longer fetch credentials at request time or send an Authorization header. The public API is unchanged.

Validation:

  • The new regression failed on the upstream base: the skipped request called the credential provider, raising the count from 1 to 2.
  • The focused regression passed after the repair.
  • cargo test --features=aws,azure,gcp,http passed: 264 unit tests, 3 integration tests, 1 HTTP test, and 57 doctests. Six unit tests and two doctests were ignored.
  • Strict Clippy passed for --features gcp and --no-default-features --features gcp-base. Formatting and diff checks passed.
  • cargo test --features gcp had one unrelated doctest failure: a doctest imports Azure when the Azure feature is disabled. The doctest passed with the repository CI feature set above.

Live GCS and custom-connector behavior were not tested. Builder-time credential-file parsing is unchanged.

Codex used.

@alamb
alamb marked this pull request as draft October 16, 2026 21:21
@alamb

alamb commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Marking as draft per #873 (comment) so i can find the PRs that need attention. Thank you

@alamb

alamb commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Reviewing this one per #873

@alamb

alamb commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

@andrewhamon and @kylebarron -- any chance you are up for helping to review this PR?

@RanaPriyansh

Copy link
Copy Markdown
Author

The test uses an explicit concrete Arc clone before conversion to the credential-provider trait object. This addresses the reported clone_on_ref_ptr finding while preserving the shared provider and existing assertions.

Formatting passed on the source committed as b9ea759. Local cargo test stopped during offline dependency resolution. The GCP regression and Clippy did not run locally. Hosted validation remains pending.

The prior emulator job failed while pulling the MinIO image, before its tests.

Codex used.

@kylebarron

Copy link
Copy Markdown
Member

I've never used GCS nor anonymous auth on GCS, so I don't have a way to validate this PR myself

@RanaPriyansh

Copy link
Copy Markdown
Author

The public-API regression passed locally on b9ea759: one test passed, with zero failures. It needs no GCS account.

The test uploads through put to a local HTTP server. With signing enabled, it asserts one credential-provider call and the received Bearer test-token header. With skip_signature=true, it asserts no additional credential call and no received Authorization header.

cargo test --features gcp --lib gcp::test::gcs_skip_signature_skips_credentials_and_authorization -- --exact --nocapture

Hosted Clippy also passed for this revision, including all targets. Live GCS anonymous uploads and bucket permissions remain untested.

Codex used.

@alamb alamb added the Prolific Author author has opened PRs in many repositories in the last month label Oct 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Prolific Author author has opened PRs in many repositories in the last month

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GoogleCloudStorageBuilder with_skip_signature(true) still attempting to fetch credentials

3 participants