Repository: aws-cloudformation/cloudformation-cli-python-plugin

Issue:
clean up filenotfound error in logs during `cfn submit`
Exception for path not being found during `cfn submit` is needlessly cluttering logs

```
[2022-10-21T01:48:21Z] DEBUG    - Package started
[2022-10-21T01:48:21Z] DEBUG    - '/home/mmaeng/source/test-mod/build' not found, skipping removal
Traceback (most recent call last):
  File "/home/mmaeng/source/cloudformation-cli/.venv/lib/python3.8/site-packages/rpdk/typescript/codegen.py", line 226, in _remove_build_artifacts
    shutil.rmtree(deps_path)
  File "/usr/lib/python3.8/shutil.py", line 709, in rmtree
    onerror(os.lstat, path, sys.exc_info())
  File "/usr/lib/python3.8/shutil.py", line 707, in rmtree
    orig_st = os.lstat(path)
FileNotFoundError: [Errno 2] No such file or directory: '/home/mmaeng/source/test-mod/build'
```

Discussion:
mmaeng: We are catching it but it is still showing up in the logs.  So switched to a existence check first.

```
[2022-10-21T01:48:21Z] DEBUG    - Package started
[2022-10-21T01:48:21Z] DEBUG    - '/home/mmaeng/source/test-mod/build' not found, skipping removal
Traceback (most recent call last):
  File "/home/mmaeng/source/cloudformation-cli/.venv/lib/python3.8/site-packages/rpdk/typescript/codegen.py", line 226, in _remove_build_artifacts
    shutil.rmtree(deps_path)
  File "/usr/lib/python3.8/shutil.py", line 709, in rmtree
    onerror(os.lstat, path, sys.exc_info())
  File "/usr/lib/python3.8/shutil.py", line 707, in rmtree
    orig_st = os.lstat(path)
FileNotFoundError: [Errno 2] No such file or directory: '/home/mmaeng/source/test-mod/build'
```
kddejong: In other parts of the code we  use `shutil.rmtree(docs_path, ignore_errors=True)` would that also fix it?

Patch:
diff --git a/.gitattributes b/.gitattributes
new file mode 100644
index 0000000..4282322
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1,2 @@
+# Set the default behavior, in case people don't have core.autocrlf set.
+* text eol=lf
diff --git a/.github/workflows/pr-ci.yml b/.github/workflows/pr-ci.yml
index 23e4e5b..67e0e33 100644
--- a/.github/workflows/pr-ci.yml
+++ b/.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
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 2871aeb..6fdcd0d 100644
--- a/.pre-commit-config.yaml
+++ b/.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:
diff --git a/python/rpdk/python/codegen.py b/python/rpdk/python/codegen.py
index 0ceaab8..dd4b61c 100644
--- a/python/rpdk/python/codegen.py
+++ b/python/rpdk/python/codegen.py
@@ -235,10 +235,8 @@ class Python36LanguagePlugin(LanguagePlugin):
 
     @staticmethod
     def _remove_build_artifacts(deps_path):
-        try:
-            shutil.rmtree(deps_path)
-        except FileNotFoundError:
-            LOG.debug("'%s' not found, skipping removal", deps_path, exc_info=True)
+        LOG.debug("Removing '%s' folder.", deps_path)
+        shutil.rmtree(deps_path, ignore_errors=True)
 
     def _build(self, base_path):
         LOG.debug("Dependencies build started from '%s'", base_path)
@@ -248,6 +246,17 @@ class Python36LanguagePlugin(LanguagePlugin):
             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 +279,13 @@ class Python36LanguagePlugin(LanguagePlugin):
     @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,6 +295,27 @@ class Python36LanguagePlugin(LanguagePlugin):
             "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(
@@ -289,7 +325,7 @@ class Python36LanguagePlugin(LanguagePlugin):
                 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


Test patch:
diff --git a/tests/plugin/codegen_test.py b/tests/plugin/codegen_test.py
index f4857ea..0eda8e5 100644
--- a/tests/plugin/codegen_test.py
+++ b/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,
+        auto_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,
+        auto_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,
+        auto_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)