Define artifact error types

In a different PR review, it was noted that defined error types for
artifacts was lacking.  We have these for most other commands and they
help with error differentiation. The changes here are to define the
errors, implement them in the library, and adopt test verifications to
match.

Signed-off-by: Brent Baude <bbaude@redhat.com>
This commit is contained in:
Brent Baude
2025-02-19 12:35:10 -06:00
parent 61e88e4205
commit 7030b559fb
4 changed files with 25 additions and 12 deletions

View File

@ -2,11 +2,11 @@ package libartifact
import (
"encoding/json"
"errors"
"fmt"
"strings"
"github.com/containers/image/v5/manifest"
"github.com/containers/podman/v5/pkg/libartifact/types"
"github.com/opencontainers/go-digest"
)
@ -33,7 +33,7 @@ func (a *Artifact) GetName() (string, error) {
}
// We don't have a concept of None for artifacts yet, but if we do,
// then we should probably not error but return `None`
return "", errors.New("artifact is unnamed")
return "", types.ErrArtifactUnamed
}
// SetName is a accessor for setting the artifact name
@ -75,5 +75,5 @@ func (al ArtifactList) GetByNameOrDigest(nameOrDigest string) (*Artifact, bool,
return artifact, true, nil
}
}
return nil, false, fmt.Errorf("no artifact found with name or digest of %s", nameOrDigest)
return nil, false, fmt.Errorf("%s: %w", nameOrDigest, types.ErrArtifactNotExist)
}

View File

@ -196,7 +196,7 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, op
// error means it exists
_, _, err := artifacts.GetByNameOrDigest(dest)
if err == nil {
return nil, fmt.Errorf("artifact %s already exists", dest)
return nil, fmt.Errorf("%s: %w", dest, libartTypes.ErrArtifactAlreadyExists)
}
artifactManifest = specV1.Manifest{
Versioned: specs.Versioned{SchemaVersion: ManifestSchemaVersion},
@ -227,7 +227,7 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, op
for _, path := range paths {
fileName := filepath.Base(path)
if _, ok := fileNames[fileName]; ok {
return nil, fmt.Errorf("file: %q already exists in artifact", fileName)
return nil, fmt.Errorf("%s: %w", fileName, libartTypes.ErrArtifactFileExists)
}
fileNames[fileName] = struct{}{}
}

View File

@ -0,0 +1,12 @@
package types
import (
"errors"
)
var (
ErrArtifactUnamed = errors.New("artifact is unnamed")
ErrArtifactNotExist = errors.New("artifact does not exist")
ErrArtifactAlreadyExists = errors.New("artifact already exists")
ErrArtifactFileExists = errors.New("file already exists in artifact")
)

View File

@ -88,7 +88,7 @@ var _ = Describe("Podman artifact", func() {
// 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: artifact %s already exists", artifact1Name)))
Expect(addAgain).Should(ExitWithError(125, fmt.Sprintf("Error: %s: artifact already exists", artifact1Name)))
})
It("podman artifact add with options", func() {
@ -157,7 +157,7 @@ var _ = Describe("Podman artifact", func() {
// Trying to remove an image that does not exist should fail
rmFail := podmanTest.Podman([]string{"artifact", "rm", "foobar"})
rmFail.WaitWithDefaultTimeout()
Expect(rmFail).Should(ExitWithError(125, fmt.Sprintf("Error: no artifact found with name or digest of %s", "foobar")))
Expect(rmFail).Should(ExitWithError(125, fmt.Sprintf("Error: %s: artifact does not exist", "foobar")))
// Add an artifact to remove later
artifact1File, err := createArtifactFile(4192)
@ -173,7 +173,7 @@ var _ = Describe("Podman artifact", func() {
// Inspecting that the removed artifact should fail
inspectArtifact := podmanTest.Podman([]string{"artifact", "inspect", artifact1Name})
inspectArtifact.WaitWithDefaultTimeout()
Expect(inspectArtifact).Should(ExitWithError(125, fmt.Sprintf("Error: no artifact found with name or digest of %s", artifact1Name)))
Expect(inspectArtifact).Should(ExitWithError(125, fmt.Sprintf("Error: %s: artifact does not exist", artifact1Name)))
})
It("podman artifact inspect with full or partial digest", func() {
@ -433,7 +433,8 @@ var _ = Describe("Podman artifact", func() {
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))))
// Error: PJYjLgoU: file already exists in artifact
Expect(appendFail).Should(ExitWithError(125, fmt.Sprintf("Error: %s: file already exists in artifact", filepath.Base(artifact1File))))
a := podmanTest.InspectArtifact(artifact1Name)
@ -449,11 +450,11 @@ var _ = Describe("Podman artifact", func() {
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))))
Expect(addFail).Should(ExitWithError(125, fmt.Sprintf("Error: %s: file 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)))
Expect(inspectFail).Should(ExitWithError(125, fmt.Sprintf("Error: %s: artifact does not exist", artifact1Name)))
})
It("podman artifact add --append file already exists in artifact", func() {
@ -465,7 +466,7 @@ var _ = Describe("Podman artifact", func() {
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))))
Expect(appendFail).Should(ExitWithError(125, fmt.Sprintf("Error: %s: file already exists in artifact", filepath.Base(artifact1File))))
a := podmanTest.InspectArtifact(artifact1Name)
Expect(a.Manifest.Layers).To(HaveLen(1))