Skip to content

Commit

Permalink
Create --append flag to add file to existing artifact
Browse files Browse the repository at this point in the history
Fixes: https://issues.redhat.com/browse/RUN-2444

Signed-off-by: Jan Rodák <hony.com@seznam.cz>
  • Loading branch information
Honny1 committed Feb 4, 2025
1 parent 16a72c8 commit 3c5c832
Show file tree
Hide file tree
Showing 8 changed files with 257 additions and 54 deletions.
8 changes: 7 additions & 1 deletion cmd/podman/artifact/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ var (
type artifactAddOptions struct {
ArtifactType string
Annotations []string
Append bool
}

var (
Expand All @@ -41,12 +42,15 @@ func init() {
flags := addCmd.Flags()

annotationFlagName := "annotation"
flags.StringArrayVar(&addOpts.Annotations, annotationFlagName, nil, "set an `annotation` for the specified artifact")
flags.StringArrayVar(&addOpts.Annotations, annotationFlagName, nil, "set an `annotation` for the specified files of artifact")
_ = addCmd.RegisterFlagCompletionFunc(annotationFlagName, completion.AutocompleteNone)

addTypeFlagName := "type"
flags.StringVar(&addOpts.ArtifactType, addTypeFlagName, "", "Use type to describe an artifact")
_ = addCmd.RegisterFlagCompletionFunc(addTypeFlagName, completion.AutocompleteNone)

appendFlagName := "append"
flags.BoolVarP(&addOpts.Append, appendFlagName, "a", false, "Append files to an existing artifact")
}

func add(cmd *cobra.Command, args []string) error {
Expand All @@ -58,6 +62,8 @@ func add(cmd *cobra.Command, args []string) error {
}
opts.Annotations = annots
opts.ArtifactType = addOpts.ArtifactType
opts.Append = addOpts.Append

report, err := registry.ImageEngine().ArtifactAdd(registry.Context(), args[0], args[1:], opts)
if err != nil {
return err
Expand Down
6 changes: 6 additions & 0 deletions docs/source/markdown/podman-artifact-add.1.md.in
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ added.

@@option annotation.manifest

Note: Set annotations for each file being added.

#### **--append**, **-a**

Append files to an existing artifact. This option cannot be used with the **--type** option.

#### **--help**

Print usage statement.
Expand Down
1 change: 1 addition & 0 deletions pkg/domain/entities/artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
type ArtifactAddOptions struct {
Annotations map[string]string
ArtifactType string
Append bool
}

type ArtifactInspectOptions struct {
Expand Down
1 change: 1 addition & 0 deletions pkg/domain/infra/abi/artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ func (ir *ImageEngine) ArtifactAdd(ctx context.Context, name string, paths []str
addOptions := types.AddOptions{
Annotations: opts.Annotations,
ArtifactType: opts.ArtifactType,
Append: opts.Append,
}

artifactDigest, err := artStore.Add(ctx, name, paths, &addOptions)
Expand Down
92 changes: 62 additions & 30 deletions pkg/libartifact/store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (

var (
ErrEmptyArtifactName = errors.New("artifact name cannot be empty")
SchemaVersion = 2
)

type ArtifactStore struct {
Expand Down Expand Up @@ -162,12 +163,20 @@ 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, options *libartTypes.AddOptions) (*digest.Digest, error) {
annots := maps.Clone(options.Annotations)
if len(dest) == 0 {
return nil, ErrEmptyArtifactName
}

artifactManifestLayers := make([]specV1.Descriptor, 0)
if options.Append && len(options.ArtifactType) > 0 {
return nil, errors.New("append option is not compatible with ArtifactType option")
}

// 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)
}

// Check if artifact already exists
artifacts, err := as.getArtifacts(ctx, nil)
Expand All @@ -177,10 +186,48 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, op

// Check if artifact exists; in GetByName not getting an
// error means it exists
if _, _, err := artifacts.GetByNameOrDigest(dest); err == nil {
artifact, _, err := artifacts.GetByNameOrDigest(dest)
if err == nil && !options.Append {
return nil, fmt.Errorf("artifact %s already exists", dest)
}

artifactManifest := specV1.Manifest{
Versioned: specs.Versioned{SchemaVersion: SchemaVersion},
MediaType: specV1.MediaTypeImageManifest,
ArtifactType: options.ArtifactType,
// TODO This should probably be configurable once the CLI is capable
Config: specV1.DescriptorEmptyJSON,
Layers: make([]specV1.Descriptor, 0),
}
var instanceDigest *digest.Digest
fileNames := map[string]struct{}{}
if artifact != nil && options.Append {
artifactManifest = artifact.Manifest.Manifest

index, err := as.readIndex()
if err != nil {
return nil, err
}
for _, manifest := range index.Manifests {
if manifest.Annotations[specV1.AnnotationRefName] == artifact.Name {
instanceDigest = &manifest.Digest
break
}
}

for _, layer := range artifactManifest.Layers {
fileNames[layer.Annotations[specV1.AnnotationTitle]] = struct{}{}
}
}

for _, path := range paths {
fileName := filepath.Base(path)
if _, ok := fileNames[fileName]; ok {
return nil, fmt.Errorf("file: %s already exist in artifact", fileName)
}
fileNames[fileName] = struct{}{}
}

ir, err := layout.NewReference(as.storePath, dest)
if err != nil {
return nil, err
Expand All @@ -192,14 +239,8 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, op
}
defer imageDest.Close()

annotations := maps.Clone(options.Annotations)
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 @@ -210,35 +251,24 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, op
return nil, err
}

annots[specV1.AnnotationTitle] = filepath.Base(path)

annotations[specV1.AnnotationTitle] = filepath.Base(path)
newLayer := specV1.Descriptor{
MediaType: detectedType,
Digest: newBlobDigest,
Size: newBlobSize,
Annotations: annots,
Annotations: annotations,
}

artifactManifestLayers = append(artifactManifestLayers, newLayer)
artifactManifest.Layers = append(artifactManifest.Layers, newLayer)
}

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

artifactManifest.ArtifactType = options.ArtifactType

rawData, err := json.Marshal(artifactManifest)
if err != nil {
return nil, err
}
if err := imageDest.PutManifest(ctx, rawData, nil); err != nil {
if err := imageDest.PutManifest(ctx, rawData, instanceDigest); err != nil {
return nil, err
}

unparsed := newUnparsedArtifactImage(ir, artifactManifest)
if err := imageDest.Commit(ctx, unparsed); err != nil {
return nil, err
Expand All @@ -254,9 +284,11 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, op
return &artifactManifestDigest, nil
}

// readIndex is currently unused but I want to keep this around until
// the artifact code is more mature.
func (as ArtifactStore) readIndex() (*specV1.Index, error) { //nolint:unused
// readIndex is currently used to get a digest to save the modified manifest with the newly attached files.
// The digest is actually recalculated for new content, so using multiple adds to the artifact doesn't work.
//
// readIndex will probably be possible to remove it when the artifact code is more mature.
func (as ArtifactStore) readIndex() (*specV1.Index, error) {
index := specV1.Index{}
rawData, err := os.ReadFile(as.indexPath())
if err != nil {
Expand All @@ -269,7 +301,7 @@ func (as ArtifactStore) readIndex() (*specV1.Index, error) { //nolint:unused
func (as ArtifactStore) createEmptyManifest() error {
index := specV1.Index{
MediaType: specV1.MediaTypeImageIndex,
Versioned: specs.Versioned{SchemaVersion: 2},
Versioned: specs.Versioned{SchemaVersion: SchemaVersion},
}
rawData, err := json.Marshal(&index)
if err != nil {
Expand Down
2 changes: 2 additions & 0 deletions pkg/libartifact/types/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,6 @@ type GetArtifactOptions struct{}
type AddOptions struct {
Annotations map[string]string `json:"annotations,omitempty"`
ArtifactType string `json:",omitempty"`
// append option is not compatible with ArtifactType option
Append bool `json:",omitempty"`
}
Loading

0 comments on commit 3c5c832

Please sign in to comment.