Skip to content

Commit

Permalink
fix(s3-deployment): fails when destinationKeyPrefix is a token with…
Browse files Browse the repository at this point in the history
… a long string representation (#22163)

We have a limitation on the length of `destinationKeyPrefix` to be no more than 104 characters. This is because it is used to tag the destination bucket, and tag keys cannot be longer than 128 characters (we prefix the prefix).

However, this validation should not apply when `destinationKeyPrefix` is a token, because in that case it's simply validating the length of the string representation of a token, which doesn't make sense.

----

### All Submissions:

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

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-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
iliapolo authored Sep 21, 2022
1 parent 9f0264c commit ce59b6a
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-s3-deployment/lib/bucket-deployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -372,8 +372,8 @@ export class BucketDeployment extends Construct {
// the tag key limit of 128
// '/this/is/a/random/key/prefix/that/is/a/lot/of/characters/do/we/think/that/it/will/ever/be/this/long?????'
// better to throw an error here than wait for CloudFormation to fail
if (tagKey.length > 128) {
throw new Error('The BucketDeployment construct requires that the "destinationKeyPrefix" be <=104 characters');
if (!cdk.Token.isUnresolved(tagKey) && tagKey.length > 128) {
throw new Error('The BucketDeployment construct requires that the "destinationKeyPrefix" be <=104 characters.');
}

/*
Expand Down
22 changes: 22 additions & 0 deletions packages/@aws-cdk/aws-s3-deployment/test/bucket-deployment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,28 @@ test('throws if destinationKeyPrefix is too long', () => {

});

test('skips destinationKeyPrefix validation if token', () => {

// GIVEN
const stack = new cdk.Stack();
const bucket = new s3.Bucket(stack, 'Dest');

// WHEN
// trick the cdk into creating a very long token
const prefixToken = cdk.Token.asString(5, { displayHint: 'longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong' });
new s3deploy.BucketDeployment(stack, 'DeployWithVpc2', {
sources: [s3deploy.Source.asset(path.join(__dirname, 'my-website'))],
destinationBucket: bucket,
destinationKeyPrefix: prefixToken,
memoryLimit: 256,
});

Template.fromStack(stack).hasResourceProperties('Custom::CDKBucketDeployment', {
DestinationBucketKeyPrefix: 5,
});

});

test('bucket has multiple deployments', () => {

// GIVEN
Expand Down

0 comments on commit ce59b6a

Please sign in to comment.