diff --git a/CHANGELOG.md b/CHANGELOG.md index dfb8758c..110ee554 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,20 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## v1.1.40 (2023-03-16) + +### Fixed + +- Check for duplicated positions in terraform escalation policies create/update + +### Added + +- Add `regex_match` Jinja filter ([1556](https://github.com/grafana/oncall/pull/1556)) + +### Changed + +- Allow passing `null` as a value for `escalation_chain` when creating routes via the public API ([1557](https://github.com/grafana/oncall/pull/1557)) + ## v1.1.39 (2023-03-16) ### Added diff --git a/docs/sources/alert-behavior/alert-templates/index.md b/docs/sources/alert-behavior/alert-templates/index.md index 0269dfc8..3cbfbf38 100644 --- a/docs/sources/alert-behavior/alert-templates/index.md +++ b/docs/sources/alert-behavior/alert-templates/index.md @@ -176,3 +176,4 @@ Built-in functions: - `iso8601_to_time` - converts time from iso8601 (`2015-02-17T18:30:20.000Z`) to datetime - `datetimeformat` - converts time from datetime to the given format (`%H:%M / %d-%m-%Y` by default) - `regex_replace` - performs a regex find and replace +- `regex_match` - performs a regex match, returns `True` or `False`. Usage example: `{{ payload.ruleName | regex_match(".*") }}` diff --git a/docs/sources/oncall-api-reference/routes.md b/docs/sources/oncall-api-reference/routes.md index 3ffa32e9..49dfc25a 100644 --- a/docs/sources/oncall-api-reference/routes.md +++ b/docs/sources/oncall-api-reference/routes.md @@ -51,7 +51,7 @@ Routes allow you to direct different alerts to different messenger channels and | Parameter | Unique | Required | Description | |-----------------------| :----: |:--------:|:--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | `integration_id` | No | Yes | Each route is assigned to a specific integration. | -| `escalation_chain_id` | No | Yes | Each route is assigned a specific escalation chain. | +| `escalation_chain_id` | No | Yes | Each route is assigned a specific escalation chain. Explicitly pass `null` to create a route without an escalation chain assigned. | | `routing_type` | Yes | No | Routing type that can be either `jinja2` or `regex`(default value) | | `routing_regex` | Yes | Yes | Jinja2 template or Python Regex query (use for debugging). OnCall chooses the route for an alert in case there is a match inside the whole alert payload. | | `position` | Yes | Optional | Route matching is performed one after another starting from position=`0`. Position=`-1` will put the route to the end of the list before `is_the_last_route`. A new route created with a position of an existing route will move the old route (and all following routes) down in the list. | diff --git a/engine/apps/public_api/serializers/escalation_policies.py b/engine/apps/public_api/serializers/escalation_policies.py index e7903cae..0193e5d3 100644 --- a/engine/apps/public_api/serializers/escalation_policies.py +++ b/engine/apps/public_api/serializers/escalation_policies.py @@ -136,6 +136,8 @@ class EscalationPolicySerializer(EagerLoadingMixin, OrderedModelSerializerMixin, instance = super().create(validated_data) self._change_position(order, instance) else: + # validate will raise if there is a duplicated order + self._validate_manual_order(None, validated_data) instance = super().create(validated_data) return instance @@ -209,6 +211,18 @@ class EscalationPolicySerializer(EagerLoadingMixin, OrderedModelSerializerMixin, result.pop(field, None) return result + def _validate_manual_order(self, instance, validated_data): + order = validated_data.get("order") + if order is None: + return + + policies_with_order = self.escalation_chain.escalation_policies.filter(order=order) + if instance and instance.id: + policies_with_order = policies_with_order.exclude(id=instance.id) + + if policies_with_order.exists(): + raise BadRequest(detail="Steps cannot have duplicated positions") + def _correct_validated_data(self, validated_data): validated_data_fields_to_remove = [ "notify_to_users_queue", @@ -302,5 +316,8 @@ class EscalationPolicyUpdateSerializer(EscalationPolicySerializer): order = validated_data.pop("order", None) self._validate_order(order, {"escalation_chain_id": instance.escalation_chain_id}) self._change_position(order, instance) + else: + # validate will raise if there is a duplicated order + self._validate_manual_order(instance, validated_data) return super().update(instance, validated_data) diff --git a/engine/apps/public_api/serializers/routes.py b/engine/apps/public_api/serializers/routes.py index abbc9f4f..07d2b398 100644 --- a/engine/apps/public_api/serializers/routes.py +++ b/engine/apps/public_api/serializers/routes.py @@ -155,6 +155,7 @@ class ChannelFilterSerializer(BaseChannelFilterSerializer): escalation_chain_id = OrganizationFilteredPrimaryKeyRelatedField( queryset=EscalationChain.objects, source="escalation_chain", + allow_null=True, ) is_the_last_route = serializers.BooleanField(read_only=True, source="is_default") diff --git a/engine/apps/public_api/tests/test_escalation_policies.py b/engine/apps/public_api/tests/test_escalation_policies.py index 06aa0345..b1b705e1 100644 --- a/engine/apps/public_api/tests/test_escalation_policies.py +++ b/engine/apps/public_api/tests/test_escalation_policies.py @@ -145,6 +145,29 @@ def test_create_escalation_policy( assert response.data == serializer.data +@pytest.mark.django_db +def test_create_escalation_policy_manual_order_duplicated_position( + make_organization_and_user_with_token, + escalation_policies_setup, +): + organization, user, token = make_organization_and_user_with_token() + escalation_chain, _, _ = escalation_policies_setup(organization, user) + + data_for_create = { + "escalation_chain_id": escalation_chain.public_primary_key, + "type": "notify_person_next_each_time", + "position": 0, + "persons_to_notify_next_each_time": [user.public_primary_key], + "manual_order": True, + } + + client = APIClient() + url = reverse("api-public:escalation_policies-list") + response = client.post(url, data=data_for_create, format="json", HTTP_AUTHORIZATION=token) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @pytest.mark.django_db def test_invalid_step_type( make_organization_and_user_with_token, @@ -219,3 +242,24 @@ def test_create_important_step( assert response.status_code == status.HTTP_201_CREATED assert escalation_policy.step == EscalationPolicy.STEP_NOTIFY_SCHEDULE_IMPORTANT assert response.data["important"] is True + + +@pytest.mark.django_db +def test_update_escalation_policy_manual_order_duplicated_position( + make_organization_and_user_with_token, + escalation_policies_setup, +): + organization, user, token = make_organization_and_user_with_token() + _, escalation_policies, _ = escalation_policies_setup(organization, user) + escalation_policy_wait = escalation_policies[1] + + client = APIClient() + url = reverse("api-public:escalation_policies-detail", kwargs={"pk": escalation_policy_wait.public_primary_key}) + response = client.get(url, format="json", HTTP_AUTHORIZATION=token) + + assert response.data["position"] != 0 + + data_to_change = {"position": 0, "manual_order": True} + response = client.put(url, data=data_to_change, format="json", HTTP_AUTHORIZATION=token) + + assert response.status_code == status.HTTP_400_BAD_REQUEST diff --git a/engine/apps/public_api/tests/test_routes.py b/engine/apps/public_api/tests/test_routes.py index 5f0db22d..300a943a 100644 --- a/engine/apps/public_api/tests/test_routes.py +++ b/engine/apps/public_api/tests/test_routes.py @@ -162,6 +162,37 @@ def test_create_route( assert response.json() == expected_response +@pytest.mark.django_db +def test_create_route_without_escalation_chain(route_public_api_setup): + _, _, token, alert_receive_channel, escalation_chain, _ = route_public_api_setup + + client = APIClient() + url = reverse("api-public:routes-list") + + data = { + "integration_id": alert_receive_channel.public_primary_key, + "routing_regex": "testreg", + "escalation_chain_id": None, + } + response = client.post(url, format="json", HTTP_AUTHORIZATION=token, data=data) + + expected_response = { + "id": response.data["id"], + "integration_id": alert_receive_channel.public_primary_key, + "escalation_chain_id": None, + "routing_type": "regex", + "routing_regex": data["routing_regex"], + "position": 0, + "is_the_last_route": False, + "slack": {"channel_id": None, "enabled": True}, + "telegram": {"id": None, "enabled": False}, + TEST_MESSAGING_BACKEND_FIELD: {"id": None, "enabled": False}, + } + + assert response.status_code == status.HTTP_201_CREATED + assert response.json() == expected_response + + @pytest.mark.django_db def test_invalid_route_data( route_public_api_setup, diff --git a/engine/common/jinja_templater/filters.py b/engine/common/jinja_templater/filters.py index 07557fca..1264aa68 100644 --- a/engine/common/jinja_templater/filters.py +++ b/engine/common/jinja_templater/filters.py @@ -30,3 +30,10 @@ def regex_replace(value, find, replace): return re.sub(find, replace, value) except (ValueError, AttributeError, TypeError): return None + + +def regex_match(pattern, value): + try: + return bool(re.match(value, pattern)) + except (ValueError, AttributeError, TypeError): + return None diff --git a/engine/common/jinja_templater/jinja_template_env.py b/engine/common/jinja_templater/jinja_template_env.py index 747b010b..32ceda6b 100644 --- a/engine/common/jinja_templater/jinja_template_env.py +++ b/engine/common/jinja_templater/jinja_template_env.py @@ -3,7 +3,7 @@ from jinja2 import BaseLoader from jinja2.exceptions import SecurityError from jinja2.sandbox import SandboxedEnvironment -from .filters import datetimeformat, iso8601_to_time, regex_replace, to_pretty_json +from .filters import datetimeformat, iso8601_to_time, regex_match, regex_replace, to_pretty_json def raise_security_exception(name): @@ -18,3 +18,4 @@ jinja_template_env.filters["tojson_pretty"] = to_pretty_json jinja_template_env.globals["time"] = timezone.now jinja_template_env.globals["range"] = lambda *args: raise_security_exception("range") jinja_template_env.filters["regex_replace"] = regex_replace +jinja_template_env.filters["regex_match"] = regex_match diff --git a/engine/common/tests/test_apply_jinja_template.py b/engine/common/tests/test_apply_jinja_template.py index aed42af1..632f5026 100644 --- a/engine/common/tests/test_apply_jinja_template.py +++ b/engine/common/tests/test_apply_jinja_template.py @@ -14,6 +14,18 @@ def test_apply_jinja_template(): assert payload == result +def test_apply_jinja_template_regex_match(): + payload = {"name": "test"} + + assert apply_jinja_template("{{ payload.name | regex_match('.*') }}", payload) == "True" + assert apply_jinja_template("{{ payload.name | regex_match('tes') }}", payload) == "True" + assert apply_jinja_template("{{ payload.name | regex_match('test1') }}", payload) == "False" + + # Check that exception is raised when regex is invalid + with pytest.raises(JinjaTemplateError): + apply_jinja_template("{{ payload.name | regex_match('*') }}", payload) + + def test_apply_jinja_template_bad_syntax_error(): with pytest.raises(JinjaTemplateError): apply_jinja_template("{{%", payload={}) diff --git a/tools/pagerduty-migrator/migrator/__main__.py b/tools/pagerduty-migrator/migrator/__main__.py index 4f544cb9..403f9232 100644 --- a/tools/pagerduty-migrator/migrator/__main__.py +++ b/tools/pagerduty-migrator/migrator/__main__.py @@ -3,15 +3,22 @@ import datetime from pdpyras import APISession from migrator import oncall_api_client -from migrator.config import MODE, MODE_PLAN, PAGERDUTY_API_TOKEN +from migrator.config import ( + EXPERIMENTAL_MIGRATE_EVENT_RULES, + MODE, + MODE_PLAN, + PAGERDUTY_API_TOKEN, +) from migrator.report import ( TAB, escalation_policy_report, format_escalation_policy, format_integration, + format_ruleset, format_schedule, format_user, integration_report, + ruleset_report, schedule_report, user_report, ) @@ -26,6 +33,7 @@ from migrator.resources.integrations import ( migrate_integration, ) from migrator.resources.notification_rules import migrate_notification_rules +from migrator.resources.rulesets import match_ruleset, migrate_ruleset from migrator.resources.schedules import match_schedule, migrate_schedule from migrator.resources.users import ( match_user, @@ -88,6 +96,14 @@ def main() -> None: oncall_integrations = oncall_api_client.list_all("integrations") + rulesets = None + if EXPERIMENTAL_MIGRATE_EVENT_RULES: + print("▶ Fetching event rules (rulesets) ...") + rulesets = session.list_all("rulesets") + for ruleset in rulesets: + rules = session.list_all(f"rulesets/{ruleset['id']}/rules") + ruleset["rules"] = rules + for user in users: match_user(user, oncall_users) @@ -108,15 +124,18 @@ def main() -> None: match_integration_type(integration, vendors) match_escalation_policy_for_integration(integration, escalation_policies) + if rulesets is not None: + for ruleset in rulesets: + match_ruleset(ruleset, oncall_integrations, escalation_policies, services) + if MODE == MODE_PLAN: - print() - print(user_report(users)) - print() - print(schedule_report(schedules)) - print() - print(escalation_policy_report(escalation_policies)) - print() - print(integration_report(integrations)) + print(user_report(users), end="\n\n") + print(schedule_report(schedules), end="\n\n") + print(escalation_policy_report(escalation_policies), end="\n\n") + print(integration_report(integrations), end="\n\n") + + if rulesets is not None: + print(ruleset_report(rulesets), end="\n\n") return @@ -147,6 +166,13 @@ def main() -> None: migrate_integration(integration, escalation_policies) print(TAB + format_integration(integration)) + if rulesets is not None: + print("▶ Migrating event rules (rulesets) ...") + for ruleset in rulesets: + if not ruleset["flawed_escalation_policies"]: + migrate_ruleset(ruleset, escalation_policies, services) + print(TAB + format_ruleset(ruleset)) + if __name__ == "__main__": main() diff --git a/tools/pagerduty-migrator/migrator/config.py b/tools/pagerduty-migrator/migrator/config.py index 35700c54..f2baeccb 100644 --- a/tools/pagerduty-migrator/migrator/config.py +++ b/tools/pagerduty-migrator/migrator/config.py @@ -41,3 +41,8 @@ SCHEDULE_MIGRATION_MODE_WEB = "web" SCHEDULE_MIGRATION_MODE = os.getenv( "SCHEDULE_MIGRATION_MODE", SCHEDULE_MIGRATION_MODE_ICAL ) + +# Experimental feature to migrate PD rulesets to OnCall integrations +EXPERIMENTAL_MIGRATE_EVENT_RULES = ( + os.getenv("EXPERIMENTAL_MIGRATE_EVENT_RULES", "false").lower() == "true" +) diff --git a/tools/pagerduty-migrator/migrator/report.py b/tools/pagerduty-migrator/migrator/report.py index ff975988..b9b6aeef 100644 --- a/tools/pagerduty-migrator/migrator/report.py +++ b/tools/pagerduty-migrator/migrator/report.py @@ -163,3 +163,36 @@ def integration_report(integrations: list[dict]) -> str: ) return result + + +def format_ruleset(ruleset: dict) -> str: + if ruleset["flawed_escalation_policies"]: + escalation_policy_names = [ + p["name"] for p in ruleset["flawed_escalation_policies"] + ] + result = "{} {} — escalation policies '{}' reference unmatched users or schedules that cannot be migrated".format( + ERROR_SIGN, ruleset["name"], ", ".join(escalation_policy_names) + ) + else: + result = "{} {}".format(SUCCESS_SIGN, ruleset["name"]) + + return result + + +def ruleset_report(rulesets: list[dict]) -> str: + result = "Event rules (rulesets) report:" + + for ruleset in sorted( + rulesets, + key=lambda r: bool(r["flawed_escalation_policies"]), + reverse=True, + ): + result += "\n" + TAB + format_ruleset(ruleset) + if not ruleset["flawed_escalation_policies"] and ruleset["oncall_integration"]: + result += ( + " (existing integration with name '{} Ruleset' will be deleted)".format( + ruleset["name"] + ) + ) + + return result diff --git a/tools/pagerduty-migrator/migrator/resources/rulesets.py b/tools/pagerduty-migrator/migrator/resources/rulesets.py new file mode 100644 index 00000000..0296c8b8 --- /dev/null +++ b/tools/pagerduty-migrator/migrator/resources/rulesets.py @@ -0,0 +1,160 @@ +from migrator import oncall_api_client +from migrator.utils import find_by_id + + +def match_ruleset( + ruleset: dict, + oncall_integrations: list[dict], + escalation_policies: list[dict], + services: list[dict], +) -> None: + # Find existing integration with the same name + oncall_integration = None + name = "{} Ruleset".format(ruleset["name"]).lower().strip() + for candidate in oncall_integrations: + if candidate["name"].lower().strip() == name: + oncall_integration = candidate + ruleset["oncall_integration"] = oncall_integration + + # Find services that use escalation policies that cannot be migrated + service_ids = [ + r["actions"]["route"]["value"] + for r in ruleset["rules"] + if not r["disabled"] and r["actions"]["route"] + ] + escalation_policy_ids = [ + find_by_id(services, service_id)["escalation_policy"]["id"] + for service_id in service_ids + ] + + flawed_escalation_policies = [] + for escalation_policy_id in escalation_policy_ids: + escalation_policy = find_by_id(escalation_policies, escalation_policy_id) + if bool( + escalation_policy["unmatched_users"] + or escalation_policy["flawed_schedules"] + ): + flawed_escalation_policies.append(escalation_policy) + + ruleset["flawed_escalation_policies"] = flawed_escalation_policies + + +def migrate_ruleset( + ruleset: dict, escalation_policies: list[dict], services: list[dict] +) -> None: + # Delete existing integration with the same name + if ruleset["oncall_integration"]: + oncall_api_client.delete( + "integrations/{}".format(ruleset["oncall_integration"]["id"]) + ) + + # Create new integration with type "webhook" + integration_payload = { + "name": "{} Ruleset".format(ruleset["name"]), + "type": "webhook", + "team_id": None, + } + integration = oncall_api_client.create("integrations", integration_payload) + + # Migrate rules that are not disabled and not catch-all + rules = [r for r in ruleset["rules"] if not r["disabled"] and not r["catch_all"]] + for rule in rules: + service_id = ( + rule["actions"]["route"]["value"] if rule["actions"]["route"] else None + ) + + escalation_chain_id = _pd_service_id_to_oncall_escalation_chain_id( + service_id, services, escalation_policies + ) + filtering_term = transform_condition_to_jinja(rule["conditions"]) + route_payload = { + "routing_type": "jinja2", + "routing_regex": filtering_term, + "position": rule["position"], + "integration_id": integration["id"], + "escalation_chain_id": escalation_chain_id, + } + oncall_api_client.create("routes", route_payload) + + # Migrate catch-all rule + catch_all_rule = [r for r in ruleset["rules"] if r["catch_all"]][0] + catch_all_service_id = ( + catch_all_rule["actions"]["route"]["value"] + if catch_all_rule["actions"]["route"] + else None + ) + catch_all_escalation_chain_id = _pd_service_id_to_oncall_escalation_chain_id( + catch_all_service_id, services, escalation_policies + ) + + if catch_all_escalation_chain_id: + # Get the default route and update it to use appropriate escalation chain + routes = oncall_api_client.list_all( + "routes/?integration_id={}".format(integration["id"]) + ) + default_route_id = routes[-1]["id"] + oncall_api_client.update( + f"routes/{default_route_id}", + {"escalation_chain_id": catch_all_escalation_chain_id}, + ) + + +def transform_condition_to_jinja(condition): + """ + Transform PD event rule condition to Jinja2 template + """ + + operator = condition["operator"] + assert operator in ("and", "or") + + # Insert "and" or "or" between subconditions + template = f" {operator} ".join( + [ + "(" + transform_subcondition_to_jinja(subcondition) + ")" + for subcondition in condition["subconditions"] + ] + ) + template = "{{ " + template + " }}" + return template + + +def transform_subcondition_to_jinja(subcondition): + """ + Transform PD event rule subcondition to Jinja2 template. + """ + operator = subcondition["operator"] + path = subcondition["parameters"]["path"] + value = subcondition["parameters"]["value"] + if value: + value = value.replace('"', '\\"').replace("'", "\\'") + + OPERATOR_TO_JINJA_TEMPLATE = { + "exists": "{path} is defined", + "nexists": "{path} is not defined", + "equals": '{path} == "{value}"', + "nequals": '{path} != "{value}"', + "contains": '"{value}" in {path}', + "ncontains": '"{value}" not in {path}', + "matches": '{path} | regex_match("{value}")', + "nmatches": 'not ({path} | regex_match("{value}"))', + } + jinja_template = OPERATOR_TO_JINJA_TEMPLATE[operator].format(path=path, value=value) + return jinja_template + + +def _pd_service_id_to_oncall_escalation_chain_id( + service_id, services, escalation_policies +): + """ + Helper function to get the OnCall escalation chain ID from a PD service ID. + """ + + if service_id is None: + return None + + service = find_by_id(services, service_id) + escalation_policy_id = service["escalation_policy"]["id"] + escalation_policy = find_by_id(escalation_policies, escalation_policy_id) + escalation_chain_id = escalation_policy["oncall_escalation_chain"]["id"] + + return escalation_chain_id