enable hostname (not only IP Address) on Metrics statsd host key #2429
Conversation
kangaechu
changed the title
kangaechu
force-pushed
the
allow-use-hostname-in-stats-configuration
branch
from
34c2960 to
96547f8
Compare
| { | ||
| description: "invalid host", | ||
| subject: raw.Metrics{ | ||
| Statsd: &raw.Statsd{ | ||
| Host: "127.0.1", | ||
| Port: "8125", | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
I deleted this test case, because:
"127.0.1" is an invalid IP address, it is also invalid hostname.
In RFC 1034 3.5 Preferred name syntax, a host name is defined as follows:
The labels must follow the rules for ARPANET host names. They must start with a letter, end with a letter or digit, and have as interior characters only letters, digits, and hyphen. There are also some restrictions on the length. Labels must be 63 characters or less.
https://datatracker.ietf.org/doc/html/rfc1034
But the validation library govalidator defines as follows:
https://github.com/asaskevich/govalidator/blob/f21760c49a8d602d863493de796926d2a5c1138d/patterns.go#L33
It allows hostname to start with a digit.
The "127.0.1" test should result in an error, but this requires modifying the govalidator library.
Since it is the govalidator's responsibility to validate the DNSName correctly, atlantis does not perform this test.
|
website_link_check is failed on master branch, too. |
|
#2430 will fix website_link_check error. |
| @@ -28,7 +28,7 @@ func (s *Statsd) Validate() error { | |||
| return validation.ValidateStruct(s, | |||
| validation.Field(&s.Host, validation.Required), | |||
| validation.Field(&s.Port, validation.Required), | |||
| validation.Field(&s.Host, is.IP), | |||
There was a problem hiding this comment.
Why are you removing the ip? and not just adding the Host
There was a problem hiding this comment.
IsHost includes IsIP condition, so I removed IsIP validation.
There was a problem hiding this comment.
@jamengual Would you review this PR?
jamengual
added
bug
kangaechu
force-pushed
the
allow-use-hostname-in-stats-configuration
branch
from
96547f8 to
740405c
Compare
|
Thank you! |
Closes #2398.