cleanup `CustomButton` backend code + add `ngrok`/`express` outgoing webhook e2e test #2544

joeyorlando · 2023-07-15T08:13:07Z

What this PR does

removes unused "custom button" backend code now that we've migrated to outgoing webhooks
adds new e2e test for webhooks asserting that an ngrok/express webhook handler receives the call as expected + payload is as expected (related to Add suite of e2e tests for new webhook functionality #2691) - skipped for now, the test passes locally but fails on GitHub Actions CI, seems to be networking related

Checklist

Unit, integration, and e2e (if applicable) tests updated
Documentation added (or pr:no public docs PR label added if not required)

github-actions · 2023-08-15T01:44:40Z

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

github-actions · 2023-09-14T01:45:00Z

This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

joeyorlando · 2024-03-06T20:34:12Z

engine/apps/alerts/escalation_snapshot/escalation_snapshot_mixin.py

-                    'passed_last_time': None,
-                    'escalation_counter': 0,
-                    'last_notified_user': None,
-                    'custom_button_trigger': None,


rather than just remove the custom_button_trigger field here from the docstring, I don't think we should be documenting large types like this in the docstring (this is what type hints are for 🙂)

joeyorlando · 2024-03-07T14:04:42Z

engine/apps/api/tests/test_schedules.py

-def test_create_invalid_ical_schedule(schedule_internal_api_setup, make_user_auth_headers):
-    user, token, _, _, _, _ = schedule_internal_api_setup
-    client = APIClient()
-    url = reverse("api-internal:custom_button-list")


not sure if this was intentional? 🤔

…fana/oncall into jorlando/cleanup-custom-button-stuff

…nment

joeyorlando · 2024-03-14T17:38:58Z

grafana-plugin/e2e-tests/outgoingWebhooks/simpleWebhook.test.ts

+    const app = express();
+    app.use(express.json());
+    app.post('/', (req, res) => {
+      resolveRequest(req.body); // Resolve the promise with the request body
+      res.send('ok');
+    });
+    app.listen(PORT);
+
+    /**
+     * TODO: this might need to be parametrized to be read from an env var
+     * rather than hardcoding the hostname to be host.docker.internal
+     */
+    const webhookUrl = `http://host.docker.internal:${PORT}`;


tried using ngrok here.. this worked locally, but it seems like there was some issues on CI, with the API being able to communicate with the express server's public internet URL assigned by ngrok (see test failure here).

Refactored in 3f66b87 to use host.docker.internal instead.

See this Slack conversation for more context

Konstantinov-Innokentii · 2024-03-28T11:17:03Z

engine/common/api_helpers/utils.py

@@ -52,34 +50,6 @@ def __repr__(self):
        return "%s()" % self.__class__.__name__


-class URLValidatorWithoutTLD(URLValidator):


I remember I added this because some OSS users wanted to make requests to docker containers, which default url validator didn't permit. Do we have url validation for webhooks url? if so, should we use something like that there:

if live_settings.DANGEROUS_WEBHOOKS_ENABLED: URLValidatorWithoutTLD()(webhook) else: URLValidator()(webhook)

👋 I believe that's how apps.webhooks.utils.parse_url works.

fwiw the URLValidatorWithoutTLD class was only referenced within the CustomButton model, so to me it made sense to remove it

Correct DANGEROUS_WEBHOOKS_ENABLED settings is handled there. Validation happens later in the process now since URLs can be templates

# What this PR does See [this conversation](https://raintank-corp.slack.com/archives/C025VMT6SPK/p1711991400490289) for more context. Also removes the `EscalationPolicy.custom_button_trigger` field which was previously marked as deprecated in #2544 and migrated [here](https://github.com/grafana/oncall/blob/dev/engine/apps/webhooks/migrations/0008_auto_20230712_1613.py#L39-L45) ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated (N/A) - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes.

wip

d9f6403

github-actions bot added the pr:stale Added to a PR that has been deemed "stale". Managed by the actions/stale GitHub Action label Aug 15, 2023

github-actions bot closed this Sep 14, 2023

joeyorlando deleted the jorlando/cleanup-custom-button-stuff branch September 25, 2023 15:50

joeyorlando restored the jorlando/cleanup-custom-button-stuff branch March 6, 2024 20:05

joeyorlando reopened this Mar 6, 2024

joeyorlando added 2 commits March 6, 2024 15:27

Merge branch 'dev' into jorlando/cleanup-custom-button-stuff

ded9f0d

wip

ab0f666

joeyorlando had a problem deploying to github-pages March 6, 2024 20:27 — with GitHub Actions Error

revert changes to docs

a010e74

joeyorlando had a problem deploying to github-pages March 6, 2024 20:29 — with GitHub Actions Error

remove unused util functions in apps.alerts.utils

705ca9d

joeyorlando had a problem deploying to github-pages March 6, 2024 20:31 — with GitHub Actions Error

add back docstring

82b1497

joeyorlando had a problem deploying to github-pages March 6, 2024 20:33 — with GitHub Actions Error

remove docstring

9b880d1

joeyorlando had a problem deploying to github-pages March 6, 2024 20:34 — with GitHub Actions Failure

joeyorlando commented Mar 6, 2024

View reviewed changes

fix alerts tasks __init__.py

aabdf8f

joeyorlando had a problem deploying to github-pages March 6, 2024 20:38 — with GitHub Actions Failure

joeyorlando added release:ignore PR will not be added to release notes and removed pr:stale Added to a PR that has been deemed "stale". Managed by the actions/stale GitHub Action labels Mar 6, 2024

WIP

71ea85f

joeyorlando had a problem deploying to github-pages March 7, 2024 13:57 — with GitHub Actions Failure

joeyorlando commented Mar 7, 2024

View reviewed changes

WIP

2bf40b6

joeyorlando had a problem deploying to github-pages March 7, 2024 14:13 — with GitHub Actions Failure

WIP

37aaa14

Merge branch 'jorlando/cleanup-custom-button-stuff' of github.com:gra…

23ea09d

…fana/oncall into jorlando/cleanup-custom-button-stuff

joeyorlando had a problem deploying to github-pages March 13, 2024 17:06 — with GitHub Actions Error

Merge branch 'dev' into jorlando/cleanup-custom-button-stuff

0422eb2

joeyorlando had a problem deploying to github-pages March 13, 2024 17:09 — with GitHub Actions Failure

joeyorlando requested a review from mderynck March 13, 2024 17:10

joeyorlando added 2 commits March 13, 2024 16:35

update db migration

5da45ba

fix UI linting error

27c5681

joeyorlando had a problem deploying to github-pages March 13, 2024 20:37 — with GitHub Actions Failure

brojd approved these changes Mar 14, 2024

View reviewed changes

remove ngrok + set DANGEROUS_WEBHOOKS_ENABLED to true for test enviro…

3f66b87

…nment

joeyorlando had a problem deploying to github-pages March 14, 2024 17:36 — with GitHub Actions Failure

joeyorlando commented Mar 14, 2024

View reviewed changes

joeyorlando added 3 commits March 27, 2024 16:03

skip new webhook test for now

fc5be2b

Merge branch 'dev' into jorlando/cleanup-custom-button-stuff

92a0bd2

update helm-local.yml

f6cf563

joeyorlando had a problem deploying to github-pages March 27, 2024 20:06 — with GitHub Actions Failure

joeyorlando enabled auto-merge March 27, 2024 20:07

fix db migration issue

1457282

joeyorlando temporarily deployed to github-pages March 27, 2024 22:42 — with GitHub Actions Inactive

Merge branch 'dev' into jorlando/cleanup-custom-button-stuff

4dd345a

joeyorlando had a problem deploying to github-pages March 27, 2024 23:09 — with GitHub Actions Failure

joeyorlando temporarily deployed to github-pages March 28, 2024 00:00 — with GitHub Actions Inactive

Konstantinov-Innokentii reviewed Mar 28, 2024

View reviewed changes

mderynck approved these changes Mar 28, 2024

View reviewed changes

joeyorlando added this pull request to the merge queue Mar 28, 2024

Merged via the queue into dev with commit c5cd675 Mar 28, 2024
21 checks passed

joeyorlando deleted the jorlando/cleanup-custom-button-stuff branch March 28, 2024 15:49

joeyorlando mentioned this pull request Apr 2, 2024

cleanup orphaned CustomButton escalation policy steps #4143

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cleanup `CustomButton` backend code + add `ngrok`/`express` outgoing webhook e2e test #2544

cleanup `CustomButton` backend code + add `ngrok`/`express` outgoing webhook e2e test #2544

joeyorlando commented Jul 15, 2023 •

edited

Loading

github-actions bot commented Aug 15, 2023

github-actions bot commented Sep 14, 2023

joeyorlando Mar 6, 2024

joeyorlando Mar 7, 2024

joeyorlando Mar 14, 2024 •

edited

Loading

Konstantinov-Innokentii Mar 28, 2024 •

edited

Loading

joeyorlando Mar 28, 2024

mderynck Mar 28, 2024

		@@ -52,34 +50,6 @@ def __repr__(self):
		return "%s()" % self.__class__.__name__


		class URLValidatorWithoutTLD(URLValidator):

cleanup CustomButton backend code + add ngrok/express outgoing webhook e2e test #2544

cleanup CustomButton backend code + add ngrok/express outgoing webhook e2e test #2544

Conversation

joeyorlando commented Jul 15, 2023 • edited Loading

What this PR does

Checklist

github-actions bot commented Aug 15, 2023

github-actions bot commented Sep 14, 2023

joeyorlando Mar 6, 2024

Choose a reason for hiding this comment

joeyorlando Mar 7, 2024

Choose a reason for hiding this comment

joeyorlando Mar 14, 2024 • edited Loading

Choose a reason for hiding this comment

Konstantinov-Innokentii Mar 28, 2024 • edited Loading

Choose a reason for hiding this comment

joeyorlando Mar 28, 2024

Choose a reason for hiding this comment

mderynck Mar 28, 2024

Choose a reason for hiding this comment

cleanup `CustomButton` backend code + add `ngrok`/`express` outgoing webhook e2e test #2544

cleanup `CustomButton` backend code + add `ngrok`/`express` outgoing webhook e2e test #2544

joeyorlando commented Jul 15, 2023 •

edited

Loading

joeyorlando Mar 14, 2024 •

edited

Loading

Konstantinov-Innokentii Mar 28, 2024 •

edited

Loading