Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add --prefer-index flag forimagetools create on a single source #2482

Merged
merged 6 commits into from
May 31, 2024
Merged

Add --prefer-index flag forimagetools create on a single source #2482

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0808747
Added application/vnd.docker.distribution.manifest.v2+json mediatype …
rvoh-tismith May 29, 2024
abf6c77
Add a --prefer-index flag that allows you to specify the preferred be…
rvoh-tismith May 29, 2024
2061550
Slightly refactored the mediaType check on single source so that now …
rvoh-tismith May 29, 2024
388af35
Updated tests to test new --prefer-index flag
rvoh-tismith May 30, 2024
117c901
Updated tests further to make sure the new flag doesn't affect copy a…
rvoh-tismith May 30, 2024
7bfae2b
Updated the docs.
rvoh-tismith May 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 13 additions & 23 deletions tests/imagetools.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,23 +61,7 @@ func testImagetoolsCopyManifest(t *testing.T, sb integration.Sandbox) {
var idx2 ocispecs.Index
err = json.Unmarshal(dt, &idx2)
require.NoError(t, err)
require.Equal(t, images.MediaTypeDockerSchema2ManifestList, idx2.MediaType)
require.Equal(t, 1, len(idx2.Manifests))

cmd = buildxCmd(sb, withArgs("imagetools", "inspect", target2+"@"+string(idx2.Manifests[0].Digest), "--raw"))
dt, err = cmd.CombinedOutput()
require.NoError(t, err, string(dt))

var mfst2 ocispecs.Manifest
err = json.Unmarshal(dt, &mfst2)
require.NoError(t, err)
require.Equal(t, images.MediaTypeDockerSchema2Manifest, mfst2.MediaType)

require.Equal(t, mfst.Config.Digest, mfst2.Config.Digest)
require.Equal(t, len(mfst.Layers), len(mfst2.Layers))
for i := range mfst.Layers {
require.Equal(t, mfst.Layers[i].Digest, mfst2.Layers[i].Digest)
}
require.Equal(t, images.MediaTypeDockerSchema2Manifest, idx2.MediaType)
}

func testImagetoolsCopyIndex(t *testing.T, sb integration.Sandbox) {
Expand Down Expand Up @@ -161,7 +145,15 @@ func testImagetoolsInspectAndFilter(t *testing.T, sb integration.Sandbox) {
mfst = idx.Manifests[1]
require.Equal(t, "linux/arm64", platforms.Format(*mfst.Platform))

// create amd64 only image
cmd = buildxCmd(sb, withArgs("imagetools", "inspect", target+"@"+string(idx.Manifests[1].Digest), "--raw"))
dt, err = cmd.CombinedOutput()
require.NoError(t, err, string(dt))

var armMfst ocispecs.Manifest
err = json.Unmarshal(dt, &armMfst)
require.NoError(t, err)

// create arm64 only image
cmd = buildxCmd(sb, withArgs("imagetools", "create", "-t", target+"-arm64", target+"@"+string(idx.Manifests[1].Digest)))
dt, err = cmd.CombinedOutput()
require.NoError(t, err, string(dt))
Expand All @@ -170,14 +162,12 @@ func testImagetoolsInspectAndFilter(t *testing.T, sb integration.Sandbox) {
dt, err = cmd.CombinedOutput()
require.NoError(t, err, string(dt))

var idx2 ocispecs.Index
var idx2 ocispecs.Manifest
err = json.Unmarshal(dt, &idx2)
require.NoError(t, err)

require.Equal(t, 1, len(idx2.Manifests))

require.Equal(t, idx.Manifests[1].Digest, idx2.Manifests[0].Digest)
require.Equal(t, platforms.Format(*idx.Manifests[1].Platform), platforms.Format(*idx2.Manifests[0].Platform))
require.Equal(t, armMfst.Config.Digest, idx2.Config.Digest)
require.Equal(t, armMfst.MediaType, idx2.MediaType)
}

func testImagetoolsAnnotation(t *testing.T, sb integration.Sandbox) {
Expand Down
2 changes: 1 addition & 1 deletion util/imagetools/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (r *Resolver) Combine(ctx context.Context, srcs []*Source, ann []string) ([

// on single source, return original bytes
if len(srcs) == 1 && len(ann) == 0 {
if mt := srcs[0].Desc.MediaType; mt == images.MediaTypeDockerSchema2ManifestList || mt == ocispec.MediaTypeImageIndex {
if mt := srcs[0].Desc.MediaType; mt == images.MediaTypeDockerSchema2ManifestList || mt == images.MediaTypeDockerSchema2Manifest || mt == ocispec.MediaTypeImageIndex {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that is the desired behavior then why is there a mediatype check at all, or at least why is ocispec.MediaTypeImageManifest missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhhhh great point! Yeah, it actually seems reasonable that there doesn't even need to be a check. Thinking through the logic, I guess I can't really think of when the logic would need to be different here. Seems like if there is one source and no annotations (and possibly if the --preserve-single-source flag is set to true, based on my response to your other comment), we should just return the original bytes. I almost feel like I'm missing something here though. It feels like this is here for a reason, but maybe it is just an oversight.

return dts[0], srcs[0].Desc, nil
}
}
Expand Down
Loading