Define the referrers API responses for two edge cases #58
Conversation
There was a problem hiding this comment.
Thanks @lahirurd,
This is great to help with consistency and clarity.
A few clarity suggests and a question on the wording.
manifest-referrers-api.md
Outdated
| @@ -151,6 +151,10 @@ GET /oras/artifacts/v1/{repository}/manifests/{digest}/referrers?n=10&artifactTy | |||
| GET /oras/artifacts/v1/net-monitor/manifests/sha256:3c3a4604a545cdc127456d94e421cd355bca5b528f4a9c1905b15da2eb4a4c6b/referrers?n=10&artifactType=application%2Fvnd.org.cncf.notary.v2 | |||
| ``` | |||
|
|
|||
| ### Error Handling | |||
|
|
|||
| If the `subject` identified by the digest does not exist in the repository, the registry MUST return a `404` response with the error code `MANIFEST_UNKNOWN`. | |||
There was a problem hiding this comment.
I believe you mean subject here is what's passed as the manifest/. However, using subject in this context is a bit confusing as it's technically the subject of the referred artifacts, but we don't use 'subjectas the parameter name in the referrers examples. We can also remove some of the wording around the repository, as the/referrers` API is already scoped to a repository.
If the digest does not exist, the registry MUST return a 404 response with the error code MANIFEST_UNKNOWN.
There was a problem hiding this comment.
The reason I used subject there was for consistency, because we currently define the response with
- `references`: A list of [artifact descriptors][descriptor] that reference the given `subject`.
Should I change both?
If we were to word this as "If the <something> does not exist...", I'd rather <something> be "manifest" instead of "digest", since the API is scoped to a manifest. So I'd be happy with "If the manifest does not exist...".
I added "in the repository" there to remove possible ambiguity that could come up in future cross repo reference scenarios (e.g. issue #26). However if we consider the repository+manifest params in the call strictly scoping to a manifest (which is what we're currently doing), I agree this it isn't required.
There was a problem hiding this comment.
Good catch:
I like your suggestion, and also like Sajay's point of clarity to suggest MANIFEST_SUBJECT_UNKNOWN as a bit more clarity. I do think we should add the scope of repository, as it's not really accurate to say the manifest is unknown in the registry, as it may very well be in another repository for the dev image that hasn't yet been promoted to the prod repository.
What about this tweaking:
If the manifest does not exist in the repository, the response MUST return a 404 response with the error code MANIFEST_SUBJECT_UNKNOWN
There was a problem hiding this comment.
I can add back the repository-scoping, but I believe MANIFEST_UNKNOWN is the right response for the referrers api. Sajay also agrees here. So it would be
If the manifest does not exist in the repository, the registry MUST return a
404response with the error codeMANIFEST_UNKNOWN.
|
@SteveLasker @michaelb990 any concern merging this in? |
|
Agreed to return an empty list if the digest is valid, and no artifacts exist that reference that digest. |
1. When the given subject has no referencing artifacts. 2. When the given subject does not exist in the repository. Signed-off-by: Lahiru Dissanayake <[email protected]>
lahirurd
force-pushed
the
error-handling
branch
from
6054c07 to
5abbf6d
Compare
|
LGTM. I'm ok merging this since the referrers API is now expected to handle the non-existing manifest case for the current draft. This also is kind of off since the URI path is like a resource and you are able to get a non-404 from path under a resource(manifest) that might not exist. The supporting factor is that we are most likely to move to the extension model and move the digest to a parameter to support this kind of behavior in draft2 or vnext most likely. We also might need to update oras-project/distribution once this gets merged. /cc @avtakkar |
This defines the response for the following cases:
Returning an error code for the second scenario makes the api's response after the subject is deleted, consistent between registries that do signature clean up and those that do not. It also makes the response deterministic in registries that clean up signatures asynchronously.
Resolves #37