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

Async / Promises in Constructs #2516

Closed
evanstachowiak opened this issue Jan 18, 2023 · 10 comments
Closed

Async / Promises in Constructs #2516

evanstachowiak opened this issue Jan 18, 2023 · 10 comments
Labels
enhancement New feature or request stale An issue or pull request that has not been updated in a very long time waiting-on-answer

Comments

@evanstachowiak
Copy link

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

I'd like to re-open this issue: #95.

I would say there are two very important issues with the lack of async support.

  1. The AWS SDK is async. If the Construct needs to query AWS in anyway, everything must be queried and passed down through the constructor or through a setter function.
  2. All ESM modules require an async function to import. This leads to the same problem as above, if any ESM modules are needed they cannot be declared directly in the Construct.

Many thanks for all your great work on this project. Looking forward to seeing async support in CDKTF.

References

@evanstachowiak evanstachowiak added enhancement New feature or request new Un-triaged issue labels Jan 18, 2023
@ansgarm
Copy link
Member

ansgarm commented Jan 19, 2023

Hi @evanstachowiak 👋

Thank you for raising this! Regarding your two points:

  1. Making requests to APIs in constructs in general is discouraged and can even be seen as an anti pattern. Changing infrastructure using CDKTF is a two stage process. The first stage synthesizes a (Terraform) configuration by running the user defined code and the second stage is planning and applying those changes using Terraform. The aforementioned first stage should be as deterministic as possible. When constructs need to query AWS, data sources should be used for that. I agree that bringing data sources into the mix can complicate things, though. We've seen some users having success with making API requests at the top-level of their application and then pass down that data as configuration to their constructs. Using async code in the top-level of a CDK application works already today, too. But I can see why one would want to make API requests in the individual constructs from an encapsulation perspective. However, running such code in constructs introduces additional challenges that can cause confusion. One example would be a construct that gets passed a string (which is supposed to be used in the API request) and that string contains a token (because it is e.g. passed a reference to the output of another resource). In this case there is no way to figure out the actual value of the string while the application is synthesizing.

  2. Do you happen to have an example for that? I can't really imagine that it is not possible to use imports from ESM modules in synchronous functions. However, I could imagine that a non-ESM application can only asynchronously import ESM modules – I don't have much experience with ESM modules, though.

While I personally (and the AWS CDK, too) don't think making constructs async is the right way forward, I still think it would be valuable to allow users to e.g. query data using code that they define as part of their CDK application. There are two different existing concepts that could tackle this from another angle:

  1. Custom resources / custom datasources: This concept would offer an API to write async code and produce a custom Terraform provider based off of it during synthesis. This would allow us to run the async code during the second stage.
  2. Lifecycle Hooks: Hooks probably would have an async API and CDKTF would ensure that they are called at the right time.

What is your concrete use-case that requires constructs to support async?

@ansgarm ansgarm added waiting-on-answer and removed new Un-triaged issue labels Jan 19, 2023
@evanstachowiak
Copy link
Author

evanstachowiak commented Jan 23, 2023

Hi @ansgarm ! Thank you for your prompt and detailed answer to my issue.

