objstore: support GCS S3-compatible signing by joechenrh · Pull Request #69721 · pingcap/tidb

joechenrh · 2026-07-08T05:08:15Z

What problem does this PR solve?

Issue Number: close #69718

Problem Summary:

IMPORT INTO can fail when reading source files from GCS through the S3-compatible endpoint after the S3 implementation moved to AWS SDK for Go v2.

With gcs s3 compatible endpoint, SDK v2 can sign accept-encoding into the SigV4 canonical request. GCS S3 interoperability can reject these requests with SignatureDoesNotMatch, and region discovery can fail on HeadBucket before the import starts.

What changed and how does it work?

This PR adds a GCS S3-compatible mode for the S3 object store:

Detect GCS S3-compatible usage by provider=gcs and endpoint=https://storage.googleapis.com, or virtual-hosted GCS endpoints.
Do AWS bucket-region discovery only for aws s3 endpoints.
Use a SigV4 signer wrapper that excludes accept-encoding from the canonical request while still sending the header on the HTTP request.

The signer keeps the AWS SDK v2 client path. Live GCS S3-compatible testing showed that amz-sdk-invocation-id and amz-sdk-request can remain signed; only accept-encoding needs to be excluded.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

Manual live test against the real GCS S3-compatible endpoint:

Setup next-gen cluster locally, and import data from gcs.
IMPORT INTO succeeded, which validates both source discovery (ListObjectsV2) and the following object reads (GetObject).
A retryable request failure was manually injected in the request path; the SDK retried and the import still completed.
The imported data was verified by row-count checks and ADMIN CHECK TABLE.
The test did not hit the previous GCS failures: SignatureDoesNotMatch, Forbidden, or failed to walk dir.

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

Fix IMPORT INTO failures when reading GCS through the S3-compatible endpoint with AWS SDK for Go v2.

Summary by CodeRabbit

Bug Fixes
- Improved compatibility with Google Cloud Storage when accessed through its S3-compatible API.
- Preserved request headers while ensuring signed requests meet Google Cloud Storage requirements.
- Improved bucket region handling for Google Cloud Storage endpoints.

ti-chi-bot · 2026-07-08T05:08:19Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

coderabbitai · 2026-07-08T05:08:26Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b275e450-2265-42f5-a985-962d0e896c8a

📥 Commits

Reviewing files that changed from the base of the PR and between fe72e6f and dcca32b.

📒 Files selected for processing (2)

pkg/objstore/s3store/BUILD.bazel
pkg/objstore/s3store/gcs_s3_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

pkg/objstore/s3store/BUILD.bazel

📝 Walkthrough

Walkthrough

Adds GCS S3-compatible endpoint detection, a custom SigV4 signer that excludes Accept-Encoding, bypasses AWS bucket-region discovery, and tests compatibility detection and signed listing requests.

Changes

GCS S3-compatible signing

Layer / File(s)	Summary
Custom signer implementation `pkg/objstore/s3store/gcs_s3_signer.go`, `pkg/objstore/s3store/BUILD.bazel`	Adds a SigV4 signer wrapper that temporarily removes and then restores `Accept-Encoding`; Bazel registers its source and signer dependency.
Endpoint detection and storage wiring `pkg/objstore/s3store/store.go`	Detects GCS providers and Google Storage endpoints, selects the custom signer, and skips AWS bucket-region discovery for compatible endpoints.
Compatibility and signing validation `pkg/objstore/s3store/gcs_s3_test.go`, `pkg/objstore/s3store/BUILD.bazel`	Tests provider and endpoint detection, verifies signed HEAD and ListObjectsV2 requests, and registers the test source in Bazel.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant NewS3Storage
  participant isGCSS3Compatible
  participant gcsS3CompatibleSigner
  participant v4Signer
  NewS3Storage->>isGCSS3Compatible: inspect provider and endpoint
  isGCSS3Compatible-->>NewS3Storage: return compatibility result
  NewS3Storage->>gcsS3CompatibleSigner: configure HTTPSignerV4
  gcsS3CompatibleSigner->>v4Signer: sign request without Accept-Encoding
  v4Signer-->>gcsS3CompatibleSigner: return signed request
  gcsS3CompatibleSigner-->>NewS3Storage: restore Accept-Encoding

Poem

