Skip to content

Commit

Permalink
shortnames: mechanism to enforce resolving to Docker Hub
Browse files Browse the repository at this point in the history 
Podman's Docker-compatible REST API is in need of a mechanism to enforce
resolving to Docker Hub only.  Yet there is the desire for the rest of
the stack to continue honoring the system's registries.conf.

We recently added a new field to containers.conf [1] which allows for
opting out from enforcing Docker Hub for Podman's compat API but we
still lack a way of enforcement when resolving short names; which
ultimately is *the* place to do that.

This change does the necessary plumbing.  The compat REST handlers will
set the new field in the `types.SystemContext` and pass that down to
libimage and buildah.

[1] containers/common@e698b8c

Context: containers/podman/issues/12320
Signed-off-by: Valentin Rothberg <[email protected]>
  • Loading branch information
@vrothberg @nlewo
vrothberg authored and nlewo committed Dec 26, 2021
1 parent af582a9 commit d703342
Show file tree
Hide file tree
Showing 3 changed files with 196 additions and 89 deletions.
59 changes: 37 additions & 22 deletions pkg/shortnames/shortnames.go
View file
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
)

// 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"})
}

// 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.
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
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)
}

// 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{
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
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

0 comments on commit d703342

Please sign in to comment.