mirror of
https://github.com/project-zot/zot.git
synced 2026-06-18 05:28:07 +08:00
feat(storage): redirect blob pulls to backend URLs (#4092)
* feat(storage): redirect blob pulls to backend URLs * fix: rebase conflicts Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * refactor: rename redirect field Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * test: relax brittle TestPeriodicGC substore log assertion Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * feat(storage): improve blob redirect config handling and validation Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * fix(storage): address PR review feedback for blob redirect Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * feat(storage): apply latest PR review fixes for blob redirect Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * test: fix blob redirect and verify test regressions Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * fix(storage): enforce redirectBlobURL validation and add redirect tests Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * fix(storage): fix err113/noctx lint errors in storage driver tests - Replace httptest.NewRequest with httptest.NewRequestWithContext in s3, gcs, and imagestore driver tests (noctx) - Replace dynamic errors.New in s3 driver test with a package-level static sentinel error (err113) Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * test(storage): use temp dirs in imagestore redirect tests Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * fix: handle ranged blob redirects and add regression tests Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * fix: validate blob digest consistently in GetBlob Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * test: fix GetBlobPartialFn mock return values for range requests The test 'does not redirect ranged blob requests' was failing because the mock was returning incorrect length values. For a range request 'bytes=0-0' (1 byte), it was returning 4 bytes, which caused a length mismatch check in GetBlob to return HTTP 500. Fix the mock to dynamically calculate the correct length: to - from + 1 Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * fix(storage): preserve signed URL bytes in normalizeBlobRedirectURL Preserve the original URL bytes from backend storage drivers (important for signed/presigned URLs) while only lowercasing the scheme prefix. URL re-serialization via net/url can invalidate signatures through path escaping or canonicalization. Add regression tests covering signed URL query parameters and mixed-case scheme handling. Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * fix(storage): address PR review comments for blob redirect - Return signed redirect URLs unchanged; validate scheme/CRLF/host only, no URL normalization that would corrupt signed URL bytes - Add inline comments for all non-obvious decisions: range bypass, soft fallback on invalid URL, local driver empty return, subpath resolution, redirectBlobURL config constraint on local/empty driver - Expand TestNormalizeBlobRedirectURL to cover allowed schemes (http/https), parse failure, missing host, and CRLF injection cases - Add TestIsBlobRedirectEnabled covering subpath-only enablement with default store disabled Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * test(storage): address remaining blob redirect review comments Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * fix: gofumpt formatting in routes_test.go Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> --------- Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> Co-authored-by: Akash Kumar <meakash7902@gmail.com>
This commit is contained in:
Ramkumar Chinchani
committed by
GitHub
GitHub
parent
95ce68ccb0
commit
55b68228da
@@ -618,6 +618,16 @@ func validateExtensionsConfig(cfg *config.Config, logger zlog.Logger) error {
|
||||
func validateStorageConfigSection(
|
||||
cfg *config.Config, logger zlog.Logger, storageConfig config.GlobalStorageConfig,
|
||||
) error {
|
||||
// Redirect mode requires a backend capable of issuing external signed URLs.
|
||||
// With local storage there is no target URL to redirect clients to.
|
||||
if storageConfig.RedirectBlobURL && len(storageConfig.StorageDriver) == 0 {
|
||||
msg := "invalid storage config, redirectBlobURL is supported only for s3/gcs storage"
|
||||
logger.Error().Err(zerr.ErrBadConfig).Bool("redirectBlobURL", storageConfig.RedirectBlobURL).
|
||||
Str("storageDriver", storageConstants.LocalStorageDriverName).Msg(msg)
|
||||
|
||||
return fmt.Errorf("%w: %s", zerr.ErrBadConfig, msg)
|
||||
}
|
||||
|
||||
if len(storageConfig.StorageDriver) != 0 {
|
||||
// enforce s3/gcs driver in case of using storage driver
|
||||
if storageConfig.StorageDriver["name"] != storageConstants.S3StorageDriverName &&
|
||||
@@ -641,6 +651,17 @@ func validateStorageConfigSection(
|
||||
// enforce s3/gcs driver on subpaths in case of using storage driver
|
||||
if len(storageConfig.SubPaths) > 0 {
|
||||
for route, subStorageConfig := range storageConfig.SubPaths {
|
||||
// Apply the same redirect precondition per subpath because subpaths can
|
||||
// override driver config independently from the default store.
|
||||
if subStorageConfig.RedirectBlobURL && len(subStorageConfig.StorageDriver) == 0 {
|
||||
msg := "invalid storage config, redirectBlobURL is supported only for s3/gcs storage"
|
||||
logger.Error().Err(zerr.ErrBadConfig).Str("subpath", route).
|
||||
Bool("redirectBlobURL", subStorageConfig.RedirectBlobURL).
|
||||
Str("storageDriver", storageConstants.LocalStorageDriverName).Msg(msg)
|
||||
|
||||
return fmt.Errorf("%w: %s", zerr.ErrBadConfig, msg)
|
||||
}
|
||||
|
||||
if len(subStorageConfig.StorageDriver) != 0 {
|
||||
if subStorageConfig.StorageDriver["name"] != storageConstants.S3StorageDriverName &&
|
||||
subStorageConfig.StorageDriver["name"] != storageConstants.GCSStorageDriverName {
|
||||
|
||||
@@ -1570,6 +1570,67 @@ storage:
|
||||
"invalid storage config, default storage root directory cannot be inside substore (route: /a) root directory")
|
||||
})
|
||||
|
||||
Convey("Test redirectBlobURL error for local storage", t, func(c C) {
|
||||
// Redirect requires a remote driver that can generate an external URL.
|
||||
content := `{"storage":{"rootDirectory":"/tmp/zot","redirectBlobURL":true,
|
||||
"subPaths": {"/a": {"rootDirectory": "/tmp/zot-a","redirectBlobURL":true}}},
|
||||
"http":{"address":"127.0.0.1","port":"8080"}}`
|
||||
|
||||
tmpfile := MakeTempFileWithContent(t, "zot-test.json", content)
|
||||
|
||||
cfg := config.New()
|
||||
err := cli.LoadConfiguration(cfg, tmpfile)
|
||||
So(err, ShouldNotBeNil)
|
||||
So(err.Error(), ShouldContainSubstring,
|
||||
"invalid storage config, redirectBlobURL is supported only for s3/gcs storage")
|
||||
})
|
||||
|
||||
Convey("Test redirectBlobURL error for empty storageDriver map", t, func(c C) {
|
||||
// An empty driver map is equivalent to local storage for redirect validation.
|
||||
content := `{"storage":{"rootDirectory":"/tmp/zot","redirectBlobURL":true,
|
||||
"storageDriver": {},
|
||||
"subPaths": {"/a": {"rootDirectory": "/tmp/zot-a","redirectBlobURL":true,"storageDriver": {}}}},
|
||||
"http":{"address":"127.0.0.1","port":"8080"}}`
|
||||
|
||||
tmpfile := MakeTempFileWithContent(t, "zot-test-empty-map.json", content)
|
||||
|
||||
cfg := config.New()
|
||||
err := cli.LoadConfiguration(cfg, tmpfile)
|
||||
So(err, ShouldNotBeNil)
|
||||
So(err.Error(), ShouldContainSubstring,
|
||||
"invalid storage config, redirectBlobURL is supported only for s3/gcs storage")
|
||||
})
|
||||
|
||||
Convey("Test redirectBlobURL error for subpath local storage", t, func(c C) {
|
||||
// Global storage has no redirect; only the subpath enables it without a driver.
|
||||
content := `{"storage":{"rootDirectory":"/tmp/zot",
|
||||
"subPaths": {"/a": {"rootDirectory": "/tmp/zot-a","redirectBlobURL":true}}},
|
||||
"http":{"address":"127.0.0.1","port":"8080"}}`
|
||||
|
||||
tmpfile := MakeTempFileWithContent(t, "zot-test-subpath-local.json", content)
|
||||
|
||||
cfg := config.New()
|
||||
err := cli.LoadConfiguration(cfg, tmpfile)
|
||||
So(err, ShouldNotBeNil)
|
||||
So(err.Error(), ShouldContainSubstring,
|
||||
"invalid storage config, redirectBlobURL is supported only for s3/gcs storage")
|
||||
})
|
||||
|
||||
Convey("Test redirectBlobURL error for subpath empty storageDriver map", t, func(c C) {
|
||||
// Global storage has no redirect; subpath has an empty driver map.
|
||||
content := `{"storage":{"rootDirectory":"/tmp/zot",
|
||||
"subPaths": {"/a": {"rootDirectory": "/tmp/zot-a","redirectBlobURL":true,"storageDriver": {}}}},
|
||||
"http":{"address":"127.0.0.1","port":"8080"}}`
|
||||
|
||||
tmpfile := MakeTempFileWithContent(t, "zot-test-subpath-empty-map.json", content)
|
||||
|
||||
cfg := config.New()
|
||||
err := cli.LoadConfiguration(cfg, tmpfile)
|
||||
So(err, ShouldNotBeNil)
|
||||
So(err.Error(), ShouldContainSubstring,
|
||||
"invalid storage config, redirectBlobURL is supported only for s3/gcs storage")
|
||||
})
|
||||
|
||||
Convey("Test verify w/ authorization and w/o authentication", t, func(c C) {
|
||||
content := `{"storage":{"rootDirectory":"/tmp/zot"},
|
||||
"http":{"address":"127.0.0.1","port":"8080","realm":"zot",
|
||||
|
||||
Reference in New Issue
Block a user