feat(images): implement RemoveImage gRPC API with extra protection for used images

internal/machine/docker/controller.go

@@ -27,7 +27,7 @@ const (
 type Controller struct {
 	// machineID is the ID of the machine where the managed Docker daemon is running.
 	machineID string
-	client    *client.Client
+	client    client.APIClient
 	service   *Service
 	store     *store.Store
 }

internal/machine/docker/image.go

@@ -0,0 +1,79 @@
+package docker
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	"github.com/containerd/errdefs"
+	"github.com/distribution/reference"
+	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/api/types/filters"
+	"github.com/docker/docker/api/types/image"
+	"github.com/docker/docker/pkg/stringid"
+	"github.com/psviderski/uncloud/pkg/api"
+)
+
+// RemoveImage removes an image while protecting names used by containers, even when force is requested.
+func (s *Service) RemoveImage(ctx context.Context, id string, opts image.RemoveOptions) ([]image.DeleteResponse, error) {
+	if err := s.checkImageRemovalConflicts(ctx, id); err != nil {
+		return nil, err
+	}
+	// Docker enforces its own conflicts, including protection of a used image's final reference.
+	return s.Client.ImageRemove(ctx, id, opts)
+}
+
+// checkImageRemovalConflicts rejects an image ID used by a running or stopped container, or a name used by a
+// container while it still resolves to that container's image. A digest request also checks the repository tags
+// Docker would remove with it. Unused aliases are allowed.
+func (s *Service) checkImageRemovalConflicts(ctx context.Context, id string) error {
+	img, err := s.Client.ImageInspect(ctx, id)
+	if err != nil {
+		return err
+	}
+	// Match Docker's resolved ID before parsing names, since a short ID also parses as a repository name.
+	isID := strings.HasPrefix(strings.TrimPrefix(img.ID, "sha256:"), strings.TrimPrefix(id, "sha256:"))
+	var named reference.Named
+	if !isID {
+		named, err = reference.ParseDockerRef(id)
+		if err != nil {
+			return errdefs.ErrInvalidArgument.WithMessage(err.Error())
+		}
+	}
+	_, isDigest := named.(reference.Canonical)
+
+	containers, err := s.Client.ContainerList(ctx, container.ListOptions{
+		All: true, Filters: filters.NewArgs(filters.Arg("ancestor", img.ID)),
+	})
+	if err != nil {
+		return fmt.Errorf("list containers using image: %w", err)
+	}
+
+	for _, ctr := range containers {
+		// The ancestor filter can include descendants. A retagged name must also be checked against its current ID.
+		if ctr.ImageID != img.ID {
+			continue
+		}
+
+		usedRef, err := reference.ParseDockerRef(ctr.Image)
+		// Familiar names make "alpine:latest" and "docker.io/library/alpine:latest" identify the same tag.
+		removesUsedName := err == nil && named != nil && reference.FamiliarString(usedRef) == reference.FamiliarString(named)
+		if isDigest && err == nil && usedRef.Name() == named.Name() {
+			// Removing a digest also removes this repository's tags pointing at the same image.
+			removesUsedName = true
+		}
+		if isID || removesUsedName {
+			ctrID, state := stringid.TruncateID(ctr.ID), "stopped"
+			if serviceName := ctr.Labels[api.LabelServiceName]; serviceName != "" {
+				ctrID = serviceName + "/" + ctrID
+			}
+			if ctr.State == container.StateRunning || ctr.State == container.StatePaused {
+				state = container.StateRunning
+			}
+			return errdefs.ErrConflict.WithMessage(
+				fmt.Sprintf("image '%s' is used by container '%s' (%s)", id, ctrID, state))
+		}
+	}
+
+	return nil
+}

internal/machine/docker/image_test.go

