Skip to content

Commit

Permalink
Merge branch 'master' into patch-1
Browse files Browse the repository at this point in the history
  • Loading branch information
mergify[bot] authored Feb 28, 2021
2 parents 0a78e35 + 6eca979 commit 9ab94aa
Show file tree
Hide file tree
Showing 11 changed files with 377 additions and 85 deletions.
18 changes: 9 additions & 9 deletions DESIGN_GUIDELINES.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ APIs and verifies that the APIs adhere to the guidelines. When a guideline is
backed by a linter rule, the rule name will be referenced like this:
_[awslint:resource-class-is-construct]_.

For the purpose of this document we will use "Foo" to denote the official name
For the purpose of this document, we will use "Foo" to denote the official name
of the resource as defined in the AWS CloudFormation resource specification
(i.e. "Bucket", "Queue", "Topic", etc). This notation allows deriving names from
the official name. For example, `FooProps` would be `BucketProps`, `TopicProps`,
Expand Down Expand Up @@ -98,8 +98,8 @@ or abstractions. However, you will notice that some sections explicitly call out
guidelines that apply only to AWS resources (and in many cases
enforced/implemented by the **Resource** base class).

AWS services are modeled around the concept of *resources*. Service normally
expose through their APIs one or more resources, which can be provisioned
AWS services are modeled around the concept of *resources*. Services normally
expose one or more resources through their APIs, which can be provisioned
through the APIs control plane or through AWS CloudFormation.

Every resource available in the AWS platform will have a corresponding resource
Expand Down Expand Up @@ -397,12 +397,12 @@ For example, prefer “readCapacity” versus “readCapacityUnits”.
We prefer the terminology used by the official AWS service documentation over
new terminology, even if you think it's not ideal. It helps users diagnose
issues and map the mental model of the construct to the service APIs,
documentation and examples. For example don't be tempted to change SQS's
documentation and examples. For example, don't be tempted to change SQS's
**dataKeyReusePeriod** with **keyRotation** because it will be hard for people
to diagnose problems. They won't be able to just search for “sqs dataKeyReuse”
and find topics on it.

