From fa071bcd6e4f8f7acf01b7b560c9bcce50bf663f Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 2 Dec 2024 10:53:18 -0500 Subject: [PATCH] chore: add `pytest-socket` library + disable network calls in tests (#5315) # What this PR does Inspired by [this discussion](https://github.com/grafana/oncall/pull/5307#discussion_r1862449480). _tldr;_ ensures that if any of our tests try making an external network call, they will fail. Setup an example test: ```python def test_external_network_call(): import requests response = requests.get('https://www.example.com') assert response.status_code == 200 ``` and it worked (failed; [example CI test run](https://github.com/grafana/oncall/actions/runs/12106416991/job/33752144727?pr=5315#step:6:389)) as expected: ```bash __________________________ test_external_network_call __________________________ def test_external_network_call(): import requests > response = requests.get('https://www.example.com') requests = apps/test_joey.py:4: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ /opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/site-packages/requests/api.py:73: in get return request("get", url, params=params, **kwargs) kwargs = {} params = None url = 'https://www.example.com' /opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/site-packages/requests/api.py:59: in request return session.request(method=method, url=url, **kwargs) kwargs = {'params': None} method = 'get' session = url = 'https://www.example.com' /opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/site-packages/requests/sessions.py:589: in request resp = self.send(prep, **send_kwargs) allow_redirects = True auth = None cert = None cookies = None data = None files = None headers = None hooks = None json = None method = 'get' params = None prep = proxies = {} req = self = send_kwargs = {'allow_redirects': True, 'cert': None, 'proxies': OrderedDict(), 'stream': False, ...} settings = {'cert': None, 'proxies': OrderedDict(), 'stream': False, 'verify': True} stream = None timeout = None url = 'https://www.example.com' verify = None /opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/site-packages/requests/sessions.py:703: in send r = adapter.send(request, **kwargs) adapter = allow_redirects = True hooks = {'response': []} kwargs = {'cert': None, 'proxies': OrderedDict(), 'stream': False, 'timeout': None, ...} request = self = start = 1733064371.649901 stream = False /opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/site-packages/requests/adapters.py:667: in send resp = conn.urlopen( cert = None chunked = False conn = proxies = OrderedDict() request = self = stream = False timeout = Timeout(connect=None, read=None, total=None) url = '/' verify = True /opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/site-packages/urllib3/connectionpool.py:715: in urlopen httplib_response = self._make_request( assert_same_host = False body = None body_pos = None chunked = False clean_exit = False conn = None destination_scheme = None err = None headers = {'User-Agent': 'python-requests/2.32.3', 'Accept-Encoding': 'gzip, deflate', 'Accept': '*/*', 'Connection': 'keep-alive'} http_tunnel_required = False is_new_proxy_conn = False method = 'GET' parsed_url = Url(scheme=None, auth=None, host=None, port=None, path='/', query=None, fragment=None) pool_timeout = None redirect = False release_conn = False release_this_conn = True response_kw = {'decode_content': False, 'preload_content': False} retries = Retry(total=0, connect=None, read=False, redirect=None, status=None) self = timeout = Timeout(connect=None, read=None, total=None) timeout_obj = Timeout(connect=None, read=None, total=None) url = '/' /opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/site-packages/urllib3/connectionpool.py:404: in _make_request self._validate_conn(conn) chunked = False conn = httplib_request_kw = {'body': None, 'headers': {'User-Agent': 'python-requests/2.32.3', 'Accept-Encoding': 'gzip, deflate', 'Accept': '*/*', 'Connection': 'keep-alive'}} method = 'GET' self = timeout = Timeout(connect=None, read=None, total=None) timeout_obj = Timeout(connect=None, read=None, total=None) url = '/' /opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/site-packages/urllib3/connectionpool.py:1060: in _validate_conn conn.connect() __class__ = conn = self = /opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/site-packages/urllib3/connection.py:363: in connect self.sock = conn = self._new_conn() self = /opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/site-packages/urllib3/connection.py:174: in _new_conn conn = connection.create_connection( extra_kw = {'socket_options': [(6, 1, 1)]} self = /opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/site-packages/urllib3/util/connection.py:85: in create_connection sock.connect(sa) address = ('www.example.com', 443) af = canonname = '' err = None family = host = 'www.example.com' port = 443 proto = 6 res = (, , 6, '', ('93.184.215.14', 443)) sa = ('93.184.215.14', 443) sock = socket_options = [(6, 1, 1)] socktype = source_address = None timeout = None _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ inst = args = (('93.184.215.14', 443),), host = '93.184.215.14' def guarded_connect(inst, *args): host = host_from_connect_args(args) if host in allowed_ip_hosts_and_hostnames or ( _is_unix_socket(inst.family) and allow_unix_socket ): return _true_connect(inst, *args) > raise SocketConnectBlockedError(allowed_list, host) E pytest_socket.SocketConnectBlockedError: A test tried to use socket.socket.connect() with host "93.184.215.14" (allowed: "calendar.google.com (142.251.167.100,142.251.167.101,142.251.167.102,142.251.167.113,142.251.167.138,142.251.167.139,2607:f8b0:4004:c09::65,2607:f8b0:4004:c09::66,2607:f8b0:4004:c09::71,2607:f8b0:4004:c09::8b),localhost (127.0.0.1,::1),oncall-dev-mariadb ()"). allow_unix_socket = False allowed_ip_hosts_and_hostnames = {'127.0.0.1', '142.251.167.100', '142.251.167.101', '142.251.167.102', '142.251.167.113', '142.251.167.138', ...} allowed_list = ['calendar.google.com (142.251.167.100,142.251.167.101,142.251.167.102,142.251.167.113,142.251.167.138,142.251.167.139...8b0:4004:c09::66,2607:f8b0:4004:c09::71,2607:f8b0:4004:c09::8b)', 'localhost (127.0.0.1,::1)', 'oncall-dev-mariadb ()'] args = (('93.184.215.14', 443),) host = '93.184.215.14' inst = /opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/site-packages/pytest_socket.py:252: SocketConnectBlockedError ``` ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [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. --- engine/apps/api/tests/test_schedule_export.py | 10 ++++------ engine/apps/api/tests/test_schedules.py | 17 +++++++++++++++-- .../apps/api/tests/test_user_schedule_export.py | 2 -- .../apps/auth_token/tests/test_plugin_auth.py | 9 +++++++-- .../apps/grafana_plugin/tests/test_sync_v2.py | 4 ++++ engine/requirements-dev.in | 1 + engine/requirements-dev.txt | 7 ++++--- engine/tox.ini | 9 ++++++++- 8 files changed, 43 insertions(+), 16 deletions(-) diff --git a/engine/apps/api/tests/test_schedule_export.py b/engine/apps/api/tests/test_schedule_export.py index f747067e..19900245 100644 --- a/engine/apps/api/tests/test_schedule_export.py +++ b/engine/apps/api/tests/test_schedule_export.py @@ -7,8 +7,6 @@ from apps.api.permissions import LegacyAccessControlRole from apps.auth_token.models import ScheduleExportAuthToken from apps.schedules.models import OnCallScheduleICal -ICAL_URL = "https://calendar.google.com/calendar/ical/amixr.io_37gttuakhrtr75ano72p69rt78%40group.calendar.google.com/private-1d00a680ba5be7426c3eb3ef1616e26d/basic.ics" # noqa - @pytest.mark.django_db @pytest.mark.parametrize( @@ -32,7 +30,7 @@ def test_get_schedule_export_token( organization, schedule_class=OnCallScheduleICal, name="test_ical_schedule", - ical_url_primary=ICAL_URL, + ical_url_primary="https://example.com", ) ScheduleExportAuthToken.create_auth_token(user=user, organization=organization, schedule=schedule) @@ -68,7 +66,7 @@ def test_schedule_export_token_not_found( organization, schedule_class=OnCallScheduleICal, name="test_ical_schedule", - ical_url_primary=ICAL_URL, + ical_url_primary="https://example.com", ) url = reverse("api-internal:schedule-export-token", kwargs={"pk": schedule.public_primary_key}) @@ -102,7 +100,7 @@ def test_schedule_create_export_token( organization, schedule_class=OnCallScheduleICal, name="test_ical_schedule", - ical_url_primary=ICAL_URL, + ical_url_primary="https://example.com", ) url = reverse("api-internal:schedule-export-token", kwargs={"pk": schedule.public_primary_key}) @@ -136,7 +134,7 @@ def test_schedule_delete_export_token( organization, schedule_class=OnCallScheduleICal, name="test_ical_schedule", - ical_url_primary=ICAL_URL, + ical_url_primary="https://example.com", ) instance, _ = ScheduleExportAuthToken.create_auth_token(user=user, organization=organization, schedule=schedule) diff --git a/engine/apps/api/tests/test_schedules.py b/engine/apps/api/tests/test_schedules.py index 8efcb6b2..5c4e9bbf 100644 --- a/engine/apps/api/tests/test_schedules.py +++ b/engine/apps/api/tests/test_schedules.py @@ -23,7 +23,20 @@ from apps.schedules.models import ( from apps.slack.models import SlackUserGroup from common.api_helpers.utils import create_engine_url, serialize_datetime_as_utc_timestamp -ICAL_URL = "https://calendar.google.com/calendar/ical/amixr.io_37gttuakhrtr75ano72p69rt78%40group.calendar.google.com/private-1d00a680ba5be7426c3eb3ef1616e26d/basic.ics" +ICAL_URL = "https://example.com" + + +@pytest.fixture(autouse=True) +def patch_fetch_ical_file(): + """ + NOTE: we patch this method for all tests in this file to avoid making actual HTTP requests.. we don't really need + to test the actual fetching of the ical file in these tests, so just simply mock out the response as an empty string + + Alternatively, if we really needed to, we could save .ical files locally here, and read/return those as the + return_value + """ + with patch("apps.schedules.ical_utils.fetch_ical_file", return_value=""): + yield @pytest.fixture() @@ -64,7 +77,7 @@ def schedule_internal_api_setup( def test_get_list_schedules( schedule_internal_api_setup, make_escalation_chain, make_escalation_policy, make_user_auth_headers ): - user, token, calendar_schedule, ical_schedule, web_schedule, slack_channel = schedule_internal_api_setup + user, token, calendar_schedule, ical_schedule, web_schedule, _ = schedule_internal_api_setup client = APIClient() url = reverse("api-internal:schedule-list") diff --git a/engine/apps/api/tests/test_user_schedule_export.py b/engine/apps/api/tests/test_user_schedule_export.py index eec82fa0..7ebcd884 100644 --- a/engine/apps/api/tests/test_user_schedule_export.py +++ b/engine/apps/api/tests/test_user_schedule_export.py @@ -6,8 +6,6 @@ from rest_framework.test import APIClient from apps.api.permissions import LegacyAccessControlRole from apps.auth_token.models import UserScheduleExportAuthToken -ICAL_URL = "https://calendar.google.com/calendar/ical/amixr.io_37gttuakhrtr75ano72p69rt78%40group.calendar.google.com/private-1d00a680ba5be7426c3eb3ef1616e26d/basic.ics" # noqa - @pytest.mark.django_db @pytest.mark.parametrize( diff --git a/engine/apps/auth_token/tests/test_plugin_auth.py b/engine/apps/auth_token/tests/test_plugin_auth.py index a44bd377..44440e73 100644 --- a/engine/apps/auth_token/tests/test_plugin_auth.py +++ b/engine/apps/auth_token/tests/test_plugin_auth.py @@ -1,4 +1,5 @@ import json +from unittest.mock import patch import pytest from django.utils import timezone @@ -79,8 +80,12 @@ def test_plugin_authentication_fail(authorization, instance_context): request = APIRequestFactory().get("/", **headers) - with pytest.raises(AuthenticationFailed): - PluginAuthentication().authenticate(request) + class MockCheckTokenResponse: + organization = None + + with patch("apps.auth_token.auth.check_token", return_value=MockCheckTokenResponse): + with pytest.raises(AuthenticationFailed): + PluginAuthentication().authenticate(request) @pytest.mark.django_db diff --git a/engine/apps/grafana_plugin/tests/test_sync_v2.py b/engine/apps/grafana_plugin/tests/test_sync_v2.py index 8f1a9271..59291a7b 100644 --- a/engine/apps/grafana_plugin/tests/test_sync_v2.py +++ b/engine/apps/grafana_plugin/tests/test_sync_v2.py @@ -142,6 +142,7 @@ def test_sync_v2_content_encoding( mock_sync.assert_called() +@patch("apps.grafana_plugin.helpers.client.GrafanaAPIClient.check_token", return_value=(None, {"connected": True})) @pytest.mark.parametrize( "irm_enabled,expected", [ @@ -151,6 +152,9 @@ def test_sync_v2_content_encoding( ) @pytest.mark.django_db def test_sync_v2_irm_enabled( + # mock this out so that we're not making a real network call, the sync v2 endpoint ends up calling + # user_management.sync._sync_organization which calls GrafanaApiClient.check_token + _mock_grafana_api_client_check_token, make_organization_and_user_with_plugin_token, make_user_auth_headers, settings, diff --git a/engine/requirements-dev.in b/engine/requirements-dev.in index 7d599ee8..ce5f7142 100644 --- a/engine/requirements-dev.in +++ b/engine/requirements-dev.in @@ -8,6 +8,7 @@ mypy==1.4.1 pre-commit==2.15.0 pytest==8.2.2 pytest-django==4.8.0 +pytest-socket==0.7.0 pytest-xdist[psutil]==3.6.1 pytest_factoryboy==2.7.0 types-beautifulsoup4==4.12.0.5 diff --git a/engine/requirements-dev.txt b/engine/requirements-dev.txt index 16256b89..03cf4606 100644 --- a/engine/requirements-dev.txt +++ b/engine/requirements-dev.txt @@ -91,11 +91,14 @@ pytest==8.2.2 # -r requirements-dev.in # pytest-django # pytest-factoryboy + # pytest-socket # pytest-xdist pytest-django==4.8.0 # via -r requirements-dev.in pytest-factoryboy==2.7.0 # via -r requirements-dev.in +pytest-socket==0.7.0 + # via -r requirements-dev.in pytest-xdist==3.6.1 # via -r requirements-dev.in python-dateutil==2.8.2 @@ -111,9 +114,7 @@ requests==2.32.3 # -c requirements.txt # djangorestframework-stubs setuptools==75.3.0 - # via - # -c requirements.txt - # nodeenv + # via nodeenv six==1.16.0 # via # -c requirements.txt diff --git a/engine/tox.ini b/engine/tox.ini index 908559ba..be2959bb 100644 --- a/engine/tox.ini +++ b/engine/tox.ini @@ -14,7 +14,14 @@ banned-modules = # # --dist no = temporarily disable xdist as it's leading to flaky tests :( # https://github.com/grafana/oncall-private/issues/2733 -addopts = --dist no --no-migrations --color=yes --showlocals + +# From pytest-socket docs (https://github.com/miketheman/pytest-socket): +# A plugin to use with Pytest to disable or restrict socket calls during tests to ensure network calls are prevented +# --disable-socket = tests should fail on any access to socket or libraries using socket with a SocketBlockedErro +# --allow-hosts = allow connections to the given hostnames/IPs. +# - localhost = our tests on CI use localhost as the host to connect to databases running locally in docker container +# - oncall-dev-mariadb = if you're running things locally, with a MariaDB instance running, there's a good chance the hostname will be this +addopts = --dist no --no-migrations --color=yes --showlocals --disable-socket --allow-hosts=localhost,oncall-dev-mariadb # https://pytest-django.readthedocs.io/en/latest/faq.html#my-tests-are-not-being-found-why python_files = tests.py test_*.py *_tests.py