From 307afbe464b9d66d9d14910e73919c751b9b1a51 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 12 May 2025 19:33:31 -0400 Subject: [PATCH] fix(pagerduty): apply several small PagerDuty migrator fixes (#5536) This PR applies the diff described in https://github.com/grafana/irm/issues/1863 to the PagerDuty migrator tool. Specifically, it: - Fixes the target type check for user references in service filtering - Ensures escalation policies are included in the PagerDuty API request - Corrects the logging output for service migration errors --- .../migrators/lib/pagerduty/resources/services.py | 11 ++++++----- .../tests/pagerduty/resources/test_services.py | 15 ++++++++++----- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/tools/migrators/lib/pagerduty/resources/services.py b/tools/migrators/lib/pagerduty/resources/services.py index e325cd7d..7be97702 100644 --- a/tools/migrators/lib/pagerduty/resources/services.py +++ b/tools/migrators/lib/pagerduty/resources/services.py @@ -49,7 +49,7 @@ def filter_services(services: List[Dict[str, Any]]) -> List[Dict[str, Any]]: if service.get("escalation_policy"): for rule in service["escalation_policy"].get("escalation_rules", []): for target in rule.get("targets", []): - if target["type"] == "user": + if target["type"] == "user_reference": service_users.add(target["id"]) if not any(user_id in service_users for user_id in PAGERDUTY_FILTER_USERS): @@ -163,6 +163,7 @@ def fetch_services( if include_teams: include_params.append("teams") + include_params.append("escalation_policies") params = {} if include_params: params["include[]"] = include_params @@ -570,7 +571,7 @@ def _migrate_technical_service( if errors: service.migration_errors = errors service.preserved = False - print(TAB + format_service(service, False)) + print(TAB + format_service(service, True)) return None if dry_run: @@ -589,7 +590,7 @@ def _migrate_technical_service( except Exception as e: service.migration_errors = str(e) service.preserved = False - print(TAB + format_service(service, False)) + print(TAB + format_service(service, True)) return None @@ -624,7 +625,7 @@ def _migrate_business_service( if errors: service.migration_errors = errors service.preserved = False - print(TAB + format_service(service, False)) + print(TAB + format_service(service, True)) return None if dry_run: @@ -643,7 +644,7 @@ def _migrate_business_service( except Exception as e: service.migration_errors = str(e) service.preserved = False - print(TAB + format_service(service, False)) + print(TAB + format_service(service, True)) return None diff --git a/tools/migrators/lib/tests/pagerduty/resources/test_services.py b/tools/migrators/lib/tests/pagerduty/resources/test_services.py index fb565df4..bc512a6a 100644 --- a/tools/migrators/lib/tests/pagerduty/resources/test_services.py +++ b/tools/migrators/lib/tests/pagerduty/resources/test_services.py @@ -50,8 +50,8 @@ def sample_services(): "escalation_rules": [ { "targets": [ - {"type": "user", "id": "U123"}, - {"type": "user", "id": "U456"}, + {"type": "user_reference", "id": "U123"}, + {"type": "user_reference", "id": "U456"}, ] } ] @@ -63,7 +63,9 @@ def sample_services(): "type": "service", "teams": [{"summary": "DevOps Team"}], "escalation_policy": { - "escalation_rules": [{"targets": [{"type": "user", "id": "U789"}]}] + "escalation_rules": [ + {"targets": [{"type": "user_reference", "id": "U789"}]} + ] }, }, { @@ -201,7 +203,8 @@ def test_fetch_services(mock_session): # Verify API call mock_session.list_all.assert_called_once_with( - "services", params={"include[]": ["integrations", "teams"]} + "services", + params={"include[]": ["integrations", "teams", "escalation_policies"]}, ) # Verify results @@ -220,7 +223,9 @@ def test_fetch_services_without_includes(mock_session): ) # Verify API call with no includes - mock_session.list_all.assert_called_once_with("services", params={}) + mock_session.list_all.assert_called_once_with( + "services", params={"include[]": ["escalation_policies"]} + ) # Verify results assert len(services) == 1