diff --git a/containers/docker/image_push.go b/containers/docker/image_push.go index 1700d52..2bf946b 100644 --- a/containers/docker/image_push.go +++ b/containers/docker/image_push.go @@ -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 @@ -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:] } diff --git a/containers/docker/image_push_test.go b/containers/docker/image_push_test.go index a59e908..b479a61 100644 --- a/containers/docker/image_push_test.go +++ b/containers/docker/image_push_test.go @@ -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 } @@ -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 @@ -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 { @@ -95,6 +221,7 @@ func TestImagePush(t *testing.T) { isJSON: tc.isJSON, tag: tc.inTag, image: tc.inImage, + msgs: tc.inMsgs, } mgr := New(fd) @@ -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) }