Merge pull request #1757 from cgwalters/get-manifest-optional

proxy: Add `OpenImageOptional`

cmd/skopeo/proxy.go

@@ -73,9 +73,12 @@ import (
 
 	"github.com/containers/image/v5/image"
 	"github.com/containers/image/v5/manifest"
+	ocilayout "github.com/containers/image/v5/oci/layout"
 	"github.com/containers/image/v5/pkg/blobinfocache"
 	"github.com/containers/image/v5/transports/alltransports"
 	"github.com/containers/image/v5/types"
+	dockerdistributionerrcode "github.com/docker/distribution/registry/api/errcode"
+	dockerdistributionapi "github.com/docker/distribution/registry/api/v2"
 	"github.com/opencontainers/go-digest"
 	imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/spf13/cobra"
@@ -100,6 +103,9 @@ const maxMsgSize = 32 * 1024
 // integers are above this.
 const maxJSONFloat = float64(uint64(1)<<53 - 1)
 
+// sentinelImageID represents "image not found" on the wire
+const sentinelImageID = 0
+
 // request is the JSON serialization of a function call
 type request struct {
 	// Method is the name of the function
@@ -197,6 +203,29 @@ func (h *proxyHandler) Initialize(args []interface{}) (replyBuf, error) {
 // OpenImage accepts a string image reference i.e. TRANSPORT:REF - like `skopeo copy`.
 // The return value is an opaque integer handle.
 func (h *proxyHandler) OpenImage(args []interface{}) (replyBuf, error) {
+	return h.openImageImpl(args, false)
+}
+
+// isDockerManifestUnknownError is a copy of code from containers/image,
+// please update there first.
+func isDockerManifestUnknownError(err error) bool {
+	var ec dockerdistributionerrcode.ErrorCoder
+	if !errors.As(err, &ec) {
+		return false
+	}
+	return ec.ErrorCode() == dockerdistributionapi.ErrorCodeManifestUnknown
+}
+
+// isNotFoundImageError heuristically attempts to determine whether an error
+// is saying the remote source couldn't find the image (as opposed to an
+// authentication error, an I/O error etc.)
+// TODO drive this into containers/image properly
+func isNotFoundImageError(err error) bool {
+	return isDockerManifestUnknownError(err) ||
+		errors.Is(err, ocilayout.ImageNotFoundError{})
+}
+
+func (h *proxyHandler) openImageImpl(args []interface{}, allowNotFound bool) (replyBuf, error) {
 	h.lock.Lock()
 	defer h.lock.Unlock()
 	var ret replyBuf
@@ -218,9 +247,15 @@ func (h *proxyHandler) OpenImage(args []interface{}) (replyBuf, error) {
 	}
 	imgsrc, err := imgRef.NewImageSource(context.Background(), h.sysctx)
 	if err != nil {
+		if allowNotFound && isNotFoundImageError(err) {
+			ret.value = sentinelImageID
+			return ret, nil
+		}
 		return ret, err
 	}
 
+	// Note that we never return zero as an imageid; this code doesn't yet
+	// handle overflow though.
 	h.imageSerial++
 	openimg := &openImage{
 		id:  h.imageSerial,
@@ -232,6 +267,13 @@ func (h *proxyHandler) OpenImage(args []interface{}) (replyBuf, error) {
 	return ret, nil
 }
 
+// OpenImage accepts a string image reference i.e. TRANSPORT:REF - like `skopeo copy`.
+// The return value is an opaque integer handle.  If the image does not exist, zero
+// is returned.
+func (h *proxyHandler) OpenImageOptional(args []interface{}) (replyBuf, error) {
+	return h.openImageImpl(args, true)
+}
+
 func (h *proxyHandler) CloseImage(args []interface{}) (replyBuf, error) {
 	h.lock.Lock()
 	defer h.lock.Unlock()
@@ -278,6 +320,9 @@ func (h *proxyHandler) parseImageFromID(v interface{}) (*openImage, error) {
 	if err != nil {
 		return nil, err
 	}
+	if imgid == sentinelImageID {
+		return nil, fmt.Errorf("Invalid imageid value of zero")
+	}
 	imgref, ok := h.images[imgid]
 	if !ok {
 		return nil, fmt.Errorf("no image %v", imgid)
@@ -678,6 +723,8 @@ func (h *proxyHandler) processRequest(readBytes []byte) (rb replyBuf, terminate
 		rb, err = h.Initialize(req.Args)
 	case "OpenImage":
 		rb, err = h.OpenImage(req.Args)
+	case "OpenImageOptional":
+		rb, err = h.OpenImageOptional(req.Args)
 	case "CloseImage":
 		rb, err = h.CloseImage(req.Args)
 	case "GetManifest":

integration/proxy_test.go

@@ -20,6 +20,9 @@ import (
 // This image is known to be x86_64 only right now
 const knownNotManifestListedImage_x8664 = "docker://quay.io/coreos/11bot"
 
+// knownNotExtantImage would be very surprising if it did exist
+const knownNotExtantImage = "docker://quay.io/centos/centos:opensusewindowsubuntu"
+
 const expectedProxySemverMajor = "0.2"
 
 // request is copied from proxy.go
@@ -240,6 +243,29 @@ func runTestGetManifestAndConfig(p *proxy, img string) error {
 		return fmt.Errorf("OpenImage return value is %T", v)
 	}
 	imgid := uint32(imgidv)
+	if imgid == 0 {
+		return fmt.Errorf("got zero from expected image")
+	}
+
+	// Also verify the optional path
+	v, err = p.callNoFd("OpenImageOptional", []interface{}{knownNotManifestListedImage_x8664})
+	if err != nil {
+		return err
+	}
+
+	imgidv, ok = v.(float64)
+	if !ok {
+		return fmt.Errorf("OpenImageOptional return value is %T", v)
+	}
+	imgid2 := uint32(imgidv)
+	if imgid2 == 0 {
+		return fmt.Errorf("got zero from expected image")
+	}
+
+	_, err = p.callNoFd("CloseImage", []interface{}{imgid2})
+	if err != nil {
+		return err
+	}
 
 	_, manifestBytes, err := p.callReadAllBytes("GetManifest", []interface{}{imgid})
 	if err != nil {
@@ -292,6 +318,23 @@ func runTestGetManifestAndConfig(p *proxy, img string) error {
 	return nil
 }
 
+func runTestOpenImageOptionalNotFound(p *proxy, img string) error {
+	v, err := p.callNoFd("OpenImageOptional", []interface{}{img})
+	if err != nil {
+		return err
+	}
+
+	imgidv, ok := v.(float64)
+	if !ok {
+		return fmt.Errorf("OpenImageOptional return value is %T", v)
+	}
+	imgid := uint32(imgidv)
+	if imgid != 0 {
+		return fmt.Errorf("Unexpected optional image id %v", imgid)
+	}
+	return nil
+}
+
 func (s *ProxySuite) TestProxy(c *check.C) {
 	p, err := newProxy()
 	c.Assert(err, check.IsNil)
@@ -307,4 +350,10 @@ func (s *ProxySuite) TestProxy(c *check.C) {
 		err = fmt.Errorf("Testing image %s: %v", knownListImage, err)
 	}
 	c.Assert(err, check.IsNil)
+
+	err = runTestOpenImageOptionalNotFound(p, knownNotExtantImage)
+	if err != nil {
+		err = fmt.Errorf("Testing optional image %s: %v", knownNotExtantImage, err)
+	}
+	c.Assert(err, check.IsNil)
 }