fix: Ensure proper support for mixed runtimes and architectures (#815)

* feat: Use function runtime & arch for docker * docs: Update readme for python3.9 * feat: Do not zip req for non-py functions * ci: Bump internal package version / python version * fix: Rename mixed test name to be more descriptive --------- Co-authored-by: Stijn IJzermans <[email protected]>
serverless · Feb 10, 2024 · 27b70f4 · 27b70f4
1 parent 787b479
commit 27b70f4
Show file tree

Hide file tree

Showing 20 changed files with 187 additions and 25 deletions.
diff --git a/.python-version b/.python-version
@@ -1 +1 @@
-3.7
+3.9
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -13,12 +13,14 @@ Welcome, and thanks in advance for your help!
 ## Setup
 
 Pre-Reqs:
-* Python 3.7
-* [poetry](https://python-poetry.org/docs/) (if you use multiple versions of Python be sure to install it with python 3.7)
-* Perl (used in the tests)
-* Node v14 or v16
+
+- Python 3.9
+- [poetry](https://python-poetry.org/docs/) (if you use multiple versions of Python be sure to install it with python 3.9)
+- Perl (used in the tests)
+- Node v14 or v16
 
 Then, to begin development:
+
 1. fork the repository
 2. `npm install -g serverless@<VERSION>` (check the peer dependencies in the root `package.json` file for the version)
 3. run `npm install` in its root folder

diff --git a/index.js b/index.js
@@ -106,13 +106,8 @@ class ServerlessPythonRequirements {
       throw new Error(
         'Python Requirements: you can provide a dockerImage or a dockerFile option, not both.'
       );
-    } else if (!options.dockerFile) {
-      // If no dockerFile is provided, use default image
-      const architecture =
-        this.serverless.service.provider.architecture || 'x86_64';
-      const defaultImage = `public.ecr.aws/sam/build-${this.serverless.service.provider.runtime}:latest-${architecture}`;
-      options.dockerImage = options.dockerImage || defaultImage;
     }
+
     if (options.layer) {
       // If layer was set as a boolean, set it to an empty object to use the layer defaults.
       if (options.layer === true) {
@@ -188,6 +183,18 @@ class ServerlessPythonRequirements {
       this.commands.requirements.type = 'container';
     }
 
+    this.dockerImageForFunction = (funcOptions) => {
+      const runtime =
+        funcOptions.runtime || this.serverless.service.provider.runtime;
+
+      const architecture =
+        funcOptions.architecture ||
+        this.serverless.service.provider.architecture ||
+        'x86_64';
+      const defaultImage = `public.ecr.aws/sam/build-${runtime}:latest-${architecture}`;
+      return this.options.dockerImage || defaultImage;
+    };
+
     const isFunctionRuntimePython = (args) => {
       // If functionObj.runtime is undefined, python.
       if (!args[1].functionObj || !args[1].functionObj.runtime) {

diff --git a/lib/pip.js b/lib/pip.js
@@ -125,12 +125,13 @@ async function pipAcceptsSystem(pythonBin, pluginInstance) {
 /**
  * Install requirements described from requirements in the targetFolder into that same targetFolder
  * @param {string} targetFolder
- * @param {Object} serverless
- * @param {Object} options
+ * @param {Object} pluginInstance
+ * @param {Object} funcOptions
  * @return {undefined}
  */
-async function installRequirements(targetFolder, pluginInstance) {
-  const { options, serverless, log, progress } = pluginInstance;
+async function installRequirements(targetFolder, pluginInstance, funcOptions) {
+  const { options, serverless, log, progress, dockerImageForFunction } =
+    pluginInstance;
   const targetRequirementsTxt = path.join(targetFolder, 'requirements.txt');
 
   let installProgress;
@@ -253,7 +254,7 @@ async function installRequirements(targetFolder, pluginInstance) {
           buildDockerImageProgress && buildDockerImageProgress.remove();
         }
       } else {
-        dockerImage = options.dockerImage;
+        dockerImage = dockerImageForFunction(funcOptions);
       }
       if (log) {
         log.info(`Docker Image: ${dockerImage}`);
@@ -691,7 +692,7 @@ async function installRequirementsIfNeeded(
   fse.copySync(slsReqsTxt, path.join(workingReqsFolder, 'requirements.txt'));
 
   // Then install our requirements from this folder
-  await installRequirements(workingReqsFolder, pluginInstance);
+  await installRequirements(workingReqsFolder, pluginInstance, funcOptions);
 
   // Copy vendor libraries to requirements folder
   if (options.vendor) {

diff --git a/lib/zip.js b/lib/zip.js
@@ -114,6 +114,11 @@ function packRequirements() {
   if (this.options.zip) {
     if (this.serverless.service.package.individually) {
       return BbPromise.resolve(this.targetFuncs)
+        .filter((func) => {
+          return (
+            func.runtime || this.serverless.service.provider.runtime
+          ).match(/^python.*/);
+        })
         .map((f) => {
           if (!get(f, 'module')) {
             set(f, ['module'], '.');

diff --git a/test.js b/test.js
@@ -1373,6 +1373,89 @@ test(
   { skip: !canUseDocker() || process.platform === 'win32' }
 );
 
+test(
+  'py3.9 can package flask running in docker with module runtime & architecture of function',
+  async (t) => {
+    process.chdir('tests/individually_mixed_runtime');
+    const path = npm(['pack', '../..']);
+    npm(['i', path]);
+
+    sls(['package'], {
+      env: { dockerizePip: 'true' },
+    });
+
+    const zipfiles_hello2 = await listZipFiles(
+      '.serverless/module2-sls-py-req-test-indiv-mixed-runtime-dev-hello2.zip'
+    );
+    t.true(
+      zipfiles_hello2.includes('handler2.py'),
+      'handler2.py is packaged at root level in function hello2'
+    );
+    t.true(
+      zipfiles_hello2.includes(`flask${sep}__init__.py`),
+      'flask is packaged in function hello2'
+    );
+  },
+  {
+    skip: !canUseDocker() || process.platform === 'win32',
+  }
+);
+
+test(
+  'py3.9 can package flask succesfully when using mixed architecture, docker and zipping',
+  async (t) => {
+    process.chdir('tests/individually_mixed_runtime');
+    const path = npm(['pack', '../..']);
+
+    npm(['i', path]);
+    sls(['package'], { env: { dockerizePip: 'true', zip: 'true' } });
+
+    const zipfiles_hello = await listZipFiles('.serverless/hello1.zip');
+    t.true(
+      zipfiles_hello.includes(`module1${sep}handler1.ts`),
+      'handler1.ts is packaged in module dir for hello1'
+    );
+    t.false(
+      zipfiles_hello.includes('handler2.py'),
+      'handler2.py is NOT packaged at root level in function hello1'
+    );
+    t.false(
+      zipfiles_hello.includes(`flask${sep}__init__.py`),
+      'flask is NOT packaged in function hello1'
+    );
+
+    const zipfiles_hello2 = await listZipFiles(
+      '.serverless/module2-sls-py-req-test-indiv-mixed-runtime-dev-hello2.zip'
+    );
+    const zippedReqs = await listRequirementsZipFiles(
+      '.serverless/module2-sls-py-req-test-indiv-mixed-runtime-dev-hello2.zip'
+    );
+    t.true(
+      zipfiles_hello2.includes('handler2.py'),
+      'handler2.py is packaged at root level in function hello2'
+    );
+    t.false(
+      zipfiles_hello2.includes(`module1${sep}handler1.ts`),
+      'handler1.ts is NOT included at module1 level in hello2'
+    );
+    t.false(
+      zipfiles_hello2.includes(`pyaml${sep}__init__.py`),
+      'pyaml is NOT packaged in function hello2'
+    );
+    t.false(
+      zipfiles_hello2.includes(`boto3${sep}__init__.py`),
+      'boto3 is NOT included in zipfile'
+    );
+    t.true(
+      zippedReqs.includes(`flask${sep}__init__.py`),
+      'flask is packaged in function hello2 in requirements.zip'
+    );
+
+    t.end();
+  },
+  { skip: !canUseDocker() || process.platform === 'win32' }
+);
+
 test(
   'py3.9 uses download cache by default option',
   async (t) => {

diff --git a/tests/base/package.json b/tests/base/package.json
@@ -9,6 +9,6 @@
   "author": "",
   "license": "ISC",
   "dependencies": {
-    "serverless-python-requirements": "file:serverless-python-requirements-6.0.0.tgz"
+    "serverless-python-requirements": "file:serverless-python-requirements-6.0.1.tgz"
   }
 }
diff --git a/tests/individually/package.json b/tests/individually/package.json
@@ -9,6 +9,6 @@
   "author": "",
   "license": "ISC",
   "dependencies": {
-    "serverless-python-requirements": "file:serverless-python-requirements-6.0.0.tgz"
+    "serverless-python-requirements": "file:serverless-python-requirements-6.0.1.tgz"
   }
 }
diff --git a/tests/individually_mixed_runtime/module1/handler1.ts b/tests/individually_mixed_runtime/module1/handler1.ts
@@ -0,0 +1,3 @@
+function hello() {
+    return "hello"
+}
diff --git a/tests/individually_mixed_runtime/module2/handler2.py b/tests/individually_mixed_runtime/module2/handler2.py
@@ -0,0 +1,6 @@
+import flask
+
+def hello(event, context):
+    return {
+        'status': 200,
+    }
diff --git a/tests/individually_mixed_runtime/module2/requirements.txt b/tests/individually_mixed_runtime/module2/requirements.txt
@@ -0,0 +1 @@
+flask==2.0.3
diff --git a/tests/individually_mixed_runtime/package.json b/tests/individually_mixed_runtime/package.json
@@ -0,0 +1,14 @@
+{
+  "name": "example",
+  "version": "1.0.0",
+  "description": "",
+  "main": "index.js",
+  "scripts": {
+    "test": "echo \"Error: no test specified\" && exit 1"
+  },
+  "author": "",
+  "license": "ISC",
+  "dependencies": {
+    "serverless-python-requirements": "file:serverless-python-requirements-6.0.1.tgz"
+  }
+}
diff --git a/tests/individually_mixed_runtime/requirements-common.txt b/tests/individually_mixed_runtime/requirements-common.txt
@@ -0,0 +1 @@
+boto3
diff --git a/tests/individually_mixed_runtime/serverless.yml b/tests/individually_mixed_runtime/serverless.yml
@@ -0,0 +1,39 @@
+service: sls-py-req-test-indiv-mixed-runtime
+
+provider:
+  name: aws
+  runtime: nodejs18.x
+  architecture: arm64
+
+package:
+  individually: true
+
+custom:
+  pythonRequirements:
+    dockerizePip: ${env:dockerizePip, self:custom.defaults.dockerizePip}
+    zip: ${env:zip, self:custom.defaults.zip}
+  defaults:
+    dockerizePip: false
+    zip: false
+
+functions:
+  hello1:
+    handler: handler1.hello
+    architecture: x86_64
+    package:
+      patterns:
+        - '!**'
+        - 'module1/**'
+
+  hello2:
+    handler: handler2.hello
+    module: module2
+    runtime: python3.9
+    architecture: x86_64
+    package:
+      patterns:
+        - '!**'
+        - 'module2/**'
+
+plugins:
+  - serverless-python-requirements
diff --git a/tests/non_build_pyproject/package.json b/tests/non_build_pyproject/package.json
@@ -9,6 +9,6 @@
   "author": "",
   "license": "ISC",
   "dependencies": {
-    "serverless-python-requirements": "file:serverless-python-requirements-6.0.0.tgz"
+    "serverless-python-requirements": "file:serverless-python-requirements-6.0.1.tgz"
   }
 }
diff --git a/tests/non_poetry_pyproject/package.json b/tests/non_poetry_pyproject/package.json
@@ -9,6 +9,6 @@
   "author": "",
   "license": "ISC",
   "dependencies": {
-    "serverless-python-requirements": "file:serverless-python-requirements-6.0.0.tgz"
+    "serverless-python-requirements": "file:serverless-python-requirements-6.0.1.tgz"
   }
 }
diff --git a/tests/pipenv/package.json b/tests/pipenv/package.json
@@ -9,6 +9,6 @@
   "author": "",
   "license": "ISC",
   "dependencies": {
-    "serverless-python-requirements": "file:serverless-python-requirements-6.0.0.tgz"
+    "serverless-python-requirements": "file:serverless-python-requirements-6.0.1.tgz"
   }
 }
diff --git a/tests/poetry/package.json b/tests/poetry/package.json
@@ -9,6 +9,6 @@
   "author": "",
   "license": "ISC",
   "dependencies": {
-    "serverless-python-requirements": "file:serverless-python-requirements-6.0.0.tgz"
+    "serverless-python-requirements": "file:serverless-python-requirements-6.0.1.tgz"
   }
 }
diff --git a/tests/poetry_individually/package.json b/tests/poetry_individually/package.json
@@ -9,6 +9,6 @@
   "author": "",
   "license": "ISC",
   "dependencies": {
-    "serverless-python-requirements": "file:serverless-python-requirements-6.0.0.tgz"
+    "serverless-python-requirements": "file:serverless-python-requirements-6.0.1.tgz"
   }
 }
diff --git a/tests/poetry_packages/package.json b/tests/poetry_packages/package.json
@@ -9,6 +9,6 @@
   "author": "",
   "license": "ISC",
   "dependencies": {
-    "serverless-python-requirements": "file:serverless-python-requirements-6.0.0.tgz"
+    "serverless-python-requirements": "file:serverless-python-requirements-6.0.1.tgz"
   }
 }