Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(lambda-layer-awscli): Dynamically load asset for AwsCliLayer, with bundled fallback #21938

Closed
Closed
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
217a199
feat(lambda): add Code.fromAssetConstruct API for creating Code objec…
madeline-k Sep 7, 2022
3819c26
add to README
madeline-k Sep 7, 2022
6d45a8f
feat: Dynamically load asset for AwsCliLayer, with bundled fallback
madeline-k Sep 7, 2022
7e68272
Merge branch 'main' into madeline-k/reduce-package-size/lambda-layer-…
madeline-k Sep 20, 2022
c244bc8
Revert "feat(lambda): add Code.fromAssetConstruct API for creating Co…
madeline-k Sep 20, 2022
a829e28
Revert "add to README"
madeline-k Sep 20, 2022
b01414b
update to use latest name and api from @aws-cdk/asset-awscli-v1 package
madeline-k Sep 20, 2022
dcaf2ce
temporarily adjust coverage thresholds so that jest succeeds, and the…
madeline-k Sep 20, 2022
5a35d5a
wip
madeline-k Sep 21, 2022
da1f1ba
Merge branch 'main' into madeline-k/reduce-package-size/lambda-layer-…
madeline-k Sep 21, 2022
1f856c7
updates
madeline-k Sep 27, 2022
f0376b4
re-add original zip generation
madeline-k Sep 28, 2022
403313a
download zip from the external npm package, and fallback to use that
madeline-k Sep 29, 2022
18fc81c
refactor to use a slightly more functional pattern. This should be mo…
madeline-k Sep 29, 2022
9312860
remove saving state in static variable
madeline-k Sep 29, 2022
3f901f1
update jest config to raise coverage thresholds again
madeline-k Sep 29, 2022
42b84ed
make some static methods public and @internal so they can be tested
kaizencc Sep 29, 2022
f236539
add quiet option on awscli layer
kaizencc Oct 4, 2022
0f0a89e
include requirements.txt in the directory that is used to generate th…
madeline-k Oct 11, 2022
19b9c8b
some logging updates
madeline-k Oct 11, 2022
a14b01a
update logging, tests, and add a construct for the Cli Notice
madeline-k Oct 11, 2022
7c7083f
add README troubleshooting instructions
madeline-k Oct 11, 2022
a5e37e7
package.json updates
madeline-k Oct 11, 2022
c9b8740
Merge branch 'main' into madeline-k/reduce-package-size/lambda-layer-…
madeline-k Oct 11, 2022
4e84738
update yarn.lock to use 2.0.0, linter fixes in README, set CDK_DEBUG …
madeline-k Oct 11, 2022
6d5dd99
add set -eu
madeline-k Oct 11, 2022
7657203
modify pre-build script to copy files from local node_modules
madeline-k Oct 12, 2022
bda4f88
refactor functions into a separate file so they can be private and us…
madeline-k Oct 12, 2022
8c2edba
more updates
madeline-k Oct 12, 2022
b2af0c6
get target version at build time
madeline-k Oct 12, 2022
01f194e
update warning message
madeline-k Oct 12, 2022
4b11d81
remove WARNING from the warning. It's a warning.
madeline-k Oct 12, 2022
a5e35f6
fix comma placement, and add real github issue link
madeline-k Oct 12, 2022
7415ae8
use semver
madeline-k Oct 12, 2022
729f9cb
yarn build --fix
madeline-k Oct 12, 2022
9227e93
fix string match in test
madeline-k Oct 12, 2022
af77de9
use compatible semver range
madeline-k Oct 12, 2022
33df916
update asset-awscli dependency, add error handling for mkdirSync
madeline-k Oct 12, 2022
70d9fb7
remove requirement.txt from being tracked by git, since it is now gen…
madeline-k Oct 12, 2022
b9d5bd8
temporarily lower coverage thresholds
madeline-k Oct 12, 2022
2ab6282
fix: remove node_modules folder after test adds it in
kaizencc Oct 13, 2022
e97124c
fix download location to avoid duplicate node_modules folders
kaizencc Oct 13, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions packages/@aws-cdk/cx-api/lib/directories.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import * as fs from 'fs';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Relocating this file from the CLI package (aws-cdk), so that it can be re-used in the CLI and the Framework. Let me know if there is a better location for this!

