compat API push: fix error handling

Make sure that the push endpoint does not always return 200 even in case
of a push failure.  Some of the code had to be massaged since encoding a
report implies sending a 200.

Fixes: #18751
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>

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)
-	w.Header().Set("Content-Type", "application/json")
-	flush()
-
-	var report jsonmessage.JSONMessage
-	enc := json.NewEncoder(w)
-	enc.SetEscapeHTML(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())
+	statusWritten := false
+	writeStatusCode := func(code int) {
+		if !statusWritten {
+			w.WriteHeader(code)
+			w.Header().Set("Content-Type", "application/json")
+			flush()
+			statusWritten = true
+		}
+	}
+
+	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/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" 200 # Error is in the response
+t POST   "images/quay.io/libpod/alpine/push?destination=localhost:9999/do/not:exist" 500
 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