Skip to content

Commit b906140

Browse files
committed
Fix user filtering in PagerDuty migrator when MIGRATE_USERS=true. When PAGERDUTY_FILTER_USERS is set and MIGRATE_USERS=true, now only the specified users will be migrated. Added filter_users function, updated migrate function, added tests, and updated documentation.
1 parent e01f69d commit b906140

File tree

3 files changed

+107
-2
lines changed

3 files changed

+107
-2
lines changed

tools/migrators/README.md

+7-2
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,11 @@ The migrator will include:
238238
- Resources associated with USER1 or USER2
239239
- Resources that match both criteria
240240

241+
Additionally, when `MIGRATE_USERS` is set to `true` and `PAGERDUTY_FILTER_USERS` is specified,
242+
only the users with the specified PagerDuty IDs will be migrated. This allows for selective user
243+
migration, which is useful when you want to test the migration with a small set of users before
244+
migrating all users.
245+
241246
This allows for more flexible and intuitive filtering when migrating specific subsets of your PagerDuty setup.
242247

243248
### Output Verbosity
@@ -276,8 +281,8 @@ Configuration is done via environment variables passed to the docker container.
276281
| `EXPERIMENTAL_MIGRATE_EVENT_RULES_LONG_NAMES` | Include service & integrations names from PD in migrated integrations (only effective when `EXPERIMENTAL_MIGRATE_EVENT_RULES` is `true`). | Boolean | `false` |
277282
| `MIGRATE_USERS` | If `false`, will allow you to important all objects, while ignoring user references in schedules and escalation policies. In addition, if `false`, will also skip importing User notification rules. This may be helpful in cases where you are unable to import your list of Grafana users, but would like to experiment with OnCall using your existing PagerDuty setup as a starting point for experimentation. | Boolean | `true` |
278283
| `PAGERDUTY_FILTER_TEAM` | Filter resources by team name. Resources associated with this team will be included in the migration. | String | N/A |
279-
| `PAGERDUTY_FILTER_USERS` | Filter resources by PagerDuty user IDs (comma-separated). Resources associated with any of these users will be included in the migration. | String | N/A |
280-
| `PAGERDUTY_FILTER_SCHEDULE_REGEX` | Filter schedules by name using a regex pattern. Schedules whose names match this pattern will be included in the migration. | String | N/A |
284+
| `PAGERDUTY_FILTER_USERS` | Filter by PagerDuty user IDs (comma-separated). This serves two purposes: 1) Resources associated with any of these users will be included in the migration, and 2) When `MIGRATE_USERS` is `true`, only these specific users will be migrated (not all users). | String | N/A |
285+
| `PAGERDUTY_FILTER_SCHEDULE_REGEX` | Filter schedules by name using a regex pattern. Schedules whose names match this pattern will be included in the migration. | String | N/A |
281286
| `PAGERDUTY_FILTER_ESCALATION_POLICY_REGEX` | Filter escalation policies by name using a regex pattern. Policies whose names match this pattern will be included in the migration. | String | N/A |
282287
| `PAGERDUTY_FILTER_INTEGRATION_REGEX` | Filter integrations by name using a regex pattern. Integrations whose names match this pattern will be included in the migration. | String | N/A |
283288
| `PRESERVE_EXISTING_USER_NOTIFICATION_RULES` | Whether to preserve existing notification rules when migrating users | Boolean | `true` |

tools/migrators/lib/pagerduty/migrate.py

+34
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,35 @@
5151
)
5252

5353

