Skip to content

Commit

Permalink
feat(s3-deployment): prune - keep missing files on destination bucket (
Browse files Browse the repository at this point in the history
…#8263)

Introduce a `prune` property that eventually controls the `--delete` flag when invoking the `aws s3 sync` command.

Resolves #953 

In addition, migrate the module from `nodeunit` to `jest`.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
iliapolo authored Jul 5, 2020
1 parent ea0552a commit 57914c7
Show file tree
Hide file tree
Showing 18 changed files with 1,165 additions and 802 deletions.
11 changes: 11 additions & 0 deletions packages/@aws-cdk/assert/jest.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as core from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import { countResources } from './lib';
import { JestFriendlyAssertion } from './lib/assertion';
import { haveOutput, HaveOutputProperties } from './lib/assertions/have-output';
import { HaveResourceAssertion, ResourcePart } from './lib/assertions/have-resource';
Expand All @@ -25,6 +26,8 @@ declare global {
comparison?: ResourcePart): R;

toHaveOutput(props: HaveOutputProperties): R;

toCountResources(resourceType: string, count: number): R;
}
}
}
Expand Down Expand Up @@ -77,6 +80,14 @@ expect.extend({

return applyAssertion(haveOutput(props), actual);
},

toCountResources(
actual: cxapi.CloudFormationStackArtifact | core.Stack,
resourceType: string,
count = 1) {

return applyAssertion(countResources(resourceType, count), actual);
},
});

