diff --git a/cache/cache.go b/cache/cache.go index 0948b6dec731c60e5e110503f654daa82e70da15..a5a403bbbf175cedfaa1b428fe1fd6f330089d4b 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -24,7 +24,7 @@ func (nopAdapter) GetGoCloudURL(ctx context.Context, upload bool) (GoCloudURL, e var createAdapter = getCreateAdapter -func GetAdapter(config *cacheconfig.Config, timeout time.Duration, shortToken, projectId, key string) Adapter { +func GetAdapter(config *cacheconfig.Config, timeout time.Duration, shortToken, projectId, key string, sharded bool) Adapter { if config == nil { return nopAdapter{} } @@ -42,7 +42,22 @@ func GetAdapter(config *cacheconfig.Config, timeout time.Duration, shortToken, p namespace = path.Join("runner", shortToken) } basePath := path.Join(config.GetPath(), namespace, "project", projectId) - fullPath := path.Join(basePath, key) + + // When sharded (i.e. FF_HASH_CACHE_KEYS is enabled), insert the first two + // hex characters of the key as an intermediate path component. This + // distributes objects across 256 distinct S3 prefixes per project, avoiding + // 503 Slow Down responses caused by all cache objects sharing the same + // prefix and landing on the same partition. + var fullPath string + if sharded { + if len(key) < 2 { + logrus.WithError(fmt.Errorf("cache key too short to shard (length %d)", len(key))).Error("Error while generating cache bucket.") + return nopAdapter{} + } + fullPath = path.Join(basePath, key[:2], key) + } else { + fullPath = path.Join(basePath, key) + } // The typical concerns regarding the use of strings.HasPrefix to detect // path traversal do not apply here. The detection here is made easier diff --git a/cache/cache_test.go b/cache/cache_test.go index 2be551e7965b5fe8a9f2b92e124baae42133167c..602133669216baf8a555aadeb2a98f6b1c629c4f 100644 --- a/cache/cache_test.go +++ b/cache/cache_test.go @@ -87,7 +87,7 @@ func testCacheOperation( prepareFakeCreateAdapter(t, operationName, tc) config := prepareFakeConfig(tc) - adaptor := GetAdapter(config, 3600*time.Second, "shorttoken", "10", tc.key) + adaptor := GetAdapter(config, 3600*time.Second, "shorttoken", "10", tc.key, false) generatedURL := operation(ctx, adaptor) assert.Equal(t, tc.expectedURL, generatedURL.URL) @@ -179,9 +179,10 @@ func defaultCacheConfig() *cacheconfig.Config { } type generateObjectNameTestCase struct { - key string - path string - shared bool + key string + path string + shared bool + sharded bool expectedObjectName string expectedError string @@ -235,6 +236,23 @@ func TestGenerateObjectName(t *testing.T) { key: "../10-outside", expectedError: "computed cache path outside of project bucket", }, + "sharded key uses first two chars as prefix": { + key: "d03a852ba491ba611e907b1ef60ad5c4516a05b8f3aae6abb77f42bc60325aed", + sharded: true, + expectedObjectName: "runner/longtoken/project/10/d0/d03a852ba491ba611e907b1ef60ad5c4516a05b8f3aae6abb77f42bc60325aed", + }, + "sharded key with path prefix": { + key: "d03a852ba491ba611e907b1ef60ad5c4516a05b8f3aae6abb77f42bc60325aed", + path: "builds", + sharded: true, + expectedObjectName: "builds/runner/longtoken/project/10/d0/d03a852ba491ba611e907b1ef60ad5c4516a05b8f3aae6abb77f42bc60325aed", + }, + "sharded key with shared runner": { + key: "d03a852ba491ba611e907b1ef60ad5c4516a05b8f3aae6abb77f42bc60325aed", + shared: true, + sharded: true, + expectedObjectName: "project/10/d0/d03a852ba491ba611e907b1ef60ad5c4516a05b8f3aae6abb77f42bc60325aed", + }, } for name, tc := range tests { @@ -255,7 +273,7 @@ func TestGenerateObjectName(t *testing.T) { createAdapter = oldCreateAdapter }) - adapter := GetAdapter(cache, 3600*time.Second, "longtoken", "10", tc.key) + adapter := GetAdapter(cache, 3600*time.Second, "longtoken", "10", tc.key, tc.sharded) if tc.expectedError != "" { // The error/warning cases return a nopAdaptor and log instead of returning an error diff --git a/common/build_step_dispatch.go b/common/build_step_dispatch.go index 7120df2890d9651de2df74f4f5418d42af886a94..898c2414ce65d1ec0b9916365c70936f44733621 100644 --- a/common/build_step_dispatch.go +++ b/common/build_step_dispatch.go @@ -166,7 +166,7 @@ func stagesToConcreteStep(ctx context.Context, executor Executor) ([]schema.Step if build.Runner.Cache != nil { opts = append(opts, builder.WithCacheMaxArchiveSize(build.Runner.Cache.MaxUploadedArchiveSize), builder.WithCacheDownloadDescriptor(func(cacheKey string) (cacheprovider.Descriptor, error) { - adapter := cache.GetAdapter(build.Runner.Cache, build.GetBuildTimeout(), build.Runner.ShortDescription(), fmt.Sprintf("%d", build.JobInfo.ProjectID), cacheKey) + adapter := cache.GetAdapter(build.Runner.Cache, build.GetBuildTimeout(), build.Runner.ShortDescription(), fmt.Sprintf("%d", build.JobInfo.ProjectID), cacheKey, build.IsFeatureFlagOn(featureflags.HashCacheKeys)) goCloudURL, err := adapter.GetGoCloudURL(ctx, false) if goCloudURL.URL != nil { @@ -187,7 +187,7 @@ func stagesToConcreteStep(ctx context.Context, executor Executor) ([]schema.Step return cacheprovider.Descriptor{}, nil }), builder.WithCacheUploadDescriptor(func(cacheKey string) (cacheprovider.Descriptor, error) { - adapter := cache.GetAdapter(build.Runner.Cache, build.GetBuildTimeout(), build.Runner.ShortDescription(), fmt.Sprintf("%d", build.JobInfo.ProjectID), cacheKey) + adapter := cache.GetAdapter(build.Runner.Cache, build.GetBuildTimeout(), build.Runner.ShortDescription(), fmt.Sprintf("%d", build.JobInfo.ProjectID), cacheKey, build.IsFeatureFlagOn(featureflags.HashCacheKeys)) goCloudURL, err := adapter.GetGoCloudURL(ctx, true) if err != nil { diff --git a/docs/configuration/advanced-configuration.md b/docs/configuration/advanced-configuration.md index 56663e83b11a42fc4a657e113f86c863540aa497..0a957e5e9945602bd8d96ad56fbdcf4250006a1a 100644 --- a/docs/configuration/advanced-configuration.md +++ b/docs/configuration/advanced-configuration.md @@ -1409,7 +1409,8 @@ credentials from the environment, you can define `AWS_ACCESS_KEY_ID` and {{< history >}} -- [Introduced](https://gitlab.com/gitlab-org/gitlab-runner/-/merge_requests/5751) in GitLab Runner v18.4.0. +- [Introduced](https://gitlab.com/gitlab-org/gitlab-runner/-/merge_requests/5751) in GitLab Runner 18.4.0. +- Object path in distributed caches [changed](https://gitlab.com/gitlab-org/gitlab-runner/-/merge_requests/6628) in GitLab Runner 19.0 to include a shard prefix when `FF_HASH_CACHE_KEYS` is enabled. {{< /history >}} @@ -1422,10 +1423,11 @@ the object in the storage bucket. If the sanitization changes the cache key, GitLab Runner logs this change. If GitLab Runner cannot sanitize the cache key, it also logs this, and does not use this specific cache. -When you turn on this feature flag, GitLab Runner hashes the cache key before using -it to build the path for the local cache artifact and the object in the remote storage -bucket. GitLab Runner does not sanitize the cache key. To help you understand which -cache key created a specific cache artifact, GitLab Runner attaches metadata to it: +When you turn on this feature flag, GitLab Runner hashes the cache key (SHA-256) +before using it to build the path for the local cache artifact and the object in +the remote storage bucket. GitLab Runner does not sanitize the cache key. To help +you understand which cache key created a specific cache artifact, GitLab Runner +attaches metadata to it: - For local cache artifacts, GitLab Runner places a `metadata.json` file next to the cache artifact `cache.zip`, with the following content: @@ -1439,6 +1441,36 @@ cache key created a specific cache artifact, GitLab Runner attaches metadata to [user-defined object metadata](https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingMetadata.html#UserMetadata) for AWS S3. +#### Distributed cache object path with `FF_HASH_CACHE_KEYS` + +In GitLab Runner 19.0 and later, when `FF_HASH_CACHE_KEYS` is enabled, +GitLab Runner inserts the first two hexadecimal characters of the SHA-256 hash +as a shard prefix in the distributed cache object path: + +```plaintext +[path/][runner//]project////cache.zip +``` + +For example: + +```plaintext +runner/abc123/project/42/d0/d03a852ba491ba611e907b1ef60ad5c4516a05b8f3aae6abb77f42bc60325aed/cache.zip +``` + +This distributes cache objects across 256 distinct object prefixes per project, +which prevents [Amazon S3 503 (Slow Down) responses](https://docs.aws.amazon.com/AmazonS3/latest/userguide/optimizing-performance.html) +when many parallel jobs access the cache at high request rates. + +> [!warning] +> Upgrading to GitLab Runner 19.0 is a breaking change if you use `FF_HASH_CACHE_KEYS`. +> If you already have `FF_HASH_CACHE_KEYS` enabled and upgrade to GitLab Runner 19.0 +> or later, the shard prefix changes the object path for all cache artifacts in +> distributed storage. Existing objects stored at the old path +> (`...//cache.zip`) become unreachable. Expect cache misses and cache +> artifacts rebuild on the first job run after upgrade. + +#### Cache key handling behavior summary + When you change `FF_HASH_CACHE_KEYS`, GitLab Runner ignores existing cache artifacts because hashing the cache key changes the cache artifact's name and location. This change applies in both directions, from `FF_HASH_CACHE_KEYS=true` to diff --git a/shells/abstract.go b/shells/abstract.go index a8b242addc87b063e2b98da5fee42f9870716409..70bae0e8748c95d39a9b068f6a923a1654eb0c15 100644 --- a/shells/abstract.go +++ b/shells/abstract.go @@ -379,7 +379,7 @@ func (b *AbstractShell) addExtractCacheCommand( // getCacheDownloadURLAndEnv will first try to generate the GoCloud URL if it's // available then fallback to a pre-signed URL. func getCacheDownloadURLAndEnv(ctx context.Context, build *common.Build, cacheKey string) ([]string, map[string]string, error) { - adapter := cache.GetAdapter(build.Runner.Cache, build.GetBuildTimeout(), build.Runner.ShortDescription(), fmt.Sprintf("%d", build.JobInfo.ProjectID), cacheKey) + adapter := cache.GetAdapter(build.Runner.Cache, build.GetBuildTimeout(), build.Runner.ShortDescription(), fmt.Sprintf("%d", build.JobInfo.ProjectID), cacheKey, build.IsFeatureFlagOn(featureflags.HashCacheKeys)) // Prefer Go Cloud URL if supported goCloudURL, err := adapter.GetGoCloudURL(ctx, false) @@ -1458,7 +1458,7 @@ func (b *AbstractShell) addCacheUploadCommand( // getCacheUploadURLAndEnv will first try to generate the GoCloud URL if it's // available then fallback to a pre-signed URL. func getCacheUploadURLAndEnv(ctx context.Context, build *common.Build, cacheKey string, metadata map[string]string) ([]string, map[string]string, error) { - adapter := cache.GetAdapter(build.Runner.Cache, build.GetBuildTimeout(), build.Runner.ShortDescription(), fmt.Sprintf("%d", build.JobInfo.ProjectID), cacheKey) + adapter := cache.GetAdapter(build.Runner.Cache, build.GetBuildTimeout(), build.Runner.ShortDescription(), fmt.Sprintf("%d", build.JobInfo.ProjectID), cacheKey, build.IsFeatureFlagOn(featureflags.HashCacheKeys)) // Prefer Go Cloud URL if supported goCloudURL, err := adapter.GetGoCloudURL(ctx, true) diff --git a/shells/abstract_test.go b/shells/abstract_test.go index de6342fb145831326c3ae5c8ce91e02266d6b178..153d2be42a63419f1b0a9ff6b6abff02693f6a6a 100644 --- a/shells/abstract_test.go +++ b/shells/abstract_test.go @@ -1642,6 +1642,22 @@ func getCacheKeyHasher(hash bool) func(string) string { } } +// getShardedObjectKey returns a function that, given a (hashed) cache key, +// returns the object path component used by GetAdapter. When sharded is true +// (i.e. FF_HASH_CACHE_KEYS is on), the first two hex characters are inserted +// as a prefix: "/". Otherwise the key is returned unchanged. +func getShardedObjectKey(sharded bool) func(string) string { + if !sharded { + return func(key string) string { return key } + } + return func(key string) string { + if len(key) < 2 { + return key + } + return key[:2] + "/" + key + } +} + func TestAbstractShell_extractCacheWithDefaultFallbackKey(t *testing.T) { const cacheEnvFile = "/some/path/to/runner-cache-env" @@ -1865,6 +1881,7 @@ func TestAbstractShell_extractCacheWithDefaultFallbackKey(t *testing.T) { for _, hashCacheKeys := range []bool{false, true} { hashed := getCacheKeyHasher(hashCacheKeys) + shardedObjectPath := getShardedObjectKey(hashCacheKeys) t.Run(fmt.Sprintf("%s:%t", featureflags.HashCacheKeys, hashCacheKeys), func(t *testing.T) { for tn, tc := range tests { @@ -1943,7 +1960,7 @@ func TestAbstractShell_extractCacheWithDefaultFallbackKey(t *testing.T) { "--timeout", "10", "--url", - fmt.Sprintf("test://download/project/1000/%s", expectedHashedCacheKey), + fmt.Sprintf("test://download/project/1000/%s", shardedObjectPath(expectedHashedCacheKey)), ).Once() } else { mockWriter.On("DotEnvVariables", "gitlab_runner_cache_env", mock.Anything).Return(cacheEnvFile).Once() @@ -1955,7 +1972,7 @@ func TestAbstractShell_extractCacheWithDefaultFallbackKey(t *testing.T) { "--timeout", "10", "--gocloud-url", - fmt.Sprintf("gocloud://test/project/1000/%s", expectedHashedCacheKey), + fmt.Sprintf("gocloud://test/project/1000/%s", shardedObjectPath(expectedHashedCacheKey)), "--env-file", cacheEnvFile, ).Once() } @@ -2081,6 +2098,7 @@ func TestAbstractShell_extractCacheWithMultipleFallbackKeys(t *testing.T) { for _, hashedCacheKey := range []bool{false, true} { hashed := getCacheKeyHasher(hashedCacheKey) + shardedObjectPath := getShardedObjectKey(hashedCacheKey) t.Run(fmt.Sprintf("%s:%t", featureflags.HashCacheKeys, hashedCacheKey), func(t *testing.T) { for tn, tc := range tests { @@ -2149,7 +2167,7 @@ func TestAbstractShell_extractCacheWithMultipleFallbackKeys(t *testing.T) { "--timeout", "10", "--url", - fmt.Sprintf("test://download/project/1000/%s", hashedCacheKey), + fmt.Sprintf("test://download/project/1000/%s", shardedObjectPath(hashedCacheKey)), ).Once() mockWriter.On("Noticef", "Successfully extracted cache").Once() mockWriter.On("Else").Once() @@ -2265,6 +2283,7 @@ func TestAbstractShell_extractCacheWithMultipleFallbackKeysWithCleanup(t *testin for _, hashedCacheKey := range []bool{false, true} { hashed := getCacheKeyHasher(hashedCacheKey) + shardedObjectPath := getShardedObjectKey(hashedCacheKey) t.Run(fmt.Sprintf("%s:%t", featureflags.HashCacheKeys, hashedCacheKey), func(t *testing.T) { for tn, tc := range tests { @@ -2337,7 +2356,7 @@ func TestAbstractShell_extractCacheWithMultipleFallbackKeysWithCleanup(t *testin "--timeout", "10", "--url", - fmt.Sprintf("test://download/project/1000/%s", hashedCacheKey), + fmt.Sprintf("test://download/project/1000/%s", shardedObjectPath(hashedCacheKey)), ).Once() mockWriter.On("Noticef", "Successfully extracted cache").Once() mockWriter.On("Else").Once()