Skip to content

Commit b4fbaa1

Browse files
kddejongkddejong
authored
Deal with JSONata differences in choices (#4051)
1 parent 1863011 commit b4fbaa1

File tree

3 files changed

+257
-3
lines changed

3 files changed

+257
-3
lines changed

‎src/cfnlint/data/schemas/other/step_functions/statemachine.json

+73-3
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,86 @@
44
"choice": {
55
"$ref": "#/definitions/common",
66
"additionalProperties": false,
7+
"allOf": [
8+
{
9+
"if": {
10+
"properties": {
11+
"QueryLanguage": {
12+
"const": "JSONata"
13+
}
14+
},
15+
"required": [
16+
"QueryLanguage"
17+
]
18+
},
19+
"then": {
20+
"properties": {
21+
"Choices": {
22+
"items": {
23+
"$ref": "#/definitions/choice/definitions/JSONataChoice"
24+
},
25+
"type": "array"
26+
}
27+
}
28+
}
29+
},
30+
{
31+
"if": {
32+
"oneOf": [
33+
{
34+
"properties": {
35+
"QueryLanguage": {
36+
"const": "JSONPath"
37+
}
38+
},
39+
"required": [
40+
"QueryLanguage"
41+
]
42+
},
43+
{
44+
"not": {
45+
"required": [
46+
"QueryLanguage"
47+
]
48+
}
49+
}
50+
]
51+
},
52+
"then": {
53+
"properties": {
54+
"Choices": {
55+
"items": {
56+
"$ref": "#/definitions/choice/definitions/Operator"
57+
},
58+
"type": "array"
59+
}
60+
}
61+
}
62+
}
63+
],
764
"definitions": {
865
"Assign": {
966
"type": "object"
1067
},
1168
"Condition": {
1269
"type": "string"
1370
},
71+
"JSONataChoice": {
72+
"properties": {
73+
"Condition": {
74+
"type": "string"
75+
},
76+
"Next": {
77+
"pattern": "^.{1,128}$",
78+
"type": "string"
79+
}
80+
},
81+
"required": [
82+
"Condition",
83+
"Next"
84+
],
85+
"type": "object"
86+
},
1487
"Operator": {
1588
"properties": {
1689
"And": {
@@ -205,9 +278,6 @@
205278
"type": "object"
206279
},
207280
"Choices": {
208-
"items": {
209-
"$ref": "#/definitions/choice/definitions/Operator"
210-
},
211281
"type": "array"
212282
},
213283
"Comment": {

‎src/cfnlint/rules/resources/stepfunctions/StateMachineDefinition.py

+17
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,23 @@ def _convert_schema_to_jsonata(self):
7272
},
7373
},
7474
]
75+
schema["definitions"]["choice"]["allOf"] = [
76+
{
77+
"properties": {
78+
"Choices": {
79+
"items": {
80+
"properties": {
81+
"Condition": {"type": "string"},
82+
"Next": {"pattern": "^.{1,128}$", "type": "string"},
83+
},
84+
"additionalProperties": False,
85+
"required": ["Condition", "Next"],
86+
"type": "object",
87+
}
88+
}
89+
}
90+
}
91+
]
7592
return schema
7693

7794
def _clean_schema(self, validator: Validator, instance: Any):

‎test/unit/rules/resources/stepfunctions/test_state_machine_definition.py

+167
Original file line numberDiff line numberDiff line change
@@ -757,6 +757,38 @@ def rule():
757757
}
758758
],
759759
},
760+
"Choices1": {
761+
"Type": "Choice",
762+
"Choices": [
763+
{
764+
"Condition": "BatchJobNotification",
765+
"Next": "Notify Success",
766+
}
767+
],
768+
"Next": "Notify Success",
769+
"QueryLanguage": "JSONata",
770+
},
771+
"Choices2": {
772+
"Type": "Choice",
773+
"Choices": [
774+
{
775+
"Condition": "BatchJobNotification",
776+
"Next": "Notify Success",
777+
}
778+
],
779+
"Next": "Notify Success",
780+
},
781+
"Choices3": {
782+
"Type": "Choice",
783+
"Choices": [
784+
{
785+
"And": "BatchJobNotification",
786+
"Next": "Notify Success",
787+
}
788+
],
789+
"Next": "Notify Success",
790+
"QueryLanguage": "JSONata",
791+
},
760792
"Notify Success": {
761793
"Type": "Task",
762794
"Resource": "arn:aws:states:::sns:publish",
@@ -857,6 +889,67 @@ def rule():
857889
["Definition", "States", "Submit Batch Job 3", "Arguments"]
858890
),
859891
),
892+
ValidationError(
893+
(
894+
"Only one of ['And', 'BooleanEquals', 'BooleanEqualsPath', 'IsBoolean', 'IsNull', "
895+
"'IsNumeric', 'IsPresent', 'IsString', 'IsTimestamp', 'Not', 'NumericEquals', "
896+
"'NumericEqualsPath', 'NumericGreaterThan', 'NumericGreaterThanPath', "
897+
"'NumericGreaterThanEquals', 'NumericGreaterThanEqualsPath', 'NumericLessThan', "
898+
"'NumericLessThanPath', 'NumericLessThanEquals', 'NumericLessThanEqualsPath', 'Or', "
899+
"'StringEquals', 'StringEqualsPath', 'StringGreaterThan', 'StringGreaterThanPath', "
900+
"'StringGreaterThanEquals', 'StringGreaterThanEqualsPath', 'StringLessThan', "
901+
"'StringLessThanPath', 'StringLessThanEquals', 'StringLessThanEqualsPath', "
902+
"'StringMatches', 'TimestampEquals', 'TimestampEqualsPath', 'TimestampGreaterThan', "
903+
"'TimestampGreaterThanPath', 'TimestampGreaterThanEquals', 'TimestampGreaterThanEqualsPath', "
904+
"'TimestampLessThan', 'TimestampLessThanPath', 'TimestampLessThanEquals', "
905+
"'TimestampLessThanEqualsPath'] is a required property"
906+
),
907+
rule=StateMachineDefinition(),
908+
validator="requiredXor",
909+
schema_path=deque(
910+
[
911+
"properties",
912+
"States",
913+
"patternProperties",
914+
"^.{1,128}$",
915+
"allOf",
916+
0,
917+
"then",
918+
"allOf",
919+
1,
920+
"then",
921+
"properties",
922+
"Choices",
923+
"items",
924+
"requiredXor",
925+
]
926+
),
927+
path=deque(["Definition", "States", "Choices2", "Choices", 0]),
928+
),
929+
ValidationError(
930+
("'Condition' is a required property"),
931+
rule=StateMachineDefinition(),
932+
validator="required",
933+
schema_path=deque(
934+
[
935+
"properties",
936+
"States",
937+
"patternProperties",
938+
"^.{1,128}$",
939+
"allOf",
940+
0,
941+
"then",
942+
"allOf",
943+
0,
944+
"then",
945+
"properties",
946+
"Choices",
947+
"items",
948+
"required",
949+
]
950+
),
951+
path=deque(["Definition", "States", "Choices3", "Choices", 0]),
952+
),
860953
],
861954
),
862955
(
@@ -932,6 +1025,26 @@ def rule():
9321025
},
9331026
"End": True,
9341027
},
1028+
"Choices1": {
1029+
"Type": "Choice",
1030+
"Choices": [
1031+
{
1032+
"Condition": "{% $states.input.type != 'Private' %}",
1033+
"Next": "Notify Failure",
1034+
},
1035+
],
1036+
"End": True,
1037+
},
1038+
"Choices2": {
1039+
"Type": "Choice",
1040+
"Choices": [
1041+
{
1042+
"And": "{% $states.input.type != 'Private' %}",
1043+
"Next": "Notify Failure",
1044+
},
1045+
],
1046+
"End": True,
1047+
},
9351048
"Notify Failure": {
9361049
"Type": "Task",
9371050
"Resource": "arn:aws:states:::sns:publish",
@@ -1019,6 +1132,54 @@ def rule():
10191132
["Definition", "States", "Notify Success", "Parameters"]
10201133
),
10211134
),
1135+
ValidationError(
1136+
("'Condition' is a required property"),
1137+
rule=StateMachineDefinition(),
1138+
validator="required",
1139+
schema_path=deque(
1140+
[
1141+
"properties",
1142+
"States",
1143+
"patternProperties",
1144+
"^.{1,128}$",
1145+
"allOf",
1146+
0,
1147+
"then",
1148+
"allOf",
1149+
0,
1150+
"properties",
1151+
"Choices",
1152+
"items",
1153+
"required",
1154+
]
1155+
),
1156+
path=deque(["Definition", "States", "Choices2", "Choices", 0]),
1157+
),
1158+
ValidationError(
1159+
("Additional properties are not allowed ('And' was unexpected)"),
1160+
rule=StateMachineDefinition(),
1161+
validator="additionalProperties",
1162+
schema_path=deque(
1163+
[
1164+
"properties",
1165+
"States",
1166+
"patternProperties",
1167+
"^.{1,128}$",
1168+
"allOf",
1169+
0,
1170+
"then",
1171+
"allOf",
1172+
0,
1173+
"properties",
1174+
"Choices",
1175+
"items",
1176+
"additionalProperties",
1177+
]
1178+
),
1179+
path=deque(
1180+
["Definition", "States", "Choices2", "Choices", 0, "And"]
1181+
),
1182+
),
10221183
ValidationError(
10231184
(
10241185
"Additional properties are not allowed ('Parameters' was unexpected)"
@@ -1056,4 +1217,10 @@ def test_validate(
10561217
validator,
10571218
):
10581219
errs = list(rule.validate(validator, {}, instance, {}))
1220+
for i, err in enumerate(errs):
1221+
print(i)
1222+
print(err.message, err.message == expected[i].message)
1223+
print(err.path, err.path == expected[i].path)
1224+
print(err.schema_path, err.schema_path == expected[i].schema_path)
1225+
print(err.validator, err.validator == expected[i].validator)
10591226
assert errs == expected, f"{name!r} test failed with {errs!r}"

0 commit comments

Comments
 (0)