Add port validator to ensure configurable ports are valid #7037
Conversation
There was a problem hiding this comment.
are these all the port numbers we want to validate? what about tunnelPort (we mentioned it in the issue)?
also you are not referencing issue #7035
cmd/antrea-agent/util.go
Outdated
| @@ -79,3 +79,11 @@ func parsePortRange(portRangeStr string) (start, end int, err error) { | |||
|
|
|||
| return start, end, nil | |||
| } | |||
|
|
|||
| // isValidPort checks if the given port number is within the valid range of 1 to 65535. | |||
| func isValidPort(port int) bool { | |||
There was a problem hiding this comment.
doesn't need to be "exported"?
There was a problem hiding this comment.
It's not required at the moment, but I will go through and double check if I missed any port beside tunnelPort. I have updated the changes for tunnelPort.
luolanzone
force-pushed
the
validate-ports
branch
5 times, most recently
from
699efa6 to
069be9b
Compare
Signed-off-by: Lan Luo <lan.luo@broadcom.com>
luolanzone
force-pushed
the
validate-ports
branch
from
069be9b to
f310d4f
Compare
| @@ -193,7 +195,7 @@ func (o *Options) setDefaults() { | |||
| if o.config.OVSRunDir == "" { | |||
| o.config.OVSRunDir = ovsconfig.DefaultOVSRunDir | |||
| } | |||
| if o.config.APIPort == 0 { | |||
| if !checks.IsValidPort(o.config.APIPort) { | |||
There was a problem hiding this comment.
wouldn't it be better to give an error if the port is not valid (and non-zero), like you do for tunnelPort below?
There was a problem hiding this comment.
maybe pkg/util/config/validate.go would be a better location for this function?
| if !checks.IsValidPort(portNum) || err != nil { | ||
| port = "443" | ||
| } |
There was a problem hiding this comment.
here also it seems like silently dropping an error may not be the right thing to do?
maybe the function could return an error?
Resolves #7035