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(s3-deployment): prune - keep missing files on destination bucket #8263

Merged
merged 18 commits into from
Jul 5, 2020
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
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
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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this because apparently the default behavior was changed in this PR, but the README was not updated.

`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 On Deploy
iliapolo marked this conversation as resolved.
Show resolved Hide resolved

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 `pruneOnDeploy: false` to disable
iliapolo marked this conversation as resolved.
Show resolved Hide resolved
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,
pruneOnDeploy: false,
});
```

This option also enables you to specify multiple bucket deployments for the same destination bucket,
iliapolo marked this conversation as resolved.
Show resolved Hide resolved
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')],
pruneOnDeploy: 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')],
pruneOnDeploy: 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
10 changes: 10 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,14 @@ 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).
*
iliapolo marked this conversation as resolved.
Show resolved Hide resolved
* @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 +205,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