Skip to content

Commit

Permalink
[WIP] Fixed pull against docker store
Browse files Browse the repository at this point in the history
We changed the default docker registry URL back when we were
adding content trust.  That URL is causing this failure.  The
URL has been changed back to the same value that is hardcoded
in the docker distribution code.

Also fixed an error interpretation in the fetcher code to match
Docker's interpretation.  We were returning image not found
when the actual error was access denied.  This led the debugging
of this issue down the wrong path and will eventually lead us
down the wrong path with future bugs.

Fixes vmware#8138
  • Loading branch information
Loc Nguyen committed Jul 16, 2018
1 parent e0dd53e commit 721e62e
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 8 deletions.
9 changes: 8 additions & 1 deletion lib/imagec/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ func tagOrDigest(r reference.Named, tag string) string {

// FetchImageManifest fetches the image manifest file
func FetchImageManifest(ctx context.Context, options Options, schemaVersion int, progressOutput progress.Output) (interface{}, string, error) {
defer trace.End(trace.Begin(options.Reference.String()))
defer trace.End(trace.Begin(fmt.Sprintf("url = %s, schema = %d", options.Reference.String(), schemaVersion)))

if schemaVersion != 1 && schemaVersion != 2 {
return nil, "", fmt.Errorf("Unknown schema version %d requested!", schemaVersion)
Expand Down Expand Up @@ -355,6 +355,13 @@ func FetchImageManifest(ctx context.Context, options Options, schemaVersion int,

manifestFileName, err := fetcher.Fetch(ctx, url, &reqHeaders, true, progressOutput)
if err != nil {
log.Debugf("Failed to fetch manifest: %s", err.Error())

switch err.(type) {
case urlfetcher.AccessDenied:
return nil, "", fmt.Errorf("pull access denied for %s, repository does not exist or may require 'docker login'", options.Reference.Name())

}
return nil, "", err
}

Expand Down
2 changes: 1 addition & 1 deletion lib/imagec/imagec.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ var (

const (
// DefaultDockerURL holds the URL of Docker registry
DefaultDockerURL = "registry.hub.docker.com"
DefaultDockerURL = "registry-1.docker.io"

// DefaultDestination specifies the default directory to use
DefaultDestination = "images"
Expand Down
4 changes: 2 additions & 2 deletions lib/imagec/imagec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -601,8 +601,8 @@ func TestFetchScenarios(t *testing.T) {
// valid token but image is missing we shouldn't retry
_, _, err = FetchImageManifest(ctx, ic.Options, 1, ic.progressOutput)
if err != nil {
// we should get a ImageNotFoundError
if _, imageErr := err.(urlfetcher.ImageNotFoundError); !imageErr {
// we should get a AccessDenied
if !strings.Contains(err.Error(), "pull access denied") {
t.Errorf(err.Error())
}
}
Expand Down
14 changes: 14 additions & 0 deletions pkg/fetcher/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,17 @@ type AuthTokenError struct {
func (e AuthTokenError) Error() string {
return fmt.Sprintf("Failed to fetch auth token from %s", e.TokenServer.Host)
}

// AccessDenied is returned when the resource requested
type AccessDenied struct {
Err error
res string
}

func (e AccessDenied) Error() string {
if e.res == "" {
return fmt.Sprintf("Access denied to requested resource")
}

return fmt.Sprintf("Access denied to %s", e.res)
}
8 changes: 4 additions & 4 deletions pkg/fetcher/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func (u *URLFetcher) Fetch(ctx context.Context, url *url.URL, reqHdrs *http.Head
}

switch err := err.(type) {
case DoNotRetry, TagNotFoundError, ImageNotFoundError:
case DoNotRetry, TagNotFoundError, ImageNotFoundError, AccessDenied:
log.Debugf("Error: %s", err.Error())
return "", err
}
Expand Down Expand Up @@ -280,10 +280,10 @@ func (u *URLFetcher) fetch(ctx context.Context, url *url.URL, reqHdrs *http.Head
if u.IsStatusUnauthorized() {
hdr := res.Header.Get("www-authenticate")

// check if image is non-existent (#757)
// Fix insufficient_scope return value.
// https://github.com/vmware/vic/blob/master/vendor/github.com/docker/distribution/registry/client/errors.go#L112
if strings.Contains(hdr, "error=\"insufficient_scope\"") {
err = fmt.Errorf("image not found")
return nil, nil, ImageNotFoundError{Err: err}
return nil, nil, AccessDenied{Err: err, res: url.String()}
} else if strings.Contains(hdr, "error=\"invalid_token\"") {
return nil, nil, fmt.Errorf("not authorized")
} else {
Expand Down

0 comments on commit 721e62e

Please sign in to comment.