> We can relax this guidelines when this is about generic terms (like
> We can relax this guideline when this is about generic terms (like
`httpStatus` instead of `statusCode`). The important semantics to preserve are
for *service features*: I wouldn't want to rename "lambda layers" to "lambda
dependencies" just because it makes more sense because then users won't be
Expand Down Expand Up @@ -697,8 +697,8 @@ _[awslint:from-signature]_:
#### “from” Methods
Resource constructs should export static “from” methods for importing unowned
resources given one more of its physical attributes such as ARN, name, etc. All
constructs should have at least one "fromXxx" method _[awslint:from-method]_:
resources given one or more of its physical attributes such as ARN, name, etc. All
constructs should have at least one `fromXxx` method _[awslint:from-method]_:
```ts
static fromFooArn(scope: Construct, id: string, bucketArn: string): IFoo;
Expand Down Expand Up @@ -870,7 +870,7 @@ vpcSubnetSelection?: ec2.SubnetSelection;
### Grants
Grants are one of the most powerful concept in the AWS Construct Library. They
Grants are one of the most powerful concepts in the AWS Construct Library. They
offer a higher level, intent-based, API for managing IAM permissions for AWS
resources.
Expand Down Expand Up @@ -974,7 +974,7 @@ class Function extends Resource implements IFunction {
### Events
Many AWS resource emit events to the CloudWatch event bus. Such resources should
Many AWS resources emit events to the CloudWatch event bus. Such resources should
have a set of “onXxx” methods available on their construct interface
_[awslint:events-in-interface]_.
Expand Down
7 changes: 0 additions & 7 deletions allowed-breaking-changes.txt
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,3 @@ incompatible-argument:@aws-cdk/aws-ecs.TaskDefinition.addVolume
# We made properties optional and it's really fine but our differ doesn't think so.
weakened:@aws-cdk/cloud-assembly-schema.DockerImageSource
weakened:@aws-cdk/cloud-assembly-schema.FileSource

# https://github.com/aws/aws-cdk/pull/13145
removed:@aws-cdk/core.AssetStaging.isArchive
removed:@aws-cdk/core.AssetStaging.packaging
removed:@aws-cdk/core.BundlingOutput
removed:@aws-cdk/core.BundlingOptions.outputType

4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,10 @@ export class Bundling implements cdk.BundlingOptions {

const esbuildCommand: string = [
npx, 'esbuild',
'--bundle', pathJoin(inputDir, this.relativeEntryPath).replace(/(\s+)/g, '\\$1'),
'--bundle', `"${pathJoin(inputDir, this.relativeEntryPath)}"`,
`--target=${this.props.target ?? toTarget(this.props.runtime)}`,
'--platform=node',
`--outfile=${pathJoin(outputDir, 'index.js')}`,
`--outfile="${pathJoin(outputDir, 'index.js')}"`,
...this.props.minify ? ['--minify'] : [],
...this.props.sourceMap ? ['--sourcemap'] : [],
...this.externals.map(external => `--external:${external}`),
Expand Down
32 changes: 6 additions & 26 deletions packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ test('esbuild bundling in Docker', () => {
},
command: [
'bash', '-c',
'npx esbuild --bundle /asset-input/lib/handler.ts --target=node12 --platform=node --outfile=/asset-output/index.js --external:aws-sdk --loader:.png=dataurl',
'npx esbuild --bundle "/asset-input/lib/handler.ts" --target=node12 --platform=node --outfile="/asset-output/index.js" --external:aws-sdk --loader:.png=dataurl',
],
workingDirectory: '/',
}),
Expand All @@ -74,7 +74,7 @@ test('esbuild bundling with handler named index.ts', () => {
bundling: expect.objectContaining({
command: [
'bash', '-c',
'npx esbuild --bundle /asset-input/lib/index.ts --target=node12 --platform=node --outfile=/asset-output/index.js --external:aws-sdk',
'npx esbuild --bundle "/asset-input/lib/index.ts" --target=node12 --platform=node --outfile="/asset-output/index.js" --external:aws-sdk',
],
}),
});
Expand All @@ -94,7 +94,7 @@ test('esbuild bundling with tsx handler', () => {
bundling: expect.objectContaining({
command: [
'bash', '-c',
'npx esbuild --bundle /asset-input/lib/handler.tsx --target=node12 --platform=node --outfile=/asset-output/index.js --external:aws-sdk',
'npx esbuild --bundle "/asset-input/lib/handler.tsx" --target=node12 --platform=node --outfile="/asset-output/index.js" --external:aws-sdk',
],
}),
});
Expand Down Expand Up @@ -139,7 +139,7 @@ test('esbuild bundling with externals and dependencies', () => {
command: [
'bash', '-c',
[
'npx esbuild --bundle /asset-input/test/bundling.test.js --target=node12 --platform=node --outfile=/asset-output/index.js --external:abc --external:delay',
'npx esbuild --bundle "/asset-input/test/bundling.test.js" --target=node12 --platform=node --outfile="/asset-output/index.js" --external:abc --external:delay',
`echo \'{\"dependencies\":{\"delay\":\"${delayVersion}\"}}\' > /asset-output/package.json`,
'cp /asset-input/package-lock.json /asset-output/package-lock.json',
'cd /asset-output',
Expand Down Expand Up @@ -181,8 +181,8 @@ test('esbuild bundling with esbuild options', () => {
command: [
'bash', '-c',
[
'npx esbuild --bundle /asset-input/lib/handler.ts',
'--target=es2020 --platform=node --outfile=/asset-output/index.js',
'npx esbuild --bundle "/asset-input/lib/handler.ts"',
'--target=es2020 --platform=node --outfile="/asset-output/index.js"',
'--minify --sourcemap --external:aws-sdk --loader:.png=dataurl',
'--define:DEBUG=true --define:process.env.KEY="VALUE"',
'--log-level=silent --keep-names --tsconfig=/asset-input/lib/custom-tsconfig.ts',
Expand Down Expand Up @@ -334,23 +334,3 @@ test('with command hooks', () => {
}),
});
});

test('escapes spaces in path', () => {
Bundling.bundle({
entry: '/project/lib/my cool lambda/handler.ts',
depsLockFilePath,
runtime: Runtime.NODEJS_12_X,
forceDockerBundling: true,
});

// Correctly bundles with esbuild
expect(Code.fromAsset).toHaveBeenCalledWith(path.dirname(depsLockFilePath), {
assetHashType: AssetHashType.OUTPUT,
bundling: expect.objectContaining({
command: [
'bash', '-c',
expect.stringContaining('lib/my\\ cool\\ lambda/handler.ts'),
],
}),
});
});
21 changes: 21 additions & 0 deletions packages/@aws-cdk/aws-s3-assets/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,27 @@ new assets.Asset(this, 'BundledAsset', {
Although optional, it's recommended to provide a local bundling method which can
greatly improve performance.

If the bundling output contains a single archive file (zip or jar) it will be
uploaded to S3 as-is and will not be zipped. Otherwise the contents of the
output directory will be zipped and the zip file will be uploaded to S3. This
is the default behavior for `bundling.outputType` (`BundlingOutput.AUTO_DISCOVER`).

Use `BundlingOutput.NOT_ARCHIVED` if the bundling output must always be zipped:

```ts
const asset = new assets.Asset(this, 'BundledAsset', {
path: '/path/to/asset',
bundling: {
image: BundlingDockerImage.fromRegistry('alpine'),
command: ['command-that-produces-an-archive.sh'],
outputType: BundlingOutput.NOT_ARCHIVED, // Bundling output will be zipped even though it produces a single archive file.
},
});
```

Use `BundlingOutput.ARCHIVED` if the bundling output contains a single archive file and
you don't want it to be zippped.

## CloudFormation Resource Metadata

> NOTE: This section is relevant for authors of AWS Resource Constructs.
Expand Down
30 changes: 3 additions & 27 deletions packages/@aws-cdk/aws-s3-assets/lib/asset.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as fs from 'fs';
import * as path from 'path';
import * as assets from '@aws-cdk/assets';
import * as iam from '@aws-cdk/aws-iam';
Expand All @@ -13,8 +12,6 @@ import { toSymlinkFollow } from './compat';
// eslint-disable-next-line no-duplicate-imports, import/order
import { Construct as CoreConstruct } from '@aws-cdk/core';

const ARCHIVE_EXTENSIONS = ['.zip', '.jar'];

export interface AssetOptions extends assets.CopyOptions, cdk.AssetOptions {
/**
* A list of principals that should be able to read this asset from S3.
Expand Down Expand Up @@ -139,17 +136,12 @@ export class Asset extends CoreConstruct implements cdk.IAsset {

this.assetPath = staging.relativeStagedPath(stack);

const packaging = determinePackaging(staging.sourcePath);

this.isFile = packaging === cdk.FileAssetPackaging.FILE;
this.isFile = staging.packaging === cdk.FileAssetPackaging.FILE;

// sets isZipArchive based on the type of packaging and file extension
this.isZipArchive = packaging === cdk.FileAssetPackaging.ZIP_DIRECTORY
? true
: ARCHIVE_EXTENSIONS.some(ext => staging.sourcePath.toLowerCase().endsWith(ext));
this.isZipArchive = staging.isArchive;

const location = stack.synthesizer.addFileAsset({
packaging,
packaging: staging.packaging,
sourceHash: this.sourceHash,
fileName: this.assetPath,
});
Expand Down Expand Up @@ -210,19 +202,3 @@ export class Asset extends CoreConstruct implements cdk.IAsset {
this.bucket.grantRead(grantee);
}
}

function determinePackaging(assetPath: string): cdk.FileAssetPackaging {
if (!fs.existsSync(assetPath)) {
throw new Error(`Cannot find asset at ${assetPath}`);
}

if (fs.statSync(assetPath).isDirectory()) {
return cdk.FileAssetPackaging.ZIP_DIRECTORY;
}

if (fs.statSync(assetPath).isFile()) {
return cdk.FileAssetPackaging.FILE;
}

throw new Error(`Asset ${assetPath} is expected to be either a directory or a regular file`);
}
Loading

0 comments on commit 9ab94aa

Please sign in to comment.