From 047a66e87d2241b9835e8ffcd6b7ab44facc9998 Mon Sep 17 00:00:00 2001 From: Timothy Messier Date: Mon, 12 Sep 2022 09:37:13 -0400 Subject: [PATCH] fix(worker): Improper reload when running as controller and worker (#2438) In cases where the server was started with both a controller and worker stanza, the reload of the config was not properly setting the initial upstream value on the new config. In this case the value is derived from other config values and not necessarily set directly. This now properly initializes the new config so that initial upstream is set in the same manner as the first load. Blame: 5d0cdf680f83bfc44f3c68237f087526911c944d --- internal/cmd/base/server_test.go | 310 --------------- internal/cmd/base/servers.go | 43 --- internal/cmd/commands/dev/dev.go | 2 +- internal/cmd/commands/server/server.go | 51 +-- internal/cmd/config/config.go | 121 ++++++ internal/cmd/config/config_test.go | 511 +++++++++++++++++++++++++ internal/daemon/controller/testing.go | 2 +- 7 files changed, 653 insertions(+), 387 deletions(-) diff --git a/internal/cmd/base/server_test.go b/internal/cmd/base/server_test.go index 8f9d63833..e450029df 100644 --- a/internal/cmd/base/server_test.go +++ b/internal/cmd/base/server_test.go @@ -286,316 +286,6 @@ func TestServer_AddEventerToContext(t *testing.T) { } } -func TestSetupControllerPublicClusterAddress(t *testing.T) { - t.Parallel() - tests := []struct { - name string - inputConfig *config.Config - inputFlagValue string - stateFn func(t *testing.T) - expErr bool - expErrStr string - expPublicClusterAddress string - }{ - { - name: "nil controller", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{}, - }, - Controller: nil, - }, - inputFlagValue: "", - expErr: false, - expErrStr: "", - expPublicClusterAddress: ":9201", - }, - { - name: "setting public cluster address directly with ip", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{}, - }, - Controller: &config.Controller{ - PublicClusterAddr: "127.0.0.1", - }, - }, - inputFlagValue: "", - expErr: false, - expErrStr: "", - expPublicClusterAddress: "127.0.0.1:9201", - }, - { - name: "setting public cluster address directly with ip:port", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{}, - }, - Controller: &config.Controller{ - PublicClusterAddr: "127.0.0.1:8080", - }, - }, - inputFlagValue: "", - expErr: false, - expErrStr: "", - expPublicClusterAddress: "127.0.0.1:8080", - }, - { - name: "setting public cluster address to env var", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{}, - }, - Controller: &config.Controller{ - PublicClusterAddr: "env://TEST_ENV_VAR_FOR_CONTROLLER_ADDR", - }, - }, - inputFlagValue: "", - stateFn: func(t *testing.T) { - t.Setenv("TEST_ENV_VAR_FOR_CONTROLLER_ADDR", "127.0.0.1:8080") - }, - expErr: false, - expErrStr: "", - expPublicClusterAddress: "127.0.0.1:8080", - }, - { - name: "setting public cluster address to env var that points to template", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{}, - }, - Controller: &config.Controller{ - PublicClusterAddr: "env://TEST_ENV_VAR_FOR_CONTROLLER_ADDR", - }, - }, - inputFlagValue: "", - stateFn: func(t *testing.T) { - t.Setenv("TEST_ENV_VAR_FOR_CONTROLLER_ADDR", `{{ GetAllInterfaces | include "flags" "loopback" | include "type" "IPV4" | join "address" " " }}`) - }, - expErr: false, - expErrStr: "", - expPublicClusterAddress: "127.0.0.1:9201", - }, - { - name: "setting public cluster address to ip template", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{}, - }, - Controller: &config.Controller{ - PublicClusterAddr: `{{ GetAllInterfaces | include "flags" "loopback" | include "type" "IPV4" | join "address" " " }}`, - }, - }, - inputFlagValue: "", - expErr: false, - expErrStr: "", - expPublicClusterAddress: "127.0.0.1:9201", - }, - { - name: "setting public cluster address to multiline ip template", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{}, - }, - Controller: &config.Controller{ - PublicClusterAddr: `{{ with $local := GetAllInterfaces | include "flags" "loopback" | include "type" "IPV4" -}} - {{- $local | join "address" " " -}} - {{- end }}`, - }, - }, - inputFlagValue: "", - expErr: false, - expErrStr: "", - expPublicClusterAddress: "127.0.0.1:9201", - }, - { - name: "using flag value with ip only", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{}, - }, - Controller: &config.Controller{}, - }, - inputFlagValue: "127.0.0.1", - expErr: false, - expErrStr: "", - expPublicClusterAddress: "127.0.0.1:9201", - }, - { - name: "using flag value with ip:port", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{}, - }, - Controller: &config.Controller{}, - }, - inputFlagValue: "127.0.0.1:8080", - expErr: false, - expErrStr: "", - expPublicClusterAddress: "127.0.0.1:8080", - }, - { - name: "using flag value with ip template", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{}, - }, - Controller: &config.Controller{}, - }, - inputFlagValue: `{{ GetAllInterfaces | include "flags" "loopback" | include "type" "IPV4" | join "address" " " }}`, - expErr: false, - expErrStr: "", - expPublicClusterAddress: "127.0.0.1:9201", - }, - { - name: "using flag value with multiline ip template", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{}, - }, - Controller: &config.Controller{}, - }, - inputFlagValue: `{{ with $local := GetAllInterfaces | include "flags" "loopback" | include "type" "IPV4" -}} - {{- $local | join "address" " " -}} - {{- end }}`, - expErr: false, - expErrStr: "", - expPublicClusterAddress: "127.0.0.1:9201", - }, - { - name: "using flag value to point to env var with ip only", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{}, - }, - Controller: &config.Controller{}, - }, - inputFlagValue: "env://TEST_ENV_VAR_FOR_CONTROLLER_ADDR", - stateFn: func(t *testing.T) { - t.Setenv("TEST_ENV_VAR_FOR_CONTROLLER_ADDR", "127.0.0.1") - }, - expErr: false, - expErrStr: "", - expPublicClusterAddress: "127.0.0.1:9201", - }, - { - name: "using flag value to point to env var with ip:port", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{}, - }, - Controller: &config.Controller{}, - }, - inputFlagValue: "env://TEST_ENV_VAR_FOR_CONTROLLER_ADDR", - stateFn: func(t *testing.T) { - t.Setenv("TEST_ENV_VAR_FOR_CONTROLLER_ADDR", "127.0.0.1:8080") - }, - expErr: false, - expErrStr: "", - expPublicClusterAddress: "127.0.0.1:8080", - }, - { - name: "read address from listeners ip only", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{ - {Purpose: []string{"cluster"}, Address: "127.0.0.1"}, - }, - }, - Controller: &config.Controller{}, - }, - expErr: false, - expErrStr: "", - expPublicClusterAddress: "127.0.0.1:9201", - }, - { - name: "read address from listeners ip:port", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{ - {Purpose: []string{"cluster"}, Address: "127.0.0.1:8080"}, - }, - }, - Controller: &config.Controller{}, - }, - expErr: false, - expErrStr: "", - expPublicClusterAddress: "127.0.0.1:8080", - }, - { - name: "read address from listeners is ignored on different purpose", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{ - {Purpose: []string{"somethingelse"}, Address: "127.0.0.1:8080"}, - }, - }, - Controller: &config.Controller{}, - }, - expErr: false, - expErrStr: "", - expPublicClusterAddress: ":9201", - }, - { - name: "using flag value to point to nonexistent file", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{}, - }, - Controller: &config.Controller{}, - }, - inputFlagValue: "file://this_doesnt_exist_for_sure", - expErr: true, - expErrStr: "Error parsing public cluster addr: error reading file at file://this_doesnt_exist_for_sure: open this_doesnt_exist_for_sure: no such file or directory", - expPublicClusterAddress: "", - }, - { - name: "using flag value to provoke error in SplitHostPort", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{}, - }, - Controller: &config.Controller{}, - }, - inputFlagValue: "abc::123", - expErr: true, - expErrStr: "Error splitting public cluster adddress host/port: address abc::123: too many colons in address", - expPublicClusterAddress: "", - }, - { - name: "bad ip template", - inputConfig: &config.Config{ - SharedConfig: &configutil.SharedConfig{ - Listeners: []*listenerutil.ListenerConfig{}, - }, - Controller: &config.Controller{}, - }, - inputFlagValue: "{{ somethingthatdoesntexist }}", - expErr: true, - expErrStr: "Error parsing IP template on controller public cluster addr: unable to parse address template \"{{ somethingthatdoesntexist }}\": unable to parse template \"{{ somethingthatdoesntexist }}\": template: sockaddr.Parse:1: function \"somethingthatdoesntexist\" not defined", - expPublicClusterAddress: "", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if tt.stateFn != nil { - tt.stateFn(t) - } - s := Server{} - err := s.SetupControllerPublicClusterAddress(tt.inputConfig, tt.inputFlagValue) - if tt.expErr { - require.EqualError(t, err, tt.expErrStr) - return - } - - require.NoError(t, err) - require.NotNil(t, tt.inputConfig.Controller) - require.Equal(t, tt.expPublicClusterAddress, tt.inputConfig.Controller.PublicClusterAddr) - }) - } -} - func TestSetupWorkerPublicAddress(t *testing.T) { t.Parallel() tests := []struct { diff --git a/internal/cmd/base/servers.go b/internal/cmd/base/servers.go index 316cd2237..c99b7659b 100644 --- a/internal/cmd/base/servers.go +++ b/internal/cmd/base/servers.go @@ -742,49 +742,6 @@ func (b *Server) DestroyDevDatabase(ctx context.Context) error { return nil } -func (b *Server) SetupControllerPublicClusterAddress(conf *config.Config, flagValue string) error { - if conf.Controller == nil { - conf.Controller = new(config.Controller) - } - if flagValue != "" { - conf.Controller.PublicClusterAddr = flagValue - } - if conf.Controller.PublicClusterAddr == "" { - FindAddr: - for _, listener := range conf.Listeners { - for _, purpose := range listener.Purpose { - if purpose == "cluster" { - conf.Controller.PublicClusterAddr = listener.Address - break FindAddr - } - } - } - } else { - var err error - conf.Controller.PublicClusterAddr, err = parseutil.ParsePath(conf.Controller.PublicClusterAddr) - if err != nil && !errors.Is(err, parseutil.ErrNotAUrl) { - return fmt.Errorf("Error parsing public cluster addr: %w", err) - } - - conf.Controller.PublicClusterAddr, err = listenerutil.ParseSingleIPTemplate(conf.Controller.PublicClusterAddr) - if err != nil { - return fmt.Errorf("Error parsing IP template on controller public cluster addr: %w", err) - } - } - - host, port, err := net.SplitHostPort(conf.Controller.PublicClusterAddr) - if err != nil { - if strings.Contains(err.Error(), "missing port") { - port = "9201" - host = conf.Controller.PublicClusterAddr - } else { - return fmt.Errorf("Error splitting public cluster adddress host/port: %w", err) - } - } - conf.Controller.PublicClusterAddr = net.JoinHostPort(host, port) - return nil -} - func (b *Server) SetupWorkerPublicAddress(conf *config.Config, flagValue string) error { if conf.Worker == nil { conf.Worker = new(config.Worker) diff --git a/internal/cmd/commands/dev/dev.go b/internal/cmd/commands/dev/dev.go index ac503759c..d06a5cbd2 100644 --- a/internal/cmd/commands/dev/dev.go +++ b/internal/cmd/commands/dev/dev.go @@ -517,7 +517,7 @@ func (c *Command) Run(args []string) int { } } - if err := c.SetupControllerPublicClusterAddress(c.Config, c.flagControllerPublicClusterAddr); err != nil { + if err := c.Config.SetupControllerPublicClusterAddress(c.flagControllerPublicClusterAddr); err != nil { c.UI.Error(err.Error()) return base.CommandUserError } diff --git a/internal/cmd/commands/server/server.go b/internal/cmd/commands/server/server.go index a8d88e097..0605b04e9 100644 --- a/internal/cmd/commands/server/server.go +++ b/internal/cmd/commands/server/server.go @@ -279,7 +279,7 @@ func (c *Command) Run(args []string) int { c.UI.Error(`Config activates controller but no listener with "cluster" purpose found`) return base.CommandUserError } - if err := c.SetupControllerPublicClusterAddress(c.Config, ""); err != nil { + if err := c.Config.SetupControllerPublicClusterAddress(""); err != nil { c.UI.Error(err.Error()) return base.CommandUserError } @@ -304,36 +304,8 @@ func (c *Command) Run(args []string) int { c.Info["worker public proxy addr"] = c.Config.Worker.PublicAddr if c.Config.Controller != nil { - switch len(c.Config.Worker.InitialUpstreams) { - case 0: - if c.Config.Controller.PublicClusterAddr != "" { - clusterAddr = c.Config.Controller.PublicClusterAddr - } - c.Config.Worker.InitialUpstreams = []string{clusterAddr} - case 1: - if c.Config.Worker.InitialUpstreams[0] == clusterAddr { - break - } - if c.Config.Controller.PublicClusterAddr != "" && - c.Config.Worker.InitialUpstreams[0] == c.Config.Controller.PublicClusterAddr { - break - } - // Best effort see if it's a domain name and if not assume it must match - host, _, err := net.SplitHostPort(c.Config.Worker.InitialUpstreams[0]) - if err != nil && strings.Contains(err.Error(), "missing port in address") { - err = nil - host = c.Config.Worker.InitialUpstreams[0] - } - if err == nil { - ip := net.ParseIP(host) - if ip == nil { - // Assume it's a domain name - break - } - } - fallthrough - default: - c.UI.Error(`When running a combined controller and worker, it's invalid to specify a "initial_upstreams" or "controllers" key in the worker block with any values other than the controller cluster or upstream worker address/port when using IPs rather than DNS names`) + if err := c.Config.SetupWorkerInitialUpstreams(); err != nil { + c.UI.Error(err.Error()) return base.CommandUserError } } @@ -846,7 +818,22 @@ func (c *Command) Reload(newConf *config.Config) error { } if newConf != nil && c.worker != nil { - c.worker.Reload(c.Context, newConf) + workerReloadErr := func() error { + if newConf.Controller != nil { + if err := newConf.SetupControllerPublicClusterAddress(""); err != nil { + return err + } + } + + if err := newConf.SetupWorkerInitialUpstreams(); err != nil { + return err + } + c.worker.Reload(c.Context, newConf) + return nil + }() + if workerReloadErr != nil { + reloadErrors = multierror.Append(reloadErrors, fmt.Errorf("error encountered reloading worker initial upstreams: %w", workerReloadErr)) + } } // Send a message that we reloaded. This prevents "guessing" sleep times diff --git a/internal/cmd/config/config.go b/internal/cmd/config/config.go index 3152b7f66..724381816 100644 --- a/internal/cmd/config/config.go +++ b/internal/cmd/config/config.go @@ -9,6 +9,7 @@ import ( "fmt" "io" "io/ioutil" + "net" "reflect" "strconv" "strings" @@ -18,6 +19,7 @@ import ( "github.com/hashicorp/boundary/internal/observability/event" wrapping "github.com/hashicorp/go-kms-wrapping/v2" configutil "github.com/hashicorp/go-secure-stdlib/configutil/v2" + "github.com/hashicorp/go-secure-stdlib/listenerutil" "github.com/hashicorp/go-secure-stdlib/parseutil" "github.com/hashicorp/go-secure-stdlib/strutil" "github.com/hashicorp/hcl" @@ -788,3 +790,122 @@ func (c *Config) Sanitized() map[string]interface{} { return result } + +// SetupControllerPublicClusterAddress will set the controller public address. +// If the flagValue is provided it will be used. Otherwise this will use the +// address from cluster listener. In either case it will check to see if no port +// is included, and if not it will set the default port of 9201. +// +// If there are any errors parsing the address from the flag or listener, +// and error is returned. +func (c *Config) SetupControllerPublicClusterAddress(flagValue string) error { + if c.Controller == nil { + c.Controller = new(Controller) + } + if flagValue != "" { + c.Controller.PublicClusterAddr = flagValue + } + if c.Controller.PublicClusterAddr == "" { + FindAddr: + for _, listener := range c.Listeners { + for _, purpose := range listener.Purpose { + if purpose == "cluster" { + c.Controller.PublicClusterAddr = listener.Address + break FindAddr + } + } + } + } else { + var err error + c.Controller.PublicClusterAddr, err = parseutil.ParsePath(c.Controller.PublicClusterAddr) + if err != nil && !errors.Is(err, parseutil.ErrNotAUrl) { + return fmt.Errorf("Error parsing public cluster addr: %w", err) + } + + c.Controller.PublicClusterAddr, err = listenerutil.ParseSingleIPTemplate(c.Controller.PublicClusterAddr) + if err != nil { + return fmt.Errorf("Error parsing IP template on controller public cluster addr: %w", err) + } + } + + host, port, err := net.SplitHostPort(c.Controller.PublicClusterAddr) + if err != nil { + if strings.Contains(err.Error(), "missing port") { + port = "9201" + host = c.Controller.PublicClusterAddr + } else { + return fmt.Errorf("Error splitting public cluster adddress host/port: %w", err) + } + } + c.Controller.PublicClusterAddr = net.JoinHostPort(host, port) + return nil +} + +// SetupWorkerInitialUpstreams will set the worker initial upstreams in cases +// where both a worker and controller stanza are provided. The initial upstreams +// will be: +// - The initialily provided value, if it is the same as the controller's cluster address +// - The controller's public cluster address if it it was set +// - The controller's cluster listener's address +// +// Any other value already set for iniital upstream will result in an error. +func (c *Config) SetupWorkerInitialUpstreams() error { + // nothing to do here + if c.Worker == nil || c.Controller == nil { + return nil + } + + var clusterAddr string + for _, lnConfig := range c.Listeners { + switch len(lnConfig.Purpose) { + case 0: + return fmt.Errorf("Listener specified without a purpose") + case 1: + purpose := lnConfig.Purpose[0] + switch purpose { + case "cluster": + clusterAddr = lnConfig.Address + if clusterAddr == "" { + clusterAddr = "127.0.0.1:9201" + lnConfig.Address = clusterAddr + } + } + default: + return fmt.Errorf("Specifying a listener with more than one purpose is not supported") + } + } + + switch len(c.Worker.InitialUpstreams) { + case 0: + if c.Controller.PublicClusterAddr != "" { + clusterAddr = c.Controller.PublicClusterAddr + } + c.Worker.InitialUpstreams = []string{clusterAddr} + case 1: + if c.Worker.InitialUpstreams[0] == clusterAddr { + break + } + if c.Controller.PublicClusterAddr != "" && + c.Worker.InitialUpstreams[0] == c.Controller.PublicClusterAddr { + break + } + // Best effort see if it's a domain name and if not assume it must match + host, _, err := net.SplitHostPort(c.Worker.InitialUpstreams[0]) + if err != nil && strings.Contains(err.Error(), "missing port in address") { + err = nil + host = c.Worker.InitialUpstreams[0] + } + if err == nil { + ip := net.ParseIP(host) + if ip == nil { + // Assume it's a domain name + break + } + } + fallthrough + default: + return fmt.Errorf(`When running a combined controller and worker, it's invalid to specify a "initial_upstreams" or "controllers" key in the worker block with any values other than the controller cluster or upstream worker address/port when using IPs rather than DNS names`) + } + + return nil +} diff --git a/internal/cmd/config/config_test.go b/internal/cmd/config/config_test.go index 90d272898..da1e8f912 100644 --- a/internal/cmd/config/config_test.go +++ b/internal/cmd/config/config_test.go @@ -1726,3 +1726,514 @@ func TestDatabaseSkipSharedLockAcquisition(t *testing.T) { }) } } + +func TestSetupControllerPublicClusterAddress(t *testing.T) { + t.Parallel() + tests := []struct { + name string + inputConfig *Config + inputFlagValue string + stateFn func(t *testing.T) + expErr bool + expErrStr string + expPublicClusterAddress string + }{ + { + name: "nil controller", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: nil, + }, + inputFlagValue: "", + expErr: false, + expErrStr: "", + expPublicClusterAddress: ":9201", + }, + { + name: "setting public cluster address directly with ip", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{ + PublicClusterAddr: "127.0.0.1", + }, + }, + inputFlagValue: "", + expErr: false, + expErrStr: "", + expPublicClusterAddress: "127.0.0.1:9201", + }, + { + name: "setting public cluster address directly with ip:port", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{ + PublicClusterAddr: "127.0.0.1:8080", + }, + }, + inputFlagValue: "", + expErr: false, + expErrStr: "", + expPublicClusterAddress: "127.0.0.1:8080", + }, + { + name: "setting public cluster address to env var", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{ + PublicClusterAddr: "env://TEST_ENV_VAR_FOR_CONTROLLER_ADDR", + }, + }, + inputFlagValue: "", + stateFn: func(t *testing.T) { + t.Setenv("TEST_ENV_VAR_FOR_CONTROLLER_ADDR", "127.0.0.1:8080") + }, + expErr: false, + expErrStr: "", + expPublicClusterAddress: "127.0.0.1:8080", + }, + { + name: "setting public cluster address to env var that points to template", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{ + PublicClusterAddr: "env://TEST_ENV_VAR_FOR_CONTROLLER_ADDR", + }, + }, + inputFlagValue: "", + stateFn: func(t *testing.T) { + t.Setenv("TEST_ENV_VAR_FOR_CONTROLLER_ADDR", `{{ GetAllInterfaces | include "flags" "loopback" | include "type" "IPV4" | join "address" " " }}`) + }, + expErr: false, + expErrStr: "", + expPublicClusterAddress: "127.0.0.1:9201", + }, + { + name: "setting public cluster address to ip template", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{ + PublicClusterAddr: `{{ GetAllInterfaces | include "flags" "loopback" | include "type" "IPV4" | join "address" " " }}`, + }, + }, + inputFlagValue: "", + expErr: false, + expErrStr: "", + expPublicClusterAddress: "127.0.0.1:9201", + }, + { + name: "setting public cluster address to multiline ip template", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{ + PublicClusterAddr: `{{ with $local := GetAllInterfaces | include "flags" "loopback" | include "type" "IPV4" -}} + {{- $local | join "address" " " -}} + {{- end }}`, + }, + }, + inputFlagValue: "", + expErr: false, + expErrStr: "", + expPublicClusterAddress: "127.0.0.1:9201", + }, + { + name: "using flag value with ip only", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{}, + }, + inputFlagValue: "127.0.0.1", + expErr: false, + expErrStr: "", + expPublicClusterAddress: "127.0.0.1:9201", + }, + { + name: "using flag value with ip:port", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{}, + }, + inputFlagValue: "127.0.0.1:8080", + expErr: false, + expErrStr: "", + expPublicClusterAddress: "127.0.0.1:8080", + }, + { + name: "using flag value with ip template", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{}, + }, + inputFlagValue: `{{ GetAllInterfaces | include "flags" "loopback" | include "type" "IPV4" | join "address" " " }}`, + expErr: false, + expErrStr: "", + expPublicClusterAddress: "127.0.0.1:9201", + }, + { + name: "using flag value with multiline ip template", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{}, + }, + inputFlagValue: `{{ with $local := GetAllInterfaces | include "flags" "loopback" | include "type" "IPV4" -}} + {{- $local | join "address" " " -}} + {{- end }}`, + expErr: false, + expErrStr: "", + expPublicClusterAddress: "127.0.0.1:9201", + }, + { + name: "using flag value to point to env var with ip only", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{}, + }, + inputFlagValue: "env://TEST_ENV_VAR_FOR_CONTROLLER_ADDR", + stateFn: func(t *testing.T) { + t.Setenv("TEST_ENV_VAR_FOR_CONTROLLER_ADDR", "127.0.0.1") + }, + expErr: false, + expErrStr: "", + expPublicClusterAddress: "127.0.0.1:9201", + }, + { + name: "using flag value to point to env var with ip:port", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{}, + }, + inputFlagValue: "env://TEST_ENV_VAR_FOR_CONTROLLER_ADDR", + stateFn: func(t *testing.T) { + t.Setenv("TEST_ENV_VAR_FOR_CONTROLLER_ADDR", "127.0.0.1:8080") + }, + expErr: false, + expErrStr: "", + expPublicClusterAddress: "127.0.0.1:8080", + }, + { + name: "read address from listeners ip only", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{ + {Purpose: []string{"cluster"}, Address: "127.0.0.1"}, + }, + }, + Controller: &Controller{}, + }, + expErr: false, + expErrStr: "", + expPublicClusterAddress: "127.0.0.1:9201", + }, + { + name: "read address from listeners ip:port", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{ + {Purpose: []string{"cluster"}, Address: "127.0.0.1:8080"}, + }, + }, + Controller: &Controller{}, + }, + expErr: false, + expErrStr: "", + expPublicClusterAddress: "127.0.0.1:8080", + }, + { + name: "read address from listeners is ignored on different purpose", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{ + {Purpose: []string{"somethingelse"}, Address: "127.0.0.1:8080"}, + }, + }, + Controller: &Controller{}, + }, + expErr: false, + expErrStr: "", + expPublicClusterAddress: ":9201", + }, + { + name: "using flag value to point to nonexistent file", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{}, + }, + inputFlagValue: "file://this_doesnt_exist_for_sure", + expErr: true, + expErrStr: "Error parsing public cluster addr: error reading file at file://this_doesnt_exist_for_sure: open this_doesnt_exist_for_sure: no such file or directory", + expPublicClusterAddress: "", + }, + { + name: "using flag value to provoke error in SplitHostPort", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{}, + }, + inputFlagValue: "abc::123", + expErr: true, + expErrStr: "Error splitting public cluster adddress host/port: address abc::123: too many colons in address", + expPublicClusterAddress: "", + }, + { + name: "bad ip template", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{}, + }, + inputFlagValue: "{{ somethingthatdoesntexist }}", + expErr: true, + expErrStr: "Error parsing IP template on controller public cluster addr: unable to parse address template \"{{ somethingthatdoesntexist }}\": unable to parse template \"{{ somethingthatdoesntexist }}\": template: sockaddr.Parse:1: function \"somethingthatdoesntexist\" not defined", + expPublicClusterAddress: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.stateFn != nil { + tt.stateFn(t) + } + err := tt.inputConfig.SetupControllerPublicClusterAddress(tt.inputFlagValue) + if tt.expErr { + require.EqualError(t, err, tt.expErrStr) + return + } + + require.NoError(t, err) + require.NotNil(t, tt.inputConfig.Controller) + require.Equal(t, tt.expPublicClusterAddress, tt.inputConfig.Controller.PublicClusterAddr) + }) + } +} + +func TestSetupWorkerInitialUpstreams(t *testing.T) { + t.Parallel() + tests := []struct { + name string + inputConfig *Config + stateFn func(t *testing.T) + expErr bool + expErrStr string + expInitialUpstreams []string + }{ + { + name: "NilController", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: nil, + Worker: &Worker{ + InitialUpstreams: []string{"192.168.0.2:9201"}, + }, + }, + expErr: false, + expErrStr: "", + expInitialUpstreams: []string{"192.168.0.2:9201"}, + }, + { + name: "NilWorker", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{ + PublicClusterAddr: "192.168.0.3:9201", + }, + Worker: nil, + }, + expErr: false, + expErrStr: "", + expInitialUpstreams: nil, + }, + { + name: "PublicClusterAddr", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{ + PublicClusterAddr: "192.168.0.4:9201", + }, + Worker: &Worker{}, + }, + expErr: false, + expErrStr: "", + expInitialUpstreams: []string{"192.168.0.4:9201"}, + }, + { + name: "ListenerNoAddr", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{ + { + Purpose: []string{"cluster"}, + }, + }, + }, + Controller: &Controller{}, + Worker: &Worker{}, + }, + expErr: false, + expErrStr: "", + expInitialUpstreams: []string{"127.0.0.1:9201"}, + }, + { + name: "ListenerAddr", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{ + { + Purpose: []string{"cluster"}, + Address: "192.168.0.5:9201", + }, + }, + }, + Controller: &Controller{}, + Worker: &Worker{}, + }, + expErr: false, + expErrStr: "", + expInitialUpstreams: []string{"192.168.0.5:9201"}, + }, + { + name: "ListenerAddrDomain", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{ + { + Purpose: []string{"cluster"}, + Address: "foo.test", + }, + }, + }, + Controller: &Controller{}, + Worker: &Worker{}, + }, + expErr: false, + expErrStr: "", + expInitialUpstreams: []string{"foo.test"}, + }, + { + name: "ListenerAddrMultiplePurpose", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{ + { + Purpose: []string{"cluster", "api"}, + }, + }, + }, + Controller: &Controller{}, + Worker: &Worker{}, + }, + expErr: true, + expErrStr: "Specifying a listener with more than one purpose is not supported", + expInitialUpstreams: nil, + }, + { + name: "ListenerAddrNoPurposes", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{ + { + Purpose: []string{}, + }, + }, + }, + Controller: &Controller{}, + Worker: &Worker{}, + }, + expErr: true, + expErrStr: "Listener specified without a purpose", + expInitialUpstreams: nil, + }, + { + name: "ListenerAddrMismatchAddress", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{ + { + Purpose: []string{"cluster"}, + Address: "192.168.0.5:9201", + }, + }, + }, + Controller: &Controller{}, + Worker: &Worker{ + InitialUpstreams: []string{"192.168.0.2:9201"}, + }, + }, + expErr: true, + expErrStr: `When running a combined controller and worker, it's invalid to specify a "initial_upstreams" or "controllers" key in the worker block with any values other than the controller cluster or upstream worker address/port when using IPs rather than DNS names`, + expInitialUpstreams: nil, + }, + { + name: "ClusterAddrMismatchAddress", + inputConfig: &Config{ + SharedConfig: &configutil.SharedConfig{ + Listeners: []*listenerutil.ListenerConfig{}, + }, + Controller: &Controller{ + PublicClusterAddr: "192.168.0.3:9201", + }, + Worker: &Worker{ + InitialUpstreams: []string{"192.168.0.2:9201"}, + }, + }, + expErr: true, + expErrStr: `When running a combined controller and worker, it's invalid to specify a "initial_upstreams" or "controllers" key in the worker block with any values other than the controller cluster or upstream worker address/port when using IPs rather than DNS names`, + expInitialUpstreams: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.stateFn != nil { + tt.stateFn(t) + } + err := tt.inputConfig.SetupWorkerInitialUpstreams() + if tt.expErr { + require.EqualError(t, err, tt.expErrStr) + return + } + + require.NoError(t, err) + if tt.inputConfig.Worker == nil { + require.Empty(t, tt.expInitialUpstreams) + } else { + require.ElementsMatch(t, tt.expInitialUpstreams, tt.inputConfig.Worker.InitialUpstreams) + } + }) + } +} diff --git a/internal/daemon/controller/testing.go b/internal/daemon/controller/testing.go index adf79b0a4..d83a6ec48 100644 --- a/internal/daemon/controller/testing.go +++ b/internal/daemon/controller/testing.go @@ -650,7 +650,7 @@ func TestControllerConfig(t testing.TB, ctx context.Context, tc *TestController, if err := tc.b.SetupListeners(nil, opts.Config.SharedConfig, []string{"api", "cluster", "ops"}); err != nil { t.Fatal(err) } - if err := tc.b.SetupControllerPublicClusterAddress(opts.Config, ""); err != nil { + if err := opts.Config.SetupControllerPublicClusterAddress(""); err != nil { t.Fatal(err) }