Skip to content

eventbridge: fix handling of list elements #10600

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
baermat merged 5 commits into from
Apr 8, 2024
Merged

eventbridge: fix handling of list elements #10600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a429d0d
fix handling of list elements
baermat Apr 3, 2024
4b3aca9
add snapshot.json
baermat Apr 3, 2024
1d81f07
trigger rerun
baermat Apr 3, 2024
b324a55
move patch to utils.py, work in review suggestions
baermat Apr 8, 2024
9326b86
Add regression test to new test suite
joe4dev Apr 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions localstack/services/events/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,16 @@ def matches_event(event_pattern: dict[str, any], event: dict[str, Any]) -> bool:
):
return False

# 3. recursively call filter_event(..) for dict types
# 3. recursively call matches_event(..) for dict types
Copy link
Member

@joe4dev joe4dev Apr 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch. Thank you @baermat 🙏

elif isinstance(value, (str, dict)):
try:
# TODO: validate whether inner JSON-encoded strings actually get decoded recursively
value = json.loads(value) if isinstance(value, str) else value
if isinstance(value, dict) and not matches_event(value, event_value):
return False
if isinstance(event_value, list):
return any(matches_event(value, ev) for ev in event_value)
else:
if isinstance(value, dict) and not matches_event(value, event_value):
return False
except json.decoder.JSONDecodeError:
return False

Expand Down
10 changes: 6 additions & 4 deletions localstack/utils/testutil.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,10 +573,12 @@ def get_log_events(func_name, delay):
raw_message = event["message"]
if (
not raw_message
or "START" in raw_message
or "END" in raw_message
or "REPORT" in raw_message
# necessary until tail is updated in docker images. See this PR:
or raw_message.startswith("INIT_START")
or raw_message.startswith("START")
or raw_message.startswith("END")
or raw_message.startswith(
"REPORT"
) # necessary until tail is updated in docker images. See this PR:
Copy link
Member

@joe4dev joe4dev Apr 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That PR links a patch from 2015 🤔 Is there a better open issue we could link to track this?

Copy link
Member Author

@baermat baermat Apr 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to admit that I didn't check this PR at all. The fix here is mostly that a message can contain the words we filter for by accident (which is exactly what happened), while we should only filter for these words at the start

Copy link
Member

@joe4dev joe4dev Apr 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the clarification. Would be a nice-to-have clarifying comment 👍

# http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v8.24-111-g1118f32
or "tail: unrecognized file system type" in raw_message
or regex_filter
Expand Down
26 changes: 26 additions & 0 deletions tests/aws/services/events/event_pattern_templates/list_within_dict.json5
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Motivated by https://github.com/localstack/localstack/pull/10600
{
"Event": {
"id": "1",
"source": "test-source",
"detail-type": "test-detail-type",
"account": "123456789012",
"region": "us-east-2",
"time": "2022-07-13T13:48:01Z",
"detail": {
"automations": [
{"key1": "value1"},
// the "exists" operator matches because at least one element of the list matches
{"id": "match-does-exist"},
{"key2": "value2"}
]
}
},
"EventPattern": {
"detail": {
"automations": {
"id": [{"exists": true}]
}
}
}
}
Loading