Skip to content

Commit 3a134c1

Browse files
hudlowhudlow
authored
Fixes for error propagation (#143)
* Fixes for error propagation * Fix for redundant cast * Fix flow logs feature
1 parent ec761db commit 3a134c1

File tree

8 files changed

+164
-40
lines changed

8 files changed

+164
-40
lines changed

‎features/c7n_interface.feature‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -950,7 +950,7 @@ Examples: subnet False
950950

951951
Scenario Outline: Some resource types (vpc, eni, and subnet) have flow-log settings.
952952
C7N can check a variety of attributes: destination, destination-type, enabled,
953-
log-group, status, and traffic-type. Pragmatically, we see only enabled and desination-type
953+
log-group, status, and traffic-type. Pragmatically, we see only enabled and destination-type
954954

955955
Given policy text
956956
"""
@@ -969,12 +969,12 @@ Scenario Outline: Some resource types (vpc, eni, and subnet) have flow-log setti
969969
And C7N.filter manager has get_model result of InstanceId
970970
And C7N.filter has flow_logs result with <flow-logs>
971971
When CEL filter is built and evaluated
972+
Then CEL text is size(resource.flow_logs()) == 0 || ! (size(resource.flow_logs()) != 0 && (resource.flow_logs().exists(x, x.LogDestinationType == "s3")))
972973
Then result is <expected>
973-
And CEL text is size(resource.flow_logs()) == 0 || ! (size(resource.flow_logs()) != 0 && (resource.flow_logs().LogDestinationType == "s3"))
974974

975-
Examples: low-logs True
975+
Examples: flow-logs True
976976
| expected | document | flow-logs |
977-
| True | {"InstanceId": "i-123456789", "ResourceType": "vpc"} | [{"ResourceId": "i-123456789", "More": "Details"}] |
977+
| True | {"InstanceId": "i-123456789", "ResourceType": "vpc"} | [{"ResourceId": "i-123456789", "LogDestinationType": "cloud-watch-logs"}] |
978978

979979

980980
######################

‎features/error_propagation.feature‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
Feature: error_propagation
2+
Tests to ensure errors propagate how they are supposed to
3+
4+
Scenario: equal
5+
6+
When CEL expression '{}.a == 1' is evaluated
7+
Then eval_error is "no such member"
8+
9+
Scenario: not_equal
10+
11+
When CEL expression '{}.a != 1' is evaluated
12+
Then eval_error is "no such member"
13+
14+
Scenario: greater_than
15+
16+
When CEL expression '{}.a > 1' is evaluated
17+
Then eval_error is "no such member"
18+
19+
Scenario: greater_than_or_equal
20+
21+
When CEL expression '{}.a >= 1' is evaluated
22+
Then eval_error is "no such member"
23+
24+
Scenario: less_than
25+
26+
When CEL expression '{}.a > 1' is evaluated
27+
Then eval_error is "no such member"
28+
29+
Scenario: less_than_or_equal
30+
31+
When CEL expression '{}.a >= 1' is evaluated
32+
Then eval_error is "no such member"
33+
34+
Scenario: add
35+
36+
When CEL expression '{}.a + 1' is evaluated
37+
Then eval_error is "no such member"
38+
39+
Scenario: subtract
40+
41+
When CEL expression '{}.a - 1' is evaluated
42+
Then eval_error is "no such member"
43+
44+
Scenario: multiply
45+
46+
When CEL expression '{}.a * 1' is evaluated
47+
Then eval_error is "no such member"
48+
49+
Scenario: divide
50+
51+
When CEL expression '{}.a / 1' is evaluated
52+
Then eval_error is "no such member"
53+
54+
Scenario: modulo
55+
56+
When CEL expression '{}.a % 1' is evaluated
57+
Then eval_error is "no such member"
58+
59+
Scenario: in
60+
61+
When CEL expression '{}.a in [1]' is evaluated
62+
Then eval_error is "no such member"
63+
64+
Scenario: function
65+
66+
When CEL expression 'size({}.a)' is evaluated
67+
Then eval_error is "no such member"
68+
69+
Scenario: method
70+
71+
When CEL expression '{}.a.size()' is evaluated
72+
Then eval_error is "no such member"
73+
74+
Scenario: not
75+
76+
When CEL expression '!{}.a' is evaluated
77+
Then eval_error is "no such member"
78+
79+
Scenario: and_error
80+
81+
When CEL expression '{}.a && true' is evaluated
82+
Then eval_error is "no such member"
83+
84+
Scenario: and_ignore
85+
86+
When CEL expression '{}.a && false' is evaluated
87+
Then eval_error is None
88+
And value is celpy.celtypes.BoolType(source=False)
89+
90+
Scenario: or_error
91+
92+
When CEL expression '{}.a || false' is evaluated
93+
Then eval_error is "no such member"
94+
95+
Scenario: or_ignore
96+
97+
When CEL expression '{}.a || true' is evaluated
98+
Then eval_error is None
99+
And value is celpy.celtypes.BoolType(source=True)
100+
101+
Scenario: all_error
102+
103+
When CEL expression '[{"a": 1}, {}].all(v, v.a == 1)' is evaluated
104+
Then eval_error is "no such member"
105+
106+
Scenario: all_ignore
107+
108+
When CEL expression '[{"a": 1}, {}].all(v, v.a == 2)' is evaluated
109+
Then eval_error is None
110+
And value is celpy.celtypes.BoolType(source=False)
111+
112+
Scenario: exists_error
113+
114+
When CEL expression '[{"a": 1}, {}].exists(v, v.a == 2)' is evaluated
115+
Then eval_error is "no such member"
116+
117+
Scenario: exists_ignore
118+
119+
When CEL expression '[{"a": 1}, {}].exists(v, v.a == 1)' is evaluated
120+
Then eval_error is None
121+
And value is celpy.celtypes.BoolType(source=True)
122+
123+
Scenario: exists_one_error
124+
125+
When CEL expression '[{"a": 1}, {}].exists_one(v, v.a == 1)' is evaluated
126+
Then eval_error is "no such member"

‎src/celpy/c7nlib.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -997,12 +997,12 @@ def flow_logs(
997997
# TODO: Refactor into a function in ``CELFilter``. Should not be here.
998998
client = C7N.filter.manager.session_factory().client("ec2")
999999
logs = client.describe_flow_logs().get("FlowLogs", ())
1000-
m = C7N.filter.manager.get_model()
1000+
dimension = C7N.filter.manager.get_model().dimension
10011001
resource_map: Dict[str, List[Dict[str, Any]]] = {}
10021002
for fl in logs:
10031003
resource_map.setdefault(fl["ResourceId"], []).append(fl)
1004-
if resource.get(m.id) in resource_map:
1005-
flogs = resource_map[cast(str, resource.get(m.id))]
1004+
if resource.get(dimension) in resource_map:
1005+
flogs = resource_map[cast(str, resource.get(dimension))]
10061006
return json_to_cel(flogs)
10071007
return json_to_cel([])
10081008

‎src/celpy/celtypes.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,8 @@ def logical_not(x: Value) -> Value:
334334
This could almost be `logical_or = evaluation.boolean(operator.not_)`,
335335
but the definition would expose Python's notion of "truthiness", which isn't appropriate for CEL.
336336
"""
337+
if isinstance(x, Exception):
338+
return x
337339
if isinstance(x, BoolType):
338340
result_value = BoolType(not x)
339341
else:

‎src/celpy/evaluation.py‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,11 @@ def boolean(
291291
def bool_function(
292292
a: celpy.celtypes.Value, b: celpy.celtypes.Value
293293
) -> celpy.celtypes.BoolType:
294+
if isinstance(a, CELEvalError):
295+
return a
296+
if isinstance(b, CELEvalError):
297+
return b
298+
294299
result_value = function(a, b)
295300
if result_value == NotImplemented:
296301
return cast(celpy.celtypes.BoolType, result_value)
@@ -323,7 +328,9 @@ def operator_in(item: Result, container: Result) -> Result:
323328
324329
- True. There was a item found. Exceptions may or may not have been found.
325330
- False. No item found AND no exceptions.
326-
- CELEvalError. No item found AND at least one exception.
331+
- CELEvalError. Either:
332+
- No item found AND at least one exception or
333+
- The input item or container itself was already an error
327334
328335
To an extent this is a little like the ``exists()`` macro.
329336
We can think of ``container.contains(item)`` as ``container.exists(r, r == item)``.
@@ -333,6 +340,11 @@ def operator_in(item: Result, container: Result) -> Result:
333340
334341
``reduce(logical_or, (item == c for c in container), BoolType(False))``
335342
"""
343+
if isinstance(item, CELEvalError):
344+
return item
345+
if isinstance(container, CELEvalError):
346+
return container
347+
336348
result_value: Result = celpy.celtypes.BoolType(False)
337349
for c in cast(Iterable[Result], container):
338350
try:
@@ -1547,6 +1559,9 @@ def function_eval(
15471559

15481560
try:
15491561
list_exprlist = cast(List[Result], exprlist or [])
1562+
for expr in list_exprlist:
1563+
if isinstance(expr, CELEvalError):
1564+
return expr
15501565
return function(*list_exprlist)
15511566
except ValueError as ex:
15521567
value = CELEvalError(

‎src/xlate/c7n_to_cel.py‎

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -877,7 +877,6 @@ def type_flow_log_rewrite(resource: str, c7n_filter: Dict[str, Any]) -> str:
877877
878878
Relies on :py:func:`celpy.c7nlib.flow_logs` to get flow_log details via the C7N Filter.
879879
"""
880-
op = c7n_filter.get("op", "equal")
881880
set_op = c7n_filter.get("set-up", "or")
882881
enabled = []
883882
if "enabled" in c7n_filter:
@@ -890,55 +889,37 @@ def type_flow_log_rewrite(resource: str, c7n_filter: Dict[str, Any]) -> str:
890889
if c7n_filter.get("log-group"):
891890
log_group = c7n_filter.get("log-group")
892891
clauses.append(
893-
C7N_Rewriter.atomic_op_map[op].format(
894-
"resource.flow_logs().LogGroupName", f"{C7N_Rewriter.q(log_group)}"
895-
)
892+
f"resource.flow_logs().exists(x, x.LogGroupName == {C7N_Rewriter.q(log_group)})"
896893
)
897894
if c7n_filter.get("log-format"):
898895
log_format = c7n_filter.get("log-format")
899896
clauses.append(
900-
C7N_Rewriter.atomic_op_map[op].format(
901-
"resource.flow_logs().LogFormat", f"{C7N_Rewriter.q(log_format)}"
902-
)
897+
f"resource.flow_logs().exists(x, x.LogFormat == {C7N_Rewriter.q(log_format)})"
903898
)
904899
if c7n_filter.get("traffic-type"):
905900
traffic_type = cast(str, c7n_filter.get("traffic-type"))
906901
clauses.append(
907-
C7N_Rewriter.atomic_op_map[op].format(
908-
"resource.flow_logs().TrafficType",
909-
f"{C7N_Rewriter.q(traffic_type.upper())}",
910-
)
902+
f"resource.flow_logs().exists(x, x.TrafficType == {C7N_Rewriter.q(traffic_type.upper())})"
911903
)
912904
if c7n_filter.get("destination-type"):
913905
destination_type = c7n_filter.get("destination-type")
914906
clauses.append(
915-
C7N_Rewriter.atomic_op_map[op].format(
916-
"resource.flow_logs().LogDestinationType",
917-
f"{C7N_Rewriter.q(destination_type)}",
918-
)
907+
f"resource.flow_logs().exists(x, x.LogDestinationType == {C7N_Rewriter.q(destination_type)})"
919908
)
920909
if c7n_filter.get("destination"):
921910
destination = c7n_filter.get("destination")
922911
clauses.append(
923-
C7N_Rewriter.atomic_op_map[op].format(
924-
"resource.flow_logs().LogDestination",
925-
f"{C7N_Rewriter.q(destination)}",
926-
)
912+
f"resource.flow_logs().exists(x, x.LogDestination == {C7N_Rewriter.q(destination)})"
927913
)
928914
if c7n_filter.get("status"):
929915
status = c7n_filter.get("status")
930916
clauses.append(
931-
C7N_Rewriter.atomic_op_map[op].format(
932-
"resource.flow_logs().FlowLogStatus", f"{C7N_Rewriter.q(status)}"
933-
)
917+
f"resource.flow_logs().exists(x, x.FlowLogStatus == {C7N_Rewriter.q(status)})"
934918
)
935919
if c7n_filter.get("deliver-status"):
936920
deliver_status = c7n_filter.get("deliver-status")
937921
clauses.append(
938-
C7N_Rewriter.atomic_op_map[op].format(
939-
"resource.flow_logs().DeliverLogsStatus",
940-
f"{C7N_Rewriter.q(deliver_status)}",
941-
)
922+
f"resource.flow_logs().exists(x, x.DeliverLogsStatus == {C7N_Rewriter.q(deliver_status)})"
942923
)
943924

944925
if len(clauses) > 0:

‎tests/test_c7n_to_cel.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -459,18 +459,18 @@ def test_flow_logs_rewrite():
459459
clause_1 = {
460460
"enabled": "true", "type": "flow-logs", "destination-type": "s3",
461461
}
462-
expected = 'size(resource.flow_logs()) != 0 && (resource.flow_logs().LogDestinationType == "s3")'
462+
expected = 'size(resource.flow_logs()) != 0 && (resource.flow_logs().exists(x, x.LogDestinationType == "s3"))'
463463
assert C7N_Rewriter.type_flow_log_rewrite("vpc", clause_1) == expected
464464

465465
clause_2 = {'type': 'flow-logs', 'enabled': True,
466466
'set-op': 'or', 'op': 'equal', 'traffic-type': 'all', 'status': 'active',
467467
'log-group': 'vpc-logs'}
468-
expected = 'size(resource.flow_logs()) != 0 && (resource.flow_logs().LogGroupName == "vpc-logs" || resource.flow_logs().TrafficType == "ALL" || resource.flow_logs().FlowLogStatus == "active")'
468+
expected = 'size(resource.flow_logs()) != 0 && (resource.flow_logs().exists(x, x.LogGroupName == "vpc-logs") || resource.flow_logs().exists(x, x.TrafficType == "ALL") || resource.flow_logs().exists(x, x.FlowLogStatus == "active"))'
469469
assert C7N_Rewriter.type_flow_log_rewrite("vpc", clause_2) == expected
470470

471471
clause_3 = {'type': 'flow-logs', 'enabled': True,
472472
"log-format": "this", "destination": "that", "deliver-status": "the-other-thing"}
473-
expected = 'size(resource.flow_logs()) != 0 && (resource.flow_logs().LogFormat == "this" || resource.flow_logs().LogDestination == "that" || resource.flow_logs().DeliverLogsStatus == "the-other-thing")'
473+
expected = 'size(resource.flow_logs()) != 0 && (resource.flow_logs().exists(x, x.LogFormat == "this") || resource.flow_logs().exists(x, x.LogDestination == "that") || resource.flow_logs().exists(x, x.DeliverLogsStatus == "the-other-thing"))'
474474
assert C7N_Rewriter.type_flow_log_rewrite("vpc", clause_3) == expected
475475

476476

‎type_check/lineprecision.txt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ celpy.__main__ 551 243 12 63 233 0
55
celpy.adapter 163 34 3 9 111 6
66
celpy.c7nlib 1663 344 16 152 1151 0
77
celpy.celparser 411 136 68 23 184 0
8-
celpy.celtypes 1483 386 15 235 810 37
9-
celpy.evaluation 3859 1127 252 242 2222 16
8+
celpy.celtypes 1497 394 15 238 812 38
9+
celpy.evaluation 3874 1135 252 243 2226 18
1010
gherkinize 1142 531 14 96 481 20
1111
test_gherkinize 5581 5014 135 4 428 0
1212
xlate 0 0 0 0 0 0
13-
xlate.c7n_to_cel 1755 387 103 144 1115 6
13+
xlate.c7n_to_cel 1736 383 103 136 1108 6

0 commit comments

Comments
 (0)