diff --git a/cmd/podman/artifact/add.go b/cmd/podman/artifact/add.go index a92018fb3f..7b5e4214ed 100644 --- a/cmd/podman/artifact/add.go +++ b/cmd/podman/artifact/add.go @@ -27,6 +27,7 @@ var ( type artifactAddOptions struct { ArtifactType string Annotations []string + Append bool } var ( @@ -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 { @@ -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 diff --git a/docs/source/markdown/podman-artifact-add.1.md.in b/docs/source/markdown/podman-artifact-add.1.md.in index 35a99f9958..17231f6e7b 100644 --- a/docs/source/markdown/podman-artifact-add.1.md.in +++ b/docs/source/markdown/podman-artifact-add.1.md.in @@ -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. diff --git a/pkg/domain/entities/artifact.go b/pkg/domain/entities/artifact.go index e7a110fe51..e8cae00c89 100644 --- a/pkg/domain/entities/artifact.go +++ b/pkg/domain/entities/artifact.go @@ -12,6 +12,7 @@ import ( type ArtifactAddOptions struct { Annotations map[string]string ArtifactType string + Append bool } type ArtifactExtractOptions struct { diff --git a/pkg/domain/infra/abi/artifact.go b/pkg/domain/infra/abi/artifact.go index 76ea1f01ae..a65e40f7cf 100644 --- a/pkg/domain/infra/abi/artifact.go +++ b/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) diff --git a/pkg/libartifact/store/store.go b/pkg/libartifact/store/store.go index f95cbb26fa..65205755eb 100644 --- a/pkg/libartifact/store/store.go +++ b/pkg/libartifact/store/store.go @@ -33,6 +33,8 @@ var ( ErrEmptyArtifactName = errors.New("artifact name cannot be empty") ) +const ManifestSchemaVersion = 2 + type ArtifactStore struct { SystemContext *types.SystemContext storePath string @@ -164,12 +166,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) @@ -177,10 +187,49 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, op return nil, err } - // Check if artifact exists; in GetByName not getting an - // error means it exists - if _, _, err := artifacts.GetByNameOrDigest(dest); err == nil { - return nil, fmt.Errorf("artifact %s already exists", dest) + var artifactManifest specV1.Manifest + var oldDigest *digest.Digest + fileNames := map[string]struct{}{} + + if !options.Append { + // Check if artifact exists; in GetByName not getting an + // error means it exists + _, _, err := artifacts.GetByNameOrDigest(dest) + if err == nil { + return nil, fmt.Errorf("artifact %s already exists", dest) + } + artifactManifest = specV1.Manifest{ + Versioned: specs.Versioned{SchemaVersion: ManifestSchemaVersion}, + 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), + } + } else { + artifact, _, err := artifacts.GetByNameOrDigest(dest) + if err != nil { + return nil, err + } + artifactManifest = artifact.Manifest.Manifest + oldDigest, err = artifact.GetDigest() + if err != nil { + return nil, err + } + + for _, layer := range artifactManifest.Layers { + if value, ok := layer.Annotations[specV1.AnnotationTitle]; ok && value != "" { + fileNames[value] = struct{}{} + } + } + } + + for _, path := range paths { + fileName := filepath.Base(path) + if _, ok := fileNames[fileName]; ok { + return nil, fmt.Errorf("file: %q already exists in artifact", fileName) + } + fileNames[fileName] = struct{}{} } ir, err := layout.NewReference(as.storePath, dest) @@ -194,14 +243,9 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, op } defer imageDest.Close() + // ImageDestination, in general, requires the caller to write a full image; here we may write only the added layers. + // This works for the oci/layout transport we hard-code. 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 { @@ -212,28 +256,17 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, op return nil, err } - annots[specV1.AnnotationTitle] = filepath.Base(path) - + annotations := maps.Clone(options.Annotations) + 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 @@ -241,6 +274,7 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, op if err := imageDest.PutManifest(ctx, rawData, nil); err != nil { return nil, err } + unparsed := newUnparsedArtifactImage(ir, artifactManifest) if err := imageDest.Commit(ctx, unparsed); err != nil { return nil, err @@ -253,6 +287,27 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, op 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) } + + // Clean up after append. Remove previous artifact from store. + if oldDigest != nil { + lrs, err := layout.List(as.storePath) + if err != nil { + return nil, err + } + + for _, l := range lrs { + if oldDigest.String() == l.ManifestDescriptor.Digest.String() { + if _, ok := l.ManifestDescriptor.Annotations[specV1.AnnotationRefName]; ok { + continue + } + + if err := l.Reference.DeleteImage(ctx, as.SystemContext); err != nil { + return nil, err + } + break + } + } + } return &artifactManifestDigest, nil } @@ -431,7 +486,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: ManifestSchemaVersion}, } rawData, err := json.Marshal(&index) if err != nil { diff --git a/pkg/libartifact/types/config.go b/pkg/libartifact/types/config.go index 932f75d855..ab0a8eb3d6 100644 --- a/pkg/libartifact/types/config.go +++ b/pkg/libartifact/types/config.go @@ -8,6 +8,8 @@ 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"` } type ExtractOptions struct { diff --git a/test/e2e/artifact_test.go b/test/e2e/artifact_test.go index 12afd62a75..dba86eb931 100644 --- a/test/e2e/artifact_test.go +++ b/test/e2e/artifact_test.go @@ -346,6 +346,162 @@ var _ = Describe("Podman artifact", func() { podmanTest.PodmanExitCleanly("artifact", "extract", "--digest", artifactDigest, ARTIFACT_EVIL, podmanTest.TempDir) Expect(readFileToString(filepath.Join(podmanTest.TempDir, digestToFilename(artifactDigest)))).To(Equal(artifactContent)) }) + + It("podman artifact simple add --append", func() { + artifact1File, err := createArtifactFile(1024) + Expect(err).ToNot(HaveOccurred()) + + artifact2File, err := createArtifactFile(2048) + Expect(err).ToNot(HaveOccurred()) + + artifact3File, err := createArtifactFile(4192) + Expect(err).ToNot(HaveOccurred()) + + layersNames := map[string]int{ + filepath.Base(artifact1File): 0, + filepath.Base(artifact2File): 0, + filepath.Base(artifact3File): 0, + } + + artifact1Name := "localhost/test/artifact1" + podmanTest.PodmanExitCleanly("artifact", "add", artifact1Name, artifact1File) + + _ = podmanTest.InspectArtifact(artifact1Name) + + podmanTest.PodmanExitCleanly("artifact", "add", "--append", artifact1Name, artifact2File) + + a := podmanTest.InspectArtifact(artifact1Name) + + Expect(a.Manifest.Layers).To(HaveLen(2)) + + annotation1 := "color=blue" + podmanTest.PodmanExitCleanly("artifact", "add", "--append", "--annotation", annotation1, artifact1Name, artifact3File) + + a = podmanTest.InspectArtifact(artifact1Name) + Expect(a.Name).To(Equal(artifact1Name)) + Expect(a.Manifest.Layers).To(HaveLen(3)) + + for _, l := range a.Manifest.Layers { + layersNames[l.Annotations["org.opencontainers.image.title"]] += 1 + + if l.Annotations["org.opencontainers.image.title"] == filepath.Base(artifact3File) { + Expect(l.Annotations["color"]).To(Equal("blue")) + } else { + Expect(l.Annotations).To(HaveLen(1)) + } + } + for _, count := range layersNames { + Expect(count).To(Equal(1)) + } + + listSession := podmanTest.PodmanExitCleanly([]string{"artifact", "ls"}...) + Expect(listSession.OutputToStringArray()).To(HaveLen(2)) + }) + + It("podman artifact add --append multiple", func() { + artifact1File, err := createArtifactFile(1024) + Expect(err).ToNot(HaveOccurred()) + + artifact2File, err := createArtifactFile(2048) + Expect(err).ToNot(HaveOccurred()) + + artifact3File, err := createArtifactFile(4192) + Expect(err).ToNot(HaveOccurred()) + + artifact1Name := "localhost/test/artifact1" + podmanTest.PodmanExitCleanly("artifact", "add", artifact1Name, artifact1File) + + podmanTest.PodmanExitCleanly("artifact", "add", "--append", artifact1Name, artifact2File, artifact3File) + + a := podmanTest.InspectArtifact(artifact1Name) + + Expect(a.Manifest.Layers).To(HaveLen(3)) + }) + + It("podman artifact add --append modified file", func() { + artifact1File, err := createArtifactFile(1024) + Expect(err).ToNot(HaveOccurred()) + + artifact1Name := "localhost/test/artifact1" + podmanTest.PodmanExitCleanly("artifact", "add", artifact1Name, artifact1File) + + f, err := os.OpenFile(artifact1File, os.O_APPEND|os.O_WRONLY, 0644) + Expect(err).ToNot(HaveOccurred()) + _, err = f.WriteString("This is modification.") + Expect(err).ToNot(HaveOccurred()) + f.Close() + + appendFail := podmanTest.Podman([]string{"artifact", "add", "--append", artifact1Name, artifact1File}) + appendFail.WaitWithDefaultTimeout() + Expect(appendFail).Should(ExitWithError(125, fmt.Sprintf("Error: file: \"%s\" already exists in artifact", filepath.Base(artifact1File)))) + + a := podmanTest.InspectArtifact(artifact1Name) + + Expect(a.Manifest.Layers).To(HaveLen(1)) + Expect(a.TotalSizeBytes()).To(Equal(int64(524288))) + }) + + It("podman artifact add file already exists in artifact", func() { + artifact1File, err := createArtifactFile(2048) + Expect(err).ToNot(HaveOccurred()) + + artifact1Name := "localhost/test/artifact1" + + addFail := podmanTest.Podman([]string{"artifact", "add", artifact1Name, artifact1File, artifact1File}) + addFail.WaitWithDefaultTimeout() + Expect(addFail).Should(ExitWithError(125, fmt.Sprintf("Error: file: \"%s\" already exists in artifact", filepath.Base(artifact1File)))) + + inspectFail := podmanTest.Podman([]string{"artifact", "inspect", artifact1Name}) + inspectFail.WaitWithDefaultTimeout() + Expect(inspectFail).Should(ExitWithError(125, fmt.Sprintf("Error: no artifact found with name or digest of %s", artifact1Name))) + }) + + It("podman artifact add --append file already exists in artifact", func() { + artifact1File, err := createArtifactFile(2048) + Expect(err).ToNot(HaveOccurred()) + + artifact1Name := "localhost/test/artifact1" + podmanTest.PodmanExitCleanly("artifact", "add", artifact1Name, artifact1File) + + appendFail := podmanTest.Podman([]string{"artifact", "add", "--append", artifact1Name, artifact1File}) + appendFail.WaitWithDefaultTimeout() + Expect(appendFail).Should(ExitWithError(125, fmt.Sprintf("Error: file: \"%s\" already exists in artifact", filepath.Base(artifact1File)))) + a := podmanTest.InspectArtifact(artifact1Name) + + Expect(a.Manifest.Layers).To(HaveLen(1)) + Expect(a.TotalSizeBytes()).To(Equal(int64(1048576))) + }) + + It("podman artifact add with --append and --type", func() { + artifact1Name := "localhost/test/artifact1" + artifact1File, err := createArtifactFile(1024) + Expect(err).ToNot(HaveOccurred()) + + artifact2File, err := createArtifactFile(2048) + Expect(err).ToNot(HaveOccurred()) + + artifact3File, err := createArtifactFile(4192) + Expect(err).ToNot(HaveOccurred()) + + artifactType := "octet/foobar" + + podmanTest.PodmanExitCleanly("artifact", "add", "--type", artifactType, artifact1Name, artifact1File) + + a := podmanTest.InspectArtifact(artifact1Name) + Expect(a.Name).To(Equal(artifact1Name)) + Expect(a.Manifest.ArtifactType).To(Equal(artifactType)) + + podmanTest.PodmanExitCleanly("artifact", "add", "--append", artifact1Name, artifact2File) + + a = podmanTest.InspectArtifact(artifact1Name) + Expect(a.Name).To(Equal(artifact1Name)) + Expect(a.Manifest.ArtifactType).To(Equal(artifactType)) + Expect(a.Manifest.Layers).To(HaveLen(2)) + + failSession := podmanTest.Podman([]string{"artifact", "add", "--type", artifactType, "--append", artifact1Name, artifact3File}) + failSession.WaitWithDefaultTimeout() + Expect(failSession).Should(ExitWithError(125, "Error: append option is not compatible with ArtifactType option")) + }) }) func digestToFilename(digest string) string {