containers
/
automation-tests
mirror of https://github.com/containers/automation-tests.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

pkg/config: remove unnecessary stat on default paths 

The current code has a small race it first stats the file and if it
exists it tries to read the file. Between this it is possible that the
file was removed and thus cause a fatal error when reading the config.
The better way is to simply read the file and ignore the ENOENT error
instead where we want this behavior. This avoids the need for the extra
stat syscalls. For CONTAINERS_CONF and modules we still need the hard
error if the file does not exists so we have to keep it there.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
This commit is contained in:
Paul Holzinger 2024-01-05 14:43:05 +01:00
parent 745268b117
commit 21d3f3431f
3 changed files with 17 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
common/pkg/config/config.go
View File

@ -1041,14 +1041,8 @@ func ReadCustomConfig() (*Config, error) {
return nil, err
}
newConfig := &Config{}
if _, err := os.Stat(path); err == nil {
if err := readConfigFromFile(path, newConfig); err != nil {
return nil, err
}
} else {
if !errors.Is(err, os.ErrNotExist) {
return nil, err
}
if err := readConfigFromFile(path, newConfig, true); err != nil {
return nil, err
}
// Let's always initialize the farm list so it is never nil
if newConfig.Farms.List == nil {

12
common/pkg/config/config_test.go
View File

@ -188,7 +188,7 @@ image_copy_tmp_dir="storage"`
// prior to reading local config, shows hard coded defaults
gomega.Expect(defaultConfig.Containers.HTTPProxy).To(gomega.Equal(true))
err := readConfigFromFile("testdata/containers_default.conf", defaultConfig)
err := readConfigFromFile("testdata/containers_default.conf", defaultConfig, false)
crunWasm := "crun-wasm"
PlatformToOCIRuntimeMap := map[string]string{
@ -351,7 +351,7 @@ image_copy_tmp_dir="storage"`
// Given
// When
conf := Config{}
err := readConfigFromFile("testdata/containers_comment.conf", &conf)
err := readConfigFromFile("testdata/containers_comment.conf", &conf, false)
// Then
gomega.Expect(err).To(gomega.BeNil())
@ -361,7 +361,7 @@ image_copy_tmp_dir="storage"`
// Given
// When
conf := Config{}
err := readConfigFromFile("/invalid/file", &conf)
err := readConfigFromFile("/invalid/file", &conf, false)
// Then
gomega.Expect(err).NotTo(gomega.BeNil())
@ -371,7 +371,7 @@ image_copy_tmp_dir="storage"`
// Given
// When
conf := Config{}
err := readConfigFromFile("config.go", &conf)
err := readConfigFromFile("config.go", &conf, false)
// Then
gomega.Expect(err).NotTo(gomega.BeNil())
@ -760,7 +760,7 @@ image_copy_tmp_dir="storage"`
content := bytes.NewBufferString("")
logrus.SetOutput(content)
logrus.SetLevel(logrus.DebugLevel)
err := readConfigFromFile("testdata/containers_broken.conf", &conf)
err := readConfigFromFile("testdata/containers_broken.conf", &conf, false)
gomega.Expect(err).To(gomega.BeNil())
gomega.Expect(conf.Containers.NetNS).To(gomega.Equal("bridge"))
gomega.Expect(conf.Containers.Umask).To(gomega.Equal("0002"))
@ -772,7 +772,7 @@ image_copy_tmp_dir="storage"`
conf := Config{}
content := bytes.NewBufferString("")
logrus.SetOutput(content)
err := readConfigFromFile("containers.conf", &conf)
err := readConfigFromFile("containers.conf", &conf, false)
gomega.Expect(err).To(gomega.BeNil())
gomega.Expect(content.String()).To(gomega.Equal(""))
logrus.SetOutput(os.Stderr)

21
common/pkg/config/new.go
View File

@ -82,7 +82,7 @@ func newLocked(options *Options) (*Config, error) {
// Merge changes in later configs with the previous configs.
// Each config file that specified fields, will override the
// previous fields.
if err = readConfigFromFile(path, config); err != nil {
if err = readConfigFromFile(path, config, true); err != nil {
return nil, fmt.Errorf("reading system config %q: %w", path, err)
}
logrus.Debugf("Merged system config %q", path)
@ -114,7 +114,7 @@ func newLocked(options *Options) (*Config, error) {
}
// readConfigFromFile reads in container config in the specified
// file and then merge changes with the current default.
if err := readConfigFromFile(add, config); err != nil {
if err := readConfigFromFile(add, config, false); err != nil {
return nil, fmt.Errorf("reading additional config %q: %w", add, err)
}
logrus.Debugf("Merged additional config %q", add)
@ -156,12 +156,8 @@ func systemConfigs() (configs []string, finalErr error) {
}
return append(configs, path), nil
}
if _, err := os.Stat(DefaultContainersConfig); err == nil {
configs = append(configs, DefaultContainersConfig)
}
if _, err := os.Stat(OverrideContainersConfig); err == nil {
configs = append(configs, OverrideContainersConfig)
}
configs = append(configs, DefaultContainersConfig)
configs = append(configs, OverrideContainersConfig)
var err error
configs, err = addConfigs(OverrideContainersConfig+".d", configs)
@ -174,9 +170,7 @@ func systemConfigs() (configs []string, finalErr error) {
return nil, err
}
if path != "" {
if _, err := os.Stat(path); err == nil {
configs = append(configs, path)
}
configs = append(configs, path)
configs, err = addConfigs(path+".d", configs)
if err != nil {
return nil, err
@ -224,10 +218,13 @@ func addConfigs(dirPath string, configs []string) ([]string, error) {
// unmarshal its content into a Config. The config param specifies the previous
// default config. If the path, only specifies a few fields in the Toml file
// the defaults from the config parameter will be used for all other fields.
func readConfigFromFile(path string, config *Config) error {
func readConfigFromFile(path string, config *Config, ignoreErrNotExist bool) error {
logrus.Tracef("Reading configuration file %q", path)
meta, err := toml.DecodeFile(path, config)
if err != nil {
if ignoreErrNotExist && errors.Is(err, fs.ErrNotExist) {
return nil
}
return fmt.Errorf("decode configuration %v: %w", path, err)
}
keys := meta.Undecoded()