From 0642df250f9d696ff73734408766a42d0ec64181 Mon Sep 17 00:00:00 2001 From: Qi Wang Date: Thu, 18 Nov 2021 22:25:39 -0500 Subject: [PATCH] Add pull-from-mirror for adding per-mirror level restrictions Close: https://github.com/containers/image/issues/1407 Add pull-from-mirror: all, digest-only, tag-only for adding per-mirror level restrictions to image pull through mirrors. The `mirror-by-digest-only` for primary is still allowed configuring, and it is honored for compatibility Signed-off-by: Qi Wang --- docs/containers-registries.conf.5.md | 18 ++- pkg/sysregistriesv2/system_registries_v2.go | 62 ++++++++-- .../system_registries_v2_test.go | 116 ++++++++++++++++++ .../testdata/invalid-config-level-mirror.conf | 11 ++ .../testdata/invalid-conflict-mirror.conf | 12 ++ .../testdata/invalid-value-mirror.conf | 11 ++ .../pull-sources-mirror-reference.conf | 86 +++++++++++++ 7 files changed, 302 insertions(+), 14 deletions(-) create mode 100644 pkg/sysregistriesv2/testdata/invalid-config-level-mirror.conf create mode 100644 pkg/sysregistriesv2/testdata/invalid-conflict-mirror.conf create mode 100644 pkg/sysregistriesv2/testdata/invalid-value-mirror.conf create mode 100644 pkg/sysregistriesv2/testdata/pull-sources-mirror-reference.conf diff --git a/docs/containers-registries.conf.5.md b/docs/containers-registries.conf.5.md index cc23e38b6..75d639c0e 100644 --- a/docs/containers-registries.conf.5.md +++ b/docs/containers-registries.conf.5.md @@ -106,20 +106,26 @@ contacted and contains the image will be used (and if none of the mirrors contai the primary location specified by the `registry.location` field, or using the unmodified user-specified reference, is tried last). -Each TOML table in the `mirror` array can contain the following fields, with the same semantics -as if specified in the `[[registry]]` TOML table directly: -- `location` -- `insecure` +Each TOML table in the `mirror` array can contain the following fields: +- `location`： same semantics +as specified in the `[[registry]]` TOML table +- `insecure`： same semantics +as specified in the `[[registry]]` TOML table +- `pull-from-mirror`: `all`, `digest-only` or `tag-only`. If "digest-only"， mirrors will only be used for digest pulls. Pulling images by tag can potentially yield different images, depending on which endpoint we pull from. Restricting mirrors to pulls by digest avoids that issue. If "tag-only", mirrors will only be used for tag pulls. For a more up-to-date and expensive mirror that it is less likely to be out of sync if tags move, it should not be unnecessarily used for digest references. Default is "all" (or left empty), mirrors will be used for both digest pulls and tag pulls unless the mirror-by-digest-only is set for the primary registry. +Note that this per-mirror setting is allowed only when `mirror-by-digest-only` is not configured for the primary registry. `mirror-by-digest-only` : `true` or `false`. If `true`, mirrors will only be used during pulling if the image reference includes a digest. +Note that if all mirrors are configured to be digest-only, images referenced by a tag will only use the primary +registry. +If all mirrors are configured to be tag-only, images referenced by a digest will only use the primary +registry. + Referencing an image by digest ensures that the same is always used (whereas referencing an image by a tag may cause different registries to return different images if the tag mapping is out of sync). -Note that if this is `true`, images referenced by a tag will only use the primary -registry, failing if that registry is not accessible. *Note*: Redirection and mirrors are currently processed only when reading images, not when pushing to a registry; that may change in the future. diff --git a/pkg/sysregistriesv2/system_registries_v2.go b/pkg/sysregistriesv2/system_registries_v2.go index dc2dcf32b..c5df241b7 100644 --- a/pkg/sysregistriesv2/system_registries_v2.go +++ b/pkg/sysregistriesv2/system_registries_v2.go @@ -43,6 +43,16 @@ const builtinRegistriesConfDirPath = "/etc/containers/registries.conf.d" // helper. const AuthenticationFileHelper = "containers-auth.json" +const ( + // configuration values for "pull-from-mirror" + // mirrors will be used for both digest pulls and tag pulls + MirrorAll = "all" + // mirrors will only be used for digest pulls + MirrorByDigestOnly = "digest-only" + // mirrors will only be used for tag pulls + MirrorByTagOnly = "tag-only" +) + // Endpoint describes a remote location of a registry. type Endpoint struct { // The endpoint's remote location. Can be empty iff Prefix contains @@ -53,6 +63,18 @@ type Endpoint struct { // If true, certs verification will be skipped and HTTP (non-TLS) // connections will be allowed. Insecure bool `toml:"insecure,omitempty"` + // PullFromMirror is used for adding restrictions to image pull through the mirror. + // Set to "all", "digest-only", or "tag-only". + // If "digest-only"， mirrors will only be used for digest pulls. Pulling images by + // tag can potentially yield different images, depending on which endpoint + // we pull from. Restricting mirrors to pulls by digest avoids that issue. + // If "tag-only", mirrors will only be used for tag pulls. For a more up-to-date and expensive mirror + // that it is less likely to be out of sync if tags move, it should not be unnecessarily + // used for digest references. + // Default is "all" (or left empty), mirrors will be used for both digest pulls and tag pulls unless the mirror-by-digest-only is set for the primary registry. + // This can only be set in a registry's Mirror field, not in the registry's primary Endpoint. + // This per-mirror setting is allowed only when mirror-by-digest-only is not configured for the primary registry. + PullFromMirror string `toml:"pull-from-mirror,omitempty"` } // userRegistriesFile is the path to the per user registry configuration file. @@ -115,7 +137,7 @@ type Registry struct { Blocked bool `toml:"blocked,omitempty"` // If true, mirrors will only be used for digest pulls. Pulling images by // tag can potentially yield different images, depending on which endpoint - // we pull from. Forcing digest-pulls for mirrors avoids that issue. + // we pull from. Restricting mirrors to pulls by digest avoids that issue. MirrorByDigestOnly bool `toml:"mirror-by-digest-only,omitempty"` } @@ -130,17 +152,29 @@ type PullSource struct { // reference. func (r *Registry) PullSourcesFromReference(ref reference.Named) ([]PullSource, error) { var endpoints []Endpoint - + _, isDigested := ref.(reference.Canonical) if r.MirrorByDigestOnly { - // Only use mirrors when the reference is a digest one. - if _, isDigested := ref.(reference.Canonical); isDigested { - endpoints = append(r.Mirrors, r.Endpoint) - } else { - endpoints = []Endpoint{r.Endpoint} + // Only use mirrors when the reference is a digested one. + if isDigested { + endpoints = append(endpoints, r.Mirrors...) } } else { - endpoints = append(r.Mirrors, r.Endpoint) + for _, mirror := range r.Mirrors { + // skip the mirror if per mirror setting exists but reference does not match the restriction + switch mirror.PullFromMirror { + case MirrorByDigestOnly: + if !isDigested { + continue + } + case MirrorByTagOnly: + if isDigested { + continue + } + } + endpoints = append(endpoints, mirror) + } } + endpoints = append(endpoints, r.Endpoint) sources := []PullSource{} for _, ep := range endpoints { @@ -374,6 +408,10 @@ func (config *V2RegistriesConf) postProcessRegistries() error { } } + // validate the mirror usage settings does not apply to primary registry + if reg.PullFromMirror != "" { + return fmt.Errorf("pull-from-mirror must not be set for a non-mirror registry %q", reg.Prefix) + } // make sure mirrors are valid for _, mir := range reg.Mirrors { mir.Location, err = parseLocation(mir.Location) @@ -387,6 +425,14 @@ func (config *V2RegistriesConf) postProcessRegistries() error { if mir.Location == "" { return &InvalidRegistries{s: "invalid condition: mirror location is unset"} } + + if reg.MirrorByDigestOnly && mir.PullFromMirror != "" { + return &InvalidRegistries{s: fmt.Sprintf("cannot set mirror usage mirror-by-digest-only for the registry (%q) and pull-from-mirror for per-mirror (%q) at the same time", reg.Prefix, mir.Location)} + } + if mir.PullFromMirror != "" && mir.PullFromMirror != MirrorAll && + mir.PullFromMirror != MirrorByDigestOnly && mir.PullFromMirror != MirrorByTagOnly { + return &InvalidRegistries{s: fmt.Sprintf("unsupported pull-from-mirror value %q for mirror %q", mir.PullFromMirror, mir.Location)} + } } if reg.Location == "" { regMap[reg.Prefix] = append(regMap[reg.Prefix], reg) diff --git a/pkg/sysregistriesv2/system_registries_v2_test.go b/pkg/sysregistriesv2/system_registries_v2_test.go index e93db41ee..11d249dee 100644 --- a/pkg/sysregistriesv2/system_registries_v2_test.go +++ b/pkg/sysregistriesv2/system_registries_v2_test.go @@ -671,6 +671,122 @@ func TestPullSourcesFromReference(t *testing.T) { assert.Equal(t, 1, len(pullSources)) } +func TestPullSourcesMirrorFromReference(t *testing.T) { + sys := &types.SystemContext{ + SystemRegistriesConfPath: "testdata/pull-sources-mirror-reference.conf", + SystemRegistriesConfDirPath: "testdata/this-does-not-exist", + } + registries, err := GetRegistries(sys) + require.NoError(t, err) + assert.Equal(t, 7, len(registries)) + + digest := "@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + tag := ":aaa" + for _, tc := range []struct { + registry string + digestSources []string + tagSources []string + }{ + // Registry A has mirrors allow any kind of pull + { + "registry-a.com/foo/image", + []string{"mirror-1.registry-a.com", "mirror-2.registry-a.com", "registry-a.com/bar"}, + []string{"mirror-1.registry-a.com", "mirror-2.registry-a.com", "registry-a.com/bar"}, + }, + // Registry B has mirrors allow digests pull only + { + "registry-b.com/foo/image", + []string{"mirror-1.registry-b.com", "mirror-2.registry-b.com", "registry-b.com/bar"}, + []string{"registry-b.com/bar"}, + }, + // Registry C has a mirror allows digest pull only and a mirror allows any kind of pull + { + "registry-c.com/foo/image", + []string{"mirror-1.registry-c.com", "mirror-2.registry-c.com", "registry-c.com/bar"}, + []string{"mirror-1.registry-c.com", "registry-c.com/bar"}, + }, + // Registry D set digest-only for registry level, allows only digest pulls + // Registry D has no digest-only set for mirrors table + { + "registry-d.com/foo/image", + []string{"mirror-1.registry-d.com", "mirror-2.registry-d.com", "registry-d.com/bar"}, + []string{"registry-d.com/bar"}, + }, + // Registry E has mirrors only allows tag pull + { + "registry-e.com/foo/image", + []string{"registry-e.com/bar"}, + []string{"mirror-1.registry-e.com", "mirror-2.registry-e.com", "registry-e.com/bar"}, + }, + // Registry F has one tag only mirror does not allow digest pull + { + "registry-f.com/foo/image", + []string{"mirror-1.registry-f.com", "registry-f.com/bar"}, + []string{"mirror-1.registry-f.com", "mirror-2.registry-f.com", "registry-f.com/bar"}, + }, + // Registry G has one digest-only pull and one tag only pull + { + "registry-g.com/foo/image", + []string{"mirror-1.registry-g.com", "mirror-3.registry-g.com", "mirror-4.registry-g.com", "registry-g.com/bar"}, + []string{"mirror-2.registry-g.com", "mirror-3.registry-g.com", "mirror-4.registry-g.com", "registry-g.com/bar"}, + }, + } { + // Digest + digestedRef := toNamedRef(t, tc.registry+digest) + registry, err := FindRegistry(sys, digestedRef.Name()) + require.NoError(t, err) + require.NotNil(t, registry) + pullSource, err := registry.PullSourcesFromReference(digestedRef) + require.NoError(t, err) + for i, s := range tc.digestSources { + assert.Equal(t, s, pullSource[i].Endpoint.Location) + } + // Tag + taggedRef := toNamedRef(t, tc.registry+tag) + registry, err = FindRegistry(sys, taggedRef.Name()) + require.NoError(t, err) + require.NotNil(t, registry) + pullSource, err = registry.PullSourcesFromReference(taggedRef) + require.NoError(t, err) + for i, s := range tc.tagSources { + assert.Equal(t, s, pullSource[i].Endpoint.Location) + } + } +} + +func TestInvalidMirrorConfig(t *testing.T) { + for _, tc := range []struct { + sys *types.SystemContext + expectErr string + }{ + { + sys: &types.SystemContext{ + SystemRegistriesConfPath: "testdata/invalid-config-level-mirror.conf", + SystemRegistriesConfDirPath: "testdata/this-does-not-exist", + }, + expectErr: fmt.Sprintf("pull-from-mirror must not be set for a non-mirror registry %q", "registry-a.com/foo"), + }, + { + sys: &types.SystemContext{ + SystemRegistriesConfPath: "testdata/invalid-conflict-mirror.conf", + SystemRegistriesConfDirPath: "testdata/this-does-not-exist", + }, + expectErr: fmt.Sprintf("cannot set mirror usage mirror-by-digest-only for the registry (%q) and pull-from-mirror for per-mirror (%q) at the same time", "registry-a.com/foo", "mirror-1.registry-a.com"), + }, + { + sys: &types.SystemContext{ + SystemRegistriesConfPath: "testdata/invalid-value-mirror.conf", + SystemRegistriesConfDirPath: "testdata/this-does-not-exist", + }, + expectErr: fmt.Sprintf("unsupported pull-from-mirror value %q for mirror %q", "notvalid", "mirror-1.registry-a.com"), + }, + } { + _, err := GetRegistries(tc.sys) + assert.ErrorContains(t, err, tc.expectErr) + } + +} + func TestTryUpdatingCache(t *testing.T) { ctx := &types.SystemContext{ SystemRegistriesConfPath: "testdata/try-update-cache-valid.conf", diff --git a/pkg/sysregistriesv2/testdata/invalid-config-level-mirror.conf b/pkg/sysregistriesv2/testdata/invalid-config-level-mirror.conf new file mode 100644 index 000000000..cdcde7e3b --- /dev/null +++ b/pkg/sysregistriesv2/testdata/invalid-config-level-mirror.conf @@ -0,0 +1,11 @@ +[[registry]] +prefix = "registry-a.com/foo" +location = "registry-a.com/bar" +pull-from-mirror = "digest-only" + +[[registry.mirror]] +location = "mirror-1.registry-a.com" + +[[registry.mirror]] +location = "mirror-2.registry-a.com" +insecure = true \ No newline at end of file diff --git a/pkg/sysregistriesv2/testdata/invalid-conflict-mirror.conf b/pkg/sysregistriesv2/testdata/invalid-conflict-mirror.conf new file mode 100644 index 000000000..f710c1350 --- /dev/null +++ b/pkg/sysregistriesv2/testdata/invalid-conflict-mirror.conf @@ -0,0 +1,12 @@ +[[registry]] +prefix = "registry-a.com/foo" +location = "registry-a.com/bar" +mirror-by-digest-only = true + +[[registry.mirror]] +pull-from-mirror = "digest-only" +location = "mirror-1.registry-a.com" + +[[registry.mirror]] +location = "mirror-2.registry-a.com" +insecure = true \ No newline at end of file diff --git a/pkg/sysregistriesv2/testdata/invalid-value-mirror.conf b/pkg/sysregistriesv2/testdata/invalid-value-mirror.conf new file mode 100644 index 000000000..bb0797bd3 --- /dev/null +++ b/pkg/sysregistriesv2/testdata/invalid-value-mirror.conf @@ -0,0 +1,11 @@ +[[registry]] +prefix = "registry-a.com/foo" +location = "registry-a.com/bar" + +[[registry.mirror]] +pull-from-mirror = "notvalid" +location = "mirror-1.registry-a.com" + +[[registry.mirror]] +location = "mirror-2.registry-a.com" +insecure = true \ No newline at end of file diff --git a/pkg/sysregistriesv2/testdata/pull-sources-mirror-reference.conf b/pkg/sysregistriesv2/testdata/pull-sources-mirror-reference.conf new file mode 100644 index 000000000..50d51c289 --- /dev/null +++ b/pkg/sysregistriesv2/testdata/pull-sources-mirror-reference.conf @@ -0,0 +1,86 @@ +[[registry]] +prefix = "registry-a.com/foo" +location = "registry-a.com/bar" + +[[registry.mirror]] +location = "mirror-1.registry-a.com" + +[[registry.mirror]] +location = "mirror-2.registry-a.com" +insecure = true + +[[registry]] +prefix = "registry-b.com/foo" +location = "registry-b.com/bar" + +[[registry.mirror]] +pull-from-mirror = "digest-only" +location = "mirror-1.registry-b.com" + +[[registry.mirror]] +pull-from-mirror = "digest-only" +location = "mirror-2.registry-b.com" + +[[registry]] +prefix = "registry-c.com/foo" +location = "registry-c.com/bar" + +[[registry.mirror]] +location = "mirror-1.registry-c.com" + +[[registry.mirror]] +pull-from-mirror = "digest-only" +location = "mirror-2.registry-c.com" + +[[registry]] +prefix = "registry-d.com/foo" +location = "registry-d.com/bar" +mirror-by-digest-only = true + +[[registry.mirror]] +location = "mirror-1.registry-d.com" + +[[registry.mirror]] +location = "mirror-2.registry-d.com" + +[[registry]] +prefix = "registry-e.com/foo" +location = "registry-e.com/bar" + +[[registry.mirror]] +pull-from-mirror = "tag-only" +location = "mirror-1.registry-e.com" + +[[registry.mirror]] +pull-from-mirror = "tag-only" +location = "mirror-2.registry-e.com" + +[[registry]] +prefix = "registry-f.com/foo" +location = "registry-f.com/bar" + +[[registry.mirror]] +location = "mirror-1.registry-f.com" + +[[registry.mirror]] +pull-from-mirror = "tag-only" +location = "mirror-2.registry-f.com" + +[[registry]] +prefix = "registry-g.com/foo" +location = "registry-g.com/bar" + +[[registry.mirror]] +pull-from-mirror = "digest-only" +location = "mirror-1.registry-g.com" + +[[registry.mirror]] +pull-from-mirror = "tag-only" +location = "mirror-2.registry-g.com" + +[[registry.mirror]] +location = "mirror-3.registry-g.com" + +[[registry.mirror]] +pull-from-mirror = "all" +location = "mirror-4.registry-g.com"