fix(docker): get container logs even on error

.gitattributes

@@ -0,0 +1,2 @@
+# Set the default behavior, in case people don't have core.autocrlf set.
+* text eol=lf

.github/workflows/pr-ci.yml

@@ -3,32 +3,44 @@
 
 name: CloudFormation Python Plugin CI
 
-on:
-  push:
-    branches: [ master ]
-  pull_request:
-    branches: [ master ]
+on: [push, pull_request]
 
 jobs:
   build:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python: [ 3.6, 3.7, 3.8 ]
+    steps:
+    - name: Set up Python ${{ matrix.python }}
+      uses: actions/setup-python@v4
+      with:
+        python-version: ${{ matrix.python }}
+  os_build:
     env:
       AWS_DEFAULT_REGION: us-east-1
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.os }}
     strategy:
       matrix:
-        python: [3.6, 3.7, 3.8]
+        os: [ubuntu-latest, macos-latest, windows-latest]
+        python: [ 3.6, 3.7, 3.8, 3.9 ]
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v3
     - name: Set up Python ${{ matrix.python }}
-      uses: actions/setup-python@v2
+      uses: actions/setup-python@v4
       with:
         python-version: ${{ matrix.python }}
+        cache: pip
     - name: Install dependencies
       run: |
         pip install --upgrade mypy 'attrs==19.2.0' -r https://raw.githubusercontent.com/aws-cloudformation/aws-cloudformation-rpdk/master/requirements.txt
     - name: Install both plugin and support lib
       run: |
         pip install . src/
+    - uses: actions/cache@v3
+      with:
+        path: ~/.cache/pre-commit/
+        key: ${{ matrix.os }}-${{ env.pythonLocation }}${{ hashFiles('.pre-commit-config.yaml') }}
     - name: pre-commit checks
       run: |
         pre-commit run --all-files

.pre-commit-config.yaml

@@ -51,8 +51,6 @@ repos:
   hooks:
   - id: bandit
     files: ^(src|python)/
-    additional_dependencies:
-      - "importlib-metadata<5"  # https://github.com/PyCQA/bandit/issues/956
 - repo: https://github.com/pre-commit/mirrors-mypy
   rev: v0.812
   hooks:

python/rpdk/python/codegen.py

@@ -248,6 +248,17 @@ def _build(self, base_path):
             self._pip_build(base_path)
         LOG.debug("Dependencies build finished")
 
+    @staticmethod
+    def _update_pip_command():
+        return [
+            "python",
+            "-m",
+            "pip",
+            "install",
+            "--upgrade",
+            "pip",
+        ]
+
     @staticmethod
     def _make_pip_command(base_path):
         return [
@@ -270,7 +281,13 @@ def _get_plugin_information() -> Dict:
     @classmethod
     def _docker_build(cls, external_path):
         internal_path = PurePosixPath("/project")
-        command = " ".join(cls._make_pip_command(internal_path))
+        command = (
+            '/bin/bash -c "'
+            + " ".join(cls._update_pip_command())
+            + " && "
+            + " ".join(cls._make_pip_command(internal_path))
+            + '"'
+        )
         LOG.debug("command is '%s'", command)
 
         volumes = {str(external_path): {"bind": str(internal_path), "mode": "rw"}}
@@ -280,16 +297,37 @@ def _docker_build(cls, external_path):
             "image '%s' needs to be pulled first.",
             image,
         )
+
+        # Docker will mount the path specified in the volumes variable in the container
+        # and pip will place all the dependent packages inside the volumes/build path.
+        # codegen will need access to this directory during package()
+        try:
+            # Use root:root for euid:group when on Windows
+            # https://docs.docker.com/desktop/windows/permission-requirements/#containers-running-as-root-within-the-linux-vm
+            if os.name == "nt":
+                localuser = "root:root"
+            # Try to get current effective user ID and Group ID.
+            # Only valid on UNIX-like systems
+            else:
+                localuser = f"{os.geteuid()}:{os.getgid()}"
+        # Catch exception if geteuid failed on non-Windows system
+        # and default to root:root
+        except AttributeError:
+            localuser = "root:root"
+            LOG.warning(
+                "User ID / Group ID not found.  Using root:root for docker build"
+            )
+
         docker_client = docker.from_env()
         try:
             logs = docker_client.containers.run(
                 image=image,
                 command=command,
-                auto_remove=True,
+                remove=True,
                 volumes=volumes,
                 stream=True,
                 entrypoint="",
-                user=f"{os.geteuid()}:{os.getgid()}",
+                user=localuser,
             )
         except RequestsConnectionError as e:
             # it seems quite hard to reliably extract the cause from

