Skip to content

Commit

Permalink
add semistrictbool for fixing booleans (#91)
Browse files Browse the repository at this point in the history
* add semistrictbool for fixing booleans

* update documentation

* update cloudformation_actions.py

* increase amount of tests to check this new resolution doesn't break the Generic

* update changelog and version

* reverting unnecessary changes as stated previously in PR suggestions

* proposal on how to cover possible str to be bool values

* update CHANGELOG.md

* increase amount of tests for bool resolver

Co-authored-by: Ramon <[email protected]>
  • Loading branch information
oscarbc96 and w0rmr1d3r authored Feb 24, 2022
1 parent f9179f6 commit 29b0cd8
Show file tree
Hide file tree
Showing 6 changed files with 146 additions and 5 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
# Change Log
All notable changes to this project will be documented in this file.

## 0.16.3 - [2022-02-24]
### Fixes
- Fix `resolve` for `bool`s that can be `str` such as `"true"` or `"false"` or similar, by making `ResolvableBool` to be resolvable to `SemiStrictBool`.
### Updates
- Update `CLOUDFORMATION_ACTIONS`.

## 0.16.2 - [2022-02-16]
### Fixes
- `resolve` was converting to string booleans, this is incompatible since 0.14.0 because bool were converted to StrictBooleans.
Expand Down
23 changes: 23 additions & 0 deletions pycfmodel/cloudformation_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,26 +228,32 @@
"amplifybackend:CreateBackendAPI",
"amplifybackend:CreateBackendAuth",
"amplifybackend:CreateBackendConfig",
"amplifybackend:CreateBackendStorage",
"amplifybackend:CreateToken",
"amplifybackend:DeleteBackend",
"amplifybackend:DeleteBackendAPI",
"amplifybackend:DeleteBackendAuth",
"amplifybackend:DeleteBackendStorage",
"amplifybackend:DeleteToken",
"amplifybackend:GenerateBackendAPIModels",
"amplifybackend:GetBackend",
"amplifybackend:GetBackendAPI",
"amplifybackend:GetBackendAPIModels",
"amplifybackend:GetBackendAuth",
"amplifybackend:GetBackendJob",
"amplifybackend:GetBackendStorage",
"amplifybackend:GetToken",
"amplifybackend:ImportBackendAuth",
"amplifybackend:ImportBackendStorage",
"amplifybackend:ListBackendJobs",
"amplifybackend:ListS3Buckets",
"amplifybackend:RemoveAllBackends",
"amplifybackend:RemoveBackendConfig",
"amplifybackend:UpdateBackendAPI",
"amplifybackend:UpdateBackendAuth",
"amplifybackend:UpdateBackendConfig",
"amplifybackend:UpdateBackendJob",
"amplifybackend:UpdateBackendStorage",
"amplifyuibuilder:CreateComponent",
"amplifyuibuilder:CreateTheme",
"amplifyuibuilder:DeleteComponent",
Expand Down Expand Up @@ -2324,22 +2330,27 @@
"comprehendmedical:DescribeICD10CMInferenceJob",
"comprehendmedical:DescribePHIDetectionJob",
"comprehendmedical:DescribeRxNormInferenceJob",
"comprehendmedical:DescribeSNOMEDCTInferenceJob",
"comprehendmedical:DetectEntitiesV2",
"comprehendmedical:DetectPHI",
"comprehendmedical:InferICD10CM",
"comprehendmedical:InferRxNorm",
"comprehendmedical:InferSNOMEDCT",
"comprehendmedical:ListEntitiesDetectionV2Jobs",
"comprehendmedical:ListICD10CMInferenceJobs",
"comprehendmedical:ListPHIDetectionJobs",
"comprehendmedical:ListRxNormInferenceJobs",
"comprehendmedical:ListSNOMEDCTInferenceJobs",
"comprehendmedical:StartEntitiesDetectionV2Job",
"comprehendmedical:StartICD10CMInferenceJob",
"comprehendmedical:StartPHIDetectionJob",
"comprehendmedical:StartRxNormInferenceJob",
"comprehendmedical:StartSNOMEDCTInferenceJob",
"comprehendmedical:StopEntitiesDetectionV2Job",
"comprehendmedical:StopICD10CMInferenceJob",
"comprehendmedical:StopPHIDetectionJob",
"comprehendmedical:StopRxNormInferenceJob",
"comprehendmedical:StopSNOMEDCTInferenceJob",
"compute-optimizer:DeleteRecommendationPreferences",
"compute-optimizer:DescribeRecommendationExportJobs",
"compute-optimizer:ExportAutoScalingGroupRecommendations",
Expand Down Expand Up @@ -2731,7 +2742,9 @@
"datasync:CancelTaskExecution",
"datasync:CreateAgent",
"datasync:CreateLocationEfs",
"datasync:CreateLocationFsxLustre",
"datasync:CreateLocationFsxWindows",
"datasync:CreateLocationHdfs",
"datasync:CreateLocationNfs",
"datasync:CreateLocationObjectStorage",
"datasync:CreateLocationS3",
Expand All @@ -2742,7 +2755,9 @@
"datasync:DeleteTask",
"datasync:DescribeAgent",
"datasync:DescribeLocationEfs",
"datasync:DescribeLocationFsxLustre",
"datasync:DescribeLocationFsxWindows",
"datasync:DescribeLocationHdfs",
"datasync:DescribeLocationNfs",
"datasync:DescribeLocationObjectStorage",
"datasync:DescribeLocationS3",
Expand All @@ -2758,6 +2773,7 @@
"datasync:TagResource",
"datasync:UntagResource",
"datasync:UpdateAgent",
"datasync:UpdateLocationHdfs",
"datasync:UpdateLocationNfs",
"datasync:UpdateLocationObjectStorage",
"datasync:UpdateLocationSmb",
Expand Down Expand Up @@ -7915,6 +7931,7 @@
"mobiletargeting:PutEvents",
"mobiletargeting:RemoveAttributes",
"mobiletargeting:SendMessages",
"mobiletargeting:SendOTPMessage",
"mobiletargeting:SendUsersMessages",
"mobiletargeting:TagResource",
"mobiletargeting:UntagResource",
Expand Down Expand Up @@ -8444,20 +8461,24 @@
"pricing:GetProducts",
"profile:AddProfileKey",
"profile:CreateDomain",
"profile:CreateIntegrationWorkflow",
"profile:CreateProfile",
"profile:DeleteDomain",
"profile:DeleteIntegration",
"profile:DeleteProfile",
"profile:DeleteProfileKey",
"profile:DeleteProfileObject",
"profile:DeleteProfileObjectType",
"profile:DeleteWorkflow",
"profile:GetAutoMergingPreview",
"profile:GetDomain",
"profile:GetIdentityResolutionJob",
"profile:GetIntegration",
"profile:GetMatches",
"profile:GetProfileObjectType",
"profile:GetProfileObjectTypeTemplate",
"profile:GetWorkflow",
"profile:GetWorkflowSteps",
"profile:ListAccountIntegrations",
"profile:ListDomains",
"profile:ListIdentityResolutionJobs",
Expand All @@ -8466,6 +8487,7 @@
"profile:ListProfileObjectTypes",
"profile:ListProfileObjects",
"profile:ListTagsForResource",
"profile:ListWorkflows",
"profile:MergeProfiles",
"profile:PutIntegration",
"profile:PutProfileObject",
Expand Down Expand Up @@ -11210,6 +11232,7 @@
"timestream:ListScheduledQueries",
"timestream:ListTables",
"timestream:ListTagsForResource",
"timestream:PrepareQuery",
"timestream:Select",
"timestream:SelectValues",
"timestream:TagResource",
Expand Down
30 changes: 28 additions & 2 deletions pycfmodel/model/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from ipaddress import IPv4Network, IPv6Network, _BaseNetwork
from typing import Any, Dict, Generator, List, TypeVar, Union

from pydantic import StrictBool
from pydantic.networks import NetworkType
from pydantic.typing import AnyCallable

Expand Down Expand Up @@ -37,6 +36,33 @@ def validate(cls, value: NetworkType) -> IPv6Network:
return IPv6Network(value, False)


class SemiStrictBool(int):
"""
SemiStrictBool to allow for bools which are not type-coerced.
"""

@classmethod
def __modify_schema__(cls, field_schema: Dict[str, Any]) -> None:
field_schema.update(type="boolean")

@classmethod
def __get_validators__(cls):
yield cls.validate

@classmethod
def validate(cls, value: Any) -> bool:
"""
Ensure that we only allow bools.
"""
if isinstance(value, bool):
return value

if isinstance(value, str) and value.lower() in ("true", "false"):
return value.lower() == "true"

raise ValueError("Value given can't be validated as bool")


T = TypeVar("T")

Resolvable = Union[T, FunctionDict]
Expand All @@ -48,7 +74,7 @@ def validate(cls, value: NetworkType) -> IPv6Network:
ResolvableInt = Resolvable[int]
ResolvableDate = Resolvable[date]
ResolvableDatetime = Resolvable[datetime]
ResolvableBool = Resolvable[StrictBool]
ResolvableBool = Resolvable[SemiStrictBool]

ResolvableIPv4Network = Resolvable[LooseIPv4Network]
ResolvableIPv6Network = Resolvable[LooseIPv6Network]
Expand Down
4 changes: 3 additions & 1 deletion pycfmodel/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,12 @@ def resolve(function: ValidResolvers, params: Dict, mappings: Dict[str, Dict], c
if CONTAINS_SSM_PARAMETER.match(function):
ssm_parameter_key = CONTAINS_SSM_PARAMETER.match(function).group(1)
return resolve_ssm(ssm_parameter_key, params)
if function.lower() in ["true", "false"]:
return function.lower()
return function

if isinstance(function, bool):
return function
return "true" if function else "false"

if isinstance(function, (int, float, date, IPv4Network, IPv6Network)):
return str(function)
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

setup(
name="pycfmodel",
version="0.16.2",
version="0.16.3",
description="A python model for CloudFormation scripts",
author="Skyscanner Product Security",
author_email="[email protected]",
Expand Down
86 changes: 85 additions & 1 deletion tests/test_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import pytest

from pycfmodel import parse
from pycfmodel.model.resources.generic_resource import GenericResource
from pycfmodel.model.resources.kms_key import KMSKey
from pycfmodel.resolver import resolve

Expand Down Expand Up @@ -193,6 +194,14 @@ def test_not(function, expected_output):
({"Fn::Equals": [False, False]}, True),
({"Fn::Equals": [False, True]}, False),
({"Fn::Equals": [True, False]}, False),
({"Fn::Equals": ["true", True]}, True),
({"Fn::Equals": ["True", True]}, True),
({"Fn::Equals": ["TRUE", True]}, True),
({"Fn::Equals": ["false", False]}, True),
({"Fn::Equals": ["False", False]}, True),
({"Fn::Equals": ["FALSE", False]}, True),
({"Fn::Equals": ["true", False]}, False),
({"Fn::Equals": ["false", True]}, False),
],
)
def test_equals(function, expected_output):
Expand Down Expand Up @@ -590,5 +599,80 @@ def test_resolve_booleans():
}
},
}
model = parse(template).resolve(extra_params={"some-service-arn:1": "vpc-123-abc"})
model = parse(template).resolve()
assert isinstance(model.Resources["KMSKey"], KMSKey)


