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.

Sign up for GitHub

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

shortnames: mechanism to enforce resolving to Docker Hub #1415

Merged
merged 1 commit into from
Nov 29, 2021
Merged

shortnames: mechanism to enforce resolving to Docker Hub #1415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
59 changes: 37 additions & 22 deletions pkg/shortnames/shortnames.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ type Resolved struct {
}

func (r *Resolved) addCandidate(named reference.Named) {
named = reference.TagNameOnly(named) // Make sure to add ":latest" if needed
r.PullCandidates = append(r.PullCandidates, PullCandidate{named, false, r})
}

Expand All @@ -138,6 +139,8 @@ const (
rationaleUSR
// Resolved value has been selected by the user (via the prompt).
rationaleUserSelection
// Resolved value has been enforced to use Docker Hub (via SystemContext).
rationaleEnforcedDockerHub
mtrmac marked this conversation as resolved.
Show resolved Hide resolved
)

// Description returns a human-readable description about the resolution
Expand All @@ -152,6 +155,8 @@ func (r *Resolved) Description() string {
return fmt.Sprintf("Resolved %q as an alias (%s)", r.userInput, r.originDescription)
case rationaleUSR:
return fmt.Sprintf("Resolving %q using unqualified-search registries (%s)", r.userInput, r.originDescription)
case rationaleEnforcedDockerHub:
return fmt.Sprintf("Resolving %q to docker.io (%s)", r.userInput, r.originDescription)
case rationaleUserSelection, rationaleNone:
fallthrough
default:
Expand Down Expand Up @@ -265,8 +270,20 @@ func Resolve(ctx *types.SystemContext, name string) (*Resolved, error) {
return nil, err
}
if !isShort { // no short name
named := reference.TagNameOnly(shortRef) // Make sure to add ":latest" if needed
resolved.addCandidate(shortRef)
return resolved, nil
}

// Resolve to docker.io only if enforced by the caller (e.g., Podman's
// Docker-compatible REST API).
if ctx != nil && ctx.PodmanOnlyShortNamesIgnoreRegistriesConfAndForceDockerHub {
named, err := reference.ParseNormalizedNamed(name)
if err != nil {
return nil, errors.Wrapf(err, "cannot normalize input: %q", name)
}
resolved.addCandidate(named)
resolved.rationale = rationaleEnforcedDockerHub
resolved.originDescription = "enforced by caller"
return resolved, nil
}

Expand Down Expand Up @@ -295,9 +312,6 @@ func Resolve(ctx *types.SystemContext, name string) (*Resolved, error) {
return nil, err
}
}
// Make sure to add ":latest" if needed
namedAlias = reference.TagNameOnly(namedAlias)

resolved.addCandidate(namedAlias)
resolved.rationale = rationaleAlias
resolved.originDescription = aliasOriginDescription
Expand Down Expand Up @@ -325,9 +339,6 @@ func Resolve(ctx *types.SystemContext, name string) (*Resolved, error) {
if err != nil {
return nil, errors.Wrapf(err, "creating reference with unqualified-search registry %q", reg)
}
// Make sure to add ":latest" if needed
named = reference.TagNameOnly(named)

resolved.addCandidate(named)
}

Expand Down Expand Up @@ -412,6 +423,23 @@ func ResolveLocally(ctx *types.SystemContext, name string) ([]reference.Named, e

var candidates []reference.Named

// Complete the candidates with the specified registries.
completeCandidates := func(registries []string) ([]reference.Named, error) {
for _, reg := range registries {
named, err := reference.ParseNormalizedNamed(fmt.Sprintf("%s/%s", reg, name))
if err != nil {
return nil, errors.Wrapf(err, "creating reference with unqualified-search registry %q", reg)
}
named = reference.TagNameOnly(named) // Make sure to add ":latest" if needed
candidates = append(candidates, named)
}
return candidates, nil
}

if ctx != nil && ctx.PodmanOnlyShortNamesIgnoreRegistriesConfAndForceDockerHub {
return completeCandidates([]string{"docker.io"})
vrothberg marked this conversation as resolved.
Show resolved Hide resolved
}

// Strip off the tag to normalize the short name for looking it up in
// the config files.
isTagged, isDigested, shortNameRepo, tag, digest := splitUserInput(shortRef)
Expand All @@ -434,9 +462,7 @@ func ResolveLocally(ctx *types.SystemContext, name string) ([]reference.Named, e
return nil, err
}
}
// Make sure to add ":latest" if needed
namedAlias = reference.TagNameOnly(namedAlias)

