validate backend #1908
validate backend #1908
Conversation
rawdaGastan
requested review from
muhamadazmy,
delandtj,
mariobassem,
ashraffouda and
xmonader
as code owners
pkg/gridtypes/zos/gw_fqdn.go
Outdated
| if err != nil { | ||
| return fmt.Errorf("failed to parse backend with error: %w", err) | ||
| } | ||
| if port == "" { |
There was a problem hiding this comment.
This is not necessary, if you check the code for the SplitHostPort function you see that it already return error if the format is wrong or if the port is empty.
But it does validate that the IP value or the Port value are valid. hence parsing of the splited ip, and port still need to be done of course
pkg/gridtypes/zos/gw_fqdn.go
Outdated
| if u.Port() == "" { | ||
| return fmt.Errorf("missing port in backend address") | ||
|
|
||
| parsedPort, err := strconv.ParseInt(port, 10, 64) |
There was a problem hiding this comment.
use ParseUint instead, since a negative value is illegal.
There was a problem hiding this comment.
Okay, this looks much better. Except this is only half the work. This only fix the validation issue. but in the code where the backend actually being used is still assuming it's always a URL.
Please check the code here
https://github.com/threefoldtech/zos/blob/main/pkg/gateway/gateway.go#L569
If you need to change few things so the validation and processing of the Backend value is not replicated everywhere.
I suggest replace the Valid function with instead some another function (say Parse(tls bool), or something else that return (string, error) where the error is return if it's not valid, but the string is the value that need to be used as backend in the configuration
| return "", fmt.Errorf("port '%s' must be <= 65535", port) | ||
| } | ||
|
|
||
| hostName = host |
There was a problem hiding this comment.
this is wrong because you need to return ip:port in case of tls, or http://ip:port in case of no tls
pkg/gateway/gateway.go
Outdated
| } | ||
| u, err := url.Parse(backend) | ||
| log.Debug().Str("hostname", u.Host).Str("backend", backend).Msg("tls passthrough") | ||
| host, err := zos.Backend(backend).Parse(true) |
There was a problem hiding this comment.
this is still wrong because instead it can be like
for idx, backend := range backends {
u, err := zos.Backend(backend).Parse(TLSPassthrout)
// check error
if TLSPassthrough {
servers[ids] = Server{ Address: u}
} else {
servers[idx] = Server{ Url: u}
}
}
Related issues