Skip to content

Commit

Permalink
Use schema cache to know if we need to validate props again (#2684)
Browse files Browse the repository at this point in the history
  • Loading branch information
kddejong committed Jan 17, 2024
1 parent efd9050 commit f4866d9
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 20 deletions.
15 changes: 10 additions & 5 deletions src/cfnlint/rules/resources/properties/JsonSchema.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
FN_PREFIX,
PSEUDOPARAMS,
REGEX_DYN_REF,
REGION_PRIMARY,
REGISTRY_SCHEMAS,
UNCONVERTED_SUFFIXES,
load_resource,
Expand Down Expand Up @@ -195,18 +196,22 @@ def match(self, cfn):
if t.startswith("Custom::"):
t = "AWS::CloudFormation::CustomResource"
if t:
cached_validation_run = []
for region in cfn.regions:
self.region = region
schema = {}
try:
schema = PROVIDER_SCHEMA_MANAGER.get_resource_schema(
region, t
).json_schema()
schema = PROVIDER_SCHEMA_MANAGER.get_resource_schema(region, t)
except ResourceNotFoundError as e:
LOGGER.info(e)
continue
if schema:
cfn_validator = self.validator(schema)
if schema.json_schema():
if t in cached_validation_run or region == REGION_PRIMARY:
if schema.is_cached:
# if its cached we already ran the same validation lets not run it again
continue
cached_validation_run.append(t)
cfn_validator = self.validator(schema.json_schema())
path = ["Resources", n, "Properties"]
for scenario in cfn.get_object_without_nested_conditions(
p, path
Expand Down
17 changes: 11 additions & 6 deletions src/cfnlint/rules/resources/properties/StringSize.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"""
import datetime
import json
from typing import Any

import regex as re

Expand All @@ -24,12 +25,10 @@ class StringSize(CloudFormationLintRule):
def _serialize_date(self, obj):
if isinstance(obj, datetime.date):
return obj.isoformat()
raise TypeError(
f"Object of type {obj.__class__.__name__} is not JSON serializable"
)
return json.JSONEncoder.default(self, o=obj)

# pylint: disable=too-many-return-statements
def _remove_functions(self, obj):
def _remove_functions(self, obj: Any) -> Any:
"""Replaces intrinsic functions with string"""
if isinstance(obj, dict):
new_obj = {}
Expand Down Expand Up @@ -70,14 +69,20 @@ def _non_string_min_length(self, instance, mL):

# pylint: disable=unused-argument
def maxLength(self, validator, mL, instance, schema):
if validator.is_type(instance, "object"):
if (
validator.is_type(instance, "object")
and validator.schema.get("type") == "object"
):
yield from self._non_string_max_length(instance, mL)
elif validator.is_type(instance, "string") and len(instance) > mL:
yield ValidationError(f"{instance!r} is too long")

# pylint: disable=unused-argument
def minLength(self, validator, mL, instance, schema):
if validator.is_type(instance, "object"):
if (
validator.is_type(instance, "object")
and validator.schema.get("type") == "object"
):
yield from self._non_string_min_length(instance, mL)
elif validator.is_type(instance, "string") and len(instance) < mL:
yield ValidationError(f"{instance!r} is too short")
17 changes: 11 additions & 6 deletions src/cfnlint/schema/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import shutil
import sys
import zipfile
from copy import copy
from typing import Any, Dict, List, Sequence, Union

import jsonpatch
Expand Down Expand Up @@ -45,7 +46,7 @@ def __init__(self, path: List[str]):


class ProviderSchemaManager:
_schemas: Dict[str, Schema] = {}
_schemas: Dict[str, Dict[str, Schema]] = {}
_provider_schema_modules: Dict[str, Any] = {}
_cache: Dict[str, Union[Sequence, str]] = {}

Expand Down Expand Up @@ -74,7 +75,7 @@ def reset(self):
Important function when processing many templates
and using spec patching
"""
self._schemas: Dict[str, Schema] = {}
self._schemas: Dict[str, Dict[str, Schema]] = {}
self._cache["ResourceTypes"] = {}
self._cache["GetAtts"] = {}
self._cache["RemovedTypes"] = []
Expand Down Expand Up @@ -107,10 +108,14 @@ def get_resource_schema(self, region: str, resource_type: str) -> Schema:
)
# load the schema
if f"{rt.provider}.json" in self._provider_schema_modules[reg.name].cached:
self._schemas[reg.name][rt.name] = self.get_resource_schema(
region=self._region_primary.name,
resource_type=rt.name,
schema_cached = copy(
self.get_resource_schema(
region=self._region_primary.name,
resource_type=rt.name,
)
)
schema_cached.is_cached = True
self._schemas[reg.name][rt.name] = schema_cached
return self._schemas[reg.name][rt.name]
try:
self._schemas[reg.name][rt.name] = Schema(
Expand Down Expand Up @@ -151,7 +156,7 @@ def get_resource_types(self, region: str) -> List[str]:
resource_type = schema.get("typeName")
if resource_type in self._cache["RemovedTypes"]:
continue
self._schemas[reg.name][resource_type] = Schema(schema)
self._schemas[reg.name][resource_type] = Schema(schema, True)
self._cache["ResourceTypes"][region].append(resource_type)
for filename in resource_listdir(
"cfnlint", resource_name=f"{'/'.join(self._root.path)}/{reg.py}"
Expand Down
5 changes: 3 additions & 2 deletions src/cfnlint/schema/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@
class Schema:
_json_schema: Dict

def __init__(self, schema) -> None:
def __init__(self, schema: Dict, is_cached: bool = False) -> None:
self.is_cached = is_cached
self.schema = deepcopy(schema)
self._json_schema = self._cleanse_schema(schema=schema)
self._json_schema = self._cleanse_schema(schema=deepcopy(schema))
self.type_name = schema["typeName"]
self._getatts = GetAtts(self.schema)

Expand Down
26 changes: 25 additions & 1 deletion test/unit/module/schema/test_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from test.testlib.testcase import BaseTestCase
from unittest.mock import ANY, MagicMock, Mock, call, mock_open, patch

from cfnlint.schema.manager import ProviderSchemaManager
from cfnlint.schema.manager import ProviderSchemaManager, ResourceNotFoundError

LOGGER = logging.getLogger("cfnlint.schema.manager")
LOGGER.disabled = True
Expand Down Expand Up @@ -246,3 +246,27 @@ def test_patch_value_error(self):
self.manager.patch("bad", regions=["us-east-1"])
self.assertEqual(mock_exit.type, SystemExit)
self.assertEqual(mock_exit.value.code == 1)


class TestManagerGetResourceSchema(BaseTestCase):
"""Test get resource schema"""

def setUp(self) -> None:
super().setUp()

self.manager = ProviderSchemaManager()

def test_getting_cached_schema(self):
rt = "AWS::EC2::VPC"

self.manager.get_resource_schema("us-east-1", rt)
schema = self.manager.get_resource_schema("us-west-2", rt)

self.assertTrue(schema.is_cached)

def test_removed_types(self):
rt = "AWS::EC2::VPC"
self.manager._cache["RemovedTypes"].append(rt)

with self.assertRaises(ResourceNotFoundError):
self.manager.get_resource_schema("us-east-1", rt)
31 changes: 31 additions & 0 deletions test/unit/rules/resources/properties/test_string_size.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
SPDX-License-Identifier: MIT-0
"""
import datetime
from collections import deque
from test.unit.rules import BaseRuleTestCase

Expand Down Expand Up @@ -31,6 +32,16 @@ def test_max_length(self):
)
self.assertEqual(list(rule.maxLength(validator, 3, "a", {})), [])
self.assertEqual(len(list(rule.maxLength(validator, 3, "abcd", {}))), 1)

def test_max_object_length(self):
"""Test object max length"""
rule = StringSize()
validator = Draft7Validator(
{
"type": "object",
"maxLength": 3,
}
)
self.assertEqual(len(list(rule.maxLength(validator, 10, {"a": "b"}, {}))), 0)
self.assertEqual(len(list(rule.maxLength(validator, 3, {"a": "bcd"}, {}))), 1)
self.assertEqual(
Expand All @@ -42,6 +53,16 @@ def test_max_length(self):
self.assertEqual(
len(list(rule.maxLength(validator, 3, {"Fn::Sub": ["abcd", {}]}, {}))), 1
)
self.assertEqual(
len(
list(rule.maxLength(validator, 100, {"now": datetime.date.today()}, {}))
),
0,
)
self.assertEqual(
len(list(rule.maxLength(validator, 100, {"now": {"Ref": "date"}}, {}))),
0,
)

with self.assertRaises(TypeError):
list(rule.maxLength(validator, 10, {"foo": Unserializable()}, {}))
Expand All @@ -57,6 +78,16 @@ def test_min_length(self):
)
self.assertEqual(list(rule.minLength(validator, 3, "abcde", {})), [])
self.assertEqual(len(list(rule.minLength(validator, 3, "ab", {}))), 1)

def test_min_object_length(self):
"""Test min object length"""
rule = StringSize()
validator = Draft7Validator(
{
"type": "object",
"minLength": 3,
}
)
self.assertEqual(len(list(rule.minLength(validator, 5, {"a": "b"}, {}))), 0)
self.assertEqual(len(list(rule.minLength(validator, 12, {"a": "bcd"}, {}))), 1)
self.assertEqual(
Expand Down

0 comments on commit f4866d9

Please sign in to comment.