Skip to content

Commit

Permalink
chore: add pytest-socket library + disable network calls in tests (#…
Browse files Browse the repository at this point in the history
…5315)

# What this PR does

Inspired by [this
discussion](#5307 (comment)).
_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   = <module 'requests' from '/opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/site-packages/requests/__init__.py'>
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    = <requests.sessions.Session object at 0x7f10ebaada90>
        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       = <PreparedRequest [GET]>
        proxies    = {}
        req        = <Request [GET]>
        self       = <requests.sessions.Session object at 0x7f10ebaada90>
        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    = <requests.adapters.HTTPAdapter object at 0x7f10ebaada30>
        allow_redirects = True
        hooks      = {'response': []}
        kwargs     = {'cert': None, 'proxies': OrderedDict(), 'stream': False, 'timeout': None, ...}
        request    = <PreparedRequest [GET]>
        self       = <requests.sessions.Session object at 0x7f10ebaada90>
        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       = <urllib3.connectionpool.HTTPSConnectionPool object at 0x7f10ebaadd30>
        proxies    = OrderedDict()
        request    = <PreparedRequest [GET]>
        self       = <requests.adapters.HTTPAdapter object at 0x7f10ebaada30>
        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       = <urllib3.connectionpool.HTTPSConnectionPool object at 0x7f10ebaadd30>
        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       = <urllib3.connection.HTTPSConnection object at 0x7f10ebaadd60>
        httplib_request_kw = {'body': None, 'headers': {'User-Agent': 'python-requests/2.32.3', 'Accept-Encoding': 'gzip, deflate', 'Accept': '*/*', 'Connection': 'keep-alive'}}
        method     = 'GET'
        self       = <urllib3.connectionpool.HTTPSConnectionPool object at 0x7f10ebaadd30>
        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__  = <class 'urllib3.connectionpool.HTTPSConnectionPool'>
        conn       = <urllib3.connection.HTTPSConnection object at 0x7f10ebaadd60>
        self       = <urllib3.connectionpool.HTTPSConnectionPool object at 0x7f10ebaadd30>
/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       = <urllib3.connection.HTTPSConnection object at 0x7f10ebaadd60>
/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       = <urllib3.connection.HTTPSConnection object at 0x7f10ebaadd60>
/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         = <AddressFamily.AF_INET: 2>
        canonname  = ''
        err        = None
        family     = <AddressFamily.AF_UNSPEC: 0>
        host       = 'www.example.com'
        port       = 443
        proto      = 6
        res        = (<AddressFamily.AF_INET: 2>, <SocketKind.SOCK_STREAM: 1>, 6, '', ('93.184.215.14', 443))
        sa         = ('93.184.215.14', 443)
        sock       = <socket.socket fd=12, family=2, type=1, proto=6, laddr=('0.0.0.0', 0)>
        socket_options = [(6, 1, 1)]
        socktype   = <SocketKind.SOCK_STREAM: 1>
        source_address = None
        timeout    = None
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
inst = <socket.socket fd=12, family=2, type=1, proto=6, laddr=('0.0.0.0', 0)>
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       = <socket.socket fd=12, family=2, type=1, proto=6, laddr=('0.0.0.0', 0)>
/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.
  • Loading branch information
joeyorlando authored Dec 2, 2024
1 parent 26ff937 commit fa071bc
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 16 deletions.
10 changes: 4 additions & 6 deletions engine/apps/api/tests/test_schedule_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
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(
Expand All @@ -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)
Expand Down Expand Up @@ -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})
Expand Down Expand Up @@ -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})
Expand Down Expand Up @@ -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)
Expand Down
17 changes: 15 additions & 2 deletions engine/apps/api/tests/test_schedules.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,20 @@
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()
Expand Down Expand Up @@ -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")

Expand Down
2 changes: 0 additions & 2 deletions engine/apps/api/tests/test_user_schedule_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
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(
Expand Down
9 changes: 7 additions & 2 deletions engine/apps/auth_token/tests/test_plugin_auth.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
from unittest.mock import patch

import pytest
from django.utils import timezone
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions engine/apps/grafana_plugin/tests/test_sync_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
[
Expand All @@ -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,
Expand Down
1 change: 1 addition & 0 deletions engine/requirements-dev.in
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions engine/requirements-dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
9 changes: 8 additions & 1 deletion engine/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit fa071bc

Please sign in to comment.