luxolus
/
open-nomad
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

Merge pull request #5158 from hashicorp/f-1157-validate-node-meta-variables 

added validation on client metadata keys
This commit is contained in:
Chris Baker 2019-01-09 16:37:57 -05:00 committed by GitHub
parent bd12e0b1f7 e9db2ae822
commit 1c2fec824f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 130 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
CHANGELOG.md
View File

@ -7,6 +7,9 @@ __BACKWARDS INCOMPATIBILITIES:__
* client: Task config interpolation requires names to be valid identifiers
(`node.region` or `NOMAD_DC`). Interpolating other variables requires a new
indexing syntax: `env[".invalid.identifier."]`. [[GH-4843](https://github.com/hashicorp/nomad/issues/4843)]
* client: Node metadata variables must have valid identifiers, whether
specified in the config file (`client.meta` stanza) or on the command line
(`-meta`). [[GH-5158](https://github.com/hashicorp/nomad/pull/5158)]
IMPROVEMENTS:
* core: Added advertise address to client node meta data [[GH-4390](https://github.com/hashicorp/nomad/issues/4390)]

78
command/agent/command.go
View File

@ -15,17 +15,18 @@ import (
"syscall"
"time"
metrics "github.com/armon/go-metrics"
"github.com/armon/go-metrics"
"github.com/armon/go-metrics/circonus"
"github.com/armon/go-metrics/datadog"
"github.com/armon/go-metrics/prometheus"
"github.com/hashicorp/consul/lib"
checkpoint "github.com/hashicorp/go-checkpoint"
discover "github.com/hashicorp/go-discover"
gsyslog "github.com/hashicorp/go-syslog"
"github.com/hashicorp/go-checkpoint"
"github.com/hashicorp/go-discover"
"github.com/hashicorp/go-syslog"
"github.com/hashicorp/logutils"
flaghelper "github.com/hashicorp/nomad/helper/flag-helpers"
gatedwriter "github.com/hashicorp/nomad/helper/gated-writer"
"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/helper/flag-helpers"
"github.com/hashicorp/nomad/helper/gated-writer"
"github.com/hashicorp/nomad/nomad/structs/config"
"github.com/hashicorp/nomad/version"
"github.com/mitchellh/cli"
@ -193,7 +194,6 @@ func (c *Command) readConfig() *Config {
c.Ui.Error(fmt.Sprintf("Error parsing Client.Meta value: %v", kv))
return nil
}
cmdConfig.Client.Meta[parts[0]] = parts[1]
}
}
@ -257,6 +257,27 @@ func (c *Command) readConfig() *Config {
}
}
// Default the plugin directory to be under that of the data directory if it
// isn't explicitly specified.
if config.PluginDir == "" && config.DataDir != "" {
config.PluginDir = filepath.Join(config.DataDir, "plugins")
}
if !c.isValidConfig(config) {
return nil
}
return config
}
func (c *Command) isValidConfig(config *Config) bool {
// Check that the server is running in at least one mode.
if !(config.Server.Enabled || config.Client.Enabled) {
c.Ui.Error("Must specify either server, client or dev mode for the agent.")
return false
}
// Set up the TLS configuration properly if we have one.
// XXX chelseakomlo: set up a TLSConfig New method which would wrap
// constructor-type actions like this.
@ -266,21 +287,10 @@ func (c *Command) readConfig() *Config {
}
}
// Default the plugin directory to be under that of the data directory if it
// isn't explicitly specified.
if config.PluginDir == "" && config.DataDir != "" {
config.PluginDir = filepath.Join(config.DataDir, "plugins")
}
if dev {
// Skip validation for dev mode
return config
}
if config.Server.EncryptKey != "" {
if _, err := config.Server.EncryptBytes(); err != nil {
c.Ui.Error(fmt.Sprintf("Invalid encryption key: %s", err))
return nil
return false
}
keyfile := filepath.Join(config.DataDir, serfKeyring)
if _, err := os.Stat(keyfile); err == nil {
@ -288,12 +298,6 @@ func (c *Command) readConfig() *Config {
}
}
// Check that the server is running in at least one mode.
if !(config.Server.Enabled || config.Client.Enabled) {
c.Ui.Error("Must specify either server, client or dev mode for the agent.")
return nil
}
// Verify the paths are absolute.
dirs := map[string]string{
"data-dir": config.DataDir,
@ -308,14 +312,28 @@ func (c *Command) readConfig() *Config {
if !filepath.IsAbs(dir) {
c.Ui.Error(fmt.Sprintf("%s must be given as an absolute path: got %v", k, dir))
return nil
return false
}
}
if config.Client.Enabled {
for k := range config.Client.Meta {
if !helper.IsValidInterpVariable(k) {
c.Ui.Error(fmt.Sprintf("Invalid Client.Meta key: %v", k))
return false
}
}
}
if config.DevMode {
// Skip the rest of the validation for dev mode
return true
}
// Ensure that we have the directories we need to run.
if config.Server.Enabled && config.DataDir == "" {
c.Ui.Error("Must specify data directory")
return nil
return false
}
// The config is valid if the top-level data-dir is set or if both
@ -323,20 +341,20 @@ func (c *Command) readConfig() *Config {
if config.Client.Enabled && config.DataDir == "" {
if config.Client.AllocDir == "" || config.Client.StateDir == "" || config.PluginDir == "" {
c.Ui.Error("Must specify the state, alloc dir, and plugin dir if data-dir is omitted.")
return nil
return false
}
}
// Check the bootstrap flags
if config.Server.BootstrapExpect > 0 && !config.Server.Enabled {
c.Ui.Error("Bootstrap requires server mode to be enabled")
return nil
return false
}
if config.Server.BootstrapExpect == 1 {
c.Ui.Error("WARNING: Bootstrap mode enabled! Potentially unsafe operation.")
}
return config
return true
}
// setupLoggers is used to setup the logGate, logWriter, and our logOutput

66
command/agent/command_test.go
View File

@ -3,6 +3,7 @@ package agent
import (
"io/ioutil"
"os"
"path/filepath"
"strings"
"testing"
@ -48,6 +49,18 @@ func TestCommand_Args(t *testing.T) {
[]string{"-client", "-alloc-dir="},
"Must specify the state, alloc dir, and plugin dir if data-dir is omitted.",
},
{
[]string{"-client", "-data-dir=" + tmpDir, "-meta=invalid..key=inaccessible-value"},
"Invalid Client.Meta key: invalid..key",
},
{
[]string{"-client", "-data-dir=" + tmpDir, "-meta=.invalid=inaccessible-value"},
"Invalid Client.Meta key: .invalid",
},
{
[]string{"-client", "-data-dir=" + tmpDir, "-meta=invalid.=inaccessible-value"},
"Invalid Client.Meta key: invalid.",
},
}
for _, tc := range tcases {
// Make a new command. We preemptively close the shutdownCh
@ -76,3 +89,56 @@ func TestCommand_Args(t *testing.T) {
}
}
}
func TestCommand_MetaConfigValidation(t *testing.T) {
tmpDir, err := ioutil.TempDir("", "nomad")
if err != nil {
t.Fatalf("err: %s", err)
}
defer os.RemoveAll(tmpDir)
tcases := []string{
"foo..invalid",
".invalid",
"invalid.",
}
for _, tc := range tcases {
configFile := filepath.Join(tmpDir, "conf1.hcl")
err = ioutil.WriteFile(configFile, []byte(`client{
enabled = true
meta = {
"valid" = "yes"
"`+tc+`" = "kaboom!"
"nested.var" = "is nested"
"deeply.nested.var" = "is deeply nested"
}
}`), 0600)
if err != nil {
t.Fatalf("err: %s", err)
}
// Make a new command. We preemptively close the shutdownCh
// so that the command exits immediately instead of blocking.
ui := new(cli.MockUi)
shutdownCh := make(chan struct{})
close(shutdownCh)
cmd := &Command{
Version: version.GetVersion(),
Ui: ui,
ShutdownCh: shutdownCh,
}
// To prevent test failures on hosts whose hostname resolves to
// a loopback address, we must append a bind address
args := []string{"-client", "-data-dir=" + tmpDir, "-config=" + configFile, "-bind=169.254.0.1"}
if code := cmd.Run(args); code != 1 {
t.Fatalf("args: %v\nexit: %d\n", args, code)
}
expect := "Invalid Client.Meta key: " + tc
out := ui.ErrorWriter.String()
if !strings.Contains(out, expect) {
t.Fatalf("expect to find %q\n\n%s", expect, out)
}
}
}

13
helper/funcs.go
View File

@ -15,6 +15,11 @@ import (
// validUUID is used to check if a given string looks like a UUID
var validUUID = regexp.MustCompile(`(?i)^[\da-f]{8}-[\da-f]{4}-[\da-f]{4}-[\da-f]{4}-[\da-f]{12}$`)
// validInterpVarKey matches valid dotted variable names for interpolation. The
// string must begin with one or more non-dot characters which may be followed
// by sequences containing a dot followed by a one or more non-dot characters.
var validInterpVarKey = regexp.MustCompile(`^[^.]+(\.[^.]+)*$`)
// IsUUID returns true if the given string is a valid UUID.
func IsUUID(str string) bool {
const uuidLen = 36
@ -25,6 +30,14 @@ func IsUUID(str string) bool {
return validUUID.MatchString(str)
}
// IsValidInterpVariable returns true if a valid dotted variable names for
// interpolation. The string must begin with one or more non-dot characters
// which may be followed by sequences containing a dot followed by a one or more
// non-dot characters.
func IsValidInterpVariable(str string) bool {
return validInterpVarKey.MatchString(str)
}
// HashUUID takes an input UUID and returns a hashed version of the UUID to
// ensure it is well distributed.
func HashUUID(input string) (output string, hashed bool) {