def test_resolve_booleans_on_conditions_for_modeled_resource():
template = {
"AWSTemplateFormatVersion": "2010-09-09",
"Resources": {
"KMSKey": {
"Type": "AWS::KMS::Key",
"Properties": {
"Description": "a key with an statement with a bool condition in it",
"Enabled": True,
"EnableKeyRotation": True,
"KeyPolicy": {
"Version": "2012-10-17",
"Id": "Key-Policy",
"Statement": [
{
"Action": ["kms:CreateGrant", "kms:ListGrants", "kms:RevokeGrant"],
"Effect": "Allow",
"Sid": "Allow attachment of persistent resources",
"Principal": {"AWS": "*"},
"Resource": "*",
"Condition": {"Bool": {"kms:GrantIsForAWSResource": "true"}},
}
],
},
},
}
},
}

model = parse(template).resolve()
resource = model.Resources["KMSKey"]
assert isinstance(resource, KMSKey)
assert resource.Properties.Enabled is True
assert resource.Properties.EnableKeyRotation is True
assert resource.Properties.KeyPolicy.Statement[0].Condition.Bool["kms:GrantIsForAWSResource"] is True


def test_resolve_booleans_different_properties_for_generic_resource():
template = {
"AWSTemplateFormatVersion": "2010-09-09",
"Resources": {
"NotModeledResource": {
"Type": "AWS::Not::Modeled",
"Properties": {
"PropertyOne": True,
"PropertyTwo": "true",
"PropertyThree": "TRUE",
"PropertyFour": "True",
"Policy": {
"Version": "2012-10-17",
"Statement": [
{
"Action": ["kms:CreateGrant", "kms:ListGrants", "kms:RevokeGrant"],
"Effect": "Allow",
"Principal": {"AWS": "*"},
"Resource": "*",
"Condition": {"Bool": {"kms:GrantIsForAWSResource": "true"}},
}
],
},
},
}
},
}

model = parse(template).resolve()
resource = model.Resources["NotModeledResource"]
assert isinstance(resource, GenericResource)
assert resource.Properties.PropertyOne is True
assert resource.Properties.PropertyTwo is True
assert resource.Properties.PropertyThree is True
assert resource.Properties.PropertyFour is True
assert resource.Properties.Policy.Statement[0].Condition.Bool["kms:GrantIsForAWSResource"] is True

0 comments on commit 29b0cd8

Please sign in to comment.