Create --append flag to add file to existing artifact

Fixes: https://issues.redhat.com/browse/RUN-2444

Signed-off-by: Jan Rodák <[email protected]>

cmd/podman/artifact/add.go

@@ -27,6 +27,7 @@ var (
 type artifactAddOptions struct {
 	ArtifactType string
 	Annotations  []string
+	Append       bool
 }
 
 var (
@@ -47,6 +48,10 @@ func init() {
 	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")
+	_ = addCmd.RegisterFlagCompletionFunc(appendFlagName, completion.AutocompleteNone)
 }
 
 func add(cmd *cobra.Command, args []string) error {
@@ -58,6 +63,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

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

@@ -21,6 +21,10 @@ added.
 
 @@option annotation.manifest
 
+#### **--append**, **-a**
+
+Append files to an existing artifact
+
 #### **--help**
 
 Print usage statement.

pkg/domain/entities/artifact.go

@@ -12,6 +12,7 @@ import (
 type ArtifactAddOptions struct {
 	Annotations  map[string]string
 	ArtifactType string
+	Append       bool
 }
 
 type ArtifactInspectOptions struct {

pkg/domain/infra/abi/artifact.go

@@ -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)

pkg/libartifact/store/store.go

@@ -29,6 +29,7 @@ import (
 
 var (
 	ErrEmptyArtifactName = errors.New("artifact name cannot be empty")
+	SchemaVersion        = 2
 )
 
 type ArtifactStore struct {
@@ -162,13 +163,10 @@ 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)
-
 	// Check if artifact already exists
 	artifacts, err := as.getArtifacts(ctx, nil)
 	if err != nil {
@@ -177,10 +175,21 @@ 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)
 	}
 
+	var instanceDigest *digest.Digest
+	currentArtifactLayers := make([]specV1.Descriptor, 0)
+	if artifact != nil && options.Append {
+		currentArtifactLayers = artifact.Manifest.Layers
+		instanceDigest, err = artifact.GetDigest()
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	ir, err := layout.NewReference(as.storePath, dest)
 	if err != nil {
 		return nil, err
@@ -192,66 +201,32 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, op
 	}
 	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 {
-			return nil, err
-		}
-		detectedType, err := determineManifestType(path)
-		if err != nil {
-			return nil, err
-		}
-
-		annots[specV1.AnnotationTitle] = filepath.Base(path)
-
-		newLayer := specV1.Descriptor{
-			MediaType:   detectedType,
-			Digest:      newBlobDigest,
-			Size:        newBlobSize,
-			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
-		Config: specV1.DescriptorEmptyJSON,
-		Layers: artifactManifestLayers,
+	newArtifactLayers, err := appendNewLocalFiles(ctx, currentArtifactLayers, imageDest, paths, options.Annotations)
+	if err != nil {
+		return nil, err
 	}
 
-	artifactManifest.ArtifactType = options.ArtifactType
-
-	rawData, err := json.Marshal(artifactManifest)
+	artifactManifest := getNewArtifactManifest(newArtifactLayers, options.ArtifactType)
+	rawData, artifactManifestDigest, err := marshalArtifactManifest(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
 	}
 
-	artifactManifestDigest := digest.FromBytes(rawData)
-
 	// the config is an empty JSON stanza i.e. '{}'; if it does not yet exist, it needs
 	// to be created
 	if err := createEmptyStanza(filepath.Join(as.storePath, specV1.ImageBlobsDir, artifactManifestDigest.Algorithm().String(), artifactManifest.Config.Digest.Encoded())); err != nil {
 		logrus.Errorf("failed to check or write empty stanza file: %v", err)
 	}
-	return &artifactManifestDigest, nil
+	return artifactManifestDigest, nil
 }
 
 // readIndex is currently unused but I want to keep this around until
@@ -269,7 +244,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 {
@@ -360,3 +335,66 @@ func determineManifestType(path string) (string, error) {
 	}
 	return http.DetectContentType(b[:n]), nil
 }
+
+func appendNewLocalFiles(ctx context.Context, currentArtifactLayers []specV1.Descriptor, imageDest types.ImageDestination, paths []string, optionsAnnotations map[string]string) ([]specV1.Descriptor, error) {
+	// 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 := optionsAnnotations[specV1.AnnotationTitle]; hasTitle {
+		return nil, fmt.Errorf("cannot override filename with %s annotation", specV1.AnnotationTitle)
+	}
+
+	annotations := maps.Clone(optionsAnnotations)
+
+	fileNames := map[string]struct{}{}
+	for _, layer := range currentArtifactLayers {
+		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)
+		}
+		// get the new artifact into the local store
+		newBlobDigest, newBlobSize, err := layout.PutBlobFromLocalFile(ctx, imageDest, path)
+		if err != nil {
+			return nil, err
+		}
+		detectedType, err := determineManifestType(path)
+		if err != nil {
+			return nil, err
+		}
+		annotations[specV1.AnnotationTitle] = fileName
+		newLayer := specV1.Descriptor{
+			MediaType:   detectedType,
+			Digest:      newBlobDigest,
+			Size:        newBlobSize,
+			Annotations: annotations,
+		}
+
+		fileNames[fileName] = struct{}{}
+		currentArtifactLayers = append(currentArtifactLayers, newLayer)
+	}
+	return currentArtifactLayers, nil
+}
+
+func getNewArtifactManifest(newArtifactLayers []specV1.Descriptor, artifactType string) specV1.Manifest {
+	return specV1.Manifest{
+		Versioned:    specs.Versioned{SchemaVersion: SchemaVersion},
+		MediaType:    specV1.MediaTypeImageManifest,
+		ArtifactType: artifactType,
+		// TODO This should probably be configurable once the CLI is capable
+		Config: specV1.DescriptorEmptyJSON,
+		Layers: newArtifactLayers,
+	}
+}
+
+func marshalArtifactManifest(artifactManifest specV1.Manifest) ([]byte, *digest.Digest, error) {
+	rawData, err := json.Marshal(artifactManifest)
+	if err != nil {
+		return nil, nil, err
+	}
+	artifactManifestDigest := digest.FromBytes(rawData)
+	return rawData, &artifactManifestDigest, nil
+}

pkg/libartifact/types/config.go

@@ -8,4 +8,5 @@ type GetArtifactOptions struct{}
 type AddOptions struct {
 	Annotations  map[string]string `json:"annotations,omitempty"`
 	ArtifactType string            `json:",omitempty"`
+	Append       bool              `json:",omitempty"`
 }