Validate Service.Spec.publicIPs to be a valid IP that is not a localhost #5508
Conversation
| for _, ip := range service.Spec.PublicIPs { | ||
| if !util.IsValidIP(ip) || ip == "0.0.0.0" { | ||
| allErrs = append(allErrs, errs.NewFieldInvalid("spec.publicIPs", ip, "is not an IP address")) | ||
| } else if ip == "127.0.0.1" { |
There was a problem hiding this comment.
How about
if net.ParseIP(ip).IsLoopback()
Being loopback is more than just 127.0.0.1.
In fact, http://golang.org/pkg/net/#ParseIP has a bunch of conditions that are probably worth checking. Maybe
if !net.ParseIP(ip).IsGlobalUnicast() {
There was a problem hiding this comment.
That simplifies logic a lot! Thanks!
Regarding IsGlobalUnicast() - isn't it only for IPv6 which we don't support anyway? IsValidIP returns true only for IPv4 addresses.
fgrzadkowski
force-pushed
the
validate_ips
branch
2 times, most recently
from
ca03867 to
385a22b
Compare
|
New version pushed and synced to HEAD. |
fgrzadkowski
force-pushed
the
validate_ips
branch
from
385a22b to
93ddc96
Compare
|
Ping. |
|
LGTM - but needs a rebase before merge please. |
fgrzadkowski
force-pushed
the
validate_ips
branch
from
93ddc96 to
24eb1a0
Compare
|
Rebased. |
| @@ -94,3 +95,8 @@ func IsCIdentifier(value string) bool { | |||
| func IsValidPortNum(port int) bool { | |||
| return 0 < port && port < 65536 | |||
| } | |||
|
|
|||
| // IsValidIP tests that the argument is a valid IPv4 address. | |||
| func IsValidIP(value string) bool { | |||
There was a problem hiding this comment.
Can you rename it to IsValidIPv4, to make it more explicit.
There was a problem hiding this comment.
To be honest I'd prefer to keep the name as is, because from k8s perspective this verifies whether it's a valid IP (because k8s works only for v4).
Validate Service.Spec.publicIPs to be a valid IP that is not a localhost
This fixes #5319