Able to parse PolicyDocument implicit in String properties (#97)

* able to parse policy documents in strings

* update cloudformation actions

* update documentation for PolicyDocument

* update changelog and setup version

* update changelog with PR number

* update changelog with updates

* PR suggestions - validate only aux

* update changelog

Co-authored-by: Ramon <ramon.guimera@skyscanner.net>

CHANGELOG.md

@@ -1,6 +1,10 @@
 # Change Log
 All notable changes to this project will be documented in this file.
 
+## 0.19.0
+### Improvements
+- Able to parse PolicyDocument that are implicit in string properties. [#97](https://github.com/Skyscanner/pycfmodel/pull/97)
+
 ## 0.18.2
 ### Fixes
 - Fix obtaining `policy_documents` for resources without properties. [#98](https://github.com/Skyscanner/pycfmodel/pull/96)

pycfmodel/model/generic.py

@@ -1,7 +1,8 @@
+import json
 from contextlib import suppress
 from typing import Union
 
-from pydantic import BaseModel, Extra, ValidationError, root_validator
+from pydantic import BaseModel, Extra, ValidationError, root_validator, validator
 
 from pycfmodel.model.base import FunctionDict
 from pycfmodel.model.resources.properties.types import Properties
@@ -31,6 +32,22 @@ class _Auxiliar(BaseModel):
         ResolvableStrOrList,
     ]
 
+    @validator("aux", pre=True)
+    def validate_string_property_formatted_as_json(cls, value):
+        """
+        We have detected some properties that are defined as String in CloudFormation but including a
+        PolicyDocument in them, such as:
+        https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-logs-resourcepolicy.html#cfn-logs-resourcepolicy-policydocument
+        For that, we try to parse them to a JSON to be evaluated then to a known property.
+        If we fail to parse it, it means the property is a String without a JSON in it, we return the property as it is.
+        """
+        if isinstance(value, str):
+            try:
+                return json.loads(value)
+            except Exception:
+                return value
+        return value
+
     @classmethod
     def cast(cls, value):
         with suppress(ValidationError):

pycfmodel/model/resources/properties/policy_document.py

@@ -10,13 +10,14 @@
 
 class PolicyDocument(Property):
     """
-    Contains information about an attached policy.
+    Contains information about a Policy Document. More info:
+    https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies.html
 
     Properties:
 
     - Statement: A [statement][pycfmodel.model.resources.properties.statement.Statement] object.
     - Id: An optional string to provide the policy document with an ID.
-    - Version: An optional date indiciating the version of the policy document being used.
+    - Version: An optional date indicating the version of the policy document being used.
     """
 
     class Config(Property.Config):

setup.py

@@ -21,7 +21,7 @@
 
 setup(
     name="pycfmodel",
-    version="0.18.2",
+    version="0.19.0",
     description="A python model for CloudFormation scripts",
     author="Skyscanner Product Security",
     author_email="security@skyscanner.net",

tests/resources/test_generic_resoure.py

@@ -1,5 +1,6 @@
 from pycfmodel.model.resources.generic_resource import GenericResource
 from pycfmodel.model.resources.properties.policy_document import PolicyDocument
+from pycfmodel.model.resources.properties.statement import Principal, Statement
 from pycfmodel.model.resources.properties.statement_condition import StatementCondition
 
 
@@ -40,6 +41,46 @@ def test_generic_resource():
     ]
 
 
+def test_generic_resource_with_policy_document_in_a_string_property():
+    resource = GenericResource.parse_obj(
+        {
+            "Type": "AWS::Logs::ResourcePolicy",
+            "Properties": {
+                "PolicyName": "guardduty-resourcepolicy",
+                "PolicyDocument": '{"Version":"2008-10-17","Statement":[{"Sid":"GDAllowLogs","Effect":"Allow","Principal":{"Service":["events.amazonaws.com","delivery.logs.amazonaws.com"]},"Action":["logs:CreateLogStream"],"Resource":"*"}]}',
+            },
+        }
+    )
+    resource_policy_document = resource.Properties.PolicyDocument
+    assert isinstance(resource, GenericResource)
+    assert isinstance(resource_policy_document, PolicyDocument)
+    assert isinstance(resource.Properties.PolicyName, str)
+    assert resource_policy_document.Statement == [
+        Statement(
+            Sid="GDAllowLogs",
+            Effect="Allow",
+            Resource="*",
+            Action=["logs:CreateLogStream"],
+            Principal=Principal(Service=["events.amazonaws.com", "delivery.logs.amazonaws.com"]),
+        )
+    ]
+
+
+def test_generic_resource_with_bad_json_as_string_is_converted_to_a_string_property():
+    resource = GenericResource.parse_obj(
+        {
+            "Type": "AWS::Logs::ResourcePolicy",
+            "Properties": {
+                "PolicyName": "guardduty-resourcepolicy",
+                "PolicyDocument": '{"Ve:{}]}',
+            },
+        }
+    )
+    assert isinstance(resource, GenericResource)
+    assert isinstance(resource.Properties.PolicyDocument, str)
+    assert isinstance(resource.Properties.PolicyName, str)
+
+
 def test_parse_generic_resource_without_properties():
     resource = GenericResource.parse_obj(
         {