From fa32a18aa190d9919f63ba630571430cc0701864 Mon Sep 17 00:00:00 2001 From: Paul Latzelsperger Date: Tue, 14 Jul 2026 17:10:45 +0200 Subject: [PATCH] fix: isolate tests from developer-local configuration files LoadConfigOrPanic searches /etc/appname, $HOME/.appname and the working directory, so launcher-level tests inherited whatever config files a developer had (e.g. a postgres setting in ~/.appname/pm.env), making them fail or silently change behavior. A CFM_CONFIG_DIR environment variable now overrides the search path exclusively, and the new fixtures.IsolateConfig helper points it at an empty temp dir in every test that boots a launcher. Also fixes the broken env cleanup in TestLoadConfigFromEnvironment, which leaked TESTAPP_* variables into subsequent tests. Co-Authored-By: Claude Fable 5 --- .../launcher/launcher_test.go | 3 ++ common/fixtures/fixtures.go | 8 ++++ common/lifecycleagent/agent_test.go | 3 ++ common/system/config.go | 16 ++++++-- common/system/config_test.go | 38 ++++++++++++------ e2e/e2etests/fixtures.go | 2 + e2e/e2etests/lifecycleagent_test.go | 3 ++ e2e/testagent/launcher/launcher_test.go | 9 +++-- pmanager/cmd/server/launcher/launcher_test.go | 13 +++--- pmanager/natsagent/launcher_test.go | 40 ++++++------------- 10 files changed, 82 insertions(+), 53 deletions(-) diff --git a/agent/lifecycle/keymanagementagent/launcher/launcher_test.go b/agent/lifecycle/keymanagementagent/launcher/launcher_test.go index f99dce12..dc0c71e6 100644 --- a/agent/lifecycle/keymanagementagent/launcher/launcher_test.go +++ b/agent/lifecycle/keymanagementagent/launcher/launcher_test.go @@ -20,6 +20,7 @@ import ( "testing" "time" + "github.com/eclipse-cfm/cfm/common/fixtures" "github.com/eclipse-cfm/cfm/common/natsclient" "github.com/eclipse-cfm/cfm/common/natsfixtures" "github.com/nats-io/nats.go/jetstream" @@ -43,6 +44,8 @@ func TestKeyManagementAgent_Integration(t *testing.T) { require.NoError(t, err) defer natsfixtures.TeardownNatsContainer(ctx, nt) + fixtures.IsolateConfig(t) + t.Setenv("KMAGENT_URI", nt.URI) t.Setenv("KMAGENT_BUCKET", bucket) t.Setenv("KMAGENT_STREAM", streamName) diff --git a/common/fixtures/fixtures.go b/common/fixtures/fixtures.go index c0203ee9..1c5e8619 100644 --- a/common/fixtures/fixtures.go +++ b/common/fixtures/fixtures.go @@ -15,8 +15,16 @@ package fixtures import ( "net" "testing" + + "github.com/eclipse-cfm/cfm/common/system" ) +// IsolateConfig points the configuration file search path at an empty temp directory so that configuration files on +// the developer's machine (e.g. $HOME/.appname) cannot leak into the test. The override is undone when the test ends. +func IsolateConfig(t *testing.T) { + t.Setenv(system.ConfigDirEnvVar, t.TempDir()) +} + // GetRandomPort returns a random available port or fails the test func GetRandomPort(t *testing.T) int { listener, err := net.Listen("tcp", ":0") diff --git a/common/lifecycleagent/agent_test.go b/common/lifecycleagent/agent_test.go index c4c6c0df..c2060aac 100644 --- a/common/lifecycleagent/agent_test.go +++ b/common/lifecycleagent/agent_test.go @@ -20,6 +20,7 @@ import ( "testing" "time" + "github.com/eclipse-cfm/cfm/common/fixtures" "github.com/eclipse-cfm/cfm/common/lifecycleagent" "github.com/eclipse-cfm/cfm/common/natsclient" "github.com/eclipse-cfm/cfm/common/natsfixtures" @@ -73,6 +74,8 @@ func provisionStream(t *testing.T, ctx context.Context, nt *natsfixtures.NatsTes } func setupAgentEnv(t *testing.T, prefix, uri string) { + fixtures.IsolateConfig(t) + // viper uppercases the env prefix and key, so the environment variables must be uppercase. p := strings.ToUpper(prefix) t.Setenv(p+"_URI", uri) diff --git a/common/system/config.go b/common/system/config.go index ee78e66a..4b7cb5fe 100644 --- a/common/system/config.go +++ b/common/system/config.go @@ -14,19 +14,29 @@ package system import ( "fmt" + "os" "strings" "github.com/spf13/viper" ) +// ConfigDirEnvVar overrides the configuration file search path. When set, only the given directory is searched for +// configuration files, replacing the default locations entirely. This pins the config location in deployments and +// isolates tests from configuration files on the developer's machine. +const ConfigDirEnvVar = "CFM_CONFIG_DIR" + // LoadConfigOrPanic initializes a Config instance with the specified configuration name. // Configuration will be read from a file (if it exists) and can be overridden using environment variables. func LoadConfigOrPanic(name string) *viper.Viper { v := viper.New() v.SetConfigName(name) - v.AddConfigPath("/etc/appname/") - v.AddConfigPath("$HOME/.appname") - v.AddConfigPath(".") + if dir := os.Getenv(ConfigDirEnvVar); dir != "" { + v.AddConfigPath(dir) + } else { + v.AddConfigPath("/etc/appname/") + v.AddConfigPath("$HOME/.appname") + v.AddConfigPath(".") + } v.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) v.AutomaticEnv() v.SetEnvPrefix(name) diff --git a/common/system/config_test.go b/common/system/config_test.go index 29faac66..b54eabc0 100644 --- a/common/system/config_test.go +++ b/common/system/config_test.go @@ -11,15 +11,10 @@ import ( const appName = "testapp" -//nolint:errcheck func TestLoadConfigFromEnvironment(t *testing.T) { // setup - os.Setenv("TESTAPP_FOO", "foo_value") - os.Setenv("TESTAPP_BAR", "bar_value") - defer func() { - os.Unsetenv("FOO") - os.Unsetenv("BAR") - }() + t.Setenv("TESTAPP_FOO", "foo_value") + t.Setenv("TESTAPP_BAR", "bar_value") v := LoadConfigOrPanic(appName) @@ -50,6 +45,27 @@ bar: "bar_value" assert.Equal(t, "bar_value", v.GetString("bar")) } +func TestLoadConfigDirOverride(t *testing.T) { + // a config file in the current directory, which must be ignored when the override is set + cwdContent := []byte(`foo: "cwd_value"`) + require.NoError(t, os.WriteFile("./testapp.yaml", cwdContent, 0644)) + defer func() { _ = os.Remove("./testapp.yaml") }() + + // a config file in the override directory + overrideDir := t.TempDir() + overrideContent := []byte(`foo: "override_value"`) + require.NoError(t, os.WriteFile(filepath.Join(overrideDir, "testapp.yaml"), overrideContent, 0644)) + + t.Setenv(ConfigDirEnvVar, overrideDir) + v := LoadConfigOrPanic(appName) + assert.Equal(t, "override_value", v.GetString("foo"), "only the override directory must be searched") + + // pointing the override at an empty directory must yield no file-based configuration at all + t.Setenv(ConfigDirEnvVar, t.TempDir()) + v = LoadConfigOrPanic(appName) + assert.Empty(t, v.GetString("foo"), "config files outside the override directory must be ignored") +} + //nolint:errcheck func TestLoadConfigEnvPriorityOverFile(t *testing.T) { // create the config file @@ -68,12 +84,8 @@ bar: "bar_value_file" defer os.Remove("./testapp.yaml") // Set environment variables - os.Setenv("TESTAPP_FOO", "foo_value_file") - os.Setenv("TESTAPP_BAR", "bar_value_env") - defer func() { - os.Unsetenv("TESTAPP_FOO") - os.Unsetenv("TESTAPP_BAR") - }() + t.Setenv("TESTAPP_FOO", "foo_value_file") + t.Setenv("TESTAPP_BAR", "bar_value_env") v := LoadConfigOrPanic(appName) diff --git a/e2e/e2etests/fixtures.go b/e2e/e2etests/fixtures.go index 23c1d70c..44cb31ac 100644 --- a/e2e/e2etests/fixtures.go +++ b/e2e/e2etests/fixtures.go @@ -44,6 +44,8 @@ func launchPlatformWithAgent(t *testing.T, natsURI string, pgDsn string) *e2efix } func launchPlatform(t *testing.T, natsURI string, pgDsn string) (chan struct{}, *e2efixtures.ApiClient) { + fixtures.IsolateConfig(t) + _ = os.Setenv("TM_POSTGRES", "true") _ = os.Setenv("TM_DSN", pgDsn) _ = os.Setenv("TM_URI", natsURI) diff --git a/e2e/e2etests/lifecycleagent_test.go b/e2e/e2etests/lifecycleagent_test.go index 428b3290..0957edff 100644 --- a/e2e/e2etests/lifecycleagent_test.go +++ b/e2e/e2etests/lifecycleagent_test.go @@ -19,6 +19,7 @@ import ( "testing" "time" + "github.com/eclipse-cfm/cfm/common/fixtures" "github.com/eclipse-cfm/cfm/common/lifecycleagent" "github.com/eclipse-cfm/cfm/common/natsclient" "github.com/eclipse-cfm/cfm/common/natsfixtures" @@ -74,6 +75,8 @@ func Test_VerifyLifecycleAgentDispatchesCloudEvent(t *testing.T) { }) require.NoError(t, err) + fixtures.IsolateConfig(t) + // viper uppercases the env prefix and key, so the environment variables must be uppercase. p := strings.ToUpper(agentName) t.Setenv(p+"_URI", nt.URI) diff --git a/e2e/testagent/launcher/launcher_test.go b/e2e/testagent/launcher/launcher_test.go index 670dcfa6..0238eb5f 100644 --- a/e2e/testagent/launcher/launcher_test.go +++ b/e2e/testagent/launcher/launcher_test.go @@ -14,10 +14,10 @@ package launcher import ( "context" - "os" "testing" "time" + "github.com/eclipse-cfm/cfm/common/fixtures" "github.com/eclipse-cfm/cfm/common/natsclient" "github.com/eclipse-cfm/cfm/common/natsfixtures" "github.com/eclipse-cfm/cfm/common/runtime" @@ -65,9 +65,10 @@ func TestTestAgent_Integration(t *testing.T) { } // Required agent config - _ = os.Setenv("TESTAGENT_URI", nt.URI) - _ = os.Setenv("TESTAGENT_BUCKET", "cfm-bucket") - _ = os.Setenv("TESTAGENT_STREAM", streamName) + fixtures.IsolateConfig(t) + t.Setenv("TESTAGENT_URI", nt.URI) + t.Setenv("TESTAGENT_BUCKET", "cfm-bucket") + t.Setenv("TESTAGENT_STREAM", streamName) // Create and start the test agent shutdownChannel := make(chan struct{}) diff --git a/pmanager/cmd/server/launcher/launcher_test.go b/pmanager/cmd/server/launcher/launcher_test.go index 9296e1a6..b27295d5 100644 --- a/pmanager/cmd/server/launcher/launcher_test.go +++ b/pmanager/cmd/server/launcher/launcher_test.go @@ -14,7 +14,6 @@ package launcher import ( "context" - "os" "strconv" "testing" "time" @@ -30,6 +29,8 @@ const ( ) func TestTestAgent_Integration(t *testing.T) { + fixtures.IsolateConfig(t) + ctx, cancel := context.WithTimeout(context.Background(), testTimeout) defer cancel() @@ -43,11 +44,11 @@ func TestTestAgent_Integration(t *testing.T) { natsfixtures.SetupTestStream(t, ctx, nt.Client, streamName) // Required agent config - _ = os.Setenv("PM_URI", nt.URI) - _ = os.Setenv("PM_BUCKET", "cfm-bucket") - _ = os.Setenv("PM_STREAM", streamName) - _ = os.Setenv("PM_HTTPPORT", strconv.Itoa(fixtures.GetRandomPort(t))) - _ = os.Setenv("PM_AUTH_ENABLED", "false") + t.Setenv("PM_URI", nt.URI) + t.Setenv("PM_BUCKET", "cfm-bucket") + t.Setenv("PM_STREAM", streamName) + t.Setenv("PM_HTTPPORT", strconv.Itoa(fixtures.GetRandomPort(t))) + t.Setenv("PM_AUTH_ENABLED", "false") // Create and start the test agent shutdownChannel := make(chan struct{}) diff --git a/pmanager/natsagent/launcher_test.go b/pmanager/natsagent/launcher_test.go index 88e52167..d252f2b5 100644 --- a/pmanager/natsagent/launcher_test.go +++ b/pmanager/natsagent/launcher_test.go @@ -13,9 +13,9 @@ package natsagent import ( - "os" "testing" + "github.com/eclipse-cfm/cfm/common/fixtures" "github.com/eclipse-cfm/cfm/common/system" "github.com/eclipse-cfm/cfm/pmanager/api" "github.com/stretchr/testify/require" @@ -23,15 +23,9 @@ import ( func Test_LaunchAgent_WithAssemblyProvider(t *testing.T) { // Setup environment variables - _ = os.Setenv("TESTAGENT_URI", "nats://localhost:4222") - _ = os.Setenv("TESTAGENT_BUCKET", "test-bucket") - _ = os.Setenv("TESTAGENT_STREAM", "test-stream") - - t.Cleanup(func() { - _ = os.Unsetenv("TESTAGENT_URI") - _ = os.Unsetenv("TESTAGENT_BUCKET") - _ = os.Unsetenv("TESTAGENT_STREAM") - }) + t.Setenv("TESTAGENT_URI", "nats://localhost:4222") + t.Setenv("TESTAGENT_BUCKET", "test-bucket") + t.Setenv("TESTAGENT_STREAM", "test-stream") // Create a flag to track if AssemblyProvider was called var assemblyProviderCalled bool @@ -90,16 +84,12 @@ func Test_LaunchAgent_WithAssemblyProvider(t *testing.T) { } func Test_LaunchAgent_WithoutAssemblyProvider(t *testing.T) { - // Setup environment variables - _ = os.Setenv("TESTAGENT_URI", "nats://localhost:4222") - _ = os.Setenv("TESTAGENT_BUCKET", "test-bucket") - _ = os.Setenv("TESTAGENT_STREAM", "test-stream") + fixtures.IsolateConfig(t) - t.Cleanup(func() { - _ = os.Unsetenv("TESTAGENT_URI") - _ = os.Unsetenv("TESTAGENT_BUCKET") - _ = os.Unsetenv("TESTAGENT_STREAM") - }) + // Setup environment variables + t.Setenv("TESTAGENT_URI", "nats://localhost:4222") + t.Setenv("TESTAGENT_BUCKET", "test-bucket") + t.Setenv("TESTAGENT_STREAM", "test-stream") // Create launcher config without AssemblyProvider config := LauncherConfig{ @@ -125,15 +115,11 @@ func Test_LaunchAgent_WithoutAssemblyProvider(t *testing.T) { } func Test_loadAgentConfig_MissingRequiredParams(t *testing.T) { + fixtures.IsolateConfig(t) + // Test with missing URI - _ = os.Setenv("TESTAGENT_BUCKET", "test-bucket") - _ = os.Setenv("TESTAGENT_STREAM", "test-stream") - - t.Cleanup(func() { - _ = os.Unsetenv("TESTAGENT_URI") - _ = os.Unsetenv("TESTAGENT_BUCKET") - _ = os.Unsetenv("TESTAGENT_STREAM") - }) + t.Setenv("TESTAGENT_BUCKET", "test-bucket") + t.Setenv("TESTAGENT_STREAM", "test-stream") // Verify that missing required params causes panic require.Panics(t, func() {