added validation on client metadata keys

CHANGELOG.md

@@ -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)]

command/agent/command.go

@@ -15,17 +15,19 @@ import (
 	"syscall"
 	"time"
 
-	metrics "github.com/armon/go-metrics"
+	"github.com/hashicorp/nomad/helper"
+
+	"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/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"
@@ -194,6 +196,11 @@ func (c *Command) readConfig() *Config {
 				return nil
 			}
 
+			if !helper.IsValidInterpVariable(parts[0]) {
+				c.Ui.Error(fmt.Sprintf("Invalid Client.Meta key: %v", parts[0]))
+				return nil
+			}
+
 			cmdConfig.Client.Meta[parts[0]] = parts[1]
 		}
 	}

command/agent/command_test.go

@@ -48,6 +48,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

command/agent/config_parse.go

@@ -437,6 +437,12 @@ func parseClient(result **ClientConfig, list *ast.ObjectList) error {
 				return err
 			}
 		}
+
+		for k := range config.Meta {
+			if !helper.IsValidInterpVariable(k) {
+				return fmt.Errorf("invalid Client.Meta key: %v", k)
+			}
+		}
 	}
 
 	// Parse out chroot_env fields. These are in HCL as a list so we need to

command/agent/config_test.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 
@@ -418,6 +419,30 @@ func TestConfig_ParseConfigFile(t *testing.T) {
 	}
 }
 
+func TestConfig_ParseInvalidClientMeta(t *testing.T) {
+	tcases := []string{
+		"foo..invalid",
+		".invalid",
+		"invalid.",
+	}
+
+	for _, tc := range tcases {
+		reader := strings.NewReader(fmt.Sprintf(`client{
+			enabled = true
+			meta = {
+				"valid" = "yes"
+				"` + tc + `" = "kaboom!"
+				"nested.var" = "is nested"
+				"deeply.nested.var" = "is deeply nested"
+			}
+    	}`))
+
+		if _, err := ParseConfig(reader); err == nil {
+			t.Fatalf("expected load error, got nothing")
+		}
+	}
+}
+
 func TestConfig_LoadConfigDir(t *testing.T) {
 	// Fails if the dir doesn't exist.
 	if _, err := LoadConfigDir("/unicorns/leprechauns"); err == nil {

helper/funcs.go

@@ -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) {