Fix for scenarios where DefinitionString or other parameter that need…

… to stay as json but the paramter has intrinsic functions like Fn::GetAtt or Fn::Sub
awslabs · Jun 29, 2019 · e9e5bf3 · e9e5bf3
1 parent a927976
commit e9e5bf3
Show file tree

Hide file tree

Showing 5 changed files with 174 additions and 5 deletions.
diff --git a/cfn_clean/__init__.py b/cfn_clean/__init__.py
@@ -13,6 +13,12 @@
 
 from cfn_tools.odict import ODict
 from cfn_tools.literal import LiteralString
+
+try:
+    from collections.abc import KeysView
+except ImportError:
+    from collections import KeysView
+
 import json
 import six
 
@@ -22,6 +28,17 @@
 ]
 
 
+def has_intrinsic_functions(parameter):
+    intrinsic_functions = ["Fn::Sub", "!Sub", "!GetAtt"]
+    result = False
+    if isinstance(parameter, (list, tuple, dict, KeysView)):
+        for item in parameter:
+            if item in intrinsic_functions:
+                result = True
+                break
+    return result
+
+
 def convert_join(value):
     """
     Fix a Join ;)
@@ -135,11 +152,12 @@ def cfn_literal_parser(source):
                         # Checking if this resource has "Properties" and the property literal to maintain
                         # Better check than just try/except KeyError :-)
                         if source.get("Properties") and source.get("Properties", {}).get(item[1]):
-                            source["Properties"][item[1]] = LiteralString(u"{}".format(json.dumps(
-                                source["Properties"][item[1]],
-                                indent=2,
-                                separators=(',', ': '))
-                            ))
+                            if not has_intrinsic_functions(source["Properties"][item[1]].keys()):
+                                source["Properties"][item[1]] = LiteralString(u"{}".format(json.dumps(
+                                    source["Properties"][item[1]],
+                                    indent=2,
+                                    separators=(',', ': '))
+                                ))
 
             else:
                 source[key] = cfn_literal_parser(value)

diff --git a/cfn_tools/literal.py b/cfn_tools/literal.py
@@ -8,6 +8,7 @@
 or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
 """
 
