From 38823ca68a51c98fe2fdcf19874e940235d6f663 Mon Sep 17 00:00:00 2001 From: Lokesh Mandvekar Date: Thu, 16 Sep 2021 14:45:36 -0400 Subject: [PATCH] Allow config authors to always use Prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. https://github.com/containers/image/pull/1191#discussion_r610622495 2. https://github.com/containers/image/pull/1191#discussion_r610621608 Co-authored-by: Miloslav Trmač Signed-off-by: Lokesh Mandvekar --- docs/containers-registries.conf.5.md | 7 ++-- pkg/sysregistriesv2/system_registries_v2.go | 33 +++++++++---------- .../system_registries_v2_test.go | 17 ++++++++-- .../testdata/find-registry.conf | 3 ++ 4 files changed, 39 insertions(+), 21 deletions(-) diff --git a/docs/containers-registries.conf.5.md b/docs/containers-registries.conf.5.md index a10c819bf8..ae0521f51e 100644 --- a/docs/containers-registries.conf.5.md +++ b/docs/containers-registries.conf.5.md @@ -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. @@ -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 diff --git a/pkg/sysregistriesv2/system_registries_v2.go b/pkg/sysregistriesv2/system_registries_v2.go index 4c1629f563..78f80d15c7 100644 --- a/pkg/sysregistriesv2/system_registries_v2.go +++ b/pkg/sysregistriesv2/system_registries_v2.go @@ -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:] @@ -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 diff --git a/pkg/sysregistriesv2/system_registries_v2_test.go b/pkg/sysregistriesv2/system_registries_v2_test.go index 9092ddba63..1009aa9256 100644 --- a/pkg/sysregistriesv2/system_registries_v2_test.go +++ b/pkg/sysregistriesv2/system_registries_v2_test.go @@ -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) @@ -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) } @@ -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"}, @@ -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"}, diff --git a/pkg/sysregistriesv2/testdata/find-registry.conf b/pkg/sysregistriesv2/testdata/find-registry.conf index 0abb7d7c7b..68cf8592f8 100644 --- a/pkg/sysregistriesv2/testdata/find-registry.conf +++ b/pkg/sysregistriesv2/testdata/find-registry.conf @@ -60,3 +60,6 @@ prefix="*.com" [[registry]] prefix="*.foobar.io" + +[[registry]] +prefix="empty-location.set-prefix.example.com"