diff --git a/cmd/podman/manifest/annotate.go b/cmd/podman/manifest/annotate.go index 60765a9b22..9a54fe2166 100644 --- a/cmd/podman/manifest/annotate.go +++ b/cmd/podman/manifest/annotate.go @@ -119,7 +119,7 @@ func annotate(cmd *cobra.Command, args []string) error { } else { opts.Annotations = annotations } - id, err := registry.ImageEngine().ManifestAnnotate(registry.Context(), args[0], args[1], opts) + id, err := registry.ImageEngine().ManifestAnnotate(registry.Context(), listImageSpec, instanceSpec, opts) if err != nil { return err } diff --git a/pkg/api/handlers/libpod/manifests.go b/pkg/api/handlers/libpod/manifests.go index d6db94dbd8..d7ab50f918 100644 --- a/pkg/api/handlers/libpod/manifests.go +++ b/pkg/api/handlers/libpod/manifests.go @@ -706,14 +706,20 @@ func ManifestModify(w http.ResponseWriter, r *http.Request) { } case strings.EqualFold("annotate", body.Operation): options := body.ManifestAnnotateOptions - for _, image := range body.Images { + images := []string{""} + if len(body.Images) > 0 { + images = body.Images + } + for _, image := range images { id, err := imageEngine.ManifestAnnotate(r.Context(), name, image, options) if err != nil { report.Errors = append(report.Errors, err) continue } report.ID = id - report.Images = append(report.Images, image) + if image != "" { + report.Images = append(report.Images, image) + } } default: utils.Error(w, http.StatusBadRequest, fmt.Errorf("illegal operation %q for %q", body.Operation, r.URL.String())) diff --git a/pkg/bindings/manifests/types.go b/pkg/bindings/manifests/types.go index aae36c9a64..82a842aab7 100644 --- a/pkg/bindings/manifests/types.go +++ b/pkg/bindings/manifests/types.go @@ -19,7 +19,7 @@ type InspectOptions struct { type CreateOptions struct { All *bool Amend *bool - Annotation map[string]string + Annotation map[string]string `json:"annotations" schema:"annotations"` } // ExistsOptions are optional options for checking @@ -35,7 +35,7 @@ type ExistsOptions struct { type AddOptions struct { All *bool - Annotation map[string]string + Annotation map[string]string `json:"annotations" schema:"annotations"` Arch *string Features []string OS *string @@ -54,7 +54,7 @@ type AddOptions struct { // //go:generate go run ../generator/generator.go AddArtifactOptions type AddArtifactOptions struct { - Annotation map[string]string + Annotation map[string]string `json:"annotations" schema:"annotations"` Arch *string Features []string OS *string @@ -87,13 +87,14 @@ type ModifyOptions struct { 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 the entries for Images in the manifest list - Arch *string // Arch overrides the architecture for the image - Features []string // Feature list for the image - OS *string // OS overrides the operating system for the image - OSFeatures []string `json:"os_features" schema:"os_features"` // OSFeatures overrides the OS features for the image - OSVersion *string `json:"os_version" schema:"os_version"` // OSVersion overrides the operating system version for the image - Variant *string // Variant overrides the architecture variant for the image + Annotations map[string]string // Annotations to add to the entries for Images in the manifest list + IndexAnnotations map[string]string `json:"index_annotations" schema:"index_annotations"` // Annotations to add to the manifest list as a whole + Arch *string // Arch overrides the architecture for the image + Features []string // Feature list for the image + OS *string // OS overrides the operating system for the image + OSFeatures []string `json:"os_features" schema:"os_features"` // OSFeatures overrides the OS features for the image + OSVersion *string `json:"os_version" schema:"os_version"` // OSVersion overrides the operating system version for the image + Variant *string // Variant overrides the architecture variant for the image Images []string // Images is an optional list of images to add/remove to/from manifest list depending on operation Authfile *string diff --git a/pkg/bindings/manifests/types_modify_options.go b/pkg/bindings/manifests/types_modify_options.go index 1957b8027a..4ae71567f4 100644 --- a/pkg/bindings/manifests/types_modify_options.go +++ b/pkg/bindings/manifests/types_modify_options.go @@ -63,6 +63,21 @@ func (o *ModifyOptions) GetAnnotations() map[string]string { return o.Annotations } +// WithIndexAnnotations set annotations to add to the manifest list as a whole +func (o *ModifyOptions) WithIndexAnnotations(value map[string]string) *ModifyOptions { + o.IndexAnnotations = value + return o +} + +// GetIndexAnnotations returns value of annotations to add to the manifest list as a whole +func (o *ModifyOptions) GetIndexAnnotations() map[string]string { + if o.IndexAnnotations == nil { + var z map[string]string + return z + } + return o.IndexAnnotations +} + // WithArch set arch overrides the architecture for the image func (o *ModifyOptions) WithArch(value string) *ModifyOptions { o.Arch = &value diff --git a/pkg/domain/entities/manifest.go b/pkg/domain/entities/manifest.go index a81e76c076..4a2cde8a78 100644 --- a/pkg/domain/entities/manifest.go +++ b/pkg/domain/entities/manifest.go @@ -82,9 +82,9 @@ type ManifestAnnotateOptions struct { // Variant for the item in the manifest list Variant string `json:"variant" schema:"variant"` // IndexAnnotation is a slice of key=value annotations to add to the manifest list itself - IndexAnnotation []string `json:"index_annotation" schema:"annotation"` + IndexAnnotation []string `json:"index_annotation" schema:"index_annotation"` // IndexAnnotations is a map of key:value annotations to add to the manifest list itself, by a map which is preferred over IndexAnnotation - IndexAnnotations map[string]string `json:"index_annotations" schema:"annotations"` + IndexAnnotations map[string]string `json:"index_annotations" schema:"index_annotations"` // IndexSubject is a subject value to set in the manifest list itself IndexSubject string `json:"subject" schema:"subject"` } diff --git a/pkg/domain/infra/abi/manifest.go b/pkg/domain/infra/abi/manifest.go index 253e173474..47b15718aa 100644 --- a/pkg/domain/infra/abi/manifest.go +++ b/pkg/domain/infra/abi/manifest.go @@ -49,9 +49,10 @@ func (ir *ImageEngine) ManifestCreate(ctx context.Context, name string, images [ } } - annotateOptions := &libimage.ManifestListAnnotateOptions{} if len(opts.Annotations) != 0 { - annotateOptions.IndexAnnotations = opts.Annotations + annotateOptions := &libimage.ManifestListAnnotateOptions{ + IndexAnnotations: opts.Annotations, + } if err := manifestList.AnnotateInstance("", annotateOptions); err != nil { return "", err } @@ -230,18 +231,14 @@ func (ir *ImageEngine) ManifestAdd(ctx context.Context, name string, images []st Variant: opts.Variant, Subject: opts.IndexSubject, } - if len(opts.Annotation) != 0 { - annotations := make(map[string]string) - for _, annotationSpec := range opts.Annotation { - key, val, hasVal := strings.Cut(annotationSpec, "=") - if !hasVal { - return "", fmt.Errorf("no value given for annotation %q", key) - } - annotations[key] = val - } - opts.Annotations = envLib.Join(opts.Annotations, annotations) + + if annotateOptions.Annotations, err = mergeAnnotations(opts.Annotations, opts.Annotation); err != nil { + return "", err + } + + if annotateOptions.IndexAnnotations, err = mergeAnnotations(opts.IndexAnnotations, opts.IndexAnnotation); err != nil { + return "", err } - annotateOptions.Annotations = opts.Annotations if err := manifestList.AnnotateInstance(instanceDigest, annotateOptions); err != nil { return "", err @@ -380,10 +377,12 @@ func (ir *ImageEngine) ManifestAddArtifact(ctx context.Context, name string, fil Variant: opts.Variant, Subject: opts.IndexSubject, } - if annotateOptions.Annotations, err = mergeAnnotations(opts.Annotations, opts.Annotation); err != nil { + + if annotateOptions.Annotations, err = mergeAnnotations(opts.ManifestAnnotateOptions.Annotations, opts.ManifestAnnotateOptions.Annotation); err != nil { return "", err } - if annotateOptions.IndexAnnotations, err = mergeAnnotations(opts.IndexAnnotations, opts.IndexAnnotation); err != nil { + + if annotateOptions.IndexAnnotations, err = mergeAnnotations(opts.ManifestAnnotateOptions.IndexAnnotations, opts.ManifestAnnotateOptions.IndexAnnotation); err != nil { return "", err } diff --git a/pkg/domain/infra/tunnel/manifest.go b/pkg/domain/infra/tunnel/manifest.go index 49585a198a..7b98d6bc88 100644 --- a/pkg/domain/infra/tunnel/manifest.go +++ b/pkg/domain/infra/tunnel/manifest.go @@ -131,18 +131,17 @@ func (ir *ImageEngine) ManifestAnnotate(ctx context.Context, name, images string options := new(manifests.ModifyOptions).WithArch(opts.Arch).WithVariant(opts.Variant) options.WithFeatures(opts.Features).WithOS(opts.OS).WithOSVersion(opts.OSVersion) - if len(opts.Annotation) != 0 { - annotations := make(map[string]string) - for _, annotationSpec := range opts.Annotation { - key, val, hasVal := strings.Cut(annotationSpec, "=") - if !hasVal { - return "", fmt.Errorf("no value given for annotation %q", key) - } - annotations[key] = val - } - opts.Annotations = envLib.Join(opts.Annotations, annotations) + annotations, err := mergeAnnotations(opts.Annotations, opts.Annotation) + if err != nil { + return "", err } - options.WithAnnotations(opts.Annotations) + options.WithAnnotations(annotations) + + indexAnnotations, err := mergeAnnotations(opts.IndexAnnotations, opts.IndexAnnotation) + if err != nil { + return "", err + } + options.WithIndexAnnotations(indexAnnotations) id, err := manifests.Annotate(ir.ClientCtx, name, []string{images}, options) if err != nil { @@ -151,6 +150,24 @@ func (ir *ImageEngine) ManifestAnnotate(ctx context.Context, name, images string return id, nil } +func mergeAnnotations(preferred map[string]string, aux []string) (map[string]string, error) { + if len(aux) != 0 { + auxAnnotations := make(map[string]string) + for _, annotationSpec := range aux { + key, val, hasVal := strings.Cut(annotationSpec, "=") + if !hasVal { + return nil, fmt.Errorf("no value given for annotation %q", key) + } + auxAnnotations[key] = val + } + if preferred == nil { + preferred = make(map[string]string) + } + preferred = envLib.Join(auxAnnotations, preferred) + } + return preferred, nil +} + // ManifestRemoveDigest removes the digest from manifest list func (ir *ImageEngine) ManifestRemoveDigest(ctx context.Context, name string, image string) (string, error) { updatedListID, err := manifests.Remove(ir.ClientCtx, name, image, nil) diff --git a/test/e2e/manifest_test.go b/test/e2e/manifest_test.go index 22bf064b4e..783e9418a9 100644 --- a/test/e2e/manifest_test.go +++ b/test/e2e/manifest_test.go @@ -10,12 +10,16 @@ import ( "strings" "github.com/containers/common/libimage/define" + "github.com/containers/image/v5/docker/reference" + manifest "github.com/containers/image/v5/manifest" + "github.com/containers/image/v5/transports/alltransports" podmanRegistry "github.com/containers/podman/v5/hack/podman-registry-go" . "github.com/containers/podman/v5/test/utils" "github.com/containers/storage/pkg/archive" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gexec" + imgspec "github.com/opencontainers/image-spec/specs-go" imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -368,7 +372,7 @@ add_compression = ["zstd"]`), 0o644) }) It("annotate", func() { - session := podmanTest.Podman([]string{"manifest", "create", "foo"}) + session := podmanTest.Podman([]string{"manifest", "create", "--annotation", "up=down", "foo"}) session.WaitWithDefaultTimeout() Expect(session).Should(ExitCleanly()) session = podmanTest.Podman([]string{"manifest", "add", "foo", imageListInstance}) @@ -377,12 +381,50 @@ add_compression = ["zstd"]`), 0o644) session = podmanTest.Podman([]string{"manifest", "annotate", "--annotation", "hello=world,withcomma", "--arch", "bar", "foo", imageListARM64InstanceDigest}) session.WaitWithDefaultTimeout() Expect(session).Should(ExitCleanly()) + session = podmanTest.Podman([]string{"manifest", "annotate", "--index", "--annotation", "top=bottom", "foo"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) session = podmanTest.Podman([]string{"manifest", "inspect", "foo"}) session.WaitWithDefaultTimeout() Expect(session).Should(ExitCleanly()) - Expect(session.OutputToString()).To(ContainSubstring(`"architecture": "bar"`)) - // Check added annotation - Expect(session.OutputToString()).To(ContainSubstring(`"hello": "world,withcomma"`)) + // Extract the digest from the name of the image that we just added to the list + ref, err := alltransports.ParseImageName(imageListInstance) + Expect(err).ToNot(HaveOccurred()) + dockerReference := ref.DockerReference() + referenceWithDigest, ok := dockerReference.(reference.Canonical) + Expect(ok).To(BeTrueBecause("we started with a canonical reference")) + // Check that the index has all of the information we've just added to it + encoded, err := json.Marshal(&imgspecv1.Index{ + Versioned: imgspec.Versioned{ + SchemaVersion: 2, + }, + // media type forced because we need to be able to represent annotations + MediaType: imgspecv1.MediaTypeImageIndex, + Manifests: []imgspecv1.Descriptor{ + { + // from imageListInstance + MediaType: manifest.DockerV2Schema2MediaType, + Digest: referenceWithDigest.Digest(), + Size: 527, + // OS from imageListInstance(?), Architecture/Variant as above + Platform: &imgspecv1.Platform{ + Architecture: "bar", + OS: "linux", + }, + // added above + Annotations: map[string]string{ + "hello": "world,withcomma", + }, + }, + }, + // added above + Annotations: map[string]string{ + "top": "bottom", + "up": "down", + }, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(session.OutputToString()).To(MatchJSON(encoded)) }) It("remove digest", func() {