Fix manifest 4.0 Endpoints

Branch forced 4.0 only endpoints

Signed-off-by: Jhon Honce <jhonce@redhat.com>

pkg/api/handlers/libpod/manifests.go

@@ -401,7 +401,7 @@ func ManifestModify(w http.ResponseWriter, r *http.Request) {
 	case len(report.Errors) > 0 && len(report.Images) > 0:
 		statusCode = http.StatusConflict
 	case len(report.Errors) > 0:
-		statusCode = http.StatusInternalServerError
+		statusCode = http.StatusBadRequest
 	}
 	utils.WriteResponse(w, statusCode, report)
 }

pkg/bindings/manifests/manifests.go

@@ -2,10 +2,8 @@ package manifests
 
 import (
 	"context"
-	"errors"
-	"fmt"
+	"io/ioutil"
 	"net/http"
-	"net/url"
 	"strconv"
 	"strings"
 
@@ -14,8 +12,10 @@ import (
 	"github.com/containers/podman/v4/pkg/api/handlers"
 	"github.com/containers/podman/v4/pkg/bindings"
 	"github.com/containers/podman/v4/pkg/bindings/images"
-	"github.com/containers/podman/v4/version"
+	"github.com/containers/podman/v4/pkg/domain/entities"
+	"github.com/containers/podman/v4/pkg/errorhandling"
 	jsoniter "github.com/json-iterator/go"
+	"github.com/pkg/errors"
 )
 
 // Create creates a manifest for the given name.  Optional images to be associated with
@@ -91,74 +91,27 @@ func Add(ctx context.Context, name string, options *AddOptions) (string, error)
 		options = new(AddOptions)
 	}
 
-	if bindings.ServiceVersion(ctx).GTE(semver.MustParse("4.0.0")) {
-		optionsv4 := ModifyOptions{
-			All:         options.All,
-			Annotations: options.Annotation,
-			Arch:        options.Arch,
-			Features:    options.Features,
-			Images:      options.Images,
-			OS:          options.OS,
-			OSFeatures:  nil,
-			OSVersion:   options.OSVersion,
-			Variant:     options.Variant,
-		}
-		optionsv4.WithOperation("update")
-		return Modify(ctx, name, options.Images, &optionsv4)
+	optionsv4 := ModifyOptions{
+		All:         options.All,
+		Annotations: options.Annotation,
+		Arch:        options.Arch,
+		Features:    options.Features,
+		Images:      options.Images,
+		OS:          options.OS,
+		OSFeatures:  nil,
+		OSVersion:   options.OSVersion,
+		Variant:     options.Variant,
 	}
-
-	// API Version < 4.0.0
-	conn, err := bindings.GetClient(ctx)
-	if err != nil {
-		return "", err
-	}
-	opts, err := jsoniter.MarshalToString(options)
-	if err != nil {
-		return "", err
-	}
-	reader := strings.NewReader(opts)
-
-	headers := make(http.Header)
-	v := version.APIVersion[version.Libpod][version.MinimalAPI]
-	headers.Add("API-Version",
-		fmt.Sprintf("%d.%d.%d", v.Major, v.Minor, v.Patch))
-	response, err := conn.DoRequest(ctx, reader, http.MethodPost, "/manifests/%s/add", nil, headers, name)
-	if err != nil {
-		return "", err
-	}
-	defer response.Body.Close()
-
-	var idr handlers.IDResponse
-	return idr.ID, response.Process(&idr)
+	optionsv4.WithOperation("update")
+	return Modify(ctx, name, options.Images, &optionsv4)
 }
 
 // Remove deletes a manifest entry from a manifest list.  Both name and the digest to be
 // removed are mandatory inputs.  The ID of the new manifest list is returned as a string.
 func Remove(ctx context.Context, name, digest string, _ *RemoveOptions) (string, error) {
-	if bindings.ServiceVersion(ctx).GTE(semver.MustParse("4.0.0")) {
-		optionsv4 := new(ModifyOptions).WithOperation("remove")
-		return Modify(ctx, name, []string{digest}, optionsv4)
-	}
+	optionsv4 := new(ModifyOptions).WithOperation("remove")
+	return Modify(ctx, name, []string{digest}, optionsv4)
 
-	// API Version < 4.0.0
-	conn, err := bindings.GetClient(ctx)
-	if err != nil {
-		return "", err
-	}
-
-	headers := http.Header{}
-	headers.Add("API-Version", "3.4.0")
-
-	params := url.Values{}
-	params.Set("digest", digest)
-	response, err := conn.DoRequest(ctx, nil, http.MethodDelete, "/manifests/%s", params, headers, name)
-	if err != nil {
-		return "", err
-	}
-	defer response.Body.Close()
-
-	var idr handlers.IDResponse
-	return idr.ID, response.Process(&idr)
 }
 
 // Push takes a manifest list and pushes to a destination.  If the destination is not specified,
