exporter: fix attestation manifest push order - #7012
Conversation
| testExportAttestations(t, sb, false, false) | ||
| testExportAttestations(t, sb, false, false, false) |
There was a problem hiding this comment.
So many booleans; perhaps we should consider a small struct for these options (can be a follow-up)
There was a problem hiding this comment.
Yes can look at this as follow-up. Wanted to keep changes as small as possible for a patch release.
ff0ebbb to
edbd366
Compare
| return nil | ||
| } | ||
| visited[desc.Digest] = struct{}{} | ||
| if images.IsManifestType(desc.MediaType) { |
There was a problem hiding this comment.
Shouldn't apply to attestations, but Index can also have Subject:
https://github.com/opencontainers/image-spec/blob/af26a05fba5ee648512f4ea3c9fda1fcc1b6d6dc/specs-go/v1/index.go#L33-L34
Could probably also easily support this by extending this check to also include Indexes.
There was a problem hiding this comment.
Yes makes sense to extend.
edbd366 to
c1f459e
Compare
| if err != nil { | ||
| return err | ||
| } | ||
| var manifest struct { |
There was a problem hiding this comment.
ultra nit, not a blocker:
| var manifest struct { | |
| var referer struct { |
or
| var manifest struct { | |
| var withSubject struct { |
OCI artifact attestations carry a subject reference to the image manifest. Push manifests in an order that keeps child manifests before parent indexes while also pushing any in-stack subject manifest before the attestation that references it. Add an integration test variant with a strict registry proxy to cover registries that reject manifests whose subject doesn't already exist. Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
c1f459e to
067b18b
Compare
|
I think this is backwards? Referrers were designed so that the referrer objects could and even should be pushed before the manifest, and IMO it's a registry bug to validate otherwise. The canonical use case was making sure (detached) signatures are pushed before the object they're signing, so that the manifest doesn't ever exist on the registry without a signature. |
Interesting. Kinda makes sense; the referrer being part of the step to validate if the registry should accept the blob. |
4 participants
OCI artifact attestations carry a subject reference to the image manifest. Push manifests in an order that keeps child manifests before parent indexes while also pushing any in-stack subject manifest before the attestation that references it.
Add an integration test variant with a strict registry proxy to cover registries that reject manifests whose subject doesn't already exist.