Skip to content

Commit

Permalink
fix(aws-lambda-python): export poetry dependencies without hashes (#2…
Browse files Browse the repository at this point in the history
…2351)

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 #14201

Fixes #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*
  • Loading branch information
a-bigelow authored Oct 7, 2022
1 parent f1886cf commit 76482f6
Show file tree
Hide file tree
Showing 13 changed files with 1,416 additions and 294 deletions.
5 changes: 4 additions & 1 deletion packages/@aws-cdk/aws-lambda-python/lib/bundling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export class Bundling implements CdkBundlingOptions {
architecture = Architecture.X86_64,
outputPathSuffix = '',
image,
poetryIncludeHashes,
} = props;

const outputPath = path.posix.join(AssetStaging.BUNDLING_OUTPUT_DIR, outputPathSuffix);
Expand All @@ -75,6 +76,7 @@ export class Bundling implements CdkBundlingOptions {
entry,
inputDir: AssetStaging.BUNDLING_INPUT_DIR,
outputDir: outputPath,
poetryIncludeHashes,
});

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

private createBundlingCommand(options: BundlingCommandOptions): string[] {
const packaging = Packaging.fromEntry(options.entry);
const packaging = Packaging.fromEntry(options.entry, options.poetryIncludeHashes);
let bundlingCommands: string[] = [];
bundlingCommands.push(`cp -rTL ${options.inputDir}/ ${options.outputDir}`);
bundlingCommands.push(`cd ${options.outputDir}`);
Expand All @@ -105,6 +107,7 @@ interface BundlingCommandOptions {
readonly entry: string;
readonly inputDir: string;
readonly outputDir: string;
readonly poetryIncludeHashes?: boolean;
}

/**
Expand Down
65 changes: 45 additions & 20 deletions packages/@aws-cdk/aws-lambda-python/lib/packaging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,48 +21,73 @@ export interface PackagingProps {
readonly exportCommand?: string;
}

export interface PoetryPackagingProps {
/**
* Whether to export Poetry dependencies with hashes. Note that this can cause builds to fail if not all dependencies
* export with a hash.
*
* @see https://github.com/aws/aws-cdk/issues/19232
* @default Hashes are NOT included in the exported `requirements.txt` file
*/
readonly poetryIncludeHashes?: boolean;
}

