-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Internal Server Error when getting blobs as manifests #3122
Comments
I think this is caused by a fix for a bug introduced in 2.1.0 https://github.com/distribution/distribution/blob/main/registry/storage/registry.go#L222 I've looked for an issue in the past, but I couldn't find anything. From reading that comment, it seems like that 2.1.0 was linking manifests into |
Thanks for finding the root cause! In my PR I check that what was retrieved is a manifest, but this would suppress a "correct" 500 error if a manifest was corrupted in storage and return a 404 instead, which is undesirable. I wonder if the same check could be applied, but only to objects retrieved by the workaround (https://github.com/distribution/distribution/blob/main/registry/storage/registry.go#L222). Meaning we could continue to 200/404/500 as intended for manifests retrieved properly, but we would not be able to 500 a corrupt manifest retrieved using this workaround. |
I'm curious if we could remove the workaround. It's been six years since 2.1.0 was released and 2.1.1 was released the next day, resolving the issue with this PR: 44c7fb9 I can't imagine there are many, if any, manifests that are still in use that were affected by this issue. |
I'd support that. It would definitely need a release note. I'll also do some work to check the https://icr.io storage and see if we have any. |
I checked and we never ran 2.1.0. I now also have strong doubts that any existing registries ever ran that version. |
Looks like we started with v2.3.1, so we shouldn't have been affected either. |
I ran into this too. It really seems like incorrect behavior - the registry should return 404 in these cases. It triggered our alerting because 500 errors are usually a sign that something's wrong. |
Should we close this @brackendawson now that #3365 has been merged? |
I think so, yes. Fixed in 3.0 |
When you get a manifest by digest, but use the digest of an image config or layer instead of the manifest, you get a 500 error.
Version info
Steps to reproduce
curl http://<registry>/v2/<repo>/manifests/sha256:<digest_of_config>
What happened
I get a 500 error with the digest of layers and the image config.
If I make up a digest I get 404, if I use the manifest's digest I get 200 and the manifest.
Console output
Registry logs
What I expected
Requesting the image config digest or a layer digest as a manifest should 404 because there is no manifest of that digest.
The text was updated successfully, but these errors were encountered: