Skip to content

Commit

Permalink
toml decoding: report unknown keys
Browse files Browse the repository at this point in the history
Despite the desire in #1284 to report unknown-key errors (e.g., for
typos), only log them on the debug level.  We have made rather bad
experiences with containers.conf which initially logged these errors
on the warning level which caused an immense noise for users.

The underlying problem is that not all tools are updated in sync,
especially for non-{CentOS,Fedora,RHEL} distributions.  Config files
changes for tools with an updated conf will hence cause noise for others
etc.

However, logging these error on the debug level will still improve user
experience and be especially helpful in analyzing users issues.

Fixes: #1284
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
  • Loading branch information
vrothberg committed Mar 25, 2022
1 parent 8c27ca2 commit 9eb950c
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 3 deletions.
6 changes: 5 additions & 1 deletion pkg/sysregistriesv2/shortnames.go
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/containers/storage/pkg/homedir"
"github.com/containers/storage/pkg/lockfile"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

// defaultShortNameMode is the default mode of registries.conf files if the
Expand Down Expand Up @@ -315,11 +316,14 @@ func (c *shortNameAliasCache) updateWithConfigurationFrom(updates *shortNameAlia
func loadShortNameAliasConf(confPath string) (*shortNameAliasConf, *shortNameAliasCache, error) {
conf := shortNameAliasConf{}

_, err := toml.DecodeFile(confPath, &conf)
meta, 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, nil, errors.Wrapf(err, "loading short-name aliases config file %q", confPath)
}
if keys := meta.Undecoded(); len(keys) > 0 {
logrus.Debugf("Failed to decode the keys %q from %q", keys, confPath)
}

// Even if we don’t always need the cache, doing so validates the machine-generated config. The
// file could still be corrupted by another process or user.
Expand Down
7 changes: 5 additions & 2 deletions pkg/sysregistriesv2/system_registries_v2.go
Expand Up @@ -877,9 +877,12 @@ func loadConfigFile(path string, forceV2 bool) (*parsedConfig, error) {

// Load the tomlConfig. Note that `DecodeFile` will overwrite set fields.
var combinedTOML tomlConfig
_, err := toml.DecodeFile(path, &combinedTOML)
meta, err := toml.DecodeFile(path, &combinedTOML)
if err != nil {
return nil, err
return nil, fmt.Errorf("decoding registries.conf: %w", err)
}
if keys := meta.Undecoded(); len(keys) > 0 {
logrus.Debugf("Failed to decode the keys %q from %q", keys, path)
}

if combinedTOML.V1RegistriesConf.Nonempty() {
Expand Down

0 comments on commit 9eb950c

Please sign in to comment.