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

@@ -9,6 +9,7 @@ import (
 	"os/signal"
 	"path/filepath"
 	"reflect"
+	"regexp"
 	"sort"
 	"strconv"
 	"strings"
@@ -186,6 +187,8 @@ func (c *Command) readConfig() *Config {
 	// Parse the meta flags.
 	metaLength := len(meta)
 	if metaLength != 0 {
+		validKeyRe, _ := regexp.Compile(`^[^.]+(\.[^.]+)*$`)
+
 		cmdConfig.Client.Meta = make(map[string]string, metaLength)
 		for _, kv := range meta {
 			parts := strings.SplitN(kv, "=", 2)
@@ -194,6 +197,11 @@ func (c *Command) readConfig() *Config {
 				return nil
 			}
 
+			if !validKeyRe.MatchString(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

@@ -6,6 +6,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"regexp"
 	"time"
 
 	multierror "github.com/hashicorp/go-multierror"
@@ -437,6 +438,13 @@ func parseClient(result **ClientConfig, list *ast.ObjectList) error {
 				return err
 			}
 		}
+
+		validKeyRe, _ := regexp.Compile(`^[^.]+(\.[^.]+)*$`)
+		for k := range config.Meta {
+			if !validKeyRe.MatchString(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 {