Merge pull request containers#16004 from sstosh/remote-manifest-annotate

remote: fix manifest add --annotation

pkg/api/handlers/libpod/manifests.go

@@ -11,8 +11,8 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/containers/common/libimage"
 	"github.com/containers/image/v5/docker/reference"
-	"github.com/containers/image/v5/manifest"
 	"github.com/containers/image/v5/types"
 	"github.com/containers/podman/v4/libpod"
 	"github.com/containers/podman/v4/pkg/api/handlers"
@@ -22,6 +22,7 @@ import (
 	"github.com/containers/podman/v4/pkg/channel"
 	"github.com/containers/podman/v4/pkg/domain/entities"
 	"github.com/containers/podman/v4/pkg/domain/infra/abi"
+	envLib "github.com/containers/podman/v4/pkg/env"
 	"github.com/containers/podman/v4/pkg/errorhandling"
 	"github.com/gorilla/mux"
 	"github.com/gorilla/schema"
@@ -164,7 +165,7 @@ func ManifestInspect(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	var schema2List manifest.Schema2List
+	var schema2List libimage.ManifestListData
 	if err := json.Unmarshal(rawManifest, &schema2List); err != nil {
 		utils.Error(w, http.StatusInternalServerError, err)
 		return
@@ -460,6 +461,24 @@ func ManifestModify(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if len(body.ManifestAddOptions.Annotation) != 0 {
+		if len(body.ManifestAddOptions.Annotations) != 0 {
+			utils.Error(w, http.StatusBadRequest, fmt.Errorf("can not set both Annotation and Annotations"))
+			return
+		}
+		annotations := make(map[string]string)
+		for _, annotationSpec := range body.ManifestAddOptions.Annotation {
+			spec := strings.SplitN(annotationSpec, "=", 2)
+			if len(spec) != 2 {
+				utils.Error(w, http.StatusBadRequest, fmt.Errorf("no value given for annotation %q", spec[0]))
+				return
+			}
+			annotations[spec[0]] = spec[1]
+		}
+		body.ManifestAddOptions.Annotations = envLib.Join(body.ManifestAddOptions.Annotations, annotations)
+		body.ManifestAddOptions.Annotation = nil
+	}
+
 	if tlsVerify, ok := r.URL.Query()["tlsVerify"]; ok {
 		tls, err := strconv.ParseBool(tlsVerify[len(tlsVerify)-1])
 		if err != nil {

pkg/bindings/manifests/manifests.go

@@ -11,6 +11,7 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/containers/common/libimage"
 	"github.com/containers/image/v5/manifest"
 	imageTypes "github.com/containers/image/v5/types"
 	"github.com/containers/podman/v4/pkg/auth"
@@ -101,6 +102,39 @@ func Inspect(ctx context.Context, name string, options *InspectOptions) (*manife
 	return &list, response.Process(&list)
 }
 
+// InspectListData returns a manifest list for a given name.
+// Contains exclusive field like `annotations` which is only
+// present in OCI spec and not in docker image spec.
+func InspectListData(ctx context.Context, name string, options *InspectOptions) (*libimage.ManifestListData, error) {
+	conn, err := bindings.GetClient(ctx)
+	if err != nil {
+		return nil, err
+	}
+	if options == nil {
+		options = new(InspectOptions)
+	}
+
+	params, err := options.ToParams()
+	if err != nil {
+		return nil, err
+	}
+	// SkipTLSVerify is special.  We need to delete the param added by
+	// ToParams() and change the key and flip the bool
+	if options.SkipTLSVerify != nil {
+		params.Del("SkipTLSVerify")
+		params.Set("tlsVerify", strconv.FormatBool(!options.GetSkipTLSVerify()))
+	}
+
+	response, err := conn.DoRequest(ctx, nil, http.MethodGet, "/manifests/%s/json", params, nil, name)
+	if err != nil {
+		return nil, err
+	}
+	defer response.Body.Close()
+
+	var list libimage.ManifestListData
+	return &list, response.Process(&list)
+}
+
 // Add adds a manifest to a given manifest list.  Additional options for the manifest
 // can also be specified.  The ID of the new manifest list is returned as a string
 func Add(ctx context.Context, name string, options *AddOptions) (string, error) {

pkg/domain/entities/manifest.go

@@ -43,6 +43,8 @@ type ManifestAddOptions struct {
 type ManifestAnnotateOptions struct {
 	// Annotation to add to manifest list
 	Annotation []string `json:"annotation" schema:"annotation"`
+	// Annotations to add to manifest list by a map which is prefferred over Annotation
+	Annotations map[string]string `json:"annotations" schema:"annotations"`
 	// Arch overrides the architecture for the image
 	Arch string `json:"arch" schema:"arch"`
 	// Feature list for the image

pkg/domain/infra/abi/manifest.go

@@ -19,6 +19,7 @@ import (
 	"github.com/containers/image/v5/transports/alltransports"
 	"github.com/containers/image/v5/types"
 	"github.com/containers/podman/v4/pkg/domain/entities"
+	envLib "github.com/containers/podman/v4/pkg/env"
 	"github.com/containers/storage"
 	"github.com/opencontainers/go-digest"
 	imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
@@ -231,8 +232,9 @@ func (ir *ImageEngine) ManifestAdd(ctx context.Context, name string, images []st
 				}
 				annotations[spec[0]] = spec[1]
 			}
-			annotateOptions.Annotations = annotations
+			opts.Annotations = envLib.Join(opts.Annotations, annotations)
 		}
+		annotateOptions.Annotations = opts.Annotations
 
 		if err := manifestList.AnnotateInstance(instanceDigest, annotateOptions); err != nil {
 			return "", err
@@ -269,8 +271,9 @@ func (ir *ImageEngine) ManifestAnnotate(ctx context.Context, name, image string,
 			}
 			annotations[spec[0]] = spec[1]
 		}
-		annotateOptions.Annotations = annotations
+		opts.Annotations = envLib.Join(opts.Annotations, annotations)
 	}
+	annotateOptions.Annotations = opts.Annotations
 
 	if err := manifestList.AnnotateInstance(instanceDigest, annotateOptions); err != nil {
 		return "", err

pkg/domain/infra/tunnel/manifest.go

@@ -11,6 +11,7 @@ import (
 	"github.com/containers/podman/v4/pkg/bindings/images"
 	"github.com/containers/podman/v4/pkg/bindings/manifests"
 	"github.com/containers/podman/v4/pkg/domain/entities"
+	envLib "github.com/containers/podman/v4/pkg/env"
 )
 
 // ManifestCreate implements manifest create via ImageEngine
@@ -43,7 +44,7 @@ func (ir *ImageEngine) ManifestInspect(ctx context.Context, name string, opts en
 		}
 	}
 
-	list, err := manifests.Inspect(ir.ClientCtx, name, options)
+	list, err := manifests.InspectListData(ir.ClientCtx, name, options)
 	if err != nil {
 		return nil, fmt.Errorf("getting content of manifest list or image %s: %w", name, err)
 	}
@@ -60,6 +61,7 @@ func (ir *ImageEngine) ManifestAdd(_ context.Context, name string, imageNames []
 	options := new(manifests.AddOptions).WithAll(opts.All).WithArch(opts.Arch).WithVariant(opts.Variant)
 	options.WithFeatures(opts.Features).WithImages(imageNames).WithOS(opts.OS).WithOSVersion(opts.OSVersion)
 	options.WithUsername(opts.Username).WithPassword(opts.Password).WithAuthfile(opts.Authfile)
+
 	if len(opts.Annotation) != 0 {
 		annotations := make(map[string]string)
 		for _, annotationSpec := range opts.Annotation {
@@ -69,8 +71,10 @@ func (ir *ImageEngine) ManifestAdd(_ context.Context, name string, imageNames []
 			}
 			annotations[spec[0]] = spec[1]
 		}
-		options.WithAnnotation(annotations)
+		opts.Annotations = envLib.Join(opts.Annotations, annotations)
 	}
+	options.WithAnnotation(opts.Annotations)
+
 	if s := opts.SkipTLSVerify; s != types.OptionalBoolUndefined {
 		if s == types.OptionalBoolTrue {
 			options.WithSkipTLSVerify(true)

test/apiv2/15-manifest.at

@@ -28,7 +28,16 @@ EOF
 )
 
 t POST /v3.4.0/libpod/manifests/$id_abc/add images="[\"containers-storage:$id_abc_image\"]" 200
-t PUT /v4.0.0/libpod/manifests/$id_xyz operation='update' images="[\"containers-storage:$id_xyz_image\"]" 200
+t PUT /v4.0.0/libpod/manifests/$id_xyz operation='update' images="[\"containers-storage:$id_xyz_image\"]" annotations="{\"foo\":\"bar\"}" annotation="[\"hoge=fuga\"]" 400 \
+    .cause='can not set both Annotation and Annotations'
+
+t PUT /v4.0.0/libpod/manifests/$id_xyz operation='update' images="[\"containers-storage:$id_xyz_image\"]" annotations="{\"foo\":\"bar\"}" 200
+t GET /v4.0.0/libpod/manifests/$id_xyz/json 200 \
+    .manifests[0].annotations.foo="bar"
+
+t PUT /v4.0.0/libpod/manifests/$id_xyz operation='update' images="[\"containers-storage:$id_xyz_image\"]" annotation="[\"hoge=fuga\"]" 200
+t GET /v4.0.0/libpod/manifests/$id_xyz/json 200 \
+    .manifests[0].annotations.hoge="fuga"
 
 t POST "/v3.4.0/libpod/manifests/abc:latest/push?destination=localhost:$REGISTRY_PORT%2Fabc:latest&tlsVerify=false&all=true" 200
 t POST "/v4.0.0/libpod/manifests/xyz:latest/registry/localhost:$REGISTRY_PORT%2Fxyz:latest?all=true" 400 \

test/e2e/manifest_test.go

@@ -1,10 +1,12 @@
 package integration
 
 import (
+	"encoding/json"
 	"os"
 	"path/filepath"
 	"strings"
 
+	"github.com/containers/common/libimage"
 	podmanRegistry "github.com/containers/podman/v4/hack/podman-registry-go"
 	. "github.com/containers/podman/v4/test/utils"
 	"github.com/containers/storage/pkg/archive"
@@ -165,6 +167,27 @@ var _ = Describe("Podman manifest", func() {
 			))
 	})
 
+	It("add --annotation", func() {
+		session := podmanTest.Podman([]string{"manifest", "create", "foo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		session = podmanTest.Podman([]string{"manifest", "add", "--annotation", "hoge", "foo", imageList})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(125))
+		Expect(session.ErrorToString()).To(ContainSubstring("no value given for annotation"))
+		session = podmanTest.Podman([]string{"manifest", "add", "--annotation", "hoge=fuga", "foo", imageList})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		session = podmanTest.Podman([]string{"manifest", "inspect", "foo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		var inspect libimage.ManifestListData
+		err := json.Unmarshal(session.Out.Contents(), &inspect)
+		Expect(err).To(BeNil())
+		Expect(inspect.Manifests[0].Annotations).To(Equal(map[string]string{"hoge": "fuga"}))
+	})
+
 	It("add --os", func() {
 		session := podmanTest.Podman([]string{"manifest", "create", "foo"})
 		session.WaitWithDefaultTimeout()