Skip to content

Commit

Permalink
agent: validate reserved_ports are valid
Browse files Browse the repository at this point in the history
Goal is to fix at least one of the causes that can cause a node to be
ineligible to receive work:
#9506 (comment)
  • Loading branch information
schmichael committed Jan 12, 2022
1 parent b446352 commit a39c8bc
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 0 deletions.
21 changes: 21 additions & 0 deletions command/agent/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,27 @@ func (c *Command) isValidConfig(config, cmdConfig *Config) bool {
return false
}

if config.Client.Reserved == nil {
// Coding error; should always be set my DefaultConfig()
c.Ui.Error("client.reserved must be initialized. Please report a bug.")
return false
}

if ports := config.Client.Reserved.ReservedPorts; ports != "" {
if _, err := structs.ParsePortRanges(ports); err != nil {
c.Ui.Error(fmt.Sprintf("reserved.reserved_ports %s invalid: %v", ports, err))
return false
}
}

for _, hn := range config.Client.HostNetworks {
if _, err := structs.ParsePortRanges(hn.ReservedPorts); err != nil {
c.Ui.Error(fmt.Sprintf("host_network[%q].reserved_ports %q invalid: %v",
hn.Name, hn.ReservedPorts, err))
return false
}
}

if !config.DevMode {
// Ensure that we have the directories we need to run.
if config.Server.Enabled && config.DataDir == "" {
Expand Down
28 changes: 28 additions & 0 deletions command/agent/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/hashicorp/nomad/nomad/structs"
"github.com/hashicorp/nomad/version"
)

Expand Down Expand Up @@ -363,6 +364,33 @@ func TestIsValidConfig(t *testing.T) {
},
},
},
{
name: "BadReservedPorts",
conf: Config{
Client: &ClientConfig{
Enabled: true,
Reserved: &Resources{
ReservedPorts: "3-2147483647",
},
},
},
err: "reserved.reserved_ports 3-2147483647 invalid: port must be < 65536 but found 3-2147483647",
},
{
name: "BadHostNetworkReservedPorts",
conf: Config{
Client: &ClientConfig{
Enabled: true,
HostNetworks: []*structs.ClientHostNetworkConfig{
&structs.ClientHostNetworkConfig{
Name: "test",
ReservedPorts: "3-2147483647",
},
},
},
},
err: `host_network["test"].reserved_ports "3-2147483647" invalid: port must be < 65536 but found 3-2147483647`,
},
}

for _, tc := range cases {
Expand Down
12 changes: 12 additions & 0 deletions nomad/structs/funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,12 @@ func ParsePortRanges(spec string) ([]uint64, error) {
return nil, fmt.Errorf("invalid range: starting value (%v) less than ending (%v) value", end, start)
}

// Full range validation is below but prevent creating
// arbitrarily large arrays here
if end > MaxValidPort {
return nil, fmt.Errorf("port must be < %d but found %s", MaxValidPort, part)
}

for i := start; i <= end; i++ {
ports[i] = struct{}{}
}
Expand All @@ -541,6 +547,12 @@ func ParsePortRanges(spec string) ([]uint64, error) {

var results []uint64
for port := range ports {
if port == 0 {
return nil, fmt.Errorf("port must be > 0")
}
if port > MaxValidPort {
return nil, fmt.Errorf("port must be < %d but found %d", MaxValidPort, port)
}
results = append(results, port)
}

Expand Down
39 changes: 39 additions & 0 deletions nomad/structs/funcs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -880,3 +880,42 @@ func TestMergeMultierrorWarnings(t *testing.T) {

require.Equal(t, "2 warning(s):\n\n* foo\n* bar", str)
}

// TestParsePortRanges asserts ParsePortRanges errors on invalid port ranges.
func TestParsePortRanges(t *testing.T) {
cases := []struct {
name string
spec string
err string
}{
{
name: "UnmatchedDash",
spec: "-1",
err: `strconv.ParseUint: parsing "": invalid syntax`,
},
{
name: "Zero",
spec: "0",
err: "port must be > 0",
},
{
name: "TooBig",
spec: fmt.Sprintf("1-%d", MaxValidPort+1),
err: "port must be < 65536 but found 1-65537",
},
{
name: "WayTooBig", // would OOM if not caught early enough
spec: "9223372036854775807", // (2**63)-1
err: "port must be < 65536 but found 9223372036854775807",
},
}

for i := range cases {
tc := cases[i]
t.Run(tc.name, func(t *testing.T) {
results, err := ParsePortRanges(tc.spec)
require.Nil(t, results)
require.EqualError(t, err, tc.err)
})
}
}

0 comments on commit a39c8bc

Please sign in to comment.