Validate NodeNames, Address and Tags fields #612
Conversation
schristoff
changed the title
cmd/serf/command/agent/config.go
Outdated
| err = fmt.Errorf("NodeName is %v characters. "+ | ||
| "Valid length is between 1 and 128 characters", len(result.NodeName)) | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
This logic for validating the name is getting repeated 3 times - we should pull it into a function for reuse. We can also avoid doing it in this serf/command/agent code at all and do it here instead: https://github.com/hashicorp/serf/blob/master/serf/serf.go#L235
serf.Create will get called on startup for Consul as well, where the code in this package is only used in the Serf binary.
cmd/serf/command/agent/config.go
Outdated
| @@ -363,7 +367,9 @@ func containsKey(keys []string, key string) bool { | |||
| func MergeConfig(a, b *Config) *Config { | |||
| var result Config = *a | |||
|
|
|||
| // Copy the strings if they're set | |||
| //TODO(schristoff): do i need to check nodename | |||
There was a problem hiding this comment.
I think we can remove this todo (since we're verifying the final config that gets passed to Create() anyway)
serf/serf.go
Outdated
| // then runs validations | ||
| func (s *Serf) ValidateNodeNames() error { | ||
| if s.config.ValidateNodeNames { | ||
| var InvalidNameRe = regexp.MustCompile(`[^A-Za-z0-9\-\.\\]+`) |
There was a problem hiding this comment.
I think the regexp should be [^A-Za-z0-9\-\.]+ - right now it's allowing \ characters
serf/serf.go
Outdated
| var InvalidNameRe = regexp.MustCompile(`[^A-Za-z0-9\-\.\\]+`) | ||
| if InvalidNameRe.MatchString(s.config.NodeName) { | ||
| return fmt.Errorf("NodeName contains invalid characters %v , Valid characters include "+ | ||
| "all alpha-numerics and dashes.", s.config.NodeName) |
There was a problem hiding this comment.
We should specify alphanumerics, dashes and .s. Same with the comments for ValidateNodeNames above.
We've been seeing an issue where NodeNames will start to get very long and filled with junk.
Fields have been added to validate memberinfo in messages are within certain restrictions.
A configuration flag titled
ValidateNodeNameshas been added to give users the option of restricting nodename length.The Address field in the memberinfo in messages must adhere to an IP:Port structure.
The Tags field in memberinfo in messages must adhere to the 512 max (which is placed elsewhere too)
ValidateNodeNames restricts NodeNames to alphanumeric and 128 characters.