namedAlias = reference.TagNameOnly(namedAlias) // Make sure to add ":latest" if needed
candidates = append(candidates, namedAlias)
}

Expand All @@ -447,16 +473,5 @@ func ResolveLocally(ctx *types.SystemContext, name string) ([]reference.Named, e
}

// Note that "localhost" has precedence over the unqualified-search registries.
vrothberg marked this conversation as resolved.
Show resolved Hide resolved
for _, reg := range append([]string{"localhost"}, unqualifiedSearchRegistries...) {
named, err := reference.ParseNormalizedNamed(fmt.Sprintf("%s/%s", reg, name))
if err != nil {
return nil, errors.Wrapf(err, "creating reference with unqualified-search registry %q", reg)
}
// Make sure to add ":latest" if needed
named = reference.TagNameOnly(named)

candidates = append(candidates, named)
}

return candidates, nil
return completeCandidates(append([]string{"localhost"}, unqualifiedSearchRegistries...))
}
221 changes: 154 additions & 67 deletions pkg/shortnames/shortnames_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,48 +105,117 @@ func TestResolve(t *testing.T) {
UserShortNameAliasConfPath: tmp.Name(),
}

sysResolveToDockerHub := &types.SystemContext{
SystemRegistriesConfPath: "testdata/aliases.conf",
SystemRegistriesConfDirPath: "testdata/this-does-not-exist",
UserShortNameAliasConfPath: tmp.Name(),
PodmanOnlyShortNamesIgnoreRegistriesConfAndForceDockerHub: true,
}

_, err = sysregistriesv2.TryUpdatingCache(sys)
require.NoError(t, err)

digest := "@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a"