function applyAssertion(assertion: JestFriendlyAssertion<StackInspector>, actual: cxapi.CloudFormationStackArtifact | core.Stack) {
Expand Down
10 changes: 7 additions & 3 deletions packages/@aws-cdk/assert/lib/assertions/count-resources.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { Assertion } from '../assertion';
import { Assertion, JestFriendlyAssertion } from '../assertion';
import { StackInspector } from '../inspector';
import { isSuperObject } from './have-resource';

/**
* An assertion to check whether a resource of a given type and with the given properties exists, disregarding properties
*/
export function countResources(resourceType: string, count = 1): Assertion<StackInspector> {
export function countResources(resourceType: string, count = 1): JestFriendlyAssertion<StackInspector> {
return new CountResourcesAssertion(resourceType, count);
}

Expand All @@ -16,7 +16,7 @@ export function countResourcesLike(resourceType: string, count = 1, props: any):
return new CountResourcesAssertion(resourceType, count, props);
}

class CountResourcesAssertion extends Assertion<StackInspector> {
class CountResourcesAssertion extends JestFriendlyAssertion<StackInspector> {
private inspected: number = 0;
private readonly props: any;

Expand Down Expand Up @@ -48,6 +48,10 @@ class CountResourcesAssertion extends Assertion<StackInspector> {
return counted === this.count;
}

public generateErrorMessage(): string {
return this.description;
}

public get description(): string {
return `stack only has ${this.inspected} resource of type ${this.resourceType}${this.props ? ' with specified properties' : ''} but we expected to find ${this.count}`;
}
Expand Down
2 changes: 2 additions & 0 deletions packages/@aws-cdk/aws-s3-deployment/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,5 @@ nyc.config.js

*.snk
!.eslintrc.js

!jest.config.js
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-s3-deployment/.npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ lambda/*.sh
tsconfig.json
.eslintrc.js

jest.config.js
# exclude cdk artifacts
**/cdk.out
**/cdk.out
42 changes: 38 additions & 4 deletions packages/@aws-cdk/aws-s3-deployment/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ This is what happens under the hood:
is set to point to the assets bucket.
3. The custom resource downloads the .zip archive, extracts it and issues `aws
s3 sync --delete` against the destination bucket (in this case
`websiteBucket`). If there is more than one source, the sources will be
`websiteBucket`). If there is more than one source, the sources will be
downloaded and merged pre-deployment at this step.

## Supported sources
Expand All @@ -59,10 +59,44 @@ all but a single file:

## Retain on Delete

By default, the contents of the destination bucket will be deleted when the
By default, the contents of the destination bucket will **not** be deleted when the
`BucketDeployment` resource is removed from the stack or when the destination is
changed. You can use the option `retainOnDelete: true` to disable this behavior,
in which case the contents will be retained.
changed. You can use the option `retainOnDelete: false` to disable this behavior,
in which case the contents will be deleted.

## Prune

By default, files in the destination bucket that don't exist in the source will be deleted
when the `BucketDeployment` resource is created or updated. You can use the option `prune: false` to disable
this behavior, in which case the files will not be deleted.

```typescript
new s3deploy.BucketDeployment(this, 'DeployMeWithoutDeletingFilesOnDestination', {
sources: [s3deploy.Source.asset(path.join(__dirname, 'my-website'))],
destinationBucket,
prune: false,
});
```

This option also enables you to specify multiple bucket deployments for the same destination bucket & prefix,
each with its own characteristics. For example, you can set different cache-control headers
based on file extensions:

```typescript
new BucketDeployment(this, 'BucketDeployment', {
sources: [Source.asset('./website', { exclude: ['index.html' })],
destinationBucket: bucket,
cacheControl: [CacheControl.fromString('max-age=31536000,public,immutable')],
prune: false,
});

new BucketDeployment(this, 'HTMLBucketDeployment', {
sources: [Source.asset('./website', { exclude: ['!index.html'] })],
destinationBucket: bucket,
cacheControl: [CacheControl.fromString('max-age=0,no-cache,no-store,must-revalidate')],
prune: false,
});
```
## Objects metadata
Expand Down
2 changes: 2 additions & 0 deletions packages/@aws-cdk/aws-s3-deployment/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const baseConfig = require('../../../tools/cdk-build-tools/config/jest.config');
module.exports = baseConfig;
16 changes: 13 additions & 3 deletions packages/@aws-cdk/aws-s3-deployment/lambda/src/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ def cfn_error(message=None):
distribution_id = props.get('DistributionId', '')
user_metadata = props.get('UserMetadata', {})
system_metadata = props.get('SystemMetadata', {})
prune = props.get('Prune', 'true').lower() == 'true'

default_distribution_path = dest_bucket_prefix
if not default_distribution_path.endswith("/"):
Expand Down Expand Up @@ -98,7 +99,7 @@ def cfn_error(message=None):
aws_command("s3", "rm", old_s3_dest, "--recursive")

if request_type == "Update" or request_type == "Create":
s3_deploy(s3_source_zips, s3_dest, user_metadata, system_metadata)
s3_deploy(s3_source_zips, s3_dest, user_metadata, system_metadata, prune)

if distribution_id:
cloudfront_invalidate(distribution_id, distribution_paths)
Expand All @@ -112,7 +113,7 @@ def cfn_error(message=None):

#---------------------------------------------------------------------------------------------------
# populate all files from s3_source_zips to a destination bucket
def s3_deploy(s3_source_zips, s3_dest, user_metadata, system_metadata):
def s3_deploy(s3_source_zips, s3_dest, user_metadata, system_metadata, prune):
# create a temporary working directory
workdir=tempfile.mkdtemp()
logger.info("| workdir: %s" % workdir)
Expand All @@ -131,7 +132,16 @@ def s3_deploy(s3_source_zips, s3_dest, user_metadata, system_metadata):
zip.extractall(contents_dir)

# sync from "contents" to destination
aws_command("s3", "sync", "--delete", contents_dir, s3_dest, *create_metadata_args(user_metadata, system_metadata))

s3_command = ["s3", "sync"]

if prune:
s3_command.append("--delete")

s3_command.extend([contents_dir, s3_dest])
s3_command.extend(create_metadata_args(user_metadata, system_metadata))
aws_command(*s3_command)

shutil.rmtree(workdir)

#---------------------------------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-s3-deployment/lambda/test/aws
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ if sys.argv[2] == "cp" and not sys.argv[4].startswith("s3://"):
sys.argv[4] = "archive.zip"

if sys.argv[2] == "sync":
sys.argv[4] = "contents.zip"
sys.argv[4 if '--delete' in sys.argv else 3] = "contents.zip"

with open("./aws.out", "a") as myfile:
myfile.write(json.dumps(sys.argv[1:]) + "\n")
30 changes: 29 additions & 1 deletion packages/@aws-cdk/aws-s3-deployment/lambda/test/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
class TestHandler(unittest.TestCase):
def setUp(self):
logger = logging.getLogger()

# clean up old aws.out file (from previous runs)
try: os.remove("aws.out")
except OSError: pass
Expand All @@ -37,6 +37,34 @@ def test_create_update(self):
["s3", "sync", "--delete", "contents.zip", "s3://<dest-bucket-name>/"]
)

def test_create_no_delete(self):
invoke_handler("Create", {
"SourceBucketNames": ["<source-bucket>"],
"SourceObjectKeys": ["<source-object-key>"],
"DestinationBucketName": "<dest-bucket-name>",
"Prune": "false"
})

self.assertAwsCommands(
["s3", "cp", "s3://<source-bucket>/<source-object-key>", "archive.zip"],
["s3", "sync", "contents.zip", "s3://<dest-bucket-name>/"]
)

def test_update_no_delete(self):
invoke_handler("Update", {
"SourceBucketNames": ["<source-bucket>"],
"SourceObjectKeys": ["<source-object-key>"],
"DestinationBucketName": "<dest-bucket-name>",
"Prune": "false"
}, old_resource_props={
"DestinationBucketName": "<dest-bucket-name>",
}, physical_id="<physical-id>")

self.assertAwsCommands(
["s3", "cp", "s3://<source-bucket>/<source-object-key>", "archive.zip"],
["s3", "sync", "contents.zip", "s3://<dest-bucket-name>/"]
)

def test_create_update_multiple_sources(self):
invoke_handler("Create", {
"SourceBucketNames": ["<source-bucket1>", "<source-bucket2>"],
Expand Down
12 changes: 12 additions & 0 deletions packages/@aws-cdk/aws-s3-deployment/lib/bucket-deployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ export interface BucketDeploymentProps {
*/
readonly destinationKeyPrefix?: string;

/**
* If this is set to false, files in the destination bucket that
* do not exist in the asset, will NOT be deleted during deployment (create/update).
*
* @see https://docs.aws.amazon.com/cli/latest/reference/s3/sync.html
*
* @default true
*/
readonly prune?: boolean

/**
* If this is set to "false", the destination files will be deleted when the
* resource is deleted or the destination is updated.
Expand Down Expand Up @@ -197,12 +207,14 @@ export class BucketDeployment extends cdk.Construct {
DestinationBucketName: props.destinationBucket.bucketName,
DestinationBucketKeyPrefix: props.destinationKeyPrefix,
RetainOnDelete: props.retainOnDelete,
Prune: props.prune ?? true,
UserMetadata: props.metadata ? mapUserMetadata(props.metadata) : undefined,
SystemMetadata: mapSystemMetadata(props),
DistributionId: props.distribution ? props.distribution.distributionId : undefined,
DistributionPaths: props.distributionPaths,
},
});

}

private renderSingletonUuid(memoryLimit?: number) {
Expand Down
7 changes: 4 additions & 3 deletions packages/@aws-cdk/aws-s3-deployment/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@
],
"test": [
"/bin/bash lambda/test.sh"
]
],
"jest": true
},
"keywords": [
"aws",
Expand All @@ -77,10 +78,10 @@
"license": "Apache-2.0",
"devDependencies": {
"@aws-cdk/assert": "0.0.0",
"@types/nodeunit": "^0.0.31",
"@types/jest": "^25.2.3",
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"nodeunit": "^0.11.3",
"jest": "^25.5.4",
"pkglint": "0.0.0"
},
"dependencies": {
Expand Down
Loading

0 comments on commit 57914c7

Please sign in to comment.