## Summary With `FF_USE_PARALLEL_CACHE_TRANSFER` enabled, the cache extractor downloads a GoCloud URL (used for S3 when `RoleARN` is set, and for Azure) in parallel chunks without checking what each chunk response holds. `handleGoCloudURL` skips the range probe for the `s3`, `gs` and `azblob` schemes ([`cache_extractor.go#L208-L218`](https://gitlab.com/gitlab-org/gitlab-runner/-/blob/b130a28af451b8b1044ad68b7c77e33d1669eb63/commands/helpers/cache_extractor.go#L208-L218)) and hands `selectedBucket.NewRangeReader` to `downloadParallel` as the chunk fetcher ([`cache_extractor.go#L494-L499`](https://gitlab.com/gitlab-org/gitlab-runner/-/blob/b130a28af451b8b1044ad68b7c77e33d1669eb63/commands/helpers/cache_extractor.go#L494-L499)). If a server in front of the store answers a chunk with `200` and the whole object, `transfer.ParallelRangeDownload` writes the opening bytes of the object at that chunk's offset, and the download reports success with a corrupt archive. The drivers do not catch it at the versions in `go.mod`: - `s3blob` in gocloud.dev v0.46.0 returns the `GetObject` body without looking at the status or `ContentRange`, and the AWS SDK accepts any `2xx`. - The `azblob` v1.8.1 download accepts both `200` and `206`. - GCS is not affected: `cloud.google.com/go/storage` v1.68.0 refuses a non-`206` answer to a request that starts after byte 0 (`storage: partial request not satisfied`). This is the GoCloud half of #40102. !7635 fixes the presigned cache path and the artifact path, where the runner sees the HTTP response. Here the driver hides it, so the check has to go through `Reader.As`. It was found in the review of !7635 ([note](https://gitlab.com/gitlab-org/gitlab-runner/-/merge_requests/7635#note_4010944787)), with a run against SeaweedFS 3.85 behind an nginx load balancer whose second node ignores `Range`. ## Steps to reproduce No object store is needed. On `main` (b130a28a), add this test to `commands/helpers/cache_extractor_test.go` (it uses that file's imports and helpers) and run `go test -run TestGoCloudS3ParallelChunkIgnoresRange -v ./commands/helpers/`. The `httptest` server stands in for an S3 endpoint that answers the first chunk as requested and every later chunk with `200` and the whole object. ```go func TestGoCloudS3ParallelChunkIgnoresRange(t *testing.T) { t.Setenv(featureflags.UseParallelCacheTransfer, "true") t.Setenv("AWS_ACCESS_KEY_ID", "test") t.Setenv("AWS_SECRET_ACCESS_KEY", "test") cdTempDir(t) payload := parallelTestZipBytes(t) lm := time.Date(2020, 5, 1, 12, 0, 0, 0, time.UTC).Format(http.TimeFormat) ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Last-Modified", lm) w.Header().Set("ETag", `"0123456789abcdef0123456789abcdef"`) w.Header().Set("Content-Type", "application/zip") start, end, ok := parseBytesRangeHeader(r.Header.Get("Range")) switch { case !ok || start > 0: // HEAD, a GET without Range, or a chunk after the first: the whole object. w.Header().Set("Content-Length", strconv.Itoa(len(payload))) if r.Method != http.MethodHead { _, _ = w.Write(payload) } default: w.Header().Set("Content-Range", fmt.Sprintf("bytes %d-%d/%d", start, end, len(payload))) w.Header().Set("Content-Length", strconv.FormatInt(end-start+1, 10)) w.WriteHeader(http.StatusPartialContent) _, _ = w.Write(payload[start : end+1]) } })) t.Cleanup(ts.Close) q := url.Values{} q.Set("endpoint", ts.URL) q.Set("region", "us-east-1") q.Set("use_path_style", "true") q.Set("disable_https", "true") cmd := CacheExtractorCommand{ File: cacheExtractorArchive, GoCloudURL: "s3://bucket/cache.zip?" + q.Encode(), ChunkSize: 7, Concurrency: 4, TransferBufferSize: defaultCacheTransferBufferSize, } err := cmd.handleGoCloudURL() got, _ := os.ReadFile(cacheExtractorArchive) t.Logf("err=%v archive=%d bytes, equal to the object: %v", err, len(got), bytes.Equal(got, payload)) } ``` It prints: ```plaintext err=<nil> archive=159 bytes, equal to the object: false ``` ## Actual behavior A chunk answered with `200` and the whole object is written at its offset. `handleGoCloudURL` returns no error and leaves a corrupt archive. As on the presigned path before !7635, the archive keeps the object's modification time, so a later run takes it as up to date. ## Expected behavior A chunk response that does not hold the requested range fails the parallel download instead of being written. When the answer is a `200` with the whole object, the extractor can download the object with a single `NewReader` instead. ## Relevant logs and/or screenshots The test output above. ## Environment description Any executor, with `FF_USE_PARALLEL_CACHE_TRANSFER` enabled, an S3 cache with `RoleARN` set or an Azure cache, and a server in front of the store that answers some Range requests with `200`. The flag defaults to `false`. ### Used GitLab Runner version GitLab Runner v19.5.0, whose `cache_extractor.go` is the same as at the commit below. `main` at b130a28af451b8b1044ad68b7c77e33d1669eb63, and the same code since the parallel cache download was added in v19.0.0. ## Possible fixes In the chunk fetcher, read the response through `Reader.As`: `*s3.GetObjectOutput.ContentRange` for S3 and the download response's `ContentRange` for Azure, and fail the chunk when it is not the requested range. A `200` of that kind can fall back to the sequential `NewReader` download, as the presigned path does after !7635. A `206` for another range is better left as a failure, since through a caching proxy whose cache key leaves out `Range` a sequential download can get the same partial answer.