Updating a BucketDeployment's memoryLimit causes stack update failure #7128

tedinski · 2020-04-01T21:38:55Z

Changing the memoryLimit of an existing BucketDeployment (@aws-cdk/aws-s3-deployment) causes the CloudFormation deploy of the update to fail with the error "Modifying service token is not allowed."

Reproduction Steps

Create a bucket deployment:

const deployment = new BucketDeployment(this, 'BucketDeployment', {
  destinationBucket: myDestinationBucket,
  sources: [Source.bucket(myBucket, myS3Obj)],
});

Then add memoryLimit: 1792, (for example) to its properties and redeploy. The CFN changeset will fail to deploy and rollback.

Error Log

CloudFormation reports the error "Modifying service token is not allowed." for the "BucketDeploymentCustomResource."

Environment

CLI Version : 1.30
Framework Version: 1.23
OS : Mac/Linux
Language : Typescript

Workaround

For anyone struggling with this issue, here's how I worked around the problem:

Remove your bucket deployment construct.
Deploy to all environments. The contents of the S3 bucket might be stale momentarily, but they aren't deleted or anything.
Add it back with the correct memoryLimit.
Deploy to all environments.

In retrospect, just changing the construct name (add 2 to the end or something) to destroy the old and create the new would probably also have worked. I haven't tested that.

Other

I noticed this was noted on the original pull request by someone else as well: #4204 (comment)

Additional request

I strongly believe the default memoryLimit should be set to 1792, which is the documented number for a full vCPU for a Lambda.

Our experience with even a small 2MB zip file showed we had a 40s deploy at 128 MB and a 3s deploy at 1792 MB. That's 13X faster at 14X resources, a basically linear speedup, which means it's nearly free in terms of cost. I strongly believe there's no reason whatsoever for this lambda to ever have a smaller memory limit, which means this is a very bad default.

Of course, to safely update that default, this bug needs fixing first. Otherwise everyone's deploy will fail.

This is 🐛 Bug Report

The text was updated successfully, but these errors were encountered:

iliapolo · 2020-04-12T09:24:27Z

Hi @tedinski - thanks for reporting this.

Writing down my initial investigation for reference to when we work on the resolution:

What happens is that we use the lambda function arn as the serviceToken of the custom resource. When the memory configuration changes, it is reflected in the function arn so that a new lambda with the new memory config will be created. This in turn causes the custom resource to have a different service token, which is not allowed.

Proposed solution:

Incorporate lambda config in the custom resource id so that a new custom resource will be created, with a new service token.

Khalian · 2020-06-08T17:45:22Z

Whats a proposed workaround for this? I dont want to go blow up my stack, only to go update bucket deployments.

iliapolo · 2020-07-09T19:32:24Z

@Khalian Apologies for the delayed response. You can change the BucketDeployment construct id. This will replace the underlying custom resource and deploy the new one with the updated config. It doesn't require blowing up the stack.

ryanwilliams83 · 2021-04-22T11:27:46Z

I suspect this lambda does not detect when the awscli command terminates due to OOM error.
I have seen this lambda (when configured at the default 128MB) execute for the full 15 minutes even though cloudwatch output abruptly stops at 62 seconds.

The problem mainly occurs when trying to BucketDeploy 161 local files totalling 133MB using the default memoryLimit

masterchop · 2021-05-29T02:08:16Z

i dont understand why uploading files through the CDK is so painful in compare with the SDK. SDK just works and faster.

masterchop · 2021-05-29T02:08:21Z

i dont understand why uploading files through the CDK is so painful in compare with the SDK. SDK just works and faster.

akuma12 · 2021-07-01T22:19:26Z

This just bit us today. We needed to bump the memory limit on the S3Deployment lambda, and when we tried to change it, it created a new resource and deleted the old one. The problem being, it deleted the files it just updated. I'm baffled as to why the memory limit is encoded in the CDK-generated resource ID. Lambda memory changes are no-replacement updates.

ghost · 2021-07-19T15:04:09Z

+1
Same here. Nasty bug as raising the memoryLimit might be useful with an increasing project. So the solution at the moment is not ideal.

sarahkadar · 2021-08-26T08:36:10Z

+1 we just got bitten by this

jaylucas · 2021-08-26T20:02:20Z

+1 same here

ffinfo · 2021-09-10T06:57:14Z

+1 same here

masterchop · 2021-09-10T15:39:18Z

Dudes you have to increase the memoryLimit to fix the problem.

gkaskonas · 2021-11-09T11:02:18Z

+1 same issue

…te failure (#17530) This fixes the issue where updating the memoryLimit or vpc of the BucketDeployment resource would result in a stack update failure. In order to fix this the ID of the BucketDeployment CustomResource includes info on the memoryLimit and vpc in the same way that the Lambda function does. This means that when either of these values are updated, the BucketDeployment CustomResource will be recreated along with the Lambda function. If anyone is setting `retainOnDelete=false` (default is `true`) then this change would result in the data in the bucket being deleted. In order to avoid this scenario, this PR introduces a bucket tag that controls whether or not a BucketDeployment resource can delete data from the bucket. The BucketDeployment resource will now add a tag to the deployment bucket with a format like `aws-cdk:cr-owned:{keyPrefix}:{uniqueHash}`. For example: ``` { Key: 'aws-cdk:cr-owned:deploy/here/:240D17B3', Value: 'true', } ``` Each bucket+keyPrefix can be "owned" by 1 or more BucketDeployment resources. Since there are some scenarios where multiple BucketDeployment resources can deploy to the same bucket and key prefix (e.g. using include/exclude) we also append part of the id to make the key unique. As long as a bucket+keyPrefix is "owned" by a BucketDeployment resource, another CR cannot delete data. There are a couple of scenarios where this comes into play. 1. If the LogicalResourceId of the CustomResource changes (e.g. memoryLimit is updated) CloudFormation will first issue a 'Create' to create the new CustomResource and will update the Tag on the bucket. CloudFormation will then issue a 'Delete' on the old CustomResource and since the new CR "owns" the Bucket+keyPrefix (indicated by the presence of the tag), the old CR will not delete the contents of the bucket 2. If the BucketDeployment resource is deleted _and_ it is the only CR for that bucket+keyPrefix then CloudFormation will first remove the tag from the bucket and then issue a "Delete" to the CR. Since there are no tags indicating that this bucket+keyPrefix is "owned" then it will delete the contents. 3. If the BucketDeployment resource is deleted _and_ it is *not* the only CR for that bucket:keyPrefix then CloudFormation will first remove the tag from the bucket and then issue a "Delete" to the CR. Since there are other CRs that also "own" that bucket+keyPrefix (indicated by the presence of tags), there will still be a tag on the bucket and the contents will not be removed. The contents will only be removed after _all_ BucketDeployment resource that "own" the bucket+keyPrefix have been removed. 4. If the BucketDeployment resource _and_ the S3 Bucket are both removed, then CloudFormation will first issue a "Delete" to the CR and since there is a tag on the bucket the contents will not be removed. If you want the contents of the bucket to be removed on bucket deletion, then `autoDeleteObjects` property should be set to true on the Bucket. Scenario 3 & 4 are changes to the existing functionality in that they now do *not* delete data in some scenarios when they did previously. fixes #7128 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

github-actions · 2021-11-23T14:09:56Z

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

…te failure (aws#17530) This fixes the issue where updating the memoryLimit or vpc of the BucketDeployment resource would result in a stack update failure. In order to fix this the ID of the BucketDeployment CustomResource includes info on the memoryLimit and vpc in the same way that the Lambda function does. This means that when either of these values are updated, the BucketDeployment CustomResource will be recreated along with the Lambda function. If anyone is setting `retainOnDelete=false` (default is `true`) then this change would result in the data in the bucket being deleted. In order to avoid this scenario, this PR introduces a bucket tag that controls whether or not a BucketDeployment resource can delete data from the bucket. The BucketDeployment resource will now add a tag to the deployment bucket with a format like `aws-cdk:cr-owned:{keyPrefix}:{uniqueHash}`. For example: ``` { Key: 'aws-cdk:cr-owned:deploy/here/:240D17B3', Value: 'true', } ``` Each bucket+keyPrefix can be "owned" by 1 or more BucketDeployment resources. Since there are some scenarios where multiple BucketDeployment resources can deploy to the same bucket and key prefix (e.g. using include/exclude) we also append part of the id to make the key unique. As long as a bucket+keyPrefix is "owned" by a BucketDeployment resource, another CR cannot delete data. There are a couple of scenarios where this comes into play. 1. If the LogicalResourceId of the CustomResource changes (e.g. memoryLimit is updated) CloudFormation will first issue a 'Create' to create the new CustomResource and will update the Tag on the bucket. CloudFormation will then issue a 'Delete' on the old CustomResource and since the new CR "owns" the Bucket+keyPrefix (indicated by the presence of the tag), the old CR will not delete the contents of the bucket 2. If the BucketDeployment resource is deleted _and_ it is the only CR for that bucket+keyPrefix then CloudFormation will first remove the tag from the bucket and then issue a "Delete" to the CR. Since there are no tags indicating that this bucket+keyPrefix is "owned" then it will delete the contents. 3. If the BucketDeployment resource is deleted _and_ it is *not* the only CR for that bucket:keyPrefix then CloudFormation will first remove the tag from the bucket and then issue a "Delete" to the CR. Since there are other CRs that also "own" that bucket+keyPrefix (indicated by the presence of tags), there will still be a tag on the bucket and the contents will not be removed. The contents will only be removed after _all_ BucketDeployment resource that "own" the bucket+keyPrefix have been removed. 4. If the BucketDeployment resource _and_ the S3 Bucket are both removed, then CloudFormation will first issue a "Delete" to the CR and since there is a tag on the bucket the contents will not be removed. If you want the contents of the bucket to be removed on bucket deletion, then `autoDeleteObjects` property should be set to true on the Bucket. Scenario 3 & 4 are changes to the existing functionality in that they now do *not* delete data in some scenarios when they did previously. fixes aws#7128 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

tedinski added bug needs-triage labels Apr 1, 2020

SomayaB added the @aws-cdk/aws-s3 label Apr 3, 2020

SomayaB assigned iliapolo Apr 3, 2020

iliapolo added the p1 label Apr 12, 2020

SomayaB removed the needs-triage label May 19, 2020

iliapolo added @aws-cdk/aws-s3-deployment and removed @aws-cdk/aws-s3 labels Jun 14, 2020

iliapolo added the effort/medium label Aug 20, 2020

iliapolo removed their assignment Jun 27, 2021

trietlu mentioned this issue Jul 15, 2021

GTT-1421 : Increase memory size of lambda used by CDK BucketDeployment aws-solutions/performance-dashboard-on-aws#524

Merged

cspalevic mentioned this issue Sep 24, 2021

Unable to create multiple services on serverless cspalevic/budgeter-api#130

Closed

corymhall self-assigned this Nov 11, 2021

corymhall mentioned this issue Nov 16, 2021

fix(s3-deployment): updating memoryLimit or vpc results in stack update failure #17530

Merged

mergify bot closed this as completed in #17530 Nov 23, 2021

haslam22 mentioned this issue Oct 24, 2022

feat(aws-s3): allow customising role used for autoDeleteObjects #21812

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updating a BucketDeployment's memoryLimit causes stack update failure #7128

Updating a BucketDeployment's memoryLimit causes stack update failure #7128

tedinski commented Apr 1, 2020

iliapolo commented Apr 12, 2020

Khalian commented Jun 8, 2020

iliapolo commented Jul 9, 2020 •

edited

Loading

ryanwilliams83 commented Apr 22, 2021

masterchop commented May 29, 2021

masterchop commented May 29, 2021

akuma12 commented Jul 1, 2021

ghost commented Jul 19, 2021

sarahkadar commented Aug 26, 2021

jaylucas commented Aug 26, 2021

ffinfo commented Sep 10, 2021

masterchop commented Sep 10, 2021

gkaskonas commented Nov 9, 2021

github-actions bot commented Nov 23, 2021

Updating a BucketDeployment's memoryLimit causes stack update failure #7128

Updating a BucketDeployment's memoryLimit causes stack update failure #7128

Comments

tedinski commented Apr 1, 2020

Reproduction Steps

Error Log

Environment

Workaround

Other

Additional request

iliapolo commented Apr 12, 2020

Khalian commented Jun 8, 2020

iliapolo commented Jul 9, 2020 • edited Loading

ryanwilliams83 commented Apr 22, 2021

masterchop commented May 29, 2021

masterchop commented May 29, 2021

akuma12 commented Jul 1, 2021

ghost commented Jul 19, 2021

sarahkadar commented Aug 26, 2021

jaylucas commented Aug 26, 2021

ffinfo commented Sep 10, 2021

masterchop commented Sep 10, 2021

gkaskonas commented Nov 9, 2021

github-actions bot commented Nov 23, 2021

⚠️COMMENT VISIBILITY WARNING⚠️

iliapolo commented Jul 9, 2020 •

edited

Loading