import * as os from 'os';
import * as path from 'path';

/**
* Return a location that will be used as the CDK home directory.
* Currently the only thing that is placed here is the cache.
*
* First try to use the users home directory (i.e. /home/someuser/),
* but if that directory does not exist for some reason create a tmp directory.
*
* Typically it wouldn't make sense to create a one time use tmp directory for
* the purpose of creating a cache, but since this only applies to users that do
* not have a home directory (some CI systems?) this should be fine.
*/
export function cdkHomeDir() {
const tmpDir = fs.realpathSync(os.tmpdir());
let home;
try {
home = path.join((os.userInfo().homedir ?? os.homedir()).trim(), '.cdk');
} catch {}
return process.env.CDK_HOME
? path.resolve(process.env.CDK_HOME)
: home || fs.mkdtempSync(path.join(tmpDir, '.cdk')).trim();
}
1 change: 1 addition & 0 deletions packages/@aws-cdk/cx-api/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ export * from './metadata';
export * from './features';
export * from './placeholders';
export * from './app';
export * from './directories';
9 changes: 9 additions & 0 deletions packages/@aws-cdk/lambda-layer-awscli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,12 @@ fn.addLayers(new AwsCliLayer(this, 'AwsCliLayer'));
```

The CLI will be installed under `/opt/awscli/aws`.

## Troubleshooting

### WARNING! ACTION REQUIRED! Your CDK application is using AwsCliLayer and this construct may experience a breaking change for your environment in a future release.

If you see the above message when synthesizing your CDK app, this is because we have introduced a change to dynamically load the Asset construct used by AwsCliLayer at runtime. This message appears when the dynamic loading fails due to restrictions in your environment, and it falls back to using an Asset bundled in aws-cdk-lib. We plan to remove this fallback, and at that time your application may be broken. To prevent this, add an explicit dependency on @aws-cdk/asset-awscli-v1. If you experience any issues, please reach out to us by commenting on https://github.com/aws/aws-cdk/issues/999999999.

TODO:
Add language-specific instructions.
9 changes: 8 additions & 1 deletion packages/@aws-cdk/lambda-layer-awscli/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,9 @@
const baseConfig = require('@aws-cdk/cdk-build-tools/config/jest.config');
module.exports = baseConfig;
module.exports = {
...baseConfig,
coverageThreshold: {
global: {
branches: 60,
},
},
};
1 change: 0 additions & 1 deletion packages/@aws-cdk/lambda-layer-awscli/layer/.dockerignore

This file was deleted.

54 changes: 0 additions & 54 deletions packages/@aws-cdk/lambda-layer-awscli/layer/Dockerfile

This file was deleted.

18 changes: 0 additions & 18 deletions packages/@aws-cdk/lambda-layer-awscli/layer/build.sh

This file was deleted.

21 changes: 21 additions & 0 deletions packages/@aws-cdk/lambda-layer-awscli/layer/download-fallback.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#!/bin/bash
madeline-k marked this conversation as resolved.
Show resolved Hide resolved

# Get the version to use from the package.json devDependencies
lineWithPackageVersion=$(grep '@aws-cdk/asset-awscli-v1' ./package.json)
version=$(echo $lineWithPackageVersion | cut -d '"' -f 4)

echo "Downloading @aws-cdk/asset-awscli-v1@$version from npm"
npm pack @aws-cdk/asset-awscli-v1@$version -q
madeline-k marked this conversation as resolved.
Show resolved Hide resolved

echo "Extracting layer.zip and requirements.txt from aws-cdk-asset-awscli-v1-$version.tgz"
tar -zxvf aws-cdk-asset-awscli-v1-$version.tgz package/lib/layer.zip package/layer/requirements.txt

echo "Moving layer.zip and requirements.txt to desired destinations"
mv ./package/lib/layer.zip ./lib/
mv ./package/layer/requirements.txt ./layer

echo "Cleaning up"
rm aws-cdk-asset-awscli-v1-$version.tgz
rm -r ./package

echo "download-fallback.sh complete"
Original file line number Diff line number Diff line change
@@ -1 +1 @@
awscli==1.25.70
awscli==1.25.46
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This diff line will go away once this PR is released cdklabs/awscdk-asset-awscli#35

139 changes: 135 additions & 4 deletions packages/@aws-cdk/lambda-layer-awscli/lib/awscli-layer.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,150 @@
/* eslint-disable no-console */
import * as childproc from 'child_process';
import * as fs from 'fs';
import * as path from 'path';
import * as lambda from '@aws-cdk/aws-lambda';
import { FileSystem } from '@aws-cdk/core';
import { Annotations, FileSystem } from '@aws-cdk/core';
import { debugModeEnabled } from '@aws-cdk/core/lib/debug';
import * as cxapi from '@aws-cdk/cx-api';
import { Construct } from 'constructs';

/**
* An AWS Lambda layer that includes the AWS CLI.
*/
export class AwsCliLayer extends lambda.LayerVersion {
/**
* @internal
*/
public static _tryLoadPackage(targetVersion: string, logs: string[]): any {
madeline-k marked this conversation as resolved.
Show resolved Hide resolved
let availableVersion;
let assetPackagePath;
try {
assetPackagePath = require.resolve(`${AwsCliLayer.assetPackageName}`);
} catch (err) {
logs.push(`require.resolve('${AwsCliLayer.assetPackageName}') failed`);
madeline-k marked this conversation as resolved.
Show resolved Hide resolved
return;
}
availableVersion = AwsCliLayer.requireWrapper(path.join(assetPackagePath, '../../package.json'), logs).version;

if (targetVersion === availableVersion) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we care exactly? Do we need semver in here to check range compatibility?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs to log if the version is there but we rejected it btw. That's good info to know.

logs.push(`${AwsCliLayer.assetPackageName} already installed with correct version: ${availableVersion}.`);
const result = AwsCliLayer.requireWrapper(AwsCliLayer.assetPackageName, logs);
if (result) {
logs.push(`Successfully loaded ${AwsCliLayer.assetPackageName} from pre-installed packages.`);
return result;
}
}
}

/**
* @internal
*/
public static _downloadPackage(targetVersion: string, logs: string[]): string | undefined {
const cdkHomeDir = cxapi.cdkHomeDir();
const downloadDir = path.join(cdkHomeDir, 'npm-cache');
const downloadPath = path.join(downloadDir, `${AwsCliLayer.assetPackageNpmTarPrefix}${targetVersion}.tgz`);

if (fs.existsSync(downloadPath)) {
logs.push(`Using package archive already available at location: ${downloadPath}`);
return downloadPath;
}
logs.push(`Downloading package using npm pack to: ${downloadDir}`);
childproc.execSync(`mkdir -p ${downloadDir}; cd ${downloadDir}; npm pack ${AwsCliLayer.assetPackageName}@${targetVersion} -q`);
madeline-k marked this conversation as resolved.
Show resolved Hide resolved
if (fs.existsSync(downloadPath)) {
logs.push('Successfully downloaded using npm pack.');
return downloadPath;
}

logs.push('Failed to download using npm pack.');
return undefined;
}

private static readonly assetPackageName: string = '@aws-cdk/asset-awscli-v1';
private static readonly assetPackageNpmTarPrefix: string = 'aws-cdk-asset-awscli-v1-';

private static requireWrapper(id: string, logs: string[]): any {
try {
// eslint-disable-next-line @typescript-eslint/no-require-imports
const result = require(id);
if (result) {
logs.push(`require('${id}') succeeded.`);
}
return result;
} catch (e) {
logs.push(`require('${id}') failed.`);
const eAsError = e as Error;
if (eAsError?.stack) {
logs.push(eAsError.stack);
}
}
}

private static installAndLoadPackage(from: string, logs: string[]): any {
const installDir = AwsCliLayer.findInstallDir();
if (!installDir) {
logs.push('Unable to find an install directory. require.main.paths[0] is undefined.');
return;
}
logs.push(`Installing from: ${from} to: ${installDir}`);
childproc.execSync(`npm install ${from} --no-save --prefix ${installDir} -q`);
return AwsCliLayer.requireWrapper(path.join(installDir, 'node_modules', AwsCliLayer.assetPackageName, 'lib/index.js'), logs);
}

private static findInstallDir(): string | undefined {
if (!require.main?.paths) {
return undefined;
}
return require.main.paths[0];
}

constructor(scope: Construct, id: string) {
super(scope, id, {
code: lambda.Code.fromAsset(path.join(__dirname, 'layer.zip'), {
const logs: string[] = [];
let fallback = false;

const targetVersion = AwsCliLayer.requireWrapper(path.join(__dirname, '../package.json'), logs).devDependencies[AwsCliLayer.assetPackageName];
madeline-k marked this conversation as resolved.
Show resolved Hide resolved

let assetPackage = AwsCliLayer._tryLoadPackage(targetVersion, logs);

if (!assetPackage) {
const downloadPath = AwsCliLayer._downloadPackage(targetVersion, logs);
if (downloadPath) {
assetPackage = AwsCliLayer.installAndLoadPackage(downloadPath, logs);
}
}

let code;
if (assetPackage) {
// ask for feedback here
const asset = new assetPackage.AwsCliAsset(scope, `${id}-asset`);
code = lambda.Code.fromBucket(asset.bucket, asset.s3ObjectKey);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After successfully loading the package, is there a good way to verify that AwsCliAsset is actually present as an API on the package? And that it is of type s3_assets.Asset? This is where my TS/JS skills are a little bit lacking. I'm not sure the right way to "type-check" here. Or is it overkill?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It all happens at runtime in JavaScript, which doesn't have types. So: we can check for the presence of the symbol, but never its type.

if (!(assetPackage as any).AwsCliAsset) {
  throw new Error('Ruh roh');
}

This is probably good enough, though we might need to stick an API version somewhere...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! What do you mean by API version? The version of AwsCliAsset being used will be locked in by the pinned devDependency.


if (!code) {
fallback = true;
logs.push(`Unable to load ${AwsCliLayer.assetPackageName}. Falling back to use layer.zip bundled with aws-cdk-lib`);
code = lambda.Code.fromAsset(path.join(__dirname, 'layer.zip'), {
// we hash the layer directory (it contains the tools versions and Dockerfile) because hashing the zip is non-deterministic
assetHash: FileSystem.fingerprint(path.join(__dirname, '../layer')),
}),
});
}

super(scope, id, {
code: code,
description: '/opt/awscli/aws',
});

if (debugModeEnabled()) {
Annotations.of(this).addInfo(logs.join('\n'));
}

if (fallback) {
Annotations.of(this).addWarning(`WARNING! ACTION REQUIRED! Your CDK application is using ${this.constructor.name} and this construct may experience a breaking change for your environment in a future release. See https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.lambda_layer_awscli-readme.html or https://github.com/aws/aws-cdk/issues/999999999 for details and resolution instructions.`);
madeline-k marked this conversation as resolved.
Show resolved Hide resolved
new Notice(this, 'cli-notice');
}
}
}

