Skip to content

Commit c3b6338

Browse files
authored
fix(docker): During cfn submit docker commands were not working on Windows (#208)
* When on windows will not use os.geteuid() but root:root, also if call fails it will add warning message * Change test for docker no euid to work on Windows
1 parent 7262fc4 commit c3b6338

File tree

2 files changed

+118
-2
lines changed

2 files changed

+118
-2
lines changed

python/rpdk/python/codegen.py

+40-2
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,17 @@ def _build(self, base_path):
248248
self._pip_build(base_path)
249249
LOG.debug("Dependencies build finished")
250250

251+
@staticmethod
252+
def _update_pip_command():
253+
return [
254+
"python",
255+
"-m",
256+
"pip",
257+
"install",
258+
"--upgrade",
259+
"pip",
260+
]
261+
251262
@staticmethod
252263
def _make_pip_command(base_path):
253264
return [
@@ -270,7 +281,13 @@ def _get_plugin_information() -> Dict:
270281
@classmethod
271282
def _docker_build(cls, external_path):
272283
internal_path = PurePosixPath("/project")
273-
command = " ".join(cls._make_pip_command(internal_path))
284+
command = (
285+
'/bin/bash -c "'
286+
+ " ".join(cls._update_pip_command())
287+
+ " && "
288+
+ " ".join(cls._make_pip_command(internal_path))
289+
+ '"'
290+
)
274291
LOG.debug("command is '%s'", command)
275292

276293
volumes = {str(external_path): {"bind": str(internal_path), "mode": "rw"}}
@@ -280,6 +297,27 @@ def _docker_build(cls, external_path):
280297
"image '%s' needs to be pulled first.",
281298
image,
282299
)
300+
301+
# Docker will mount the path specified in the volumes variable in the container
302+
# and pip will place all the dependent packages inside the volumes/build path.
303+
# codegen will need access to this directory during package()
304+
try:
305+
# Use root:root for euid:group when on Windows
306+
# https://docs.docker.com/desktop/windows/permission-requirements/#containers-running-as-root-within-the-linux-vm
307+
if os.name == "nt":
308+
localuser = "root:root"
309+
# Try to get current effective user ID and Group ID.
310+
# Only valid on UNIX-like systems
311+
else:
312+
localuser = f"{os.geteuid()}:{os.getgid()}"
313+
# Catch exception if geteuid failed on non-Windows system
314+
# and default to root:root
315+
except AttributeError:
316+
localuser = "root:root"
317+
LOG.warning(
318+
"User ID / Group ID not found. Using root:root for docker build"
319+
)
320+
283321
docker_client = docker.from_env()
284322
try:
285323
logs = docker_client.containers.run(
@@ -289,7 +327,7 @@ def _docker_build(cls, external_path):
289327
volumes=volumes,
290328
stream=True,
291329
entrypoint="",
292-
user=f"{os.geteuid()}:{os.getgid()}",
330+
user=localuser,
293331
)
294332
except RequestsConnectionError as e:
295333
# it seems quite hard to reliably extract the cause from

tests/plugin/codegen_test.py

+78
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,84 @@ def test__build_docker(plugin):
422422
mock_docker.assert_called_once_with(sentinel.base_path)
423423

424424

425+
# Test _build_docker on Linux/Unix-like systems
426+
def test__build_docker_posix(plugin):
427+
plugin._use_docker = True
428+
429+
patch_pip = patch.object(plugin, "_pip_build", autospec=True)
430+
patch_from_env = patch("rpdk.python.codegen.docker.from_env", autospec=True)
431+
patch_os_name = patch("rpdk.python.codegen.os.name", "posix")
432+
433+
with patch_pip as mock_pip, patch_from_env as mock_from_env:
434+
mock_run = mock_from_env.return_value.containers.run
435+
with patch_os_name:
436+
plugin._build(sentinel.base_path)
437+
438+
mock_pip.assert_not_called()
439+
mock_run.assert_called_once_with(
440+
image=ANY,
441+
command=ANY,
442+
auto_remove=True,
443+
volumes={str(sentinel.base_path): {"bind": "/project", "mode": "rw"}},
444+
stream=True,
445+
entrypoint="",
446+
user=ANY,
447+
)
448+
449+
450+
# Test _build_docker on Windows
451+
def test__build_docker_windows(plugin):
452+
plugin._use_docker = True
453+
454+
patch_pip = patch.object(plugin, "_pip_build", autospec=True)
455+
patch_from_env = patch("rpdk.python.codegen.docker.from_env", autospec=True)
456+
patch_os_name = patch("rpdk.python.codegen.os.name", "nt")
457+
458+
with patch_pip as mock_pip, patch_from_env as mock_from_env:
459+
mock_run = mock_from_env.return_value.containers.run
460+
with patch_os_name:
461+
plugin._build(sentinel.base_path)
462+
463+
mock_pip.assert_not_called()
464+
mock_run.assert_called_once_with(
465+
image=ANY,
466+
command=ANY,
467+
auto_remove=True,
468+
volumes={str(sentinel.base_path): {"bind": "/project", "mode": "rw"}},
469+
stream=True,
470+
entrypoint="",
471+
user="root:root",
472+
)
473+
474+
475+
# Test _build_docker if geteuid fails
476+
def test__build_docker_no_euid(plugin):
477+
plugin._use_docker = True
478+
479+
patch_pip = patch.object(plugin, "_pip_build", autospec=True)
480+
patch_from_env = patch("rpdk.python.codegen.docker.from_env", autospec=True)
481+
# os.geteuid does not exist on Windows so we can not autospec os
482+
patch_os = patch("rpdk.python.codegen.os")
483+
patch_os_name = patch("rpdk.python.codegen.os.name", "posix")
484+
485+
with patch_pip as mock_pip, patch_from_env as mock_from_env, patch_os as mock_patch_os: # noqa: B950 pylint: disable=line-too-long
486+
mock_run = mock_from_env.return_value.containers.run
487+
mock_patch_os.geteuid.side_effect = AttributeError()
488+
with patch_os_name:
489+
plugin._build(sentinel.base_path)
490+
491+
mock_pip.assert_not_called()
492+
mock_run.assert_called_once_with(
493+
image=ANY,
494+
command=ANY,
495+
auto_remove=True,
496+
volumes={str(sentinel.base_path): {"bind": "/project", "mode": "rw"}},
497+
stream=True,
498+
entrypoint="",
499+
user="root:root",
500+
)
501+
502+
425503
def test__docker_build_good_path(plugin, tmp_path):
426504
patch_from_env = patch("rpdk.python.codegen.docker.from_env", autospec=True)
427505

0 commit comments

Comments
 (0)