tests/plugin/codegen_test.py

@@ -422,6 +422,84 @@ def test__build_docker(plugin):
     mock_docker.assert_called_once_with(sentinel.base_path)
 
 
+# Test _build_docker on Linux/Unix-like systems
+def test__build_docker_posix(plugin):
+    plugin._use_docker = True
+
+    patch_pip = patch.object(plugin, "_pip_build", autospec=True)
+    patch_from_env = patch("rpdk.python.codegen.docker.from_env", autospec=True)
+    patch_os_name = patch("rpdk.python.codegen.os.name", "posix")
+
+    with patch_pip as mock_pip, patch_from_env as mock_from_env:
+        mock_run = mock_from_env.return_value.containers.run
+        with patch_os_name:
+            plugin._build(sentinel.base_path)
+
+    mock_pip.assert_not_called()
+    mock_run.assert_called_once_with(
+        image=ANY,
+        command=ANY,
+        remove=True,
+        volumes={str(sentinel.base_path): {"bind": "/project", "mode": "rw"}},
+        stream=True,
+        entrypoint="",
+        user=ANY,
+    )
+
+
+# Test _build_docker on Windows
+def test__build_docker_windows(plugin):
+    plugin._use_docker = True
+
+    patch_pip = patch.object(plugin, "_pip_build", autospec=True)
+    patch_from_env = patch("rpdk.python.codegen.docker.from_env", autospec=True)
+    patch_os_name = patch("rpdk.python.codegen.os.name", "nt")
+
+    with patch_pip as mock_pip, patch_from_env as mock_from_env:
+        mock_run = mock_from_env.return_value.containers.run
+        with patch_os_name:
+            plugin._build(sentinel.base_path)
+
+    mock_pip.assert_not_called()
+    mock_run.assert_called_once_with(
+        image=ANY,
+        command=ANY,
+        remove=True,
+        volumes={str(sentinel.base_path): {"bind": "/project", "mode": "rw"}},
+        stream=True,
+        entrypoint="",
+        user="root:root",
+    )
+
+
+# Test _build_docker if geteuid fails
+def test__build_docker_no_euid(plugin):
+    plugin._use_docker = True
+
+    patch_pip = patch.object(plugin, "_pip_build", autospec=True)
+    patch_from_env = patch("rpdk.python.codegen.docker.from_env", autospec=True)
+    # os.geteuid does not exist on Windows so we can not autospec os
+    patch_os = patch("rpdk.python.codegen.os")
+    patch_os_name = patch("rpdk.python.codegen.os.name", "posix")
+
+    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
+        mock_run = mock_from_env.return_value.containers.run
+        mock_patch_os.geteuid.side_effect = AttributeError()
+        with patch_os_name:
+            plugin._build(sentinel.base_path)
+
+    mock_pip.assert_not_called()
+    mock_run.assert_called_once_with(
+        image=ANY,
+        command=ANY,
+        remove=True,
+        volumes={str(sentinel.base_path): {"bind": "/project", "mode": "rw"}},
+        stream=True,
+        entrypoint="",
+        user="root:root",
+    )
+
+
 def test__docker_build_good_path(plugin, tmp_path):
     patch_from_env = patch("rpdk.python.codegen.docker.from_env", autospec=True)
 
@@ -434,7 +512,7 @@ def test__docker_build_good_path(plugin, tmp_path):
     mock_run.assert_called_once_with(
         image=ANY,
         command=ANY,
-        auto_remove=True,
+        remove=True,
         volumes={str(tmp_path): {"bind": "/project", "mode": "rw"}},
         stream=True,
         entrypoint="",
@@ -476,7 +554,7 @@ def test__docker_build_bad_path(plugin, tmp_path, exception):
     mock_run.assert_called_once_with(
         image=ANY,
         command=ANY,
-        auto_remove=True,
+        remove=True,
         volumes={str(tmp_path): {"bind": "/project", "mode": "rw"}},
         stream=True,
         entrypoint="",