+
 class LiteralString(str):
     """
     Class to represent json literal

diff --git a/examples/test_json_def_string_with_sub.json b/examples/test_json_def_string_with_sub.json
@@ -0,0 +1,85 @@
+{
+    "Resources": {
+        "FeedbackRequestStateMachine": {
+            "Properties": {
+                "DefinitionString": {
+                    "Fn::Sub": [
+                        "\n        {\n            \"Comment\": \"State machine which awaits until defined date to submit a feedback request\",\n            \"StartAt\": \"WaitForDueDate\",\n            \"States\": {\n                \"WaitForDueDate\": {\n                    \"Type\": \"Wait\",\n                    \"TimestampPath\": \"$.plannedAt\",\n                    \"Next\": \"SubmitFeedbackRequestToSQS\"\n                },\n                \"SubmitFeedbackRequestToSQS\": {\n                    \"Type\": \"Task\",\n                    \"Resource\": \"arn:aws:states:::sqs:sendMessage\",\n                    \"Parameters\": {\n                        \"MessageBody.$\": \"$\",\n                        \"QueueUrl\": \"${feedbackQueueUrl}\"\n                    },\n                    \"End\": true\n                }\n            }\n        }\n        ",
+                        {
+                            "feedbackQueueUrl": {
+                                "Ref": "FeedbackRequestsQueue"
+                            }
+                        }
+                    ]
+                },
+                "RoleArn": {
+                    "Fn::Join": [
+                        "/",
+                        [
+                            {
+                                "Fn::Sub": "arn:aws:iam::${AWS::AccountId}:role"
+                            },
+                            {
+                                "Ref": "FeedbackRequestStateMachineRole"
+                            }
+                        ]
+                    ]
+                },
+                "StateMachineName": {
+                    "Fn::Sub": "${AWS::StackName}-FeedbackRequestStateMachine"
+                }
+            },
+            "Type": "AWS::StepFunctions::StateMachine"
+        },
+        "FeedbackRequestStateMachineRole": {
+            "Properties": {
+                "AssumeRolePolicyDocument": {
+                    "Statement": [
+                        {
+                            "Action": [
+                                "sts:AssumeRole"
+                            ],
+                            "Effect": "Allow",
+                            "Principal": {
+                                "Service": [
+                                    {
+                                        "Fn::Sub": "states.${AWS::Region}.amazonaws.com"
+                                    }
+                                ]
+                            }
+                        }
+                    ]
+                },
+                "Policies": [
+                    {
+                        "PolicyDocument": {
+                            "Statement": [
+                                {
+                                    "Action": [
+                                        "sqs:SendMessage"
+                                    ],
+                                    "Effect": "Allow",
+                                    "Resource": {
+                                        "Fn::GetAtt": [
+                                            "FeedbackRequestsQueue",
+                                            "Arn"
+                                        ]
+                                    }
+                                }
+                            ]
+                        },
+                        "PolicyName": "submit_request_to_sqs"
+                    }
+                ]
+            },
+            "Type": "AWS::IAM::Role"
+        },
+        "FeedbackRequestsQueue": {
+            "Properties": {
+                "MessageRetentionPeriod": 172800,
+                "VisibilityTimeout": 600
+            },
+            "Type": "AWS::SQS::Queue"
+        }
+    }
+}
diff --git a/examples/test_yaml_def_string_with_sub.yaml b/examples/test_yaml_def_string_with_sub.yaml
@@ -0,0 +1,47 @@
+Resources:
+  FeedbackRequestStateMachine:
+    Properties:
+      DefinitionString: !Sub
+        - "\n        {\n            \"Comment\": \"State machine which awaits until\
+          \ defined date to submit a feedback request\",\n            \"StartAt\"\
+          : \"WaitForDueDate\",\n            \"States\": {\n                \"WaitForDueDate\"\
+          : {\n                    \"Type\": \"Wait\",\n                    \"TimestampPath\"\
+          : \"$.plannedAt\",\n                    \"Next\": \"SubmitFeedbackRequestToSQS\"\
+          \n                },\n                \"SubmitFeedbackRequestToSQS\": {\n\
+          \                    \"Type\": \"Task\",\n                    \"Resource\"\
+          : \"arn:aws:states:::sqs:sendMessage\",\n                    \"Parameters\"\
+          : {\n                        \"MessageBody.$\": \"$\",\n               \
+          \         \"QueueUrl\": \"${feedbackQueueUrl}\"\n                    },\n\
+          \                    \"End\": true\n                }\n            }\n \
+          \       }\n        "
+        - feedbackQueueUrl: !Ref 'FeedbackRequestsQueue'
+      RoleArn: !Join
+        - /
+        - - !Sub 'arn:aws:iam::${AWS::AccountId}:role'
+          - !Ref 'FeedbackRequestStateMachineRole'
+      StateMachineName: !Sub '${AWS::StackName}-FeedbackRequestStateMachine'
+    Type: AWS::StepFunctions::StateMachine
+  FeedbackRequestStateMachineRole:
+    Properties:
+      AssumeRolePolicyDocument:
+        Statement:
+          - Action:
+              - sts:AssumeRole
+            Effect: Allow
+            Principal:
+              Service:
+                - !Sub 'states.${AWS::Region}.amazonaws.com'
+      Policies:
+        - PolicyDocument:
+            Statement:
+              - Action:
+                  - sqs:SendMessage
+                Effect: Allow
+                Resource: !GetAtt 'FeedbackRequestsQueue.Arn'
+          PolicyName: submit_request_to_sqs
+    Type: AWS::IAM::Role
+  FeedbackRequestsQueue:
+    Properties:
+      MessageRetentionPeriod: 172800
+      VisibilityTimeout: 600
+    Type: AWS::SQS::Queue
diff --git a/tests/test_flip.py b/tests/test_flip.py
@@ -42,6 +42,18 @@ def input_json_with_literal():
         return f.read().strip()
 
 
+@pytest.fixture
+def input_json_with_def_string_with_sub():
+    with open("examples/test_json_def_string_with_sub.json", "r") as f:
+        return f.read().strip()
+
+
+@pytest.fixture
+def input_yaml_with_def_string_with_sub():
+    with open("examples/test_yaml_def_string_with_sub.yaml", "r") as f:
+        return f.read()
+
+
 @pytest.fixture
 def input_yaml():
     with open("examples/test.yaml", "r") as f:
@@ -636,3 +648,9 @@ def test_flip_to_yaml_with_json_literal(input_json_with_literal, parsed_yaml_wit
 
     actual = cfn_flip.to_yaml(input_json_with_literal)
     assert load_yaml(actual) == parsed_yaml_with_json_literal
+
+
+def test_flip_to_yaml_with_json_literal_with_sub(input_json_with_def_string_with_sub,
+                                                 input_yaml_with_def_string_with_sub):
+    actual = cfn_flip.to_yaml(input_json_with_def_string_with_sub)
+    assert actual == input_yaml_with_def_string_with_sub