Files
Ramkumar Chinchani 55b68228da 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>
2026-06-15 14:36:07 -07:00

503 lines
11 KiB
Go

package local
import (
"bufio"
"bytes"
"context"
"errors"
"io"
"net/http"
"os"
"path"
"sort"
"time"
"unicode/utf8"
storagedriver "github.com/distribution/distribution/v3/registry/storage/driver"
zerr "zotregistry.dev/zot/v2/errors"
storageConstants "zotregistry.dev/zot/v2/pkg/storage/constants"
"zotregistry.dev/zot/v2/pkg/test/inject"
)
type Driver struct {
commit bool
}
func New(commit bool) *Driver {
return &Driver{commit: commit}
}
func (driver *Driver) Name() string {
return storageConstants.LocalStorageDriverName
}
func (driver *Driver) EnsureDir(path string) error {
err := os.MkdirAll(path, storageConstants.DefaultDirPerms)
return driver.formatErr(err)
}
func (driver *Driver) DirExists(path string) bool {
if !utf8.ValidString(path) {
return false
}
fileInfo, err := os.Stat(path)
if err != nil {
// if os.Stat returns any error, fileInfo will be nil
// we can't check if the path is a directory using fileInfo if we received an error
// let's assume the directory doesn't exist in all error cases
// see possible errors http://man.he.net/man2/newfstatat
return false
}
if !fileInfo.IsDir() {
return false
}
return true
}
func (driver *Driver) Reader(path string, offset int64) (io.ReadCloser, error) {
file, err := os.OpenFile(path, os.O_RDONLY, storageConstants.DefaultFilePerms)
if err != nil {
if os.IsNotExist(err) {
return nil, storagedriver.PathNotFoundError{Path: path}
}
return nil, driver.formatErr(err)
}
seekPos, err := file.Seek(offset, io.SeekStart)
if err != nil {
file.Close()
return nil, driver.formatErr(err)
} else if seekPos < offset {
file.Close()
return nil, storagedriver.InvalidOffsetError{Path: path, Offset: offset}
}
return file, nil
}
func (driver *Driver) ReadFile(path string) ([]byte, error) {
reader, err := driver.Reader(path, 0)
if err != nil {
return nil, err
}
defer reader.Close()
buf, err := io.ReadAll(reader)
if err != nil {
return nil, driver.formatErr(err)
}
return buf, nil
}
func (driver *Driver) Delete(path string) error {
_, err := os.Stat(path)
if err != nil && !os.IsNotExist(err) {
return driver.formatErr(err)
} else if err != nil {
return storagedriver.PathNotFoundError{Path: path}
}
return os.RemoveAll(path)
}
func (driver *Driver) Stat(path string) (storagedriver.FileInfo, error) {
fi, err := os.Stat(path) //nolint: varnamelen
if err != nil {
if os.IsNotExist(err) {
return nil, storagedriver.PathNotFoundError{Path: path}
}
return nil, driver.formatErr(err)
}
return fileInfo{
path: path,
FileInfo: fi,
}, nil
}
func (driver *Driver) Writer(filepath string, append bool) (storagedriver.FileWriter, error) { //nolint:predeclared
if append {
_, err := os.Stat(filepath)
if err != nil {
if os.IsNotExist(err) {
return nil, storagedriver.PathNotFoundError{Path: filepath}
}
return nil, driver.formatErr(err)
}
}
parentDir := path.Dir(filepath)
if err := os.MkdirAll(parentDir, storageConstants.DefaultDirPerms); err != nil {
return nil, driver.formatErr(err)
}
file, err := os.OpenFile(filepath, os.O_WRONLY|os.O_CREATE, storageConstants.DefaultFilePerms)
if err != nil {
return nil, driver.formatErr(err)
}
var offset int64
if !append {
err := file.Truncate(0)
if err != nil {
file.Close()
return nil, driver.formatErr(err)
}
} else {
n, err := file.Seek(0, io.SeekEnd) //nolint: varnamelen
if err != nil {
file.Close()
return nil, driver.formatErr(err)
}
offset = n
}
return newFileWriter(file, offset, driver.commit), nil
}
func (driver *Driver) WriteFile(filepath string, content []byte) (int, error) {
writer, err := driver.Writer(filepath, false)
if err != nil {
return -1, err
}
nbytes, err := io.Copy(writer, bytes.NewReader(content))
if err != nil {
_ = writer.Cancel(context.Background())
return -1, driver.formatErr(err)
}
return int(nbytes), writer.Close()
}
func (driver *Driver) Walk(path string, walkFn storagedriver.WalkFn) error {
children, err := driver.List(path)
if err != nil {
return err
}
sort.Stable(sort.StringSlice(children))
for _, child := range children {
// Calling driver.Stat for every entry is quite
// expensive when running against backends with a slow Stat
// implementation, such as s3. This is very likely a serious
// performance bottleneck.
fileInfo, err := driver.Stat(child)
if err != nil {
switch errors.As(err, &storagedriver.PathNotFoundError{}) {
case true:
// repository was removed in between listing and enumeration. Ignore it.
continue
default:
return err
}
}
err = walkFn(fileInfo)
if err == nil && fileInfo.IsDir() { //nolint: gocritic
if err := driver.Walk(child, walkFn); err != nil {
return err
}
} else if errors.Is(err, storagedriver.ErrSkipDir) {
// Stop iteration if it's a file, otherwise noop if it's a directory
if !fileInfo.IsDir() {
return nil
}
} else if err != nil {
return driver.formatErr(err)
}
}
return nil
}
func (driver *Driver) List(fullpath string) ([]string, error) {
entries, err := os.ReadDir(fullpath)
if err != nil {
if os.IsNotExist(err) {
return nil, storagedriver.PathNotFoundError{Path: fullpath}
}
return nil, driver.formatErr(err)
}
keys := make([]string, 0, len(entries))
for _, entry := range entries {
keys = append(keys, path.Join(fullpath, entry.Name()))
}
return keys, nil
}
func (driver *Driver) Move(sourcePath string, destPath string) error {
if _, err := os.Stat(sourcePath); os.IsNotExist(err) {
return storagedriver.PathNotFoundError{Path: sourcePath}
}
if err := os.MkdirAll(path.Dir(destPath), storageConstants.DefaultDirPerms); err != nil {
return driver.formatErr(err)
}
// Use renameReplace so an existing destination is replaced (POSIX rename); on Windows,
// os.Rename does not overwrite an existing file — see driver_unix.go / driver_windows.go.
return driver.formatErr(renameReplace(sourcePath, destPath, driver.commit))
}
func (driver *Driver) SameFile(path1, path2 string) bool {
file1, err := os.Stat(path1)
if err != nil {
return false
}
file2, err := os.Stat(path2)
if err != nil {
return false
}
return os.SameFile(file1, file2)
}
func (driver *Driver) Link(src, dest string) error {
if err := os.Remove(dest); err != nil && !os.IsNotExist(err) {
return err
}
if err := os.Link(src, dest); err != nil {
return driver.formatErr(err)
}
/* also update the modtime, so that gc won't remove recently linked blobs
otherwise ifBlobOlderThan(gcDelay) will return the modtime of the inode */
currentTime := time.Now() //nolint: gosmopolitan
if err := os.Chtimes(dest, currentTime, currentTime); err != nil {
return driver.formatErr(err)
}
return nil
}
func (driver *Driver) RedirectURL(_ *http.Request, _ string) (string, error) {
return "", nil
}
func (driver *Driver) formatErr(err error) error {
switch actual := err.(type) { //nolint: errorlint
case nil:
return nil
case storagedriver.PathNotFoundError:
actual.DriverName = driver.Name()
return actual
case storagedriver.InvalidPathError:
actual.DriverName = driver.Name()
return actual
case storagedriver.InvalidOffsetError:
actual.DriverName = driver.Name()
return actual
default:
storageError := storagedriver.Error{
DriverName: driver.Name(),
Detail: err,
}
return storageError
}
}
type fileInfo struct {
os.FileInfo
path string
}
// asserts fileInfo implements storagedriver.FileInfo.
var _ storagedriver.FileInfo = fileInfo{}
// Path provides the full path of the target of this file info.
func (fi fileInfo) Path() string {
return fi.path
}
// Size returns current length in bytes of the file. The return value can
// be used to write to the end of the file at path. The value is
// meaningless if IsDir returns true.
func (fi fileInfo) Size() int64 {
if fi.IsDir() {
return 0
}
return fi.FileInfo.Size()
}
// ModTime returns the modification time for the file. For backends that
// don't have a modification time, the creation time should be returned.
func (fi fileInfo) ModTime() time.Time {
return fi.FileInfo.ModTime()
}
// IsDir returns true if the path is a directory.
func (fi fileInfo) IsDir() bool {
return fi.FileInfo.IsDir()
}
// fileInternal defines the interface for file operations (internal).
type fileInternal interface {
io.WriteCloser
Sync() error
Name() string
}
type fileWriter struct {
file fileInternal
size int64
bw *bufio.Writer
closed bool
committed bool
cancelled bool
commit bool
}
func newFileWriter(file fileInternal, size int64, commit bool) *fileWriter {
return &fileWriter{
file: file,
size: size,
commit: commit,
bw: bufio.NewWriter(file),
}
}
// NewFileWriter creates a new fileWriter for testing purposes.
func NewFileWriter(file fileInternal, size int64, commit bool) *fileWriter {
return newFileWriter(file, size, commit)
}
func (fw *fileWriter) Write(buf []byte) (int, error) {
//nolint: gocritic
if fw.closed {
return 0, zerr.ErrFileAlreadyClosed
} else if fw.committed {
return 0, zerr.ErrFileAlreadyCommitted
} else if fw.cancelled {
return 0, zerr.ErrFileAlreadyCancelled
}
n, err := fw.bw.Write(buf)
fw.size += int64(n)
return n, err
}
func (fw *fileWriter) Size() int64 {
return fw.size
}
func (fw *fileWriter) Close() error {
if fw.closed {
return zerr.ErrFileAlreadyClosed
}
if err := fw.bw.Flush(); err != nil {
return err
}
if fw.commit {
if err := inject.Error(fw.file.Sync()); err != nil {
return err
}
}
if err := inject.Error(fw.file.Close()); err != nil {
return err
}
fw.closed = true
return nil
}
func (fw *fileWriter) Cancel(_ context.Context) error {
if fw.closed {
return zerr.ErrFileAlreadyClosed
}
fw.cancelled = true
fw.file.Close()
return os.Remove(fw.file.Name())
}
func (fw *fileWriter) Commit(_ context.Context) error {
//nolint: gocritic
if fw.closed {
return zerr.ErrFileAlreadyClosed
} else if fw.committed {
return zerr.ErrFileAlreadyCommitted
} else if fw.cancelled {
return zerr.ErrFileAlreadyCancelled
}
if err := fw.bw.Flush(); err != nil {
return err
}
if fw.commit {
if err := fw.file.Sync(); err != nil {
return err
}
}
fw.committed = true
return nil
}
func ValidateHardLink(rootDir string) error {
if err := os.MkdirAll(rootDir, storageConstants.DefaultDirPerms); err != nil {
return err
}
err := os.WriteFile(path.Join(rootDir, "hardlinkcheck.txt"),
[]byte("check whether hardlinks work on filesystem"), storageConstants.DefaultFilePerms)
if err != nil {
return err
}
err = os.Link(path.Join(rootDir, "hardlinkcheck.txt"), path.Join(rootDir, "duphardlinkcheck.txt"))
if err != nil {
// Remove hardlinkcheck.txt if hardlink fails
zerr := os.RemoveAll(path.Join(rootDir, "hardlinkcheck.txt"))
if zerr != nil {
return zerr
}
return err
}
err = os.RemoveAll(path.Join(rootDir, "hardlinkcheck.txt"))
if err != nil {
return err
}
return os.RemoveAll(path.Join(rootDir, "duphardlinkcheck.txt"))
}