Skip to content

Commit

Permalink
fix(s3-deployment): source markers missing when there are multiple so…
Browse files Browse the repository at this point in the history
…urces (#23364)

Follow up to #23321. There is an interesting edge case where if there is _any_ source that has source markers then _all_ sources have to have source markers. This is due to the way the custom resource logic works

https://github.com/aws/aws-cdk/blob/02d0876bbb196e9fbeb32d977e7cf65229c8559d/packages/%40aws-cdk/aws-s3-deployment/lib/lambda/index.py#L64

https://github.com/aws/aws-cdk/blob/02d0876bbb196e9fbeb32d977e7cf65229c8559d/packages/%40aws-cdk/aws-s3-deployment/lib/lambda/index.py#L137


----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
corymhall committed Dec 16, 2022
1 parent 02d0876 commit 8a7ec37
Show file tree
Hide file tree
Showing 21 changed files with 1,010 additions and 416 deletions.
4 changes: 4 additions & 0 deletions packages/@aws-cdk/aws-s3-deployment/lib/bucket-deployment.ts
Expand Up @@ -369,6 +369,10 @@ export class BucketDeployment extends Construct {
return this.sources.reduce((acc, source) => {
if (source.markers) {
acc.push(source.markers);
// if there are more than 1 source, then all sources
// require markers (custom resource will throw an error otherwise)
} else if (this.sources.length > 1) {
acc.push({});
}
return acc;
}, [] as Array<Record<string, any>>);
Expand Down
20 changes: 20 additions & 0 deletions packages/@aws-cdk/aws-s3-deployment/test/bucket-deployment.test.ts
Expand Up @@ -1384,6 +1384,26 @@ test('can add sources with addSource', () => {
});
});

test('if any source has markers then all sources have markers', () => {
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Test');
const bucket = new s3.Bucket(stack, 'Bucket');
const deployment = new s3deploy.BucketDeployment(stack, 'Deploy', {
sources: [s3deploy.Source.data('my/path.txt', 'helloWorld')],
destinationBucket: bucket,
});
deployment.addSource(s3deploy.Source.asset(path.join(__dirname, 'my-website')));

const result = app.synth();
const content = readDataFile(result, 'my/path.txt');
expect(content).toStrictEqual('helloWorld');
Template.fromStack(stack).hasResourceProperties('Custom::CDKBucketDeployment', {
SourceMarkers: [
{},
{},
],
});
});

function readDataFile(casm: cxapi.CloudAssembly, relativePath: string): string {
const assetDirs = readdirSync(casm.directory).filter(f => f.startsWith('asset.'));
Expand Down
Expand Up @@ -236,7 +236,6 @@ def aws_command(*args):
def cfn_send(event, context, responseStatus, responseData={}, physicalResourceId=None, noEcho=False, reason=None):

responseUrl = event['ResponseURL']
logger.info(responseUrl)

responseBody = {}
responseBody['Status'] = responseStatus
Expand Down Expand Up @@ -306,4 +305,4 @@ def replace_markers(filename, markers):

# # delete the original file and rename the new one to the original
os.remove(filename)
os.rename(outfile, filename)
os.rename(outfile, filename)

Large diffs are not rendered by default.

Binary file not shown.

This file was deleted.

This file was deleted.

This file was deleted.

@@ -0,0 +1 @@
helloworld
@@ -0,0 +1,2 @@
<h1>Hello, S3 bucket deployments!</h1>
<img src="rabir2v.gif">

0 comments on commit 8a7ec37

Please sign in to comment.