Skip to content
This repository was archived by the owner on May 5, 2025. It is now read-only.

Commit d1eba3c

Browse files
suejungshinsuejungshin
authored
fix: Fix yaml validation for key named "type" (#588)
* fix: Fix yaml validation for key named "type" * cleanup * cleanup * add test * add tests * cleanup * fix allow_unknown * retrigger tests * run lint * fix lint
1 parent 133c177 commit d1eba3c

2 files changed

Lines changed: 153 additions & 11 deletions

File tree

‎shared/validation/user_schema.py‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@
7878

7979
custom_status_common_config = {
8080
"name_prefix": {"type": "string", "regex": r"^[\w\-\.]+$"},
81+
# Note that "type" is a reserved word in Cerberus parser so use with caution as a
82+
# key in the schema. See workaround at places that call this schema.
8183
"type": {"type": "string", "allowed": ("project", "patch", "changes")},
8284
"target": percent_type_or_auto,
8385
"threshold": percent_type,
@@ -119,8 +121,17 @@
119121

120122
flags_rule_basic_properties = {
121123
"statuses": {
122-
"type": "list",
123-
"schema": {"type": "dict", "schema": flag_status_attributes},
124+
# Use "anyof" to avoid error in Cerberus when child has a key named "type". More background at https://github.com/codecov/shared/pull/588
125+
"anyof": [
126+
{
127+
"type": "list",
128+
"schema": {
129+
"type": "dict",
130+
"schema": flag_status_attributes,
131+
"allow_unknown": False,
132+
},
133+
},
134+
]
124135
},
125136
"carryforward_mode": {
126137
"type": "string",
@@ -134,8 +145,17 @@
134145

135146
component_rule_basic_properties = {
136147
"statuses": {
137-
"type": "list",
138-
"schema": {"type": "dict", "schema": component_status_attributes},
148+
# Use "anyof" to avoid error in Cerberus when child has a key named "type". More background at https://github.com/codecov/shared/pull/588
149+
"anyof": [
150+
{
151+
"type": "list",
152+
"schema": {
153+
"type": "dict",
154+
"schema": component_status_attributes,
155+
"allow_unknown": False,
156+
},
157+
}
158+
],
139159
},
140160
"flag_regexes": {"type": "list", "schema": {"type": "string"}},
141161
"paths": path_list_structure,

‎tests/unit/validation/test_validation.py‎

Lines changed: 129 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,61 @@ def test_many_flags_validation(self):
459459
}
460460
assert validate_yaml(user_input) == expected_result
461461

462+
def test_flags_schema_error_for_key_named_type(self):
463+
user_input = {
464+
"flag_management": {
465+
"default_rules": {
466+
"carryforward": False,
467+
"statuses": [{"name_prefix": "aaa", "type": "patch"}],
468+
},
469+
"individual_flags": [
470+
{
471+
"name": "cawcaw",
472+
"paths": ["banana"],
473+
"after_n_builds": 3,
474+
# expected "statuses" to be a list but is an object instead & that
475+
# object contains a key named "type" (reserved word)
476+
"statuses": {"type": "patch"},
477+
}
478+
],
479+
},
480+
}
481+
482+
with pytest.raises(Exception) as exp:
483+
validate_yaml(user_input)
484+
485+
err = exp.value
486+
assert err is not None, "validate_yaml didn't raise anything"
487+
assert err.error_location == [
488+
"flag_management",
489+
"individual_flags",
490+
0,
491+
"statuses",
492+
]
493+
assert err.error_message == "no definitions validate"
494+
assert err.error_dict == {
495+
"flag_management": [
496+
{
497+
"individual_flags": [
498+
{
499+
0: [
500+
{
501+
"statuses": [
502+
"no definitions validate",
503+
{
504+
"anyof definition 0": [
505+
"must be of list type"
506+
]
507+
},
508+
]
509+
}
510+
],
511+
}
512+
]
513+
}
514+
]
515+
}
516+
462517
def test_validate_bot_none(self):
463518
user_input = {"codecov": {"bot": None}}
464519
expected_result = {"codecov": {"bot": None}}
@@ -797,14 +852,30 @@ def test_yaml_with_flag_management_statuses_with_flags(self):
797852
}
798853
with pytest.raises(InvalidYamlException) as exc:
799854
validate_yaml(user_input)
800-
assert exc.value.error_location == [
801-
"flag_management",
802-
"default_rules",
803-
"statuses",
804-
0,
805-
"flags",
855+
assert exc.value.error_location == [
856+
"flag_management",
857+
"default_rules",
858+
"statuses",
859+
]
860+
assert exc.value.error_message == "no definitions validate"
861+
assert exc.value.error_dict == {
862+
"flag_management": [
863+
{
864+
"default_rules": [
865+
{
866+
"statuses": [
867+
"no definitions validate",
868+
{
869+
"anyof definition 0": [
870+
{0: [{"flags": ["unknown field"]}]}
871+
]
872+
},
873+
]
874+
}
875+
]
876+
}
806877
]
807-
assert exc.value.error_message == "extra keys not allowed"
878+
}
808879

809880
def test_github_checks(self):
810881
user_input = {"github_checks": True}
@@ -1331,6 +1402,57 @@ def test_components_schema_error():
13311402
}
13321403

13331404

1405+
def test_components_schema_error_for_key_named_type():
1406+
user_input = {
1407+
"component_management": {
1408+
"default_rules": {
1409+
"flag_regexes": ["global_flag"],
1410+
},
1411+
"individual_components": [
1412+
{
1413+
"name": "fruits",
1414+
"component_id": "app_0",
1415+
"flag_regexes": ["fruit_.*", "^specific_flag$"],
1416+
"paths": ["src/.*"],
1417+
# expected "statuses" to be a list but is an object instead & that
1418+
# object contains a key named "type" (reserved word)
1419+
"statuses": {"type": "patch", "name_prefix": "co", "target": 90},
1420+
}
1421+
],
1422+
}
1423+
}
1424+
1425+
with pytest.raises(Exception) as exp:
1426+
validate_yaml(user_input)
1427+
err = exp.value
1428+
assert err is not None, "validate_yaml didn't raise anything"
1429+
assert err.error_location == [
1430+
"component_management",
1431+
"individual_components",
1432+
0,
1433+
"statuses",
1434+
]
1435+
assert err.error_message == "no definitions validate"
1436+
assert err.error_dict == {
1437+
"component_management": [
1438+
{
1439+
"individual_components": [
1440+
{
1441+
0: [
1442+
{
1443+
"statuses": [
1444+
"no definitions validate",
1445+
{"anyof definition 0": ["must be of list type"]},
1446+
]
1447+
}
1448+
],
1449+
}
1450+
]
1451+
}
1452+
]
1453+
}
1454+
1455+
13341456
def test_removed_code_behavior_config_valid():
13351457
user_input = {
13361458
"coverage": {

0 commit comments

Comments
 (0)