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(all): Fn::ImportValue fails during deserialization if it contains nested intrinsic function #581

Merged
merged 12 commits into from
Feb 27, 2024
7 changes: 5 additions & 2 deletions src/ir/resources/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub enum ResourceIr {
Sub(Vec<ResourceIr>),
Map(String, Box<ResourceIr>, Box<ResourceIr>),
Base64(Box<ResourceIr>),
ImportValue(String),
ImportValue(Box<ResourceIr>),
GetAZs(Box<ResourceIr>),
Select(usize, Box<ResourceIr>),
Cidr(Box<ResourceIr>, Box<ResourceIr>, Box<ResourceIr>),
Expand Down Expand Up @@ -257,7 +257,10 @@ impl<'a, 'b> ResourceTranslator<'a, 'b> {
Ok(ResourceIr::Base64(Box::new(ir)))
}
},
IntrinsicFunction::ImportValue(name) => Ok(ResourceIr::ImportValue(name)),
IntrinsicFunction::ImportValue(x) => {
let ir = self.translate(x)?;
Ok(ResourceIr::ImportValue(Box::new(ir)))
}
IntrinsicFunction::Select { index, list } => {
let index = match index {
ResourceValue::String(x) => match x.parse::<usize>() {
Expand Down
2 changes: 1 addition & 1 deletion src/parser/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub enum IntrinsicFunction {
value_if_true: ResourceValue,
value_if_false: ResourceValue,
},
ImportValue(String),
ImportValue(ResourceValue),
Join {
sep: String,
list: ResourceValue,
Expand Down
4 changes: 2 additions & 2 deletions src/parser/resource/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,14 +168,14 @@ fn intrinsic_import_value() {
const SHARED_VALUE: &str = "SharedValue.ToImport";
assert_eq!(
ResourceValue::from_value(json!({ "Fn::ImportValue": SHARED_VALUE })).unwrap(),
IntrinsicFunction::ImportValue(SHARED_VALUE.into()).into(),
IntrinsicFunction::ImportValue(ResourceValue::String(SHARED_VALUE.into())).into(),
);
assert_eq!(
ResourceValue::from_value(
serde_yaml::from_str(&format!("!ImportValue {SHARED_VALUE}")).unwrap()
)
.unwrap(),
IntrinsicFunction::ImportValue(SHARED_VALUE.into()).into(),
IntrinsicFunction::ImportValue(ResourceValue::String(SHARED_VALUE.into())).into(),
);
}

Expand Down
4 changes: 3 additions & 1 deletion src/synthesizer/csharp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,9 @@ impl ResourceIr {
output.text(" as string)");
}
ResourceIr::ImportValue(import) => {
output.text(format!("Fn.ImportValue(\"{import}\")"));
output.text("Fn.ImportValue(");
import.emit_csharp(output, schema);
output.text(")");
}
ResourceIr::GetAZs(region) => {
output.text("Fn.GetAzs(");
Expand Down
4 changes: 3 additions & 1 deletion src/synthesizer/golang/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,9 @@ impl GolangEmitter for ResourceIr {
when_false.emit_golang(context, &call, Some(","));
}
Self::ImportValue(import) => {
output.text(format!("cdk.Fn_ImportValue(jsii.String({import:?}))"))
output.text("cdk.Fn_ImportValue(");
import.emit_golang(context, output, None);
output.text(")");
}
Self::Join(sep, list) => {
let items = output.indent_with_options(IndentOptions {
Expand Down
6 changes: 5 additions & 1 deletion src/synthesizer/java/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -787,7 +787,11 @@ fn emit_java(this: ResourceIr, output: &CodeBuffer, class: Option<&str>, schema:
output.text(format!("\n{DOUBLE_INDENT}: "));
emit_java(*if_false, output, class, schema);
}
ResourceIr::ImportValue(text) => output.text(format!("Fn.importValue(\"{text}\")")),
ResourceIr::ImportValue(import) => {
output.text("Fn.importValue(");
emit_java(*import, output, None, schema);
output.text(")");
}
ResourceIr::Join(sep, list) => {
let items = output.indent_with_options(IndentOptions {
indent: DOUBLE_INDENT,
Expand Down
6 changes: 4 additions & 2 deletions src/synthesizer/python/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -633,8 +633,10 @@ fn emit_resource_ir(
output.text(format!(" if {} else ", snake_case(cond_name)));
emit_resource_ir(context, output, if_false, None)
}
ResourceIr::ImportValue(name) => {
output.text(format!("cdk.Fn.import_value('{}')", name.escape_debug()))
ResourceIr::ImportValue(import) => {
output.text("cdk.Fn.import_value(");
emit_resource_ir(context, output, import, None);
output.text(")");
}
ResourceIr::Join(sep, list) => {
let items = output.indent_with_options(IndentOptions {
Expand Down
6 changes: 4 additions & 2 deletions src/synthesizer/typescript/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -598,8 +598,10 @@ fn emit_resource_ir(
output.text(" : ");
emit_resource_ir(context, output, if_false, None)
}
ResourceIr::ImportValue(name) => {
output.text(format!("cdk.Fn.importValue('{}')", name.escape_debug()))
ResourceIr::ImportValue(import) => {
output.text("cdk.Fn.importValue(");
emit_resource_ir(context, output, import, None);
output.text(")");
}
ResourceIr::Join(sep, list) => {
let items = output.indent_with_options(IndentOptions {
Expand Down
3 changes: 2 additions & 1 deletion tests/end-to-end.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ test_case!(vpc, "VpcStack");
test_case!(sam_nodejs_lambda, "SAMNodeJSLambda");
// These stack should be identical to the ones above
test_case!(sam_nodejs_lambda_arr_transform, "SAMNodeJSLambda", ALL);
test_case!(batch, "BatchStack", &["golang"]);
test_case!(batch, "BatchStack", &["golang", "java"]); //java fails cdk synth bc template produced has non-deterministic order
test_case!(cloudwatch, "CloudwatchStack", &["golang"]);

// Add new test cases here

Expand Down
17 changes: 17 additions & 0 deletions tests/end-to-end/cloudwatch/csharp/Program.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//Auto-generated
using Amazon.CDK;
sealed class Program
{
public static void Main(string[] args)
{
var app = new App(new AppProps
{
DefaultStackSynthesizer = new DefaultStackSynthesizer(new DefaultStackSynthesizerProps
{
GenerateBootstrapVersionRule = false,
}),
});
new CloudwatchStack.CloudwatchStack(app, "Stack");
app.Synth();
}
}
53 changes: 53 additions & 0 deletions tests/end-to-end/cloudwatch/csharp/Stack.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
using Amazon.CDK;
using Amazon.CDK.AWS.CloudWatch;
using Constructs;
using System.Collections.Generic;

namespace CloudwatchStack
{
public class CloudwatchStackProps : StackProps
{
/// <summary>
/// Environment used for this deployment.
/// </summary>
public string EnvironmentName { get; set; }

}

public class CloudwatchStack : Stack
{
public CloudwatchStack(Construct scope, string id, CloudwatchStackProps props = null) : base(scope, id, props)
{
// Applying default props
props ??= new CloudwatchStackProps();
props.EnvironmentName ??= "dev";


// Resources
var myApi5xxErrorsAlarm = new CfnAlarm(this, "MyApi5xxErrorsAlarm", new CfnAlarmProps
{
AlarmDescription = "Example alarm",
Namespace = "AWS/ApiGateway",
Dimensions = new []
{
new CfnAlarm.DimensionProperty
{
Name = "ApiName",
Value = "MyApi",
},
},
MetricName = "5XXError",
ComparisonOperator = "GreaterThanThreshold",
Statistic = "Average",
Threshold = 0,
Period = 900,
EvaluationPeriods = 1,
TreatMissingData = "notBreaching",
AlarmActions = new []
{
Fn.ImportValue($"{props.EnvironmentName}AlarmsTopicArn"),
},
});
}
}
}
65 changes: 65 additions & 0 deletions tests/end-to-end/cloudwatch/csharp/Stack.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
diff --git a/./tests/end-to-end/cloudwatch/template.json b/tests/end-to-end/cloudwatch-csharp-working-dir/cdk.out/Stack.template.json
index 8d26c7b..aad44e9 100644
--- a/./tests/end-to-end/cloudwatch/template.json
+++ b/tests/end-to-end/cloudwatch-csharp-working-dir/cdk.out/Stack.template.json
@@ -1,50 +1,28 @@
{
- "AWSTemplateFormatVersion": "2010-09-09",
- "Parameters": {
- "EnvironmentName": {
- "Default": "dev",
- "Description": "Environment used for this deployment.",
- "Type": "String",
- "AllowedValues": [
- "dev",
- "stage",
- "prod"
- ]
- }
- },
"Resources": {
"MyApi5xxErrorsAlarm": {
"Type": "AWS::CloudWatch::Alarm",
"Properties": {
+ "AlarmActions": [
+ {
+ "Fn::ImportValue": "devAlarmsTopicArn"
+ }
+ ],
"AlarmDescription": "Example alarm",
- "Namespace": "AWS/ApiGateway",
+ "ComparisonOperator": "GreaterThanThreshold",
"Dimensions": [
{
"Name": "ApiName",
"Value": "MyApi"
}
],
+ "EvaluationPeriods": 1,
"MetricName": "5XXError",
- "ComparisonOperator": "GreaterThanThreshold",
+ "Namespace": "AWS/ApiGateway",
+ "Period": 900,
"Statistic": "Average",
"Threshold": 0,
- "Period": 900,
- "EvaluationPeriods": 1,
- "TreatMissingData": "notBreaching",
- "AlarmActions": [
- {
- "Fn::ImportValue": {
- "Fn::Sub": [
- "${Environment}AlarmsTopicArn",
- {
- "Environment": {
- "Ref": "EnvironmentName"
- }
- }
- ]
- }
- }
- ]
+ "TreatMissingData": "notBreaching"
}
}
}
29 changes: 29 additions & 0 deletions tests/end-to-end/cloudwatch/csharp/Stack.template.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"Resources": {
"MyApi5xxErrorsAlarm": {
"Type": "AWS::CloudWatch::Alarm",
"Properties": {
"AlarmActions": [
{
"Fn::ImportValue": "devAlarmsTopicArn"
}
],
"AlarmDescription": "Example alarm",
"ComparisonOperator": "GreaterThanThreshold",
"Dimensions": [
{
"Name": "ApiName",
"Value": "MyApi"
}
],
"EvaluationPeriods": 1,
"MetricName": "5XXError",
"Namespace": "AWS/ApiGateway",
"Period": 900,
"Statistic": "Average",
"Threshold": 0,
"TreatMissingData": "notBreaching"
}
}
}
}
58 changes: 58 additions & 0 deletions tests/end-to-end/cloudwatch/golang/stack.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package main

import (
"fmt"

cdk "github.com/aws/aws-cdk-go/awscdk/v2"
cloudwatch "github.com/aws/aws-cdk-go/awscdk/v2/awscloudwatch"
"github.com/aws/constructs-go/constructs/v10"
"github.com/aws/jsii-runtime-go"
)

type CloudwatchStackProps struct {
cdk.StackProps
/// Environment used for this deployment.
EnvironmentName *string
}

type CloudwatchStack struct {
cdk.Stack
}

func NewCloudwatchStack(scope constructs.Construct, id string, props *CloudwatchStackProps) *CloudwatchStack {
var sprops cdk.StackProps
if props != nil {
sprops = props.StackProps
}
stack := cdk.NewStack(scope, &id, &sprops)

cloud_watch.NewCfnAlarm(
stack,
jsii.String("MyApi5xxErrorsAlarm"),
&cloud_watch.CfnAlarmProps{
AlarmDescription: jsii.String("Example alarm"),
Namespace: jsii.String("AWS/ApiGateway"),
Dimensions: &[]interface{}{
&DimensionProperty{
Name: jsii.String("ApiName"),
Value: jsii.String("MyApi"),
},
},
MetricName: jsii.String("5XXError"),
ComparisonOperator: jsii.String("GreaterThanThreshold"),
Statistic: jsii.String("Average"),
Threshold: jsii.Number(0),
Period: jsii.Number(900),
EvaluationPeriods: jsii.Number(1),
TreatMissingData: jsii.String("notBreaching"),
AlarmActions: &[]*string{
cdk.Fn_ImportValue(jsii.String(fmt.Sprintf("%vAlarmsTopicArn", props.EnvironmentName))),
},
},
)

return &CloudwatchStack{
Stack: stack,
}
}

Loading
Loading