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

remote: fix manifest add --annotation
mheon · Sep 29, 2022 · f52fede · f52fede
2 parents 3269ee9 + 32f54a8
commit f52fede
Show file tree

Hide file tree

Showing 8 changed files with 36 additions and 34 deletions.
diff --git a/pkg/api/handlers/libpod/manifests.go b/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"
@@ -148,7 +148,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

diff --git a/pkg/bindings/manifests/manifests.go b/pkg/bindings/manifests/manifests.go
@@ -11,7 +11,7 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/containers/image/v5/manifest"
+	"github.com/containers/common/libimage"
 	imageTypes "github.com/containers/image/v5/types"
 	"github.com/containers/podman/v4/pkg/auth"
 	"github.com/containers/podman/v4/pkg/bindings"
@@ -71,7 +71,7 @@ func Exists(ctx context.Context, name string, options *ExistsOptions) (bool, err
 }
 
 // Inspect returns a manifest list for a given name.
-func Inspect(ctx context.Context, name string, _ *InspectOptions) (*manifest.Schema2List, error) {
+func Inspect(ctx context.Context, name string, _ *InspectOptions) (*libimage.ManifestListData, error) {
 	conn, err := bindings.GetClient(ctx)
 	if err != nil {
 		return nil, err
@@ -83,7 +83,7 @@ func Inspect(ctx context.Context, name string, _ *InspectOptions) (*manifest.Sch
 	}
 	defer response.Body.Close()
 
-	var list manifest.Schema2List
+	var list libimage.ManifestListData
 	return &list, response.Process(&list)
 }
 

diff --git a/pkg/bindings/manifests/types.go b/pkg/bindings/manifests/types.go
@@ -22,7 +22,7 @@ type ExistsOptions struct {
 // AddOptions are optional options for adding manifest lists
 type AddOptions struct {
 	All           *bool
-	Annotation    map[string]string
+	Annotation    []string
 	Arch          *string
 	Features      []string
 	Images        []string
@@ -46,12 +46,12 @@ type ModifyOptions struct {
 	// Operation values are "update", "remove" and "annotate". This allows the service to
 	//   efficiently perform each update on a manifest list.
 	Operation   *string
-	All         *bool             // All when true, operate on all images in a manifest list that may be included in Images
-	Annotations map[string]string // Annotations to add to manifest list
-	Arch        *string           // Arch overrides the architecture for the image
-	Features    []string          // Feature list for the image
-	Images      []string          // Images is an optional list of images to add/remove to/from manifest list depending on operation
-	OS          *string           // OS overrides the operating system for the image
+	All         *bool    // All when true, operate on all images in a manifest list that may be included in Images
+	Annotations []string // Annotations to add to manifest list
+	Arch        *string  // Arch overrides the architecture for the image
+	Features    []string // Feature list for the image
+	Images      []string // Images is an optional list of images to add/remove to/from manifest list depending on operation
+	OS          *string  // OS overrides the operating system for the image
 	// OS features for the image
 	OSFeatures []string `json:"os_features" schema:"os_features"`
 	// OSVersion overrides the operating system for the image

diff --git a/pkg/bindings/manifests/types_add_options.go b/pkg/bindings/manifests/types_add_options.go
diff --git a/pkg/bindings/manifests/types_modify_options.go b/pkg/bindings/manifests/types_modify_options.go
diff --git a/pkg/domain/entities/manifest.go b/pkg/domain/entities/manifest.go
@@ -36,7 +36,7 @@ type ManifestAddOptions struct {
 // ManifestAnnotateOptions provides model for annotating manifest list
 type ManifestAnnotateOptions struct {
 	// Annotation to add to manifest list
-	Annotation []string `json:"annotation" schema:"annotation"`
+	Annotation []string `json:"annotations" schema:"annotations"`
 	// Arch overrides the architecture for the image
 	Arch string `json:"arch" schema:"arch"`
 	// Feature list for the image

diff --git a/pkg/domain/infra/tunnel/manifest.go b/pkg/domain/infra/tunnel/manifest.go
@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"strings"
 
 	"github.com/containers/image/v5/types"
 	"github.com/containers/podman/v4/pkg/bindings/images"
@@ -48,20 +47,9 @@ func (ir *ImageEngine) ManifestInspect(_ context.Context, name string) ([]byte,
 
 // ManifestAdd adds images to the manifest list
 func (ir *ImageEngine) ManifestAdd(_ context.Context, name string, imageNames []string, opts entities.ManifestAddOptions) (string, error) {
-	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 {
-			spec := strings.SplitN(annotationSpec, "=", 2)
-			if len(spec) != 2 {
-				return "", fmt.Errorf("no value given for annotation %q", spec[0])
-			}
-			annotations[spec[0]] = spec[1]
-		}
-		options.WithAnnotation(annotations)
-	}
+	options := new(manifests.AddOptions).WithAll(opts.All).WithAnnotation(opts.Annotation).WithArch(opts.Arch)
+	options.WithVariant(opts.Variant).WithFeatures(opts.Features).WithImages(imageNames).WithOS(opts.OS)
+	options.WithOSVersion(opts.OSVersion).WithUsername(opts.Username).WithPassword(opts.Password).WithAuthfile(opts.Authfile)
 	if s := opts.SkipTLSVerify; s != types.OptionalBoolUndefined {
 		if s == types.OptionalBoolTrue {
 			options.WithSkipTLSVerify(true)

diff --git a/test/e2e/manifest_test.go b/test/e2e/manifest_test.go
@@ -165,6 +165,20 @@ 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=fuga", "foo", imageList})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		session = podmanTest.Podman([]string{"manifest", "inspect", "foo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		Expect(session.OutputToString()).To(ContainSubstring(`"annotations"`))
+		Expect(session.OutputToString()).To(ContainSubstring(`"hoge": "fuga"`))
+	})
+
 	It("add --os", func() {
 		session := podmanTest.Podman([]string{"manifest", "create", "foo"})
 		session.WaitWithDefaultTimeout()