Move deleting images from ImageSource to ImageReference

For lookaside signature store, and separating the read and write URLs,
we need to set up read-only and read-write states differently; having
read-write “delete” in dockerImageSource is incovenient.

In tue future, ImageSource.Delete will be a really poor fit for
docker-daemon:, where initializing the ImageSource causes the tarball
to be copied from the daemon.  We could instead implement the
docker-daemon source so that it only copies the tarball on demand, but
not sharing the object is much simpler.

This leaves the Docker implementation in docker_image_src.go to make
reviewing easier.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

directory/directory_src.go

@@ -1,7 +1,6 @@
 package directory
 
 import (
-	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
@@ -59,7 +58,3 @@ func (s *dirImageSource) GetSignatures() ([][]byte, error) {
 	}
 	return signatures, nil
 }
-
-func (s *dirImageSource) Delete() error {
-	return fmt.Errorf("directory#dirImageSource.Delete() not implmented")
-}

directory/directory_test.go

@@ -127,16 +127,6 @@ func TestGetPutSignatures(t *testing.T) {
 	assert.Equal(t, signatures, sigs)
 }
 
-func TestDelete(t *testing.T) {
-	ref, tmpDir := refToTempDir(t)
-	defer os.RemoveAll(tmpDir)
-
-	src, err := ref.NewImageSource(nil, nil)
-	require.NoError(t, err)
-	err = src.Delete()
-	assert.Error(t, err)
-}
-
 func TestSourceReference(t *testing.T) {
 	ref, tmpDir := refToTempDir(t)
 	defer os.RemoveAll(tmpDir)

directory/directory_transport.go

@@ -145,6 +145,11 @@ func (ref dirReference) NewImageDestination(ctx *types.SystemContext) (types.Ima
 	return newImageDestination(ref), nil
 }
 
+// DeleteImage deletes the named image from the registry, if supported.
+func (ref dirReference) DeleteImage(ctx *types.SystemContext) error {
+	return fmt.Errorf("Deleting images not implemented for dir: images")
+}
+
 // manifestPath returns a path for the manifest within a directory using our conventions.
 func (ref dirReference) manifestPath() string {
 	return filepath.Join(ref.path, "manifest.json")

directory/directory_transport_test.go

@@ -167,6 +167,13 @@ func TestReferenceNewImageDestination(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestReferenceDeleteImage(t *testing.T) {
+	ref, tmpDir := refToTempDir(t)
+	defer os.RemoveAll(tmpDir)
+	err := ref.DeleteImage(nil)
+	assert.Error(t, err)
+}
+
 func TestReferenceManifestPath(t *testing.T) {
 	ref, tmpDir := refToTempDir(t)
 	defer os.RemoveAll(tmpDir)

docker/docker_image_src.go

@@ -113,42 +113,46 @@ func (s *dockerImageSource) GetSignatures() ([][]byte, error) {
 	return [][]byte{}, nil
 }
 
-func (s *dockerImageSource) Delete() error {
-	var body []byte
+// deleteImage deletes the named image from the registry, if supported.
+func deleteImage(ctx *types.SystemContext, ref dockerReference) error {
+	c, err := newDockerClient(ctx, ref.ref.Hostname())
+	if err != nil {
+		return err
+	}
 
 	// When retrieving the digest from a registry >= 2.3 use the following header:
 	//   "Accept": "application/vnd.docker.distribution.manifest.v2+json"
 	headers := make(map[string][]string)
 	headers["Accept"] = []string{manifest.DockerV2Schema2MIMEType}
 
-	reference, err := s.ref.tagOrDigest()
+	reference, err := ref.tagOrDigest()
 	if err != nil {
 		return err
 	}
-	getURL := fmt.Sprintf(manifestURL, s.ref.ref.RemoteName(), reference)
-	get, err := s.c.makeRequest("GET", getURL, headers, nil)
+	getURL := fmt.Sprintf(manifestURL, ref.ref.RemoteName(), reference)
+	get, err := c.makeRequest("GET", getURL, headers, nil)
 	if err != nil {
 		return err
 	}
 	defer get.Body.Close()
-	body, err = ioutil.ReadAll(get.Body)
+	body, err := ioutil.ReadAll(get.Body)
 	if err != nil {
 		return err
 	}
 	switch get.StatusCode {
 	case http.StatusOK:
 	case http.StatusNotFound:
-		return fmt.Errorf("Unable to delete %v. Image may not exist or is not stored with a v2 Schema in a v2 registry.", s.ref.ref)
+		return fmt.Errorf("Unable to delete %v. Image may not exist or is not stored with a v2 Schema in a v2 registry.", ref.ref)
 	default:
-		return fmt.Errorf("Failed to delete %v: %s (%v)", s.ref.ref, string(body), get.Status)
+		return fmt.Errorf("Failed to delete %v: %s (%v)", ref.ref, string(body), get.Status)
 	}
 
 	digest := get.Header.Get("Docker-Content-Digest")
-	deleteURL := fmt.Sprintf(manifestURL, s.ref.ref.RemoteName(), digest)
+	deleteURL := fmt.Sprintf(manifestURL, ref.ref.RemoteName(), digest)
 
 	// When retrieving the digest from a registry >= 2.3 use the following header:
 	//   "Accept": "application/vnd.docker.distribution.manifest.v2+json"
-	delete, err := s.c.makeRequest("DELETE", deleteURL, headers, nil)
+	delete, err := c.makeRequest("DELETE", deleteURL, headers, nil)
 	if err != nil {
 		return err
 	}

docker/docker_transport.go

@@ -132,6 +132,11 @@ func (ref dockerReference) NewImageDestination(ctx *types.SystemContext) (types.
 	return newImageDestination(ctx, ref)
 }
 
+// DeleteImage deletes the named image from the registry, if supported.
+func (ref dockerReference) DeleteImage(ctx *types.SystemContext) error {
+	return deleteImage(ctx, ref)
+}
+
 // tagOrDigest returns a tag or digest from the reference.
 func (ref dockerReference) tagOrDigest() (string, error) {
 	if ref, ok := ref.ref.(reference.Canonical); ok {

oci/oci_transport.go

@@ -181,6 +181,11 @@ func (ref ociReference) NewImageDestination(ctx *types.SystemContext) (types.Ima
 	return newImageDestination(ref), nil
 }
 
+// DeleteImage deletes the named image from the registry, if supported.
+func (ref ociReference) DeleteImage(ctx *types.SystemContext) error {
+	return fmt.Errorf("Deleting images not implemented for oci: images")
+}
+
 // ociLayoutPathPath returns a path for the oci-layout within a directory using OCI conventions.
 func (ref ociReference) ociLayoutPath() string {
 	return filepath.Join(ref.dir, "oci-layout")

oci/oci_transport_test.go

@@ -223,6 +223,13 @@ func TestReferenceNewImageDestination(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestReferenceDeleteImage(t *testing.T) {
+	ref, tmpDir := refToTempOCI(t)
+	defer os.RemoveAll(tmpDir)
+	err := ref.DeleteImage(nil)
+	assert.Error(t, err)
+}
+
 func TestReferenceOCILayoutPath(t *testing.T) {
 	ref, tmpDir := refToTempOCI(t)
 	defer os.RemoveAll(tmpDir)

openshift/openshift.go

@@ -517,7 +517,3 @@ type status struct {
 	// Details *StatusDetails `json:"details,omitempty"`
 	Code int32 `json:"code,omitempty"`
 }
-
-func (s *openshiftImageSource) Delete() error {
-	return fmt.Errorf("openshift#openshiftImageSource.Delete() not implmented")
-}

openshift/openshift_transport.go

@@ -169,3 +169,8 @@ func (ref openshiftReference) NewImageSource(ctx *types.SystemContext, requested
 func (ref openshiftReference) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) {
 	return newImageDestination(ctx, ref)
 }
+
+// DeleteImage deletes the named image from the registry, if supported.
+func (ref openshiftReference) DeleteImage(ctx *types.SystemContext) error {
+	return fmt.Errorf("Deleting images not implemented for atomic: images")
+}

openshift/openshift_transport_test.go

@@ -119,3 +119,10 @@ func TestReferenceNewImage(t *testing.T) {
 
 // openshiftReference.NewImageSource, openshiftReference.NewImageDestination untested because they depend
 // on per-user configuration when initializing httpClient.
+
+func TestReferenceDeleteImage(t *testing.T) {
+	ref, err := NewReference(testBaseURL, "ns", "stream", "notlatest")
+	require.NoError(t, err)
+	err = ref.DeleteImage(nil)
+	assert.Error(t, err)
+}

signature/policy_eval_simple_test.go

@@ -43,6 +43,9 @@ func (ref nameOnlyImageReferenceMock) NewImageSource(ctx *types.SystemContext, r
 func (ref nameOnlyImageReferenceMock) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) {
 	panic("unexpected call to a mock function")
 }
+func (ref nameOnlyImageReferenceMock) DeleteImage(ctx *types.SystemContext) error {
+	panic("unexpected call to a mock function")
+}
 
 func TestPRInsecureAcceptAnythingIsSignatureAuthorAccepted(t *testing.T) {
 	pr := NewPRInsecureAcceptAnything()

signature/policy_eval_test.go

@@ -99,6 +99,9 @@ func (ref pcImageReferenceMock) NewImageSource(ctx *types.SystemContext, request
 func (ref pcImageReferenceMock) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) {
 	panic("unexpected call to a mock function")
 }
+func (ref pcImageReferenceMock) DeleteImage(ctx *types.SystemContext) error {
+	panic("unexpected call to a mock function")
+}
 
 func TestPolicyContextRequirementsForImageRef(t *testing.T) {
 	ktGPG := SBKeyTypeGPGKeys

signature/policy_reference_match_test.go

@@ -110,6 +110,9 @@ func (ref refImageReferenceMock) NewImageSource(ctx *types.SystemContext, reques
 func (ref refImageReferenceMock) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) {
 	panic("unexpected call to a mock function")
 }
+func (ref refImageReferenceMock) DeleteImage(ctx *types.SystemContext) error {
+	panic("unexpected call to a mock function")
+}
 
 // nameImageTransportMock is a mock of types.ImageTransport which returns itself in Name.
 type nameImageTransportMock string

types/types.go

@@ -78,6 +78,9 @@ type ImageReference interface {
 	NewImageSource(ctx *SystemContext, requestedManifestMIMETypes []string) (ImageSource, error)
 	// NewImageDestination returns a types.ImageDestination for this reference.
 	NewImageDestination(ctx *SystemContext) (ImageDestination, error)
+
+	// DeleteImage deletes the named image from the registry, if supported.
+	DeleteImage(ctx *SystemContext) error
 }
 
 // ImageSource is a service, possibly remote (= slow), to download components of a single image.
@@ -95,8 +98,6 @@ type ImageSource interface {
 	GetBlob(digest string) (io.ReadCloser, int64, error)
 	// GetSignatures returns the image's signatures.  It may use a remote (= slow) service.
 	GetSignatures() ([][]byte, error)
-	// Delete image from registry, if operation is supported
-	Delete() error
 }
 
 // ImageDestination is a service, possibly remote (= slow), to store components of a single image.