From b765c91580d57244a48534b73e17ffeb6ba7a9d6 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Tue, 30 Sep 2025 06:28:41 -0400 Subject: [PATCH] Add --replace option to podman artifact add command This commit implements the --replace functionality for the artifact add command, allowing users to replace existing artifacts without having to manually remove them first. Changes made: - Add Replace field to ArtifactAddOptions entity types - Add --replace CLI flag with validation to prevent conflicts with --append - Implement replace logic in ABI backend to remove existing artifacts before adding - Update API handlers and tunnel implementation for podman-remote support - Add comprehensive documentation and examples to man page - Add e2e and system BATS tests for --replace functionality - Fix code formatting in pkg/bindings/artifacts/types_pull_options.go: * Reorder imports with proper spacing * Fix function declaration spacing * Convert spaces to proper tab indentation * Remove extraneous blank lines The --replace option follows the same pattern as other podman replace options like 'podman container create --replace' and 'podman pod create --replace'. It gracefully handles cases where no existing artifact exists (no error thrown). Usage examples: podman artifact add --replace quay.io/myimage/artifact:latest /path/to/file podman artifact add --replace localhost/test/artifact /tmp/newfile.txt Fixes: Implements requested --replace functionality for artifact add command Signed-off-by: Daniel J Walsh --- cmd/podman/artifact/add.go | 8 ++ .../markdown/podman-artifact-add.1.md.in | 35 ++++-- docs/source/markdown/podman-artifact.1.md | 2 +- pkg/api/handlers/libpod/artifacts.go | 2 + pkg/api/server/register_artifacts.go | 5 + pkg/bindings/artifacts/types.go | 1 + pkg/bindings/artifacts/types_add_options.go | 15 +++ pkg/domain/entities/artifact.go | 1 + pkg/domain/infra/abi/artifact.go | 8 ++ pkg/domain/infra/tunnel/artifact.go | 1 + pkg/libartifact/types/config.go | 2 + .../python/rest_api/test_v2_0_0_artifact.py | 59 +++++++++ test/system/700-artifact.bats | 113 ++++++++++++++++++ test/system/701-artifact-created.bats | 107 +++++++++++++++++ 14 files changed, 345 insertions(+), 14 deletions(-) create mode 100644 test/system/700-artifact.bats create mode 100644 test/system/701-artifact-created.bats diff --git a/cmd/podman/artifact/add.go b/cmd/podman/artifact/add.go index 8a40e53573..3e5903eff3 100644 --- a/cmd/podman/artifact/add.go +++ b/cmd/podman/artifact/add.go @@ -53,6 +53,9 @@ func init() { appendFlagName := "append" flags.BoolVarP(&addOpts.Append, appendFlagName, "a", false, "Append files to an existing artifact") + replaceFlagName := "replace" + flags.BoolVar(&addOpts.Replace, replaceFlagName, false, "Replace an existing artifact") + fileMIMETypeFlagName := "file-type" flags.StringVarP(&addOpts.FileMIMEType, fileMIMETypeFlagName, "", "", "Set file type to use for the artifact (layer)") _ = addCmd.RegisterFlagCompletionFunc(fileMIMETypeFlagName, completion.AutocompleteNone) @@ -62,6 +65,10 @@ func add(_ *cobra.Command, args []string) error { artifactName := args[0] blobs := args[1:] + if addOpts.Append && addOpts.Replace { + return fmt.Errorf("--append and --replace options cannot be used together") + } + annots, err := utils.ParseAnnotations(addOpts.AnnotationsCLI) if err != nil { return err @@ -72,6 +79,7 @@ func add(_ *cobra.Command, args []string) error { ArtifactMIMEType: addOpts.ArtifactMIMEType, Append: addOpts.Append, FileMIMEType: addOpts.FileMIMEType, + Replace: addOpts.Replace, } artifactBlobs := make([]entities.ArtifactBlob, 0, len(blobs)) diff --git a/docs/source/markdown/podman-artifact-add.1.md.in b/docs/source/markdown/podman-artifact-add.1.md.in index 8306049585..24d4793827 100644 --- a/docs/source/markdown/podman-artifact-add.1.md.in +++ b/docs/source/markdown/podman-artifact-add.1.md.in @@ -1,10 +1,10 @@ % podman-artifact-add 1 ## NAME -podman\-artifact\-add - Add an OCI artifact to the local store +podman\-artifact\-add - Add an OCI artifact to local artifact store ## SYNOPSIS -**podman artifact add** *name* *file* [*file*]... +**podman artifact add** [*options*] *artifact-name* *file* [*file* ...] ## DESCRIPTION @@ -35,6 +35,11 @@ Set the media type of the artifact file instead of allowing detection to determi Print usage statement. +#### **--replace** + +If an artifact with the same name already exists, replace and remove it. The default is **false**. +This option cannot be used with the **--append** option. + #### **--type** Set a type for the artifact being added. @@ -48,25 +53,29 @@ $ podman artifact add quay.io/myartifact/myml:latest /tmp/foobar.ml 0fe1488ecdef8cc4093e11a55bc048d9fc3e13a4ba846efd24b5a715006c95b3 ``` +Add OCI artifact to the store with type: + +``` +$ podman artifact add --artifact-type application/com.example.ai --file-type application/vnd.gguf quay.io/myimage/myartifact:latest /home/user/model.gguf +``` + +Replace an existing artifact with the same name + +``` +$ podman artifact add quay.io/myartifact/myml:latest /tmp/foobar.ml +0fe1488ecdef8cc4093e11a55bc048d9fc3e13a4ba846efd24b5a715006c95b3 +``` + Add multiple files to an artifact ``` $ podman artifact add quay.io/myartifact/myml:latest /tmp/foobar1.ml /tmp/foobar2.ml 1487acae11b5a30948c50762882036b41ac91a7b9514be8012d98015c95ddb78 ``` -Set an annotation for an artifact -``` -$ podman artifact add --annotation date=2025-01-30 quay.io/myartifact/myml:latest /tmp/foobar1.ml -``` +Add files to an existing OCI artifact -Append a file to an existing artifact ``` -$ podman artifact add --append quay.io/myartifact/tarballs:latest /tmp/foobar.tar.gz -``` - -Override the media type of the artifact being added -``` -$ podman artifact add --file-type text/yaml quay.io/myartifact/descriptors:latest /tmp/info.yaml +$ podman artifact add --append quay.io/myimage/myartifact:latest /home/user/config.yaml ``` diff --git a/docs/source/markdown/podman-artifact.1.md b/docs/source/markdown/podman-artifact.1.md index 68cdf23bc5..8298aea449 100644 --- a/docs/source/markdown/podman-artifact.1.md +++ b/docs/source/markdown/podman-artifact.1.md @@ -17,7 +17,7 @@ from its local "artifact store". | Command | Man Page | Description | |---------|------------------------------------------------------------|--------------------------------------------------------------| -| add | [podman-artifact-add(1)](podman-artifact-add.1.md) | Add an OCI artifact to the local store | +| add | [podman-artifact-add(1)](podman-artifact-add.1.md) | Add an OCI artifact to local artifact store | | extract | [podman-artifact-extract(1)](podman-artifact-extract.1.md) | Extract an OCI artifact to a local path | | inspect | [podman-artifact-inspect(1)](podman-artifact-inspect.1.md) | Inspect an OCI artifact | | ls | [podman-artifact-ls(1)](podman-artifact-ls.1.md) | List OCI artifacts in local store | diff --git a/pkg/api/handlers/libpod/artifacts.go b/pkg/api/handlers/libpod/artifacts.go index 41d32d97b1..4369134f4b 100644 --- a/pkg/api/handlers/libpod/artifacts.go +++ b/pkg/api/handlers/libpod/artifacts.go @@ -223,6 +223,7 @@ func AddArtifact(w http.ResponseWriter, r *http.Request) { Annotations []string `schema:"annotations"` ArtifactMIMEType string `schema:"artifactMIMEType"` Append bool `schema:"append"` + Replace bool `schema:"replace"` }{} if err := decoder.Decode(&query, r.URL.Query()); err != nil { @@ -246,6 +247,7 @@ func AddArtifact(w http.ResponseWriter, r *http.Request) { Annotations: annotations, ArtifactMIMEType: query.ArtifactMIMEType, FileMIMEType: query.FileMIMEType, + Replace: query.Replace, } artifactBlobs := []entities.ArtifactBlob{{ diff --git a/pkg/api/server/register_artifacts.go b/pkg/api/server/register_artifacts.go index 8fd58f7877..123984c6fb 100644 --- a/pkg/api/server/register_artifacts.go +++ b/pkg/api/server/register_artifacts.go @@ -191,6 +191,11 @@ func (s *APIServer) registerArtifactHandlers(r *mux.Router) error { // description: Append files to an existing artifact // type: boolean // default: false + // - name: replace + // in: query + // description: Replace an existing artifact with the same name + // type: boolean + // default: false // - name: inputStream // in: body // description: Binary stream of the file to add to an artifact diff --git a/pkg/bindings/artifacts/types.go b/pkg/bindings/artifacts/types.go index e9d06dadac..e7761bfb93 100644 --- a/pkg/bindings/artifacts/types.go +++ b/pkg/bindings/artifacts/types.go @@ -64,6 +64,7 @@ type AddOptions struct { ArtifactMIMEType *string Append *bool FileMIMEType *string + Replace *bool } // ExtractOptions diff --git a/pkg/bindings/artifacts/types_add_options.go b/pkg/bindings/artifacts/types_add_options.go index f8088009ec..7458e61c6a 100644 --- a/pkg/bindings/artifacts/types_add_options.go +++ b/pkg/bindings/artifacts/types_add_options.go @@ -76,3 +76,18 @@ func (o *AddOptions) GetFileMIMEType() string { } return *o.FileMIMEType } + +// WithReplace set field Replace to given value +func (o *AddOptions) WithReplace(value bool) *AddOptions { + o.Replace = &value + return o +} + +// GetReplace returns value of field Replace +func (o *AddOptions) GetReplace() bool { + if o.Replace == nil { + var z bool + return z + } + return *o.Replace +} diff --git a/pkg/domain/entities/artifact.go b/pkg/domain/entities/artifact.go index 48dc62a9f6..3edbedda11 100644 --- a/pkg/domain/entities/artifact.go +++ b/pkg/domain/entities/artifact.go @@ -13,6 +13,7 @@ type ArtifactAddOptions struct { ArtifactMIMEType string Append bool FileMIMEType string + Replace bool } type ArtifactAddReport = entitiesTypes.ArtifactAddReport diff --git a/pkg/domain/infra/abi/artifact.go b/pkg/domain/infra/abi/artifact.go index 26c6a203ea..faa16d7ce4 100644 --- a/pkg/domain/infra/abi/artifact.go +++ b/pkg/domain/infra/abi/artifact.go @@ -209,11 +209,19 @@ func (ir *ImageEngine) ArtifactAdd(ctx context.Context, name string, artifactBlo return nil, err } + // If replace is true, try to remove existing artifact (ignore errors if it doesn't exist) + if opts.Replace { + if _, err = artStore.Remove(ctx, name); err != nil && !errors.Is(err, types.ErrArtifactNotExist) { + logrus.Debugf("Artifact %q removal failed: %s", name, err) + } + } + addOptions := types.AddOptions{ Annotations: opts.Annotations, ArtifactMIMEType: opts.ArtifactMIMEType, Append: opts.Append, FileMIMEType: opts.FileMIMEType, + Replace: opts.Replace, } artifactDigest, err := artStore.Add(ctx, name, artifactBlobs, &addOptions) diff --git a/pkg/domain/infra/tunnel/artifact.go b/pkg/domain/infra/tunnel/artifact.go index 826b1f6343..19cbd2126c 100644 --- a/pkg/domain/infra/tunnel/artifact.go +++ b/pkg/domain/infra/tunnel/artifact.go @@ -89,6 +89,7 @@ func (ir *ImageEngine) ArtifactAdd(_ context.Context, name string, artifactBlob Append: &opts.Append, ArtifactMIMEType: &opts.ArtifactMIMEType, FileMIMEType: &opts.FileMIMEType, + Replace: &opts.Replace, } for k, v := range opts.Annotations { diff --git a/pkg/libartifact/types/config.go b/pkg/libartifact/types/config.go index 19f516b4ec..6980f631b3 100644 --- a/pkg/libartifact/types/config.go +++ b/pkg/libartifact/types/config.go @@ -13,6 +13,8 @@ type AddOptions struct { // FileType describes the media type for the layer. It is an override // for the standard detection FileMIMEType string `json:",omitempty"` + // Replace option removes existing artifact before adding new one + Replace bool `json:",omitempty"` } // FilterBlobOptions options used to filter for a single blob in an artifact diff --git a/test/apiv2/python/rest_api/test_v2_0_0_artifact.py b/test/apiv2/python/rest_api/test_v2_0_0_artifact.py index c584ca386a..919a578d7c 100644 --- a/test/apiv2/python/rest_api/test_v2_0_0_artifact.py +++ b/test/apiv2/python/rest_api/test_v2_0_0_artifact.py @@ -43,6 +43,65 @@ class ArtifactTestCase(APITestCase): # Assert blob media type fallback detection is working self.assertEqual(artifact_layer["mediaType"], "application/octet-stream") + def test_add_with_replace(self): + ARTIFACT_NAME = "quay.io/myimage/newartifact:latest" + + # Create first artifact + file1 = ArtifactFile() + parameters1: dict[str, str | list[str]] = { + "name": ARTIFACT_NAME, + "fileName": file1.name, + } + + artifact1 = Artifact(self.uri(""), ARTIFACT_NAME, parameters1, file1) + add_response1 = artifact1.add() + self.assertEqual(add_response1.status_code, 201, add_response1.text) + + original_digest = add_response1.json()["ArtifactDigest"] + + # Create replacement artifact with replace=true + file2 = ArtifactFile() + parameters2: dict[str, str | list[str]] = { + "name": ARTIFACT_NAME, + "fileName": file2.name, + "replace": "true", + } + + artifact2 = Artifact(self.uri(""), ARTIFACT_NAME, parameters2, file2) + add_response2 = artifact2.add() + self.assertEqual(add_response2.status_code, 201, add_response2.text) + + new_digest = add_response2.json()["ArtifactDigest"] + + # Verify artifacts were replaced (different digests) + self.assertNotEqual(original_digest, new_digest) + + # Verify artifact exists and has the new content + inspect_response = artifact2.do_artifact_inspect_request() + self.assertEqual(inspect_response.status_code, 200) + + inspect_json = inspect_response.json() + artifact_layer = inspect_json["Manifest"]["layers"][0] + + # Should have only one layer (replaced, not appended) + self.assertEqual(len(inspect_json["Manifest"]["layers"]), 1) + + # Verify it's the second file's content + self.assertEqual(artifact_layer["size"], file2.size) + self.assertEqual( + artifact_layer["annotations"]["org.opencontainers.image.title"], file2.name + ) + + url = self.uri("/artifacts/" + ARTIFACT_NAME) + r = requests.delete(url) + rjson = r.json() + + # Assert correct response code + self.assertEqual(r.status_code, 200, r.text) + + # Assert return response is json and contains digest + self.assertIn("sha256:", rjson["ArtifactDigests"][0]) + def test_add_with_append(self): ARTIFACT_NAME = "quay.io/myimage/myartifact:latest" file = ArtifactFile(name="test_file_2") diff --git a/test/system/700-artifact.bats b/test/system/700-artifact.bats new file mode 100644 index 0000000000..19116f3083 --- /dev/null +++ b/test/system/700-artifact.bats @@ -0,0 +1,113 @@ +#!/usr/bin/env bats -*- bats -*- +# +# Tests for podman artifact functionality +# + +load helpers + +function setup() { + basic_setup +} + +function teardown() { + basic_teardown +} + +# Helper function to create a test artifact file +create_test_file() { + local size=${1:-1024} + local filename=$(mktemp --tmpdir="${PODMAN_TMPDIR}" artifactfile.XXXXXX) + dd if=/dev/urandom of="$filename" bs=1 count="$size" 2>/dev/null + echo "$filename" +} + +# bats test_tags=ci:parallel +@test "podman artifact add --replace basic functionality" { + local artifact_name="localhost/test/replace-artifact" + local file1 file2 + + file1=$(create_test_file 1024) + file2=$(create_test_file 2048) + + # Add initial artifact + run_podman artifact add "$artifact_name" "$file1" + local first_digest="$output" + + # Verify initial artifact exists + run_podman artifact inspect "$artifact_name" + + # Replace with different file + run_podman artifact add --replace "$artifact_name" "$file2" + local second_digest="$output" + + # Verify artifact was replaced (different digest) + assert "$first_digest" != "$second_digest" "Replace should create different digest" + + # Verify artifact still exists and is accessible + run_podman artifact inspect "$artifact_name" + + # Cleanup + run_podman artifact rm "$artifact_name" + rm -f "$file1" "$file2" +} + +# bats test_tags=ci:parallel +@test "podman artifact add --replace nonexistent artifact" { + local artifact_name="localhost/test/nonexistent-artifact" + local file1 + + file1=$(create_test_file 1024) + + # Using --replace on nonexistent artifact should succeed + run_podman artifact add --replace "$artifact_name" "$file1" + + # Verify artifact was created + run_podman artifact inspect "$artifact_name" + + # Cleanup + run_podman artifact rm "$artifact_name" + rm -f "$file1" +} + +# bats test_tags=ci:parallel +@test "podman artifact add --replace and --append conflict" { + local artifact_name="localhost/test/conflict-artifact" + local file1 + + file1=$(create_test_file 1024) + + # Using --replace and --append together should fail + run_podman 125 artifact add --replace --append "$artifact_name" "$file1" + assert "$output" =~ "--append and --replace options cannot be used together" + + rm -f "$file1" +} + +# bats test_tags=ci:parallel +@test "podman artifact add --replace with existing artifact" { + local artifact_name="localhost/test/existing-artifact" + local file1 file2 + + file1=$(create_test_file 512) + file2=$(create_test_file 1024) + + # Create initial artifact + run_podman artifact add "$artifact_name" "$file1" + + # Verify initial artifact exists + run_podman artifact inspect "$artifact_name" + + # Adding same name without --replace should fail + run_podman 125 artifact add "$artifact_name" "$file2" + assert "$output" =~ "artifact already exists" + + # Replace should succeed + run_podman artifact add --replace "$artifact_name" "$file2" + + # Verify artifact was replaced + run_podman artifact inspect "$artifact_name" + + # Cleanup + run_podman artifact rm "$artifact_name" + rm -f "$file1" "$file2" +} diff --git a/test/system/701-artifact-created.bats b/test/system/701-artifact-created.bats new file mode 100644 index 0000000000..be0c396cba --- /dev/null +++ b/test/system/701-artifact-created.bats @@ -0,0 +1,107 @@ +#!/usr/bin/env bats -*- bats -*- +# +# Tests for podman artifact created date functionality +# + +load helpers + +# Create temporary artifact file for testing +function create_test_file() { + local content="$1" + local filename=$(random_string 12) + local filepath="$PODMAN_TMPDIR/$filename.txt" + echo "$content" > "$filepath" + echo "$filepath" +} + +function setup() { + basic_setup + skip_if_remote "artifacts are not remote" +} + +function teardown() { + run_podman artifact rm --all --ignore || true + basic_teardown +} + +@test "podman artifact inspect shows created date in RFC3339" { + local content="test content for created date" + local testfile=$(create_test_file "$content") + local artifact_name="localhost/test/created-test" + + # Record time before creation (in seconds for comparison) + local before_epoch=$(date +%s) + + # Create artifact + run_podman artifact add $artifact_name "$testfile" + + # Record time after creation (in seconds for comparison) + local after_epoch=$(date +%s) + + # Inspect the artifact + run_podman artifact inspect $artifact_name + local output="$output" + + # Parse the JSON output to get the created annotation + local created_annotation + created_annotation=$(echo "$output" | jq -r '.Manifest.annotations["org.opencontainers.image.created"]') + + # Verify created annotation exists and is not null + assert "$created_annotation" != "null" "Should have org.opencontainers.image.created annotation" + assert "$created_annotation" != "" "Created annotation should not be empty" + + # Verify it's a valid RFC3339 timestamp by trying to parse it + # Convert to epoch for comparison + local created_epoch + created_epoch=$(date -d "$created_annotation" +%s 2>/dev/null) + + # Verify parsing succeeded + assert "$?" -eq 0 "Created timestamp should be valid RFC3339 format" + + # Verify timestamp is within reasonable bounds + assert "$created_epoch" -ge "$before_epoch" "Created time should be after before_epoch" + assert "$created_epoch" -le "$after_epoch" "Created time should be before after_epoch" + + # Clean up + rm -f "$testfile" +} + +@test "podman artifact append preserves original created date" { + local content1="initial content" + local content2="appended content" + local testfile1=$(create_test_file "$content1") + local testfile2=$(create_test_file "$content2") + local artifact_name="localhost/test/append-test" + + # Create initial artifact + run_podman artifact add $artifact_name "$testfile1" + + # Get the original created timestamp + run_podman artifact inspect $artifact_name + local original_created + original_created=$(echo "$output" | jq -r '.Manifest.annotations["org.opencontainers.image.created"]') + + # Wait a bit to ensure timestamps would differ if created new + sleep 1 + + # Append to artifact + run_podman artifact add --append $artifact_name "$testfile2" + + # Get the created timestamp after append + run_podman artifact inspect $artifact_name + local current_created + current_created=$(echo "$output" | jq -r '.Manifest.annotations["org.opencontainers.image.created"]') + + # Verify the created timestamp is preserved + assert "$current_created" = "$original_created" "Created timestamp should be preserved during append" + + # Verify we have 2 layers now + local layer_count + layer_count=$(echo "$output" | jq '.Manifest.layers | length') + assert "$layer_count" -eq 2 "Should have 2 layers after append" + + # Clean up + rm -f "$testfile1" "$testfile2" +} + +# vim: filetype=sh