zeroshade opened a new pull request, #1704:
URL: https://github.com/apache/iceberg-go/pull/1704
## What
Fixes the CI failure on #1631 (`build(deps): bump the gomod_updates group
with 10 updates`).
This branch is #1631's head commit (`b46e8f7`) plus one fix commit, so it
carries the dependency bumps *and* the code change they need.
## Root cause
The failing test is `TestCompatModeTransferManagerNoAwsChunked` in
`io/gocloud`, which asserts that S3 "compat mode" (used for GCS's S3 interop
endpoint and other custom endpoints) sends no checksum framing on writes.
The breaking bump is
`github.com/aws/aws-sdk-go-v2/feature/s3/transfermanager` **v0.2.3 -> v0.3.5**.
In v0.3.x that module reworked how it picks an upload checksum algorithm:
- v0.2.3 had `resolveChecksumAlgorithm`, which set
`Options.ChecksumAlgorithm` to CRC32 and passed it into the S3 input. Our
middleware cleared that input field, so nothing was sent.
- v0.3.5 replaced it with `resolveRequestChecksumCalculation` plus a new
`transfermanager.Options.RequestChecksumCalculation` field, defaulting to
`aws.RequestChecksumCalculationWhenSupported`. In `api_op_UploadObject.go` the
uploader now applies that per call:
```go
clientOptions := []func(o *s3.Options){
func(o *s3.Options) {
o.RequestChecksumCalculation = u.options.RequestChecksumCalculation
...
```
That per-call option **overrides** the `o.RequestChecksumCalculation =
aws.RequestChecksumCalculationWhenRequired` we set on the client in
`createS3Bucket`. `AWSChecksum:SetupInputContext` then sees `WhenSupported` and
records a default CRC32 algorithm on the request context:
```go
if m.RequireChecksum || m.RequestChecksumCalculation ==
aws.RequestChecksumCalculationWhenSupported {
ctx = internalcontext.SetChecksumInputAlgorithm(ctx,
string(AlgorithmCRC32))
}
```
`AWSChecksum:ComputeInputPayloadChecksum` reads the context (not the input
struct) and emits `X-Amz-Checksum-Crc32`. Our `stripS3InputChecksumAlgorithm`
only cleared `PutObjectInput.ChecksumAlgorithm`, which suppresses the
`x-amz-sdk-checksum-algorithm` header but has no effect on this context-driven
default. Note the transfer manager constructs its own `transfermanager.Client`
inside `gocloud.dev/blob/s3blob.NewTypedWriter`, so we cannot pass
transfer-manager options in to fix it there.
## The fix
`io/gocloud/s3.go` only. Instead of inserting a middleware *before*
`AWSChecksum:SetupInputContext`, compat mode now **swaps that middleware in
place**, keeping its ID and stack position:
- For the object write inputs (`PutObjectInput`, `UploadPartInput`,
`CreateMultipartUploadInput`) it clears the input algorithm and skips the SDK
setup, so no algorithm is recorded on the context. The compute middleware
becomes a no-op and the request carries no `x-amz-checksum-*`, no
`x-amz-trailer`, no `x-amz-sdk-checksum-algorithm`, no `aws-chunked`
`Content-Encoding`, and no `STREAMING-` payload hash.
- Every other operation delegates to the captured original middleware, so
operations S3 *requires* a checksum for (e.g. `DeleteObjects`,
`RequireChecksum: true`) still get one.
- If the swap fails (reads, or an SDK without that middleware) it falls back
to the previous input-clearing behavior.
Because this is gated on `s3.compat-mode`, clients talking to real AWS S3
are untouched and still checksum normally.
The test was **not** modified.
## Before / after
Before (at `b46e8f7`):
```
=== NAME TestCompatModeTransferManagerNoAwsChunked
s3_test.go:491:
Error Trace: io/gocloud/s3_test.go:405
io/gocloud/s3_test.go:491
Error: Should be false
Test: TestCompatModeTransferManagerNoAwsChunked
Messages: transfer-manager PutObject must not send checksum
headers against custom endpoints, got X-Amz-Checksum-Crc32="NhCmhg=="
--- FAIL: TestCompatModeTransferManagerNoAwsChunked (0.00s)
FAIL github.com/apache/iceberg-go/io/gocloud
```
After:
```
--- PASS: TestCompatModeTransferManagerNoAwsChunked (0.00s)
--- PASS: TestCompatModeGetObjectStripsSignedHeaders (0.00s)
--- PASS: TestCompatModePutObjectNoAwsChunked (0.00s)
PASS
ok github.com/apache/iceberg-go/io/gocloud 0.465s
```
## Verification
All run locally in a worktree at this branch:
| Gate | Result |
| --- | --- |
| `go build ./...` | pass |
| `go vet ./...` | pass |
| `go test ./...` | pass, 0 failures across all 30 packages |
| `gofmt -l io/gocloud/s3.go` | clean |
| `golangci-lint run --timeout=10m ./io/gocloud/...` (v2.8.0, the pinned
version) | `0 issues.` |
| `go mod tidy` | no diff beyond what #1631 already has |
Integration tests (`-tags=integration`, requiring docker/spark) were **not**
run.
Beyond the repo's own tests I also checked, with throwaway tests against an
`httptest` server, that:
- compat-mode **multipart** uploads are clean too: `CreateMultipartUpload`,
all `UploadPart`s, and `CompleteMultipartUpload` emit no checksum framing;
- compat-mode `DeleteObjects` (a `RequireChecksum: true` operation)
**still** sends `x-amz-checksum-crc32`;
- an explicit caller-supplied `ChecksumCRC32` value is still sent in compat
mode;
- non-compat clients, both plain `PutObject` and transfer-manager uploads,
**still** send `X-Amz-Checksum-Crc32`.
Those scratch tests were removed and are not part of this PR.
## Note for maintainers
This branch includes #1631's dependency bumps, so #1631 and this PR overlap.
Either merge this one and close #1631, or cherry-pick the fix commit onto the
dependabot branch. Your call: the dependabot PR cannot go green without this
change.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
