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

Fix bucket location patch for the TemplateURL when deploying a stack #101

Merged
merged 8 commits into from
Dec 20, 2024

Conversation

RobertLucian
Copy link
Contributor

@RobertLucian RobertLucian commented Dec 17, 2024

Description

When S3 is used as a place of store for the CF artifacts, the resolving of the actual bucketUrl within our patch would lead to the presence of a stringified version of a promise, i.e.: '[object Promise]/cdk/digitalu-local-services/dbdbe3071265081abdd49572f43ec8168bc6fb4250c190631f9e84f51a5ccb4c.yml'. Because of this, what was supposed to be the first prefix within the bucket is now considered to be the bucket. Chaos ensues, and the stack fails to deploy because the S3 bucket "does not exist".

So instead of using promises for our patched version of bucketUrl attribute, we instead determine the bucket name/URL outside of the function associated with it.

While doing this, it was also noticed how the ToolkitInfo object isn't always a ToolkitInfo object anymore:

  1. Can be {} (empty key-value pair object).
  2. Can be BootstrapStackNotFoundInfo in which case calling bucketUrl or bucketName would throw an exception.
  3. Can be ExistingToolkitInfo, which is the ideal case, and also the case when we should patch bucketUrl attribute.

These classes can be observed here: https://github.com/aws/aws-cdk/blob/87e21d625af86873716734dd5568940d41096c45/packages/aws-cdk/lib/api/toolkit-info.ts

@RobertLucian RobertLucian added the bug Something isn't working label Dec 17, 2024
@RobertLucian RobertLucian self-assigned this Dec 17, 2024
@RobertLucian RobertLucian changed the title Fix TemplateURL containing stringified version of a promise Fix patching of getUrl public method of ToolkitInfo base class Dec 18, 2024
@RobertLucian RobertLucian marked this pull request as ready for review December 18, 2024 04:41
Copy link
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

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

Thanks for jumping on this issue.

I'm still not 100% sure for which cases this code path is triggered, as I have not been able to trigger it with a 0815 cdk init ... project & manipulating the stack to be at a size that forces the usage of a TemplateURL parameter when deploying. 🤔

bin/cdklocal Outdated Show resolved Hide resolved
bin/cdklocal Outdated Show resolved Hide resolved
bin/cdklocal Outdated Show resolved Hide resolved
bin/cdklocal Outdated Show resolved Hide resolved
package-lock.json Outdated Show resolved Hide resolved
Copy link
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

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

Thanks for jumping on this. I'll have a few more comments that we can iterate over after getting he immediate fix over the line. 🚀

@dominikschubert dominikschubert changed the title Fix patching of getUrl public method of ToolkitInfo base class Fix bucket location patch for the TemplateURL when deploying a stack Dec 20, 2024
@dominikschubert dominikschubert merged commit 38df226 into main Dec 20, 2024
12 checks passed
@RobertLucian RobertLucian deleted the fix/template-url-promise branch December 20, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants