Merge pull request #18954 from vrothberg/fix-15828

compat API create/pull/push: fix error handling

go.mod

@@ -25,6 +25,7 @@ require (
 	github.com/crc-org/vfkit v0.0.5-0.20230602131541-3d57f09010c9
 	github.com/cyphar/filepath-securejoin v0.2.3
 	github.com/digitalocean/go-qemu v0.0.0-20221209210016-f035778c97f7
+	github.com/docker/distribution v2.8.2+incompatible
 	github.com/docker/docker v24.0.2+incompatible
 	github.com/docker/go-connections v0.4.1-0.20210727194412-58542c764a11
 	github.com/docker/go-plugins-helpers v0.0.0-20211224144127-6eecb7beb651
@@ -90,7 +91,6 @@ require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/digitalocean/go-libvirt v0.0.0-20220804181439-8648fbde413e // indirect
 	github.com/disiqueira/gotree/v3 v3.0.2 // indirect
-	github.com/docker/distribution v2.8.2+incompatible // indirect
 	github.com/docker/docker-credential-helpers v0.7.0 // indirect
 	github.com/felixge/httpsnoop v1.0.3 // indirect
 	github.com/fsouza/go-dockerclient v1.9.7 // indirect

pkg/api/handlers/compat/images.go

@@ -23,6 +23,7 @@ import (
 	"github.com/containers/podman/v4/pkg/domain/infra/abi"
 	"github.com/containers/podman/v4/pkg/util"
 	"github.com/containers/storage"
+	"github.com/docker/distribution/registry/api/errcode"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/gorilla/schema"
 	"github.com/opencontainers/go-digest"
@@ -313,18 +314,24 @@ func CreateImageFromImage(w http.ResponseWriter, r *http.Request) {
 		pullResChan <- pullResult{images: pulledImages, err: err}
 	}()
 
+	enc := json.NewEncoder(w)
+	enc.SetEscapeHTML(true)
+
 	flush := func() {
 		if flusher, ok := w.(http.Flusher); ok {
 			flusher.Flush()
 		}
 	}
 
-	w.WriteHeader(http.StatusOK)
+	statusWritten := false
-	w.Header().Set("Content-Type", "application/json")
+	writeStatusCode := func(code int) {
-	flush()
+		if !statusWritten {
-
+			w.WriteHeader(code)
-	enc := json.NewEncoder(w)
+			w.Header().Set("Content-Type", "application/json")
-	enc.SetEscapeHTML(true)
+			flush()
+			statusWritten = true
+		}
+	}
 
 loop: // break out of for/select infinite loop
 	for {
@@ -332,6 +339,7 @@ loop: // break out of for/select infinite loop
 		report.Progress = &jsonmessage.JSONProgress{}
 		select {
 		case e := <-progress:
+			writeStatusCode(http.StatusOK)
 			switch e.Event {
 			case types.ProgressEventNewArtifact:
 				report.Status = "Pulling fs layer"
@@ -352,14 +360,20 @@ loop: // break out of for/select infinite loop
 			flush()
 		case pullRes := <-pullResChan:
 			err := pullRes.err
-			pulledImages := pullRes.images
 			if err != nil {
+				var errcd errcode.ErrorCoder
+				if errors.As(err, &errcd) {
+					writeStatusCode(errcd.ErrorCode().Descriptor().HTTPStatusCode)
+				} else {
+					writeStatusCode(http.StatusInternalServerError)
+				}
 				msg := err.Error()
 				report.Error = &jsonmessage.JSONError{
 					Message: msg,
 				}
 				report.ErrorMessage = msg
 			} else {
+				pulledImages := pullRes.images
 				if len(pulledImages) > 0 {
 					img := pulledImages[0].ID()
 					if utils.IsLibpodRequest(r) {
@@ -374,6 +388,7 @@ loop: // break out of for/select infinite loop
 						Message: msg,
 					}
 					report.ErrorMessage = msg
+					writeStatusCode(http.StatusInternalServerError)
 				}
 			}
 			if err := enc.Encode(report); err != nil {

pkg/api/handlers/compat/images_push.go

@@ -108,24 +108,36 @@ func PushImage(w http.ResponseWriter, r *http.Request) {
 		destination = imageName
 	}
 
+	enc := json.NewEncoder(w)
+	enc.SetEscapeHTML(true)
+
 	flush := func() {}
 	if flusher, ok := w.(http.Flusher); ok {
 		flush = flusher.Flush
 	}
 
-	w.WriteHeader(http.StatusOK)
+	statusWritten := false
-	w.Header().Set("Content-Type", "application/json")
+	writeStatusCode := func(code int) {
-	flush()
+		if !statusWritten {
-
+			w.WriteHeader(code)
-	var report jsonmessage.JSONMessage
+			w.Header().Set("Content-Type", "application/json")
-	enc := json.NewEncoder(w)
+			flush()
-	enc.SetEscapeHTML(true)
+			statusWritten = true
-
+		}
-	report.Status = fmt.Sprintf("The push refers to repository [%s]", imageName)
+	}
-	if err := enc.Encode(report); err != nil {
+
-		logrus.Warnf("Failed to json encode error %q", err.Error())
+	referenceWritten := false
+	writeReference := func() {
+		if !referenceWritten {
+			var report jsonmessage.JSONMessage
+			report.Status = fmt.Sprintf("The push refers to repository [%s]", imageName)
+			if err := enc.Encode(report); err != nil {
+				logrus.Warnf("Failed to json encode error %q", err.Error())
+			}
+			flush()
+			referenceWritten = true
+		}
 	}
-	flush()
 
 	pushErrChan := make(chan error)
 	var pushReport *entities.ImagePushReport
@@ -137,10 +149,12 @@ func PushImage(w http.ResponseWriter, r *http.Request) {
 
 loop: // break out of for/select infinite loop
 	for {
-		report = jsonmessage.JSONMessage{}
+		var report jsonmessage.JSONMessage
 
 		select {
 		case e := <-options.Progress:
+			writeStatusCode(http.StatusOK)
+			writeReference()
 			switch e.Event {
 			case types.ProgressEventNewArtifact:
 				report.Status = "Preparing"
@@ -165,8 +179,11 @@ loop: // break out of for/select infinite loop
 			if err != nil {
 				var msg string
 				if errors.Is(err, storage.ErrImageUnknown) {
+					// Image may have been removed in the meantime.
+					writeStatusCode(http.StatusNotFound)
 					msg = "An image does not exist locally with the tag: " + imageName
 				} else {
+					writeStatusCode(http.StatusInternalServerError)
 					msg = err.Error()
 				}
 				report.Error = &jsonmessage.JSONError{
@@ -178,6 +195,9 @@ loop: // break out of for/select infinite loop
 				}
 				flush()
 				break loop
+			} else {
+				writeStatusCode(http.StatusOK)
+				writeReference() // There may not be any progess, so make sure the reference gets written
 			}
 
 			tag := query.Tag

test/apiv2/10-images.at

@@ -72,6 +72,7 @@ t POST "images/create?fromSrc=-&repo=myimage&tag=mytag" - 200
 t GET "images/myimage:mytag/json" 200 \
   .Id~'^sha256:[0-9a-f]\{64\}$' \
   .RepoTags[0]="docker.io/library/myimage:mytag"
+t POST /images/create?fromImage=busybox:invalidtag123 404
 
 # Display the image history
 t GET libpod/images/nonesuch/history 404

test/apiv2/12-imagesMore.at

@@ -25,7 +25,7 @@ t GET libpod/images/$IMAGE/json 200 \
   .RepoTags[1]=localhost:$REGISTRY_PORT/myrepo:mytag
 
 # Push to local registry...
-t POST "images/localhost:$REGISTRY_PORT/myrepo/push?tag=mytag" 200 \
+t POST "/v1.40/images/localhost:$REGISTRY_PORT/myrepo/push?tag=mytag" 500 \
   .error~".*x509: certificate signed by unknown authority"
 t POST "images/localhost:$REGISTRY_PORT/myrepo/push?tlsVerify=false&tag=mytag" 200 \
   .error~null

test/apiv2/70-short-names.at

@@ -82,10 +82,9 @@ t DELETE "images/foo"                                200
 
 ########## PUSH
 
-t POST   "images/alpine/push?destination=localhost:9999/do:exist"                    200
+t POST   "images/quay.io/libpod/alpine/push?destination=localhost:9999/do/not:exist" 500
-t POST   "images/quay.io/libpod/alpine/push?destination=localhost:9999/do/not:exist" 200 # Error is in the response
 t POST   "images/quay.io/libpod/alpine/tag?repo=foo"                                 201
-t POST   "images/foo/push?destination=localhost:9999/do/not:exist"                   200 # Error is in the response
+t POST   "images/foo/push?destination=localhost:9999/do/not:exist"                   500
 t DELETE "images/foo"                                                                200
 
 
@@ -125,7 +124,7 @@ t POST   "images/alpine/tag?repo=foo"                                 201
 t GET    "images/localhost/foo:latest/get"                            200
 t DELETE "images/foo"                                                 200
 t GET    "images/alpine/history"                                      200
-t POST   "images/alpine/push?destination=localhost:9999/do/not:exist" 200 # Error is in the response
+t POST   "images/alpine/push?destination=localhost:9999/do/not:exist" 500
 t POST   "containers/create" Image=alpine                             201
 cid=$(jq -r '.Id' <<<"$output")
 t POST   "commit?container=$cid&repo=foo&tag=tag"                     201