@@ -0,0 +1,175 @@
+package docker
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/containerd/errdefs"
+	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/api/types/image"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/psviderski/uncloud/pkg/api"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+func TestCheckImageRemovalConflicts(t *testing.T) {
+	imageID := "sha256:" + strings.Repeat("a", 64)
+	otherID := "sha256:" + strings.Repeat("b", 64)
+	digestRef := "myapp@" + imageID
+
+	tests := []struct {
+		name             string
+		requested        string
+		configured       string
+		containerImageID string
+		serviceName      string
+		blocked          bool
+	}{
+		{name: "used tag", requested: "myapp:current", configured: "myapp:current", serviceName: "web", blocked: true},
+		{name: "qualified request", requested: "docker.io/library/myapp:current", configured: "myapp:current", blocked: true},
+		{name: "qualified container reference", requested: "myapp:current", configured: "docker.io/library/myapp:current", blocked: true},
+		{name: "implicit latest", requested: "myapp", configured: "myapp:latest", blocked: true},
+		{name: "unused alias", requested: "myapp:old", configured: "myapp:current"},
+		{name: "unused alias of ID-used image", requested: "myapp:old", configured: imageID},
+		{name: "tag may clean up a used digest", requested: "myapp:old", configured: digestRef},
+		{name: "used digest", requested: digestRef, configured: digestRef, blocked: true},
+		{name: "digest also removes used tag", requested: digestRef, configured: "myapp:current", blocked: true},
+		{name: "unused repository digest", requested: digestRef, configured: "other:keep"},
+		{name: "image ID", requested: imageID, configured: "myapp:current", blocked: true},
+		{name: "short image ID", requested: "aaaaaaaa", configured: "myapp:current", blocked: true},
+		{name: "short image ID with algorithm", requested: "sha256:aaaaaaaa", configured: "myapp:current", blocked: true},
+		{name: "retagged name", requested: "myapp:current", configured: "myapp:current", containerImageID: otherID},
+	}
+
+	for _, tt := range tests {
+		for _, state := range []string{container.StateExited, container.StateRunning} {
+			t.Run(fmt.Sprintf("%s/%s", tt.name, state), func(t *testing.T) {
+				containerImageID := tt.containerImageID
+				if containerImageID == "" {
+					containerImageID = imageID
+				}
+
+				cli := NewMockAPIClient(t)
+				cli.EXPECT().ImageInspect(mock.Anything, tt.requested).
+					Return(image.InspectResponse{ID: imageID}, nil).Once()
+				cli.EXPECT().ContainerList(mock.Anything, mock.MatchedBy(func(opts container.ListOptions) bool {
+					return opts.All // Stopped containers must be checked too.
+				})).Return([]container.Summary{{
+					ID:      strings.Repeat("c", 64),
+					Labels:  map[string]string{api.LabelServiceName: tt.serviceName},
+					Names:   []string{"/web"},
+					ImageID: containerImageID,
+					Image:   tt.configured,
+					State:   state,
+				}}, nil).Once()
+
+				service := NewService(cli, nil)
+				err := service.checkImageRemovalConflicts(context.Background(), tt.requested)
+
+				if tt.blocked {
+					require.True(t, errdefs.IsConflict(err), "%v", err)
+					assert.ErrorContains(t, err, tt.requested)
+					if tt.serviceName != "" {
+						assert.ErrorContains(t, err, "container 'web/cccccccccccc'")
+					} else {
+						assert.ErrorContains(t, err, "container 'cccccccccccc'")
+					}
+					wantState := "stopped"
+					if state == container.StateRunning {
+						wantState = state
+					}
+					assert.ErrorContains(t, err, wantState)
+				} else {
+					require.NoError(t, err)
+				}
+			})
+		}
+	}
+}
+
+func TestCheckImageRemovalConflicts_PropagatesLookupErrors(t *testing.T) {
+	imageID := "sha256:" + strings.Repeat("a", 64)
+	t.Run("image lookup fails", func(t *testing.T) {
+		cli := NewMockAPIClient(t)
+		lookupErr := errdefs.ErrNotFound.WithMessage("no such image")
+		cli.EXPECT().ImageInspect(mock.Anything, "myapp:old").Return(image.InspectResponse{}, lookupErr).Once()
+
+		service := NewService(cli, nil)
+		require.ErrorIs(t, service.checkImageRemovalConflicts(context.Background(), "myapp:old"), lookupErr)
+	})
+
+	t.Run("container lookup fails", func(t *testing.T) {
+		cli := NewMockAPIClient(t)
+		lookupErr := errors.New("Docker unavailable")
+		cli.EXPECT().ImageInspect(mock.Anything, "myapp:old").Return(image.InspectResponse{ID: imageID}, nil).Once()
+		cli.EXPECT().ContainerList(mock.Anything, mock.Anything).Return(nil, lookupErr).Once()
+
+		service := NewService(cli, nil)
+		err := service.checkImageRemovalConflicts(context.Background(), "myapp:old")
+
+		require.ErrorIs(t, err, lookupErr)
+		assert.ErrorContains(t, err, "list containers using image")
+	})
+}
+
+func TestServiceRemoveImage_RejectsUsedNameBeforeDeletion(t *testing.T) {
+	imageID := "sha256:" + strings.Repeat("a", 64)
+	cli := NewMockAPIClient(t)
+	cli.EXPECT().ImageInspect(mock.Anything, "myapp:current").Return(image.InspectResponse{ID: imageID}, nil).Once()
+	cli.EXPECT().ContainerList(mock.Anything, mock.Anything).Return([]container.Summary{{
+		ID: "container-id", ImageID: imageID, Image: "myapp:current", State: container.StateRunning,
+	}}, nil).Once()
+
+	service := NewService(cli, nil)
+	// ImageRemove has no expectation: any deletion call would fail this test.
+	results, err := service.RemoveImage(context.Background(), "myapp:current", image.RemoveOptions{Force: true})
+
+	require.True(t, errdefs.IsConflict(err), "%v", err)
+	assert.Nil(t, results)
+}
+
+func TestServiceRemoveImage_DelegatesToDocker(t *testing.T) {
+	imageID := "sha256:" + strings.Repeat("a", 64)
+	tests := []struct {
+		name    string
+		opts    image.RemoveOptions
+		results []image.DeleteResponse
+		err     error
+	}{
+		{
+			name:    "options and results pass through",
+			opts:    image.RemoveOptions{Force: true, PruneChildren: true, Platforms: []ocispec.Platform{{OS: "linux", Architecture: "amd64"}}},
+			results: []image.DeleteResponse{{Deleted: imageID}},
+		},
+		{
+			name: "Docker rejects final reference removal",
+			opts: image.RemoveOptions{PruneChildren: true},
+			err:  errdefs.ErrConflict.WithMessage("container is using its referenced image"),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cli := NewMockAPIClient(t)
+			cli.EXPECT().ImageInspect(mock.Anything, "myapp:old").Return(image.InspectResponse{ID: imageID}, nil).Once()
+			cli.EXPECT().ContainerList(mock.Anything, mock.Anything).Return([]container.Summary{{
+				ID: "container-id", ImageID: imageID, Image: "myapp@" + imageID, State: container.StateRunning,
+			}}, nil).Once()
+			cli.EXPECT().ImageRemove(mock.Anything, "myapp:old", tt.opts).Return(tt.results, tt.err).Once()
+
+			service := NewService(cli, nil)
+			results, err := service.RemoveImage(context.Background(), "myapp:old", tt.opts)
+			if tt.err != nil {
+				require.ErrorIs(t, err, tt.err)
+			} else {
+				require.NoError(t, err)
+			}
+			assert.Equal(t, tt.results, results)
+		})
+	}
+}

