Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mounting doesn't work when specifying a domain for dockerhub #689

Closed
jonjohnsonjr opened this issue Mar 9, 2020 · 1 comment · Fixed by #690
Closed

Mounting doesn't work when specifying a domain for dockerhub #689

jonjohnsonjr opened this issue Mar 9, 2020 · 1 comment · Fixed by #690

Comments

@jonjohnsonjr
Copy link
Collaborator

jonjohnsonjr commented Mar 9, 2020

# broken 
crane cp hello-world index.docker.io/jonjohnson/hello-world

# broken
crane cp hello-world docker.io/jonjohnson/hello-world

# works
crane cp hello-world jonjohnson/hello-world

I'm suspicious of these bits of code:

func scopesForUploadingImage(repo name.Repository, layers []v1.Layer) []string {
// use a map as set to remove duplicates scope strings
scopeSet := map[string]struct{}{}
for _, l := range layers {
if ml, ok := l.(*MountableLayer); ok {
// we will add push scope for ref.Context() after the loop.
// for now we ask pull scope for references of the same registry
if ml.Reference.Context() != repo && ml.Reference.Context().Registry == repo.Registry {
scopeSet[ml.Reference.Scope(transport.PullScope)] = struct{}{}
}
}
}
scopes := make([]string, 0)
// Push scope should be the first element because a few registries just look at the first scope to determine access.
scopes = append(scopes, repo.Scope(transport.PushScope))
for scope := range scopeSet {
scopes = append(scopes, scope)
}
return scopes
}

// http.Client handles redirects at a layer above the http.RoundTripper
// abstraction, so to avoid forwarding Authorization headers to places
// we are redirected, only set it when the authorization header matches
// the registry with which we are interacting.
// In case of redirect http.Client can use an empty Host, check URL too.
if matchesHost(bt.registry, in, bt.scheme) {
hdr := fmt.Sprintf("Bearer %s", bt.bearer.RegistryToken)
in.Header.Set("Authorization", hdr)
}

Looking at the token I get back from dockerhub, it looks like we're missing pull scopes for the source repo:

"access":[{"type":"repository","name":"jonjohnson/hello-world","actions":["pull","push"]}]
@jonjohnsonjr
Copy link
Collaborator Author

So this fixes things:

diff --git a/pkg/v1/remote/write.go b/pkg/v1/remote/write.go
index 7b431c0..13f6873 100644
--- a/pkg/v1/remote/write.go
+++ b/pkg/v1/remote/write.go
@@ -449,7 +449,7 @@ func scopesForUploadingImage(repo name.Repository, layers []v1.Layer) []string {
                if ml, ok := l.(*MountableLayer); ok {
                        // we will add push scope for ref.Context() after the loop.
                        // for now we ask pull scope for references of the same registry
-                       if ml.Reference.Context() != repo && ml.Reference.Context().Registry == repo.Registry {
+                       if ml.Reference.Context().String() != repo.String() && ml.Reference.Context().Registry.String() == repo.Registry.String() {
                                scopeSet[ml.Reference.Scope(transport.PullScope)] = struct{}{}
                        }
                }

but I'm not actually sure how we're getting to this state.

Printing some nonsense out...

diff --git a/pkg/v1/remote/write.go b/pkg/v1/remote/write.go
index 7b431c0..089449c 100644
--- a/pkg/v1/remote/write.go
+++ b/pkg/v1/remote/write.go
@@ -449,6 +449,8 @@ func scopesForUploadingImage(repo name.Repository, layers []v1.Layer) []string {
                if ml, ok := l.(*MountableLayer); ok {
                        // we will add push scope for ref.Context() after the loop.
                        // for now we ask pull scope for references of the same registry
+                       logs.Debug.Printf("MOUNTABLE LAYER: %#v", ml.Reference.Context().Registry)
+                       logs.Debug.Printf("TARGET REPO: %#v", repo.Registry)
                        if ml.Reference.Context() != repo && ml.Reference.Context().Registry == repo.Registry {
                                scopeSet[ml.Reference.Scope(transport.PullScope)] = struct{}{}
                        }

I see:

2020/03/09 17:16:19 MOUNTABLE LAYER: name.Registry{insecure:false, registry:""}
2020/03/09 17:16:19 TARGET REPO: name.Registry{insecure:false, registry:"index.docker.io"}                                                                                                                                                         

So for some reason, the layer registry is an empty string, but the target repo gets the default registry.

Looks like this defaulting is deferred to an accessor, so comparing the struct values doesn't work:

// RegistryStr returns the registry component of the Registry.
func (r Registry) RegistryStr() string {
if r.registry != "" {
return r.registry
}
return DefaultRegistry
}

I think we should clean that up but let's just do string comparisons first?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant