-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
fixed s3 deletion from stack in cdk #11700
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch! And it is indeed because of the late binding here where the actual value of s3_bucket
was only resolved at the time of the lambda function execution, rather than the definition.
@@ -243,9 +243,10 @@ def provision(self, skip_deployment: Optional[bool] = False): | |||
] | |||
|
|||
for s3_bucket in s3_buckets: | |||
bucket_to_delete = s3_bucket # Capture current value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: this line doesn't actually do anything in regards to the root cause, it would still lead to the same issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion. I've removed this line.
lambda bucket=bucket_to_delete: cleanup_s3_bucket( | ||
self.aws_client.s3, bucket, delete_bucket=False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lambda bucket=bucket_to_delete: cleanup_s3_bucket( | |
self.aws_client.s3, bucket, delete_bucket=False | |
lambda s3_bucket=s3_bucket: cleanup_s3_bucket( | |
self.aws_client.s3, s3_bucket, delete_bucket=False |
this alone would be enough to bind the value here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've update the lambda
to use the loop variable.
60d796a
to
1b8f9b7
Compare
Motivation
This PR aims to fix the S3 bucket cleaning when a stack contains more than one buckets (with objects in it). According to my theory due to late binding in the lambda function, the
cleanup_s3_bucket
is not able to get the reference to the actual variable.Changes