internal/machine/docker/server.go

@@ -56,7 +56,7 @@ var fullDockerIDRegex = regexp.MustCompile(`^[a-f0-9]{64}$`)
 // Server implements the gRPC Docker service that proxies requests to the Docker daemon.
 type Server struct {
 	pb.UnimplementedDockerServer
-	client  *client.Client
+	client  client.APIClient
 	service *Service
 	db      *sqlx.DB
 	// internalDNSIP is a function that returns the IP address of the internal DNS server. It may return an empty
@@ -349,6 +349,41 @@ func (s *Server) InspectImage(ctx context.Context, req *pb.InspectImageRequest)
 	}, nil
 }
 
+// RemoveImage removes an image while protecting names used by containers, even when force is requested.
+func (s *Server) RemoveImage(ctx context.Context, req *pb.RemoveImageRequest) (*pb.RemoveImageResponse, error) {
+	opts := image.RemoveOptions{Force: req.Force, PruneChildren: req.PruneChildren}
+	for _, platform := range req.Platforms {
+		opts.Platforms = append(opts.Platforms, ocispec.Platform{
+			Architecture: platform.Architecture,
+			OS:           platform.Os,
+			OSVersion:    platform.OsVersion,
+			OSFeatures:   platform.OsFeatures,
+			Variant:      platform.Variant,
+		})
+	}
+
+	removals, err := s.service.RemoveImage(ctx, req.Id, opts)
+	if err != nil {
+		switch {
+		case errdefs.IsNotFound(err):
+			return nil, status.Error(codes.NotFound, err.Error())
+		case errdefs.IsConflict(err):
+			return nil, status.Error(codes.FailedPrecondition, err.Error())
+		case errdefs.IsInvalidArgument(err):
+			return nil, status.Error(codes.InvalidArgument, err.Error())
+		default:
+			return nil, status.Error(codes.Internal, err.Error())
+		}
+	}
+
+	result := &pb.MachineImageRemoval{}
+	for _, removal := range removals {
+		result.Results = append(result.Results, &pb.ImageRemoval{Deleted: removal.Deleted, Untagged: removal.Untagged})
+	}
+
+	return &pb.RemoveImageResponse{Messages: []*pb.MachineImageRemoval{result}}, nil
+}
+
 // InspectRemoteImage returns the image metadata for an image in a remote registry using the machine's Docker auth
 // credentials if necessary.
 func (s *Server) InspectRemoteImage(

internal/machine/docker/service.go

@@ -31,13 +31,13 @@ import (
 // from the machine database.
 type Service struct {
 	// Client is a Docker client for managing Docker resources.
-	Client *client.Client
+	Client client.APIClient
 	// db is a connection to the machine database.
 	db *sqlx.DB
 }
 
 // NewService creates a new Docker service instance.
-func NewService(client *client.Client, db *sqlx.DB) *Service {
+func NewService(client client.APIClient, db *sqlx.DB) *Service {
 	return &Service{
 		Client: client,
 		db:     db,