Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cli/command/manifest/annotate.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (

"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/manifest/store"
"github.com/docker/cli/cli/manifest/types"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -60,7 +60,7 @@ func runManifestAnnotate(dockerCli command.Cli, opts annotateOptions) error {
manifestStore := dockerCli.ManifestStore()
imageManifest, err := manifestStore.Get(targetRef, imgRef)
switch {
case store.IsNotFound(err):
case errors.Is(err, types.ErrManifestNotFound):
return fmt.Errorf("manifest for image %s does not exist in %s", opts.image, opts.target)
case err != nil:
return err
Expand Down
4 changes: 2 additions & 2 deletions cli/command/manifest/create_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (

"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/manifest/store"
"github.com/docker/cli/cli/manifest/types"
"github.com/docker/docker/registry"
"github.com/pkg/errors"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -50,7 +50,7 @@ func createManifestList(dockerCli command.Cli, args []string, opts createOpts) e
manifestStore := dockerCli.ManifestStore()
_, err = manifestStore.GetList(targetRef)
switch {
case store.IsNotFound(err):
case errors.Is(err, types.ErrManifestNotFound):
// New manifest list
case err != nil:
return err
Expand Down
6 changes: 3 additions & 3 deletions cli/command/manifest/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,16 +99,16 @@ func TestManifestCreateNoManifest(t *testing.T) {
cli.SetManifestStore(store)
cli.SetRegistryClient(&fakeRegistryClient{
getManifestFunc: func(_ context.Context, ref reference.Named) (manifesttypes.ImageManifest, error) {
return manifesttypes.ImageManifest{}, errors.Errorf("No such image: %v", ref)
return manifesttypes.ImageManifest{}, errors.Wrapf(manifesttypes.ErrManifestNotFound, "%q does not exist", ref)
},
getManifestListFunc: func(ctx context.Context, ref reference.Named) ([]manifesttypes.ImageManifest, error) {
return nil, errors.Errorf("No such manifest: %s", ref)
return nil, errors.Wrapf(manifesttypes.ErrManifestNotFound, "%q does not exist", ref)
},
})

cmd := newCreateListCommand(cli)
cmd.SetArgs([]string{"example.com/list:v1", "example.com/alpine:3.0"})
cmd.SetOut(io.Discard)
err := cmd.Execute()
assert.Error(t, err, "No such image: example.com/alpine:3.0")
assert.Error(t, err, `"example.com/alpine:3.0" does not exist: manifest not found`)
}
10 changes: 5 additions & 5 deletions cli/command/manifest/inspect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestInspectCommandLocalManifestNotFound(t *testing.T) {
cmd.SetOut(io.Discard)
cmd.SetArgs([]string{"example.com/list:v1", "example.com/alpine:3.0"})
err := cmd.Execute()
assert.Error(t, err, "No such manifest: example.com/alpine:3.0")
assert.Error(t, err, `"example.com/alpine:3.0" does not exist: manifest not found`)
}

func TestInspectCommandNotFound(t *testing.T) {
Expand All @@ -79,19 +79,19 @@ func TestInspectCommandNotFound(t *testing.T) {
cli := test.NewFakeCli(nil)
cli.SetManifestStore(store)
cli.SetRegistryClient(&fakeRegistryClient{
getManifestFunc: func(_ context.Context, _ reference.Named) (types.ImageManifest, error) {
return types.ImageManifest{}, errors.New("missing")
getManifestFunc: func(ctx context.Context, ref reference.Named) (types.ImageManifest, error) {
return types.ImageManifest{}, errors.Wrapf(types.ErrManifestNotFound, "%q does not exist", ref)
},
getManifestListFunc: func(ctx context.Context, ref reference.Named) ([]types.ImageManifest, error) {
return nil, errors.Errorf("No such manifest: %s", ref)
return nil, errors.Wrapf(types.ErrManifestNotFound, "%q does not exist", ref)
},
})

cmd := newInspectCommand(cli)
cmd.SetOut(io.Discard)
cmd.SetArgs([]string{"example.com/alpine:3.0"})
err := cmd.Execute()
assert.Error(t, err, "No such manifest: example.com/alpine:3.0")
assert.Error(t, err, `"example.com/alpine:3.0" does not exist: manifest not found`)
}

func TestInspectCommandLocalManifest(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions cli/command/manifest/rm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ func TestRmSeveralManifests(t *testing.T) {

_, search1 := cli.ManifestStore().GetList(list1)
_, search2 := cli.ManifestStore().GetList(list2)
assert.Error(t, search1, "No such manifest: example.com/first:1")
assert.Error(t, search2, "No such manifest: example.com/second:2")
assert.Error(t, search1, `"example.com/first:1" does not exist: manifest not found`)
assert.Error(t, search2, `"example.com/second:2" does not exist: manifest not found`)
}

// attempt to remove a manifest list which was never created
Expand All @@ -57,8 +57,8 @@ func TestRmManifestNotCreated(t *testing.T) {
cmd.SetArgs([]string{"example.com/first:1", "example.com/second:2"})
cmd.SetOut(io.Discard)
err = cmd.Execute()
assert.Error(t, err, "No such manifest: example.com/first:1")
assert.Error(t, err, `"example.com/first:1" does not exist: manifest not found`)

_, err = cli.ManifestStore().GetList(list2)
assert.Error(t, err, "No such manifest: example.com/second:2")
assert.Error(t, err, `"example.com/second:2" does not exist: manifest not found`)
}
33 changes: 22 additions & 11 deletions cli/command/manifest/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import (
"context"

"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/manifest/store"
"github.com/docker/cli/cli/manifest/types"
"github.com/docker/distribution/reference"
"github.com/pkg/errors"
)

type osArch struct {
Expand Down Expand Up @@ -70,15 +70,26 @@ func normalizeReference(ref string) (reference.Named, error) {
// getManifest from the local store, and fallback to the remote registry if it
// doesn't exist locally
func getManifest(ctx context.Context, dockerCli command.Cli, listRef, namedRef reference.Named, insecure bool) (types.ImageManifest, error) {
data, err := dockerCli.ManifestStore().Get(listRef, namedRef)
switch {
case store.IsNotFound(err):
return dockerCli.RegistryClient(insecure).GetManifest(ctx, namedRef)
case err != nil:
return types.ImageManifest{}, err
case len(data.Raw) == 0:
return dockerCli.RegistryClient(insecure).GetManifest(ctx, namedRef)
default:
return data, nil
// load from the local store
if listRef != nil {
data, err := dockerCli.ManifestStore().Get(listRef, namedRef)
if err == nil {
return data, nil
} else if !errors.Is(err, types.ErrManifestNotFound) {
return types.ImageManifest{}, err
}
}

// load from the remote registry
client := dockerCli.RegistryClient(insecure)
if client != nil {
data, err := client.GetManifest(ctx, namedRef)
if err == nil {
return data, nil
} else if !errors.Is(err, types.ErrManifestNotFound) {
return types.ImageManifest{}, err
}
}

return types.ImageManifest{}, errors.Wrapf(types.ErrManifestNotFound, "%q does not exist", namedRef)
}
30 changes: 2 additions & 28 deletions cli/manifest/store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package store

import (
"encoding/json"
"fmt"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -49,7 +48,7 @@ func (s *fsStore) getFromFilename(ref reference.Reference, filename string) (typ
bytes, err := os.ReadFile(filename)
switch {
case os.IsNotExist(err):
return types.ImageManifest{}, newNotFoundError(ref.String())
return types.ImageManifest{}, errors.Wrapf(types.ErrManifestNotFound, "%q does not exist", ref.String())
case err != nil:
return types.ImageManifest{}, err
}
Expand Down Expand Up @@ -93,7 +92,7 @@ func (s *fsStore) GetList(listRef reference.Reference) ([]types.ImageManifest, e
case err != nil:
return nil, err
case filenames == nil:
return nil, newNotFoundError(listRef.String())
return nil, errors.Wrapf(types.ErrManifestNotFound, "%q does not exist", listRef.String())
}

manifests := []types.ImageManifest{}
Expand Down Expand Up @@ -152,28 +151,3 @@ func makeFilesafeName(ref string) string {
fileName := strings.Replace(ref, ":", "-", -1)
return strings.Replace(fileName, "/", "_", -1)
}

type notFoundError struct {
object string
}

func newNotFoundError(ref string) *notFoundError {
return &notFoundError{object: ref}
}

func (n *notFoundError) Error() string {
return fmt.Sprintf("No such manifest: %s", n.object)
}

// NotFound interface
func (n *notFoundError) NotFound() {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering; instead of a new error-type, could we change the errors to use these, and use errdefs.IsNotFound() (etc)?

// IsNotFound returns if the passed in error is an ErrNotFound
func IsNotFound(err error) bool {
_, ok := getImplementer(err).(ErrNotFound)
return ok
}


// IsNotFound returns true if the error is a not found error
func IsNotFound(err error) bool {
_, ok := err.(notFound)
return ok
}

type notFound interface {
NotFound()
}
15 changes: 7 additions & 8 deletions cli/manifest/store/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/docker/cli/cli/manifest/types"
"github.com/docker/distribution/reference"
"github.com/google/go-cmp/cmp"
"github.com/pkg/errors"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
)
Expand Down Expand Up @@ -62,7 +63,7 @@ func TestStoreSaveAndGet(t *testing.T) {
listRef reference.Reference
manifestRef reference.Reference
expected types.ImageManifest
expectedErr string
expectedErr error
}{
{
listRef: listRef,
Expand All @@ -72,22 +73,21 @@ func TestStoreSaveAndGet(t *testing.T) {
{
listRef: listRef,
manifestRef: ref("exist:does-not"),
expectedErr: "No such manifest: exist:does-not",
expectedErr: types.ErrManifestNotFound,
},
{
listRef: ref("list:does-not-exist"),
manifestRef: ref("manifest:does-not-exist"),
expectedErr: "No such manifest: manifest:does-not-exist",
expectedErr: types.ErrManifestNotFound,
},
}

for _, testcase := range testcases {
testcase := testcase
t.Run(testcase.manifestRef.String(), func(t *testing.T) {
actual, err := store.Get(testcase.listRef, testcase.manifestRef)
if testcase.expectedErr != "" {
assert.Error(t, err, testcase.expectedErr)
assert.Check(t, IsNotFound(err))
if testcase.expectedErr != nil {
assert.Check(t, errors.Is(err, types.ErrManifestNotFound))
return
}
assert.NilError(t, err)
Expand Down Expand Up @@ -117,6 +117,5 @@ func TestStoreGetListDoesNotExist(t *testing.T) {
store := NewStore(t.TempDir())
listRef := ref("list")
_, err := store.GetList(listRef)
assert.Error(t, err, "No such manifest: list")
assert.Check(t, IsNotFound(err))
assert.Check(t, errors.Is(err, types.ErrManifestNotFound))
}
10 changes: 10 additions & 0 deletions cli/manifest/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,16 @@ import (
"github.com/pkg/errors"
)

var (
// ErrManifestNotFound is an error returned when a requested manifest or
// manifest list cannot be found in the store or the registry
ErrManifestNotFound = errors.New("manifest not found")

// ErrUnknownType is an error returned when a manifest was found, but it
// was of an unrecognized type
ErrUnknownType = errors.New("unknown manifest type")
)

// ImageManifest contains info to output for a manifest object.
type ImageManifest struct {
Ref *SerializableNamed
Expand Down
29 changes: 8 additions & 21 deletions cli/registry/client/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ func fetchManifest(ctx context.Context, repo distribution.Repository, ref refere
}
return imageManifest, nil
case *manifestlist.DeserializedManifestList:
return types.ImageManifest{}, errors.Errorf("%s is a manifest list", ref)
return types.ImageManifest{}, errors.Wrapf(types.ErrManifestNotFound, "%q is a manifest list", ref)
}
return types.ImageManifest{}, errors.Errorf("%s is not a manifest", ref)
return types.ImageManifest{}, errors.Wrapf(types.ErrUnknownType, "%q does not exist", ref)
}

func fetchList(ctx context.Context, repo distribution.Repository, ref reference.Named) ([]types.ImageManifest, error) {
Expand All @@ -55,14 +55,16 @@ func fetchList(ctx context.Context, repo distribution.Repository, ref reference.
}

switch v := manifest.(type) {
case *schema2.DeserializedManifest, *ocischema.DeserializedManifest:
return nil, errors.Wrapf(types.ErrManifestNotFound, "%q is not a manifest list", ref)
case *manifestlist.DeserializedManifestList:
imageManifests, err := pullManifestList(ctx, ref, repo, *v)
if err != nil {
return nil, err
}
return imageManifests, nil
default:
return nil, errors.Errorf("unsupported manifest format: %v", v)
return nil, types.ErrUnknownType
}
}

Expand All @@ -74,7 +76,7 @@ func getManifest(ctx context.Context, repo distribution.Repository, ref referenc

dgst, opts, err := getManifestOptionsFromReference(ref)
if err != nil {
return nil, errors.Errorf("image manifest for %q does not exist", ref)
return nil, errors.Wrapf(types.ErrManifestNotFound, "%q does not exist", ref)
}
return manSvc.Get(ctx, dgst, opts...)
}
Expand Down Expand Up @@ -195,7 +197,7 @@ func pullManifestList(ctx context.Context, ref reference.Named, repo distributio
case *ocischema.DeserializedManifest:
imageManifest, err = pullManifestOCISchema(ctx, manifestRef, repo, *v)
default:
err = errors.Errorf("unsupported manifest type: %T", manifest)
err = types.ErrUnknownType
}
if err != nil {
return nil, err
Expand Down Expand Up @@ -287,7 +289,7 @@ func (c *client) iterateEndpoints(ctx context.Context, namedRef reference.Named,
return nil
}
}
return newNotFoundError(namedRef.String())
return errors.Wrapf(types.ErrManifestNotFound, "%q does not exist", namedRef)
}

// allEndpoints returns a list of endpoints ordered by priority (v2, https, v1).
Expand All @@ -310,18 +312,3 @@ func allEndpoints(namedRef reference.Named, insecure bool) ([]registry.APIEndpoi
logrus.Debugf("endpoints for %s: %v", namedRef, endpoints)
return endpoints, err
}

func newNotFoundError(ref string) *notFoundError {
return &notFoundError{err: errors.New("no such manifest: " + ref)}
}

type notFoundError struct {
err error
}

func (n *notFoundError) Error() string {
return n.err.Error()
}

// NotFound satisfies interface github.com/docker/docker/errdefs.ErrNotFound
func (n *notFoundError) NotFound() {}