I will try to give you as detailed answers to your questions.

  1. An example I can give you is that when migrating to CDK, I'm opting to no longer use remote terraform states within different modules. They become brittle when refactoring and cause too much interdependence between unrelated modules. As such, it has been very convenient to use the native aws javascript APIs to query resources instead of relying on remote terraform state.
    One specific use-case is to query the private subnets for provisioning a service behind a loadbalancer. I am using the terraform registry VPC module in which private subnets are following the naming convention: ${vpcName}-private-${az}. I can easily filter the resources I receive back based on ${vpcName}-private and ignore the az as that is not relevant. To do this in terraform is pretty gnarly and is the reason I switched to CDK in the first place.
    Querying other simple data sources is done within terraform but when needing to chain together multiple data sources, e.g. first querying vpc id, then querying subnets, then filtering private subnets, this is a very bad experience in terraform.
    I'm also surprised to hear that doing these requests within a Construct is an anti-pattern. In the CDK docs itself it encourages using loops and other native language features, but how can one utilize these language niceties when using data sources? Then one is forced back to using the awkward terraform 'for' loop syntax which I am switching to get away from.

  2. Sorry, I was not specific enough. It is not possible to import an ESM module synchronously from a non-ESM module (as far as I can tell, I also don't have so much experience with ESM modules). The way to do the import is with the dynamic import function, which returns a Promise. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import. If you have a better idea how to import an ESM module into a Construct, I'm happy to be wrong.

Could you also clarify this statement? "Using async code in the top-level of a CDK application works already today". I assume you mean that before declaring the stack & construct, one can use async. Is there something I am missing here?

@ansgarm
Copy link
Member

ansgarm commented Feb 3, 2023

Hi @evanstachowiak!

Thanks for your detailed answer and insights into your use-case.

  1. I can see where you're coming from and I agree that using data sources takes away the benefits you gained by using a high-level programming language at this time. I think the reason why CDKTF is in this tight spot is because we're using imperative programming languages to create a declarative definition of infrastructure. With declarative definition of infrastructure being basically the foundation of Terraform. And this is a hard problem to solve. By introducing Iterators we tried to take a first step towards solving this problem in a way that works for both worlds but using them still doesn't enable you to use native loops. But I'm confident that we can improve this aspect of CDKTF in the future to make it less painful to work with remote data.

  2. I don't have a better idea, unfortunately. I always found them to be a pain, to be honest. I assume you'd have to switch your entire project to ESM modules to be able to use them properly.

Could you also clarify this statement? "Using async code in the top-level of a CDK application works already today"

Sure! Your assumption is right. You could e.g. change your entrypoint to something along the lines of this:

const config = await someFunctionDoingAPIStuff();

const app = new App();
new MyStack(app, "my-stack", config);
app.synth();

(Using the --experimental-top-level-await, alternatively you could wrap this code in (async function(){ ... })() to achieve the same effect)

But I agree, that this breaks encapsulation in a way that you'd need to know at a root level, which kind of config your constructs need (and you'd need to pass that down). However, you could write your own base Stack implementation that asserts it has the right config based on the constructs it contains (using e.g. an Aspect) and at least helps the caller to figure out the right someFunctionDoingAPIStuff call.

@ansgarm
Copy link
Member

ansgarm commented Feb 3, 2023

As this whole topic intrigued me, I was wondering whether it would be possible to build my proposed idea in a way that wouldn't break encapsulation and I'd say that is already working today. Here's a link to the prototype.

@evanstachowiak
Copy link
Author

@ansgarm , thanks a lot for taking the time to write up such a detailed answer.

Nice tip with the top-level wrapped async function.

@shishkin
Copy link

One use case for async constructs I have is the need to call async AWS API.

The need for that a circular dependency I have otherwise. I need to create a CloudFront distribution with a Lambda@Edge function intercepting viewer requests. Lambda needs to know distribution URL which is only knowable after deployment. And there is no separate resource to create a lambda function association on the distribution. The only way I see is a custom resource that will update the distribution afterwards with the lambda ARN. I know I could specify domain alias for the distribution which is known upfront, but I'd like to avoid that constraint.

What I'm considering now is to write a node script and execute it a child process synchronously. @ansgarm 's prototype seems pretty interesting too, though a bit hacky.

But if there is a cleaner way to solve my particular issue without async code, I'd be very happy to hear that.

@RichiCoder1
Copy link

RichiCoder1 commented Feb 26, 2023

Howdy! Just stumbling upon this issue. I've run into variations of this issue a lot with the AWS CDK, and I would absolutely advocate for two solutions somewhat outlined in this thread.

  • Custom Resources: I use the AWS Equivalent all the time, and while not perfect it solves the issues of need to inject custom resource or resource associations (like described in the above comment) pretty well. Makes the logic declarative & observable while allowing an imperative implementation.
  • Context Providers: While I think custom datasources also solves this, context providers fit the native CDK model better. Sadly the AWS CDK hasn't implemented this as a generic open framework, I think it would fit both the AWS CDK and CDKTF very well as a way to get dynamic data at Terraform Construction time while still allowing/encouraging the outputted Terraform to be declarative. This also allows committing and invalidating context data for immutable builds/deploys.

@github-actions
Copy link
Contributor

Hi there! 👋 We haven't heard from you in 30 days and would like to know if the problem has been resolved or if you still need help. If we don't hear from you before then, I'll auto-close this issue in 30 days.

@github-actions github-actions bot added the stale An issue or pull request that has not been updated in a very long time label Mar 29, 2023
@github-actions
Copy link
Contributor

I'm closing this issue because we haven't heard back in 60 days. ⌛️ If you still need help, feel free to comment or reopen the issue!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 29, 2023
@github-actions
Copy link
Contributor

I'm going to lock this issue because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request stale An issue or pull request that has not been updated in a very long time waiting-on-answer
Projects
None yet
Development

No branches or pull requests

4 participants