Validate metadata config earlier and handle multiple filters

This commit is contained in:
Kyle Havlovitz 2017-01-11 14:41:12 -05:00
parent 03273e4ed2
commit c9e430c396
No known key found for this signature in database
GPG Key ID: 8A5E6B173056AD6C
11 changed files with 132 additions and 105 deletions

View File

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

View File

@ -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), "")
longValue := fmt.Sprintf(fmt.Sprintf("%%%ds", metaValueMaxLength+1), "")
longKey := strings.Repeat("a", metaKeyMaxLength+1)
longValue := strings.Repeat("b", metaValueMaxLength+1)
pairs := []struct {
Key string
Value string
Success bool
Key string
Value string
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) {
t.Fatalf("bad: %v, %v", pair, err)
if pair.Error == "" && err != nil {
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)
}
}
}

View File

@ -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
}

View File

@ -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 {
errs = multierror.Append(errs, fmt.Errorf("Failed unloading metadata: %s", err))
return nil, errs
}
c.agent.unloadMetadata()
// Reload service/check definitions and metadata.
if err := c.agent.loadServices(newConf); err != nil {

View File

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

View File

@ -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) {
if filter, ok := req.URL.Query()["node-meta"]; ok && len(filter) > 0 {
pair := strings.SplitN(filter[0], ":", 2)
*key = pair[0]
if len(pair) == 2 {
*value = pair[1]
func (s *HTTPServer) parseMetaFilter(req *http.Request) map[string]string {
if filterList, ok := req.URL.Query()["node-meta"]; ok {
filters := make(map[string]string)
for _, filter := range filterList {
pair := strings.SplitN(filter, ":", 2)
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

View File

@ -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 != "" {
index, nodes, err = state.NodesByMeta(args.NodeMetaKey, args.NodeMetaValue)
if len(args.NodeMetaFilters) > 0 {
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 != "" {
index, services, err = state.ServicesByNodeMeta(args.NodeMetaKey, args.NodeMetaValue)
if len(args.NodeMetaFilters) > 0 {
index, services, err = state.ServicesByNodeMeta(args.NodeMetaFilters)
} else {
index, services, err = state.Services()
}

View File

@ -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",
NodeMetaKey: "somekey",
NodeMetaValue: "invalid",
Datacenter: "dc1",
NodeMetaFilters: map[string]string{
"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",
NodeMetaKey: "somekey",
NodeMetaValue: "invalid",
Datacenter: "dc1",
NodeMetaFilters: map[string]string{
"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)
}

View File

@ -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.
func (s *StateStore) ServicesByNodeMeta(key, value string) (uint64, structs.Services, error) {
// Services returns all services, filtered by the given node metadata.
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)
}

View File

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

View File

@ -229,10 +229,9 @@ type QuerySource struct {
// DCSpecificRequest is used to query about a specific DC
type DCSpecificRequest struct {
Datacenter string
NodeMetaKey string
NodeMetaValue string
Source QuerySource
Datacenter string
NodeMetaFilters map[string]string
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