diff --git a/command/agent/command.go b/command/agent/command.go index 01f196706ba..d540806f66f 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -30,6 +30,7 @@ import ( gatedwriter "github.com/hashicorp/nomad/helper/gated-writer" "github.com/hashicorp/nomad/helper/logging" "github.com/hashicorp/nomad/helper/winsvc" + "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/nomad/structs/config" "github.com/hashicorp/nomad/version" "github.com/mitchellh/cli" @@ -372,9 +373,16 @@ func (c *Command) isValidConfig(config, cmdConfig *Config) bool { return false } - if config.Client.MinDynamicPort < 0 || config.Client.MaxDynamicPort > 65535 || - config.Client.MinDynamicPort >= config.Client.MaxDynamicPort { - c.Ui.Error("Invalid dynamic port range") + if config.Client.MinDynamicPort < 0 || config.Client.MinDynamicPort > structs.MaxValidPort { + c.Ui.Error(fmt.Sprintf("Invalid dynamic port range: min_dynamic_port=%d", config.Client.MinDynamicPort)) + return false + } + if config.Client.MaxDynamicPort < 0 || config.Client.MaxDynamicPort > structs.MaxValidPort { + c.Ui.Error(fmt.Sprintf("Invalid dynamic port range: max_dynamic_port=%d", config.Client.MaxDynamicPort)) + return false + } + if config.Client.MinDynamicPort > config.Client.MaxDynamicPort { + c.Ui.Error(fmt.Sprintf("Invalid dynamic port range: min_dynamic_port=%d and max_dynamic_port=%d", config.Client.MinDynamicPort, config.Client.MaxDynamicPort)) return false } diff --git a/command/agent/command_test.go b/command/agent/command_test.go index 7be831ad430..6b2c80618e0 100644 --- a/command/agent/command_test.go +++ b/command/agent/command_test.go @@ -2,12 +2,15 @@ package agent import ( "io/ioutil" + "math" "os" "path/filepath" "strings" "testing" "github.com/mitchellh/cli" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/hashicorp/nomad/version" ) @@ -241,3 +244,143 @@ func TestCommand_NullCharInRegion(t *testing.T) { } } } + +// TestIsValidConfig asserts that invalid configurations return false. +func TestIsValidConfig(t *testing.T) { + + cases := []struct { + name string + conf Config // merged into DefaultConfig() + + // err should appear in error output; success expected if err + // is empty + err string + }{ + { + name: "Default", + conf: Config{ + DataDir: "/tmp", + Client: &ClientConfig{Enabled: true}, + }, + }, + { + name: "NoMode", + conf: Config{ + Client: &ClientConfig{Enabled: false}, + Server: &ServerConfig{Enabled: false}, + }, + err: "Must specify either", + }, + { + name: "InvalidRegion", + conf: Config{ + Client: &ClientConfig{ + Enabled: true, + }, + Region: "Hello\000World", + }, + err: "Region contains", + }, + { + name: "InvalidDatacenter", + conf: Config{ + Client: &ClientConfig{ + Enabled: true, + }, + Datacenter: "Hello\000World", + }, + err: "Datacenter contains", + }, + { + name: "RelativeDir", + conf: Config{ + Client: &ClientConfig{ + Enabled: true, + }, + DataDir: "foo/bar", + }, + err: "must be given as an absolute", + }, + { + name: "NegativeMinDynamicPort", + conf: Config{ + Client: &ClientConfig{ + Enabled: true, + MinDynamicPort: -1, + }, + }, + err: "min_dynamic_port", + }, + { + name: "NegativeMaxDynamicPort", + conf: Config{ + Client: &ClientConfig{ + Enabled: true, + MaxDynamicPort: -1, + }, + }, + err: "max_dynamic_port", + }, + { + name: "BigMinDynamicPort", + conf: Config{ + Client: &ClientConfig{ + Enabled: true, + MinDynamicPort: math.MaxInt32, + }, + }, + err: "min_dynamic_port", + }, + { + name: "BigMaxDynamicPort", + conf: Config{ + Client: &ClientConfig{ + Enabled: true, + MaxDynamicPort: math.MaxInt32, + }, + }, + err: "max_dynamic_port", + }, + { + name: "MinMaxDynamicPortSwitched", + conf: Config{ + Client: &ClientConfig{ + Enabled: true, + MinDynamicPort: 5000, + MaxDynamicPort: 4000, + }, + }, + err: "and max", + }, + { + name: "DynamicPortOk", + conf: Config{ + DataDir: "/tmp", + Client: &ClientConfig{ + Enabled: true, + MinDynamicPort: 4000, + MaxDynamicPort: 5000, + }, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + mui := cli.NewMockUi() + cmd := &Command{Ui: mui} + config := DefaultConfig().Merge(&tc.conf) + result := cmd.isValidConfig(config, DefaultConfig()) + if tc.err == "" { + // No error expected + assert.True(t, result, mui.ErrorWriter.String()) + return + } + + // Error expected + assert.False(t, result) + require.Contains(t, mui.ErrorWriter.String(), tc.err) + t.Logf("%s returned: %s", tc.name, mui.ErrorWriter.String()) + }) + } +} diff --git a/nomad/structs/network.go b/nomad/structs/network.go index 3439c359ba5..14dbfa9641d 100644 --- a/nomad/structs/network.go +++ b/nomad/structs/network.go @@ -20,8 +20,8 @@ const ( // to assign a random port maxRandPortAttempts = 20 - // maxValidPort is the max valid port number - maxValidPort = 65536 + // MaxValidPort is the max valid port number + MaxValidPort = 65536 ) var ( @@ -67,7 +67,7 @@ func (idx *NetworkIndex) getUsedPortsFor(ip string) Bitmap { used = raw.(Bitmap) used.Clear() } else { - used, _ = NewBitmap(maxValidPort) + used, _ = NewBitmap(MaxValidPort) } idx.UsedPorts[ip] = used } @@ -215,7 +215,7 @@ func (idx *NetworkIndex) AddReserved(n *NetworkResource) (collide bool) { for _, ports := range [][]Port{n.ReservedPorts, n.DynamicPorts} { for _, port := range ports { // Guard against invalid port - if port.Value < 0 || port.Value >= maxValidPort { + if port.Value < 0 || port.Value >= MaxValidPort { return true } if used.Check(uint(port.Value)) { @@ -234,7 +234,7 @@ func (idx *NetworkIndex) AddReserved(n *NetworkResource) (collide bool) { func (idx *NetworkIndex) AddReservedPorts(ports AllocatedPorts) (collide bool) { for _, port := range ports { used := idx.getUsedPortsFor(port.HostIP) - if port.Value < 0 || port.Value >= maxValidPort { + if port.Value < 0 || port.Value >= MaxValidPort { return true } if used.Check(uint(port.Value)) { @@ -265,7 +265,7 @@ func (idx *NetworkIndex) AddReservedPortRange(ports string) (collide bool) { for _, used := range idx.UsedPorts { for _, port := range resPorts { // Guard against invalid port - if port >= maxValidPort { + if port >= MaxValidPort { return true } if used.Check(uint(port)) { @@ -291,7 +291,7 @@ func (idx *NetworkIndex) AddReservedPortsForIP(ports string, ip string) (collide used := idx.getUsedPortsFor(ip) for _, port := range resPorts { // Guard against invalid port - if port >= maxValidPort { + if port >= MaxValidPort { return true } if used.Check(uint(port)) { @@ -345,7 +345,7 @@ func (idx *NetworkIndex) AssignPorts(ask *NetworkResource) (AllocatedPorts, erro for _, addr := range idx.AvailAddresses[port.HostNetwork] { used := idx.getUsedPortsFor(addr.Address) // Guard against invalid port - if port.Value < 0 || port.Value >= maxValidPort { + if port.Value < 0 || port.Value >= MaxValidPort { return nil, fmt.Errorf("invalid port %d (out of range)", port.Value) } @@ -438,7 +438,7 @@ func (idx *NetworkIndex) AssignNetwork(ask *NetworkResource) (out *NetworkResour // Check if any of the reserved ports are in use for _, port := range ask.ReservedPorts { // Guard against invalid port - if port.Value < 0 || port.Value >= maxValidPort { + if port.Value < 0 || port.Value >= MaxValidPort { err = fmt.Errorf("invalid port %d (out of range)", port.Value) return } @@ -510,7 +510,7 @@ func getDynamicPortsPrecise(nodeUsed Bitmap, minDynamicPort, maxDynamicPort int, return nil, err } } else { - usedSet, err = NewBitmap(maxValidPort) + usedSet, err = NewBitmap(MaxValidPort) if err != nil { return nil, err }