From 461fc415f39c7e7d1c3d52cb0e17a815b057e12e Mon Sep 17 00:00:00 2001 From: Pavlo Golub Date: Mon, 4 May 2026 17:59:42 +0200 Subject: [PATCH] [!] refactor config handling and remove deprecated parameters - drop support for deprecated config parameter names - use local viper instance instead of global singleton - use local `pflag.FlagSet` instead of `pflag.CommandLine` - extract `newConfig(args)` to decouple config loading from `os.Args` - add unit tests for `vipconfig` --- vipconfig/config.go | 243 +++++++---------- vipconfig/config_test.go | 555 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 644 insertions(+), 154 deletions(-) create mode 100644 vipconfig/config_test.go diff --git a/vipconfig/config.go b/vipconfig/config.go index bac19c6..64890a9 100644 --- a/vipconfig/config.go +++ b/vipconfig/config.go @@ -45,153 +45,81 @@ type Config struct { Logger *zap.Logger } -func defineFlags() { +func defineFlags() *pflag.FlagSet { // When adding new flags here, consider adding them to the Config struct above // and then make sure to insert them into the conf instance in NewConfig down below. - pflag.String("config", "", "Location of the configuration file.") - pflag.Bool("version", false, "Show the version number.") + flags := pflag.NewFlagSet("vip-manager", pflag.ContinueOnError) - pflag.String("ip", "", "Virtual IP address to configure.") - pflag.String("netmask", "", "The netmask used for the IP address. Defaults to -1 which assigns ipv4 default mask.") - pflag.String("interface", "", "Network interface to configure on .") + flags.String("config", "", "Location of the configuration file.") + flags.Bool("version", false, "Show the version number.") - pflag.String("trigger-key", "", "Key in the DCS to monitor, e.g. \"/service/batman/leader\".") - pflag.String("trigger-value", "", "Value to monitor for.") + flags.String("ip", "", "Virtual IP address to configure.") + flags.String("netmask", "", "The netmask used for the IP address. Defaults to -1 which assigns ipv4 default mask.") + flags.String("interface", "", "Network interface to configure on .") - pflag.String("dcs-type", "etcd", "Type of endpoint used for key storage. Supported values: etcd, consul, patroni.") - // note: can't put a default value into dcs-endpoints as that would mess with applying default localhost when using consul - pflag.String("dcs-endpoints", "", "DCS endpoint(s), separate multiple endpoints using commas. (default \"http://127.0.0.1:2379\", \"http://127.0.0.1:8500\" or \"http://127.0.0.1:8008/\" depending on dcs-type.)") - pflag.String("etcd-user", "", "Username for etcd DCS endpoints.") - pflag.String("etcd-password", "", "Password for etcd DCS endpoints.") - pflag.String("etcd-ca-file", "", "Trusted CA certificate for the etcd server.") - pflag.String("etcd-cert-file", "", "Client certificate used for authentiaction with etcd.") - pflag.String("etcd-key-file", "", "Private key matching etcd-cert-file to decrypt messages sent from etcd.") + flags.String("trigger-key", "", "Key in the DCS to monitor, e.g. \"/service/batman/leader\".") + flags.String("trigger-value", "", "Value to monitor for.") - pflag.String("consul-token", "", "Token for consul DCS endpoints.") + flags.String("dcs-type", "etcd", "Type of endpoint used for key storage. Supported values: etcd, consul, patroni.") + // note: can't put a default value into dcs-endpoints as that would mess with applying default localhost when using consul + flags.String("dcs-endpoints", "", "DCS endpoint(s), separate multiple endpoints using commas. (default \"http://127.0.0.1:2379\", \"http://127.0.0.1:8500\" or \"http://127.0.0.1:8008/\" depending on dcs-type.)") + flags.String("etcd-user", "", "Username for etcd DCS endpoints.") + flags.String("etcd-password", "", "Password for etcd DCS endpoints.") + flags.String("etcd-ca-file", "", "Trusted CA certificate for the etcd server.") + flags.String("etcd-cert-file", "", "Client certificate used for authentiaction with etcd.") + flags.String("etcd-key-file", "", "Private key matching etcd-cert-file to decrypt messages sent from etcd.") - pflag.Int("interval", 1000, "DCS scan interval in milliseconds.") - pflag.String("manager-type", "basic", "Type of VIP-management to be used. Supported values: basic, hetzner.") + flags.String("consul-token", "", "Token for consul DCS endpoints.") - pflag.Int("retry-after", 250, "Time to wait before retrying interactions with outside components in milliseconds.") - pflag.Int("retry-num", 3, "Number of times interactions with outside components are retried.") + flags.Int("interval", 1000, "DCS scan interval in milliseconds.") + flags.String("manager-type", "basic", "Type of VIP-management to be used. Supported values: basic, hetzner.") - pflag.Bool("verbose", false, "Be verbose. Currently only implemented for manager-type=hetzner .") + flags.Int("retry-after", 250, "Time to wait before retrying interactions with outside components in milliseconds.") + flags.Int("retry-num", 3, "Number of times interactions with outside components are retried.") - pflag.CommandLine.SortFlags = false -} - -func mapDeprecated() error { - deprecated := map[string]string{ - // "deprecated" : "new", - "mask": "netmask", - "iface": "interface", - "key": "trigger-key", - "nodename": "trigger-value", - "etcd_user": "etcd-user", - "etcd_password": "etcd-password", - "type": "dcs-type", - "endpoint": "dcs-endpoints", - "endpoints": "dcs-endpoints", - "hostingtype": "manager-type", - "hosting_type": "manager-type", - "endpoint_type": "dcs-type", - "retry_num": "retry-num", - "retry_after": "retry-after", - "consul_token": "consul-token", - "host": "trigger-value", - } - - complaints := []string{} - errors := false - for k, v := range deprecated { - if viper.IsSet(k) { - - if _, exists := os.LookupEnv("VIP_" + strings.ToUpper(k)); !exists { - // using deprecated key in config file (as not exists in ENV) - complaints = append(complaints, fmt.Sprintf("Parameter \"%s\" has been deprecated, please use \"%s\" instead", k, v)) - } else { - if strings.ReplaceAll(k, "_", "-") != v { - // this string is not a direct replacement (e.g. etcd-user replaces etcd-user, i.e. in both cases VIP_ETCD_USER is the valid env key) - // for example, complain about VIP_IFACE, but not VIP_CONSUL_TOKEN or VIP_ETCD_USER... - complaints = append(complaints, fmt.Sprintf("Parameter \"%s\" has been deprecated, please use \"%s\" instead", "VIP_"+strings.ToUpper(k), "VIP_"+strings.ReplaceAll(strings.ToUpper(v), "-", "_"))) - } else { - continue - } - } - - if viper.IsSet(v) { - // don't forget to reset the desired replacer when exiting - replacer := strings.NewReplacer("-", "_") - defer viper.SetEnvKeyReplacer(replacer) - - // Check if there is only a collision because ENV vars always use _ instead of - and the deprecated mapping only maps from *_* to *-*. - testReplacer := strings.NewReplacer("", "") // just don't replace anything - viper.SetEnvKeyReplacer(testReplacer) - if viper.IsSet(v) { - complaints = append(complaints, fmt.Sprintf("Conflicting settings: %s or %s and %s or %s are both specified…", k, "VIP_"+strings.ToUpper(k), v, "VIP_"+strings.ReplaceAll(strings.ToUpper(v), "-", "_"))) - - if viper.Get(k) == viper.Get(v) { - complaints = append(complaints, fmt.Sprintf("… But no conflicting values: %s and %s are equal…ignoring.", viper.GetString(k), viper.GetString(v))) - continue - } - complaints = append(complaints, fmt.Sprintf("…conflicting values: %s and %s", viper.GetString(k), viper.GetString(v))) - errors = true - continue - } - } - // if this is a valid mapping due to deprecation, set the new key explicitly to the value of the deprecated key. - viper.Set(v, viper.Get(k)) - // "unset" the deprecated setting so it will not show up in our config later - viper.Set(k, "") + flags.Bool("verbose", false, "Be verbose. Currently only implemented for manager-type=hetzner .") - } - } - for c := range complaints { - fmt.Println(complaints[c]) - } - if errors { - panic("Cannot continue due to conflicts.") - } - return nil + flags.SortFlags = false + return flags } -func setDefaults() { +func setDefaults(v *viper.Viper) { defaults := map[string]any{ - "hostingtype": "basic", - "dcs-type": "etcd", - "interval": 1000, - "retry-after": 250, - "retry-num": 3, + "manager-type": "basic", + "dcs-type": "etcd", + "interval": 1000, + "retry-after": 250, + "retry-num": 3, } - for k, v := range defaults { - if !viper.IsSet(k) { - viper.SetDefault(k, v) + for k, val := range defaults { + if !v.IsSet(k) { + v.SetDefault(k, val) } } // apply defaults for endpoints - if !viper.IsSet("dcs-endpoints") { + if !v.IsSet("dcs-endpoints") { fmt.Println("No dcs-endpoints specified, trying to use localhost with standard ports!") - switch viper.GetString("dcs-type") { + switch v.GetString("dcs-type") { case "consul": - viper.Set("dcs-endpoints", []string{"http://127.0.0.1:8500"}) + v.Set("dcs-endpoints", []string{"http://127.0.0.1:8500"}) case "etcd", "etcd3": - viper.Set("dcs-endpoints", []string{"http://127.0.0.1:2379"}) + v.Set("dcs-endpoints", []string{"http://127.0.0.1:2379"}) case "patroni": - viper.Set("dcs-endpoints", []string{"http://127.0.0.1:8008/"}) + v.Set("dcs-endpoints", []string{"http://127.0.0.1:8008/"}) } } // set trigger-key to '/leader' if DCS type is patroni and nothing is specified - if viper.GetString("trigger-key") == "" && viper.GetString("dcs-type") == "patroni" { - viper.Set("trigger-key", "/leader") + if v.GetString("trigger-key") == "" && v.GetString("dcs-type") == "patroni" { + v.Set("trigger-key", "/leader") } // set trigger-value to default value if nothing is specified - if triggerValue := viper.GetString("trigger-value"); triggerValue == "" { + if triggerValue := v.GetString("trigger-value"); triggerValue == "" { var err error - if viper.GetString("dcs-type") == "patroni" { + if v.GetString("dcs-type") == "patroni" { triggerValue = "200" } else { triggerValue, err = os.Hostname() @@ -200,25 +128,25 @@ func setDefaults() { fmt.Printf("No trigger-value specified, hostname could not be retrieved: %s", err) } else { fmt.Printf("No trigger-value specified, instead using: %v", triggerValue) - viper.Set("trigger-value", triggerValue) + v.Set("trigger-value", triggerValue) } } // set retry-num to default if not set or set to zero - if retryNum := viper.GetInt("retry-num"); retryNum <= 0 { - viper.Set("retry-num", 3) + if retryNum := v.GetInt("retry-num"); retryNum <= 0 { + v.Set("retry-num", 3) } } -func checkSetting(name string) bool { - if !viper.IsSet(name) { +func checkSetting(v *viper.Viper, name string) bool { + if !v.IsSet(name) { fmt.Printf("Setting %s is mandatory", name) return false } return true } -func checkMandatory() error { +func checkMandatory(v *viper.Viper) error { mandatory := []string{ "ip", "netmask", @@ -228,18 +156,18 @@ func checkMandatory() error { "dcs-endpoints", } success := true - for _, v := range mandatory { - success = checkSetting(v) && success + for _, name := range mandatory { + success = checkSetting(v, name) && success } if !success { return errors.New("one or more mandatory settings were not set") } - return checkImpliedMandatory() + return checkImpliedMandatory(v) } // if reason is set, but implied is not set, return false. -func checkImpliedSetting(implied string, reason string) bool { - if viper.IsSet(reason) && !viper.IsSet(implied) { +func checkImpliedSetting(v *viper.Viper, implied string, reason string) bool { + if v.IsSet(reason) && !v.IsSet(implied) { fmt.Printf("Setting %s is mandatory when setting %s is specified.", implied, reason) return false } @@ -247,7 +175,7 @@ func checkImpliedSetting(implied string, reason string) bool { } // Some settings imply that another setting must be set as well. -func checkImpliedMandatory() error { +func checkImpliedMandatory(v *viper.Viper) error { mandatory := map[string]string{ // "implied" : "reason" "etcd-user": "etcd-password", @@ -255,8 +183,8 @@ func checkImpliedMandatory() error { "etcd-ca-file": "etcd-cert-file", } success := true - for k, v := range mandatory { - success = checkImpliedSetting(k, v) && success + for k, reason := range mandatory { + success = checkImpliedSetting(v, k, reason) && success } if !success { return errors.New("one or more implied mandatory settings were not set") @@ -264,18 +192,18 @@ func checkImpliedMandatory() error { return nil } -func printSettings() { +func printSettings(v *viper.Viper) { s := []string{} - for k, v := range viper.AllSettings() { - if v != "" { + for k, val := range v.AllSettings() { + if val != "" { switch k { case "etcd-password": fallthrough case "consul-token": s = append(s, fmt.Sprintf("\t%s : *****\n", k)) default: - s = append(s, fmt.Sprintf("\t%s : %v\n", k, v)) + s = append(s, fmt.Sprintf("\t%s : %v\n", k, val)) } } } @@ -287,34 +215,41 @@ func printSettings() { } } -func loadConfigFile() error { - if viper.IsSet("config") { - viper.SetConfigFile(viper.GetString("config")) - if err := viper.ReadInConfig(); err != nil { +func loadConfigFile(v *viper.Viper) error { + if v.IsSet("config") { + v.SetConfigFile(v.GetString("config")) + if err := v.ReadInConfig(); err != nil { return err } - fmt.Printf("Using config from file: %s\n", viper.ConfigFileUsed()) + fmt.Printf("Using config from file: %s\n", v.ConfigFileUsed()) } - return mapDeprecated() + return nil } // NewConfig returns a new Config instance func NewConfig() (*Config, error) { + return newConfig(os.Args[1:]) +} + +func newConfig(args []string) (*Config, error) { var err error - defineFlags() - pflag.Parse() + v := viper.New() + flags := defineFlags() + if err = flags.Parse(args); err != nil { + return nil, fmt.Errorf("error parsing flags: %w", err) + } // import pflags into viper - _ = viper.BindPFlags(pflag.CommandLine) + _ = v.BindPFlags(flags) // make viper look for env variables that are prefixed VIP_... - // e.g.: viper.getString("ip") will return the value of env variable VIP_IP - viper.SetEnvPrefix("vip") - viper.AutomaticEnv() + // e.g.: v.GetString("ip") will return the value of env variable VIP_IP + v.SetEnvPrefix("vip") + v.AutomaticEnv() //replace dashes (in flags) with underscores (in ENV vars) - // so that e.g. viper.GetString("dcs-endpoints") will return value of VIP_DCS_ENDPOINTS + // so that e.g. v.GetString("dcs-endpoints") will return value of VIP_DCS_ENDPOINTS replacer := strings.NewReplacer("-", "_") - viper.SetEnvKeyReplacer(replacer) + v.SetEnvKeyReplacer(replacer) // viper precedence order // - explicit call to Set @@ -325,26 +260,26 @@ func NewConfig() (*Config, error) { // - default // if a configfile has been passed, make viper read it - if err = loadConfigFile(); err != nil { + if err = loadConfigFile(v); err != nil { return nil, fmt.Errorf("fatal error reading config file: %w", err) } // convert string of csv to String Slice - if endpointsString := viper.GetString("dcs-endpoints"); endpointsString != "" && strings.Contains(endpointsString, ",") { - viper.Set("dcs-endpoints", strings.Split(endpointsString, ",")) + if endpointsString := v.GetString("dcs-endpoints"); endpointsString != "" && strings.Contains(endpointsString, ",") { + v.Set("dcs-endpoints", strings.Split(endpointsString, ",")) } - setDefaults() - if err = checkMandatory(); err != nil { + setDefaults(v) + if err = checkMandatory(v); err != nil { return nil, err } conf := &Config{} - if err = viper.Unmarshal(conf); err != nil { + if err = v.Unmarshal(conf); err != nil { zap.L().Fatal("unable to decode viper config into config struct, %v", zap.Error(err)) } conf.initLogger() - printSettings() + printSettings(v) return conf, nil } diff --git a/vipconfig/config_test.go b/vipconfig/config_test.go new file mode 100644 index 0000000..b111078 --- /dev/null +++ b/vipconfig/config_test.go @@ -0,0 +1,555 @@ +package vipconfig + +import ( + "bytes" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/spf13/viper" +) + +// --------------------------------------------------------------------------- +// checkSetting +// --------------------------------------------------------------------------- + +func TestCheckSetting_NotSet(t *testing.T) { + v := viper.New() + if checkSetting(v, "nonexistent-key") { + t.Error("expected false for unset key") + } +} + +func TestCheckSetting_Set(t *testing.T) { + v := viper.New() + v.Set("some-key", "value") + if !checkSetting(v, "some-key") { + t.Error("expected true for set key") + } +} + +// --------------------------------------------------------------------------- +// checkImpliedSetting +// --------------------------------------------------------------------------- + +func TestCheckImpliedSetting_ReasonNotSet(t *testing.T) { + v := viper.New() + // reason not set => implied is not required + if !checkImpliedSetting(v, "etcd-user", "etcd-password") { + t.Error("expected true when reason is not set") + } +} + +func TestCheckImpliedSetting_BothSet(t *testing.T) { + v := viper.New() + v.Set("etcd-password", "secret") + v.Set("etcd-user", "admin") + if !checkImpliedSetting(v, "etcd-user", "etcd-password") { + t.Error("expected true when both implied and reason are set") + } +} + +func TestCheckImpliedSetting_ReasonSetImpliedMissing(t *testing.T) { + v := viper.New() + v.Set("etcd-password", "secret") + // etcd-user not set + if checkImpliedSetting(v, "etcd-user", "etcd-password") { + t.Error("expected false when reason is set but implied is not") + } +} + +// --------------------------------------------------------------------------- +// checkMandatory +// --------------------------------------------------------------------------- + +func TestCheckMandatory_AllMissing(t *testing.T) { + v := viper.New() + if err := checkMandatory(v); err == nil { + t.Error("expected error when all mandatory settings are missing") + } +} + +func TestCheckMandatory_PartiallySet(t *testing.T) { + v := viper.New() + v.Set("ip", "10.0.0.1") + v.Set("netmask", 24) + // interface, trigger-key, trigger-value, dcs-endpoints still missing + if err := checkMandatory(v); err == nil { + t.Error("expected error when some mandatory settings are missing") + } +} + +func TestCheckMandatory_AllSet(t *testing.T) { + v := viper.New() + v.Set("ip", "10.0.0.1") + v.Set("netmask", 24) + v.Set("interface", "eth0") + v.Set("trigger-key", "/service/pgcluster/leader") + v.Set("trigger-value", "host1") + v.Set("dcs-endpoints", []string{"http://127.0.0.1:2379"}) + if err := checkMandatory(v); err != nil { + t.Errorf("expected no error, got: %v", err) + } +} + +// --------------------------------------------------------------------------- +// checkImpliedMandatory +// --------------------------------------------------------------------------- + +func TestCheckImpliedMandatory_NoneSet(t *testing.T) { + v := viper.New() + if err := checkImpliedMandatory(v); err != nil { + t.Errorf("expected no error, got: %v", err) + } +} + +func TestCheckImpliedMandatory_EtcdPasswordWithoutUser(t *testing.T) { + v := viper.New() + v.Set("etcd-password", "secret") + if err := checkImpliedMandatory(v); err == nil { + t.Error("expected error: etcd-password set without etcd-user") + } +} + +func TestCheckImpliedMandatory_EtcdCertFileWithoutKeyFile(t *testing.T) { + v := viper.New() + v.Set("etcd-cert-file", "/path/to/cert") + if err := checkImpliedMandatory(v); err == nil { + t.Error("expected error: etcd-cert-file set without etcd-key-file") + } +} + +func TestCheckImpliedMandatory_EtcdCertFileWithoutCAFile(t *testing.T) { + v := viper.New() + v.Set("etcd-cert-file", "/path/to/cert") + v.Set("etcd-key-file", "/path/to/key") + // etcd-ca-file still missing + if err := checkImpliedMandatory(v); err == nil { + t.Error("expected error: etcd-cert-file set without etcd-ca-file") + } +} + +func TestCheckImpliedMandatory_AllEtcdTLSSet(t *testing.T) { + v := viper.New() + v.Set("etcd-user", "admin") + v.Set("etcd-password", "secret") + v.Set("etcd-cert-file", "/path/to/cert") + v.Set("etcd-key-file", "/path/to/key") + v.Set("etcd-ca-file", "/path/to/ca") + if err := checkImpliedMandatory(v); err != nil { + t.Errorf("expected no error, got: %v", err) + } +} + +// --------------------------------------------------------------------------- +// setDefaults +// --------------------------------------------------------------------------- + +func TestSetDefaults_DcsTypeDefaultsToEtcd(t *testing.T) { + v := viper.New() + v.Set("trigger-value", "host1") + setDefaults(v) + if got := v.GetString("dcs-type"); got != "etcd" { + t.Errorf("expected dcs-type=etcd, got %q", got) + } +} + +func TestSetDefaults_ManagerTypeDefaultsToBasic(t *testing.T) { + v := viper.New() + v.Set("trigger-value", "host1") + setDefaults(v) + if got := v.GetString("manager-type"); got != "basic" { + t.Errorf("expected manager-type=basic, got %q", got) + } +} + +func TestSetDefaults_EndpointsDefaultEtcd(t *testing.T) { + v := viper.New() + v.Set("trigger-value", "host1") + setDefaults(v) + endpoints := v.GetStringSlice("dcs-endpoints") + if len(endpoints) == 0 || endpoints[0] != "http://127.0.0.1:2379" { + t.Errorf("expected etcd default endpoint, got %v", endpoints) + } +} + +func TestSetDefaults_EndpointsDefaultConsul(t *testing.T) { + v := viper.New() + v.Set("dcs-type", "consul") + v.Set("trigger-value", "host1") + setDefaults(v) + endpoints := v.GetStringSlice("dcs-endpoints") + if len(endpoints) == 0 || endpoints[0] != "http://127.0.0.1:8500" { + t.Errorf("expected consul default endpoint, got %v", endpoints) + } +} + +func TestSetDefaults_EndpointsDefaultPatroni(t *testing.T) { + v := viper.New() + v.Set("dcs-type", "patroni") + v.Set("trigger-value", "host1") + setDefaults(v) + endpoints := v.GetStringSlice("dcs-endpoints") + if len(endpoints) == 0 || endpoints[0] != "http://127.0.0.1:8008/" { + t.Errorf("expected patroni default endpoint, got %v", endpoints) + } +} + +func TestSetDefaults_PatroniTriggerKeyDefault(t *testing.T) { + v := viper.New() + v.Set("dcs-type", "patroni") + v.Set("trigger-value", "host1") + setDefaults(v) + if got := v.GetString("trigger-key"); got != "/leader" { + t.Errorf("expected trigger-key=/leader for patroni, got %q", got) + } +} + +func TestSetDefaults_PatroniTriggerValueDefault(t *testing.T) { + v := viper.New() + v.Set("dcs-type", "patroni") + setDefaults(v) + if got := v.GetString("trigger-value"); got != "200" { + t.Errorf("expected trigger-value=200 for patroni, got %q", got) + } +} + +func TestSetDefaults_TriggerValueFallsBackToHostname(t *testing.T) { + v := viper.New() + // dcs-type will default to etcd; no trigger-value set + setDefaults(v) + hostname, err := os.Hostname() + if err != nil { + t.Skip("hostname unavailable, skipping") + } + if got := v.GetString("trigger-value"); got != hostname { + t.Errorf("expected trigger-value=%q (hostname), got %q", hostname, got) + } +} + +func TestSetDefaults_ExplicitEndpointsNotOverridden(t *testing.T) { + v := viper.New() + v.Set("dcs-endpoints", []string{"http://192.168.1.1:2379"}) + v.Set("trigger-value", "host1") + setDefaults(v) + endpoints := v.GetStringSlice("dcs-endpoints") + if len(endpoints) == 0 || endpoints[0] != "http://192.168.1.1:2379" { + t.Errorf("explicit endpoint should not be overridden, got %v", endpoints) + } +} + +func TestSetDefaults_RetryNumZeroResetsToDefault(t *testing.T) { + v := viper.New() + v.Set("trigger-value", "host1") + v.Set("retry-num", 0) + setDefaults(v) + if got := v.GetInt("retry-num"); got != 3 { + t.Errorf("expected retry-num=3 when set to 0, got %d", got) + } +} + +// --------------------------------------------------------------------------- +// loadConfigFile +// --------------------------------------------------------------------------- + +func TestLoadConfigFile_ConfigKeyNotSet(t *testing.T) { + v := viper.New() + if err := loadConfigFile(v); err != nil { + t.Errorf("expected nil when config key is not set, got: %v", err) + } +} + +func TestLoadConfigFile_NonexistentFile(t *testing.T) { + v := viper.New() + v.Set("config", "/nonexistent/path/config.yml") + if err := loadConfigFile(v); err == nil { + t.Error("expected error for nonexistent config file") + } +} + +func TestLoadConfigFile_ValidYAML(t *testing.T) { + v := viper.New() + content := []byte("ip: 10.0.0.1\nnetmask: 24\ninterface: eth0\n") + path := filepath.Join(t.TempDir(), "config.yml") + if err := os.WriteFile(path, content, 0600); err != nil { + t.Fatal(err) + } + v.Set("config", path) + if err := loadConfigFile(v); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := v.GetString("ip"); got != "10.0.0.1" { + t.Errorf("expected ip=10.0.0.1, got %q", got) + } + if got := v.GetInt("netmask"); got != 24 { + t.Errorf("expected netmask=24, got %d", got) + } +} + +func TestLoadConfigFile_InvalidYAML(t *testing.T) { + v := viper.New() + content := []byte("this: is: not: valid: yaml: :\n") + path := filepath.Join(t.TempDir(), "bad.yml") + if err := os.WriteFile(path, content, 0600); err != nil { + t.Fatal(err) + } + v.Set("config", path) + if err := loadConfigFile(v); err == nil { + t.Error("expected error for invalid YAML") + } +} + +// --------------------------------------------------------------------------- +// printSettings +// --------------------------------------------------------------------------- + +// captureStdout redirects os.Stdout for the duration of fn and returns +// everything written to it. +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe: %v", err) + } + old := os.Stdout + os.Stdout = w + defer func() { os.Stdout = old }() + + fn() + + _ = w.Close() + var buf bytes.Buffer + if _, err := io.Copy(&buf, r); err != nil { + t.Fatalf("io.Copy: %v", err) + } + return buf.String() +} + +func TestPrintSettings_MasksSensitiveValues(t *testing.T) { + v := viper.New() + v.Set("etcd-password", "supersecret") + v.Set("consul-token", "mytoken") + v.Set("ip", "10.0.0.1") + + out := captureStdout(t, func() { printSettings(v) }) + + if strings.Contains(out, "supersecret") { + t.Error("etcd-password value should be masked") + } + if strings.Contains(out, "mytoken") { + t.Error("consul-token value should be masked") + } + if !strings.Contains(out, "*****") { + t.Error("expected masked placeholder '*****' in output") + } + if !strings.Contains(out, "10.0.0.1") { + t.Error("expected non-sensitive ip value to appear in output") + } +} + +func TestPrintSettings_EmptySettings(t *testing.T) { + v := viper.New() + // should not panic with no settings + captureStdout(t, func() { printSettings(v) }) +} + +// --------------------------------------------------------------------------- +// initLogger +// --------------------------------------------------------------------------- + +func TestInitLogger_NonVerbose(t *testing.T) { + conf := &Config{Verbose: false} + conf.initLogger() + if conf.Logger == nil { + t.Fatal("expected non-nil logger") + } + _ = conf.Logger.Sync() +} + +func TestInitLogger_Verbose(t *testing.T) { + conf := &Config{Verbose: true} + conf.initLogger() + if conf.Logger == nil { + t.Fatal("expected non-nil logger") + } + _ = conf.Logger.Sync() +} + +// --------------------------------------------------------------------------- +// defineFlags +// --------------------------------------------------------------------------- + +func TestDefineFlags_AllFlagsPresent(t *testing.T) { + expected := []string{ + "config", "version", + "ip", "netmask", "interface", + "trigger-key", "trigger-value", + "dcs-type", "dcs-endpoints", + "etcd-user", "etcd-password", "etcd-ca-file", "etcd-cert-file", "etcd-key-file", + "consul-token", + "interval", "manager-type", + "retry-after", "retry-num", + "verbose", + } + flags := defineFlags() + for _, name := range expected { + if flags.Lookup(name) == nil { + t.Errorf("expected flag %q to be defined", name) + } + } +} + +func TestDefineFlags_Defaults(t *testing.T) { + flags := defineFlags() + cases := []struct { + flag string + want string + }{ + {"dcs-type", "etcd"}, + {"manager-type", "basic"}, + {"interval", "1000"}, + {"retry-after", "250"}, + {"retry-num", "3"}, + {"verbose", "false"}, + {"version", "false"}, + } + for _, tc := range cases { + if got := flags.Lookup(tc.flag).DefValue; got != tc.want { + t.Errorf("flag %q default: got %q, want %q", tc.flag, got, tc.want) + } + } +} + +func TestDefineFlags_SortFlagsDisabled(t *testing.T) { + flags := defineFlags() + if flags.SortFlags { + t.Error("expected SortFlags=false") + } +} + +func TestDefineFlags_ParseValues(t *testing.T) { + flags := defineFlags() + args := []string{"--ip=10.0.0.1", "--netmask=24", "--dcs-type=consul"} + if err := flags.Parse(args); err != nil { + t.Fatalf("Parse: %v", err) + } + if got, _ := flags.GetString("ip"); got != "10.0.0.1" { + t.Errorf("ip: got %q, want 10.0.0.1", got) + } + if got, _ := flags.GetString("netmask"); got != "24" { + t.Errorf("netmask: got %q, want \"24\"", got) + } + if got, _ := flags.GetString("dcs-type"); got != "consul" { + t.Errorf("dcs-type: got %q, want consul", got) + } +} + +// --------------------------------------------------------------------------- +// newConfig +// --------------------------------------------------------------------------- + +// minimalConfigFile writes a valid config YAML to a temp file and returns its path. +func minimalConfigFile(t *testing.T, extra ...string) string { + t.Helper() + base := ` +ip: 10.0.0.1 +netmask: 24 +interface: eth0 +trigger-key: /service/pgcluster/leader +trigger-value: host1 +dcs-type: etcd +dcs-endpoints: + - http://127.0.0.1:2379 +` + content := base + strings.Join(extra, "\n") + path := filepath.Join(t.TempDir(), "vip-manager.yml") + if err := os.WriteFile(path, []byte(content), 0600); err != nil { + t.Fatal(err) + } + return path +} + +func TestNewConfig_ValidConfigFile(t *testing.T) { + path := minimalConfigFile(t) + conf, err := newConfig([]string{fmt.Sprintf("--config=%s", path)}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if conf.IP != "10.0.0.1" { + t.Errorf("IP: got %q, want 10.0.0.1", conf.IP) + } + if conf.Mask != 24 { + t.Errorf("Mask: got %d, want 24", conf.Mask) + } + if conf.Iface != "eth0" { + t.Errorf("Iface: got %q, want eth0", conf.Iface) + } + if conf.Logger == nil { + t.Error("expected non-nil logger") + } +} + +func TestNewConfig_FlagOverridesFile(t *testing.T) { + path := minimalConfigFile(t) + conf, err := newConfig([]string{ + fmt.Sprintf("--config=%s", path), + "--trigger-value=overridden", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if conf.TriggerValue != "overridden" { + t.Errorf("TriggerValue: got %q, want overridden", conf.TriggerValue) + } +} + +func TestNewConfig_MissingMandatory(t *testing.T) { + // no config file, no flags → mandatory settings missing + _, err := newConfig([]string{}) + if err == nil { + t.Error("expected error for missing mandatory settings") + } +} + +func TestNewConfig_NonexistentConfigFile(t *testing.T) { + _, err := newConfig([]string{"--config=/nonexistent/path.yml"}) + if err == nil { + t.Error("expected error for nonexistent config file") + } +} + +func TestNewConfig_CSVEndpoints(t *testing.T) { + path := minimalConfigFile(t) + conf, err := newConfig([]string{ + fmt.Sprintf("--config=%s", path), + "--dcs-endpoints=http://127.0.0.1:2379,http://127.0.0.2:2379", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(conf.Endpoints) != 2 { + t.Errorf("expected 2 endpoints, got %d: %v", len(conf.Endpoints), conf.Endpoints) + } +} + +func TestNewConfig_EnvVarOverride(t *testing.T) { + path := minimalConfigFile(t) + t.Setenv("VIP_TRIGGER_VALUE", "from-env") + conf, err := newConfig([]string{fmt.Sprintf("--config=%s", path)}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if conf.TriggerValue != "from-env" { + t.Errorf("TriggerValue: got %q, want from-env", conf.TriggerValue) + } +} + +func TestNewConfig_InvalidFlag(t *testing.T) { + _, err := newConfig([]string{"--nonexistent-flag=value"}) + if err == nil { + t.Error("expected error for unknown flag") + } +}