From 1294122947b642ed3333dece9c7bef50d279fdee Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 27 Jan 2025 18:18:04 +0100 Subject: [PATCH 1/3] ignore ENOENT errors when parsing registries.d files As always listing files in a dir to then read them is racy as the file might have been removed in the meantime. Thus we must ignore ENOENT errors when the file is opened. This is not just a theoretical problem, the reason I am here is because it caused a flake in the podman CI[1]: ... open /etc/containers/registries.d/podman-test-only-temporary-addition.yaml: no such file or directory [1] https://api.cirrus-ci.com/v1/artifact/task/6673405799301120/html/int-podman-fedora-40-root-host-boltdb.log.html#t--Podman-push-podman-push-to-local-registry-with-authorization--1 Signed-off-by: Paul Holzinger --- docker/registries_d.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docker/registries_d.go b/docker/registries_d.go index 3619c3baef..89d48cc4fe 100644 --- a/docker/registries_d.go +++ b/docker/registries_d.go @@ -3,6 +3,7 @@ package docker import ( "errors" "fmt" + "io/fs" "net/url" "os" "path" @@ -129,6 +130,11 @@ func loadAndMergeConfig(dirPath string) (*registryConfiguration, error) { configPath := filepath.Join(dirPath, configName) configBytes, err := os.ReadFile(configPath) if err != nil { + if errors.Is(err, fs.ErrNotExist) { + // file must have been removed between the directory listing + // and the open call, ignore that as it is a expected race + continue + } return nil, err } From c9771a80f77c2ad641796e827324171490f9a8da Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 27 Jan 2025 18:26:06 +0100 Subject: [PATCH 2/3] ignore ENOENT errors when parsing registries.conf.d files As always listing files in a dir to then read them is racy as the file might have been removed in the meantime. Thus we must ignore ENOENT errors when the file is opened. Signed-off-by: Paul Holzinger --- pkg/sysregistriesv2/system_registries_v2.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/sysregistriesv2/system_registries_v2.go b/pkg/sysregistriesv2/system_registries_v2.go index 1b161474da..9ac050512a 100644 --- a/pkg/sysregistriesv2/system_registries_v2.go +++ b/pkg/sysregistriesv2/system_registries_v2.go @@ -1,6 +1,7 @@ package sysregistriesv2 import ( + "errors" "fmt" "io/fs" "os" @@ -744,6 +745,11 @@ func tryUpdatingCache(ctx *types.SystemContext, wrapper configWrapper) (*parsedC // Enforce v2 format for drop-in-configs. dropIn, err := loadConfigFile(path, true) if err != nil { + if errors.Is(err, fs.ErrNotExist) { + // file must have been removed between the directory listing + // and the open call, ignore that as it is a expected race + continue + } return nil, fmt.Errorf("loading drop-in registries configuration %q: %w", path, err) } config.updateWithConfigurationFrom(dropIn) From 3f17e2e843bc967f7ca91716cedd0d60ac0339a9 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 27 Jan 2025 18:35:19 +0100 Subject: [PATCH 3/3] ignore ENOENT errors when parsing .crt files As always listing files in a dir to then read them is racy as the file might have been removed in the meantime. Thus we must ignore ENOENT errors when the file is opened. Now here the code already did not cause an hard error but it will cause a spurious warning in such case. There is really no need to log that as it can cause flakes for podman. Now there is the case here for .cert and .key files where both files must be present for a valid config. Ignoring ENOENT there seems wrong as it would hide a common misconfiguration where only one of the files exists. That mean the race can still cause a failure when these files are removed from the dir. Signed-off-by: Paul Holzinger --- pkg/tlsclientconfig/tlsclientconfig.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/tlsclientconfig/tlsclientconfig.go b/pkg/tlsclientconfig/tlsclientconfig.go index f6c0576e07..4e0ee57e91 100644 --- a/pkg/tlsclientconfig/tlsclientconfig.go +++ b/pkg/tlsclientconfig/tlsclientconfig.go @@ -3,6 +3,7 @@ package tlsclientconfig import ( "crypto/tls" "crypto/x509" + "errors" "fmt" "net" "net/http" @@ -36,12 +37,9 @@ func SetupCertificates(dir string, tlsc *tls.Config) error { logrus.Debugf(" crt: %s", fullPath) data, err := os.ReadFile(fullPath) if err != nil { - if os.IsNotExist(err) { - // Dangling symbolic link? - // Race with someone who deleted the - // file after we read the directory's - // list of contents? - logrus.Warnf("error reading certificate %q: %v", fullPath, err) + if errors.Is(err, os.ErrNotExist) { + // file must have been removed between the directory listing + // and the open call, ignore that as it is a expected race continue } return err