Skip to content

Commit 7030b55

Browse files
committed
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 <[email protected]>
1 parent 61e88e4 commit 7030b55

File tree

4 files changed

+25
-12
lines changed

4 files changed

+25
-12
lines changed

pkg/libartifact/artifact.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ package libartifact
22

33
import (
44
"encoding/json"
5-
"errors"
65
"fmt"
76
"strings"
87

98
"github.com/containers/image/v5/manifest"
9+
"github.com/containers/podman/v5/pkg/libartifact/types"
1010
"github.com/opencontainers/go-digest"
1111
)
1212

@@ -33,7 +33,7 @@ func (a *Artifact) GetName() (string, error) {
3333
}
3434
// We don't have a concept of None for artifacts yet, but if we do,
3535
// then we should probably not error but return `None`
36-
return "", errors.New("artifact is unnamed")
36+
return "", types.ErrArtifactUnamed
3737
}
3838

3939
// SetName is a accessor for setting the artifact name
@@ -75,5 +75,5 @@ func (al ArtifactList) GetByNameOrDigest(nameOrDigest string) (*Artifact, bool,
7575
return artifact, true, nil
7676
}
7777
}
78-
return nil, false, fmt.Errorf("no artifact found with name or digest of %s", nameOrDigest)
78+
return nil, false, fmt.Errorf("%s: %w", nameOrDigest, types.ErrArtifactNotExist)
7979
}

pkg/libartifact/store/store.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, op
196196
// error means it exists
197197
_, _, err := artifacts.GetByNameOrDigest(dest)
198198
if err == nil {
199-
return nil, fmt.Errorf("artifact %s already exists", dest)
199+
return nil, fmt.Errorf("%s: %w", dest, libartTypes.ErrArtifactAlreadyExists)
200200
}
201201
artifactManifest = specV1.Manifest{
202202
Versioned: specs.Versioned{SchemaVersion: ManifestSchemaVersion},
@@ -227,7 +227,7 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, paths []string, op
227227
for _, path := range paths {
228228
fileName := filepath.Base(path)
229229
if _, ok := fileNames[fileName]; ok {
230-
return nil, fmt.Errorf("file: %q already exists in artifact", fileName)
230+
return nil, fmt.Errorf("%s: %w", fileName, libartTypes.ErrArtifactFileExists)
231231
}
232232
fileNames[fileName] = struct{}{}
233233
}

pkg/libartifact/types/errors.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package types
2+
3+
import (
4+
"errors"
5+
)
6+
7+
var (
8+
ErrArtifactUnamed = errors.New("artifact is unnamed")
9+
ErrArtifactNotExist = errors.New("artifact does not exist")
10+
ErrArtifactAlreadyExists = errors.New("artifact already exists")
11+
ErrArtifactFileExists = errors.New("file already exists in artifact")
12+
)

test/e2e/artifact_test.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ var _ = Describe("Podman artifact", func() {
8888
// Adding an artifact with an existing name should fail
8989
addAgain := podmanTest.Podman([]string{"artifact", "add", artifact1Name, artifact1File})
9090
addAgain.WaitWithDefaultTimeout()
91-
Expect(addAgain).Should(ExitWithError(125, fmt.Sprintf("Error: artifact %s already exists", artifact1Name)))
91+
Expect(addAgain).Should(ExitWithError(125, fmt.Sprintf("Error: %s: artifact already exists", artifact1Name)))
9292
})
9393

9494
It("podman artifact add with options", func() {
@@ -157,7 +157,7 @@ var _ = Describe("Podman artifact", func() {
157157
// Trying to remove an image that does not exist should fail
158158
rmFail := podmanTest.Podman([]string{"artifact", "rm", "foobar"})
159159
rmFail.WaitWithDefaultTimeout()
160-
Expect(rmFail).Should(ExitWithError(125, fmt.Sprintf("Error: no artifact found with name or digest of %s", "foobar")))
160+
Expect(rmFail).Should(ExitWithError(125, fmt.Sprintf("Error: %s: artifact does not exist", "foobar")))
161161

162162
// Add an artifact to remove later
163163
artifact1File, err := createArtifactFile(4192)
@@ -173,7 +173,7 @@ var _ = Describe("Podman artifact", func() {
173173
// Inspecting that the removed artifact should fail
174174
inspectArtifact := podmanTest.Podman([]string{"artifact", "inspect", artifact1Name})
175175
inspectArtifact.WaitWithDefaultTimeout()
176-
Expect(inspectArtifact).Should(ExitWithError(125, fmt.Sprintf("Error: no artifact found with name or digest of %s", artifact1Name)))
176+
Expect(inspectArtifact).Should(ExitWithError(125, fmt.Sprintf("Error: %s: artifact does not exist", artifact1Name)))
177177
})
178178

179179
It("podman artifact inspect with full or partial digest", func() {
@@ -433,7 +433,8 @@ var _ = Describe("Podman artifact", func() {
433433

434434
appendFail := podmanTest.Podman([]string{"artifact", "add", "--append", artifact1Name, artifact1File})
435435
appendFail.WaitWithDefaultTimeout()
436-
Expect(appendFail).Should(ExitWithError(125, fmt.Sprintf("Error: file: \"%s\" already exists in artifact", filepath.Base(artifact1File))))
436+
// Error: PJYjLgoU: file already exists in artifact
437+
Expect(appendFail).Should(ExitWithError(125, fmt.Sprintf("Error: %s: file already exists in artifact", filepath.Base(artifact1File))))
437438

438439
a := podmanTest.InspectArtifact(artifact1Name)
439440

@@ -449,11 +450,11 @@ var _ = Describe("Podman artifact", func() {
449450

450451
addFail := podmanTest.Podman([]string{"artifact", "add", artifact1Name, artifact1File, artifact1File})
451452
addFail.WaitWithDefaultTimeout()
452-
Expect(addFail).Should(ExitWithError(125, fmt.Sprintf("Error: file: \"%s\" already exists in artifact", filepath.Base(artifact1File))))
453+
Expect(addFail).Should(ExitWithError(125, fmt.Sprintf("Error: %s: file already exists in artifact", filepath.Base(artifact1File))))
453454

454455
inspectFail := podmanTest.Podman([]string{"artifact", "inspect", artifact1Name})
455456
inspectFail.WaitWithDefaultTimeout()
456-
Expect(inspectFail).Should(ExitWithError(125, fmt.Sprintf("Error: no artifact found with name or digest of %s", artifact1Name)))
457+
Expect(inspectFail).Should(ExitWithError(125, fmt.Sprintf("Error: %s: artifact does not exist", artifact1Name)))
457458
})
458459

459460
It("podman artifact add --append file already exists in artifact", func() {
@@ -465,7 +466,7 @@ var _ = Describe("Podman artifact", func() {
465466

466467
appendFail := podmanTest.Podman([]string{"artifact", "add", "--append", artifact1Name, artifact1File})
467468
appendFail.WaitWithDefaultTimeout()
468-
Expect(appendFail).Should(ExitWithError(125, fmt.Sprintf("Error: file: \"%s\" already exists in artifact", filepath.Base(artifact1File))))
469+
Expect(appendFail).Should(ExitWithError(125, fmt.Sprintf("Error: %s: file already exists in artifact", filepath.Base(artifact1File))))
469470
a := podmanTest.InspectArtifact(artifact1Name)
470471

471472
Expect(a.Manifest.Layers).To(HaveLen(1))

0 commit comments

Comments
 (0)