Validate metadata config earlier and handle multiple filters

command/agent/agent.go

@@ -1695,20 +1695,13 @@ func (a *Agent) restoreCheckState(snap map[types.CheckID]*structs.HealthCheck) {
 	}
 }
 
-// loadMetadata loads and validates node metadata fields from the config and
+// loadMetadata loads node metadata fields from the agent config and
 // updates them on the local agent.
 func (a *Agent) loadMetadata(conf *Config) error {
 	a.state.Lock()
 	defer a.state.Unlock()
 
-	if len(conf.Meta) > metaMaxKeyPairs {
-		return fmt.Errorf("Node metadata cannot contain more than %d key/value pairs", metaMaxKeyPairs)
-	}
-
 	for key, value := range conf.Meta {
-		if err := validateMetaPair(key, value); err != nil {
-			return fmt.Errorf("Couldn't load metadata pair ('%s', '%s'): %s", key, value, err)
-		}
 		a.state.metadata[key] = value
 	}
 
@@ -1717,6 +1710,21 @@ func (a *Agent) loadMetadata(conf *Config) error {
 	return nil
 }
 
+// validateMeta validates a set of key/value pairs from the agent config
+func validateMetadata(meta map[string]string) error {
+	if len(meta) > metaMaxKeyPairs {
+		return fmt.Errorf("Node metadata cannot contain more than %d key/value pairs", metaMaxKeyPairs)
+	}
+
+	for key, value := range meta {
+		if err := validateMetaPair(key, value); err != nil {
+			return fmt.Errorf("Couldn't load metadata pair ('%s', '%s'): %s", key, value, err)
+		}
+	}
+
+	return nil
+}
+
 // validateMetaPair checks that the given key/value pair is in a valid format
 func validateMetaPair(key, value string) error {
 	if key == "" {
@@ -1726,25 +1734,23 @@ func validateMetaPair(key, value string) error {
 		return fmt.Errorf("Key contains invalid characters")
 	}
 	if len(key) > metaKeyMaxLength {
-		return fmt.Errorf("Key is longer than %d chars", metaKeyMaxLength)
+		return fmt.Errorf("Key is too long (limit: %d characters)", metaKeyMaxLength)
 	}
 	if strings.HasPrefix(key, metaKeyReservedPrefix) {
 		return fmt.Errorf("Key prefix '%s' is reserved for internal use", metaKeyReservedPrefix)
 	}
 	if len(value) > metaValueMaxLength {
-		return fmt.Errorf("Value is longer than %d characters", metaValueMaxLength)
+		return fmt.Errorf("Value is too long (limit: %d characters)", metaValueMaxLength)
 	}
 	return nil
 }
 
 // unloadMetadata resets the local metadata state
-func (a *Agent) unloadMetadata() error {
+func (a *Agent) unloadMetadata() {
 	a.state.Lock()
 	defer a.state.Unlock()
 
 	a.state.metadata = make(map[string]string)
-
-	return nil
 }
 
 // serviceMaintCheckID returns the ID of a given service's maintenance check

command/agent/agent_test.go

@@ -19,6 +19,7 @@ import (
 	"github.com/hashicorp/consul/logger"
 	"github.com/hashicorp/consul/testutil"
 	"github.com/hashicorp/raft"
+	"strings"
 )
 
 const (
@@ -1852,69 +1853,64 @@ func TestAgent_purgeCheckState(t *testing.T) {
 }
 
 func TestAgent_metadata(t *testing.T) {
-	config := nextConfig()
-	dir, agent := makeAgent(t, config)
-	defer os.RemoveAll(dir)
-	defer agent.Shutdown()
-
 	// Load a valid set of key/value pairs
-	config.Meta = map[string]string{
+	meta := map[string]string{
 		"key1": "value1",
 		"key2": "value2",
 	}
 	// Should succeed
-	if err := agent.loadMetadata(config); err != nil {
+	if err := validateMetadata(meta); err != nil {
 		t.Fatalf("err: %s", err)
 	}
-	agent.unloadMetadata()
 
 	// Should get error
-	config.Meta = map[string]string{
+	meta = map[string]string{
 		"": "value1",
 	}
-	if err := agent.loadMetadata(config); err == nil {
+	if err := validateMetadata(meta); !strings.Contains(err.Error(), "Couldn't load metadata pair") {
 		t.Fatalf("should have failed")
 	}
-	agent.unloadMetadata()
 
 	// Should get error
-	tooManyKeys := make(map[string]string)
+	meta = make(map[string]string)
 	for i := 0; i < metaMaxKeyPairs+1; i++ {
-		tooManyKeys[string(i)] = "value"
+		meta[string(i)] = "value"
 	}
-	if err := agent.loadMetadata(config); err == nil {
+	if err := validateMetadata(meta); !strings.Contains(err.Error(), "cannot contain more than") {
 		t.Fatalf("should have failed")
 	}
 }
 
 func TestAgent_validateMetaPair(t *testing.T) {
-	longKey := fmt.Sprintf(fmt.Sprintf("%%%ds", metaKeyMaxLength+1), "")
+	longKey := strings.Repeat("a", metaKeyMaxLength+1)
-	longValue := fmt.Sprintf(fmt.Sprintf("%%%ds", metaValueMaxLength+1), "")
+	longValue := strings.Repeat("b", metaValueMaxLength+1)
 	pairs := []struct {
-		Key     string
+		Key   string
-		Value   string
+		Value string
-		Success bool
+		Error string
 	}{
 		// valid pair
-		{"key", "value", true},
+		{"key", "value", ""},
 		// invalid, blank key
-		{"", "value", false},
+		{"", "value", "cannot be blank"},
 		// allowed special chars in key name
-		{"k_e-y", "value", true},
+		{"k_e-y", "value", ""},
 		// disallowed special chars in key name
-		{"(%key&)", "value", false},
+		{"(%key&)", "value", "invalid characters"},
 		// key too long
-		{longKey, "value", false},
+		{longKey, "value", "Key is too long"},
 		// reserved prefix
-		{metaKeyReservedPrefix + "key", "value", false},
+		{metaKeyReservedPrefix + "key", "value", "reserved for internal use"},
 		// value too long
-		{"key", longValue, false},
+		{"key", longValue, "Value is too long"},
 	}
 
 	for _, pair := range pairs {
 		err := validateMetaPair(pair.Key, pair.Value)
-		if pair.Success != (err == nil) {
+		if pair.Error == "" && err != nil {
-			t.Fatalf("bad: %v, %v", pair, err)
+			t.Fatalf("should have succeeded: %v, %v", pair, err)
+		} else if pair.Error != "" && !strings.Contains(err.Error(), pair.Error) {
+			t.Fatalf("should have failed: %v, %v", pair, err)
 		}
 	}
 }

command/agent/catalog_endpoint.go

@@ -64,7 +64,7 @@ func (s *HTTPServer) CatalogNodes(resp http.ResponseWriter, req *http.Request) (
 	// Setup the request
 	args := structs.DCSpecificRequest{}
 	s.parseSource(req, &args.Source)
-	s.parseMetaFilter(req, &args.NodeMetaKey, &args.NodeMetaValue)
+	args.NodeMetaFilters = s.parseMetaFilter(req)
 	if done := s.parse(resp, req, &args.Datacenter, &args.QueryOptions); done {
 		return nil, nil
 	}
@@ -86,7 +86,7 @@ func (s *HTTPServer) CatalogNodes(resp http.ResponseWriter, req *http.Request) (
 func (s *HTTPServer) CatalogServices(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
 	// Set default DC
 	args := structs.DCSpecificRequest{}
-	s.parseMetaFilter(req, &args.NodeMetaKey, &args.NodeMetaValue)
+	args.NodeMetaFilters = s.parseMetaFilter(req)
 	if done := s.parse(resp, req, &args.Datacenter, &args.QueryOptions); done {
 		return nil, nil
 	}

command/agent/command.go

@@ -364,6 +364,12 @@ func (c *Command) readConfig() *Config {
 		}
 	}
 
+	// Verify the node metadata entries are valid
+	if err := validateMetadata(config.Meta); err != nil {
+		c.Ui.Error(fmt.Sprintf("Failed to parse node metadata: %v", err))
+		return nil
+	}
+
 	// Set the version info
 	config.Revision = c.Revision
 	config.Version = c.Version
@@ -1081,10 +1087,7 @@ func (c *Command) handleReload(config *Config) (*Config, error) {
 		errs = multierror.Append(errs, fmt.Errorf("Failed unloading checks: %s", err))
 		return nil, errs
 	}
-	if err := c.agent.unloadMetadata(); err != nil {
+	c.agent.unloadMetadata()
-		errs = multierror.Append(errs, fmt.Errorf("Failed unloading metadata: %s", err))
-		return nil, errs
-	}
 
 	// Reload service/check definitions and metadata.
 	if err := c.agent.loadServices(newConf); err != nil {

command/agent/config.go

@@ -342,7 +342,9 @@ type Config struct {
 	// they are configured with TranslateWanAddrs set to true.
 	TaggedAddresses map[string]string
 
-	// Node metadata
+	// Node metadata key/value pairs. These are excluded from JSON output
+	// because they can be reloaded and might be stale when shown from the
+	// config instead of the local state.
 	Meta map[string]string `mapstructure:"node_meta" json:"-"`
 
 	// LeaveOnTerm controls if Serf does a graceful leave when receiving

command/agent/http.go

@@ -586,14 +586,20 @@ func (s *HTTPServer) parseSource(req *http.Request, source *structs.QuerySource)
 
 // parseMetaFilter is used to parse the ?node-meta=key:value query parameter, used for
 // filtering results to nodes with the given metadata key/value
-func (s *HTTPServer) parseMetaFilter(req *http.Request, key *string, value *string) {
+func (s *HTTPServer) parseMetaFilter(req *http.Request) map[string]string {
-	if filter, ok := req.URL.Query()["node-meta"]; ok && len(filter) > 0 {
+	if filterList, ok := req.URL.Query()["node-meta"]; ok {
-		pair := strings.SplitN(filter[0], ":", 2)
+		filters := make(map[string]string)
-		*key = pair[0]
+		for _, filter := range filterList {
-		if len(pair) == 2 {
+			pair := strings.SplitN(filter, ":", 2)
-			*value = pair[1]
+			if len(pair) == 2 {
+				filters[pair[0]] = pair[1]
+			} else {
+				filters[pair[0]] = ""
+			}
 		}
+		return filters
 	}
+	return nil
 }
 
 // parse is a convenience method for endpoints that need

consul/catalog_endpoint.go

@@ -166,8 +166,8 @@ func (c *Catalog) ListNodes(args *structs.DCSpecificRequest, reply *structs.Inde
 			var index uint64
 			var nodes structs.Nodes
 			var err error
-			if args.NodeMetaKey != "" {
+			if len(args.NodeMetaFilters) > 0 {
-				index, nodes, err = state.NodesByMeta(args.NodeMetaKey, args.NodeMetaValue)
+				index, nodes, err = state.NodesByMeta(args.NodeMetaFilters)
 			} else {
 				index, nodes, err = state.Nodes()
 			}
@@ -199,8 +199,8 @@ func (c *Catalog) ListServices(args *structs.DCSpecificRequest, reply *structs.I
 			var index uint64
 			var services structs.Services
 			var err error
-			if args.NodeMetaKey != "" {
+			if len(args.NodeMetaFilters) > 0 {
-				index, services, err = state.ServicesByNodeMeta(args.NodeMetaKey, args.NodeMetaValue)
+				index, services, err = state.ServicesByNodeMeta(args.NodeMetaFilters)
 			} else {
 				index, services, err = state.Services()
 			}

consul/catalog_endpoint_test.go

@@ -599,18 +599,6 @@ func TestCatalog_ListNodes_MetaFilter(t *testing.T) {
 	codec := rpcClient(t, s1)
 	defer codec.Close()
 
-	// Filter by a specific meta k/v pair
-	args := structs.DCSpecificRequest{
-		Datacenter:    "dc1",
-		NodeMetaKey:   "somekey",
-		NodeMetaValue: "somevalue",
-	}
-	var out structs.IndexedNodes
-	err := msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &out)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-
 	testutil.WaitForLeader(t, s1.RPC, "dc1")
 
 	// Add a new node with the right meta k/v pair
@@ -619,6 +607,15 @@ func TestCatalog_ListNodes_MetaFilter(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
+	// Filter by a specific meta k/v pair
+	args := structs.DCSpecificRequest{
+		Datacenter: "dc1",
+		NodeMetaFilters: map[string]string{
+			"somekey": "somevalue",
+		},
+	}
+	var out structs.IndexedNodes
+
 	testutil.WaitForResult(func() (bool, error) {
 		msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &out)
 		return len(out.Nodes) == 1, nil
@@ -639,12 +636,13 @@ func TestCatalog_ListNodes_MetaFilter(t *testing.T) {
 
 	// Now filter on a nonexistent meta k/v pair
 	args = structs.DCSpecificRequest{
-		Datacenter:    "dc1",
+		Datacenter: "dc1",
-		NodeMetaKey:   "somekey",
+		NodeMetaFilters: map[string]string{
-		NodeMetaValue: "invalid",
+			"somekey": "invalid",
+		},
 	}
 	out = structs.IndexedNodes{}
-	err = msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &out)
+	err := msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &out)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -1069,18 +1067,6 @@ func TestCatalog_ListServices_MetaFilter(t *testing.T) {
 	codec := rpcClient(t, s1)
 	defer codec.Close()
 
-	// Filter by a specific meta k/v pair
-	args := structs.DCSpecificRequest{
-		Datacenter:    "dc1",
-		NodeMetaKey:   "somekey",
-		NodeMetaValue: "somevalue",
-	}
-	var out structs.IndexedServices
-	err := msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &args, &out)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-
 	testutil.WaitForLeader(t, s1.RPC, "dc1")
 
 	// Add a new node with the right meta k/v pair
@@ -1093,6 +1079,14 @@ func TestCatalog_ListServices_MetaFilter(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
+	// Filter by a specific meta k/v pair
+	args := structs.DCSpecificRequest{
+		Datacenter: "dc1",
+		NodeMetaFilters: map[string]string{
+			"somekey": "somevalue",
+		},
+	}
+	var out structs.IndexedServices
 	if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &args, &out); err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -1112,12 +1106,13 @@ func TestCatalog_ListServices_MetaFilter(t *testing.T) {
 
 	// Now filter on a nonexistent meta k/v pair
 	args = structs.DCSpecificRequest{
-		Datacenter:    "dc1",
+		Datacenter: "dc1",
-		NodeMetaKey:   "somekey",
+		NodeMetaFilters: map[string]string{
-		NodeMetaValue: "invalid",
+			"somekey": "invalid",
+		},
 	}
 	out = structs.IndexedServices{}
-	err = msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &args, &out)
+	err := msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &args, &out)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}

consul/state/state_store.go

@@ -550,7 +550,10 @@ func (s *StateStore) Nodes() (uint64, structs.Nodes, error) {
 }
 
 // NodesByMeta is used to return all nodes with the given meta key/value pair.
-func (s *StateStore) NodesByMeta(key, value string) (uint64, structs.Nodes, error) {
+func (s *StateStore) NodesByMeta(filters map[string]string) (uint64, structs.Nodes, error) {
+	if len(filters) > 1 {
+		return 0, nil, fmt.Errorf("multiple meta filters not supported")
+	}
 	tx := s.db.Txn(false)
 	defer tx.Abort()
 
@@ -558,7 +561,11 @@ func (s *StateStore) NodesByMeta(key, value string) (uint64, structs.Nodes, erro
 	idx := maxIndexTxn(tx, s.getWatchTables("Nodes")...)
 
 	// Retrieve all of the nodes
-	nodes, err := tx.Get("nodes", "meta", key, value)
+	var args []interface{}
+	for key, value := range filters {
+		args = append(args, key, value)
+	}
+	nodes, err := tx.Get("nodes", "meta", args...)
 	if err != nil {
 		return 0, nil, fmt.Errorf("failed nodes lookup: %s", err)
 	}
@@ -781,8 +788,11 @@ func (s *StateStore) Services() (uint64, structs.Services, error) {
 	return idx, results, nil
 }
 
-// Services returns all services, filtered by given node metadata.
+// Services returns all services, filtered by the given node metadata.
-func (s *StateStore) ServicesByNodeMeta(key, value string) (uint64, structs.Services, error) {
+func (s *StateStore) ServicesByNodeMeta(filters map[string]string) (uint64, structs.Services, error) {
+	if len(filters) > 1 {
+		return 0, nil, fmt.Errorf("multiple meta filters not supported")
+	}
 	tx := s.db.Txn(false)
 	defer tx.Abort()
 
@@ -790,7 +800,11 @@ func (s *StateStore) ServicesByNodeMeta(key, value string) (uint64, structs.Serv
 	idx := maxIndexTxn(tx, s.getWatchTables("ServiceNodes")...)
 
 	// Retrieve all of the nodes with the meta k/v pair
-	nodes, err := tx.Get("nodes", "meta", key, value)
+	var args []interface{}
+	for key, value := range filters {
+		args = append(args, key, value)
+	}
+	nodes, err := tx.Get("nodes", "meta", args...)
 	if err != nil {
 		return 0, nil, fmt.Errorf("failed nodes lookup: %s", err)
 	}

consul/state/state_store_test.go

@@ -759,7 +759,7 @@ func TestStateStore_GetNodesByMeta(t *testing.T) {
 	s := testStateStore(t)
 
 	// Listing with no results returns nil
-	idx, res, err := s.NodesByMeta("somekey", "somevalue")
+	idx, res, err := s.NodesByMeta(map[string]string{"somekey": "somevalue"})
 	if idx != 0 || res != nil || err != nil {
 		t.Fatalf("expected (0, nil, nil), got: (%d, %#v, %#v)", idx, res, err)
 	}
@@ -775,7 +775,7 @@ func TestStateStore_GetNodesByMeta(t *testing.T) {
 	}
 
 	// Retrieve the node with role=client
-	idx, nodes, err := s.NodesByMeta("role", "client")
+	idx, nodes, err := s.NodesByMeta(map[string]string{"role": "client"})
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
@@ -800,7 +800,7 @@ func TestStateStore_GetNodesByMeta(t *testing.T) {
 	}
 
 	// Retrieve both nodes via their common meta field
-	idx, nodes, err = s.NodesByMeta("common", "1")
+	idx, nodes, err = s.NodesByMeta(map[string]string{"common": "1"})
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
@@ -1160,7 +1160,7 @@ func TestStateStore_ServicesByNodeMeta(t *testing.T) {
 	s := testStateStore(t)
 
 	// Listing with no results returns nil
-	idx, res, err := s.ServicesByNodeMeta("somekey", "somevalue")
+	idx, res, err := s.ServicesByNodeMeta(map[string]string{"somekey": "somevalue"})
 	if idx != 0 || len(res) != 0 || err != nil {
 		t.Fatalf("expected (0, nil, nil), got: (%d, %#v, %#v)", idx, res, err)
 	}
@@ -1196,7 +1196,7 @@ func TestStateStore_ServicesByNodeMeta(t *testing.T) {
 	}
 
 	// Filter the services by the first node's meta value
-	idx, res, err = s.ServicesByNodeMeta("role", "client")
+	idx, res, err = s.ServicesByNodeMeta(map[string]string{"role": "client"})
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
@@ -1211,7 +1211,7 @@ func TestStateStore_ServicesByNodeMeta(t *testing.T) {
 	}
 
 	// Get all services using the common meta value
-	idx, res, err = s.ServicesByNodeMeta("common", "1")
+	idx, res, err = s.ServicesByNodeMeta(map[string]string{"common": "1"})
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}

consul/structs/structs.go

@@ -229,10 +229,9 @@ type QuerySource struct {
 
 // DCSpecificRequest is used to query about a specific DC
 type DCSpecificRequest struct {
-	Datacenter    string
+	Datacenter      string
-	NodeMetaKey   string
+	NodeMetaFilters map[string]string
-	NodeMetaValue string
+	Source          QuerySource
-	Source        QuerySource
 	QueryOptions
 }
 
@@ -288,6 +287,12 @@ type Node struct {
 }
 type Nodes []*Node
 
+// Used for filtering nodes by metadata key/value pairs
+type NodeMetaFilter struct {
+	Key   string
+	Value string
+}
+
 // Used to return information about a provided services.
 // Maps service name to available tags
 type Services map[string][]string