export class Packaging {

/**
* Standard packaging with `pip`.
*/
public static readonly PIP = new Packaging({
dependenciesFile: DependenciesFile.PIP,
});
public static withPip(): Packaging {
return new Packaging({
dependenciesFile: DependenciesFile.PIP,
});
}

/**
* Packaging with `pipenv`.
*/
public static readonly PIPENV = new Packaging({
dependenciesFile: DependenciesFile.PIPENV,
// By default, pipenv creates a virtualenv in `/.local`, so we force it to create one in the package directory.
// At the end, we remove the virtualenv to avoid creating a duplicate copy in the Lambda package.
exportCommand: `PIPENV_VENV_IN_PROJECT=1 pipenv lock -r > ${DependenciesFile.PIP} && rm -rf .venv`,
});
public static withPipenv(): Packaging {
return new Packaging({
dependenciesFile: DependenciesFile.PIPENV,
// By default, pipenv creates a virtualenv in `/.local`, so we force it to create one in the package directory.
// At the end, we remove the virtualenv to avoid creating a duplicate copy in the Lambda package.
exportCommand: `PIPENV_VENV_IN_PROJECT=1 pipenv lock -r > ${DependenciesFile.PIP} && rm -rf .venv`,
});
}

/**
* Packaging with `poetry`.
*/
public static readonly POETRY = new Packaging({
dependenciesFile: DependenciesFile.POETRY,
// Export dependencies with credentials avaiable in the bundling image.
exportCommand: `poetry export --with-credentials --format ${DependenciesFile.PIP} --output ${DependenciesFile.PIP}`,
});
public static withPoetry(props?: PoetryPackagingProps) {
return new Packaging({
dependenciesFile: DependenciesFile.POETRY,
// Export dependencies with credentials available in the bundling image.
exportCommand: [
'poetry', 'export',
...props?.poetryIncludeHashes ? [] : ['--without-hashes'],
'--with-credentials',
'--format', DependenciesFile.PIP,
'--output', DependenciesFile.PIP,
].join(' '),
});
}

/**
* No dependencies or packaging.
*/
public static readonly NONE = new Packaging({ dependenciesFile: DependenciesFile.NONE });
public static withNoPackaging(): Packaging {
return new Packaging({ dependenciesFile: DependenciesFile.NONE });
}

public static fromEntry(entry: string): Packaging {
public static fromEntry(entry: string, poetryIncludeHashes?: boolean): Packaging {
if (fs.existsSync(path.join(entry, DependenciesFile.PIPENV))) {
return Packaging.PIPENV;
return this.withPipenv();
} if (fs.existsSync(path.join(entry, DependenciesFile.POETRY))) {
return Packaging.POETRY;
return this.withPoetry({ poetryIncludeHashes });
} else if (fs.existsSync(path.join(entry, DependenciesFile.PIP))) {
return Packaging.PIP;
return this.withPip();
} else {
return Packaging.NONE;
return this.withNoPackaging();
}
}

Expand Down
10 changes: 10 additions & 0 deletions packages/@aws-cdk/aws-lambda-python/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,16 @@ import { AssetHashType, DockerImage } from '@aws-cdk/core';
* Options for bundling
*/
export interface BundlingOptions {

/**
* Whether to export Poetry dependencies with hashes. Note that this can cause builds to fail if not all dependencies
* export with a hash.
*
* @see https://github.com/aws/aws-cdk/issues/19232
* @default Hashes are NOT included in the exported `requirements.txt` file
*/
readonly poetryIncludeHashes?: boolean;

/**
* Output path suffix: the suffix for the directory into which the bundled output is written.
*
Expand Down
28 changes: 28 additions & 0 deletions packages/@aws-cdk/aws-lambda-python/test/bundling.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,34 @@ test('Bundling a function with poetry dependencies', () => {
outputPathSuffix: 'python',
});

expect(Code.fromAsset).toHaveBeenCalledWith(entry, expect.objectContaining({
bundling: expect.objectContaining({
command: [
'bash', '-c',
'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',
],
}),
}));

const files = fs.readdirSync(assetCode.path);
expect(files).toContain('index.py');
expect(files).toContain('pyproject.toml');
expect(files).toContain('poetry.lock');
// Contains hidden files.
expect(files).toContain('.ignorefile');
});

test('Bundling a function with poetry dependencies, with hashes', () => {
const entry = path.join(__dirname, 'lambda-handler-poetry');

const assetCode = Bundling.bundle({
entry: path.join(entry, '.'),
runtime: Runtime.PYTHON_3_9,
architecture: Architecture.X86_64,
outputPathSuffix: 'python',
poetryIncludeHashes: true,
});

expect(Code.fromAsset).toHaveBeenCalledWith(entry, expect.objectContaining({
bundling: expect.objectContaining({
command: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -404,20 +404,11 @@ var CustomResourceHandler = class {
}
async handle() {
try {
console.log(`Event: ${JSON.stringify({ ...this.event, ResponseURL: "..." })}`);
const response = await this.processEvent(this.event.ResourceProperties);
console.log(`Event output : ${JSON.stringify(response)}`);
await this.respond({
status: "SUCCESS",
reason: "OK",
data: response
});
return response;
} catch (e) {
console.log(e);
await this.respond({
status: "FAILED",
reason: e.message ?? "Internal Error"
});
throw e;
} finally {
clearTimeout(this.timeout);
}
Expand Down Expand Up @@ -479,7 +470,8 @@ var AssertionHandler = class extends CustomResourceHandler {
matchResult.finished();
if (matchResult.hasFailed()) {
result = {
data: JSON.stringify({
failed: true,
assertion: JSON.stringify({
status: "fail",
message: [
...matchResult.toHumanStrings(),
Expand All @@ -488,11 +480,11 @@ var AssertionHandler = class extends CustomResourceHandler {
})
};
if (request2.failDeployment) {
throw new Error(result.data);
throw new Error(result.assertion);
}
} else {
result = {
data: JSON.stringify({
assertion: JSON.stringify({
status: "success"
})
};
Expand Down Expand Up @@ -562,7 +554,10 @@ function flatten(object) {
{},
...function _flatten(child, path = []) {
return [].concat(...Object.keys(child).map((key) => {
const childKey = Buffer.isBuffer(child[key]) ? child[key].toString("utf8") : child[key];
let childKey = Buffer.isBuffer(child[key]) ? child[key].toString("utf8") : child[key];
if (typeof childKey === "string") {
childKey = isJsonString(childKey);
}
return typeof childKey === "object" && childKey !== null ? _flatten(childKey, path.concat([key])) : { [path.concat([key]).join(".")]: childKey };
}));
}(object)
Expand All @@ -572,6 +567,9 @@ var AwsApiCallHandler = class extends CustomResourceHandler {
async processEvent(request2) {
const AWS = require("aws-sdk");
console.log(`AWS SDK VERSION: ${AWS.VERSION}`);
if (!Object.prototype.hasOwnProperty.call(AWS, request2.service)) {
throw Error(`Service ${request2.service} does not exist in AWS SDK version ${AWS.VERSION}.`);
}
const service = new AWS[request2.service]();
const response = await service[request2.api](request2.parameters && decode(request2.parameters)).promise();
console.log(`SDK response received ${JSON.stringify(response)}`);
Expand All @@ -582,28 +580,87 @@ var AwsApiCallHandler = class extends CustomResourceHandler {
const flatData = {
...flatten(respond)
};
return request2.flattenResponse === "true" ? flatData : respond;
const resp = request2.flattenResponse === "true" ? flatData : respond;
console.log(`Returning result ${JSON.stringify(resp)}`);
return resp;
}
};
function isJsonString(value) {
try {
return JSON.parse(value);
} catch {
return value;
}
}

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

// lib/assertions/providers/lambda-handler/index.ts
async function handler(event, context) {
console.log(`Event: ${JSON.stringify({ ...event, ResponseURL: "..." })}`);
const provider = createResourceHandler(event, context);
await provider.handle();
try {
if (event.RequestType === "Delete") {
await provider.respond({
status: "SUCCESS",
reason: "OK"
});
return;
}
const result = await provider.handle();
const actualPath = event.ResourceProperties.actualPath;
const actual = actualPath ? result[`apiCallResponse.${actualPath}`] : result.apiCallResponse;
if ("expected" in event.ResourceProperties) {
const assertion = new AssertionHandler({
...event,
ResourceProperties: {
ServiceToken: event.ServiceToken,
actual,
expected: event.ResourceProperties.expected
}
}, context);
try {
const assertionResult = await assertion.handle();
await provider.respond({
status: "SUCCESS",
reason: "OK",
data: {
...assertionResult,
...result
}
});
return;
} catch (e) {
await provider.respond({
status: "FAILED",
reason: e.message ?? "Internal Error"
});
return;
}
}
await provider.respond({
status: "SUCCESS",
reason: "OK",
data: result
});
} catch (e) {
await provider.respond({
status: "FAILED",
reason: e.message ?? "Internal Error"
});
return;
}
return;
}
function createResourceHandler(event, context) {
if (event.ResourceType.startsWith(SDK_RESOURCE_TYPE_PREFIX)) {
return new AwsApiCallHandler(event, context);
}
switch (event.ResourceType) {
case ASSERT_RESOURCE_TYPE:
return new AssertionHandler(event, context);
default:
throw new Error(`Unsupported resource type "${event.ResourceType}`);
} else if (event.ResourceType.startsWith(ASSERT_RESOURCE_TYPE)) {
return new AssertionHandler(event, context);
} else {
throw new Error(`Unsupported resource type "${event.ResourceType}`);
}
}
// Annotate the CommonJS export names for ESM import in node:
Expand Down
Loading

0 comments on commit 76482f6

Please sign in to comment.