From 5b8a8dc37749e85828ccc97245a404ac4806554c Mon Sep 17 00:00:00 2001 From: Spencer Date: Thu, 13 Jun 2024 15:24:31 +0100 Subject: [PATCH 01/13] changed env variable prefix from NMS to NGINX_AGENT --- main.go | 4 ++++ src/core/config/config.go | 18 ++++++++++++++++-- src/core/config/config_test.go | 2 +- src/core/config/defaults.go | 3 ++- 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/main.go b/main.go index 0fa995e5a2..12debc3d34 100644 --- a/main.go +++ b/main.go @@ -59,6 +59,10 @@ func main() { defer logFile.Close() } + if config.MigratedEnv { + log.Warnf("The environment variable prefix 'NMS' is deprecated. Prefix has been migrated to 'NGINX_AGENT'. Please update your configuration to use the new prefix.") + } + log.Tracef("Config loaded from disk, %v", loadedConfig) if loadedConfig.DisplayName == "" { diff --git a/src/core/config/config.go b/src/core/config/config.go index 58c5ce0f36..a12bac556c 100644 --- a/src/core/config/config.go +++ b/src/core/config/config.go @@ -47,7 +47,10 @@ const ( ` ) -var Viper = viper.NewWithOptions(viper.KeyDelimiter(agent_config.KeyDelimiter)) +var ( + Viper = viper.NewWithOptions(viper.KeyDelimiter(agent_config.KeyDelimiter)) + MigratedEnv = false +) func SetVersion(version, commit string) { ROOT_COMMAND.Version = version + "-" + commit @@ -133,7 +136,7 @@ func deprecateFlags() { } func RegisterFlags() { - Viper.SetEnvPrefix(EnvPrefix) + Viper.SetEnvPrefix(NewEnvPrefix) Viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) Viper.AutomaticEnv() @@ -149,6 +152,17 @@ func RegisterFlags() { if err := Viper.BindPFlag(strings.ReplaceAll(flag.Name, "-", "_"), fs.Lookup(flag.Name)); err != nil { return } + + oldKey := strings.ToUpper(OldEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) + newKey := strings.ToUpper(NewEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) + + if os.Getenv(oldKey) != "" && os.Getenv(newKey) == "" { + if err := os.Setenv(newKey, os.Getenv(oldKey)); err != nil { + log.Warnf("Failed to set environment variable %s: %v", newKey, err) + } + MigratedEnv = true + } + err := Viper.BindEnv(flag.Name) if err != nil { log.Warnf("Error occurred binding env %s: %v", flag.Name, err) diff --git a/src/core/config/config_test.go b/src/core/config/config_test.go index 44dc135f8f..aa2c31be23 100644 --- a/src/core/config/config_test.go +++ b/src/core/config/config_test.go @@ -480,7 +480,7 @@ func TestUpdateAgentConfig(t *testing.T) { } func setEnvVariable(t *testing.T, name string, value string) { - key := strings.ToUpper(EnvPrefix + agent_config.KeyDelimiter + name) + key := strings.ToUpper(NewEnvPrefix + agent_config.KeyDelimiter + name) err := os.Setenv(key, value) require.NoError(t, err) } diff --git a/src/core/config/defaults.go b/src/core/config/defaults.go index 731d6e48a1..03ec8557e0 100644 --- a/src/core/config/defaults.go +++ b/src/core/config/defaults.go @@ -103,7 +103,8 @@ const ( DynamicConfigFileAbsFreeBsdPath = "/var/db/nginx-agent/agent-dynamic.conf" ConfigFileName = "nginx-agent.conf" ConfigFileType = "yaml" - EnvPrefix = "nms" + OldEnvPrefix = "nms" + NewEnvPrefix = "nginx_agent" ConfigPathKey = "path" DynamicConfigPathKey = "dynamic_config_path" From 1997ecd82551862b07cf3d3a048eeb0b94aa8995 Mon Sep 17 00:00:00 2001 From: Spencer Date: Thu, 13 Jun 2024 15:32:57 +0100 Subject: [PATCH 02/13] changed env variable prefix from NMS to NGINX_AGENT --- .../nginx/agent/v2/src/core/config/config.go | 18 ++++++++++++++++-- .../nginx/agent/v2/src/core/config/defaults.go | 3 ++- .../nginx/agent/v2/src/core/config/config.go | 18 ++++++++++++++++-- .../nginx/agent/v2/src/core/config/defaults.go | 3 ++- 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go index 58c5ce0f36..a12bac556c 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go @@ -47,7 +47,10 @@ const ( ` ) -var Viper = viper.NewWithOptions(viper.KeyDelimiter(agent_config.KeyDelimiter)) +var ( + Viper = viper.NewWithOptions(viper.KeyDelimiter(agent_config.KeyDelimiter)) + MigratedEnv = false +) func SetVersion(version, commit string) { ROOT_COMMAND.Version = version + "-" + commit @@ -133,7 +136,7 @@ func deprecateFlags() { } func RegisterFlags() { - Viper.SetEnvPrefix(EnvPrefix) + Viper.SetEnvPrefix(NewEnvPrefix) Viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) Viper.AutomaticEnv() @@ -149,6 +152,17 @@ func RegisterFlags() { if err := Viper.BindPFlag(strings.ReplaceAll(flag.Name, "-", "_"), fs.Lookup(flag.Name)); err != nil { return } + + oldKey := strings.ToUpper(OldEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) + newKey := strings.ToUpper(NewEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) + + if os.Getenv(oldKey) != "" && os.Getenv(newKey) == "" { + if err := os.Setenv(newKey, os.Getenv(oldKey)); err != nil { + log.Warnf("Failed to set environment variable %s: %v", newKey, err) + } + MigratedEnv = true + } + err := Viper.BindEnv(flag.Name) if err != nil { log.Warnf("Error occurred binding env %s: %v", flag.Name, err) diff --git a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go index 731d6e48a1..03ec8557e0 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go @@ -103,7 +103,8 @@ const ( DynamicConfigFileAbsFreeBsdPath = "/var/db/nginx-agent/agent-dynamic.conf" ConfigFileName = "nginx-agent.conf" ConfigFileType = "yaml" - EnvPrefix = "nms" + OldEnvPrefix = "nms" + NewEnvPrefix = "nginx_agent" ConfigPathKey = "path" DynamicConfigPathKey = "dynamic_config_path" diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go index 58c5ce0f36..a12bac556c 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go @@ -47,7 +47,10 @@ const ( ` ) -var Viper = viper.NewWithOptions(viper.KeyDelimiter(agent_config.KeyDelimiter)) +var ( + Viper = viper.NewWithOptions(viper.KeyDelimiter(agent_config.KeyDelimiter)) + MigratedEnv = false +) func SetVersion(version, commit string) { ROOT_COMMAND.Version = version + "-" + commit @@ -133,7 +136,7 @@ func deprecateFlags() { } func RegisterFlags() { - Viper.SetEnvPrefix(EnvPrefix) + Viper.SetEnvPrefix(NewEnvPrefix) Viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) Viper.AutomaticEnv() @@ -149,6 +152,17 @@ func RegisterFlags() { if err := Viper.BindPFlag(strings.ReplaceAll(flag.Name, "-", "_"), fs.Lookup(flag.Name)); err != nil { return } + + oldKey := strings.ToUpper(OldEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) + newKey := strings.ToUpper(NewEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) + + if os.Getenv(oldKey) != "" && os.Getenv(newKey) == "" { + if err := os.Setenv(newKey, os.Getenv(oldKey)); err != nil { + log.Warnf("Failed to set environment variable %s: %v", newKey, err) + } + MigratedEnv = true + } + err := Viper.BindEnv(flag.Name) if err != nil { log.Warnf("Error occurred binding env %s: %v", flag.Name, err) diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go index 731d6e48a1..03ec8557e0 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go @@ -103,7 +103,8 @@ const ( DynamicConfigFileAbsFreeBsdPath = "/var/db/nginx-agent/agent-dynamic.conf" ConfigFileName = "nginx-agent.conf" ConfigFileType = "yaml" - EnvPrefix = "nms" + OldEnvPrefix = "nms" + NewEnvPrefix = "nginx_agent" ConfigPathKey = "path" DynamicConfigPathKey = "dynamic_config_path" From 3c5157c6704e79722efb2f295e62760920917181 Mon Sep 17 00:00:00 2001 From: Spencer Date: Mon, 17 Jun 2024 09:57:22 +0100 Subject: [PATCH 03/13] renamed env prefix constants --- src/core/config/config.go | 6 +++--- src/core/config/config_test.go | 2 +- src/core/config/defaults.go | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/core/config/config.go b/src/core/config/config.go index a12bac556c..75b72a9e14 100644 --- a/src/core/config/config.go +++ b/src/core/config/config.go @@ -136,7 +136,7 @@ func deprecateFlags() { } func RegisterFlags() { - Viper.SetEnvPrefix(NewEnvPrefix) + Viper.SetEnvPrefix(EnvPrefix) Viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) Viper.AutomaticEnv() @@ -153,8 +153,8 @@ func RegisterFlags() { return } - oldKey := strings.ToUpper(OldEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) - newKey := strings.ToUpper(NewEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) + oldKey := strings.ToUpper(LegacyEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) + newKey := strings.ToUpper(EnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) if os.Getenv(oldKey) != "" && os.Getenv(newKey) == "" { if err := os.Setenv(newKey, os.Getenv(oldKey)); err != nil { diff --git a/src/core/config/config_test.go b/src/core/config/config_test.go index aa2c31be23..44dc135f8f 100644 --- a/src/core/config/config_test.go +++ b/src/core/config/config_test.go @@ -480,7 +480,7 @@ func TestUpdateAgentConfig(t *testing.T) { } func setEnvVariable(t *testing.T, name string, value string) { - key := strings.ToUpper(NewEnvPrefix + agent_config.KeyDelimiter + name) + key := strings.ToUpper(EnvPrefix + agent_config.KeyDelimiter + name) err := os.Setenv(key, value) require.NoError(t, err) } diff --git a/src/core/config/defaults.go b/src/core/config/defaults.go index 03ec8557e0..9a39c9a52a 100644 --- a/src/core/config/defaults.go +++ b/src/core/config/defaults.go @@ -103,8 +103,8 @@ const ( DynamicConfigFileAbsFreeBsdPath = "/var/db/nginx-agent/agent-dynamic.conf" ConfigFileName = "nginx-agent.conf" ConfigFileType = "yaml" - OldEnvPrefix = "nms" - NewEnvPrefix = "nginx_agent" + LegacyEnvPrefix = "nms" + EnvPrefix = "nginx_agent" ConfigPathKey = "path" DynamicConfigPathKey = "dynamic_config_path" From aa1a5eace7fec1b664a5977d36db61289c928be0 Mon Sep 17 00:00:00 2001 From: Spencer Date: Mon, 17 Jun 2024 09:58:42 +0100 Subject: [PATCH 04/13] renamed env prefix constants --- .../github.com/nginx/agent/v2/src/core/config/config.go | 6 +++--- .../github.com/nginx/agent/v2/src/core/config/defaults.go | 4 ++-- .../github.com/nginx/agent/v2/src/core/config/config.go | 6 +++--- .../github.com/nginx/agent/v2/src/core/config/defaults.go | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go index a12bac556c..75b72a9e14 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go @@ -136,7 +136,7 @@ func deprecateFlags() { } func RegisterFlags() { - Viper.SetEnvPrefix(NewEnvPrefix) + Viper.SetEnvPrefix(EnvPrefix) Viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) Viper.AutomaticEnv() @@ -153,8 +153,8 @@ func RegisterFlags() { return } - oldKey := strings.ToUpper(OldEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) - newKey := strings.ToUpper(NewEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) + oldKey := strings.ToUpper(LegacyEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) + newKey := strings.ToUpper(EnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) if os.Getenv(oldKey) != "" && os.Getenv(newKey) == "" { if err := os.Setenv(newKey, os.Getenv(oldKey)); err != nil { diff --git a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go index 03ec8557e0..9a39c9a52a 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go @@ -103,8 +103,8 @@ const ( DynamicConfigFileAbsFreeBsdPath = "/var/db/nginx-agent/agent-dynamic.conf" ConfigFileName = "nginx-agent.conf" ConfigFileType = "yaml" - OldEnvPrefix = "nms" - NewEnvPrefix = "nginx_agent" + LegacyEnvPrefix = "nms" + EnvPrefix = "nginx_agent" ConfigPathKey = "path" DynamicConfigPathKey = "dynamic_config_path" diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go index a12bac556c..75b72a9e14 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go @@ -136,7 +136,7 @@ func deprecateFlags() { } func RegisterFlags() { - Viper.SetEnvPrefix(NewEnvPrefix) + Viper.SetEnvPrefix(EnvPrefix) Viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) Viper.AutomaticEnv() @@ -153,8 +153,8 @@ func RegisterFlags() { return } - oldKey := strings.ToUpper(OldEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) - newKey := strings.ToUpper(NewEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) + oldKey := strings.ToUpper(LegacyEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) + newKey := strings.ToUpper(EnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) if os.Getenv(oldKey) != "" && os.Getenv(newKey) == "" { if err := os.Setenv(newKey, os.Getenv(oldKey)); err != nil { diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go index 03ec8557e0..9a39c9a52a 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go @@ -103,8 +103,8 @@ const ( DynamicConfigFileAbsFreeBsdPath = "/var/db/nginx-agent/agent-dynamic.conf" ConfigFileName = "nginx-agent.conf" ConfigFileType = "yaml" - OldEnvPrefix = "nms" - NewEnvPrefix = "nginx_agent" + LegacyEnvPrefix = "nms" + EnvPrefix = "nginx_agent" ConfigPathKey = "path" DynamicConfigPathKey = "dynamic_config_path" From 5ff0aa5b2c74d6843433879350443383b935154c Mon Sep 17 00:00:00 2001 From: Spencer Date: Mon, 17 Jun 2024 10:27:34 +0100 Subject: [PATCH 05/13] refactored string building of oldKey and newKey --- src/core/config/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/config/config.go b/src/core/config/config.go index 75b72a9e14..ab46af64ab 100644 --- a/src/core/config/config.go +++ b/src/core/config/config.go @@ -153,8 +153,8 @@ func RegisterFlags() { return } - oldKey := strings.ToUpper(LegacyEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) - newKey := strings.ToUpper(EnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) + oldKey := strings.ToUpper(fmt.Sprintf("%s%s%s", LegacyEnvPrefix, agent_config.KeyDelimiter, strings.ReplaceAll(flag.Name, "-", "_"))) + newKey := strings.ToUpper(fmt.Sprintf("%s%s%s", EnvPrefix, agent_config.KeyDelimiter, strings.ReplaceAll(flag.Name, "-", "_"))) if os.Getenv(oldKey) != "" && os.Getenv(newKey) == "" { if err := os.Setenv(newKey, os.Getenv(oldKey)); err != nil { From 8873427c18efb5d96c477f088abda3ea71d426ae Mon Sep 17 00:00:00 2001 From: Spencer Date: Mon, 17 Jun 2024 10:28:29 +0100 Subject: [PATCH 06/13] refactored string building of oldKey and newKey --- .../github.com/nginx/agent/v2/src/core/config/config.go | 4 ++-- .../github.com/nginx/agent/v2/src/core/config/config.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go index 75b72a9e14..ab46af64ab 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go @@ -153,8 +153,8 @@ func RegisterFlags() { return } - oldKey := strings.ToUpper(LegacyEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) - newKey := strings.ToUpper(EnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) + oldKey := strings.ToUpper(fmt.Sprintf("%s%s%s", LegacyEnvPrefix, agent_config.KeyDelimiter, strings.ReplaceAll(flag.Name, "-", "_"))) + newKey := strings.ToUpper(fmt.Sprintf("%s%s%s", EnvPrefix, agent_config.KeyDelimiter, strings.ReplaceAll(flag.Name, "-", "_"))) if os.Getenv(oldKey) != "" && os.Getenv(newKey) == "" { if err := os.Setenv(newKey, os.Getenv(oldKey)); err != nil { diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go index 75b72a9e14..ab46af64ab 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go @@ -153,8 +153,8 @@ func RegisterFlags() { return } - oldKey := strings.ToUpper(LegacyEnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) - newKey := strings.ToUpper(EnvPrefix + "_" + strings.ReplaceAll(flag.Name, "-", "_")) + oldKey := strings.ToUpper(fmt.Sprintf("%s%s%s", LegacyEnvPrefix, agent_config.KeyDelimiter, strings.ReplaceAll(flag.Name, "-", "_"))) + newKey := strings.ToUpper(fmt.Sprintf("%s%s%s", EnvPrefix, agent_config.KeyDelimiter, strings.ReplaceAll(flag.Name, "-", "_"))) if os.Getenv(oldKey) != "" && os.Getenv(newKey) == "" { if err := os.Setenv(newKey, os.Getenv(oldKey)); err != nil { From 596b76426fe9eed8cac4ca61b941c838aafccf8d Mon Sep 17 00:00:00 2001 From: Spencer Date: Mon, 17 Jun 2024 12:37:56 +0100 Subject: [PATCH 07/13] added test for env prefix migration --- src/core/config/config_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/core/config/config_test.go b/src/core/config/config_test.go index 44dc135f8f..ce45b25f50 100644 --- a/src/core/config/config_test.go +++ b/src/core/config/config_test.go @@ -479,6 +479,18 @@ func TestUpdateAgentConfig(t *testing.T) { } } +func TestDeprecatedEnvPrefixMigration(t *testing.T){ + want := true + + t.Setenv("NMS_TLS_SKIP_VERIFY","true") + + SetDefaults() + RegisterFlags() + + got := MigratedEnv + assert.Equal(t, want, got) +} + func setEnvVariable(t *testing.T, name string, value string) { key := strings.ToUpper(EnvPrefix + agent_config.KeyDelimiter + name) err := os.Setenv(key, value) From f1d4a55272eddb90fadb878fa9f8fff6a0c7c66c Mon Sep 17 00:00:00 2001 From: Spencer Date: Mon, 17 Jun 2024 12:39:10 +0100 Subject: [PATCH 08/13] added test for env prefix migration --- src/core/config/config_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/config/config_test.go b/src/core/config/config_test.go index ce45b25f50..59ac39f2fa 100644 --- a/src/core/config/config_test.go +++ b/src/core/config/config_test.go @@ -479,17 +479,17 @@ func TestUpdateAgentConfig(t *testing.T) { } } -func TestDeprecatedEnvPrefixMigration(t *testing.T){ +func TestDeprecatedEnvPrefixMigration(t *testing.T) { want := true - - t.Setenv("NMS_TLS_SKIP_VERIFY","true") + + t.Setenv("NMS_TLS_SKIP_VERIFY", "true") SetDefaults() RegisterFlags() got := MigratedEnv assert.Equal(t, want, got) -} +} func setEnvVariable(t *testing.T, name string, value string) { key := strings.ToUpper(EnvPrefix + agent_config.KeyDelimiter + name) From e4e8ca6316937a1d009f02385ba236a695fec54e Mon Sep 17 00:00:00 2001 From: Spencer Date: Mon, 17 Jun 2024 12:45:51 +0100 Subject: [PATCH 09/13] refactored string builing of oldKey and newKey --- src/core/config/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/config/config.go b/src/core/config/config.go index ab46af64ab..5703bf109d 100644 --- a/src/core/config/config.go +++ b/src/core/config/config.go @@ -153,8 +153,8 @@ func RegisterFlags() { return } - oldKey := strings.ToUpper(fmt.Sprintf("%s%s%s", LegacyEnvPrefix, agent_config.KeyDelimiter, strings.ReplaceAll(flag.Name, "-", "_"))) - newKey := strings.ToUpper(fmt.Sprintf("%s%s%s", EnvPrefix, agent_config.KeyDelimiter, strings.ReplaceAll(flag.Name, "-", "_"))) + oldKey := strings.ToUpper(LegacyEnvPrefix + agent_config.KeyDelimiter + strings.ReplaceAll(flag.Name, "-", agent_config.KeyDelimiter)) + newKey := strings.ToUpper(EnvPrefix + agent_config.KeyDelimiter + strings.ReplaceAll(flag.Name, "-", agent_config.KeyDelimiter)) if os.Getenv(oldKey) != "" && os.Getenv(newKey) == "" { if err := os.Setenv(newKey, os.Getenv(oldKey)); err != nil { From c3c01d32d147ee353ddb08e5f0d57857a7867387 Mon Sep 17 00:00:00 2001 From: Spencer Date: Mon, 17 Jun 2024 12:46:31 +0100 Subject: [PATCH 10/13] refactored string builing of oldKey and newKey --- .../github.com/nginx/agent/v2/src/core/config/config.go | 4 ++-- .../github.com/nginx/agent/v2/src/core/config/config.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go index ab46af64ab..5703bf109d 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go @@ -153,8 +153,8 @@ func RegisterFlags() { return } - oldKey := strings.ToUpper(fmt.Sprintf("%s%s%s", LegacyEnvPrefix, agent_config.KeyDelimiter, strings.ReplaceAll(flag.Name, "-", "_"))) - newKey := strings.ToUpper(fmt.Sprintf("%s%s%s", EnvPrefix, agent_config.KeyDelimiter, strings.ReplaceAll(flag.Name, "-", "_"))) + oldKey := strings.ToUpper(LegacyEnvPrefix + agent_config.KeyDelimiter + strings.ReplaceAll(flag.Name, "-", agent_config.KeyDelimiter)) + newKey := strings.ToUpper(EnvPrefix + agent_config.KeyDelimiter + strings.ReplaceAll(flag.Name, "-", agent_config.KeyDelimiter)) if os.Getenv(oldKey) != "" && os.Getenv(newKey) == "" { if err := os.Setenv(newKey, os.Getenv(oldKey)); err != nil { diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go index ab46af64ab..5703bf109d 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go @@ -153,8 +153,8 @@ func RegisterFlags() { return } - oldKey := strings.ToUpper(fmt.Sprintf("%s%s%s", LegacyEnvPrefix, agent_config.KeyDelimiter, strings.ReplaceAll(flag.Name, "-", "_"))) - newKey := strings.ToUpper(fmt.Sprintf("%s%s%s", EnvPrefix, agent_config.KeyDelimiter, strings.ReplaceAll(flag.Name, "-", "_"))) + oldKey := strings.ToUpper(LegacyEnvPrefix + agent_config.KeyDelimiter + strings.ReplaceAll(flag.Name, "-", agent_config.KeyDelimiter)) + newKey := strings.ToUpper(EnvPrefix + agent_config.KeyDelimiter + strings.ReplaceAll(flag.Name, "-", agent_config.KeyDelimiter)) if os.Getenv(oldKey) != "" && os.Getenv(newKey) == "" { if err := os.Setenv(newKey, os.Getenv(oldKey)); err != nil { From a31da69cf3017ee52a9e1cc63fdd4dbacc4b13a8 Mon Sep 17 00:00:00 2001 From: Spencer Date: Mon, 17 Jun 2024 13:59:26 +0100 Subject: [PATCH 11/13] fixed deprecated env prefix migration test --- src/core/config/config_test.go | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/core/config/config_test.go b/src/core/config/config_test.go index 59ac39f2fa..3e390a7802 100644 --- a/src/core/config/config_test.go +++ b/src/core/config/config_test.go @@ -482,12 +482,30 @@ func TestUpdateAgentConfig(t *testing.T) { func TestDeprecatedEnvPrefixMigration(t *testing.T) { want := true - t.Setenv("NMS_TLS_SKIP_VERIFY", "true") + curDir, err := os.Getwd() + require.NoError(t, err) - SetDefaults() - RegisterFlags() + tempConfDeleteFunc, err := sysutils.CopyFile(fmt.Sprintf("%s/%s", testCfgDir, emptyConfigFile), tempCfgFile) + defer func() { + err := tempConfDeleteFunc() + require.NoError(t, err, "deletion of temp config file failed") + }() + require.NoError(t, err) + + tempDynamicDeleteFunc, err := sysutils.CopyFile(fmt.Sprintf("%s/%s", testCfgDir, emptyConfigFile), tempDynamicCfgFile) + defer func() { + err := tempDynamicDeleteFunc() + require.NoError(t, err, "deletion of temp dynamic config file failed") + }() + require.NoError(t, err) + + cleanEnv(t, tempCfgFile, fmt.Sprintf("%s/%s", curDir, tempDynamicCfgFile)) + setEnvVariable(t, "tls_skip_verify", "true") + + config, err := GetConfig("1234") + require.NoError(t, err) - got := MigratedEnv + got := config.TLS.SkipVerify assert.Equal(t, want, got) } From f711e49c8b5f68dfb4643cf3014ab200073f52df Mon Sep 17 00:00:00 2001 From: Oliver O'Mahony Date: Mon, 17 Jun 2024 15:42:25 +0100 Subject: [PATCH 12/13] more robust test --- src/core/config/config_test.go | 54 ++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/src/core/config/config_test.go b/src/core/config/config_test.go index 3e390a7802..a5423c4367 100644 --- a/src/core/config/config_test.go +++ b/src/core/config/config_test.go @@ -130,7 +130,7 @@ func TestGetConfig(t *testing.T) { require.NoError(t, err) // Initialize environment with the empty configs - cleanEnv(t, tempCfgFile, fmt.Sprintf("%s/%s", curDir, tempDynamicCfgFile)) + cleanEnv(t, tempCfgFile, fmt.Sprintf("%s/%s", curDir, tempDynamicCfgFile), true) config, err := GetConfig("12345") require.NoError(t, err) @@ -183,7 +183,7 @@ func TestGetConfig(t *testing.T) { require.NoError(t, err) // Initialize environment with the empty configs - cleanEnv(t, tempCfgFile, fmt.Sprintf("%s/%s", curDir, tempDynamicCfgFile)) + cleanEnv(t, tempCfgFile, fmt.Sprintf("%s/%s", curDir, tempDynamicCfgFile), true) updatedTag := "updated-tag" updatedLogLevel := "fatal" @@ -226,7 +226,7 @@ func TestGetConfig(t *testing.T) { require.NoError(t, err) // Initialize environment with the empty configs - cleanEnv(t, tempCfgFile, fmt.Sprintf("%s/%s", curDir, tempDynamicCfgFile)) + cleanEnv(t, tempCfgFile, fmt.Sprintf("%s/%s", curDir, tempDynamicCfgFile), true) // Copy config file with updated values to current directory updatedTempConfDeleteFunc, err := sysutils.CopyFile(fmt.Sprintf("%s/%s", testCfgDir, updateCfgFile), updatedTempCfgFile) @@ -292,13 +292,13 @@ func TestGetConfig(t *testing.T) { require.NoError(t, err) // Initialize environment with specified configs - cleanEnv(t, tempCfgFile, fmt.Sprintf("%s/%s", curDir, tempDynamicCfgFile)) + cleanEnv(t, tempCfgFile, fmt.Sprintf("%s/%s", curDir, tempDynamicCfgFile), true) envTags := "env tags" - setEnvVariable(t, ServerHost, updatedServerHost) - setEnvVariable(t, LogLevel, updatedLogLevel) - setEnvVariable(t, LogPath, updatedLogPath) - setEnvVariable(t, TagsKey, envTags) + setEnvVariable(t, EnvPrefix, ServerHost, updatedServerHost) + setEnvVariable(t, EnvPrefix, LogLevel, updatedLogLevel) + setEnvVariable(t, EnvPrefix, LogPath, updatedLogPath) + setEnvVariable(t, EnvPrefix, TagsKey, envTags) config, err := GetConfig("5678") require.NoError(t, err) @@ -331,13 +331,13 @@ func TestGetConfig(t *testing.T) { require.NoError(t, err) // Initialize environment with the empty configs - cleanEnv(t, tempCfgFile, fmt.Sprintf("%s/%s", curDir, tempDynamicCfgFile)) + cleanEnv(t, tempCfgFile, fmt.Sprintf("%s/%s", curDir, tempDynamicCfgFile), true) envTags := "env tags" - setEnvVariable(t, ServerHost, updatedServerHost) - setEnvVariable(t, LogLevel, updatedLogLevel) - setEnvVariable(t, LogPath, updatedLogPath) - setEnvVariable(t, TagsKey, envTags) + setEnvVariable(t, EnvPrefix, ServerHost, updatedServerHost) + setEnvVariable(t, EnvPrefix, LogLevel, updatedLogLevel) + setEnvVariable(t, EnvPrefix, LogPath, updatedLogPath) + setEnvVariable(t, EnvPrefix, TagsKey, envTags) config, err := GetConfig("5678") require.NoError(t, err) @@ -370,7 +370,7 @@ extensions: require.NoError(t, err) // Initialize environment with specified configs - cleanEnv(t, tempCfgFile, fmt.Sprintf("%s/%s", curDir, tempDynamicCfgFile)) + cleanEnv(t, tempCfgFile, fmt.Sprintf("%s/%s", curDir, tempDynamicCfgFile), true) config, err := GetConfig("5678") require.NoError(t, err) @@ -394,7 +394,7 @@ func TestUpdateAgentConfig(t *testing.T) { }() require.NoError(t, err) - cleanEnv(t, "empty_config.conf", fmt.Sprintf("%s/%s", curDir, tempDynamicCfgFile)) + cleanEnv(t, "empty_config.conf", fmt.Sprintf("%s/%s", curDir, tempDynamicCfgFile), true) // Get the current config so we can correctly set a few testcase variables curConf, err := GetConfig("12345") @@ -499,8 +499,11 @@ func TestDeprecatedEnvPrefixMigration(t *testing.T) { }() require.NoError(t, err) - cleanEnv(t, tempCfgFile, fmt.Sprintf("%s/%s", curDir, tempDynamicCfgFile)) - setEnvVariable(t, "tls_skip_verify", "true") + cleanEnv(t, tempCfgFile, fmt.Sprintf("%s/%s", curDir, tempDynamicCfgFile), false) + setEnvVariable(t, LegacyEnvPrefix, "tls_skip_verify", "true") + setEnvVariable(t, EnvPrefix, "tls_skip_verify", "") + + RegisterFlags() config, err := GetConfig("1234") require.NoError(t, err) @@ -509,19 +512,26 @@ func TestDeprecatedEnvPrefixMigration(t *testing.T) { assert.Equal(t, want, got) } -func setEnvVariable(t *testing.T, name string, value string) { - key := strings.ToUpper(EnvPrefix + agent_config.KeyDelimiter + name) - err := os.Setenv(key, value) +func setEnvVariable(t *testing.T, prefix, name, value string) { + var err error + key := strings.ToUpper(prefix + agent_config.KeyDelimiter + name) + if value == "" { + err = os.Unsetenv(key) + } else { + err = os.Setenv(key, value) + } require.NoError(t, err) } -func cleanEnv(t *testing.T, confFileName, dynamicConfFileAbsPath string) { +func cleanEnv(t *testing.T, confFileName, dynamicConfFileAbsPath string, register bool) { os.Clearenv() ROOT_COMMAND.ResetFlags() ROOT_COMMAND.ResetCommands() Viper = viper.NewWithOptions(viper.KeyDelimiter(agent_config.KeyDelimiter)) SetDefaults() - RegisterFlags() + if register { + RegisterFlags() + } cfg, err := RegisterConfigFile(dynamicConfFileAbsPath, confFileName, searchPaths...) require.NoError(t, err) From 683b128ead3fa8195ab093e9d798dde0634c75fd Mon Sep 17 00:00:00 2001 From: Oliver O'Mahony Date: Mon, 24 Jun 2024 14:48:04 +0100 Subject: [PATCH 13/13] env tidy up --- src/core/config/config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/config/config_test.go b/src/core/config/config_test.go index a5423c4367..d9a74f72d2 100644 --- a/src/core/config/config_test.go +++ b/src/core/config/config_test.go @@ -518,7 +518,7 @@ func setEnvVariable(t *testing.T, prefix, name, value string) { if value == "" { err = os.Unsetenv(key) } else { - err = os.Setenv(key, value) + t.Setenv(key, value) } require.NoError(t, err) }