From 928636ab8b69e878a9931f3326cf269ea87faafd Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 6 Oct 2020 13:28:58 +0200 Subject: [PATCH] sysregistriesv2: short-name aliasing The use of unqualified-search registries entails an ambiguity as it's unclear from which registry a given image, referenced by a short name, may be pulled from. There have been reports of squatting on some registries to trap users into pulling from a registry that may have pull precedence over the intended registry; all depending on the relative order of the entries in the `unqualified-search-registries` field in the `registries.conf`. Removing the feature of unqualified-search registries is not an option, as many users depend on it. The agreed on alternative is to introduce aliases. Aliases make short-name resolution explicit. Similar to bash aliases, the new `[aliases]` table (internally a `map[string]string`) has a left-hand name and a right-hand value: ```TOML [aliases] name="registry.com/namespace/name" ``` Consumers of containers/image can now use the new API to resolve aliases and add new ones. Signed-off-by: Valentin Rothberg --- go.sum | 5 + pkg/sysregistriesv2/shortnames.go | 316 ++++++++++++++++++ pkg/sysregistriesv2/shortnames_test.go | 276 +++++++++++++++ pkg/sysregistriesv2/system_registries_v2.go | 119 ++++++- .../system_registries_v2_test.go | 75 ++++- pkg/sysregistriesv2/testdata/aliases.conf | 7 + .../testdata/invalid-aliases.conf | 3 + .../testdata/invalid-short-name-mode.conf | 1 + .../testdata/registries.conf.d/config-1.conf | 7 +- .../testdata/registries.conf.d/config-2.conf | 10 +- .../testdata/registries.conf.d/config-3.conf | 0 .../registries.conf.d/config-3.ignore | 5 +- types/types.go | 26 ++ 13 files changed, 842 insertions(+), 8 deletions(-) create mode 100644 pkg/sysregistriesv2/shortnames.go create mode 100644 pkg/sysregistriesv2/shortnames_test.go create mode 100644 pkg/sysregistriesv2/testdata/aliases.conf create mode 100644 pkg/sysregistriesv2/testdata/invalid-aliases.conf create mode 100644 pkg/sysregistriesv2/testdata/invalid-short-name-mode.conf create mode 100644 pkg/sysregistriesv2/testdata/registries.conf.d/config-3.conf diff --git a/go.sum b/go.sum index 18a88575..8b5b5903 100644 --- a/go.sum +++ b/go.sum @@ -78,7 +78,9 @@ github.com/containers/storage v1.23.4 h1:1raHKGNs2C52tEq2ydHqZ+wu2u1d79BHMO6O5JO github.com/containers/storage v1.23.4/go.mod h1:KzpVgmUucelPYHq2YsseUTiTuucdVh3xfpPNmxmPZRU= github.com/containers/storage v1.23.5 h1:He9I6y1vRVXYoQg4v2Q9HFAcX4dI3V5MCCrjeBcjkCY= github.com/containers/storage v1.23.5/go.mod h1:ha26Q6ngehFNhf3AWoXldvAvwI4jFe3ETQAf/CeZPyM= +github.com/containers/storage v1.23.6 h1:3rcZ1KTNv8q7SkZ75gcrFGYqTeiuI04Zg7m9X1sCg/s= github.com/containers/storage v1.23.6/go.mod h1:haFs0HRowKwyzvWEx9EgI3WsL8XCSnBDb5f8P5CAxJY= +github.com/containers/storage v1.23.7 h1:43ImvG/npvQSZXRjaudVvKISIuZSfI6qvtSNQQSGO/A= github.com/containers/storage v1.23.7/go.mod h1:cUT2zHjtx+WlVri30obWmM2gpqpi8jfPsmIzP1TVpEI= github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4= github.com/coreos/go-systemd/v22 v22.0.0/go.mod h1:xO0FLkIi5MaZafQlIrOotqXZ90ih+1atmu1JpKERPPk= @@ -214,7 +216,9 @@ github.com/mistifyio/go-zfs v2.1.1+incompatible h1:gAMO1HM9xBRONLHHYnu5iFsOJUiJd github.com/mistifyio/go-zfs v2.1.1+incompatible/go.mod h1:8AuVvqP/mXw1px98n46wfvcGfQ4ci2FwoAjKYxuo3Z4= github.com/moby/sys/mountinfo v0.1.3 h1:KIrhRO14+AkwKvG/g2yIpNMOUVZ02xNhOw8KY1WsLOI= github.com/moby/sys/mountinfo v0.1.3/go.mod h1:w2t2Avltqx8vE7gX5l+QiBKxODu2TX0+Syr3h52Tw4o= +github.com/moby/sys/mountinfo v0.3.1 h1:R+C9GycEzoR3GdwQ7mANRhJORnVDJiRkf0JMY82MeI0= github.com/moby/sys/mountinfo v0.3.1/go.mod h1:rEr8tzG/lsIZHBtN/JjGG+LMYx9eXgW2JI+6q0qou+A= +github.com/moby/sys/mountinfo v0.4.0 h1:1KInV3Huv18akCu58V7lzNlt+jFmqlu1EaErnEHE/VM= github.com/moby/sys/mountinfo v0.4.0/go.mod h1:rEr8tzG/lsIZHBtN/JjGG+LMYx9eXgW2JI+6q0qou+A= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= @@ -432,6 +436,7 @@ golang.org/x/sys v0.0.0-20200728102440-3e129f6d46b1 h1:sIky/MyNRSHTrdxfsiUSS4WIA golang.org/x/sys v0.0.0-20200728102440-3e129f6d46b1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200810151505-1b9f1253b3ed h1:WBkVNH1zd9jg/dK4HCM4lNANnmd12EHC9z+LmcCG4ns= golang.org/x/sys v0.0.0-20200810151505-1b9f1253b3ed/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20200909081042-eff7692f9009 h1:W0lCpv29Hv0UaM1LXb9QlBHLNP8UFfcKjblhVCWftOM= golang.org/x/sys v0.0.0-20200909081042-eff7692f9009/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/pkg/sysregistriesv2/shortnames.go b/pkg/sysregistriesv2/shortnames.go new file mode 100644 index 00000000..23ceee4d --- /dev/null +++ b/pkg/sysregistriesv2/shortnames.go @@ -0,0 +1,316 @@ +package sysregistriesv2 + +import ( + "os" + "path/filepath" + "strings" + + "github.com/BurntSushi/toml" + "github.com/containers/image/v5/docker/reference" + "github.com/containers/image/v5/types" + "github.com/containers/storage/pkg/lockfile" + "github.com/docker/docker/pkg/homedir" + "github.com/pkg/errors" +) + +// defaultShortNameMode is the default mode of registries.conf files if the +// corresponding field is left empty. +const defaultShortNameMode = types.ShortNameModePermissive + +// userShortNamesFile is the user-specific config file to store aliases. +var userShortNamesFile = filepath.FromSlash("containers/short-name-aliases.conf") + +// shortNameAliasesConfPath returns the path to the machine-generated +// short-name-aliases.conf file. +func shortNameAliasesConfPath(ctx *types.SystemContext) (string, error) { + if ctx != nil && len(ctx.UserShortNameAliasConfPath) > 0 { + return ctx.UserShortNameAliasConfPath, nil + } + + configHome, err := homedir.GetConfigHome() + if err != nil { + return "", err + } + + return filepath.Join(configHome, userShortNamesFile), nil +} + +// alias combines the parsed value of an alias with the config file it has been +// specified in. The config file is crucial for an improved user experience +// such that users are able to resolve potential pull errors. +type alias struct { + // The parsed value of an alias. May be nil if set to "" in a config. + value reference.Named + // The config file the alias originates from. + configOrigin string +} + +// shortNameAliasConf is a subset of the `V2RegistriesConf` format. It's used in the +// software-maintained `userShortNamesFile`. +type shortNameAliasConf struct { + // A map for aliasing short names to their fully-qualified image + // reference counter parts. + // Note that Aliases is niled after being loaded from a file. + Aliases map[string]string `toml:"aliases"` + + // Note that an alias value may be nil iff it's set as an empty string + // in the config. + namedAliases map[string]alias +} + +// ResolveShortNameAlias performs an alias resolution of the specified name. +// The user-specific short-name-aliases.conf has precedence over aliases in the +// assembled registries.conf. It returns the possibly resolved alias or nil, a +// human-readable description of the config where the alias is specified, and +// an error. The origin of the config file is crucial for an improved user +// experience such that users are able to resolve potential pull errors. +// Almost all callers should use pkg/shortnames instead. +// +// Note that it’s the caller’s responsibility to pass only a repository +// (reference.IsNameOnly) as the short name. +func ResolveShortNameAlias(ctx *types.SystemContext, name string) (reference.Named, string, error) { + if err := validateShortName(name); err != nil { + return nil, "", err + } + confPath, lock, err := shortNameAliasesConfPathAndLock(ctx) + if err != nil { + return nil, "", err + } + + // Acquire the lock as a reader to allow for multiple routines in the + // same process space to read simultaneously. + lock.RLock() + defer lock.Unlock() + + aliasConf, err := loadShortNameAliasConf(confPath) + if err != nil { + return nil, "", err + } + + // First look up the short-name-aliases.conf. Note that a value may be + // nil iff it's set as an empty string in the config. + alias, resolved := aliasConf.namedAliases[name] + if resolved { + return alias.value, alias.configOrigin, nil + } + + config, err := getConfig(ctx) + if err != nil { + return nil, "", err + } + alias, resolved = config.namedAliases[name] + if resolved { + return alias.value, alias.configOrigin, nil + } + return nil, "", nil +} + +// editShortNameAlias loads the aliases.conf file and changes it. If value is +// set, it adds the name-value pair as a new alias. Otherwise, it will remove +// name from the config. +func editShortNameAlias(ctx *types.SystemContext, name string, value *string) error { + if err := validateShortName(name); err != nil { + return err + } + if value != nil { + if _, err := parseShortNameValue(*value); err != nil { + return err + } + } + + confPath, lock, err := shortNameAliasesConfPathAndLock(ctx) + if err != nil { + return err + } + + // Acquire the lock as a writer to prevent data corruption. + lock.Lock() + defer lock.Unlock() + + // Load the short-name-alias.conf, add the specified name-value pair, + // and write it back to the file. + conf, err := loadShortNameAliasConf(confPath) + if err != nil { + return err + } + + if value != nil { + conf.Aliases[name] = *value + } else { + // If the name does not exist, throw an error. + if _, exists := conf.Aliases[name]; !exists { + return errors.Errorf("short-name alias %q not found in %q: please check registries.conf files", name, confPath) + } + + delete(conf.Aliases, name) + } + + f, err := os.OpenFile(confPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600) + if err != nil { + return err + } + defer f.Close() + + encoder := toml.NewEncoder(f) + return encoder.Encode(conf) +} + +// AddShortNameAlias adds the specified name-value pair as a new alias to the +// user-specific aliases.conf. It may override an existing alias for `name`. +// +// Note that it’s the caller’s responsibility to pass only a repository +// (reference.IsNameOnly) as the short name. +func AddShortNameAlias(ctx *types.SystemContext, name string, value string) error { + return editShortNameAlias(ctx, name, &value) +} + +// RemoveShortNameAlias clears the alias for the specified name. It throws an +// error in case name does not exist in the machine-generated +// short-name-alias.conf. In such case, the alias must be specified in one of +// the registries.conf files, which is the users' responsibility. +// +// Note that it’s the caller’s responsibility to pass only a repository +// (reference.IsNameOnly) as the short name. +func RemoveShortNameAlias(ctx *types.SystemContext, name string) error { + return editShortNameAlias(ctx, name, nil) +} + +// parseShortNameValue parses the specified alias into a reference.Named. The alias is +// expected to not be tagged or carry a digest and *must* include a +// domain/registry. +// +// Note that the returned reference is always normalized. +func parseShortNameValue(alias string) (reference.Named, error) { + ref, err := reference.Parse(alias) + if err != nil { + return nil, errors.Wrapf(err, "error parsing alias %q", alias) + } + + if _, ok := ref.(reference.Digested); ok { + return nil, errors.Errorf("invalid alias %q: must not contain digest", alias) + } + + if _, ok := ref.(reference.Tagged); ok { + return nil, errors.Errorf("invalid alias %q: must not contain tag", alias) + } + + named, ok := ref.(reference.Named) + if !ok { + return nil, errors.Errorf("invalid alias %q: must contain registry and repository", alias) + } + + registry := reference.Domain(named) + if !(strings.ContainsAny(registry, ".:") || registry == "localhost") { + return nil, errors.Errorf("invalid alias %q: must contain registry and repository", alias) + } + + // A final parse to make sure that docker.io references are correctly + // normalized (e.g., docker.io/alpine to docker.io/library/alpine. + named, err = reference.ParseNormalizedNamed(alias) + return named, err +} + +// validateShortName parses the specified `name` of an alias (i.e., the left-hand +// side) and checks if it's a short name and does not include a tag or digest. +func validateShortName(name string) error { + repo, err := reference.Parse(name) + if err != nil { + return errors.Wrapf(err, "cannot parse short name: %q", name) + } + + if _, ok := repo.(reference.Digested); ok { + return errors.Errorf("invalid short name %q: must not contain digest", name) + } + + if _, ok := repo.(reference.Tagged); ok { + return errors.Errorf("invalid short name %q: must not contain tag", name) + } + + named, ok := repo.(reference.Named) + if !ok { + return errors.Errorf("invalid short name %q: no name", name) + } + + registry := reference.Domain(named) + if strings.ContainsAny(registry, ".:") || registry == "localhost" { + return errors.Errorf("invalid short name %q: must not contain registry", name) + } + return nil +} + +// parseAndValidate parses and validates all entries in conf.Aliases and stores +// the results in conf.namedAliases. +func (conf *shortNameAliasConf) parseAndValidate(path string) error { + if conf.Aliases == nil { + conf.Aliases = make(map[string]string) + } + if conf.namedAliases == nil { + conf.namedAliases = make(map[string]alias) + } + errs := []error{} + for name, value := range conf.Aliases { + if err := validateShortName(name); err != nil { + errs = append(errs, err) + } + + // Empty right-hand side values in config files allow to reset + // an alias in a previously loaded config. This way, drop-in + // config files from registries.conf.d can reset potentially + // malconfigured aliases. + if value == "" { + conf.namedAliases[name] = alias{nil, path} + continue + } + + named, err := parseShortNameValue(value) + if err != nil { + // We want to report *all* malformed entries to avoid a + // whack-a-mole for the user. + errs = append(errs, err) + } else { + conf.namedAliases[name] = alias{named, path} + } + } + var err error // nil if no errors + for _, e := range errs { + if err == nil { + err = e + } else { + err = errors.Wrapf(err, "%v\n", e) + } + } + return err +} + +func loadShortNameAliasConf(confPath string) (*shortNameAliasConf, error) { + conf := shortNameAliasConf{} + + _, err := toml.DecodeFile(confPath, &conf) + if err != nil && !os.IsNotExist(err) { + // It's okay if the config doesn't exist. Other errors are not. + return nil, errors.Wrapf(err, "error loading short-name aliases config file %q", confPath) + } + + // Better safe than sorry: validate the machine-generated config. The + // file could still be corrupted by another process or user. + if err := conf.parseAndValidate(confPath); err != nil { + return nil, errors.Wrapf(err, "error loading short-name aliases config file %q", confPath) + } + + return &conf, nil +} + +func shortNameAliasesConfPathAndLock(ctx *types.SystemContext) (string, lockfile.Locker, error) { + shortNameAliasesConfPath, err := shortNameAliasesConfPath(ctx) + if err != nil { + return "", nil, err + } + // Make sure the path to file exists. + if err := os.MkdirAll(filepath.Dir(shortNameAliasesConfPath), 0700); err != nil { + return "", nil, err + } + + lockPath := shortNameAliasesConfPath + ".lock" + locker, err := lockfile.GetLockfile(lockPath) + return shortNameAliasesConfPath, locker, err +} diff --git a/pkg/sysregistriesv2/shortnames_test.go b/pkg/sysregistriesv2/shortnames_test.go new file mode 100644 index 00000000..e6683ece --- /dev/null +++ b/pkg/sysregistriesv2/shortnames_test.go @@ -0,0 +1,276 @@ +package sysregistriesv2 + +import ( + "io/ioutil" + "os" + "testing" + + "github.com/containers/image/v5/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseShortNameValue(t *testing.T) { + tests := []struct { + input string + valid bool + }{ + // VALID INPUT + {"docker.io/library/fedora", true}, + {"localhost/fedora", true}, + {"localhost:5000/fedora", true}, + {"localhost:5000/namespace/fedora", true}, + // INVALID INPUT + {"docker.io/library/fedora:latest", false}, // tag + {"docker.io/library/fedora@sha256:b87dd5f837112a9e1e9882963a6406387597698268c0ad371b187151a5dfe6bf", false}, // digest + {"fedora", false}, // short name + {"fedora:latest", false}, // short name + tag + {"library/fedora", false}, // no registry + {"library/fedora:latest", false}, // no registry + tag + {"$$4455%%", false}, // gargabe + {"docker://foo", false}, // transports are not supported + {"docker-archive://foo", false}, // transports are not supported + {"", false}, // empty + } + + for _, test := range tests { + named, err := parseShortNameValue(test.input) + if test.valid { + require.NoError(t, err, "%q should be a valid alias", test.input) + assert.NotNil(t, named) + assert.Equal(t, test.input, named.String()) + } else { + require.Error(t, err, "%q should be an invalid alias", test.input) + assert.Nil(t, named) + } + } + + // Now make sure that docker.io references are normalized. + named, err := parseShortNameValue("docker.io/fedora") + require.NoError(t, err) + assert.NotNil(t, named) + assert.Equal(t, "docker.io/library/fedora", named.String()) + +} + +func TestValidateShortName(t *testing.T) { + tests := []struct { + input string + valid bool + }{ + // VALID INPUT + {"library/fedora", true}, + {"fedora", true}, + {"1234567489", true}, + // INVALID INPUT + {"docker.io/library/fedora:latest", false}, + {"docker.io/library/fedora@sha256:b87dd5f837112a9e1e9882963a6406387597698268c0ad371b187151a5dfe6bf", false}, // digest + {"fedora:latest", false}, + {"library/fedora:latest", false}, + {"$$4455%%", false}, + {"docker://foo", false}, + {"docker-archive://foo", false}, + {"", false}, + } + + for _, test := range tests { + err := validateShortName(test.input) + if test.valid { + require.NoError(t, err, "%q should be a valid alias", test.input) + } else { + require.Error(t, err, "%q should be an invalid alias", test.input) + } + } +} + +func TestResolveShortNameAlias(t *testing.T) { + tmp, err := ioutil.TempFile("", "aliases.conf") + require.NoError(t, err) + defer os.Remove(tmp.Name()) + + sys := &types.SystemContext{ + SystemRegistriesConfPath: "testdata/aliases.conf", + SystemRegistriesConfDirPath: "testdata/this-does-not-exist", + UserShortNameAliasConfPath: tmp.Name(), + } + + configCache = make(map[configWrapper]*V2RegistriesConf) + conf, err := TryUpdatingCache(sys) + require.NoError(t, err) + assert.Len(t, conf.namedAliases, 4) + assert.Len(t, conf.Aliases, 0) + + aliases := []struct { + name, value string + }{ + { + "docker", + "docker.io/library/foo", + }, + { + "quay/foo", + "quay.io/library/foo", + }, + { + "example", + "example.com/library/foo", + }, + } + + for _, alias := range aliases { + value, path, err := ResolveShortNameAlias(sys, alias.name) + require.NoError(t, err) + require.NotNil(t, value) + assert.Equal(t, alias.value, value.String()) + assert.Equal(t, "testdata/aliases.conf", path) + } + + // Non-existent alias. + value, path, err := ResolveShortNameAlias(sys, "idonotexist") + require.NoError(t, err) + assert.Nil(t, value) + assert.Equal(t, "", path) + + // Empty right-hand value (special case) -> does not resolve. + value, path, err = ResolveShortNameAlias(sys, "empty") + require.NoError(t, err) + assert.Nil(t, value) + assert.Equal(t, "testdata/aliases.conf", path) +} + +func TestAliasesWithDropInConfigs(t *testing.T) { + tmp, err := ioutil.TempFile("", "aliases.conf") + require.NoError(t, err) + defer os.Remove(tmp.Name()) + + sys := &types.SystemContext{ + SystemRegistriesConfPath: "testdata/aliases.conf", + SystemRegistriesConfDirPath: "testdata/registries.conf.d", + UserShortNameAliasConfPath: tmp.Name(), + } + + configCache = make(map[configWrapper]*V2RegistriesConf) + conf, err := TryUpdatingCache(sys) + require.NoError(t, err) + assert.Len(t, conf.namedAliases, 8) + assert.Len(t, conf.Aliases, 0) + + aliases := []struct { + name, value, config string + }{ + { + "docker", + "docker.io/library/config1", + "testdata/registries.conf.d/config-1.conf", + }, + { + "quay/foo", + "quay.io/library/foo", + "testdata/aliases.conf", + }, + { + "config1", + "config1.com/image", // from config1 + "testdata/registries.conf.d/config-1.conf", + }, + { + "barz", + "barz.com/config2", // from config1, overridden by config2 + "testdata/registries.conf.d/config-2.conf", + }, + { + "config2", + "config2.com/image", // from config2 + "testdata/registries.conf.d/config-2.conf", + }, + { + "added1", + "aliases.conf/added1", // from AddShortNameAlias + tmp.Name(), + }, + { + "added2", + "aliases.conf/added2", // from AddShortNameAlias + tmp.Name(), + }, + { + "added3", + "aliases.conf/added3", // from config2, overridden by AddShortNameAlias + tmp.Name(), + }, + } + + require.NoError(t, AddShortNameAlias(sys, "added1", "aliases.conf/added1")) + require.NoError(t, AddShortNameAlias(sys, "added2", "aliases.conf/added2")) + require.NoError(t, AddShortNameAlias(sys, "added3", "aliases.conf/added3")) + + for _, alias := range aliases { + value, path, err := ResolveShortNameAlias(sys, alias.name) + require.NoError(t, err) + require.NotNil(t, value, "%v", alias) + assert.Equal(t, alias.value, value.String()) + assert.Equal(t, alias.config, path) + } + + value, path, err := ResolveShortNameAlias(sys, "i/do/no/exist") + require.NoError(t, err) + assert.Nil(t, value) + assert.Equal(t, "", path) + + // Empty right-hand value (special case) -> does not resolve. + value, path, err = ResolveShortNameAlias(sys, "empty") // from aliases.conf, overridden by config2 + require.NoError(t, err) + assert.Nil(t, value) + assert.Equal(t, "testdata/aliases.conf", path) + + mode, err := GetShortNameMode(sys) + require.NoError(t, err) + assert.Equal(t, types.ShortNameModePermissive, mode) // from alias.conf, overridden by config2 + + // Now remove the aliases from the machine config. + require.NoError(t, RemoveShortNameAlias(sys, "added1")) + require.NoError(t, RemoveShortNameAlias(sys, "added2")) + require.NoError(t, RemoveShortNameAlias(sys, "added3")) + + // Make sure that 1 and 2 are gone. + for _, alias := range []string{"added1", "added2"} { + value, path, err := ResolveShortNameAlias(sys, alias) + require.NoError(t, err) + assert.Nil(t, value) + assert.Equal(t, "", path) + } + + // 3 is still present in config2 + value, path, err = ResolveShortNameAlias(sys, "added3") + require.NoError(t, err) + require.NotNil(t, value) + assert.Equal(t, "xxx.com/image", value.String()) + assert.Equal(t, "testdata/registries.conf.d/config-2.conf", path) + + require.Error(t, RemoveShortNameAlias(sys, "added3")) // we cannot remove it from config2 +} + +func TestInvalidAliases(t *testing.T) { + tmp, err := ioutil.TempFile("", "aliases.conf") + require.NoError(t, err) + defer os.Remove(tmp.Name()) + + sys := &types.SystemContext{ + SystemRegistriesConfPath: "testdata/invalid-aliases.conf", + SystemRegistriesConfDirPath: "testdata/this-does-not-exist", + UserShortNameAliasConfPath: tmp.Name(), + } + + configCache = make(map[configWrapper]*V2RegistriesConf) + _, err = TryUpdatingCache(sys) + require.Error(t, err) + + // We validate the alias value before loading existing configuration, + // so this tests the validation although the pre-existing configuration + // is invalid. + assert.Error(t, AddShortNameAlias(sys, "added1", "aliases")) + assert.Error(t, AddShortNameAlias(sys, "added2", "aliases.conf")) + assert.Error(t, AddShortNameAlias(sys, "added3", "")) + assert.Error(t, AddShortNameAlias(sys, "added3", " ")) + assert.Error(t, AddShortNameAlias(sys, "added3", "$$$")) +} diff --git a/pkg/sysregistriesv2/system_registries_v2.go b/pkg/sysregistriesv2/system_registries_v2.go index ea2b2157..bd966cf8 100644 --- a/pkg/sysregistriesv2/system_registries_v2.go +++ b/pkg/sysregistriesv2/system_registries_v2.go @@ -154,6 +154,28 @@ type V2RegistriesConf struct { Registries []Registry `toml:"registry"` // An array of host[:port] (not prefix!) entries to use for resolving unqualified image references UnqualifiedSearchRegistries []string `toml:"unqualified-search-registries"` + + // Absolut path to the configuration file that set the UnqualifiedSearchRegistries. + unqualifiedSearchRegistriesOrigin string + + // ShortNameMode defines how short-name resolution should be handled by + // _consumers_ of this package. Depending on the mode, the user should + // be prompted with a choice of using one of the unqualified-search + // registries when referring to a short name. + // + // Valid modes are: * "prompt": prompt if stdout is a TTY, otherwise + // use all unqualified-search registries * "enforcing": always prompt + // and error if stdout is not a TTY * "disabled": do not prompt and + // potentially use all unqualified-search registries + ShortNameMode string `toml:"short-name-mode"` + + // TODO: separate upper format from internal data below: + // https://github.com/containers/image/pull/1060#discussion_r503386541 + + // shortNameMode is stored _once_ when loading the config. + shortNameMode types.ShortNameMode + + shortNameAliasConf } // Nonempty returns true if config contains at least one configuration entry. @@ -254,7 +276,7 @@ var anchoredDomainRegexp = regexp.MustCompile("^" + reference.DomainRegexp.Strin // postProcess checks the consistency of all the configuration, looks for conflicts, // and normalizes the configuration (e.g., sets the Prefix to Location if not set). -func (config *V2RegistriesConf) postProcess() error { +func (config *V2RegistriesConf) postProcessRegistries() error { regMap := make(map[string][]*Registry) for i := range config.Registries { @@ -301,6 +323,7 @@ func (config *V2RegistriesConf) postProcess() error { msg := fmt.Sprintf("registry '%s' is defined multiple times with conflicting 'insecure' setting", reg.Location) return &InvalidRegistries{s: msg} } + if reg.Blocked != other.Blocked { msg := fmt.Sprintf("registry '%s' is defined multiple times with conflicting 'blocked' setting", reg.Location) return &InvalidRegistries{s: msg} @@ -385,6 +408,7 @@ func newConfigWrapper(ctx *types.SystemContext) configWrapper { } else { wrapper.userConfigDirPath = userRegistriesDirPath } + return wrapper } else if ctx != nil && ctx.RootForImplicitAbsolutePaths != "" { wrapper.configPath = filepath.Join(ctx.RootForImplicitAbsolutePaths, systemRegistriesConfPath) @@ -563,11 +587,44 @@ func GetRegistries(ctx *types.SystemContext) ([]Registry, error) { // UnqualifiedSearchRegistries returns a list of host[:port] entries to try // for unqualified image search, in the returned order) func UnqualifiedSearchRegistries(ctx *types.SystemContext) ([]string, error) { + registries, _, err := UnqualifiedSearchRegistriesWithOrigin(ctx) + return registries, err +} + +// UnqualifiedSearchRegistriesWithOrigin returns a list of host[:port] entries +// to try for unqualified image search, in the returned order. It also returns +// a human-readable description of where these entries are specified (e.g., a +// registries.conf file). +func UnqualifiedSearchRegistriesWithOrigin(ctx *types.SystemContext) ([]string, string, error) { config, err := getConfig(ctx) if err != nil { - return nil, err + return nil, "", err } - return config.UnqualifiedSearchRegistries, nil + return config.UnqualifiedSearchRegistries, config.unqualifiedSearchRegistriesOrigin, nil +} + +// parseShortNameMode translates the string into well-typed +// types.ShortNameMode. +func parseShortNameMode(mode string) (types.ShortNameMode, error) { + switch mode { + case "disabled": + return types.ShortNameModeDisabled, nil + case "enforcing": + return types.ShortNameModeEnforcing, nil + case "permissive": + return types.ShortNameModePermissive, nil + default: + return types.ShortNameModeInvalid, errors.Errorf("invalid short-name mode: %q", mode) + } +} + +// GetShortNameMode returns the configured types.ShortNameMode. +func GetShortNameMode(ctx *types.SystemContext) (types.ShortNameMode, error) { + config, err := getConfig(ctx) + if err != nil { + return -1, err + } + return config.shortNameMode, err } // refMatchesPrefix returns true iff ref, @@ -631,6 +688,9 @@ func FindRegistry(ctx *types.SystemContext, ref string) (*Registry, error) { // Note that specified fields in path will replace already set fields in the // tomlConfig. Only the [[registry]] tables are merged by prefix. func (c *tomlConfig) loadConfig(path string, forceV2 bool) error { + // TODO: the code became hard to brain-parse and maintain. The function + // should separate the loadding, converting and merging steps more + // clearly. logrus.Debugf("Loading registries configuration %q", path) // Save the registries before decoding the file where they could be lost. @@ -640,8 +700,24 @@ func (c *tomlConfig) loadConfig(path string, forceV2 bool) error { registryMap[c.Registries[i].Prefix] = c.Registries[i] } + prevAliases := c.namedAliases // store the aliases so they're not overridden + if prevAliases == nil { + prevAliases = make(map[string]alias) + } + + // Initialize the USR origin. + if len(c.unqualifiedSearchRegistriesOrigin) == 0 { + c.unqualifiedSearchRegistriesOrigin = path + } + + // Store the current USRs so we can determine _after_ loading if they + // changed. + prevUSRs := c.UnqualifiedSearchRegistries + c.UnqualifiedSearchRegistries = nil + // Load the tomlConfig. Note that `DecodeFile` will overwrite set fields. c.Registries = nil // important to clear the memory to prevent us from overlapping fields + c.Aliases = nil _, err := toml.DecodeFile(path, c) if err != nil { return err @@ -665,11 +741,29 @@ func (c *tomlConfig) loadConfig(path string, forceV2 bool) error { c.V2RegistriesConf = *v2 } + // Now check if the newly loaded config set the USRs. + if c.UnqualifiedSearchRegistries != nil { + // USRs set -> record it as the new origin. + c.unqualifiedSearchRegistriesOrigin = path + } else { + // USRs not set -> restore the previous USRs + c.UnqualifiedSearchRegistries = prevUSRs + } + // Post process registries, set the correct prefixes, sanity checks, etc. - if err := c.postProcess(); err != nil { + if err := c.postProcessRegistries(); err != nil { return err } + // Parse and validate short-name aliases. + if err := c.shortNameAliasConf.parseAndValidate(path); err != nil { + return errors.Wrap(err, "error validating short-name aliases") + } + // Nil conf.Aliases to make it available for garbage collection and + // reduce memory consumption. We're consulting conf.namedAliases for + // look ups. + c.Aliases = nil + // Merge the freshly loaded registries. for i := range c.Registries { registryMap[c.Registries[i].Prefix] = c.Registries[i] @@ -691,5 +785,22 @@ func (c *tomlConfig) loadConfig(path string, forceV2 bool) error { c.Registries = append(c.Registries, registryMap[prefix]) } + // Merge the alias maps. New configs override previous entries. + newAliases := c.namedAliases + c.namedAliases = prevAliases // point back to the previous map and override + for name, value := range newAliases { + c.namedAliases[name] = value + } + + // If set, parse & store the specified short-name mode. + if len(c.ShortNameMode) > 0 { + c.shortNameMode, err = parseShortNameMode(c.ShortNameMode) + if err != nil { + return err + } + } else { + c.shortNameMode = defaultShortNameMode + } + return nil } diff --git a/pkg/sysregistriesv2/system_registries_v2_test.go b/pkg/sysregistriesv2/system_registries_v2_test.go index 5e161a9b..446812d6 100644 --- a/pkg/sysregistriesv2/system_registries_v2_test.go +++ b/pkg/sysregistriesv2/system_registries_v2_test.go @@ -271,9 +271,10 @@ func TestFindUnqualifiedSearchRegistries(t *testing.T) { assert.Nil(t, err) assert.Equal(t, 4, len(registries)) - unqRegs, err := UnqualifiedSearchRegistries(sys) + unqRegs, origin, err := UnqualifiedSearchRegistriesWithOrigin(sys) assert.Nil(t, err) assert.Equal(t, []string{"registry-a.com", "registry-c.com", "registry-d.com"}, unqRegs) + assert.Equal(t, "testdata/unqualified-search.conf", origin) _, err = UnqualifiedSearchRegistries(&types.SystemContext{ SystemRegistriesConfPath: "testdata/invalid-search.conf", @@ -557,4 +558,76 @@ func TestRegistriesConfDirectory(t *testing.T) { reg, err := FindRegistry(ctx, "base.com/test:latest") require.NoError(t, err) assert.True(t, reg.Blocked) + + usrs, origin, err := UnqualifiedSearchRegistriesWithOrigin(ctx) + require.NoError(t, err) + assert.Equal(t, []string{"example-overwrite.com"}, usrs) + assert.Equal(t, "testdata/registries.conf.d/config-1.conf", origin) +} + +func TestParseShortNameMode(t *testing.T) { + tests := []struct { + input string + result types.ShortNameMode + mustFail bool + }{ + {"disabled", types.ShortNameModeDisabled, false}, + {"enforcing", types.ShortNameModeEnforcing, false}, + {"permissive", types.ShortNameModePermissive, false}, + {"", types.ShortNameModePermissive, true}, + {"xxx", -1, true}, + } + + for _, test := range tests { + shortName, err := parseShortNameMode(test.input) + if test.mustFail { + assert.Error(t, err) + continue + } + require.NoError(t, err) + assert.Equal(t, test.result, shortName) + } +} + +func TestGetShortNameMode(t *testing.T) { + tests := []struct { + path string + mode types.ShortNameMode + mustFail bool + }{ + { + "testdata/aliases.conf", + types.ShortNameModeEnforcing, + false, + }, + { + "testdata/registries.conf.d/config-2.conf", + types.ShortNameModePermissive, + false, + }, + { + "testdata/registries.conf.d/config-3.conf", + types.ShortNameModePermissive, // empty -> default to permissive + false, + }, + { + "testdata/invalid-short-name-mode.conf", + -1, + true, + }, + } + + for _, test := range tests { + sys := &types.SystemContext{ + SystemRegistriesConfPath: test.path, + SystemRegistriesConfDirPath: "testdata/this-does-not-exist", + } + mode, err := GetShortNameMode(sys) + if test.mustFail { + assert.Error(t, err) + continue + } + require.NoError(t, err) + assert.Equal(t, test.mode, mode, "%s", test.path) + } } diff --git a/pkg/sysregistriesv2/testdata/aliases.conf b/pkg/sysregistriesv2/testdata/aliases.conf new file mode 100644 index 00000000..cb05b275 --- /dev/null +++ b/pkg/sysregistriesv2/testdata/aliases.conf @@ -0,0 +1,7 @@ +short-name-mode="enforcing" + +[aliases] +docker="docker.io/library/foo" +"quay/foo"="quay.io/library/foo" +example="example.com/library/foo" +empty="" diff --git a/pkg/sysregistriesv2/testdata/invalid-aliases.conf b/pkg/sysregistriesv2/testdata/invalid-aliases.conf new file mode 100644 index 00000000..80530ca9 --- /dev/null +++ b/pkg/sysregistriesv2/testdata/invalid-aliases.conf @@ -0,0 +1,3 @@ +[aliases] +image1="quay.io/repo/image:1" +image2="image:1" diff --git a/pkg/sysregistriesv2/testdata/invalid-short-name-mode.conf b/pkg/sysregistriesv2/testdata/invalid-short-name-mode.conf new file mode 100644 index 00000000..acf2eb20 --- /dev/null +++ b/pkg/sysregistriesv2/testdata/invalid-short-name-mode.conf @@ -0,0 +1 @@ +short-name-mode="invalid" diff --git a/pkg/sysregistriesv2/testdata/registries.conf.d/config-1.conf b/pkg/sysregistriesv2/testdata/registries.conf.d/config-1.conf index e808cff7..f02e618a 100644 --- a/pkg/sysregistriesv2/testdata/registries.conf.d/config-1.conf +++ b/pkg/sysregistriesv2/testdata/registries.conf.d/config-1.conf @@ -1,4 +1,9 @@ unqualified-search-registries = ["example-overwrite.com"] [[registry]] -location = "1.com" \ No newline at end of file +location = "1.com" + +[aliases] +docker="docker.io/library/config1" +config1="config1.com/image" +barz="barz.com/image/config1" diff --git a/pkg/sysregistriesv2/testdata/registries.conf.d/config-2.conf b/pkg/sysregistriesv2/testdata/registries.conf.d/config-2.conf index ad011b9d..7ec82c75 100644 --- a/pkg/sysregistriesv2/testdata/registries.conf.d/config-2.conf +++ b/pkg/sysregistriesv2/testdata/registries.conf.d/config-2.conf @@ -1,6 +1,14 @@ +short-name-mode="permissive" + [[registry]] location = "2.com" [[registry]] location = "base.com" -blocked = true \ No newline at end of file +blocked = true + +[aliases] +config2="config2.com/image" +barz="barz.com/config2" +added3="xxx.com/image" +example="" diff --git a/pkg/sysregistriesv2/testdata/registries.conf.d/config-3.conf b/pkg/sysregistriesv2/testdata/registries.conf.d/config-3.conf new file mode 100644 index 00000000..e69de29b diff --git a/pkg/sysregistriesv2/testdata/registries.conf.d/config-3.ignore b/pkg/sysregistriesv2/testdata/registries.conf.d/config-3.ignore index 6486d8f9..65866fd7 100644 --- a/pkg/sysregistriesv2/testdata/registries.conf.d/config-3.ignore +++ b/pkg/sysregistriesv2/testdata/registries.conf.d/config-3.ignore @@ -1,4 +1,7 @@ unqualified-search-registries = ["ignore-example-overwrite.com"] [[registry]] -location = "ignore-me-because-i-have-a-wrong-suffix.com" \ No newline at end of file +location = "ignore-me-because-i-have-a-wrong-suffix.com" + +[aliases] +ignore="me because i have a wrong suffix" diff --git a/types/types.go b/types/types.go index 5a91f009..3972175d 100644 --- a/types/types.go +++ b/types/types.go @@ -486,6 +486,30 @@ func NewOptionalBool(b bool) OptionalBool { return o } +// ShortNameMode defines the mode of short-name resolution. +// +// The use of unqualified-search registries entails an ambiguity as it's +// unclear from which registry a given image, referenced by a short name, may +// be pulled from. +// +// The ShortNameMode type defines how short names should resolve. +type ShortNameMode int + +const ( + ShortNameModeInvalid ShortNameMode = iota + // Use all configured unqualified-search registries without prompting + // the user. + ShortNameModeDisabled + // If stdout is a TTY, prompt the user to select a configured + // unqualified-search registry. Otherwise, use all configured + // unqualified-search registries. + ShortNameModePermissive + // Always prompt the user to select a configured unqualified-serach + // registry. Throw an error if stdout is not a TTY as prompting + // isn't possible. + ShortNameModeEnforcing +) + // SystemContext allows parameterizing access to implicitly-accessed resources, // like configuration files in /etc and users' login state in their home directory. // Various components can share the same field only if their semantics is exactly @@ -509,6 +533,8 @@ type SystemContext struct { SystemRegistriesConfPath string // Path to the system-wide registries configuration directory SystemRegistriesConfDirPath string + // Path to the user-specific short-names configuration file + UserShortNameAliasConfPath string // 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;