image/inspect: Add platform selection #49586
Conversation
427049b to
8ee1605
Compare
4e69561 to
17840c8
Compare
Signed-off-by: Paweł Gronowski <[email protected]>
| const ( | ||
| // DefaultVersion of the current REST API. | ||
| DefaultVersion = "1.48" | ||
| DefaultVersion = "1.49" |
There was a problem hiding this comment.
This commit should also update the swagger and version-history to add the new version; for an example, see 2b43979
We could do this as a separate PR i we want it to be more visible (change-log / release-notes)
| if manifests && platform != nil { | ||
| return errdefs.InvalidParameter(errors.New("manifests and platform options cannot both be set")) | ||
| } |
There was a problem hiding this comment.
I think for most cases where two options cannot be combined, we used conflicting options: as prefix; we could consider that here as well to be consistent.
I'm slightly wondering though if we should make this a conflict; was there a specific reason we could not support both? Or was that because of size-calculation (e.g.)?
we could;
- Filter the list of manifests (?)
- or keep it as-is (showing all?)
- The "outer" summary would show whatever was requested
There was a problem hiding this comment.
Technically we can support it, but I don't think it's actually useful and the result would possibly be ambiguous.
We can always lift the restriction if we decide on concrete behavior.
There was a problem hiding this comment.
but I don't think it's actually useful and the result would possibly be ambiguous.
Hm, right, but currently we allow including the manifests, which also contains manifests for attestations. But with this PR, I need to choose;
- Inspect the default platform and get information about other platforms and attestations
- Inspect the non-default platform, but now miss information about other platforms an attestations.
There was a problem hiding this comment.
Without this PR you didn't have that choice at all! 😄
Yeah I get what you mean, however I think it's more appropriate/useful to consider passing a platform as a mean of enforcing a single-platform view of a multi-platform image. So platform = nil doesn't mean "default platform".
If you want a multi-platform information (provided by Manifests), you wouldn't choose an explicit platform because that forces a single-platform (graphdriver-like) view of the image.
If you don't set an explicit platform, then you get a view of the "total" image and the "outer" information defaults to the "native" or "any" platform-specific image to keep the API response compatible with the pre-containerd clients.
07dc7e6 to
6f5137a
Compare
6f5137a to
fba1090
Compare
84234b6 to
8af6c14
Compare
`GET /image/{name}/json` now supports `platform` parameter allowing to
specify which platform variant of a multi-platform image to inspect.
For servers that do not use containerd image store integration, this
option will cause an error if the requested platform doesn't match the
image's actual platform
Signed-off-by: Paweł Gronowski <[email protected]>
8af6c14 to
59169d0
Compare
Prior to this commit, performing a build on a ARM Mac with the default configuration and then building it again with the image platform set to `linux/amd64` results in an "Image platform mismatch detected" failure. This is due to the fact that `docker inspect` returns JSON for the default platform, regardless of the fact that another architecture has been pulled. To solve the issue, the `inspect` API call on Docker 1.49+ can now accept a platform query parameter which when specified returns platform specific JSON. At the time of this commit, the Docker API documentation hasn't been updated, despite PR moby/moby#49586 being merged. In addition to using the correct inspect JSON, we also need to pin the run image we use to a specific digest. Without doing this, buildpacks revert back to the default platform image and "content digest not found" errors are thrown (similar to https://github.com/buildpacks/docs/issues/818). See gh-47292 Signed-off-by: hojooo <[email protected]>
GET /image/{name}/jsonnow supportsplatformparameter allowing tospecify which platform variant of a multi-platform image to inspect.
For servers that do not use containerd image store integration, this
option will cause an error if the requested platform doesn't match the
image's actual platform