Skip to content

Commit 9eb78c3

Browse files
yuvipandayuvipanda
authored
Merge pull request #534 from betatim/test-test-test
[MRG] Add tests for port mapping conversion
2 parents 18e9d2f + 498b09e commit 9eb78c3

File tree

4 files changed

+73
-63
lines changed

4 files changed

+73
-63
lines changed

‎repo2docker/app.py‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,7 @@
3333
CondaBuildPack, JuliaBuildPack, RBuildPack, NixBuildPack
3434
)
3535
from . import contentproviders
36-
from .utils import (
37-
ByteSpecification, is_valid_docker_image_name,
38-
validate_and_generate_port_mapping, chdir
39-
)
36+
from .utils import ByteSpecification, chdir
4037

4138

4239
class Repo2Docker(Application):

‎repo2docker/utils.py‎

Lines changed: 44 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -68,24 +68,18 @@ def chdir(path):
6868
os.chdir(old_dir)
6969

7070

71-
def validate_and_generate_port_mapping(port_mapping):
71+
def validate_and_generate_port_mapping(port_mappings):
7272
"""
73-
Validate the port mapping list and return a list of validated tuples.
74-
75-
Each entry in the passed port mapping list will be converted to a
76-
tuple with a containing a string with the format 'key:value' with the
77-
`key` being the container's port and the
78-
`value` being `None`, `host_port` or `['interface_ip','host_port']`
79-
73+
Validate a list of port mappings and return a dictionary of port mappings.
8074
8175
Args:
82-
port_mapping (list): List of strings of format
76+
port_mappings (list): List of strings of format
8377
`'host_port:container_port'` with optional tcp udp values and host
8478
network interface
8579
8680
Returns:
87-
List of validated tuples of form ('host_port:container_port') with
88-
optional tcp udp values and host network interface
81+
Dictionary of port mappings in the format accepted by docker-py's
82+
`containers.run()` method (https://docker-py.readthedocs.io/en/stable/containers.html)
8983
9084
Raises:
9185
Exception on invalid port mapping
@@ -94,55 +88,50 @@ def validate_and_generate_port_mapping(port_mapping):
9488
One limitation of repo2docker is it cannot bind a
9589
single container_port to multiple host_ports
9690
(docker-py supports this but repo2docker does not)
97-
98-
Examples:
99-
Valid port mappings are:
100-
- `127.0.0.1:90:900`
101-
- `:999` (match to any host port)
102-
- `999:999/tcp` (bind 999 host port to 999 tcp container port)
103-
104-
Invalid port mapping:
105-
- `127.0.0.1::999` (even though docker accepts it)
106-
- other invalid ip address combinations
10791
"""
108-
reg_regex = re.compile(r"""
109-
^(
110-
( # or capturing group
111-
(?: # start capturing ip address of network interface
112-
(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3} # first three parts
113-
(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?) # last part of the ip address
114-
:(?:6553[0-5]|655[0-2][0-9]|65[0-4](\d){2}|6[0-4](\d){3}|[1-5](\d){4}|(\d){1,4})
115-
)?
116-
| # host ip with port or only port
117-
(?:6553[0-5]|655[0-2][0-9]|65[0-4](\d){2}|6[0-4](\d){3}|[1-5](\d){4}|(\d){0,4})
118-
)
119-
:
120-
(?:6553[0-5]|655[0-2][0-9]|65[0-4](\d){2}|6[0-4](\d){3}|[1-5](\d){4}|(\d){0,4})
121-
(?:/udp|/tcp)?
122-
)$
123-
""", re.VERBOSE)
92+
def check_port(port):
93+
try:
94+
p = int(port)
95+
except ValueError as e:
96+
raise ValueError('Port specification "{}" has '
97+
'an invalid port.'.format(mapping))
98+
if p > 65535:
99+
raise ValueError('Port specification "{}" specifies '
100+
'a port above 65535.'.format(mapping))
101+
return port
102+
103+
def check_port_string(p):
104+
parts = p.split('/')
105+
if len(parts) == 2: # 134/tcp
106+
port, protocol = parts
107+
if protocol not in ('tcp', 'udp'):
108+
raise ValueError('Port specification "{}" has '
109+
'an invalid protocol.'.format(mapping))
110+
elif len(parts) == 1:
111+
port = parts[0]
112+
protocol = 'tcp'
113+
114+
check_port(port)
115+
116+
return '/'.join((port, protocol))
117+
124118
ports = {}
125-
if port_mapping is None:
119+
if port_mappings is None:
126120
return ports
127-
for p in port_mapping:
128-
if reg_regex.match(p) is None:
129-
raise Exception('Invalid port mapping ' + str(p))
130-
# Do a reverse split twice on the separator :
131-
port_host = str(p).rsplit(':', 2)
132-
host = None
133-
if len(port_host) == 3:
134-
# host, optional host_port and container port information given
135-
host = port_host[0]
136-
host_port = port_host[1]
137-
container_port = port_host[2]
138-
else:
139-
host_port = port_host[0] if len(port_host[0]) > 0 else None
140-
container_port = port_host[1]
141121

