aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.5k stars 3.85k forks source link

(aws-cdk-lib): (Add support for this CfnIntrinsic function -- Fn::AccountIdFromAlias) #27642

Open YYang30 opened 10 months ago

YYang30 commented 10 months ago

Describe the bug

CfnInclude fails to import CFN templates if templates contain the usage of this CFN function -- "Fn:: AccountIdFromAlias". CfnInclude fails with error -- "Error: Unsupported CloudFormation function 'Fn::AccountIdFromAlias". CfnParser.parseIfCfnIntrinsic reports Fn::AccountIdFromAlias as an "unsupported" CFN function.

Example stacktrace

    at CfnParser.parseIfCfnIntrinsic (/Volumes/workplace/starship_cdk/src/AWSSCMStarshipServiceCDK/node_modules/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.js:1:12544)
    at CfnParser.parseValue (/Volumes/workplace/starship_cdk/src/AWSSCMStarshipServiceCDK/node_modules/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.js:1:8679)
    ... <truncated for readability> 
    at CfnInclude.getOrCreateResource (/Volumes/workplace/starship_cdk/src/AWSSCMStarshipServiceCDK/node_modules/aws-cdk-lib/cloudformation-include/lib/cfn-include.js:1:13148)

Expected Behavior

CfnInclude can properly import the CFN template that uses AccountIdFromAlias.

Current Behavior

CfnInclude fails to import the CFN template that uses AccountIdFromAlias with error -- "Error: Unsupported CloudFormation function 'Fn::AccountIdFromAlias".

Reproduction Steps

Possible Solution

This function needs to be updated to support this CFN function. source code link. Please add a new "case" statement to support this CFN function.

private parseIfCfnIntrinsic(object: any): any {
    const key = this.looksLikeCfnIntrinsic(object);
    switch (key) {
    case 'Fn::GetAtt': {...}
    case 'Ref':'{...}
    // Need to add a new case for `Fn::AccountIdFromAlias`
...
}

Additional Information/Context

No response

CDK CLI Version

2.87.0

Framework Version

No response

Node.js Version

v18.16.0

OS

Mac, Amazon Linux

Language

TypeScript

Language Version

No response

Other information

No response

indrora commented 10 months ago

Thank you for your report.

indrora commented 10 months ago

There is a workaround to use the AWS SDK to turn the alias into an account ID, but that doesn't replace it not being supported in CfnInclude.

YYang30 commented 10 months ago

@indrora You're right on this. I wanted to add a note on that. We have 15+ services that are not in CDK yet. But we're migrating them to CDK using CfnInclude. For those 15+ services, if their cloud formation templates contain the usage of this CFN function AccountIdFromAlias, CfnInclude would fail to import their CFN templates. While we could technically use AccountIdFromAlias directly in CDK, that requires deprecating the usage in CFN and re-writing the same functionalities in CDK. This is do-able, but not an efficient way (or slows down us migrating to CDK).

On the other hand, in terms of region build automation, AccountIdFromAlias is a preferred way for resolving account ID in CFN templates. We can use that to get rid of a lot of hardcoded account-IDs (and mappings). So adding/supporting CFN function in the CfnInclude (or CfnInclude recognizes it as a valid CFN function) produces a lot of values to all teams that are focusing on region build automation.

Thanks for looking into this issue and the update.

YYang30 commented 9 months ago

Hey @indrora and team, Thanks again for looking into this. Do we have an ETA on the fix? Thank you.

indrora commented 9 months ago

I do not have an ETA. @cgarvis might have an update though.

YYang30 commented 9 months ago

Thank you @indrora . Looking forward to an update on ETA. : ) Meanwhile, we're researching and looking to send a pull request for this issue (if it's feasible).

YYang30 commented 9 months ago

Hello @indrora

ps: I am not sure if this is the right place for asking this question. Apologize if I post the question at wrong place.

Context: I followed the instruction and setup the dev environment for cdk-lib, and was experimenting a code fix for this -- adding support for the new CFN intrinsic function mentioned in this issue -- "Fn:: AccountIdFromAlias". And I was referring to existing codes to understand how existing CFN functions are handled, and just have one question.

I see some CFN functions "extends" [FnBase] class while some CFN functions "implements" [IResolvable] interface. For example, class FnSplit extends FnBase while class FnJoin implements IResolvable. By checking definitions of FnBase class and IResolvable interface, I could understand what they do but am confused on which interface should be used (and why?) For example, when to implement IResolable and when to extend FnBase? Any guidance or notes will be highly appreciated.

Thanks, -Yang