Add validations to Egress router script #15249
Conversation
pravisankar
commented
Jul 17, 2017
pravisankar
commented
- In case of multiple destinations, ensure unique local port is redirected to each destination
- Early port validation instead of implicitly depending on iptables
|
@openshift/networking PTAL |
|
[test] |
| if [[ "${dest}" =~ ^${IP_REGEX}$ ]]; then | ||
| # single IP address: do fallback "all ports to same IP" | ||
| echo -A PREROUTING -i eth0 -j DNAT --to-destination "${dest}" | ||
| did_fallback=1 | ||
|
|
||
| elif [[ "${dest}" =~ ^${PORT_REGEX}\ +${PROTO_REGEX}\ +${IP_REGEX}$ ]]; then | ||
| read localport proto destip <<< "${dest}" | ||
| localport_set=1 |
There was a problem hiding this comment.
meh... can see here that there's tabs-vs-spaces inconsistency in this file. maybe add another commit fully untabifying?
|
|
||
| fi | ||
|
|
||
| if [[ "${localport_set}" == "1" ]]; then |
There was a problem hiding this comment.
you don't really need localport_set; if you just set localport="" at the top of the loop, you could check [[ -n "${localport}" ]] here
| if [[ "${localport_set}" == "1" ]]; then | ||
| validate_port ${localport} | ||
|
|
||
| if [[ "${used_ports[${localport}]:-x}" == "x" ]]; then |
There was a problem hiding this comment.
you can remove both xs in this line
There was a problem hiding this comment.
Removing x in this line returns used_ports[${localport}]: unbound variable, I guess this is needed because we are using strict mode(set -o nounset).
There was a problem hiding this comment.
Did you remove the :- too? [[ "${used_ports[${localport}]:-}" = "" ]] should work.
[[ -z "${used_ports[${localport}]:-}" ]] would be more idiomatic.
pravisankar
force-pushed
the
egress-router-validations
branch
from
8e1beba to
8af5df6
Compare
|
Evaluated for origin test up to 8af5df6 |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/3242/) (Base Commit: 20e72f7) (PR Branch Commit: 8af5df6) |
openshift-merge-robot
added
the
size/L
pravisankar
force-pushed
the
egress-router-validations
branch
from
8af5df6 to
f4a3e3b
Compare
openshift-merge-robot
added
approved
- In case of multiple destinations, ensure unique local port is redirected to each destination - Early port validation instead of implicitly depending on iptables
pravisankar
force-pushed
the
egress-router-validations
branch
from
f4a3e3b to
0dc3ee0
Compare
|
/retest |
|
Automatic merge from submit-queue |
0xmichalis
added
the
approved
