Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add type and annotations to artifact add #25170

Merged
merged 1 commit into from
Jan 31, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions cmd/podman/artifact/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions docs/source/markdown/.gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
podman-artifact-add.1.md
podman-artifact-pull.1.md
podman-artifact-push.1.md
podman-attach.1.md
Expand Down
2 changes: 1 addition & 1 deletion docs/source/markdown/options/annotation.manifest.md
Original file line number Diff line number Diff line change
@@ -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*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,15 @@ added.

## OPTIONS

@@option annotation.manifest

#### **--help**

Print usage statement.

#### **--type**

Set a type for the artifact being added.

## EXAMPLES

Expand All @@ -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
Expand Down
27 changes: 13 additions & 14 deletions pkg/api/handlers/libpod/manifests.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -520,32 +521,30 @@ 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 {
if len(body.ManifestAddOptions.IndexAnnotations) != 0 {
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
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/domain/entities/artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import (
"github.com/opencontainers/go-digest"
)

type ArtifactAddoptions struct {
type ArtifactAddOptions struct {
Annotations map[string]string
ArtifactType string
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/domain/entities/engine_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 9 additions & 2 deletions pkg/domain/infra/abi/artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/domain/infra/tunnel/artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand All @@ -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")
}
15 changes: 15 additions & 0 deletions pkg/domain/utils/utils.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package utils

import (
"fmt"
"net/url"
"strings"

Expand Down Expand Up @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not blocking BUT we really need to start to do better package names, this util/utils package madness is inside.
Point in case you having to rename the import to utils2 in the api code. That is just super ugly to read.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think that is a lovely tech debt item we could do ...

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
}
26 changes: 19 additions & 7 deletions pkg/libartifact/store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"errors"
"fmt"
"io"
"maps"
"net/http"
"os"
"path/filepath"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand All @@ -200,26 +209,29 @@ 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)
}

artifactManifest := specV1.Manifest{
Versioned: specs.Versioned{SchemaVersion: 2},
MediaType: specV1.MediaTypeImageManifest,
// TODO This should probably be configurable once the CLI is capable
ArtifactType: "",
Config: specV1.DescriptorEmptyJSON,
Layers: artifactManifestLayers,
Config: specV1.DescriptorEmptyJSON,
Layers: artifactManifestLayers,
}

artifactManifest.ArtifactType = options.ArtifactType

rawData, err := json.Marshal(artifactManifest)
if err != nil {
return nil, err
Expand Down
6 changes: 6 additions & 0 deletions pkg/libartifact/types/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
25 changes: 25 additions & 0 deletions test/e2e/artifact_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down