Merge pull request #25170 from baude/artifactoptions

Add type and annotations to artifact add

cmd/podman/artifact/add.go

@@ -3,15 +3,17 @@ package artifact
 import (
 	"fmt"
 
+	"github.com/containers/common/pkg/completion"
 	"github.com/containers/podman/v5/cmd/podman/common"
 	"github.com/containers/podman/v5/cmd/podman/registry"
 	"github.com/containers/podman/v5/pkg/domain/entities"
+	"github.com/containers/podman/v5/pkg/domain/utils"
 	"github.com/spf13/cobra"
 )
 
 var (
 	addCmd = &cobra.Command{
-		Use:               "add ARTIFACT PATH [...PATH]",
+		Use:               "add [options] ARTIFACT PATH [...PATH]",
 		Short:             "Add an OCI artifact to the local store",
 		Long:              "Add an OCI artifact to the local store from the local filesystem",
 		RunE:              add,
@@ -22,15 +24,41 @@ var (
 	}
 )
 
+type artifactAddOptions struct {
+	ArtifactType string
+	Annotations  []string
+}
+
+var (
+	addOpts artifactAddOptions
+)
+
 func init() {
 	registry.Commands = append(registry.Commands, registry.CliCommand{
 		Command: addCmd,
 		Parent:  artifactCmd,
 	})
+	flags := addCmd.Flags()
+
+	annotationFlagName := "annotation"
+	flags.StringArrayVar(&addOpts.Annotations, annotationFlagName, nil, "set an `annotation` for the specified artifact")
+	_ = addCmd.RegisterFlagCompletionFunc(annotationFlagName, completion.AutocompleteNone)
+
+	addTypeFlagName := "type"
+	flags.StringVar(&addOpts.ArtifactType, addTypeFlagName, "", "Use type to describe an artifact")
+	_ = addCmd.RegisterFlagCompletionFunc(addTypeFlagName, completion.AutocompleteNone)
 }
 
 func add(cmd *cobra.Command, args []string) error {
-	report, err := registry.ImageEngine().ArtifactAdd(registry.Context(), args[0], args[1:], entities.ArtifactAddoptions{})
+	opts := new(entities.ArtifactAddOptions)
+
+	annots, err := utils.ParseAnnotations(addOpts.Annotations)
+	if err != nil {
+		return err
+	}
+	opts.Annotations = annots
+	opts.ArtifactType = addOpts.ArtifactType
+	report, err := registry.ImageEngine().ArtifactAdd(registry.Context(), args[0], args[1:], opts)
 	if err != nil {
 		return err
 	}

docs/source/markdown/.gitignore

@@ -1,3 +1,4 @@
+podman-artifact-add.1.md
 podman-artifact-pull.1.md
 podman-artifact-push.1.md
 podman-attach.1.md

docs/source/markdown/options/annotation.manifest.md

@@ -1,5 +1,5 @@
 ####> This option file is used in:
-####>   podman manifest add, manifest annotate
+####>   podman artifact add, manifest add, manifest annotate
 ####> If file is edited, make sure the changes
 ####> are applicable to all of those.
 #### **--annotation**=*annotation=value*

docs/source/markdown/podman-artifact-add.1.md.in

@@ -19,10 +19,15 @@ added.
 
 ## OPTIONS
 
+@@option annotation.manifest
+
 #### **--help**
 
 Print usage statement.
 
+#### **--type**
+
+Set a type for the artifact being added.
 
 ## EXAMPLES
 
@@ -39,6 +44,10 @@ $ podman artifact add quay.io/myartifact/myml:latest /tmp/foobar1.ml /tmp/foobar
 1487acae11b5a30948c50762882036b41ac91a7b9514be8012d98015c95ddb78
 ```
 
+Set an annotation for an artifact
+```
+$ podman artifact add --annotation date=2025-01-30 quay.io/myartifact/myml:latest /tmp/foobar1.ml
+```
 
 
 ## SEE ALSO

pkg/api/handlers/libpod/manifests.go

@@ -29,6 +29,7 @@ import (
 	"github.com/containers/podman/v5/pkg/channel"
 	"github.com/containers/podman/v5/pkg/domain/entities"
 	"github.com/containers/podman/v5/pkg/domain/infra/abi"
+	domainUtils "github.com/containers/podman/v5/pkg/domain/utils"
 	"github.com/containers/podman/v5/pkg/errorhandling"
 	"github.com/gorilla/mux"
 	"github.com/gorilla/schema"
@@ -520,24 +521,17 @@ func ManifestModify(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	annotationsFromAnnotationSlice := func(annotation []string) map[string]string {
-		annotations := make(map[string]string)
-		for _, annotationSpec := range annotation {
-			key, val, hasVal := strings.Cut(annotationSpec, "=")
-			if !hasVal {
-				utils.Error(w, http.StatusBadRequest, fmt.Errorf("no value given for annotation %q", key))
-				return nil
-			}
-			annotations[key] = val
-		}
-		return annotations
-	}
 	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
 		}
-		body.ManifestAddOptions.Annotations = annotationsFromAnnotationSlice(body.ManifestAddOptions.Annotation)
+		annots, err := domainUtils.ParseAnnotations(body.ManifestAddOptions.Annotation)
+		if err != nil {
+			utils.Error(w, http.StatusBadRequest, err)
+			return
+		}
+		body.ManifestAddOptions.Annotations = annots
 		body.ManifestAddOptions.Annotation = nil
 	}
 	if len(body.ManifestAddOptions.IndexAnnotation) != 0 {
@@ -545,7 +539,12 @@ func ManifestModify(w http.ResponseWriter, r *http.Request) {
 			utils.Error(w, http.StatusBadRequest, fmt.Errorf("can not set both IndexAnnotation and IndexAnnotations"))
 			return
 		}
-		body.ManifestAddOptions.IndexAnnotations = annotationsFromAnnotationSlice(body.ManifestAddOptions.IndexAnnotation)
+		annots, err := domainUtils.ParseAnnotations(body.ManifestAddOptions.IndexAnnotation)
+		if err != nil {
+			utils.Error(w, http.StatusBadRequest, err)
+			return
+		}
+		body.ManifestAddOptions.IndexAnnotations = annots
 		body.ManifestAddOptions.IndexAnnotation = nil
 	}
 

pkg/domain/entities/artifact.go

@@ -9,7 +9,8 @@ import (
 	"github.com/opencontainers/go-digest"
 )
 
-type ArtifactAddoptions struct {
+type ArtifactAddOptions struct {
+	Annotations  map[string]string
 	ArtifactType string
 }
 

pkg/domain/entities/engine_image.go

@@ -9,7 +9,7 @@ import (
 )
 
 type ImageEngine interface { //nolint:interfacebloat
-	ArtifactAdd(ctx context.Context, name string, paths []string, opts ArtifactAddoptions) (*ArtifactAddReport, error)
+	ArtifactAdd(ctx context.Context, name string, paths []string, opts *ArtifactAddOptions) (*ArtifactAddReport, error)
 	ArtifactInspect(ctx context.Context, name string, opts ArtifactInspectOptions) (*ArtifactInspectReport, error)
 	ArtifactList(ctx context.Context, opts ArtifactListOptions) ([]*ArtifactListReport, error)
 	ArtifactPull(ctx context.Context, name string, opts ArtifactPullOptions) (*ArtifactPullReport, error)

pkg/domain/infra/abi/artifact.go

@@ -11,6 +11,7 @@ import (
 	"github.com/containers/common/libimage"
 	"github.com/containers/podman/v5/pkg/domain/entities"
 	"github.com/containers/podman/v5/pkg/libartifact/store"
+	"github.com/containers/podman/v5/pkg/libartifact/types"
 )
 
 func getDefaultArtifactStore(ir *ImageEngine) string {
@@ -152,12 +153,18 @@ func (ir *ImageEngine) ArtifactPush(ctx context.Context, name string, opts entit
 	err = artStore.Push(ctx, name, name, copyOpts)
 	return &entities.ArtifactPushReport{}, err
 }
-func (ir *ImageEngine) ArtifactAdd(ctx context.Context, name string, paths []string, opts entities.ArtifactAddoptions) (*entities.ArtifactAddReport, error) {
+func (ir *ImageEngine) ArtifactAdd(ctx context.Context, name string, paths []string, opts *entities.ArtifactAddOptions) (*entities.ArtifactAddReport, error) {
 	artStore, err := store.NewArtifactStore(getDefaultArtifactStore(ir), ir.Libpod.SystemContext())
 	if err != nil {
 		return nil, err
 	}
-	artifactDigest, err := artStore.Add(ctx, name, paths, opts.ArtifactType)
+
+	addOptions := types.AddOptions{
+		Annotations:  opts.Annotations,
+		ArtifactType: opts.ArtifactType,
+	}
+
+	artifactDigest, err := artStore.Add(ctx, name, paths, &addOptions)
 	if err != nil {
 		return nil, err
 	}

pkg/domain/infra/tunnel/artifact.go

@@ -9,7 +9,7 @@ import (
 
 // TODO For now, no remote support has been added. We need the API to firm up first.
 
-func ArtifactAdd(ctx context.Context, path, name string, opts entities.ArtifactAddoptions) error {
+func ArtifactAdd(ctx context.Context, path, name string, opts entities.ArtifactAddOptions) error {
 	return fmt.Errorf("not implemented")
 }
 
@@ -33,6 +33,6 @@ func (ir *ImageEngine) ArtifactPush(ctx context.Context, name string, opts entit
 	return nil, fmt.Errorf("not implemented")
 }
 
-func (ir *ImageEngine) ArtifactAdd(ctx context.Context, name string, paths []string, opts entities.ArtifactAddoptions) (*entities.ArtifactAddReport, error) {
+func (ir *ImageEngine) ArtifactAdd(ctx context.Context, name string, paths []string, opts *entities.ArtifactAddOptions) (*entities.ArtifactAddReport, error) {
 	return nil, fmt.Errorf("not implemented")
 }

pkg/domain/utils/utils.go

@@ -1,6 +1,7 @@
 package utils
 
 import (
+	"fmt"
 	"net/url"
 	"strings"
 
@@ -39,3 +40,17 @@ func ToURLValues(f []string) (filters url.Values) {
 	}
 	return
 }
+
+// ParseAnnotations takes a string slice of options, expected to be "key=val" and returns
+// a string map where the map index is the key and points to the value
+func ParseAnnotations(options []string) (map[string]string, error) {
+	annotations := make(map[string]string)
+	for _, annotationSpec := range options {
+		key, val, hasVal := strings.Cut(annotationSpec, "=")
+		if !hasVal {
+			return nil, fmt.Errorf("no value given for annotation %q", key)
+		}
+		annotations[key] = val
+	}
+	return annotations, nil
+}

pkg/libartifact/store/store.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"maps"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -160,7 +161,8 @@ func (as ArtifactStore) Push(ctx context.Context, src, dest string, opts libimag
 
 // Add takes one or more local files and adds them to the local artifact store.  The empty
 // string input is for possible custom artifact types.
-func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, _ string) (*digest.Digest, error) {
+func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, options *libartTypes.AddOptions) (*digest.Digest, error) {
+	annots := maps.Clone(options.Annotations)
 	if len(dest) == 0 {
 		return nil, ErrEmptyArtifactName
 	}
@@ -191,6 +193,13 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, _
 	defer imageDest.Close()
 
 	for _, path := range paths {
+		// currently we don't allow override of the filename ; if a user requirement emerges,
+		// we could seemingly accommodate but broadens possibilities of something bad happening
+		// for things like `artifact extract`
+		if _, hasTitle := options.Annotations[specV1.AnnotationTitle]; hasTitle {
+			return nil, fmt.Errorf("cannot override filename with %s annotation", specV1.AnnotationTitle)
+		}
+
 		// get the new artifact into the local store
 		newBlobDigest, newBlobSize, err := layout.PutBlobFromLocalFile(ctx, imageDest, path)
 		if err != nil {
@@ -200,14 +209,16 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, _
 		if err != nil {
 			return nil, err
 		}
-		newArtifactAnnotations := map[string]string{}
+
-		newArtifactAnnotations[specV1.AnnotationTitle] = filepath.Base(path)
+		annots[specV1.AnnotationTitle] = filepath.Base(path)
+
 		newLayer := specV1.Descriptor{
 			MediaType:   detectedType,
 			Digest:      newBlobDigest,
 			Size:        newBlobSize,
-			Annotations: newArtifactAnnotations,
+			Annotations: annots,
 		}
+
 		artifactManifestLayers = append(artifactManifestLayers, newLayer)
 	}
 
@@ -215,11 +226,12 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, _
 		Versioned: specs.Versioned{SchemaVersion: 2},
 		MediaType: specV1.MediaTypeImageManifest,
 		// TODO This should probably be configurable once the CLI is capable
-		ArtifactType: "",
+		Config: specV1.DescriptorEmptyJSON,
-		Config:       specV1.DescriptorEmptyJSON,
+		Layers: artifactManifestLayers,
-		Layers:       artifactManifestLayers,
 	}
 
+	artifactManifest.ArtifactType = options.ArtifactType
+
 	rawData, err := json.Marshal(artifactManifest)
 	if err != nil {
 		return nil, err

pkg/libartifact/types/config.go

@@ -3,3 +3,9 @@ package types
 // GetArtifactOptions is a struct containing options that for obtaining artifacts.
 // It is meant for future growth or changes required without wacking the API
 type GetArtifactOptions struct{}
+
+// AddOptions are additional descriptors of an artifact file
+type AddOptions struct {
+	Annotations  map[string]string `json:"annotations,omitempty"`
+	ArtifactType string            `json:",omitempty"`
+}

test/e2e/artifact_test.go

@@ -67,6 +67,31 @@ var _ = Describe("Podman artifact", func() {
 		Expect(addAgain.ErrorToString()).To(Equal(fmt.Sprintf("Error: artifact %s already exists", artifact1Name)))
 	})
 
+	It("podman artifact add with options", func() {
+		artifact1Name := "localhost/test/artifact1"
+		artifact1File, err := createArtifactFile(1024)
+		Expect(err).ToNot(HaveOccurred())
+
+		artifactType := "octet/foobar"
+		annotation1 := "color=blue"
+		annotation2 := "flavor=lemon"
+
+		podmanTest.PodmanExitCleanly([]string{"artifact", "add", "--type", artifactType, "--annotation", annotation1, "--annotation", annotation2, artifact1Name, artifact1File}...)
+		inspectSingleSession := podmanTest.PodmanExitCleanly([]string{"artifact", "inspect", artifact1Name}...)
+		a := libartifact.Artifact{}
+		err = json.Unmarshal([]byte(inspectSingleSession.OutputToString()), &a)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(a.Name).To(Equal(artifact1Name))
+		Expect(a.Manifests[0].ArtifactType).To(Equal(artifactType))
+		Expect(a.Manifests[0].Layers[0].Annotations["color"]).To(Equal("blue"))
+		Expect(a.Manifests[0].Layers[0].Annotations["flavor"]).To(Equal("lemon"))
+
+		failSession := podmanTest.Podman([]string{"artifact", "add", "--annotation", "org.opencontainers.image.title=foobar", "foobar", artifact1File})
+		failSession.WaitWithDefaultTimeout()
+		Expect(failSession).Should(Exit(125))
+		Expect(failSession.ErrorToString()).Should(Equal("Error: cannot override filename with org.opencontainers.image.title annotation"))
+	})
+
 	It("podman artifact add multiple", func() {
 		artifact1File1, err := createArtifactFile(1024)
 		Expect(err).ToNot(HaveOccurred())