@@ -229,8 +182,36 @@ func Modify(ctx context.Context, name string, images []string, options *ModifyOp
 	}
 	defer response.Body.Close()
 
-	var idr handlers.IDResponse
-	return idr.ID, response.Process(&idr)
+	data, err := ioutil.ReadAll(response.Body)
+	if err != nil {
+		return "", errors.Wrap(err, "unable to process API response")
+	}
+
+	if response.IsSuccess() || response.IsRedirection() {
+		var report entities.ManifestModifyReport
+		if err = jsoniter.Unmarshal(data, &report); err != nil {
+			return "", errors.Wrap(err, "unable to decode API response")
+		}
+
+		err = errorhandling.JoinErrors(report.Errors)
+		if err != nil {
+			errModel := errorhandling.ErrorModel{
+				Because:      (errors.Cause(err)).Error(),
+				Message:      err.Error(),
+				ResponseCode: response.StatusCode,
+			}
+			return report.ID, &errModel
+		}
+		return report.ID, nil
+	} else {
+		errModel := errorhandling.ErrorModel{
+			ResponseCode: response.StatusCode,
+		}
+		if err = jsoniter.Unmarshal(data, &errModel); err != nil {
+			return "", errors.Wrap(err, "unable to decode API response")
+		}
+		return "", &errModel
+	}
 }
 
 // Annotate modifies the given manifest list using options and the optional list of images

pkg/bindings/test/common_test.go

@@ -8,6 +8,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"strings"
+	"testing"
 	"time"
 
 	"github.com/containers/podman/v4/libpod/define"
@@ -151,7 +152,12 @@ func createTempDirInTempDir() (string, error) {
 }
 
 func (b *bindingTest) startAPIService() *gexec.Session {
-	cmd := []string{"--log-level=debug", "system", "service", "--timeout=0", b.sock}
+	logLevel := "debug"
+	if testing.Verbose() {
+		logLevel = "trace"
+	}
+
+	cmd := []string{"--log-level=" + logLevel, "system", "service", "--timeout=0", b.sock}
 	session := b.runPodman(cmd)
 
 	sock := strings.TrimPrefix(b.sock, "unix://")

pkg/bindings/test/manifests_test.go

@@ -87,7 +87,6 @@ var _ = Describe("podman manifest", func() {
 
 		list, err := manifests.Inspect(bt.conn, id, nil)
 		Expect(err).ToNot(HaveOccurred())
-
 		Expect(len(list.Manifests)).To(BeNumerically("==", 1))
 
 		// add bogus name to existing list should fail
@@ -96,7 +95,7 @@ var _ = Describe("podman manifest", func() {
 		Expect(err).To(HaveOccurred())
 
 		code, _ = bindings.CheckResponseCode(err)
-		Expect(code).To(BeNumerically("==", http.StatusInternalServerError))
+		Expect(code).To(BeNumerically("==", http.StatusBadRequest))
 	})
 
 	It("remove digest", func() {
@@ -129,7 +128,6 @@ var _ = Describe("podman manifest", func() {
 		// removal on good manifest with good digest should work
 		data, err = manifests.Inspect(bt.conn, id, nil)
 		Expect(err).ToNot(HaveOccurred())
-
 		Expect(data.Manifests).Should(BeEmpty())
 	})