Relay cache-miss blob Range requests to the upstream when
proxy.rangepassthrough is enabled, without populating the blob cache.
Preserve upstream 200, 206 and 416 responses while keeping ordinary GET
cache fills and cache-hit handling unchanged.
Reuse the authenticated client, abort interrupted streams, and exclude
416 responses from blob transfer metrics and pull notifications.
Document the option and cover the HTTP and cache behavior in tests.
Signed-off-by: jeongho-kim <jeongho.kim@toss.im>
Cancel removes the upload directory, but the handler then closes the
writer, and Close stores the hash state again. That recreates the
upload directory without a startedat file, so the upload purger can
never remove it, and Close reports "already closed". Skip storing the
hash state once the upload has been cancelled.
Signed-off-by: YoungjuKim <fjvbn2003@gmail.com>
Run make test across Ubuntu and Windows with the latest Go 1.26 and 1.27
patch releases. This includes filesystem driver tests, which the existing
coverage target excludes, and catches Windows regressions before release.
Keep coverage and the Docker-backed S3 and Azure tests in a Linux-only job,
and require both test jobs before building artifacts.
Make the file health checks portable by using temporary directories and
closing the marker file before removing it. Check removal errors so a
failed deletion is reported directly.
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
Since v3.1.0, the filesystem driver has tried to rename an open temporary
file when creating an upload session, causing every blob upload on Windows
to fail with HTTP 500. Closing the file exposes a second failure when the
driver tries to sync the parent directory.
Close the temporary file before renaming it and make directory syncing a
no-op on Windows. Preserve file syncing, replacement semantics, and the
existing writer lifecycle contract. Return slash-separated storage keys
from List so callers can use them on every platform.
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
PathNotFoundError previously printed a storage-driver-relative path that callers treated as an absolute OS path. Include the filesystem driver's rootdirectory in the error message so the configured storage root is visible. Fixes#1793.
Signed-off-by: yusheng.wang <yusheng.wang@chaitin.com>
When the final path component resolves to a directory, `q = q[i+1:]` with `i == -1` leaves `q` unchanged and recurses into the child -- going one level deeper than intended. That is normally harmless, because the repeated lookup finds nothing and returns the directory it is already in, but it returns the wrong node when that directory has a child with the same name.
Guard with `i >= 0` so the found directory is returned directly.
Fixes an intermittent flake (~3-7% of runs) in `TestDeleteOnlyDeletesSubpaths`, which constructs `dirname/dirname` to deliberately trigger the ambiguous case.
Assisted-By: "claude my eyes right out"
Signed-off-by: Tianon Gravi <admwiggin@gmail.com>
A repository name whose path is longer than 255 characters fails
reference.WithName in the dispatcher. That branch appended a bare
distribution.ErrRepositoryNameInvalid to context.Errors, and since it
is not an errcode.ErrorCoder, errcode.ServeJSON answered 500 UNKNOWN.
Wrap it in errcode.ErrorCodeNameInvalid, as the Repository() branch
right below already does for the same error type.
Since a811c1bb moved the automatic error response into a deferred call
at the top of the dispatcher, the early returns for an invalid name, a
failed Repository() call and a failed repository middleware have each
written the error envelope twice: net/http drops the second WriteHeader
as superfluous and the body ends up with two JSON documents. Remove the
explicit ServeJSON calls and leave the response to the deferred handler.
The Repository() switch gains a default case, so an error of any other
type still gets a 500, now with an UNKNOWN entry instead of an empty
envelope.
Fixes#4699
Assisted-by: Claude Opus 5.5 (via Claude Code)
Signed-off-by: somaz <genius5711@gmail.com>
The logstash formatter was originally added in a75f0f2 using the
formatter that was provided by logrus at the time.
Commit 1d6c1a6 switched to logrus-logstash-hook after the formatter
was removed from logrus. However, it instantiated LogstashFormatter
directly instead of using DefaultFormatter, so the Logstash field
mappings and default fields provided by that package are not used.
The only additional formatting currently provided by the wrapper is
copying caller information to "function" and "file". Logrus'
JSONFormatter supports configuring these field names directly and
already formats the file as "<file>:<line>".
Replace the external formatter with JSONFormatter and map the caller
function field to preserve the current output.
This removes the logrus-logstash-hook dependency without intentionally
changing the logstash formatter output.
Refs: a75f0f26f7
Refs: 1d6c1a6468
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Motivation:
directoryDiff walks up S3 object-key path components while Walk infers
directories from a sorted key listing (used by, among others, the
background PurgeUploads upload-cleanup job). It used filepath.Dir from
path/filepath, which is OS-dependent. S3 keys always use "/" as a
separator regardless of host OS, but on Windows filepath.Dir/Clean
normalizes to "\" as the separator. Since the loop's only unconditional
exit check is `parent == "/"`, once repeated filepath.Dir calls converge
to the fixed point "\", the check never fires, none of the other break
conditions match either (they compare against prev, which still uses
"/"), and the loop runs forever, endlessly appending to paths. This
matches a reported Windows crash where a heap profile attributed
essentially all of a ~28 GB allocation spike to this append call during
a PurgeUploads walk of a ~2 GB bucket.
Approach:
Swap the path/filepath import for path and use path.Dir, which is pure
string manipulation with no OS-specific behavior, matching how S3 keys
actually behave everywhere. This is a no-op change on Unix-like
platforms, where filepath.Dir already behaves identically to path.Dir
for forward-slash-only input. A pagination test added earlier
(registry/storage/catalog_pagination_test.go's directoryDiffLocal, a
hand-written mirror of this function) already independently used
path.Dir, corroborating that this is the correct choice.
Validation:
Ran on macOS/arm64:
go build ./registry/storage/driver/s3-aws/...
go test ./registry/storage/driver/s3-aws/... -run 'TestDirectoryDiff|TestIsSubpath' -v
golangci-lint run ./registry/storage/driver/s3-aws/...
All pass, including a new TestDirectoryDiff covering the function's
documented examples. Note: macOS and Linux cannot reproduce the
Windows-specific divergence that causes the reported crash, because
their path separator is already "/", so filepath.Dir and path.Dir are
behaviorally identical there and the added/existing tests pass
identically before and after this change on this platform. Correctness
for Windows rests on documented, deterministic Go stdlib semantics
(path/filepath is OS-dependent; path is not) rather than an executed
repro of the crash, and was independently re-derived by reading the Go
stdlib source (internal/filepathlite and path_windows.go) rather than
taken on faith. The live S3 integration tests in this package
(TestWalk, TestEmptyRootList, etc.) require AWS/S3 credentials and were
not run.
Report: https://github.com/distribution/distribution/issues/4949
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Assisted-by: claude-sonnet-5 (via Claude Code)
`registry/handlers/blobupload.go`: `PutBlobUploadComplete` never validated `Content-Range` contiguity on the final chunked-upload PUT, so an out-of-order last chunk fell through to digest validation and returned 400 instead of the spec-mandated 416. d7a2b14489 ("add content range handling in patch blob") added this contiguity check to `PatchBlobData` but never extended it to the PUT completion path, so this reuses the same check there.
`registry/handlers/manifests.go`: `manifestDispatcher` silently treated any reference containing `:` that failed `digest.Parse` as a tag name, so a malformed digest reference (such as `sha256:baddigeststring`) reached the tag-based storage path and produced a 500 from an invalid filesystem path instead of a clean 400. Reject it as `ErrorCodeDigestInvalid` before the tag fallback instead.
Verified against the OCI distribution-spec conformance suite -- the three previously-failing tests (chunked out-of-order + put chunk, invalid-digest-format manifest-put, invalid-digest-format manifest-get) now pass; 779 pass / 6 skip / 4 disabled / 0 fail out of 789 total.
Assisted-By: "claude my eyes right out"
Co-Authored-By: Tianon Gravi <admwiggin@gmail.com>
Signed-off-by: Tianon Gravi <tianon.gravi@docker.com>
Hardcoded :5000/:5001 collide with a local dev registry and make
TestGracefulShutdown / cipher-suite tests talk to the wrong process.
Also fix unbuffered nil errchan in the TLS tests.
Fixes: #4928
Signed-off-by: Dean Chen <862469039@qq.com>
Reported in #4903: with the S3 driver, paging /v2/_catalog silently skips a
repository whose name is a prefix of another's.
The S3 Walk lists keys in byte order (where '/' is 0x2F), but the catalog
filter in handleRepository compared the page cursor with lessPath, which
sorts '/' first. For a prefix pair like ".../math" / ".../math-voila" (next
byte '-' is 0x2D, below '/') the orders disagree, so a page ending on the
longer name drops the shorter one on every later page.
The driver already positions the walk after `last` via the start-after hint,
so the extra lessPath check is what breaks pagination. Drop it and only
de-duplicate the boundary repo. Each driver then stays self-consistent: byte
order for S3, component-wise for the filesystem/inmemory WalkFallback.
S3-only (WalkFallback trims the hint per level). Regressed in 3.0.0
(cf81f67a, e22f7cbc); 2.8.x is fine. Adds a regression test replaying the S3
byte-order Walk that fails before and passes after.
Fixes#4903
Signed-off-by: Dustin Kaiser <8209087+mrnicegyu11@users.noreply.github.com>
The S3 driver's Walk used a bare strings.HasPrefix to decide whether a
walkInfo was under the last ErrSkipDir directory. Without a trailing
"/" on the parent, a sibling whose name starts with the skipped
directory's name (e.g. "0.1.20" after "0.1.2") falsely matched and was
skipped, so its tag dir was never emitted to handleTag and the tag was
omitted from /v2/<name>/tags/list.
Replace the prefix check with isSubpath, which appends "/" to the
parent so only true descendants match. Add a unit test pinning the
sibling-with-lexical-prefix case.
The bug is S3-only because the filesystem and inmemory drivers list
directories recursively via WalkFallback and don't use this skip
mechanism. It surfaced in 3.1.0 when the tags handler switched from
tagService.All() (driver List with delimiter) to tagService.List()
(driver Walk) for pagination support.
Fixes#4891
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
Write PutContent objects as block blobs so Azure retries overwrite small objects instead of appending duplicate link contents. Delete legacy append-blob objects before uploading because Azure cannot replace an AppendBlob with a BlockBlob in place. Preserve zero-byte append compatibility and add Azure regression coverage for link-style content, legacy migration, and concurrent same-path writes.
Signed-off-by: Baptiste Girard-Carrabin <baptiste.girardcarrabin@datadoghq.com>
This PR changes the code based on the `go fix` suggestions
to modernize the codebase and keep up with the latest Go features.
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
Currently, the Azure driver always creates blobs as an AppendBlob, but in previous versions, it used to create them as BlockBlobs.
There is migration logic to handle this, but it's currently inversed, so if the blob exists as a BlockBlob, we don't delete it, and get an error back from Azure:
```
RESPONSE 409: 409 The blob type is invalid for this operation.
ERROR CODE: InvalidBlobType
```
Since the check is inversed, this also means that any operation against AppendBlobs does an extra delete -> create for no reason.
Signed-off-by: David Marby <david@dmarby.se>
PurgeUploads' Walk callback split the visited path with path.Split and
indexed file[0] immediately. path.Split returns an empty basename for
paths that end in a trailing slash - in practice this happens when an
S3 driver surfaces a bare directory (common prefix) with an empty
Key. Indexing a zero-length string then panics with
'index out of range [0] with length 0' and takes down the whole
PurgeUploads goroutine (#4713).
Guard the length before touching file[0] so a trailing-slash /
empty-basename entry is simply skipped as 'not a reserved directory',
which matches what the branch was trying to do anyway. Runtime
behaviour for every non-empty entry is unchanged.
Closes#4713
Signed-off-by: SAY-5 <SAY-5@users.noreply.github.com>
PR #4353 made MaxTags (default 1000) a hard ceiling on the `n` query
parameter — anything larger and the handler returns 400
PAGINATION_NUMBER_INVALID before the request ever reaches storage or
the proxy tag service. That broke clients like Renovate which use
n=10000 against pull-through caches. #4846 fixed a related 500 in
proxy mode but not this 400, so users reported the regression still
persisted.
The OCI distribution-spec describes pagination differently: a server
MAY return fewer than `n` results "when the total number of tags
attached to the repository is less than <int> or a Link header is
provided" — otherwise it MUST include `<int>` results. In other
words, the right answer for "client asked for more than we'll serve"
is `maxtags` results plus a Link header, not a rejection.
PAGINATION_NUMBER_INVALID isn't among the 14 error codes the spec
defines, either.
Drop the oversized-n rejection and clamp to MaxTags instead; the
existing Link-header path already handles continuation correctly.
Malformed (non-integer) and negative `n` values keep returning 400,
since the spec defines `n` as a non-negative integer and those
requests are genuinely invalid.
Verified end-to-end against registry-1.docker.io in proxy mode:
n=10000 now returns the tag list (or a clamped page with Link)
instead of 400. Also restores pre-3.1.0 behavior for Renovate-style
clients without needing proxy-specific logic.
Spec reference:
https://github.com/opencontainers/distribution-spec/blob/main/spec.md#listing-tags
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
jeongho-kim