I’m a bunny with a signed request,
Accept-Encoding tucked in my vest.
GCS paths now know the way,
SigV4 headers hop and play.
HEAD and GET both leave the nest!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly states the main change: adding GCS S3-compatible signing support in objstore.
Description check	✅ Passed	The description matches the template with issue number, problem summary, change details, tests, side effects, and release note.
Linked Issues check	✅ Passed	The changes address the issue by detecting GCS endpoints, skipping AWS bucket-region discovery, and fixing SigV4 signing for accept-encoding.
Out of Scope Changes check	✅ Passed	The modified files and tests stay focused on GCS S3 compatibility, with no obvious unrelated code paths added.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

codecov · 2026-07-08T08:03:10Z

Codecov Report

❌ Patch coverage is 0% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.9111%. Comparing base (8512729) to head (dcca32b).
⚠️ Report is 29 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #69721        +/-   ##
================================================
- Coverage   76.3227%   73.9111%   -2.4117%     
================================================
  Files          2041       2057        +16     
  Lines        560306     587856     +27550     
================================================
+ Hits         427641     434491      +6850     
- Misses       131764     152142     +20378     
- Partials        901       1223       +322

Flag	Coverage Δ
integration	`40.8364% <0.0000%> (+1.1312%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`60.4471% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`47.4060% <ø> (-15.3154%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ingress-bot · 2026-07-08T08:17:12Z

🔍 Starting code review for this PR...

ingress-bot

This review was generated by AI and should be verified by a human reviewer.
Manual follow-up is recommended before merge.

Summary

Total findings: 6
Inline comments: 2
Summary-only findings (no inline anchor): 4

Findings (highest risk first)

🟡 [Minor] (5)

Header assertions run in the httptest handler goroutine, degrading the regression guard (pkg/objstore/s3store/gcs_test.go:163)
New GCS carve-out in officialS3 has no rationale comment near the enforcement point (pkg/objstore/s3store/store.go:243)
Signer header-stripping test is driven through full storage construction instead of the signer directly (pkg/objstore/s3store/gcs_test.go:50, pkg/objstore/s3store/gcs_s3_signer.go:34)
GCS signer regression test runs require.* assertions and mutates a shared counter inside the httptest handler goroutine (pkg/objstore/s3store/gcs_test.go:63, pkg/objstore/s3store/gcs_test.go:70, pkg/objstore/s3store/gcs_test.go:98)
GCS signer test asserts and mutates shared state inside the httptest handler goroutine (pkg/objstore/s3store/gcs_test.go:65, pkg/objstore/s3store/gcs_test.go:71, pkg/objstore/s3store/gcs_test.go:98)

🧹 [Nit] (1)

getSignedHeaders name implies a parsed list but returns a raw joined string (pkg/objstore/s3store/gcs_test.go:201)

Unanchored findings

🟡 [Minor] (3)

Header assertions run in the httptest handler goroutine, degrading the regression guard
- Request: Capture the signed-header snapshot(s) from the handler into a slice/channel guarded from the goroutine (or record and compare after NewS3Storage returns), then run the require.* header assertions in the test goroutine so a leaked header is reported directly and cleanly.
New GCS carve-out in officialS3 has no rationale comment near the enforcement point
- Request: Add a short comment on or right above this line stating that GCS-compatible backends are excluded from AWS bucket-region auto-detection because that API is AWS-specific and not meaningful against a GCS endpoint.
Signer header-stripping test is driven through full storage construction instead of the signer directly
- Request: Add a direct unit test that calls newGCSS3CompatibleSigner().SignHTTP(ctx, creds, req, ...) on a constructed request and asserts the header set, so signer behavior is isolated from storage construction and permission-check request counts. Keep the current integration-style test only if it needs to prove the signer wiring itself (o.HTTPSignerV4 = ...), not the header-stripping logic.

🧹 [Nit] (1)

getSignedHeaders name implies a parsed list but returns a raw joined string
- Request: Rename to something like signedHeadersRaw/signedHeaderList (string) or split the value into a []string and use exact element checks (e.g. require.NotContains(t, headers, "accept-encoding") on a slice) to make the return shape match the name.

joechenrh · 2026-07-08T10:10:40Z

/retest

ingress-bot

This review was generated by AI and should be verified by a human reviewer.
Manual follow-up is recommended before merge.

Summary

Total findings: 3
Inline comments: 2
Summary-only findings (no inline anchor): 1

Findings (highest risk first)

🟡 [Minor] (3)

New GCS host constant breaks the file's existing domain-naming convention (pkg/objstore/s3store/store.go:52, pkg/objstore/s3store/store.go:54)
Endpoint-based GCS reclassification of existing configs has no end-to-end regression coverage (pkg/objstore/s3store/store.go:243, pkg/objstore/s3store/store.go:316, pkg/objstore/s3store/gcs_test.go:196)
Virtual-hosted GCS endpoint is not detected, so the GCS signer is silently skipped (pkg/objstore/s3store/store.go:327)

Unanchored findings

🟡 [Minor] (1)

New GCS host constant breaks the file's existing domain-naming convention
- Request: Rename gcsEndpoint to follow the existing domain* convention (for example domainGCS), or rename both to a shared, consistent scheme if Endpoint is intended to become the new convention going forward.

D3Hunter · 2026-07-10T02:15:32Z

+	// SDK v1 did not. Keep sending them, but exclude all three from the canonical
+	// request to stay close to the AWS SDK v1 signing behavior.
+	savedHeaders := http.Header{}
+	for _, key := range []string{"Accept-Encoding", "Amz-Sdk-Invocation-Id", "Amz-Sdk-Request"} {


seems only remove Accept-Encoding is enough according to aws/aws-sdk-go-v2#1816 (comment) , why remove the other 2

Updated in fe72e6f . I narrowed the signer wrapper to remove only Accept-Encoding from the canonical request.

I also verified this with a live GCS S3-compatible end-to-end run before changing the main PR: exact-file import covered GetObject, wildcard import covered ListObjectsV2 plus GetObject, and retry injection still completed without SignatureDoesNotMatch. The PR description now summarizes that coverage.

D3Hunter · 2026-07-10T02:23:50Z

 	// Perform region detection and validation
 	var detectedRegion string
-	officialS3 := len(qs.Provider) == 0 || qs.Provider == "aws"
+	officialS3 := !gcsS3Compatible && (len(qs.Provider) == 0 || qs.Provider == "aws")


a little hard to read, maybe qs.Provider == "aws" || (len(qs.Provider) == 0 && !gcsS3Compatible)

Adjusted in fe72e6f . I split the provider check into awsProvider and then use officialS3 := awsProvider && !gcsS3Compatible.

I avoided the exact suggested expression because provider=aws with a GCS endpoint still needs the GCS-compatible path; otherwise it can fall back into AWS bucket-region discovery and hit the same HeadBucket issue.

D3Hunter · 2026-07-10T02:24:44Z

 	// Perform region detection and validation
 	var detectedRegion string
-	officialS3 := len(qs.Provider) == 0 || qs.Provider == "aws"
+	officialS3 := !gcsS3Compatible && (len(qs.Provider) == 0 || qs.Provider == "aws")


and please comment why we check gcsS3Compatible here, i.e. HeadBucket issue...

Done in fe72e6f . I added the comment right above officialS3 to explain why GCS S3-compatible endpoints bypass AWS bucket-region discovery: the HeadBucket request can be rejected before normal object access starts, and the configured region is only used for signing in this path.

D3Hunter · 2026-07-10T02:25:17Z

+	// GCS S3 interoperability rejects signatures that include Accept-Encoding.
+	// AWS SDK v2 also signs the SDK request metadata headers below, while AWS
+	// SDK v1 did not. Keep sending them, but exclude all three from the canonical
+	// request to stay close to the AWS SDK v1 signing behavior.


please add solution sources as comment

Added in fe72e6f . The signer comment now links the AWS SDK Go v2 issue for the Accept-Encoding signing behavior, and the GCS interoperability / request endpoint links stay next to the gcsEndpoint constant where we define the endpoint detection rule.

D3Hunter · 2026-07-16T03:07:34Z

/cherry-pick release-nextgen-202603

ti-chi-bot · 2026-07-16T03:07:37Z

@D3Hunter: once the present PR merges, I will cherry-pick it on top of release-nextgen-202603 in the new PR and assign it to you.

Details

In response to this:

/cherry-pick release-nextgen-202603

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

D3Hunter · 2026-07-16T07:41:24Z

/retest

ti-chi-bot · 2026-07-16T08:02:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: D3Hunter, GMHDBJD

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~pkg/objstore/OWNERS~~ [D3Hunter,GMHDBJD]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2026-07-16T08:02:42Z

[LGTM Timeline notifier]

Timeline:

2026-07-16 03:07:26.2774505 +0000 UTC m=+855832.313545546: ☑️ agreed by D3Hunter.
2026-07-16 08:02:41.046409143 +0000 UTC m=+873547.082504189: ☑️ agreed by GMHDBJD.

joechenrh · 2026-07-16T08:20:50Z

/retest

ti-chi-bot · 2026-07-16T10:38:01Z

@D3Hunter: new pull request created to branch release-nextgen-202603: #69888.

Details

In response to this:

/cherry-pick release-nextgen-202603

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

joechenrh removed the do-not-merge/needs-triage-completed label Jul 8, 2026

ti-chi-bot Bot added do-not-merge/needs-triage-completed and removed do-not-merge/needs-tests-checked labels Jul 8, 2026

joechenrh force-pushed the fix-gcs-s3-compatible-signer branch from dcb9b59 to 339f3fa Compare July 8, 2026 05:14

objstore: support GCS S3-compatible signing

b2ee22c

joechenrh force-pushed the fix-gcs-s3-compatible-signer branch from 339f3fa to b2ee22c Compare July 8, 2026 06:57

joechenrh added 3 commits July 8, 2026 15:08

objstore: move GCS endpoint reference comment

7beefe7

objstore: clarify GCS signed header workaround

be2d163

objstore: rename GCS S3 signer file

4fb58ea

joechenrh marked this pull request as ready for review July 8, 2026 07:39

ti-chi-bot Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 8, 2026

objstore: add GCS signer files to Bazel

9ae4ce2

ti-chi-bot Bot removed the do-not-merge/needs-triage-completed label Jul 8, 2026

ingress-bot reviewed Jul 8, 2026

View reviewed changes

Comment thread pkg/objstore/s3store/gcs_test.go Outdated

Comment thread pkg/objstore/s3store/gcs_test.go Outdated

objstore: avoid assertions in GCS test handler

0ccd998

ingress-bot reviewed Jul 8, 2026

View reviewed changes

Comment thread pkg/objstore/s3store/store.go Outdated

Comment thread pkg/objstore/s3store/store.go Outdated

D3Hunter reviewed Jul 10, 2026

View reviewed changes

joechenrh mentioned this pull request Jul 10, 2026

objstore: test accept-encoding-only GCS signing #69759

Closed

13 tasks

objstore: narrow GCS signer workaround

fe72e6f

ti-chi-bot Bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 13, 2026

objstore: rename GCS S3 test file

dcca32b

D3Hunter approved these changes Jul 16, 2026

View reviewed changes

ti-chi-bot Bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jul 16, 2026

GMHDBJD approved these changes Jul 16, 2026

View reviewed changes

ti-chi-bot Bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jul 16, 2026

ti-chi-bot Bot merged commit d573e28 into pingcap:master Jul 16, 2026
34 checks passed

ti-chi-bot mentioned this pull request Jul 16, 2026

objstore: support GCS S3-compatible signing (#69721) #69888

Open

13 tasks

Uh oh!

Conversation

joechenrh commented Jul 8, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Summary by CodeRabbit

Uh oh!

ti-chi-bot Bot commented Jul 8, 2026

Uh oh!

coderabbitai Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ingress-bot commented Jul 8, 2026

Uh oh!

ingress-bot left a comment

Choose a reason for hiding this comment

Summary

🟡 [Minor] (5)

🧹 [Nit] (1)

Unanchored findings

🟡 [Minor] (3)

🧹 [Nit] (1)

Uh oh!

Uh oh!

Uh oh!

joechenrh commented Jul 8, 2026

Uh oh!

ingress-bot left a comment

Choose a reason for hiding this comment

Summary

🟡 [Minor] (3)

Unanchored findings

🟡 [Minor] (1)

Uh oh!

Uh oh!

Uh oh!

D3Hunter Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

joechenrh Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

D3Hunter Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joechenrh Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

D3Hunter Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

joechenrh Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

D3Hunter Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

joechenrh Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

D3Hunter commented Jul 16, 2026

Uh oh!

ti-chi-bot commented Jul 16, 2026

Uh oh!

D3Hunter commented Jul 16, 2026

Uh oh!

ti-chi-bot Bot commented Jul 16, 2026

Uh oh!

joechenrh commented Jul 8, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading

codecov Bot commented Jul 8, 2026 •

edited

Loading

D3Hunter Jul 10, 2026 •

edited

Loading