feat(artifacts): Default to latest tag if not provided

This change implements a feature where podman artifacts will default to using the 'latest' tag if no tag is provided by the user. This aligns the behavior of artifacts with that of container images, providing a more consistent user experience.

Fixes: #27083

Signed-off-by: Lewis Roy <[email protected]>

Author: ninja-quokka

pkg/api/handlers/libpod/artifacts.go

@@ -346,6 +346,11 @@ func PushArtifact(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
+		if errors.Is(err, libartifact_types.ErrArtifactNotExist) {
+			utils.ArtifactNotFound(w, name, err)
+			return
+		}
+
 		utils.InternalServerError(w, err)
 		return
 	}

pkg/domain/infra/abi/artifact.go

@@ -15,6 +15,7 @@ import (
 	"github.com/opencontainers/go-digest"
 	"github.com/sirupsen/logrus"
 	"go.podman.io/common/libimage"
+	"go.podman.io/image/v5/docker/reference"
 )
 
 func (ir *ImageEngine) ArtifactInspect(ctx context.Context, name string, _ entities.ArtifactInspectOptions) (*entities.ArtifactInspectReport, error) {
@@ -57,6 +58,16 @@ func (ir *ImageEngine) ArtifactList(ctx context.Context, _ entities.ArtifactList
 }
 
 func (ir *ImageEngine) ArtifactPull(ctx context.Context, name string, opts entities.ArtifactPullOptions) (*entities.ArtifactPullReport, error) {
+	named, err := reference.Parse(name)
+	if err != nil {
+		return nil, fmt.Errorf("parsing reference %q: %w", name, err)
+	}
+	namedRef, ok := named.(reference.Named)
+	if !ok {
+		return nil, fmt.Errorf("reference %q is not a Named reference", name)
+	}
+	name = reference.TagNameOnly(namedRef).String()
+
 	pullOptions := &libimage.CopyOptions{}
 	pullOptions.AuthFilePath = opts.AuthFilePath
 	pullOptions.CertDirPath = opts.CertDirPath

pkg/libartifact/artifact.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/containers/podman/v5/pkg/libartifact/types"
 	"github.com/opencontainers/go-digest"
+	"go.podman.io/image/v5/docker/reference"
 	"go.podman.io/image/v5/manifest"
 )
 
@@ -58,9 +59,23 @@ type ArtifactList []*Artifact
 // GetByNameOrDigest returns an artifact, if present, by a given name
 // Returns an error if not found
 func (al ArtifactList) GetByNameOrDigest(nameOrDigest string) (*Artifact, bool, error) {
+	named, err := reference.Parse(nameOrDigest)
+	if err != nil {
+		return nil, false, fmt.Errorf("parsing reference %q: %w", nameOrDigest, err)
+	}
+
+	// Assert that named implements reference.Named interface
+	namedRef, ok := named.(reference.Named)
+	if !ok {
+		return nil, false, fmt.Errorf("reference %q is not a Named reference", nameOrDigest)
+	}
+
+	// Use TagNameOnly with the asserted Named reference
+	refStr := reference.TagNameOnly(namedRef).String()
+
 	// This is the hot route through
 	for _, artifact := range al {
-		if artifact.Name == nameOrDigest {
+		if artifact.Name == refStr {
 			return artifact, false, nil
 		}
 	}

pkg/libartifact/store/store.go

@@ -26,6 +26,7 @@ import (
 	specV1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/sirupsen/logrus"
 	"go.podman.io/common/libimage"
+	"go.podman.io/image/v5/docker/reference"
 	"go.podman.io/image/v5/image"
 	"go.podman.io/image/v5/manifest"
 	"go.podman.io/image/v5/oci/layout"
@@ -101,14 +102,14 @@ func (as ArtifactStore) Remove(ctx context.Context, name string) (*digest.Digest
 		return nil, err
 	}
 
-	arty, nameIsDigest, err := artifacts.GetByNameOrDigest(name)
+	arty, _, err := artifacts.GetByNameOrDigest(name)
 	if err != nil {
 		return nil, err
 	}
-	if nameIsDigest {
-		name = arty.Name
-	}
-	ir, err := layout.NewReference(as.storePath, name)
+
+	// Use the canonical name from the found artifact, which will include the tag
+	// if one was resolved by GetByNameOrDigest.
+	ir, err := layout.NewReference(as.storePath, arty.Name)
 	if err != nil {
 		return nil, err
 	}
@@ -189,7 +190,17 @@ func (as ArtifactStore) Push(ctx context.Context, src, dest string, opts libimag
 	as.lock.Lock()
 	defer as.lock.Unlock()
 
-	srcRef, err := layout.NewReference(as.storePath, src)
+	artifacts, err := as.getArtifacts(ctx, nil)
+	if err != nil {
+		return "", err
+	}
+
+	arty, _, err := artifacts.GetByNameOrDigest(src)
+	if err != nil {
+		return "", err
+	}
+
+	srcRef, err := layout.NewReference(as.storePath, arty.Name)
 	if err != nil {
 		return "", err
 	}
@@ -217,6 +228,18 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, artifactBlobs []en
 		return nil, ErrEmptyArtifactName
 	}
 
+	named, err := reference.Parse(dest)
+	if err != nil {
+		return nil, fmt.Errorf("parsing reference %q: %w", dest, err)
+	}
+
+	namedRef, ok := named.(reference.Named)
+	if !ok {
+		return nil, fmt.Errorf("reference %q is not a Named reference", dest)
+	}
+
+	dest = reference.TagNameOnly(namedRef).String()
+
 	if options.Append && len(options.ArtifactMIMEType) > 0 {
 		return nil, errors.New("append option is not compatible with type option")
 	}
@@ -416,20 +439,16 @@ func getArtifactAndImageSource(ctx context.Context, as ArtifactStore, nameOrDige
 		return nil, nil, err
 	}
 
-	arty, nameIsDigest, err := artifacts.GetByNameOrDigest(nameOrDigest)
+	arty, _, err := artifacts.GetByNameOrDigest(nameOrDigest)
 	if err != nil {
 		return nil, nil, err
 	}
-	name := nameOrDigest
-	if nameIsDigest {
-		name = arty.Name
-	}
 
 	if len(arty.Manifest.Layers) == 0 {
 		return nil, nil, fmt.Errorf("the artifact has no blobs, nothing to extract")
 	}
 
-	ir, err := layout.NewReference(as.storePath, name)
+	ir, err := layout.NewReference(as.storePath, arty.Name)
 	if err != nil {
 		return nil, nil, err
 	}

test/apiv2/python/rest_api/test_v2_0_0_artifact.py

@@ -11,7 +11,7 @@
 
 class ArtifactTestCase(APITestCase):
     def test_add(self):
-        ARTIFACT_NAME = "quay.io/myimage/myartifact:latest"
+        ARTIFACT_NAME = "quay.io/myimage/myartifact"
         file = ArtifactFile()
         parameters: dict[str, str | list[str]] = {
             "name": ARTIFACT_NAME,
@@ -43,6 +43,10 @@ def test_add(self):
         # Assert blob media type fallback detection is working
         self.assertEqual(artifact_layer["mediaType"], "application/octet-stream")
 
+        # Assert latest tag was added by default
+        self.assertEqual(inspect_response_json["Name"], "quay.io/myimage/myartifact:latest")
+
+
     def test_add_with_replace(self):
         ARTIFACT_NAME = "quay.io/myimage/newartifact:latest"
 
@@ -128,7 +132,7 @@ def test_add_with_append(self):
         self.assertEqual(len(artifact_layers), 2)
 
     def test_add_with_artifactMIMEType_override(self):
-        ARTIFACT_NAME = "quay.io/myimage/myartifact_artifactType:latest"
+        ARTIFACT_NAME = "quay.io/myimage/myartifact_artifact_type:latest"
         file = ArtifactFile()
         parameters: dict[str, str | list[str]] = {
             "name": ARTIFACT_NAME,
@@ -672,7 +676,7 @@ def test_push_missing_artifact(self):
 
         # Assert return error response is json and contains correct message
         self.assertIn(
-            "no descriptor found for reference",
+            "artifact does not exist",
             rjson["cause"],
         )
 

test/e2e/artifact_created_test.go

@@ -47,7 +47,7 @@ var _ = Describe("Podman artifact created timestamp", func() {
 
 		// Inspect artifact
 		a := podmanTest.InspectArtifact(artifactName)
-		Expect(a.Name).To(Equal(artifactName))
+		Expect(a.Name).To(Equal(artifactName + ":latest"))
 
 		// Check that created annotation exists and is in valid RFC3339 format
 		createdStr, exists := a.Manifest.Annotations["org.opencontainers.image.created"]

test/e2e/artifact_test.go

@@ -88,12 +88,12 @@ var _ = Describe("Podman artifact", func() {
 
 		a := podmanTest.InspectArtifact(artifact1Name)
 
-		Expect(a.Name).To(Equal(artifact1Name))
+		Expect(a.Name).To(Equal(artifact1Name + ":latest"))
 
 		// Adding an artifact with an existing name should fail
 		addAgain := podmanTest.Podman([]string{"artifact", "add", artifact1Name, artifact1File})
 		addAgain.WaitWithDefaultTimeout()
-		Expect(addAgain).Should(ExitWithError(125, fmt.Sprintf("Error: %s: artifact already exists", artifact1Name)))
+		Expect(addAgain).Should(ExitWithError(125, fmt.Sprintf("Error: %s: artifact already exists", artifact1Name+":latest")))
 	})
 
 	It("podman artifact add with options", func() {
@@ -109,7 +109,7 @@ var _ = Describe("Podman artifact", func() {
 		podmanTest.PodmanExitCleanly("artifact", "add", "--file-type", yamlType, "--type", artifactType, "--annotation", annotation1, "--annotation", annotation2, artifact1Name, artifact1File)
 
 		a := podmanTest.InspectArtifact(artifact1Name)
-		Expect(a.Name).To(Equal(artifact1Name))
+		Expect(a.Name).To(Equal(artifact1Name + ":latest"))
 		Expect(a.Manifest.ArtifactType).To(Equal(artifactType))
 		Expect(a.Manifest.Layers[0].Annotations["color"]).To(Equal("blue"))
 		Expect(a.Manifest.Layers[0].Annotations["flavor"]).To(Equal("lemon"))
@@ -131,7 +131,7 @@ var _ = Describe("Podman artifact", func() {
 		podmanTest.PodmanExitCleanly("artifact", "add", artifact1Name, artifact1File1, artifact1File2)
 
 		a := podmanTest.InspectArtifact(artifact1Name)
-		Expect(a.Name).To(Equal(artifact1Name))
+		Expect(a.Name).To(Equal(artifact1Name + ":latest"))
 
 		Expect(a.Manifest.Layers).To(HaveLen(2))
 	})
@@ -168,7 +168,7 @@ var _ = Describe("Podman artifact", func() {
 
 		a := podmanTest.InspectArtifact(artifact1Name)
 
-		Expect(a.Name).To(Equal(artifact1Name))
+		Expect(a.Name).To(Equal(artifact1Name + ":latest"))
 	})
 
 	It("podman artifact push with authorization", func() {
@@ -476,7 +476,7 @@ var _ = Describe("Podman artifact", func() {
 		podmanTest.PodmanExitCleanly("artifact", "add", "--append", "--annotation", annotation1, artifact1Name, artifact3File)
 
 		a = podmanTest.InspectArtifact(artifact1Name)
-		Expect(a.Name).To(Equal(artifact1Name))
+		Expect(a.Name).To(Equal(artifact1Name + ":latest"))
 		Expect(a.Manifest.Layers).To(HaveLen(3))
 
 		for _, l := range a.Manifest.Layers {
@@ -522,7 +522,6 @@ var _ = Describe("Podman artifact", func() {
 
 		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.")
@@ -587,16 +586,13 @@ var _ = Describe("Podman artifact", func() {
 		podmanTest.PodmanExitCleanly("artifact", "add", "--type", artifactType, artifact1Name, artifact1File)
 
 		a := podmanTest.InspectArtifact(artifact1Name)
-		Expect(a.Name).To(Equal(artifact1Name))
+		Expect(a.Name).To(Equal(artifact1Name + ":latest"))
 		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))
-
+		Expect(a.Name).To(Equal(artifact1Name + ":latest"))
 		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 type option"))
@@ -615,7 +611,7 @@ var _ = Describe("Podman artifact", func() {
 
 		// Inspect artifact
 		a := podmanTest.InspectArtifact(artifact1Name)
-		Expect(a.Name).To(Equal(artifact1Name))
+		Expect(a.Name).To(Equal(artifact1Name + ":latest"))
 
 		// Check that created annotation exists and is in valid Unix nanosecond format
 		createdStr, exists := a.Manifest.Annotations["org.opencontainers.image.created"]

test/e2e/inspect_test.go

@@ -626,7 +626,7 @@ var _ = Describe("Podman inspect", func() {
 		inspect = podmanTest.Podman([]string{"inspect", "--format", "{{.Name}}", artifactName})
 		inspect.WaitWithDefaultTimeout()
 		Expect(inspect).Should(ExitCleanly())
-		Expect(inspect.OutputToString()).To(Equal(artifactName))
+		Expect(inspect.OutputToString()).To(Equal(artifactName + ":latest"))
 
 		inspect2 := podmanTest.Podman([]string{"inspect", "--format", "{{.Digest}}", artifactName})
 		inspect2.WaitWithDefaultTimeout()