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 36 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
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@
"@aws-cdk/core/minimatch/**",
"@aws-cdk/cx-api/semver",
"@aws-cdk/cx-api/semver/**",
"@aws-cdk/lambda-layer-awscli/semver",
"@aws-cdk/lambda-layer-awscli/semver/**",
"@aws-cdk/pipelines/aws-sdk",
"@aws-cdk/pipelines/aws-sdk/**",
"@aws-cdk/yaml-cfn/yaml",
Expand Down
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';
17 changes: 17 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,20 @@ fn.addLayers(new AwsCliLayer(this, 'AwsCliLayer'));
```

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

## Troubleshooting

### WARNING! [ACTION REQUIRED] Your CDK application is using ${this.constructor.name}. Add a dependency on ${AwsCliLayer.assetPackageName}, or the equivalent in your language, to remove this warning

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, or the equivalent in your language. If you experience
any issues, please reach out to us by commenting on
https://github.com/aws/aws-cdk/issues/22470.

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.

16 changes: 16 additions & 0 deletions packages/@aws-cdk/lambda-layer-awscli/layer/pre-build.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!/bin/bash

# Copy files from the @aws-cdk/asset-awscli-v1 devDependency into
# this package, so that they will be included in the released package.

# Set bash to exit the script immediately on any error (e) and if any unset (u)
# variable is referenced.
set -eu

dir=$(node -pe 'path.resolve(require.resolve("@aws-cdk/asset-awscli-v1"), "../..")')

cp $dir/layer/requirements.txt ./layer
cp $dir/lib/layer.zip ./lib

target_version=$(node -pe 'require("@aws-cdk/asset-awscli-v1/package.json").version')
echo "export const TARGET_VERSION = '${target_version}';" > lib/asset-package-version.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
echo "export const TARGET_VERSION = '${target_version}';" > lib/asset-package-version.ts
echo "export const RECOMMENDED_VERSION = '${target_version}';" > lib/asset-package-version.ts

?

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

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const TARGET_VERSION = '2.0.0';
60 changes: 56 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,71 @@
/* eslint-disable no-console */
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 { Construct } from 'constructs';
import { TARGET_VERSION } from './asset-package-version';
import { installAndLoadPackage, _downloadPackage, _tryLoadPackage } from './private/package-loading-functions';

/**
* An AWS Lambda layer that includes the AWS CLI.
*/
export class AwsCliLayer extends lambda.LayerVersion {

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

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

let assetPackage = _tryLoadPackage(AwsCliLayer.assetPackageName, TARGET_VERSION, logs);

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

let code;
if (assetPackage) {
// ask for feedback here
if (!assetPackage.AwsCliAsset) {
logs.push(`ERROR: loaded ${AwsCliLayer.assetPackageName}, but AwsCliAsset is undefined in the module.`);
} else {
const asset = new assetPackage.AwsCliAsset(scope, `${id}-asset`);
code = lambda.Code.fromBucket(asset.bucket, asset.s3ObjectKey);
}
}

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',
});
console.log(logs.join('\n'));

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

if (fallback) {
Annotations.of(this).addWarning(`[ACTION REQUIRED] Your CDK application is using ${this.constructor.name}. Add a dependency on ${AwsCliLayer.assetPackageName}, or the equivalent in your language, to remove this warning. See https://github.com/aws/aws-cdk/issues/22470 for more information.`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Annotations.of(this).addWarning(`[ACTION REQUIRED] Your CDK application is using ${this.constructor.name}. Add a dependency on ${AwsCliLayer.assetPackageName}, or the equivalent in your language, to remove this warning. See https://github.com/aws/aws-cdk/issues/22470 for more information.`);
Annotations.of(this).addWarning(`[ACTION REQUIRED] Your CDK application is using ${this.constructor.name} and synthesizes in an environment where ${AwsCliLayer.assetPackageName} cannot be automatically downloaded. Add a dependency on ${AwsCliLayer.assetPackageName}, or the equivalent in your language to remove this warning. See https://github.com/aws/aws-cdk/issues/22470 for more information.`);

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 {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import * as childproc from 'child_process';
import * as fs from 'fs';
import * as path from 'path';
import * as cxapi from '@aws-cdk/cx-api';
import * as semver from 'semver';

export function _tryLoadPackage(packageName: string, targetVersion: string, logs: string[]): any {
let availableVersion;
let assetPackagePath;
try {
assetPackagePath = require.resolve(`${packageName}`);
} catch (e) {
logs.push(`require.resolve('${packageName}') failed`);
const eAsError = e as Error;
if (eAsError?.stack) {
logs.push(eAsError.stack);
}
return;
}
availableVersion = requireWrapper(path.join(assetPackagePath, '../../package.json'), logs).version;

if (semver.satisfies(availableVersion, targetVersion)) {
logs.push(`${packageName} already installed with correct version: ${availableVersion}.`);
const result = requireWrapper(packageName, logs);
if (result) {
logs.push(`Successfully loaded ${packageName} from pre-installed packages.`);
return result;
}
} else {
logs.push(`${packageName} already installed with incorrect version: ${availableVersion}. Target version was: ${targetVersion}.`);
}
}

export function _downloadPackage(packageName: string, packageNpmTarPrefix: string, targetVersion: string, logs: string[]): string | undefined {
const cdkHomeDir = cxapi.cdkHomeDir();
const downloadDir = path.join(cdkHomeDir, 'npm-cache');
const downloadPath = path.join(downloadDir, `${packageNpmTarPrefix}${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}`);
fs.mkdirSync(downloadDir);
childproc.execSync(`npm pack ${packageName}@${targetVersion} -q`, {
cwd: downloadDir,
});
if (fs.existsSync(downloadPath)) {
logs.push('Successfully downloaded using npm pack.');
return downloadPath;
}

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

export function 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);
}
}
}