tests := []struct {
name, value string
input, value, dockerHubValue string
}{
{"docker", "docker.io/library/foo:latest"},
{"docker:tag", "docker.io/library/foo:tag"},
{
"docker@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a",
"docker.io/library/foo@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a",
{ // alias
"docker",
"docker.io/library/foo:latest",
"docker.io/library/docker:latest",
},
{"quay/foo", "quay.io/library/foo:latest"},
{"quay/foo:tag", "quay.io/library/foo:tag"},
{
"quay/foo@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a",
"quay.io/library/foo@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a",
{ // alias tagged
"docker:tag",
"docker.io/library/foo:tag",
"docker.io/library/docker:tag",
},
{"example", "example.com/library/foo:latest"},
{"example:tag", "example.com/library/foo:tag"},
{
"example@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a",
"example.com/library/foo@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a",
{ // alias digested
"docker" + digest,
"docker.io/library/foo" + digest,
"docker.io/library/docker" + digest,
},
{ // alias with repo
"quay/foo",
"quay.io/library/foo:latest",
"docker.io/quay/foo:latest",
},
{ // alias with repo tagged
"quay/foo:tag",
"quay.io/library/foo:tag",
"docker.io/quay/foo:tag",
},
{ // alias with repo digested
"quay/foo" + digest,
"quay.io/library/foo" + digest,
"docker.io/quay/foo" + digest,
},
{ // alias
"example",
"example.com/library/foo:latest",
"docker.io/library/example:latest",
},
{ // alias with tag
"example:tag",
"example.com/library/foo:tag",
"docker.io/library/example:tag",
},
{ // alias digested
"example" + digest,
"example.com/library/foo" + digest,
"docker.io/library/example" + digest,
},
{ // FQN
"registry.com/repo/image",
"registry.com/repo/image:latest",
"registry.com/repo/image:latest",
},
{ // FQN tagged
"registry.com/repo/image:tag",
"registry.com/repo/image:tag",
"registry.com/repo/image:tag",
},
{ // FQN digested
"registry.com/repo/image" + digest,
"registry.com/repo/image" + digest,
"registry.com/repo/image" + digest,
},
}

// All of them should resolve correctly.
for _, test := range tests {
resolved, err := Resolve(sys, test.name)
resolved, err := Resolve(sys, test.input)
require.NoError(t, err, "%v", test)
require.NotNil(t, resolved)
require.Len(t, resolved.PullCandidates, 1)
assert.Equal(t, test.value, resolved.PullCandidates[0].Value.String())
assert.False(t, resolved.PullCandidates[0].record)
}

// Non-existent should return an error as no search registries are
// configured in the config.
// Now another run with enforcing resolution to Docker Hub.
for _, test := range tests {
resolved, err := Resolve(sysResolveToDockerHub, test.input)
require.NoError(t, err, "%v", test)
require.NotNil(t, resolved)
require.Len(t, resolved.PullCandidates, 1)
assert.Equal(t, test.dockerHubValue, resolved.PullCandidates[0].Value.String())
assert.False(t, resolved.PullCandidates[0].record)
}

vrothberg marked this conversation as resolved.
Show resolved Hide resolved
// Non-existent alias should return an error as no search registries
// are configured in the config.
resolved, err := Resolve(sys, "doesnotexist")
require.Error(t, err)
require.Nil(t, resolved)

// It'll work though when enforcing resolving to Docker Hub.
resolved, err = Resolve(sysResolveToDockerHub, "doesnotexist")
require.NoError(t, err)
require.NotNil(t, resolved)
require.Len(t, resolved.PullCandidates, 1)
assert.Equal(t, "docker.io/library/doesnotexist:latest", resolved.PullCandidates[0].Value.String())
assert.False(t, resolved.PullCandidates[0].record)

// An empty name is not valid.
resolved, err = Resolve(sys, "")
require.Error(t, err)
Expand All @@ -156,14 +225,6 @@ func TestResolve(t *testing.T) {
resolved, err = Resolve(sys, "Invalid#$")
require.Error(t, err)
require.Nil(t, resolved)

// Fully-qualified input will be returned as is.
resolved, err = Resolve(sys, "quay.io/repo/fedora")
require.NoError(t, err)
require.NotNil(t, resolved)
require.Len(t, resolved.PullCandidates, 1)
assert.Equal(t, "quay.io/repo/fedora:latest", resolved.PullCandidates[0].Value.String())
assert.False(t, resolved.PullCandidates[0].record)
}

func toNamed(t *testing.T, input string, trim bool) reference.Named {
Expand Down Expand Up @@ -470,48 +531,74 @@ func TestResolveLocally(t *testing.T) {
SystemRegistriesConfDirPath: "testdata/this-does-not-exist",
UserShortNameAliasConfPath: tmp.Name(),
}
sysResolveToDockerHub := &types.SystemContext{
mtrmac marked this conversation as resolved.
Show resolved Hide resolved
SystemRegistriesConfPath: "testdata/two-reg.conf",
SystemRegistriesConfDirPath: "testdata/this-does-not-exist",
UserShortNameAliasConfPath: tmp.Name(),
PodmanOnlyShortNamesIgnoreRegistriesConfAndForceDockerHub: true,
}

aliases, err := ResolveLocally(sys, "repo/image") // alias match
require.NoError(t, err)
require.Len(t, aliases, 4) // alias + localhost + two regs
assert.Equal(t, "quay.io/repo/image:latest", aliases[0].String()) // alias
assert.Equal(t, "localhost/repo/image:latest", aliases[1].String()) // localhost
assert.Equal(t, "quay.io/repo/image:latest", aliases[2].String()) // registry 0
assert.Equal(t, "registry.com/repo/image:latest", aliases[3].String()) // registry 0

aliases, err = ResolveLocally(sys, "foo") // no alias match
require.NoError(t, err)
require.Len(t, aliases, 3) // localhost + two regs
assert.Equal(t, "localhost/foo:latest", aliases[0].String()) // localhost
assert.Equal(t, "quay.io/foo:latest", aliases[1].String()) // registry 0
assert.Equal(t, "registry.com/foo:latest", aliases[2].String()) // registry 0

aliases, err = ResolveLocally(sys, "foo:tag") // no alias match tagged
require.NoError(t, err)
require.Len(t, aliases, 3) // localhost + two regs
assert.Equal(t, "localhost/foo:tag", aliases[0].String()) // localhost
assert.Equal(t, "quay.io/foo:tag", aliases[1].String()) // registry 0
assert.Equal(t, "registry.com/foo:tag", aliases[2].String()) // registry 0

aliases, err = ResolveLocally(sys, "foo@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a") // no alias match digested
require.NoError(t, err)
require.Len(t, aliases, 3) // localhost + two regs
assert.Equal(t, "localhost/foo@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a", aliases[0].String()) // localhost
assert.Equal(t, "quay.io/foo@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a", aliases[1].String()) // registry 0
assert.Equal(t, "registry.com/foo@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a", aliases[2].String()) // registry 0
digest := "@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a"

aliases, err = ResolveLocally(sys, "localhost/foo") // localhost
require.NoError(t, err)
require.Len(t, aliases, 1)
assert.Equal(t, "localhost/foo:latest", aliases[0].String())
tests := []struct {
input string
expectedSys []string
expectedSysResolveToDockerHub string
}{
{ // alias match
"repo/image",
[]string{"quay.io/repo/image:latest", "localhost/repo/image:latest", "quay.io/repo/image:latest", "registry.com/repo/image:latest"},
"docker.io/repo/image:latest",
},
{ // no alias match
"foo",
[]string{"localhost/foo:latest", "quay.io/foo:latest", "registry.com/foo:latest"},
"docker.io/library/foo:latest",
},
{ // no alias match tagged
"foo:tag",
[]string{"localhost/foo:tag", "quay.io/foo:tag", "registry.com/foo:tag"},
"docker.io/library/foo:tag",
},
{ // no alias match digested
"foo" + digest,
[]string{"localhost/foo" + digest, "quay.io/foo" + digest, "registry.com/foo" + digest},
"docker.io/library/foo" + digest,
},
{ // localhost
"localhost/foo",
[]string{"localhost/foo:latest"},
"localhost/foo:latest",
},
{ // localhost tagged
"localhost/foo:tag",
[]string{"localhost/foo:tag"},
"localhost/foo:tag",
},
{ // localhost digested
"localhost/foo" + digest,
[]string{"localhost/foo" + digest},
"localhost/foo" + digest,
},
{ // non-localhost FQN + digest
"registry.com/repo/image" + digest,
[]string{"registry.com/repo/image" + digest},
"registry.com/repo/image" + digest,
},
}

aliases, err = ResolveLocally(sys, "localhost/foo:tag") // localhost + tag
require.NoError(t, err)
require.Len(t, aliases, 1)
assert.Equal(t, "localhost/foo:tag", aliases[0].String())
for _, test := range tests {
aliases, err := ResolveLocally(sys, test.input)
require.NoError(t, err)
require.Len(t, aliases, len(test.expectedSys))
for i := range aliases {
assert.Equal(t, test.expectedSys[i], aliases[i].String())
}

aliases, err = ResolveLocally(sys, "localhost/foo@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a") // localhost + digest
require.NoError(t, err)
require.Len(t, aliases, 1)
assert.Equal(t, "localhost/foo@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a", aliases[0].String())
// Another run enforcing resolving to Docker Hub.
aliases, err = ResolveLocally(sysResolveToDockerHub, test.input)
require.NoError(t, err)
require.Len(t, aliases, 1)
assert.Equal(t, test.expectedSysResolveToDockerHub, aliases[0].String())
}
}
5 changes: 5 additions & 0 deletions types/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,11 @@ type SystemContext struct {
UserShortNameAliasConfPath string
// If set, short-name resolution in pkg/shortnames must follow the specified mode
ShortNameMode *ShortNameMode
// If set, short names will resolve in pkg/shortnames to docker.io only, and unqualified-search registries and
// short-name aliases in registries.conf are ignored. Note that this field is only intended to help enforce
// resolving to Docker Hub in the Docker-compatible REST API of Podman; it should never be used outside this
// specific context.
PodmanOnlyShortNamesIgnoreRegistriesConfAndForceDockerHub bool
// If not "", overrides the default path for the authentication file, but only new format files
AuthFilePath string
// if not "", overrides the default path for the authentication file, but with the legacy format;
Expand Down