-
Notifications
You must be signed in to change notification settings - Fork 206
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
[Vote] Should manifests with a refers field that points to something that doesn't exist in the repository be accepted by a registry? #340
Comments
The use case I'm thinking of are users that want to ensure an image is complete before it's available to pull. It's similar to pushing blobs before pushing the manifest, but in this case, that descriptor is a reverse link on the child artifact. The biggest example is ensuring the signature is available so there is never an unsigned image that could be pulled and trigger security alerts. Over in the working group, we did pretty well putting these to a vote. So I'm happy to resolve this one in a similar way since we have two use cases pushing for different directions. |
Like this comment if you want the wording to change to "A registry MUST accept a manifest with a refers field that references a manifest that does not exist." |
Like this comment if you want the wording unchanged. |
I’m confused about the use case and statement. You are saying there is risk in pushing an artifact that refers to something that doesn’t exist. E.g, I can only push my image that has its signature if the signature exists (otherwise I might forget). I’d assume the fix is to require the refers to field to exist first. But the change is saying that the refers to field must NOT exist. Why should it be required to not exist? Sorry if I’m missing something! |
You can always push the image because the registry doesn't know if it will be later signed. The change here requires registries to allow a signature before the image is pushed. The signature manifest would have the refers field, and the image manifest it refers to would not exist. |
Ok so the distinction is that I am required to push the signature first, and if I try pushing a signature and the image manifest is refers to does exist the push is rejected? And then as a user I would be required to do the whole process from scratch - deleting the image manifest being referred to, then doing the signature first followed by the image manifest. So the answer to this question, based on that:
is that not only should it be accepted, but it should be required / mandated. And we would do this because we believe it enforces that an image that is supposed to have a signature definitely has it. But is this special to signatures? E.g.,think of all the metadata types to describe an image (that could be generated after) that would not allowed to be pushed because their image manifest already exists. Does that make sense? If I understand this correctly it feels like a weird way of solving the original problem of ensuring the signature is there, because ultimately it would lead to people re-pulling, deleting, and then re-pushing things in the correct order. There could be two other ideas - perhaps it should be a custom requirement for only a specific kind of artifact, signatures (e.g., there is no harm in pushing new metadata in an artifact that points to an image that exists) or it should be enforced via the pull - the registry requires all image manifests to be signed (and if the signature isn't pushed yet, no go), and no worries about the temporal order that you do things. But maybe I'm overthinking it. |
This is for the registry server, saying what it's required to accept. If we say the registry may reject (or "should accept") the signature that is pushed before the image, then clients that want portability must push the image first. This change removes the ability for a registry to reject the early signature push, giving clients the option to push the signature and image in either order. |
Ok gotcha! I guess that didn’t come across clearly from the statement. It’s hard to vote when the language leads me to multiple conclusions that were wrong, doh. Thanks for the explanation! |
Reading I think I'm seeing the confusion and why the blob exception was written the way it was. We should say "if", so changing:
to:
But even that feels like it's missing it since it can be implied that an invalid manifest suddenly overrides other checks by setting a refers field to an unknown manifest. Perhaps:
Is there a better word than "because"? |
The last one is much more clear! You could also use “when.” |
One of the scenarios here was about storing a reference without storing, say a large image. If we don't support registries from being able to store a reference artifact without the image that it points to, we probably need to define that this is not a goal or a way in which we can accomplish this goal? |
Both SHOULD and MUST allow a registry to support this. I think it's more a question of whether a client can assume that's supported and whether those artifacts are portable to any OCI registry.
It starts to get into GC, but I'd like to see this defined. |
I think that would be a nice compromise. I guess it would be up to the registry to communicate the practice? |
I don't think we addressed this in this discussion. What if the server, in its response, would return a blob with a list of all artifacts that point to it, and clients can be set to verify this. Something along the lines of...
|
I think this meant manifest, rather than blob. A blob has no structure to a registry, it can contain anything. Is it normal for an HTTP PUT request to respond with a body? How would those APIs look? Overall I think that's a dramatically bigger change than we're describing here.
Does this mean pushing a manifest without first pushing an artifact would fail? If so, that's the reverse of what we're addressing here and would break existing registry use cases. |
On the call today we decided to move forward with stronger language around not rejecting manifests that refer to digests that aren't currently in the registry. @sudo-bmitch will open a PR for that to close this issue. |
Follow up from #339.
from spec.md.
In the community meeting on 8/25, we talked about changing this to "A registry MUST accept a manifest with a refers field that references a manifest that does not exist."
I'm concerned that this will open users up to accidentally pushing reference artifacts to the wrong repository and not getting feedback about it. I'd like to hear about the scenarios we're trying to support / will be blocked if we don't do this.
cc/ @sudo-bmitch @sajayantony
The text was updated successfully, but these errors were encountered: