Skip to content

Commit

Permalink
Allow config authors to always use Prefix
Browse files Browse the repository at this point in the history
Previously an unset Location was only allowed for wildcarded Prefixes.
This commit will allow configs with emtpy location or empty prefix, so
long as the other is valid.

Remove wildcard prefix check for empty location in rewriteReference.

Fixes:
1. #1191 (comment)
2. #1191 (comment)

Co-authored-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Lokesh Mandvekar <lsm5@fedoraproject.org>
  • Loading branch information
lsm5 and mtrmac committed Sep 16, 2021
1 parent 1bf5c4a commit 38823ca
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 21 deletions.
7 changes: 5 additions & 2 deletions docs/containers-registries.conf.5.md
Expand Up @@ -82,8 +82,7 @@ location = "internal-registry-for-example.net/bar"
requests for the image `example.com/foo/myimage:latest` will actually work with the
`internal-registry-for-example.net/bar/myimage:latest` image.

With a `prefix` containing a wildcard in the format: "*.example.com" for subdomain matching,
the location can be empty. In such a case,
With any valid `prefix`, the location can be emtpy. In such a case,
prefix matching will occur, but no reference rewrite will occur. The
original requested image string will be used as-is. But other settings like
`insecure` / `blocked` / `mirrors` will be applied to matching images.
Expand All @@ -92,6 +91,10 @@ Example: Given
```
prefix = "*.example.com"
```
OR
```
prefix = "blah.example.com"
```
requests for the image `blah.example.com/foo/myimage:latest` will be used
as-is. But other settings like insecure/blocked/mirrors will be applied to matching images

Expand Down
33 changes: 16 additions & 17 deletions pkg/sysregistriesv2/system_registries_v2.go
Expand Up @@ -75,14 +75,7 @@ func (e *Endpoint) rewriteReference(ref reference.Named, prefix string) (referen
}
// In the case of an empty `location` field, simply return the original
// input ref as-is.
//
// FIXME: already validated in postProcessRegistries, so check can probably
// be dropped.
// https://github.com/containers/image/pull/1191#discussion_r610621608
if e.Location == "" {
if prefix[:2] != "*." {
return nil, fmt.Errorf("invalid prefix '%v' for empty location, should be in the format: *.example.com", prefix)
}
return ref, nil
}
newNamedRef = e.Location + refString[prefixLen:]
Expand Down Expand Up @@ -357,21 +350,27 @@ func (config *V2RegistriesConf) postProcessRegistries() error {
return err
}

if reg.Prefix == "" {
if reg.Location == "" {
return &InvalidRegistries{s: "invalid condition: both location and prefix are unset"}
}
reg.Prefix = reg.Location
} else {
// Prefix and Location cannot both be empty.
// Either one at least must be set.
if reg.Prefix != "" {
reg.Prefix, err = parseLocation(reg.Prefix)
if err != nil {
return err
}
// FIXME: allow config authors to always use Prefix.
// https://github.com/containers/image/pull/1191#discussion_r610622495
if reg.Prefix[:2] != "*." && reg.Location == "" {
return &InvalidRegistries{s: "invalid condition: location is unset and prefix is not in the format: *.example.com"}
if reg.Location != "" {
reg.Location, err = parseLocation(reg.Location)
if err != nil {
return err
}
}
} else if reg.Location != "" {
reg.Prefix, err = parseLocation(reg.Location)
if err != nil {
return err
}
reg.Location = reg.Prefix
} else {
return &InvalidRegistries{s: "invalid condition: both location and prefix are unset"}
}

// make sure mirrors are valid
Expand Down
17 changes: 15 additions & 2 deletions pkg/sysregistriesv2/system_registries_v2_test.go
Expand Up @@ -291,7 +291,7 @@ func TestFindRegistry(t *testing.T) {

registries, err := GetRegistries(sys)
assert.Nil(t, err)
assert.Equal(t, 19, len(registries))
assert.Equal(t, 20, len(registries))

reg, err := FindRegistry(sys, "simple-prefix.com/foo/bar:latest")
assert.Nil(t, err)
Expand Down Expand Up @@ -383,6 +383,12 @@ func TestFindRegistry(t *testing.T) {
assert.Equal(t, "empty-prefix.com", reg.Prefix)
assert.Equal(t, "empty-prefix.com", reg.Location)

reg, err = FindRegistry(sys, "empty-location.set-prefix.example.com/namespace/repo")
assert.Nil(t, err)
assert.NotNil(t, reg)
assert.NotNil(t, "empty-location.set-prefix.example.com", reg.Prefix)
assert.Equal(t, "", reg.Location)

_, err = FindRegistry(&types.SystemContext{SystemRegistriesConfPath: "testdata/this-does-not-exist.conf"}, "example.com")
assert.Error(t, err)
}
Expand Down Expand Up @@ -579,6 +585,14 @@ func TestRewriteReferenceSuccess(t *testing.T) {
{"sub.example.io/library/prefix/image", "*.example.io", "example.com", "example.com/library/prefix/image"},
{"another.sub.example.io:5000/library/prefix/image:latest", "*.sub.example.io", "example.com", "example.com:5000/library/prefix/image:latest"},
{"foo.bar.io/ns1/ns2/ns3/ns4", "*.bar.io", "omg.bbq.com/roflmao", "omg.bbq.com/roflmao/ns1/ns2/ns3/ns4"},
// Empty location with non-wildcard prefix examples. Essentially, no
// rewrite occurs and original reference is used as-is.
{"registry.com/foo", "registry.com", "", "registry.com/foo"},
{"abc.internal.registry.com/foo:bar", "abc.internal.registry.com", "", "abc.internal.registry.com/foo:bar"},
{"abc.internal.registry.com/foo/bar:baz", "abc.internal.registry.com", "", "abc.internal.registry.com/foo/bar:baz"},
{"alien.vs.predator.foobar.io:5000/foo@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "alien.vs.predator.foobar.io", "",
"alien.vs.predator.foobar.io:5000/foo@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"},
{"alien.vs.predator.foobar.io:5000/omg:bbq", "alien.vs.predator.foobar.io", "", "alien.vs.predator.foobar.io:5000/omg:bbq"},
// Empty location with wildcard prefix examples. Essentially, no
// rewrite occurs and original reference is used as-is.
{"abc.internal.registry.com/foo:bar", "*.internal.registry.com", "", "abc.internal.registry.com/foo:bar"},
Expand All @@ -602,7 +616,6 @@ func TestRewriteReferenceFailedDuringParseNamed(t *testing.T) {
{"example.com/foo/image:latest", "example.com/foo", "example.com/path/"},
{"example.com/foo@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
"example.com/foo", "example.com"},
{"example.com:5000/image:latest", "example.com", ""},
{"example.com:5000/image:latest", "example.com", "example.com:5000"},
// Malformed prefix
{"example.com/foo/image:latest", "example.com//foo", "example.com/path"},
Expand Down
3 changes: 3 additions & 0 deletions pkg/sysregistriesv2/testdata/find-registry.conf
Expand Up @@ -60,3 +60,6 @@ prefix="*.com"

[[registry]]
prefix="*.foobar.io"

[[registry]]
prefix="empty-location.set-prefix.example.com"

0 comments on commit 38823ca

Please sign in to comment.