54+
def filter_users(users: List[Dict[str, Any]]) -> List[Dict[str, Any]]:
55+
"""
56+
Filter users based on PAGERDUTY_FILTER_USERS.
57+
58+
When PAGERDUTY_FILTER_USERS is set, only users with IDs in that list will be included.
59+
"""
60+
if not PAGERDUTY_FILTER_USERS:
61+
return users # No filtering, return all users
62+
63+
filtered_users = []
64+
filtered_out = 0
65+
66+
for user in users:
67+
if user["id"] in PAGERDUTY_FILTER_USERS:
68+
filtered_users.append(user)
69+
else:
70+
filtered_out += 1
71+
72+
if filtered_out > 0:
73+
summary = f"Filtered out {filtered_out} users (keeping only users specified in PAGERDUTY_FILTER_USERS)"
74+
print(summary)
75+
76+
# Only print detailed info in verbose mode
77+
if VERBOSE_LOGGING:
78+
print(f"{TAB}Keeping only users with IDs: {', '.join(PAGERDUTY_FILTER_USERS)}")
79+
80+
return filtered_users
81+
82+
5483
def filter_schedules(schedules: List[Dict[str, Any]]) -> List[Dict[str, Any]]:
5584
"""
5685
Filter schedules based on configured filters.
@@ -252,6 +281,11 @@ def migrate() -> None:
252281
users = session.list_all("users", params={"include[]": "notification_rules"})
253282
oncall_users = OnCallAPIClient.list_users_with_notification_rules()
254283

284+
# Apply filtering to users if specified
285+
if PAGERDUTY_FILTER_USERS:
286+
print(f"▶ Filtering users based on PAGERDUTY_FILTER_USERS...")
287+
users = filter_users(users)
288+
255289
# Match users with Grafana OnCall users
256290
for user in users:
257291
match_user(user, oncall_users)

tools/migrators/lib/tests/pagerduty/test_migrate.py

+66
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
filter_escalation_policies,
55
filter_integrations,
66
filter_schedules,
7+
filter_users,
78
migrate,
89
)
910

@@ -35,6 +36,51 @@ def test_users_are_skipped_when_migrate_users_is_false(
3536
mock_oncall_client.list_users_with_notification_rules.assert_not_called()
3637

3738

39+
@patch("lib.pagerduty.migrate.MIGRATE_USERS", True)
40+
@patch("lib.pagerduty.migrate.PAGERDUTY_FILTER_USERS", ["USER1", "USER3"])
41+
@patch("lib.pagerduty.migrate.MODE", "migrate") # Skip report generation
42+
@patch("lib.pagerduty.migrate.APISession")
43+
@patch("lib.pagerduty.migrate.OnCallAPIClient")
44+
def test_only_specified_users_are_processed_when_filter_users_is_set(
45+
MockOnCallAPIClient, MockAPISession
46+
):
47+
mock_session = MockAPISession.return_value
48+
49+
# Create test users with required fields
50+
users = [
51+
{"id": "USER1", "name": "User 1", "oncall_user": None, "email": "[email protected]"},
52+
{"id": "USER2", "name": "User 2", "oncall_user": None, "email": "[email protected]"},
53+
{"id": "USER3", "name": "User 3", "oncall_user": None, "email": "[email protected]"},
54+
{"id": "USER4", "name": "User 4", "oncall_user": None, "email": "[email protected]"}
55+
]
56+
57+
# Configure mock to return test users for first call, empty lists for other calls
58+
mock_session.list_all.side_effect = [
59+
users, # users
60+
[], # schedules
61+
[], # escalation_policies
62+
[], # services
63+
[], # vendors
64+
]
65+
mock_session.jget.return_value = {"overrides": []}
66+
67+
# Mock the user matching function to set oncall_user
68+
with patch("lib.pagerduty.migrate.match_user") as mock_match_user:
69+
def set_oncall_user(user, _):
70+
# Just leave oncall_user as it is (None)
71+
pass
72+
73+
mock_match_user.side_effect = set_oncall_user
74+
75+
# Run migrate
76+
migrate()
77+
78+
# Check that match_user was only called for USER1 and USER3
79+
assert mock_match_user.call_count == 2
80+
user_ids = [call_args[0][0]["id"] for call_args in mock_match_user.call_args_list]
81+
assert set(user_ids) == {"USER1", "USER3"}
82+
83+
3884
class TestPagerDutyFiltering:
3985
def setup_method(self):
4086
self.mock_schedule = {
@@ -74,6 +120,26 @@ def setup_method(self):
74120
},
75121
}
76122

123+
self.users = [
124+
{"id": "USER1", "name": "User 1"},
125+
{"id": "USER2", "name": "User 2"},
126+
{"id": "USER3", "name": "User 3"},
127+
]
128+
129+
@patch("lib.pagerduty.migrate.PAGERDUTY_FILTER_USERS", ["USER1", "USER3"])
130+
def test_filter_users(self):
131+
"""Test filtering users by ID when PAGERDUTY_FILTER_USERS is set."""
132+
filtered = filter_users(self.users)
133+
assert len(filtered) == 2
134+
assert {u["id"] for u in filtered} == {"USER1", "USER3"}
135+
136+
@patch("lib.pagerduty.migrate.PAGERDUTY_FILTER_USERS", [])
137+
def test_filter_users_no_filter(self):
138+
"""Test that all users are kept when PAGERDUTY_FILTER_USERS is empty."""
139+
filtered = filter_users(self.users)
140+
assert len(filtered) == 3
141+
assert {u["id"] for u in filtered} == {"USER1", "USER2", "USER3"}
142+
77143
@patch("lib.pagerduty.migrate.PAGERDUTY_FILTER_TEAM", "Team 1")
78144
def test_filter_schedules_by_team(self):
79145
schedules = [

0 commit comments

Comments
 (0)