From e145d3ba30202d9118bd0b226d61dab5301d5cc6 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Wed, 14 Aug 2019 15:29:37 -0400 Subject: [PATCH] agent: add optional param to -dev flag for connect (#6126) Consul Connect must route traffic between network namespaces through a public interface (i.e. not localhost). In order to support testing in dev mode, users needed to manually set the interface which doesn't make for a smooth experience. This commit adds a facility for adding optional parameters to the `nomad agent -dev` flag and uses it to add a `-dev=connect` flag that binds to a public interface on the host. --- command/agent/agent_test.go | 2 +- command/agent/command.go | 19 +++- command/agent/config.go | 101 ++++++++++++++++-- command/agent/config_test.go | 56 ++++++++++ command/agent/testagent.go | 2 +- helper/flag-helpers/flag.go | 8 ++ .../source/docs/commands/agent.html.md.erb | 14 ++- 7 files changed, 183 insertions(+), 19 deletions(-) diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go index 2fec663d4..27b09a895 100644 --- a/command/agent/agent_test.go +++ b/command/agent/agent_test.go @@ -413,7 +413,7 @@ func TestAgent_HTTPCheckPath(t *testing.T) { t.Parallel() // Agent.agentHTTPCheck only needs a config and logger a := &Agent{ - config: DevConfig(), + config: DevConfig(nil), logger: testlog.HCLogger(t), } if err := a.config.normalizeAddrs(); err != nil { diff --git a/command/agent/command.go b/command/agent/command.go index 34567c0e2..ad7047b7a 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -56,7 +56,7 @@ type Command struct { } func (c *Command) readConfig() *Config { - var dev bool + var dev *devModeConfig var configPath []string var servers string var meta []string @@ -77,7 +77,10 @@ func (c *Command) readConfig() *Config { flags.Usage = func() { c.Ui.Error(c.Help()) } // Role options - flags.BoolVar(&dev, "dev", false, "") + flags.Var((flaghelper.FuncOptionalStringVar)(func(s string) (err error) { + dev, err = newDevModeConfig(s) + return err + }), "dev", "") flags.BoolVar(&cmdConfig.Server.Enabled, "server", false, "") flags.BoolVar(&cmdConfig.Client.Enabled, "client", false, "") @@ -204,8 +207,8 @@ func (c *Command) readConfig() *Config { // Load the configuration var config *Config - if dev { - config = DevConfig() + if dev != nil { + config = DevConfig(dev) } else { config = DefaultConfig() } @@ -1164,7 +1167,13 @@ General Options (clients and servers): Start the agent in development mode. This enables a pre-configured dual-role agent (client + server) which is useful for developing or testing Nomad. No other configuration is required to start the - agent in this mode. + agent in this mode, but you may pass an optional comma-separated + list of mode configurations: + + -dev=connect + Start the agent in development mode, but bind to a public network + interface rather than localhost for using Consul Connect. This + mode is supported only on Linux as root. Server Options: diff --git a/command/agent/config.go b/command/agent/config.go index 5c36e3762..969ecd447 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -7,6 +7,7 @@ import ( "io" "net" "os" + "os/user" "path/filepath" "runtime" "sort" @@ -14,6 +15,7 @@ import ( "strings" "time" + sockaddr "github.com/hashicorp/go-sockaddr" "github.com/hashicorp/go-sockaddr/template" client "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/helper" @@ -668,22 +670,105 @@ func (r *Resources) CanParseReserved() error { return err } +// devModeConfig holds the config for the -dev flag +type devModeConfig struct { + // mode flags are set at the command line via -dev= + defaultMode bool + connectMode bool + + bindAddr string + iface string +} + +// newDevModeConfig parses the optional string value of the -dev flag +func newDevModeConfig(s string) (*devModeConfig, error) { + if s == "" { + return nil, nil // no -dev flag + } + mode := &devModeConfig{} + modeFlags := strings.Split(s, ",") + for _, modeFlag := range modeFlags { + switch modeFlag { + case "true": // -dev flag with no params + mode.defaultMode = true + case "connect": + if runtime.GOOS != "linux" { + // strictly speaking -dev=connect only binds to the + // non-localhost interface, but given its purpose + // is to support a feature with network namespaces + // we'll return an error here rather than let the agent + // come up and fail unexpectedly to run jobs + return nil, fmt.Errorf("-dev=connect is only supported on linux.") + } + u, err := user.Current() + if err != nil { + return nil, fmt.Errorf( + "-dev=connect uses network namespaces and is only supported for root: %v", err) + } + if u.Uid != "0" { + return nil, fmt.Errorf( + "-dev=connect uses network namespaces and is only supported for root.") + } + mode.connectMode = true + default: + return nil, fmt.Errorf("invalid -dev flag: %q", s) + } + } + err := mode.networkConfig() + if err != nil { + return nil, err + } + return mode, nil +} + +func (mode *devModeConfig) networkConfig() error { + if runtime.GOOS == "darwin" { + mode.bindAddr = "127.0.0.1" + mode.iface = "lo0" + return nil + } + if mode != nil && mode.connectMode { + // if we hit either of the errors here we're in a weird situation + // where syscalls to get the list of network interfaces are failing. + // rather than throwing errors, we'll fall back to the default. + ifAddrs, err := sockaddr.GetDefaultInterfaces() + errMsg := "-dev=connect uses network namespaces: %v" + if err != nil { + return fmt.Errorf(errMsg, err) + } + if len(ifAddrs) < 1 { + return fmt.Errorf(errMsg, "could not find public network inteface") + } + iface := ifAddrs[0].Name + addr, err := sockaddr.GetInterfaceIP(iface) + if err != nil { + return fmt.Errorf(errMsg, "could not find address for public interface") + } + mode.iface = iface + mode.bindAddr = addr + return nil + } + mode.bindAddr = "127.0.0.1" + mode.iface = "lo" + return nil +} + // DevConfig is a Config that is used for dev mode of Nomad. -func DevConfig() *Config { +func DevConfig(mode *devModeConfig) *Config { + if mode == nil { + mode = &devModeConfig{defaultMode: true} + mode.networkConfig() + } conf := DefaultConfig() - conf.BindAddr = "127.0.0.1" + conf.BindAddr = mode.bindAddr conf.LogLevel = "DEBUG" conf.Client.Enabled = true conf.Server.Enabled = true - conf.DevMode = true + conf.DevMode = mode != nil conf.EnableDebug = true conf.DisableAnonymousSignature = true conf.Consul.AutoAdvertise = helper.BoolToPtr(true) - if runtime.GOOS == "darwin" { - conf.Client.NetworkInterface = "lo0" - } else if runtime.GOOS == "linux" { - conf.Client.NetworkInterface = "lo" - } + conf.Client.NetworkInterface = mode.iface conf.Client.Options = map[string]string{ "driver.raw_exec.enable": "true", "driver.docker.volumes": "true", diff --git a/command/agent/config_test.go b/command/agent/config_test.go index d5d49bc4b..3bc7198c0 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -7,6 +7,8 @@ import ( "os" "path/filepath" "reflect" + "runtime" + "strings" "testing" "time" @@ -620,6 +622,60 @@ func TestConfig_Listener(t *testing.T) { } } +func TestConfig_DevModeFlag(t *testing.T) { + cases := []struct { + flag string + expected *devModeConfig + expectedErr string + }{} + if runtime.GOOS != "linux" { + cases = []struct { + flag string + expected *devModeConfig + expectedErr string + }{ + {"", nil, ""}, + {"true", &devModeConfig{defaultMode: true, connectMode: false}, ""}, + {"true,connect", nil, "-dev=connect is only supported on linux"}, + {"connect", nil, "-dev=connect is only supported on linux"}, + {"xxx", nil, "invalid -dev flag"}, + } + } + if runtime.GOOS == "linux" { + cases = []struct { + flag string + expected *devModeConfig + expectedErr string + }{ + {"", nil, ""}, + {"true", &devModeConfig{defaultMode: true, connectMode: false}, ""}, + {"true,connect", &devModeConfig{defaultMode: true, connectMode: true}, ""}, + {"connect", &devModeConfig{defaultMode: false, connectMode: true}, ""}, + {"xxx", nil, "invalid -dev flag"}, + } + } + for _, c := range cases { + t.Run(c.flag, func(t *testing.T) { + mode, err := newDevModeConfig(c.flag) + if err != nil && c.expectedErr == "" { + t.Fatalf("unexpected error: %v", err) + } + if err != nil && !strings.Contains(err.Error(), c.expectedErr) { + t.Fatalf("expected %s; got %v", c.expectedErr, err) + } + if mode == nil && c.expected != nil { + t.Fatalf("expected %+v but got nil", c.expected) + } + if mode != nil { + if c.expected.defaultMode != mode.defaultMode || + c.expected.connectMode != mode.connectMode { + t.Fatalf("expected %+v, got %+v", c.expected, mode) + } + } + }) + } +} + // TestConfig_normalizeAddrs_DevMode asserts that normalizeAddrs allows // advertising localhost in dev mode. func TestConfig_normalizeAddrs_DevMode(t *testing.T) { diff --git a/command/agent/testagent.go b/command/agent/testagent.go index 2b2bfedee..59d8eaeb8 100644 --- a/command/agent/testagent.go +++ b/command/agent/testagent.go @@ -311,7 +311,7 @@ func (a *TestAgent) pickRandomPorts(c *Config) { // TestConfig returns a unique default configuration for testing an // agent. func (a *TestAgent) config() *Config { - conf := DevConfig() + conf := DevConfig(nil) // Customize the server configuration config := nomad.DefaultConfig() diff --git a/helper/flag-helpers/flag.go b/helper/flag-helpers/flag.go index 10a5644e2..3156e3710 100644 --- a/helper/flag-helpers/flag.go +++ b/helper/flag-helpers/flag.go @@ -58,3 +58,11 @@ func (f FuncDurationVar) Set(s string) error { } func (f FuncDurationVar) String() string { return "" } func (f FuncDurationVar) IsBoolFlag() bool { return false } + +// FuncOptionalStringVar is a flag that accepts a function which it +// calls on the optional string given by the user. +type FuncOptionalStringVar func(s string) error + +func (f FuncOptionalStringVar) Set(s string) error { return f(s) } +func (f FuncOptionalStringVar) String() string { return "" } +func (f FuncOptionalStringVar) IsBoolFlag() bool { return true } diff --git a/website/source/docs/commands/agent.html.md.erb b/website/source/docs/commands/agent.html.md.erb index 20834b1d4..80ee6fa56 100644 --- a/website/source/docs/commands/agent.html.md.erb +++ b/website/source/docs/commands/agent.html.md.erb @@ -14,7 +14,7 @@ or server functionality, including exposing interfaces for client consumption and running jobs. Due to the power and flexibility of this command, the Nomad agent is documented -in its own section. See the [Nomad Agent](/guides/install/production/nomad-agent.html) +in its own section. See the [Nomad Agent](/guides/install/production/nomad-agent.html) guide and the [Configuration](/docs/configuration/index.html) documentation section for more information on how to use this command and the options it has. @@ -55,7 +55,13 @@ via CLI arguments. The `agent` command accepts the following arguments: * `-dc=`: Equivalent to the [datacenter](#datacenter) config option. * `-dev`: Start the agent in development mode. This enables a pre-configured dual-role agent (client + server) which is useful for developing or testing - Nomad. No other configuration is required to start the agent in this mode. + Nomad. No other configuration is required to start the agent in this mode, + but you may pass an optional comma-separated list of mode configurations: + + `-dev=connect`: Start the agent in development mode, but bind to a public + network interface rather than localhost for using Consul Connect. This mode + is supported only on Linux as root. + * `-encrypt`: Set the Serf encryption key. See the [Encryption Overview](/guides/security/encryption.html) for more details. * `-join=
`: Address of another agent to join upon starting up. This can be specified multiple times to specify multiple agents to join. @@ -102,8 +108,8 @@ via CLI arguments. The `agent` command accepts the following arguments: * `vault-cert-file=`: The path to the certificate for Vault communication. * `vault-key-file=`: The path to the private key for Vault communication. * `vault-namespace=`: The Vault namespace used for the integration. - Required for servers and clients. Overrides the Vault namespace read from the - VAULT_NAMESPACE environment variable. + Required for servers and clients. Overrides the Vault namespace read from the + VAULT_NAMESPACE environment variable. * `vault-tls-skip-verify`: A boolean that determines whether to skip SSL certificate verification. * `vault-tls-server-name=`: Used to set the SNI host when connecting to