142-
if host is None:
143-
ports[str(container_port)] = host_port
122+
for mapping in port_mappings:
123+
parts = mapping.split(':')
124+
125+
*host, container_port = parts
126+
# just a port
127+
if len(host) == 1:
128+
host = check_port(host[0])
144129
else:
145-
ports[str(container_port)] = (host, host_port)
130+
host = tuple((host[0], check_port(host[1])))
131+
132+
container_port = check_port_string(container_port)
133+
ports[container_port] = host
134+
146135
return ports
147136

148137

‎tests/unit/test_argumentvalidation.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ def test_invalid_port_mapping_fail(temp_cwd):
190190
# builddir passed in the function will be an argument for the run command
191191
args_list = ['-p', '75000:80', builddir, 'ls']
192192

193-
assert not validate_arguments(builddir, args_list, 'Invalid port mapping')
193+
assert not validate_arguments(builddir, args_list, 'Port specification')
194194

195195

196196
def test_invalid_protocol_port_mapping_fail(temp_cwd):
@@ -201,7 +201,7 @@ def test_invalid_protocol_port_mapping_fail(temp_cwd):
201201
# builddir passed in the function will be an argument for the run command
202202
args_list = ['-p', '80/tpc:8000', builddir, 'ls']
203203

204-
assert not validate_arguments(builddir, args_list, 'Invalid port mapping')
204+
assert not validate_arguments(builddir, args_list, 'Port specification')
205205

206206

207207
def test_invalid_container_port_protocol_mapping_fail(temp_cwd):
@@ -212,7 +212,7 @@ def test_invalid_container_port_protocol_mapping_fail(temp_cwd):
212212
# builddir passed in the function will be an argument for the run command
213213
args_list = ['-p', '80:8000/upd', builddir, 'ls']
214214

215-
assert not validate_arguments(builddir, args_list, 'Invalid port mapping')
215+
assert not validate_arguments(builddir, args_list, 'Port specification')
216216

217217

218218
@pytest.mark.xfail(reason="Regression in new arg parsing")

‎tests/unit/test_utils.py‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
"""
44
import traitlets
55
import os
6-
from tempfile import TemporaryDirectory
76
from repo2docker import utils
87
import pytest
98
import subprocess
@@ -16,6 +15,7 @@ def test_capture_cmd_no_capture_success():
1615
]):
1716
pass
1817

18+
1919
def test_capture_cmd_no_capture_fail():
2020
with pytest.raises(subprocess.CalledProcessError):
2121
for line in utils.execute_cmd([
@@ -64,3 +64,27 @@ def test_byte_spec_validation():
6464

6565
with pytest.raises(traitlets.TraitError):
6666
bs.validate(None, '1m')
67+
68+
69+
@pytest.mark.parametrize("input,expected", [
70+
(["8888:8888"], {'8888/tcp': '8888'}),
71+
(["8888:4321"], {'4321/tcp': '8888'}),
72+
(["8888:4321/udp"], {'4321/udp': '8888'}),
73+
(["8888:4321/udp", "8888:4321/tcp"], {'4321/udp': '8888',
74+
'4321/tcp': '8888'}),
75+
(['127.0.0.1:80:8000'], {'8000/tcp': ('127.0.0.1', '80')}),
76+
(["8888:4321", "1234:12345"], {'4321/tcp': '8888', '12345/tcp': '1234'}),
77+
])
78+
def test_valid_port_mapping(input, expected):
79+
actual = utils.validate_and_generate_port_mapping(input)
80+
assert actual == expected
81+
82+
83+
@pytest.mark.parametrize("port_spec", [
84+
"a8888:8888", "888:888/abc"
85+
])
86+
def test_invalid_port_mapping(port_spec):
87+
with pytest.raises(ValueError) as e:
88+
utils.validate_and_generate_port_mapping([port_spec])
89+
90+
assert 'Port specification "{}"'.format(port_spec) in str(e.value)

0 commit comments

Comments
 (0)