From 86022202ebbe90bbbfd2a65a30f581f093704147 Mon Sep 17 00:00:00 2001 From: Dan Hudlow Date: Thu, 11 Dec 2025 23:14:01 -0600 Subject: [PATCH 1/3] Fixes for error propagation --- features/error_propagation.feature | 122 +++++++++++++++++++++++++++++ src/celpy/celtypes.py | 2 + src/celpy/evaluation.py | 17 +++- 3 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 features/error_propagation.feature diff --git a/features/error_propagation.feature b/features/error_propagation.feature new file mode 100644 index 0000000..fe3378c --- /dev/null +++ b/features/error_propagation.feature @@ -0,0 +1,122 @@ +Feature: error_propagation + Tests to ensure errors propagate how they are supposed to + +Scenario: equal + + When CEL expression '{}.a == 1' is evaluated + Then eval_error is "no such member" + +Scenario: not_equal + + When CEL expression '{}.a != 1' is evaluated + Then eval_error is "no such member" + +Scenario: greater_than + + When CEL expression '{}.a > 1' is evaluated + Then eval_error is "no such member" + +Scenario: greater_than_or_equal + + When CEL expression '{}.a >= 1' is evaluated + Then eval_error is "no such member" + +Scenario: less_than + + When CEL expression '{}.a > 1' is evaluated + Then eval_error is "no such member" + +Scenario: less_than_or_equal + + When CEL expression '{}.a >= 1' is evaluated + Then eval_error is "no such member" + +Scenario: add + + When CEL expression '{}.a + 1' is evaluated + Then eval_error is "no such member" + +Scenario: subtract + + When CEL expression '{}.a - 1' is evaluated + Then eval_error is "no such member" + +Scenario: multiply + + When CEL expression '{}.a * 1' is evaluated + Then eval_error is "no such member" + +Scenario: divide + + When CEL expression '{}.a / 1' is evaluated + Then eval_error is "no such member" + +Scenario: modulo + + When CEL expression '{}.a % 1' is evaluated + Then eval_error is "no such member" + +Scenario: in + + When CEL expression '{}.a in [1]' is evaluated + Then eval_error is "no such member" + +Scenario: function + + When CEL expression 'size({}.a)' is evaluated + Then eval_error is "no such member" + +Scenario: method + + When CEL expression '{}.a.size()' is evaluated + Then eval_error is "no such member" + +Scenario: not + + When CEL expression '!{}.a' is evaluated + Then eval_error is "no such member" + +Scenario: and_error + + When CEL expression '{}.a && true' is evaluated + Then eval_error is "no such member" + +Scenario: and_ignore + + When CEL expression '{}.a && false' is evaluated + Then eval_error is None + +Scenario: or_error + + When CEL expression '{}.a || false' is evaluated + Then eval_error is "no such member" + +Scenario: or_ignore + + When CEL expression '{}.a || true' is evaluated + Then eval_error is None + +Scenario: all_error + + When CEL expression '[{"a": 1}, {}].all(v, v.a == 1)' is evaluated + Then eval_error is "no such member" + +Scenario: all_ignore + + When CEL expression '[{"a": 1}, {}].all(v, v.a == 2)' is evaluated + Then eval_error is None + +Scenario: exists_error + + When CEL expression '[{"a": 1}, {}].exists(v, v.a == 2)' is evaluated + Then eval_error is "no such member" + +Scenario: exists_ignore + + When CEL expression '[{"a": 1}, {}].exists(v, v.a == 1)' is evaluated + Then eval_error is None + +Scenario: exists_one_error + + When CEL expression '[{"a": 1}, {}].exists_one(v, v.a == 1)' is evaluated + Then eval_error is "no such member" diff --git a/src/celpy/celtypes.py b/src/celpy/celtypes.py index 0d1bf85..33708a0 100644 --- a/src/celpy/celtypes.py +++ b/src/celpy/celtypes.py @@ -334,6 +334,8 @@ def logical_not(x: Value) -> Value: This could almost be `logical_or = evaluation.boolean(operator.not_)`, but the definition would expose Python's notion of "truthiness", which isn't appropriate for CEL. """ + if isinstance(x, Exception): + return x if isinstance(x, BoolType): result_value = BoolType(not x) else: diff --git a/src/celpy/evaluation.py b/src/celpy/evaluation.py index ac0e109..528b67f 100644 --- a/src/celpy/evaluation.py +++ b/src/celpy/evaluation.py @@ -291,6 +291,11 @@ def boolean( def bool_function( a: celpy.celtypes.Value, b: celpy.celtypes.Value ) -> celpy.celtypes.BoolType: + if isinstance(a, CELEvalError): + return a + if isinstance(b, CELEvalError): + return b + result_value = function(a, b) if result_value == NotImplemented: return cast(celpy.celtypes.BoolType, result_value) @@ -323,7 +328,9 @@ def operator_in(item: Result, container: Result) -> Result: - True. There was a item found. Exceptions may or may not have been found. - False. No item found AND no exceptions. - - CELEvalError. No item found AND at least one exception. + - CELEvalError. Either: + - No item found AND at least one exception or + - The input item or container itself was already an error To an extent this is a little like the ``exists()`` macro. 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: ``reduce(logical_or, (item == c for c in container), BoolType(False))`` """ + if isinstance(item, CELEvalError): + return item + if isinstance(container, CELEvalError): + return container + result_value: Result = celpy.celtypes.BoolType(False) for c in cast(Iterable[Result], container): try: @@ -1547,6 +1559,9 @@ def function_eval( try: list_exprlist = cast(List[Result], exprlist or []) + for expr in list_exprlist: + if isinstance(expr, CELEvalError): + return expr return function(*list_exprlist) except ValueError as ex: value = CELEvalError( From 39ef5d856c7a808e911a63cd559820c940e7e676 Mon Sep 17 00:00:00 2001 From: Dan Hudlow Date: Fri, 19 Dec 2025 17:12:53 -0600 Subject: [PATCH 2/3] Fix for redundant cast --- src/celpy/celtypes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/celpy/celtypes.py b/src/celpy/celtypes.py index 33708a0..d9e7548 100644 --- a/src/celpy/celtypes.py +++ b/src/celpy/celtypes.py @@ -443,7 +443,7 @@ def __new__( cast(bytes, source.get(StringType("value"))), ) elif isinstance(source, Iterable): - return super().__new__(cls, cast(Iterable[int], source)) + return super().__new__(cls, source) else: raise TypeError(f"Invalid initial value type: {type(source)}") From da920bee972a890111188b7c8cfde39802c0c072 Mon Sep 17 00:00:00 2001 From: Dan Hudlow Date: Fri, 19 Dec 2025 22:22:56 -0600 Subject: [PATCH 3/3] Fix flow logs feature --- features/c7n_interface.feature | 8 ++++---- features/error_propagation.feature | 4 ++++ src/celpy/c7nlib.py | 6 +++--- src/xlate/c7n_to_cel.py | 33 +++++++----------------------- tests/test_c7n_to_cel.py | 6 +++--- type_check/lineprecision.txt | 6 +++--- 6 files changed, 24 insertions(+), 39 deletions(-) diff --git a/features/c7n_interface.feature b/features/c7n_interface.feature index 91dc5c0..3a8a281 100644 --- a/features/c7n_interface.feature +++ b/features/c7n_interface.feature @@ -950,7 +950,7 @@ Examples: subnet False Scenario Outline: Some resource types (vpc, eni, and subnet) have flow-log settings. C7N can check a variety of attributes: destination, destination-type, enabled, - log-group, status, and traffic-type. Pragmatically, we see only enabled and desination-type + log-group, status, and traffic-type. Pragmatically, we see only enabled and destination-type Given policy text """ @@ -969,12 +969,12 @@ Scenario Outline: Some resource types (vpc, eni, and subnet) have flow-log setti And C7N.filter manager has get_model result of InstanceId And C7N.filter has flow_logs result with When CEL filter is built and evaluated + Then CEL text is size(resource.flow_logs()) == 0 || ! (size(resource.flow_logs()) != 0 && (resource.flow_logs().exists(x, x.LogDestinationType == "s3"))) Then result is - And CEL text is size(resource.flow_logs()) == 0 || ! (size(resource.flow_logs()) != 0 && (resource.flow_logs().LogDestinationType == "s3")) -Examples: low-logs True +Examples: flow-logs True | expected | document | flow-logs | - | True | {"InstanceId": "i-123456789", "ResourceType": "vpc"} | [{"ResourceId": "i-123456789", "More": "Details"}] | + | True | {"InstanceId": "i-123456789", "ResourceType": "vpc"} | [{"ResourceId": "i-123456789", "LogDestinationType": "cloud-watch-logs"}] | ###################### diff --git a/features/error_propagation.feature b/features/error_propagation.feature index fe3378c..81315f3 100644 --- a/features/error_propagation.feature +++ b/features/error_propagation.feature @@ -85,6 +85,7 @@ Scenario: and_ignore When CEL expression '{}.a && false' is evaluated Then eval_error is None + And value is celpy.celtypes.BoolType(source=False) Scenario: or_error @@ -95,6 +96,7 @@ Scenario: or_ignore When CEL expression '{}.a || true' is evaluated Then eval_error is None + And value is celpy.celtypes.BoolType(source=True) Scenario: all_error @@ -105,6 +107,7 @@ Scenario: all_ignore When CEL expression '[{"a": 1}, {}].all(v, v.a == 2)' is evaluated Then eval_error is None + And value is celpy.celtypes.BoolType(source=False) Scenario: exists_error @@ -115,6 +118,7 @@ Scenario: exists_ignore When CEL expression '[{"a": 1}, {}].exists(v, v.a == 1)' is evaluated Then eval_error is None + And value is celpy.celtypes.BoolType(source=True) Scenario: exists_one_error diff --git a/src/celpy/c7nlib.py b/src/celpy/c7nlib.py index b9120af..ef8d07c 100644 --- a/src/celpy/c7nlib.py +++ b/src/celpy/c7nlib.py @@ -997,12 +997,12 @@ def flow_logs( # TODO: Refactor into a function in ``CELFilter``. Should not be here. client = C7N.filter.manager.session_factory().client("ec2") logs = client.describe_flow_logs().get("FlowLogs", ()) - m = C7N.filter.manager.get_model() + dimension = C7N.filter.manager.get_model().dimension resource_map: Dict[str, List[Dict[str, Any]]] = {} for fl in logs: resource_map.setdefault(fl["ResourceId"], []).append(fl) - if resource.get(m.id) in resource_map: - flogs = resource_map[cast(str, resource.get(m.id))] + if resource.get(dimension) in resource_map: + flogs = resource_map[cast(str, resource.get(dimension))] return json_to_cel(flogs) return json_to_cel([]) diff --git a/src/xlate/c7n_to_cel.py b/src/xlate/c7n_to_cel.py index 4971ae9..6d2967b 100644 --- a/src/xlate/c7n_to_cel.py +++ b/src/xlate/c7n_to_cel.py @@ -877,7 +877,6 @@ def type_flow_log_rewrite(resource: str, c7n_filter: Dict[str, Any]) -> str: Relies on :py:func:`celpy.c7nlib.flow_logs` to get flow_log details via the C7N Filter. """ - op = c7n_filter.get("op", "equal") set_op = c7n_filter.get("set-up", "or") enabled = [] if "enabled" in c7n_filter: @@ -890,55 +889,37 @@ def type_flow_log_rewrite(resource: str, c7n_filter: Dict[str, Any]) -> str: if c7n_filter.get("log-group"): log_group = c7n_filter.get("log-group") clauses.append( - C7N_Rewriter.atomic_op_map[op].format( - "resource.flow_logs().LogGroupName", f"{C7N_Rewriter.q(log_group)}" - ) + f"resource.flow_logs().exists(x, x.LogGroupName == {C7N_Rewriter.q(log_group)})" ) if c7n_filter.get("log-format"): log_format = c7n_filter.get("log-format") clauses.append( - C7N_Rewriter.atomic_op_map[op].format( - "resource.flow_logs().LogFormat", f"{C7N_Rewriter.q(log_format)}" - ) + f"resource.flow_logs().exists(x, x.LogFormat == {C7N_Rewriter.q(log_format)})" ) if c7n_filter.get("traffic-type"): traffic_type = cast(str, c7n_filter.get("traffic-type")) clauses.append( - C7N_Rewriter.atomic_op_map[op].format( - "resource.flow_logs().TrafficType", - f"{C7N_Rewriter.q(traffic_type.upper())}", - ) + f"resource.flow_logs().exists(x, x.TrafficType == {C7N_Rewriter.q(traffic_type.upper())})" ) if c7n_filter.get("destination-type"): destination_type = c7n_filter.get("destination-type") clauses.append( - C7N_Rewriter.atomic_op_map[op].format( - "resource.flow_logs().LogDestinationType", - f"{C7N_Rewriter.q(destination_type)}", - ) + f"resource.flow_logs().exists(x, x.LogDestinationType == {C7N_Rewriter.q(destination_type)})" ) if c7n_filter.get("destination"): destination = c7n_filter.get("destination") clauses.append( - C7N_Rewriter.atomic_op_map[op].format( - "resource.flow_logs().LogDestination", - f"{C7N_Rewriter.q(destination)}", - ) + f"resource.flow_logs().exists(x, x.LogDestination == {C7N_Rewriter.q(destination)})" ) if c7n_filter.get("status"): status = c7n_filter.get("status") clauses.append( - C7N_Rewriter.atomic_op_map[op].format( - "resource.flow_logs().FlowLogStatus", f"{C7N_Rewriter.q(status)}" - ) + f"resource.flow_logs().exists(x, x.FlowLogStatus == {C7N_Rewriter.q(status)})" ) if c7n_filter.get("deliver-status"): deliver_status = c7n_filter.get("deliver-status") clauses.append( - C7N_Rewriter.atomic_op_map[op].format( - "resource.flow_logs().DeliverLogsStatus", - f"{C7N_Rewriter.q(deliver_status)}", - ) + f"resource.flow_logs().exists(x, x.DeliverLogsStatus == {C7N_Rewriter.q(deliver_status)})" ) if len(clauses) > 0: diff --git a/tests/test_c7n_to_cel.py b/tests/test_c7n_to_cel.py index 1acbf58..85b68c2 100644 --- a/tests/test_c7n_to_cel.py +++ b/tests/test_c7n_to_cel.py @@ -459,18 +459,18 @@ def test_flow_logs_rewrite(): clause_1 = { "enabled": "true", "type": "flow-logs", "destination-type": "s3", } - expected = 'size(resource.flow_logs()) != 0 && (resource.flow_logs().LogDestinationType == "s3")' + expected = 'size(resource.flow_logs()) != 0 && (resource.flow_logs().exists(x, x.LogDestinationType == "s3"))' assert C7N_Rewriter.type_flow_log_rewrite("vpc", clause_1) == expected clause_2 = {'type': 'flow-logs', 'enabled': True, 'set-op': 'or', 'op': 'equal', 'traffic-type': 'all', 'status': 'active', 'log-group': 'vpc-logs'} - expected = 'size(resource.flow_logs()) != 0 && (resource.flow_logs().LogGroupName == "vpc-logs" || resource.flow_logs().TrafficType == "ALL" || resource.flow_logs().FlowLogStatus == "active")' + 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"))' assert C7N_Rewriter.type_flow_log_rewrite("vpc", clause_2) == expected clause_3 = {'type': 'flow-logs', 'enabled': True, "log-format": "this", "destination": "that", "deliver-status": "the-other-thing"} - expected = 'size(resource.flow_logs()) != 0 && (resource.flow_logs().LogFormat == "this" || resource.flow_logs().LogDestination == "that" || resource.flow_logs().DeliverLogsStatus == "the-other-thing")' + 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"))' assert C7N_Rewriter.type_flow_log_rewrite("vpc", clause_3) == expected diff --git a/type_check/lineprecision.txt b/type_check/lineprecision.txt index 2b1f945..a2be391 100644 --- a/type_check/lineprecision.txt +++ b/type_check/lineprecision.txt @@ -5,9 +5,9 @@ celpy.__main__ 551 243 12 63 233 0 celpy.adapter 163 34 3 9 111 6 celpy.c7nlib 1663 344 16 152 1151 0 celpy.celparser 411 136 68 23 184 0 -celpy.celtypes 1483 386 15 235 810 37 -celpy.evaluation 3859 1127 252 242 2222 16 +celpy.celtypes 1497 394 15 238 812 38 +celpy.evaluation 3874 1135 252 243 2226 18 gherkinize 1142 531 14 96 481 20 test_gherkinize 5581 5014 135 4 428 0 xlate 0 0 0 0 0 0 -xlate.c7n_to_cel 1755 387 103 144 1115 6 +xlate.c7n_to_cel 1736 383 103 136 1108 6