/**
* An empty construct that can be added to the tree as a marker for the CLI Notices
*/
class Notice extends Construct {}
12 changes: 11 additions & 1 deletion packages/@aws-cdk/lambda-layer-awscli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,23 @@
"@aws-cdk/integ-runner": "0.0.0",
"@aws-cdk/integ-tests": "0.0.0",
"@aws-cdk/pkglint": "0.0.0",
"@aws-cdk/asset-awscli-v1": "0.0.13",
"@types/jest": "^27.5.2",
"jest": "^27.5.1"
},
"dependencies": {
"@aws-cdk/aws-lambda": "0.0.0",
"@aws-cdk/aws-s3-assets": "0.0.0",
"@aws-cdk/core": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"constructs": "^10.0.0"
},
"homepage": "https://github.com/aws/aws-cdk",
"peerDependencies": {
"@aws-cdk/aws-lambda": "0.0.0",
"@aws-cdk/aws-s3-assets": "0.0.0",
"@aws-cdk/core": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"constructs": "^10.0.0"
},
"engines": {
Expand All @@ -102,7 +107,7 @@
},
"cdk-build": {
"pre": [
"layer/build.sh"
"layer/download-fallback.sh"
],
"env": {
"AWSLINT_BASE_CONSTRUCT": true
Expand All @@ -120,5 +125,10 @@
"publishConfig": {
"tag": "latest"
},
"awslint": {
"exclude": [
"props-physical-name:@aws-cdk/lambda-layer-awscli.AwsCliLayerProps"
]
},
"private": true
}
Loading