Skip to content

Commit 2369844

Browse files
a-bigelowmadeline-k
authored andcommitted
fix(aws-lambda-python): export poetry dependencies without hashes (aws#22351)
Export poetry dependencies without hashes to prevent bundling failures when a dependency provides a hash. Without this flag, users relying on the Poetry python dependency manager need to manually export their own `requirements.txt` file, as described in aws#14201 Fixes aws#19232 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent da4efaf commit 2369844

File tree

13 files changed

+1416
-294
lines changed

13 files changed

+1416
-294
lines changed

Diff for: packages/@aws-cdk/aws-lambda-python/lib/bundling.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ export class Bundling implements CdkBundlingOptions {
6767
architecture = Architecture.X86_64,
6868
outputPathSuffix = '',
6969
image,
70+
poetryIncludeHashes,
7071
} = props;
7172

7273
const outputPath = path.posix.join(AssetStaging.BUNDLING_OUTPUT_DIR, outputPathSuffix);
@@ -75,6 +76,7 @@ export class Bundling implements CdkBundlingOptions {
7576
entry,
7677
inputDir: AssetStaging.BUNDLING_INPUT_DIR,
7778
outputDir: outputPath,
79+
poetryIncludeHashes,
7880
});
7981

8082
this.image = image ?? DockerImage.fromBuild(path.join(__dirname, '../lib'), {
@@ -89,7 +91,7 @@ export class Bundling implements CdkBundlingOptions {
8991
}
9092

9193
private createBundlingCommand(options: BundlingCommandOptions): string[] {
92-
const packaging = Packaging.fromEntry(options.entry);
94+
const packaging = Packaging.fromEntry(options.entry, options.poetryIncludeHashes);
9395
let bundlingCommands: string[] = [];
9496
bundlingCommands.push(`cp -rTL ${options.inputDir}/ ${options.outputDir}`);
9597
bundlingCommands.push(`cd ${options.outputDir}`);
@@ -105,6 +107,7 @@ interface BundlingCommandOptions {
105107
readonly entry: string;
106108
readonly inputDir: string;
107109
readonly outputDir: string;
110+
readonly poetryIncludeHashes?: boolean;
108111
}
109112

110113
/**

Diff for: packages/@aws-cdk/aws-lambda-python/lib/packaging.ts

+45-20
Original file line numberDiff line numberDiff line change
@@ -21,48 +21,73 @@ export interface PackagingProps {
2121
readonly exportCommand?: string;
2222
}
2323

24+
export interface PoetryPackagingProps {
25+
/**
26+
* Whether to export Poetry dependencies with hashes. Note that this can cause builds to fail if not all dependencies
27+
* export with a hash.
28+
*
29+
* @see https://github.com/aws/aws-cdk/issues/19232
30+
* @default Hashes are NOT included in the exported `requirements.txt` file
31+
*/
32+
readonly poetryIncludeHashes?: boolean;
33+
}
34+
2435
export class Packaging {
2536

2637
/**
2738
* Standard packaging with `pip`.
2839
*/
29-
public static readonly PIP = new Packaging({
30-
dependenciesFile: DependenciesFile.PIP,
31-
});
40+
public static withPip(): Packaging {
41+
return new Packaging({
42+
dependenciesFile: DependenciesFile.PIP,
43+
});
44+
}
3245

3346
/**
3447
* Packaging with `pipenv`.
3548
*/
36-
public static readonly PIPENV = new Packaging({
37-
dependenciesFile: DependenciesFile.PIPENV,
38-
// By default, pipenv creates a virtualenv in `/.local`, so we force it to create one in the package directory.
39-
// At the end, we remove the virtualenv to avoid creating a duplicate copy in the Lambda package.
40-
exportCommand: `PIPENV_VENV_IN_PROJECT=1 pipenv lock -r > ${DependenciesFile.PIP} && rm -rf .venv`,
41-
});
49+
public static withPipenv(): Packaging {
50+
return new Packaging({
51+
dependenciesFile: DependenciesFile.PIPENV,
52+
// By default, pipenv creates a virtualenv in `/.local`, so we force it to create one in the package directory.
53+
// At the end, we remove the virtualenv to avoid creating a duplicate copy in the Lambda package.
54+
exportCommand: `PIPENV_VENV_IN_PROJECT=1 pipenv lock -r > ${DependenciesFile.PIP} && rm -rf .venv`,
55+
});
56+
}
4257

4358
/**
4459
* Packaging with `poetry`.
4560
*/
46-
public static readonly POETRY = new Packaging({
47-
dependenciesFile: DependenciesFile.POETRY,
48-
// Export dependencies with credentials avaiable in the bundling image.
49-
exportCommand: `poetry export --with-credentials --format ${DependenciesFile.PIP} --output ${DependenciesFile.PIP}`,
50-
});
61+
public static withPoetry(props?: PoetryPackagingProps) {
62+
return new Packaging({
63+
dependenciesFile: DependenciesFile.POETRY,
64+
// Export dependencies with credentials available in the bundling image.
65+
exportCommand: [
66+
'poetry', 'export',
67+
...props?.poetryIncludeHashes ? [] : ['--without-hashes'],
68+
'--with-credentials',
69+
'--format', DependenciesFile.PIP,
70+
'--output', DependenciesFile.PIP,
71+
].join(' '),
72+
});
73+
}
5174

5275
/**
5376
* No dependencies or packaging.
5477
*/
55-
public static readonly NONE = new Packaging({ dependenciesFile: DependenciesFile.NONE });
78+
public static withNoPackaging(): Packaging {
79+
return new Packaging({ dependenciesFile: DependenciesFile.NONE });
80+
}
5681

57-
public static fromEntry(entry: string): Packaging {
82+
public static fromEntry(entry: string, poetryIncludeHashes?: boolean): Packaging {
5883
if (fs.existsSync(path.join(entry, DependenciesFile.PIPENV))) {
59-
return Packaging.PIPENV;
84+
return this.withPipenv();
6085
} if (fs.existsSync(path.join(entry, DependenciesFile.POETRY))) {
61-
return Packaging.POETRY;
86+
return this.withPoetry({ poetryIncludeHashes });
6287
} else if (fs.existsSync(path.join(entry, DependenciesFile.PIP))) {
63-
return Packaging.PIP;
88+
return this.withPip();
6489
} else {
65-
return Packaging.NONE;
90+
return this.withNoPackaging();
6691
}
6792
}
6893

Diff for: packages/@aws-cdk/aws-lambda-python/lib/types.ts

+10
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,16 @@ import { AssetHashType, DockerImage } from '@aws-cdk/core';
55
* Options for bundling
66
*/
77
export interface BundlingOptions {
8+
9+
/**
10+
* Whether to export Poetry dependencies with hashes. Note that this can cause builds to fail if not all dependencies
11+
* export with a hash.
12+
*
13+
* @see https://github.com/aws/aws-cdk/issues/19232
14+
* @default Hashes are NOT included in the exported `requirements.txt` file
15+
*/
16+
readonly poetryIncludeHashes?: boolean;
17+
818
/**
919
* Output path suffix: the suffix for the directory into which the bundled output is written.
1020
*

Diff for: packages/@aws-cdk/aws-lambda-python/test/bundling.test.ts

+28
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,34 @@ test('Bundling a function with poetry dependencies', () => {
172172
outputPathSuffix: 'python',
173173
});
174174

175+
expect(Code.fromAsset).toHaveBeenCalledWith(entry, expect.objectContaining({
176+
bundling: expect.objectContaining({
177+
command: [
178+
'bash', '-c',
179+
'cp -rTL /asset-input/ /asset-output/python && cd /asset-output/python && poetry export --without-hashes --with-credentials --format requirements.txt --output requirements.txt && python -m pip install -r requirements.txt -t /asset-output/python',
180+
],
181+
}),
182+
}));
183+
184+
const files = fs.readdirSync(assetCode.path);
185+
expect(files).toContain('index.py');
186+
expect(files).toContain('pyproject.toml');
187+
expect(files).toContain('poetry.lock');
188+
// Contains hidden files.
189+
expect(files).toContain('.ignorefile');
190+
});
191+
192+
test('Bundling a function with poetry dependencies, with hashes', () => {
193+
const entry = path.join(__dirname, 'lambda-handler-poetry');
194+
195+
const assetCode = Bundling.bundle({
196+
entry: path.join(entry, '.'),
197+
runtime: Runtime.PYTHON_3_9,
198+
architecture: Architecture.X86_64,
199+
outputPathSuffix: 'python',
200+
poetryIncludeHashes: true,
201+
});
202+
175203
expect(Code.fromAsset).toHaveBeenCalledWith(entry, expect.objectContaining({
176204
bundling: expect.objectContaining({
177205
command: [
Original file line numberDiff line numberDiff line change
@@ -404,20 +404,11 @@ var CustomResourceHandler = class {
404404
}
405405
async handle() {
406406
try {
407-
console.log(`Event: ${JSON.stringify({ ...this.event, ResponseURL: "..." })}`);
408407
const response = await this.processEvent(this.event.ResourceProperties);
409-
console.log(`Event output : ${JSON.stringify(response)}`);
410-
await this.respond({
411-
status: "SUCCESS",
412-
reason: "OK",
413-
data: response
414-
});
408+
return response;
415409
} catch (e) {
416410
console.log(e);
417-
await this.respond({
418-
status: "FAILED",
419-
reason: e.message ?? "Internal Error"
420-
});
411+
throw e;
421412
} finally {
422413
clearTimeout(this.timeout);
423414
}
@@ -479,7 +470,8 @@ var AssertionHandler = class extends CustomResourceHandler {
479470
matchResult.finished();
480471
if (matchResult.hasFailed()) {
481472
result = {
482-
data: JSON.stringify({
473+
failed: true,
474+
assertion: JSON.stringify({
483475
status: "fail",
484476
message: [
485477
...matchResult.toHumanStrings(),
@@ -488,11 +480,11 @@ var AssertionHandler = class extends CustomResourceHandler {
488480
})
489481
};
490482
if (request2.failDeployment) {
491-
throw new Error(result.data);
483+
throw new Error(result.assertion);
492484
}
493485
} else {
494486
result = {
495-
data: JSON.stringify({
487+
assertion: JSON.stringify({
496488
status: "success"
497489
})
498490
};
@@ -562,7 +554,10 @@ function flatten(object) {
562554
{},
563555
...function _flatten(child, path = []) {
564556
return [].concat(...Object.keys(child).map((key) => {
565-
const childKey = Buffer.isBuffer(child[key]) ? child[key].toString("utf8") : child[key];
557+
let childKey = Buffer.isBuffer(child[key]) ? child[key].toString("utf8") : child[key];
558+
if (typeof childKey === "string") {
559+
childKey = isJsonString(childKey);
560+
}
566561
return typeof childKey === "object" && childKey !== null ? _flatten(childKey, path.concat([key])) : { [path.concat([key]).join(".")]: childKey };
567562
}));
568563
}(object)
@@ -572,6 +567,9 @@ var AwsApiCallHandler = class extends CustomResourceHandler {
572567
async processEvent(request2) {
573568
const AWS = require("aws-sdk");
574569
console.log(`AWS SDK VERSION: ${AWS.VERSION}`);
570+
if (!Object.prototype.hasOwnProperty.call(AWS, request2.service)) {
571+
throw Error(`Service ${request2.service} does not exist in AWS SDK version ${AWS.VERSION}.`);
572+
}
575573
const service = new AWS[request2.service]();
576574
const response = await service[request2.api](request2.parameters && decode(request2.parameters)).promise();
577575
console.log(`SDK response received ${JSON.stringify(response)}`);
@@ -582,28 +580,87 @@ var AwsApiCallHandler = class extends CustomResourceHandler {
582580
const flatData = {
583581
...flatten(respond)
584582
};
585-
return request2.flattenResponse === "true" ? flatData : respond;
583+
const resp = request2.flattenResponse === "true" ? flatData : respond;
584+
console.log(`Returning result ${JSON.stringify(resp)}`);
585+
return resp;
586586
}
587587
};
588+
function isJsonString(value) {
589+
try {
590+
return JSON.parse(value);
591+
} catch {
592+
return value;
593+
}
594+
}
588595

589596
// lib/assertions/providers/lambda-handler/types.ts
590597
var ASSERT_RESOURCE_TYPE = "Custom::DeployAssert@AssertEquals";
591598
var SDK_RESOURCE_TYPE_PREFIX = "Custom::DeployAssert@SdkCall";
592599

593600
// lib/assertions/providers/lambda-handler/index.ts
594601
async function handler(event, context) {
602+
console.log(`Event: ${JSON.stringify({ ...event, ResponseURL: "..." })}`);
595603
const provider = createResourceHandler(event, context);
596-
await provider.handle();
604+
try {
605+
if (event.RequestType === "Delete") {
606+
await provider.respond({
607+
status: "SUCCESS",
608+
reason: "OK"
609+
});
610+
return;
611+
}
612+
const result = await provider.handle();
613+
const actualPath = event.ResourceProperties.actualPath;
614+
const actual = actualPath ? result[`apiCallResponse.${actualPath}`] : result.apiCallResponse;
615+
if ("expected" in event.ResourceProperties) {
616+
const assertion = new AssertionHandler({
617+
...event,
618+
ResourceProperties: {
619+
ServiceToken: event.ServiceToken,
620+
actual,
621+
expected: event.ResourceProperties.expected
622+
}
623+
}, context);
624+
try {
625+
const assertionResult = await assertion.handle();
626+
await provider.respond({
627+
status: "SUCCESS",
628+
reason: "OK",
629+
data: {
630+
...assertionResult,
631+
...result
632+
}
633+
});
634+
return;
635+
} catch (e) {
636+
await provider.respond({
637+
status: "FAILED",
638+
reason: e.message ?? "Internal Error"
639+
});
640+
return;
641+
}
642+
}
643+
await provider.respond({
644+
status: "SUCCESS",
645+
reason: "OK",
646+
data: result
647+
});
648+
} catch (e) {
649+
await provider.respond({
650+
status: "FAILED",
651+
reason: e.message ?? "Internal Error"
652+
});
653+
return;
654+
}
655+
return;
597656
}
598657
function createResourceHandler(event, context) {
599658
if (event.ResourceType.startsWith(SDK_RESOURCE_TYPE_PREFIX)) {
600659
return new AwsApiCallHandler(event, context);
601-
}
602-
switch (event.ResourceType) {
603-
case ASSERT_RESOURCE_TYPE:
604-
return new AssertionHandler(event, context);
605-
default:
606-
throw new Error(`Unsupported resource type "${event.ResourceType}`);
660+
} else if (event.ResourceType.startsWith(ASSERT_RESOURCE_TYPE)) {
661+
return new AssertionHandler(event, context);
662+
} else {
663+
throw new Error(`Unsupported resource type "${event.ResourceType}`);
607664
}
608665
}
609666
// Annotate the CommonJS export names for ESM import in node:

0 commit comments

Comments
 (0)