export function installAndLoadPackage(packageName: string, from: string, logs: string[]): any {
const installDir = 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 requireWrapper(path.join(installDir, 'node_modules', packageName, 'lib/index.js'), logs);
}

export function findInstallDir(): string | undefined {
if (!require.main?.paths) {
return undefined;
}
return require.main.paths[0];
}
16 changes: 14 additions & 2 deletions packages/@aws-cdk/lambda-layer-awscli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,25 +71,32 @@
"organization": true
},
"license": "Apache-2.0",
"bundledDependencies": [
"semver"
Comment on lines +74 to +75
Copy link
Contributor Author

@madeline-k madeline-k Oct 12, 2022

Choose a reason for hiding this comment

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

I'm not sure I did this correctly... Adding the non-JSII package as a dependency forced me to add it here as well. And then having a bundled dependency led me to add it to the "nohoist" list in our top-level package.json using yarn build --fix. It's strange because semver is already a dependency of aws-cdk-lib, so this will get ignored in the release anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

Potentially, we could add a utility function in @aws-cdk/core that does the version range check and uses the semver bundled there. Just available to TS/JS, nothing else would be good enough to start. Something like:

export function versionMatchesRange(version: string, range: string) {
  // Use semver here
}

],
"devDependencies": {
"@aws-cdk/assertions": "0.0.0",
"@aws-cdk/custom-resources": "0.0.0",
"@aws-cdk/cdk-build-tools": "0.0.0",
"@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": "2.0.0",
"@types/jest": "^27.5.2",
"jest": "^27.5.1"
},
"dependencies": {
"@aws-cdk/aws-lambda": "0.0.0",
"@aws-cdk/core": "0.0.0",
"constructs": "^10.0.0"
"@aws-cdk/cx-api": "0.0.0",
"constructs": "^10.0.0",
"semver": "^7.3.8"
},
"homepage": "https://github.com/aws/aws-cdk",
"peerDependencies": {
"@aws-cdk/aws-lambda": "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 +109,7 @@
},
"cdk-build": {
"pre": [
"layer/build.sh"
"layer/pre-build.sh"
],
"env": {
"AWSLINT_BASE_CONSTRUCT": true
Expand All @@ -120,5 +127,10 @@
"publishConfig": {
"tag": "latest"
},
"awslint": {
"exclude": [
"props-physical-name:@aws-cdk/lambda-layer-awscli.AwsCliLayerProps"
]
},
"private": true
}
Loading