From eea9df4454c518aede893b60638908d97c18c36e Mon Sep 17 00:00:00 2001 From: Qi Wang Date: Thu, 18 Nov 2021 22:25:39 -0500 Subject: [PATCH] set digest-only for each mirror Close: https://github.com/containers/image/issues/1407 Add the `digest-only` field to each mirror table, so it will be configured for each mirror instead of for primary registry. For the same primary registry, it can support both digest only pull and tag allowed pull. 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 | 22 +++++-- pkg/sysregistriesv2/system_registries_v2.go | 30 ++++++--- .../system_registries_v2_test.go | 65 +++++++++++++++++++ .../pull-sources-mirror-reference.conf | 44 +++++++++++++ 4 files changed, 147 insertions(+), 14 deletions(-) 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 928387b69a..3bb356bd87 100644 --- a/docs/containers-registries.conf.5.md +++ b/docs/containers-registries.conf.5.md @@ -104,20 +104,30 @@ 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 +- `digest-only`： `true` or `false`. If `true` an individual mirror will only be used during pulling if the image reference includes a digest. +Note that digest-only will be overridden if mirror-by-digest is set for the entire 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, failing if that registry is not accessible. + +`digest-only`: `true` or `false`. +: if `true` an individual mirror will only be used during pulling if the image reference includes a digest. +Note that digest-only will be overridden if mirror-by-digest is set for the entire registry. +Also note that if all mirrors in the `mirror` array have `digest-only = true`, images referenced by a tag will only use the primary +registry, failing if that registry is not accessible. + 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 c8a603c4ef..9828c0d31c 100644 --- a/pkg/sysregistriesv2/system_registries_v2.go +++ b/pkg/sysregistriesv2/system_registries_v2.go @@ -53,6 +53,12 @@ type Endpoint struct { // If true, certs verification will be skipped and HTTP (non-TLS) // connections will be allowed. Insecure bool `toml:"insecure,omitempty"` + // If true, the mirror 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. + // This can only be set in a registry's Mirror field, not in the registry's primary Endpoint. + // If mirror-by-digest-only is set to true for the primary registry, this per-mirror setting is ignored. + DigestOnly bool `toml:"digest-only,omitempty"` } // userRegistriesFile is the path to the per user registry configuration file. @@ -115,7 +121,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 for mirrors avoids that issue. MirrorByDigestOnly bool `toml:"mirror-by-digest-only,omitempty"` } @@ -130,17 +136,21 @@ 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) + // check digest-only for each mirror if non mirror-by-digest-only set, since mirror-by-digest-only is honored + for _, mirror := range r.Mirrors { + if !mirror.DigestOnly || isDigested { + endpoints = append(endpoints, mirror) + } + } } + endpoints = append(endpoints, r.Endpoint) sources := []PullSource{} for _, ep := range endpoints { @@ -374,6 +384,10 @@ func (config *V2RegistriesConf) postProcessRegistries() error { } } + // validate the mirror usage settings does not apply to primary registry + if reg.DigestOnly { + return fmt.Errorf("digest-only must not be set for a non-mirror registry %s", reg.Prefix) + } // make sure mirrors are valid for _, mir := range reg.Mirrors { mir.Location, err = parseLocation(mir.Location) diff --git a/pkg/sysregistriesv2/system_registries_v2_test.go b/pkg/sysregistriesv2/system_registries_v2_test.go index 0c3f141b0e..b967750014 100644 --- a/pkg/sysregistriesv2/system_registries_v2_test.go +++ b/pkg/sysregistriesv2/system_registries_v2_test.go @@ -675,6 +675,71 @@ 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, 4, 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 mirror-by-digest-only, honor this configuration for registry + // Regsitry 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"}, + }, + } { + // Digest + digestedRef := toNamedRef(t, tc.registry+digest) + registry, err := FindRegistry(sys, digestedRef.Name()) + require.NoError(t, err) + assert.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) + assert.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 TestTryUpdatingCache(t *testing.T) { ctx := &types.SystemContext{ SystemRegistriesConfPath: "testdata/try-update-cache-valid.conf", 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 0000000000..3682cb2921 --- /dev/null +++ b/pkg/sysregistriesv2/testdata/pull-sources-mirror-reference.conf @@ -0,0 +1,44 @@ +[[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]] +digest-only = true +location = "mirror-1.registry-b.com" + +[[registry.mirror]] +digest-only = true +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]] +digest-only = true +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"