Skip to content

Fix image load response parsing in the docker manager #96

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

Merged
alshabib merged 1 commit into from
Nov 5, 2026
Merged

Fix image load response parsing in the docker manager #96

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
105 changes: 90 additions & 15 deletions containers/docker/image_push.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ import (
"context"
"encoding/json"
"fmt"
"io"
"os"
"github.com/docker/docker/client"
"strings"

"github.com/docker/docker/client"
"github.com/moby/moby/pkg/jsonmessage"
"github.com/openconfig/containerz/containers"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"github.com/openconfig/containerz/containers"
)

// ImagePush pushes the container file to the containerz server. It can optionally tag the
Expand All @@ -30,31 +31,105 @@ func (m *Manager) ImagePush(ctx context.Context, file *os.File, opts ...options.
defer resp.Body.Close()

if resp.Body != nil && resp.JSON {
var jm jsonmessage.JSONMessage
dec := json.NewDecoder(resp.Body)
if err := dec.Decode(&jm); err != nil {
return "", "", status.Convert(err).Err()
loaded, err := loadedImageFromStream(json.NewDecoder(resp.Body))
if err != nil {
return "", "", err
}

imageNameAndTag := extractImageNameFromStream(jm.Stream)
if options.TargetName != "" && imageNameAndTag != "" {
if err := m.client.ImageTag(ctx, imageNameAndTag, fmt.Sprintf("%s:%s", options.TargetName, options.TargetTag)); err != nil {
if options.TargetName != "" {
if err := m.client.ImageTag(ctx, loaded.ref(), fmt.Sprintf("%s:%s", options.TargetName, options.TargetTag)); err != nil {
return "", "", status.Convert(err).Err()
}
return options.TargetName, options.TargetTag, nil
}

parts := strings.SplitN(imageNameAndTag, ":", 2)
return parts[0], parts[1], nil
if loaded.name == "" {
return "", "", status.Errorf(codes.InvalidArgument,
"image was loaded as %s without a name and tag; a target name must be provided", loaded.id)
}
return loaded.name, loaded.tag, nil
}

return options.TargetName, options.TargetTag, nil
}

func extractImageNameFromStream(stream string) string {
if len(stream) == 0 {
return ""
const (
// loadedImagePrefix is the prefix docker uses to report a loaded image that
// has a name and tag.
loadedImagePrefix = "Loaded image: "
// loadedImageIDPrefix is the prefix docker uses to report a loaded image
// that has no name or tag.
loadedImageIDPrefix = "Loaded image ID: "
)

// loadedImage describes the image reported by docker after an image load.
// Either name (and tag) or id is set.
type loadedImage struct {
name, tag string
id string
}

// ref returns a reference to the loaded image that can be used as the source
// of an image tag operation.
func (l loadedImage) ref() string {
if l.name == "" {
return l.id
}
return l.name + ":" + l.tag
}

// loadedImageFromStream reads all messages docker sends in response to an
// image load and returns the first image it reports as loaded. Other messages,
// such as the warning that an existing image with the same tag was renamed,
// are ignored. Errors reported in the stream are returned.
func loadedImageFromStream(dec *json.Decoder) (loadedImage, error) {
var loaded loadedImage
for {
var jm jsonmessage.JSONMessage
if err := dec.Decode(&jm); err != nil {
if err == io.EOF {
break
}
return loadedImage{}, status.Errorf(codes.Internal, "unable to decode image load response: %v", err)
}

if jm.Error != nil {
return loadedImage{}, status.Errorf(codes.Internal, "unable to load image: %s", jm.Error.Message)
}
if jm.ErrorMessage != "" {
return loadedImage{}, status.Errorf(codes.Internal, "unable to load image: %s", jm.ErrorMessage)
}

if loaded.name != "" || loaded.id != "" {
// Keep reading to surface errors reported after the image was loaded.
continue
}
for _, line := range strings.Split(jm.Stream, "\n") {
line = strings.TrimSpace(line)
if strings.HasPrefix(line, loadedImageIDPrefix) {
loaded.id = strings.TrimPrefix(line, loadedImageIDPrefix)
break
}
if strings.HasPrefix(line, loadedImagePrefix) {
loaded.name, loaded.tag = splitImageReference(strings.TrimPrefix(line, loadedImagePrefix))
break
}
}
}

return strings.Trim(strings.Replace(stream, "Loaded image: ", "", 1), "\n")
if loaded.name == "" && loaded.id == "" {
return loadedImage{}, status.Error(codes.Internal, "image load response did not report a loaded image")
}
return loaded, nil
}

// splitImageReference splits an image reference of the form name[:tag] into
// its name and tag. The tag defaults to "latest". A colon that is part of a
// registry host:port is not treated as the tag separator.
func splitImageReference(ref string) (string, string) {
i := strings.LastIndex(ref, ":")
if i < 0 || strings.Contains(ref[i+1:], "/") {
return ref, "latest"
}
return ref[:i], ref[i+1:]
}
143 changes: 137 additions & 6 deletions containers/docker/image_push_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,30 @@ type fakePushingDocker struct {
fakeDocker
image, tag string
isJSON bool
// msgs, if set, replaces the default "Loaded image" message.
msgs []*jsonmessage.JSONMessage

Source, Target string
}

func (f fakePushingDocker) ImageLoad(ctx context.Context, input io.Reader, options ...client.ImageLoadOption) (image.LoadResponse, error) {
jm := &jsonmessage.JSONMessage{
Stream: fmt.Sprintf("Loaded image: %s\n", f.image+":"+f.tag),
msgs := f.msgs
if msgs == nil {
msgs = []*jsonmessage.JSONMessage{{
Stream: fmt.Sprintf("Loaded image: %s\n", f.image+":"+f.tag),
}}
}

data, err := json.Marshal(jm)
if err != nil {
return image.LoadResponse{}, err
var buf bytes.Buffer
enc := json.NewEncoder(&buf)
for _, jm := range msgs {
if err := enc.Encode(jm); err != nil {
return image.LoadResponse{}, err
}
}

return image.LoadResponse{
Body: io.NopCloser(bytes.NewBuffer(data)),
Body: io.NopCloser(&buf),
JSON: f.isJSON,
}, nil
}
Expand All @@ -55,6 +63,7 @@ func TestImagePush(t *testing.T) {
inOpts []options.Option
inFile *os.File
inImage, inTag string
inMsgs []*jsonmessage.JSONMessage
isJSON bool
wantState *fakePushingDocker
wantImage, wantTag string
Expand Down Expand Up @@ -87,6 +96,123 @@ func TestImagePush(t *testing.T) {
wantImage: "another-image",
wantTag: "another-tag",
},
{
name: "existing-image-warning-then-loaded",
isJSON: true,
inFile: os.Stdin,
inMsgs: []*jsonmessage.JSONMessage{
{Stream: "The image some-image:latest already exists, renaming the old one with ID sha256:1234 to empty string\n"},
{Stream: "Loaded image: some-image:latest\n"},
},
wantImage: "some-image",
wantTag: "latest",
},
{
name: "existing-image-warning-then-loaded-with-tagging",
isJSON: true,
inFile: os.Stdin,
inMsgs: []*jsonmessage.JSONMessage{
{Stream: "The image some-image:latest already exists, renaming the old one with ID sha256:1234 to empty string\n"},
{Stream: "Loaded image: some-image:latest\n"},
},
inOpts: []options.Option{options.WithTarget("another-image", "another-tag")},
wantState: &fakePushingDocker{
Source: "some-image:latest",
Target: "another-image:another-tag",
},
wantImage: "another-image",
wantTag: "another-tag",
},
{
name: "warning-and-loaded-in-one-message",
isJSON: true,
inFile: os.Stdin,
inMsgs: []*jsonmessage.JSONMessage{
{Stream: "The image some-image:latest already exists, renaming the old one with ID sha256:1234 to empty string\nLoaded image: some-image:latest\n"},
},
wantImage: "some-image",
wantTag: "latest",
},
{
name: "registry-with-port",
isJSON: true,
inFile: os.Stdin,
inMsgs: []*jsonmessage.JSONMessage{
{Stream: "Loaded image: registry:5000/some-image:some-tag\n"},
},
wantImage: "registry:5000/some-image",
wantTag: "some-tag",
},
{
name: "no-tag-defaults-to-latest",
isJSON: true,
inFile: os.Stdin,
inMsgs: []*jsonmessage.JSONMessage{
{Stream: "Loaded image: registry:5000/some-image\n"},
},
wantImage: "registry:5000/some-image",
wantTag: "latest",
},
{
name: "image-id-only-with-tagging",
isJSON: true,
inFile: os.Stdin,
inMsgs: []*jsonmessage.JSONMessage{
{Stream: "Loaded image ID: sha256:1234\n"},
},
inOpts: []options.Option{options.WithTarget("another-image", "another-tag")},
wantState: &fakePushingDocker{
Source: "sha256:1234",
Target: "another-image:another-tag",
},
wantImage: "another-image",
wantTag: "another-tag",
},
{
name: "image-id-only-without-target",
isJSON: true,
inFile: os.Stdin,
inMsgs: []*jsonmessage.JSONMessage{
{Stream: "Loaded image ID: sha256:1234\n"},
},
wantErr: status.Error(codes.InvalidArgument, "image was loaded as sha256:1234 without a name and tag; a target name must be provided"),
},
{
name: "error-detail",
isJSON: true,
inFile: os.Stdin,
inMsgs: []*jsonmessage.JSONMessage{
{Error: &jsonmessage.JSONError{Message: "archive/tar: invalid tar header"}},
},
wantErr: status.Error(codes.Internal, "unable to load image: archive/tar: invalid tar header"),
},
{
name: "error-after-loaded",
isJSON: true,
inFile: os.Stdin,
inMsgs: []*jsonmessage.JSONMessage{
{Stream: "Loaded image: some-image:latest\n"},
{ErrorMessage: "something went wrong"},
},
wantErr: status.Error(codes.Internal, "unable to load image: something went wrong"),
},
{
name: "empty-stream",
isJSON: true,
inFile: os.Stdin,
inMsgs: []*jsonmessage.JSONMessage{},
wantErr: status.Error(codes.Internal, "image load response did not report a loaded image"),
},
{
name: "empty-stream-with-tagging",
isJSON: true,
inFile: os.Stdin,
inMsgs: []*jsonmessage.JSONMessage{
{Stream: "\n"},
},
inOpts: []options.Option{options.WithTarget("another-image", "another-tag")},
wantErr: status.Error(codes.Internal, "image load response did not report a loaded image"),
},
}

for _, tc := range tests {
Expand All @@ -95,6 +221,7 @@ func TestImagePush(t *testing.T) {
isJSON: tc.isJSON,
tag: tc.inTag,
image: tc.inImage,
msgs: tc.inMsgs,
}
mgr := New(fd)

Expand All @@ -109,6 +236,10 @@ func TestImagePush(t *testing.T) {
t.Errorf("ImagePush(file) returned error: %v", err)
}

if tc.wantErr != nil {
t.Fatalf("ImagePush(file) returned no error, want %v", tc.wantErr)
}

if gotImage != tc.wantImage || gotTag != tc.wantTag {
t.Errorf("ImagePush(file) returned wrong info; want %s/%s, got %s/%s", tc.wantImage, tc.wantTag, gotImage, gotTag)
}
Expand Down
Loading