From 13e842a11005e2281e9791e101fc238e73b76e2e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 28 Jun 2023 12:40:38 +0200 Subject: [PATCH] cli/config: add synchronisation for configDir (Dir, SetDir) commit 8a30653ed5276179cc4dc0978af13ea2068c7d4e introduced a sync.Once to allow for the config-directory (and home-dir) to be looked up lazily instead of in an `init()`. However, the package-level `configDir` variable can be set through two separate paths; implicitly (through `config.Dir()`), and explicitly, through `config.SetDir()`. The existing code had no synchronisation for this, which could lead to a potential race-condition (code requesting `config.Dir()` and code setting a custom path through `config.SetDir()`). This patch adds synchronisation by triggering the `sync.Once` as part of `config.SetDir()` to prevent it being triggered later (overwriting the value that was set). It also restores the `resetConfigDir()` utility that was removed in 379122b033774d00b3d223fee8d62b0ed4b917e3, to allow resetting the `sync.Once` for this test. In general, we should get rid of this package-level variable, and store it as a config on the client (passing the option to locations where its used instead). Signed-off-by: Sebastiaan van Stijn --- cli/config/config.go | 9 +++++++++ cli/config/config_test.go | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/cli/config/config.go b/cli/config/config.go index cbcdbc1858..b7002fac97 100644 --- a/cli/config/config.go +++ b/cli/config/config.go @@ -27,6 +27,13 @@ var ( configDir string ) +// resetConfigDir is used in testing to reset the "configDir" package variable +// and its sync.Once to force re-lookup between tests. +func resetConfigDir() { + configDir = "" + initConfigDir = new(sync.Once) +} + // Dir returns the directory the configuration file is stored in func Dir() string { initConfigDir.Do(func() { @@ -45,6 +52,8 @@ func ContextStoreDir() string { // SetDir sets the directory the configuration file is stored in func SetDir(dir string) { + // trigger the sync.Once to synchronise with Dir() + initConfigDir.Do(func() {}) configDir = filepath.Clean(dir) } diff --git a/cli/config/config_test.go b/cli/config/config_test.go index 33c9b83177..0efd675b38 100644 --- a/cli/config/config_test.go +++ b/cli/config/config_test.go @@ -382,3 +382,12 @@ func TestConfigPath(t *testing.T) { SetDir(oldDir) } + +// TestSetDir verifies that Dir() does not overwrite the value set through +// SetDir() if it has not been run before. +func TestSetDir(t *testing.T) { + const expected = "my_config_dir" + resetConfigDir() + SetDir(expected) + assert.Check(t, is.Equal(Dir(), expected)) +}