From cc9be0ae3a8293bb5277910158aa71721bf80778 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amadeusz=20=C5=BBo=C5=82nowski?= Date: Tue, 20 May 2025 17:59:48 +0100 Subject: [PATCH 1/2] Reformat package.py with ruff before making changes --- package.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/package.py b/package.py index 6e19846c..6bb81db8 100644 --- a/package.py +++ b/package.py @@ -1088,7 +1088,7 @@ def install_pip_requirements(query, requirements_file, tmp_dir): ok = True elif docker_file or docker_build_root: raise ValueError( - "docker_image must be specified " "for a custom image future references" + "docker_image must be specified for a custom image future references" ) working_dir = os.getcwd() @@ -1108,7 +1108,7 @@ def install_pip_requirements(query, requirements_file, tmp_dir): elif OSX: # Workaround for OSX when XCode command line tools' # python becomes the main system python interpreter - os_path = "{}:/Library/Developer/CommandLineTools" "/usr/bin".format( + os_path = "{}:/Library/Developer/CommandLineTools/usr/bin".format( os.environ["PATH"] ) subproc_env = os.environ.copy() @@ -1390,7 +1390,7 @@ def install_npm_requirements(query, requirements_file, tmp_dir): ok = True elif docker_file or docker_build_root: raise ValueError( - "docker_image must be specified " "for a custom image future references" + "docker_image must be specified for a custom image future references" ) log.info("Installing npm requirements: %s", requirements_file) @@ -1649,7 +1649,7 @@ def prepare_command(args): timestamp = timestamp_now_ns() was_missing = True else: - timestamp = "" + timestamp = "" # Replace variables in the build command with calculated values. build_data = { From 5b70e1dec55964a81996619e12ef90ea7a2bb245 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amadeusz=20=C5=BBo=C5=82nowski?= Date: Tue, 20 May 2025 16:36:17 +0100 Subject: [PATCH 2/2] feat: Respect the package-lock.json for a NodeJS Lambda function Respect the `package-lock.json` so NodeJS Lambda for reproducible builds which are critical in production environments. Similarly like for the Poetry, copy a lock file, if such is present, to a temporary build directory. npm will use a `package-lock.json` file when available in a working directory. In the example `package.json`, require lower `requests` version to demonstrate `package-lock.json` usage. `package.json` specifies `~0.2.0` and the latest available matching version is `0.2.2`, but `package-lock.json` freezes version `0.2.1` and that version gets installed with this change, while previously the `0.2.2` would be installed. --- examples/build-package/README.md | 2 + examples/build-package/main.tf | 26 ++++++ examples/fixtures/nodejs14.x-app2/index.js | 16 ++++ .../nodejs14.x-app2/package-lock.json | 83 +++++++++++++++++++ .../fixtures/nodejs14.x-app2/package.json | 8 ++ package.py | 70 +++++++++++++++- 6 files changed, 201 insertions(+), 4 deletions(-) create mode 100644 examples/fixtures/nodejs14.x-app2/index.js create mode 100644 examples/fixtures/nodejs14.x-app2/package-lock.json create mode 100644 examples/fixtures/nodejs14.x-app2/package.json diff --git a/examples/build-package/README.md b/examples/build-package/README.md index d26739dd..6585644c 100644 --- a/examples/build-package/README.md +++ b/examples/build-package/README.md @@ -45,6 +45,7 @@ Note that this example may create resources which cost money. Run `terraform des | [package\_dir\_poetry](#module\_package\_dir\_poetry) | ../../ | n/a | | [package\_dir\_poetry\_no\_docker](#module\_package\_dir\_poetry\_no\_docker) | ../../ | n/a | | [package\_dir\_with\_npm\_install](#module\_package\_dir\_with\_npm\_install) | ../../ | n/a | +| [package\_dir\_with\_npm\_install\_lock\_file](#module\_package\_dir\_with\_npm\_install\_lock\_file) | ../../ | n/a | | [package\_dir\_without\_npm\_install](#module\_package\_dir\_without\_npm\_install) | ../../ | n/a | | [package\_dir\_without\_pip\_install](#module\_package\_dir\_without\_pip\_install) | ../../ | n/a | | [package\_file](#module\_package\_file) | ../../ | n/a | @@ -53,6 +54,7 @@ Note that this example may create resources which cost money. Run `terraform des | [package\_src\_poetry2](#module\_package\_src\_poetry2) | ../../ | n/a | | [package\_with\_commands\_and\_patterns](#module\_package\_with\_commands\_and\_patterns) | ../../ | n/a | | [package\_with\_docker](#module\_package\_with\_docker) | ../../ | n/a | +| [package\_with\_npm\_lock\_in\_docker](#module\_package\_with\_npm\_lock\_in\_docker) | ../../ | n/a | | [package\_with\_npm\_requirements\_in\_docker](#module\_package\_with\_npm\_requirements\_in\_docker) | ../../ | n/a | | [package\_with\_patterns](#module\_package\_with\_patterns) | ../../ | n/a | | [package\_with\_pip\_requirements\_in\_docker](#module\_package\_with\_pip\_requirements\_in\_docker) | ../../ | n/a | diff --git a/examples/build-package/main.tf b/examples/build-package/main.tf index 2afce855..48f7bc8c 100644 --- a/examples/build-package/main.tf +++ b/examples/build-package/main.tf @@ -365,6 +365,18 @@ module "package_dir_with_npm_install" { source_path = "${path.module}/../fixtures/nodejs14.x-app1" } +# Create zip-archive of a single directory where "npm install" will also be +# executed (default for nodejs runtime). This example has package-lock.json which +# is respected when installing dependencies. +module "package_dir_with_npm_install_lock_file" { + source = "../../" + + create_function = false + + runtime = "nodejs14.x" + source_path = "${path.module}/../fixtures/nodejs14.x-app2" +} + # Create zip-archive of a single directory without running "npm install" (which is the default for nodejs runtime) module "package_dir_without_npm_install" { source = "../../" @@ -393,6 +405,20 @@ module "package_with_npm_requirements_in_docker" { hash_extra = "something-unique-to-not-conflict-with-module.package_dir_with_npm_install" } +# Create zip-archive of a single directory where "npm install" will also be +# executed using docker. This example has package-lock.json which is respected +# when installing dependencies. +module "package_with_npm_lock_in_docker" { + source = "../../" + + create_function = false + + runtime = "nodejs14.x" + source_path = "${path.module}/../fixtures/nodejs14.x-app2" + build_in_docker = true + hash_extra = "something-unique-to-not-conflict-with-module.package_dir_with_npm_install" +} + ################################ # Build package in Docker and # use it to deploy Lambda Layer diff --git a/examples/fixtures/nodejs14.x-app2/index.js b/examples/fixtures/nodejs14.x-app2/index.js new file mode 100644 index 00000000..97968e4a --- /dev/null +++ b/examples/fixtures/nodejs14.x-app2/index.js @@ -0,0 +1,16 @@ +'use strict'; + +module.exports.hello = async (event) => { + console.log(event); + return { + statusCode: 200, + body: JSON.stringify( + { + message: `Go Serverless.tf! Your Nodejs function executed successfully!`, + input: event, + }, + null, + 2 + ), + }; +}; diff --git a/examples/fixtures/nodejs14.x-app2/package-lock.json b/examples/fixtures/nodejs14.x-app2/package-lock.json new file mode 100644 index 00000000..adf88ca6 --- /dev/null +++ b/examples/fixtures/nodejs14.x-app2/package-lock.json @@ -0,0 +1,83 @@ +{ + "name": "nodejs14.x-app1", + "version": "1.0.0", + "lockfileVersion": 1, + "requires": true, + "dependencies": { + "axo": { + "version": "0.0.2", + "resolved": "https://registry.npmjs.org/axo/-/axo-0.0.2.tgz", + "integrity": "sha512-8CC4Mb+OhK97UEng0PgiqUDNZjzVcWDsV+G2vLYCQn1jEL7y6VqiRVlZlRu+aA/ckSznmNzW6X1I6nj2As/haQ==" + }, + "eventemitter3": { + "version": "2.0.3", + "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-2.0.3.tgz", + "integrity": "sha512-jLN68Dx5kyFHaePoXWPsCGW5qdyZQtLYHkxkg02/Mz6g0kYpDx4FyP6XfArhQdlOC4b8Mv+EMxPo/8La7Tzghg==" + }, + "extendible": { + "version": "0.1.1", + "resolved": "https://registry.npmjs.org/extendible/-/extendible-0.1.1.tgz", + "integrity": "sha512-AglckQA0TJV8/ZmhQcNmaaFcFFPXFIoZbfuoQOlGDK7Jh/roWotYzJ7ik1FBBCHBr8n7CgTR8lXXPAN8Rfb7rw==" + }, + "failure": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/failure/-/failure-1.1.1.tgz", + "integrity": "sha512-lzrrk0NUfjVeU3jLmfU01zP5bfg4XVFxHREYGvgJowaCqHLSQtqIGENH/CU+oSs6yfYObdSM7b9UY/3p2VJOSg==" + }, + "hang": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/hang/-/hang-1.0.0.tgz", + "integrity": "sha512-vtBz98Bt/Tbm03cZO5Ymc7ZL8ead/jIx9T5Wg/xuz+9BXPAJNJSdGQW63LoaesogUQKTpHyal339hxTaTf/APg==" + }, + "loads": { + "version": "0.0.4", + "resolved": "https://registry.npmjs.org/loads/-/loads-0.0.4.tgz", + "integrity": "sha512-XjPzzYIHkuMNqYyvh6AECQAHi682nyKO9TMdMYnaz7QbPDI/KIeSIjRhAlXIbRMPYAgtLUYgPlD3mtKZ4Q8SYA==", + "requires": { + "failure": "1.1.x", + "one-time": "0.0.x", + "xhr-response": "1.0.x", + "xhr-status": "1.0.x" + } + }, + "node-http-xhr": { + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/node-http-xhr/-/node-http-xhr-1.2.1.tgz", + "integrity": "sha512-eRKOQNY8V2BNp/P8A2A+eVwprVFI64ciunsBimQ4WBb1m841vn7ksDRGlmWBCyE/tLRoPwvH/sUig9krKMehwA==" + }, + "one-time": { + "version": "0.0.4", + "resolved": "https://registry.npmjs.org/one-time/-/one-time-0.0.4.tgz", + "integrity": "sha512-qAMrwuk2xLEutlASoiPiAMW3EN3K96Ka/ilSXYr6qR1zSVXw2j7+yDSqGTC4T9apfLYxM3tLLjKvgPdAUK7kYQ==" + }, + "requests": { + "version": "0.2.1", + "resolved": "https://registry.npmjs.org/requests/-/requests-0.2.1.tgz", + "integrity": "sha512-SlvQm7cl4z285qs5ZrthHjr4TI0Ngb0pzX4jLzSOr7rsA4AlFj+0qhkzF3zKsVBFuU+HseU+iUz4qBA6bV087Q==", + "requires": { + "axo": "0.0.x", + "eventemitter3": "~2.0.2", + "extendible": "0.1.x", + "hang": "1.0.x", + "loads": "0.0.x", + "node-http-xhr": "~1.2.1", + "xhr-send": "1.0.x" + } + }, + "xhr-response": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/xhr-response/-/xhr-response-1.0.1.tgz", + "integrity": "sha512-m2FlVRCl3VqDcpc8UaWZJpwuHpFR2vYeXv6ipXU2Uuu4vNKFYVEFI0emuJN370Fge+JCbiAnS+JJmSoWVmWrjQ==" + }, + "xhr-send": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/xhr-send/-/xhr-send-1.0.0.tgz", + "integrity": "sha512-789EG4qW6Z0nPvG74AV3WWQCnBG5HxJXNiBsnEivZ8OpbvVA0amH0+g+MNT99o5kt/XLdRezm5KS1wJzcGJztw==" + }, + "xhr-status": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/xhr-status/-/xhr-status-1.0.1.tgz", + "integrity": "sha512-VF0WSqtmkf56OmF26LCWsWvRb1a+WYGdHDoQnPPCVUQTM8CVUAOBcUDsm7nP7SQcgEEdrvF4DmhEADuXdGieyw==" + } + } +} diff --git a/examples/fixtures/nodejs14.x-app2/package.json b/examples/fixtures/nodejs14.x-app2/package.json new file mode 100644 index 00000000..b332ac7e --- /dev/null +++ b/examples/fixtures/nodejs14.x-app2/package.json @@ -0,0 +1,8 @@ +{ + "name": "nodejs14.x-app1", + "version": "1.0.0", + "main": "index.js", + "dependencies": { + "requests": "^0.2.0" + } +} diff --git a/package.py b/package.py index 6bb81db8..74af6e7c 100644 --- a/package.py +++ b/package.py @@ -733,6 +733,14 @@ def npm_requirements_step(path, prefix=None, required=False, tmp_dir=None): requirements = path if os.path.isdir(path): requirements = os.path.join(path, "package.json") + npm_lock_file = os.path.join(path, "package-lock.json") + else: + npm_lock_file = os.path.join(os.path.dirname(path), "package-lock.json") + + if os.path.isfile(npm_lock_file): + hash(npm_lock_file) + log.info("Added npm lock file: %s", npm_lock_file) + if not os.path.isfile(requirements): if required: raise RuntimeError("File not found: {}".format(requirements)) @@ -1395,9 +1403,10 @@ def install_npm_requirements(query, requirements_file, tmp_dir): log.info("Installing npm requirements: %s", requirements_file) with tempdir(tmp_dir) as temp_dir: - requirements_filename = os.path.basename(requirements_file) - target_file = os.path.join(temp_dir, requirements_filename) - shutil.copyfile(requirements_file, target_file) + temp_copy = TemporaryCopy(os.path.dirname(requirements_file), temp_dir, log) + temp_copy.add(os.path.basename(requirements_file)) + temp_copy.add("package-lock.json", required=False) + temp_copy.copy_to_target_dir() subproc_env = None npm_exec = "npm" @@ -1442,10 +1451,63 @@ def install_npm_requirements(query, requirements_file, tmp_dir): "available in system PATH".format(runtime) ) from e - os.remove(target_file) + temp_copy.remove_from_target_dir() yield temp_dir +class TemporaryCopy: + """Temporarily copy files to a specified location and remove them when + not needed. + """ + + def __init__(self, source_dir_path, target_dir_path, logger=None): + """Initialise with a target and a source directories.""" + self.source_dir_path = source_dir_path + self.target_dir_path = target_dir_path + self._filenames = [] + self._logger = logger + + def _make_source_path(self, filename): + return os.path.join(self.source_dir_path, filename) + + def _make_target_path(self, filename): + return os.path.join(self.target_dir_path, filename) + + def add(self, filename, *, required=True): + """Add a file to be copied from from source to target directory + when `TemporaryCopy.copy_to_target_dir()` is called. + + By default, the file must exist in the source directory. Set `required` + to `False` if the file is optional. + """ + if os.path.exists(self._make_source_path(filename)): + self._filenames.append(filename) + elif required: + raise RuntimeError("File not found: {}".format(filename)) + + def copy_to_target_dir(self): + """Copy files (added so far) to the target directory.""" + for filename in self._filenames: + if self._logger: + self._logger.info("Copying temporarily '%s'", filename) + + shutil.copyfile( + self._make_source_path(filename), + self._make_target_path(filename), + ) + + def remove_from_target_dir(self): + """Remove files (added so far) from the target directory.""" + for filename in self._filenames: + if self._logger: + self._logger.info("Removing temporarily copied '%s'", filename) + + try: + os.remove(self._make_target_path(filename)) + except FileNotFoundError: + pass + + def docker_image_id_command(tag): """""" docker_cmd = ["docker", "images", "--format={{.ID}}", tag]