Feature: Eventbridge v2: Add tagging #10840

maxhoheiser · 2024-05-16T12:14:38Z

Motivation

Event bridge allows for adding tags to event buses and rules: https://docs.aws.amazon.com/eventbridge/latest/userguide/eb-tagging.html
This PR introduces the ability to create event buses and rules with tags, add and remove tags to existing buses or rules and list tags for buses and rules.

Changes

Use most widely used pattern of handling tags from multiple other services by extend store with TagDict mapping resource arn to the dictionary of tags.
Exten test suite to cover all different options of tagging.

github-actions · 2024-05-16T13:12:10Z

LocalStack Community integration with Pro

2 files ± 0 2 suites ±0 1h 39m 6s ⏱️ -12s
2 979 tests +13 2 674 ✅ +9 305 💤 +4 0 ❌ ±0
2 981 runs +13 2 674 ✅ +9 307 💤 +4 0 ❌ ±0

Results for commit 8628c6f. ± Comparison against base commit 179a8e3.

This pull request removes 1 and adds 14 tests. Note that renamed tests count towards both.

tests.aws.services.events.test_events.TestEvents ‑ test_list_tags_for_resource

tests.aws.services.events.test_events_tags ‑ test_recreate_tagged_resource_without_tags[event_bus-event_bus_custom]
tests.aws.services.events.test_events_tags ‑ test_recreate_tagged_resource_without_tags[event_bus-event_bus_default]
tests.aws.services.events.test_events_tags ‑ test_recreate_tagged_resource_without_tags[rule-event_bus_custom]
tests.aws.services.events.test_events_tags ‑ test_recreate_tagged_resource_without_tags[rule-event_bus_default]
tests.aws.services.events.test_events_tags ‑ tests_tag_list_untag_not_existing_resource[not_existing_event_bus]
tests.aws.services.events.test_events_tags ‑ tests_tag_list_untag_not_existing_resource[not_existing_rule]
tests.aws.services.events.test_events_tags ‑ tests_tag_untag_resource[event_bus-event_bus_custom]
tests.aws.services.events.test_events_tags ‑ tests_tag_untag_resource[event_bus-event_bus_default]
tests.aws.services.events.test_events_tags ‑ tests_tag_untag_resource[rule-event_bus_custom]
tests.aws.services.events.test_events_tags ‑ tests_tag_untag_resource[rule-event_bus_default]
…

♻️ This comment has been updated with latest results.

joe4dev

The tests are quite comprehensive. Nice coverage 👍

My main question is whether it would be feasible to reuse the tagging service (maybe not because the resource type differentiation is too specific)?
I added some suggestions to simplify the code and resolve potential smaller bugs.

joe4dev · 2024-05-17T07:10:14Z

localstack/services/events/models.py


 class EventsStore(BaseStore):
    # Map of eventbus names to eventbus objects. The name MUST be unique per account and region (works with AccountRegionBundle)
    event_buses: EventBusDict = LocalAttribute(default=dict)

+    # Maps resource ARN to tags
+    TAGS: TagDict = CrossRegionAttribute(default=dict)


We have a TaggingService that implements tagging transparently (noticed by looking into DMS models as one of our new implementations). Is that applicable here or does different handling based on the resource type require a special case?

used TaggingService in: df9b76c

The only issue I see is, that the pattern other services use is to store the TagginService objects in the store, which violates a bit of what we discussed a couple of weeks ago that data and functions should be strictly separated. This comes back to the issue that the AcountRegionBundle needs additional logic that handles this separation which would make a lot of providers cleaner.

Very neat ✨
IMHO, that's a compromise worth taking vs. re-implementing the same thing for every service. The likelihood of a major change in tagging is very low.