Merge from develop to master (#270)

* fix: improve support for pep517 builds (#265)

* A pep517 build can declare build dependencies. Pip will then
know to install these dependencies before trying to build a wheel
file.

* When creating a build environment, it's only guaranteed to last
for the duration of the build process. It's not accessible once
a pip command finishes running.

* When we try to retrieve the version of a package we run a "modified"
form of "python setup.py egg_info".

* The problem with this is that we're not using the build environment
that has all the build dependencies installed (it's already gone),
so if setup.py imports a module (e.g. cython) because it expects
it to be there because it declared it as a build dependency
the egg_info command will fail.

* We don't check the RC or have a fallback case if we can't generate
egg info.

* We fail with an indecipherable IndexError.

We now have a fallback where if we can't import/run the setup.py file,
we assume the PKG-INFO file should be in the top level directory of the
sdist so we check if it's there, and if so we use that file.

* Fixed Unit Test Requiring "test" Binary (#266)

* fix(go version parts): remove alphabets from the version for validation (#259)

* fix(go version parts): remove alphabets from the version for validation

- Go versions like 1.12rc1 or 1.16beta1 are supported.
- Test added.

* fix: use regex for go versions

* chore: aws lambda builders version set to 1.7.0 (#269)

Co-authored-by: Cosh_ <[email protected]>

aws_lambda_builders/__init__.py

@@ -1,5 +1,5 @@
 """
 AWS Lambda Builder Library
 """
-__version__ = "1.6.0"
+__version__ = "1.7.0"
 RPC_PROTOCOL_VERSION = "0.3"

aws_lambda_builders/workflows/go_modules/validator.py

@@ -3,6 +3,7 @@
 """
 
 import logging
+import re
 import os
 import subprocess
 
@@ -12,9 +13,9 @@
 
 
 class GoRuntimeValidator(object):
-
     LANGUAGE = "go"
     SUPPORTED_RUNTIMES = {"go1.x"}
+    GO_VERSION_REGEX = re.compile("go(\\d)\\.(x|\\d+)")
 
     def __init__(self, runtime):
         self.runtime = runtime
@@ -28,6 +29,15 @@ def has_runtime(self):
         """
         return self.runtime in self.SUPPORTED_RUNTIMES
 
+    @staticmethod
+    def get_go_versions(version_string):
+        parts = GoRuntimeValidator.GO_VERSION_REGEX.findall(version_string)
+        try:
+            # NOTE(sriram-mv): The version parts need to be a list with a major and minor version.
+            return int(parts[0][0]), int(parts[0][1])
+        except IndexError:
+            return 0, 0
+
     def validate(self, runtime_path):
         """
         Checks if the language supplied matches the required lambda runtime
@@ -42,16 +52,13 @@ def validate(self, runtime_path):
         min_expected_minor_version = 11 if expected_major_version == 1 else 0
 
         p = subprocess.Popen([runtime_path, "version"], cwd=os.getcwd(), stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-        out, _ = p.communicate()
+        version_string, _ = p.communicate()
 
         if p.returncode == 0:
-            out_parts = out.decode().split()
-            if len(out_parts) >= 3:
-                version_parts = [int(x.replace("rc", "")) for x in out_parts[2].replace(self.LANGUAGE, "").split(".")]
-                if len(version_parts) >= 2:
-                    if version_parts[0] == expected_major_version and version_parts[1] >= min_expected_minor_version:
-                        self._valid_runtime_path = runtime_path
-                        return self._valid_runtime_path
+            major_version, minor_version = GoRuntimeValidator.get_go_versions(version_string.decode())
+            if major_version == expected_major_version and minor_version >= min_expected_minor_version:
+                self._valid_runtime_path = runtime_path
+                return self._valid_runtime_path
 
         # otherwise, raise mismatch exception
         raise MisMatchRuntimeError(language=self.LANGUAGE, required_runtime=self.runtime, runtime_path=runtime_path)

aws_lambda_builders/workflows/python_pip/packager.py

@@ -64,6 +64,14 @@ class PackageDownloadError(PackagerError):
     pass
 
 
+class UnsupportedPackageError(Exception):
+    """Unable to parse package metadata."""
+
+    def __init__(self, package_name):
+        # type: (str) -> None
+        super(UnsupportedPackageError, self).__init__("Unable to retrieve name/version for package: %s" % package_name)
+
+
 class UnsupportedPythonVersion(PackagerError):
     """Generic networking error during a package download."""
 
@@ -538,7 +546,7 @@ def _parse_pkg_info_file(self, filepath):
         parser.feed(data)
         return parser.close()
 
-    def _generate_egg_info(self, package_dir):
+    def _get_pkg_info_filepath(self, package_dir):
         setup_py = self._osutils.joinpath(package_dir, "setup.py")
         script = self._SETUPTOOLS_SHIM % setup_py
 
@@ -548,9 +556,20 @@ def _generate_egg_info(self, package_dir):
         p = subprocess.Popen(
             cmd, cwd=package_dir, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=self._osutils.original_environ()
         )
-        p.communicate()
+        _, stderr = p.communicate()
         info_contents = self._osutils.get_directory_contents(egg_info_dir)
-        pkg_info_path = self._osutils.joinpath(egg_info_dir, info_contents[0], "PKG-INFO")
+        if p.returncode != 0:
+            LOG.debug("Non zero rc (%s) from the setup.py egg_info command: %s", p.returncode, stderr)
+        if info_contents:
+            pkg_info_path = self._osutils.joinpath(egg_info_dir, info_contents[0], "PKG-INFO")
+        else:
+            # This might be a pep 517 package in which case this PKG-INFO file
+            # should be available right in the top level directory of the sdist
+            # in the case where the egg_info command fails.
+            LOG.debug("Using fallback location for PKG-INFO file in package directory: %s", package_dir)
+            pkg_info_path = self._osutils.joinpath(package_dir, "PKG-INFO")
+        if not self._osutils.file_exists(pkg_info_path):
+            raise UnsupportedPackageError(self._osutils.basename(package_dir))
         return pkg_info_path
 
     def _unpack_sdist_into_dir(self, sdist_path, unpack_dir):
@@ -567,7 +586,7 @@ def _unpack_sdist_into_dir(self, sdist_path, unpack_dir):
     def get_package_name_and_version(self, sdist_path):
         with self._osutils.tempdir() as tempdir:
             package_dir = self._unpack_sdist_into_dir(sdist_path, tempdir)
-            pkg_info_filepath = self._generate_egg_info(package_dir)
+            pkg_info_filepath = self._get_pkg_info_filepath(package_dir)
             metadata = self._parse_pkg_info_file(pkg_info_filepath)
             name = metadata["Name"]
             version = metadata["Version"]

aws_lambda_builders/workflows/python_pip/utils.py

@@ -102,3 +102,7 @@ def mtime(self, path):
     @property
     def pipe(self):
         return subprocess.PIPE
+
+    def basename(self, path):
+        # type: (str) -> str
+        return os.path.basename(path)

tests/functional/workflows/python_pip/test_packager.py

@@ -8,7 +8,7 @@
 import pytest
 import mock
 
-from aws_lambda_builders.workflows.python_pip.packager import PipRunner
+from aws_lambda_builders.workflows.python_pip.packager import PipRunner, UnsupportedPackageError
 from aws_lambda_builders.workflows.python_pip.packager import DependencyBuilder
 from aws_lambda_builders.workflows.python_pip.packager import Package
 from aws_lambda_builders.workflows.python_pip.packager import MissingDependencyError
@@ -858,18 +858,24 @@ class TestSdistMetadataFetcher(object):
     _SETUP_PY = "%s\n" "setup(\n" '    name="%s",\n' '    version="%s"\n' ")\n"
     _VALID_TAR_FORMATS = ["tar.gz", "tar.bz2"]
 
-    def _write_fake_sdist(self, setup_py, directory, ext):
+    def _write_fake_sdist(self, setup_py, directory, ext, pkg_info_contents=None):
         filename = "sdist.%s" % ext
         path = "%s/%s" % (directory, filename)
         if ext == "zip":
             with zipfile.ZipFile(path, "w", compression=zipfile.ZIP_DEFLATED) as z:
                 z.writestr("sdist/setup.py", setup_py)
+                if pkg_info_contents is not None:
+                    z.writestr("sdist/PKG-INFO", pkg_info_contents)
         elif ext in self._VALID_TAR_FORMATS:
             compression_format = ext.split(".")[1]
             with tarfile.open(path, "w:%s" % compression_format) as tar:
                 tarinfo = tarfile.TarInfo("sdist/setup.py")
                 tarinfo.size = len(setup_py)
                 tar.addfile(tarinfo, io.BytesIO(setup_py.encode()))
+                if pkg_info_contents is not None:
+                    tarinfo = tarfile.TarInfo("sdist/PKG-INFO")
+                    tarinfo.size = len(pkg_info_contents)
+                    tar.addfile(tarinfo, io.BytesIO(pkg_info_contents.encode()))
         else:
             open(path, "a").close()
         filepath = os.path.join(directory, filename)
@@ -967,6 +973,29 @@ def test_bad_format(self, osutils, sdist_reader):
             with pytest.raises(InvalidSourceDistributionNameError):
                 name, version = sdist_reader.get_package_name_and_version(filepath)
 
+    def test_cant_get_egg_info_filename(self, osutils, sdist_reader):
+        # In this scenario the setup.py file will fail with an import
+        # error so we should verify we try a fallback to look for
+        # PKG-INFO.
+        bad_setup_py = self._SETUP_PY % (
+            "import some_build_dependency",
+            "foo",
+            "1.0",
+        )
+        pkg_info_file = "Name: foo\n" "Version: 1.0\n"
+        with osutils.tempdir() as tempdir:
+            filepath = self._write_fake_sdist(bad_setup_py, tempdir, "zip", pkg_info_file)
+            name, version = sdist_reader.get_package_name_and_version(filepath)
+        assert name == "foo"
+        assert version == "1.0"
+
+    def test_pkg_info_fallback_fails_raises_error(self, osutils, sdist_reader):
+        setup_py = self._SETUP_PY % ("import build_time_dependency", "foo", "1.0")
+        with osutils.tempdir() as tempdir:
+            filepath = self._write_fake_sdist(setup_py, tempdir, "tar.gz")
+            with pytest.raises(UnsupportedPackageError):
+                sdist_reader.get_package_name_and_version(filepath)
+
 
 class TestPackage(object):
     def test_same_pkg_sdist_and_wheel_collide(self, osutils, sdist_builder):

tests/unit/test_workflow.py

@@ -176,6 +176,21 @@ def setUp(self):
             options={"c": "d"},
         )
 
+    def mock_binaries(self):
+        self.validator_mock = Mock()
+        self.validator_mock.validate = Mock()
+        self.validator_mock.validate.return_value = "/usr/bin/binary"
+        self.resolver_mock = Mock()
+        self.resolver_mock.exec_paths = ["/usr/bin/binary"]
+        self.binaries_mock = Mock()
+        self.binaries_mock.return_value = []
+
+        self.work.get_validators = lambda: self.validator_mock
+        self.work.get_resolvers = lambda: self.resolver_mock
+        self.work.binaries = {
+            "binary": BinaryPath(resolver=self.resolver_mock, validator=self.validator_mock, binary="binary")
+        }
+
     def test_get_binaries(self):
         self.assertIsNotNone(self.work.binaries)
         for binary, binary_path in self.work.binaries.items():
@@ -187,63 +202,39 @@ def test_get_validator(self):
             self.assertTrue(isinstance(validator, RuntimeValidator))
 
     def test_must_execute_actions_in_sequence(self):
+        self.mock_binaries()
         action_mock = Mock()
-        validator_mock = Mock()
-        validator_mock.validate = Mock()
-        validator_mock.validate.return_value = "/usr/bin/binary"
-        resolver_mock = Mock()
-        resolver_mock.exec_paths = ["/usr/bin/binary"]
-        binaries_mock = Mock()
-        binaries_mock.return_value = []
-
-        self.work.get_validators = lambda: validator_mock
-        self.work.get_resolvers = lambda: resolver_mock
         self.work.actions = [action_mock.action1, action_mock.action2, action_mock.action3]
-        self.work.binaries = {"binary": BinaryPath(resolver=resolver_mock, validator=validator_mock, binary="binary")}
         self.work.run()
 
         self.assertEqual(
             action_mock.method_calls, [call.action1.execute(), call.action2.execute(), call.action3.execute()]
         )
-        self.assertTrue(validator_mock.validate.call_count, 1)
+        self.assertTrue(self.validator_mock.validate.call_count, 1)
 
     def test_must_fail_workflow_binary_resolution_failure(self):
+        self.mock_binaries()
         action_mock = Mock()
-        validator_mock = Mock()
-        validator_mock.validate = Mock()
-        validator_mock.validate.return_value = None
-        resolver_mock = Mock()
-        resolver_mock.exec_paths = MagicMock(side_effect=ValueError("Binary could not be resolved"))
-        binaries_mock = Mock()
-        binaries_mock.return_value = []
-
-        self.work.get_validators = lambda: validator_mock
-        self.work.get_resolvers = lambda: resolver_mock
+        self.resolver_mock.exec_paths = MagicMock(side_effect=ValueError("Binary could not be resolved"))
+
         self.work.actions = [action_mock.action1, action_mock.action2, action_mock.action3]
-        self.work.binaries = {"binary": BinaryPath(resolver=resolver_mock, validator=validator_mock, binary="binary")}
         with self.assertRaises(WorkflowFailedError) as ex:
             self.work.run()
 
     def test_must_fail_workflow_binary_validation_failure(self):
-        action_mock = Mock()
-        validator_mock = Mock()
-        validator_mock.validate = Mock()
-        validator_mock.validate = MagicMock(
+        self.mock_binaries()
+        self.validator_mock.validate = MagicMock(
             side_effect=MisMatchRuntimeError(language="test", required_runtime="test1", runtime_path="/usr/bin/binary")
         )
-        resolver_mock = Mock()
-        resolver_mock.exec_paths = ["/usr/bin/binary"]
-        binaries_mock = Mock()
-        binaries_mock.return_value = []
 
-        self.work.get_validators = lambda: validator_mock
-        self.work.get_resolvers = lambda: resolver_mock
+        action_mock = Mock()
         self.work.actions = [action_mock.action1, action_mock.action2, action_mock.action3]
-        self.work.binaries = {"binary": BinaryPath(resolver=resolver_mock, validator=validator_mock, binary="binary")}
         with self.assertRaises(WorkflowFailedError) as ex:
             self.work.run()
 
     def test_must_raise_with_no_actions(self):
+        self.mock_binaries()
+
         self.work.actions = []
 
         with self.assertRaises(WorkflowFailedError) as ctx:
@@ -252,6 +243,7 @@ def test_must_raise_with_no_actions(self):
         self.assertIn("Workflow does not have any actions registered", str(ctx.exception))
 
     def test_must_raise_if_action_failed(self):
+        self.mock_binaries()
         action_mock = Mock()
         self.work.actions = [action_mock.action1, action_mock.action2, action_mock.action3]
 
@@ -264,6 +256,7 @@ def test_must_raise_if_action_failed(self):
         self.assertIn("testfailure", str(ctx.exception))
 
     def test_must_raise_if_action_crashed(self):
+        self.mock_binaries()
         action_mock = Mock()
         self.work.actions = [action_mock.action1, action_mock.action2, action_mock.action3]
 
@@ -290,6 +283,8 @@ def test_supply_executable_path(self):
             options={"c": "d"},
         )
         self.work.actions = [action_mock.action1, action_mock.action2, action_mock.action3]
+        self.mock_binaries()
+
         self.work.run()
 
 

tests/unit/workflows/go_modules/test_validator.py

@@ -14,7 +14,7 @@ def __init__(self, returncode, out=b"", err=b""):
         self.err = err
 
     def communicate(self):
-        return (self.out, self.err)
+        return self.out, self.err
 
 
 class TestGoRuntimeValidator(TestCase):
@@ -30,36 +30,50 @@ def test_runtime_validate_unsupported_language_fail_open(self):
         validator = GoRuntimeValidator(runtime="go2.x")
         validator.validate(runtime_path="/usr/bin/go2")
 
-    @parameterized.expand([(b"go version go1.11.2 test",), (b"go version go1.11rc.2 test",)])
+    @parameterized.expand(
+        [
+            ("go1.11.2", (1, 11)),
+            ("go1.11rc.2", (1, 11)),
+            ("go1.16beta1", (1, 16)),
+            ("go%$", (0, 0)),
+            ("unknown", (0, 0)),
+        ]
+    )
+    def test_get_go_versions(self, version_string, version_parts):
+        self.assertEqual(self.validator.get_go_versions(version_string), version_parts)
+
+    @parameterized.expand(
+        [(b"go version go1.11.2 test",), (b"go version go1.11rc.2 test",), (b"go version go1.16beta1 test",)]
+    )
     def test_runtime_validate_supported_version_runtime(self, go_version_output):
         with mock.patch("subprocess.Popen") as mock_subprocess:
             mock_subprocess.return_value = MockSubProcess(0, out=go_version_output)
             self.validator.validate(runtime_path="/usr/bin/go")
-            self.assertTrue(mock_subprocess.call_count, 1)
+            self.assertEqual(mock_subprocess.call_count, 1)
 
     def test_runtime_validate_supported_higher_than_min_version_runtime(self):
         with mock.patch("subprocess.Popen") as mock_subprocess:
             mock_subprocess.return_value = MockSubProcess(0, out=b"go version go1.12 test")
             self.validator.validate(runtime_path="/usr/bin/go")
-            self.assertTrue(mock_subprocess.call_count, 1)
+            self.assertEqual(mock_subprocess.call_count, 1)
 
     def test_runtime_validate_mismatch_nonzero_exit(self):
         with mock.patch("subprocess.Popen") as mock_subprocess:
             mock_subprocess.return_value = MockSubProcess(1)
             with self.assertRaises(MisMatchRuntimeError):
                 self.validator.validate(runtime_path="/usr/bin/go")
-                self.assertTrue(mock_subprocess.call_count, 1)
+                self.assertEqual(mock_subprocess.call_count, 1)
 
     def test_runtime_validate_mismatch_invalid_version(self):
         with mock.patch("subprocess.Popen") as mock_subprocess:
             mock_subprocess.return_value = MockSubProcess(0, out=b"go version")
             with self.assertRaises(MisMatchRuntimeError):
                 self.validator.validate(runtime_path="/usr/bin/go")
-                self.assertTrue(mock_subprocess.call_count, 1)
+                self.assertEqual(mock_subprocess.call_count, 1)
 
     def test_runtime_validate_mismatch_minor_version(self):
         with mock.patch("subprocess.Popen") as mock_subprocess:
             mock_subprocess.return_value = MockSubProcess(0, out=b"go version go1.10.2 test")
             with self.assertRaises(MisMatchRuntimeError):
                 self.validator.validate(runtime_path="/usr/bin/go")
-                self.assertTrue(mock_subprocess.call_count, 1)
+                self.assertEqual(mock_subprocess.call_count, 1)