From 0f07ee92bdcc172d34973461b1ecb925cc41de08 Mon Sep 17 00:00:00 2001 From: Sriram Madapusi Vasudevan <3770774+sriram-mv@users.noreply.github.com> Date: Tue, 31 Aug 2021 10:18:10 -0700 Subject: [PATCH] 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_ --- aws_lambda_builders/__init__.py | 2 +- .../workflows/go_modules/validator.py | 25 +++++--- .../workflows/python_pip/packager.py | 27 ++++++-- .../workflows/python_pip/utils.py | 4 ++ .../workflows/python_pip/test_packager.py | 33 +++++++++- tests/unit/test_workflow.py | 63 +++++++++---------- .../workflows/go_modules/test_validator.py | 28 ++++++--- 7 files changed, 125 insertions(+), 57 deletions(-) diff --git a/aws_lambda_builders/__init__.py b/aws_lambda_builders/__init__.py index 952e5f9a3..cb6f432a3 100644 --- a/aws_lambda_builders/__init__.py +++ b/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" diff --git a/aws_lambda_builders/workflows/go_modules/validator.py b/aws_lambda_builders/workflows/go_modules/validator.py index fbb2e6877..6ac325b68 100644 --- a/aws_lambda_builders/workflows/go_modules/validator.py +++ b/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) diff --git a/aws_lambda_builders/workflows/python_pip/packager.py b/aws_lambda_builders/workflows/python_pip/packager.py index 68c2fc892..0e884c8da 100644 --- a/aws_lambda_builders/workflows/python_pip/packager.py +++ b/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"] diff --git a/aws_lambda_builders/workflows/python_pip/utils.py b/aws_lambda_builders/workflows/python_pip/utils.py index eab58bda4..19ee7656d 100644 --- a/aws_lambda_builders/workflows/python_pip/utils.py +++ b/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) diff --git a/tests/functional/workflows/python_pip/test_packager.py b/tests/functional/workflows/python_pip/test_packager.py index 7c00aa66c..2b627fac0 100644 --- a/tests/functional/workflows/python_pip/test_packager.py +++ b/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): diff --git a/tests/unit/test_workflow.py b/tests/unit/test_workflow.py index 390297e49..149a82fe7 100644 --- a/tests/unit/test_workflow.py +++ b/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() diff --git a/tests/unit/workflows/go_modules/test_validator.py b/tests/unit/workflows/go_modules/test_validator.py index 2f0311925..221fea415 100644 --- a/tests/unit/workflows/go_modules/test_validator.py +++ b/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)