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

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

Open
YYang30 opened this issue Oct 23, 2023 · 7 comments
Labels
aws-cdk-lib Related to the aws-cdk-lib package bug This issue is a bug. feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs p1

Comments

@YYang30
Copy link

YYang30 commented Oct 23, 2023

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

  • Prepare an CFN template that contains the usage of "AccountIdFromAlias".
  • Use aws-cdk-lib/cloudformation-include to include the CFN template.

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

@YYang30 YYang30 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 23, 2023
@github-actions github-actions bot added the aws-cdk-lib Related to the aws-cdk-lib package label Oct 23, 2023
@indrora indrora added p2 feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs and removed needs-triage This issue or PR still needs to be triaged. labels Oct 23, 2023
@indrora
Copy link
Contributor

indrora commented Oct 23, 2023

Thank you for your report.

@peterwoodworth peterwoodworth removed the feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs label Oct 23, 2023
@indrora indrora added feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs p1 and removed p2 labels Oct 25, 2023
@indrora
Copy link
Contributor

indrora commented Oct 25, 2023

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
Copy link
Author

YYang30 commented Oct 25, 2023

@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
Copy link
Author

YYang30 commented Nov 20, 2023

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

@indrora
Copy link
Contributor

indrora commented Nov 20, 2023

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

@YYang30
Copy link
Author

YYang30 commented Nov 20, 2023

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
Copy link
Author

YYang30 commented Nov 22, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws-cdk-lib Related to the aws-cdk-lib package bug This issue is a bug. feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs p1
Projects
None yet
3 participants