Skip to content

feat(codecov): initial client for requests to codecov api #91830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file added src/sentry/codecov/__init__.py
Empty file.
129 changes: 129 additions & 0 deletions src/sentry/codecov/client.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
import datetime
import logging
from enum import StrEnum
from typing import TypeAlias

import requests
from rest_framework import status

from sentry import options
from sentry.api.exceptions import SentryAPIException
from sentry.utils import jwt

GitProviderId: TypeAlias = str


class GitProvider(StrEnum):
"""
Enum representing the Git provider that hosts the user/org that a
`CodecovApiClient` instance is acting on behalf of.

Codecov doesn't require this to be GitHub, but that's all that's implemented
for now.
"""

GitHub = "github"


logger = logging.getLogger(__name__)

TIMEOUT_SECONDS = 10


class ConfigurationError(SentryAPIException):
status_code = status.HTTP_500_INTERNAL_SERVER_ERROR
code = "configuration-error"


class CodecovApiClient:
"""
Thin client for making JWT-authenticated requests to the Codecov API.

For each request, Sentry creates and signs (HS256) a JWT with a key shared
with Codecov. This JWT contains information that Codecov needs to service
the request.
"""

def _create_jwt(self):
now = int(datetime.datetime.now(datetime.UTC).timestamp())
exp = now + 300 # 5 minutes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Let's make 300 a constant (with a descriptive name like FIVE_MINUTES)

claims = {
"iss": "https://sentry.io",
"iat": now,
"exp": exp,
}
claims.update(self.custom_claims)

return jwt.encode(claims, self.signing_secret, algorithm="HS256")

def __init__(
self,
git_provider_user: GitProviderId,
git_provider: GitProvider = GitProvider.GitHub,
):
"""
Creates a `CodecovApiClient`.

:param git_provider_user: The ID of the current Sentry user's linked git
provider account, according to the git provider.
:param git_provider: The git provider that the above user's account is
hosted on.
"""

if not (base_url := options.get("codecov.base-url")):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could likely also be a django setting, as it is infrastructure plumbing and wouldn't be manipulated at runtime.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a way to override django settings for local development, i imagine? i'll look into that, ty

raise ConfigurationError()

if not (signing_secret := options.get("codecov.api-bridge-signing-secret")):
raise ConfigurationError()
Comment on lines +76 to +77
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're trying to not store secrets in runtime options, as they are stored in plain text everywhere (code + db). Instead could we use a django setting + environment var for this? That will allow us to put secrets in encrypted storage.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this actually is populated by an environment variable in https://github.com/getsentry/getsentry/pull/17508, will that work?


self.base_url = base_url
self.signing_secret = signing_secret
self.custom_claims = {
"g_u": git_provider_user,
"g_p": git_provider,
}

def get(self, endpoint: str, params=None, headers=None) -> requests.Response | None:
"""
Makes a GET request to the specified endpoint of the configured Codecov
API host with the provided params and headers.

:param endpoint: The endpoint to request, without the host portion. For
examples: `/api/v2/gh/getsentry/users` or `/graphql`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: example vs examples

:param params: Dictionary of query params.
:param headers: Dictionary of request headers.
"""
headers = headers or {}
token = self._create_jwt()
headers.update(jwt.authorization_header(token))

url = f"{self.base_url}{endpoint}"
try:
response = requests.get(url, params=params, headers=headers, timeout=TIMEOUT_SECONDS)
except Exception:
logger.exception("Error when making GET request")
return None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we will probably want to differentiate a 400 vs 500 response from Codecov in the API response. They should be handled differently in the Sentry REST API.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, just realized that you would need a raise_for_status to actually raise the status exceptions. Anyways, we can either handle the errors here in the client, which might be easier for standardization, or leave it to the caller to handle.


return response

def post(self, endpoint: str, data=None, headers=None) -> requests.Response | None:
"""
Makes a POST request to the specified endpoint of the configured Codecov
API host with the provided data and headers.

:param endpoint: The endpoint to request, without the host portion. For
examples: `/api/v2/gh/getsentry/users` or `/graphql`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comments here for example and the log

:param data: Dictionary of form data.
:param headers: Dictionary of request headers.
"""
headers = headers or {}
token = self._create_jwt()
headers.update(jwt.authorization_header(token))
url = f"{self.base_url}{endpoint}"
try:
response = requests.post(url, data=data, headers=headers, timeout=TIMEOUT_SECONDS)
except Exception:
logger.exception("Error when making POST request")
return None
Comment on lines +123 to +127
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts about capturing server errors and retrying?


return response
2 changes: 2 additions & 0 deletions src/sentry/options/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,8 @@

# Codecov Integration
register("codecov.client-secret", flags=FLAG_CREDENTIAL | FLAG_PRIORITIZE_DISK)
register("codecov.base-url", default="https://api.codecov.io")
register("codecov.api-bridge-signing-secret", flags=FLAG_CREDENTIAL | FLAG_PRIORITIZE_DISK)

# GitHub Integration
register("github-app.id", default=0, flags=FLAG_AUTOMATOR_MODIFIABLE)
Expand Down
105 changes: 105 additions & 0 deletions tests/sentry/codecov/test_client.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
import datetime
from unittest.mock import patch

import pytest

from sentry.codecov.client import CodecovApiClient, ConfigurationError, GitProvider
from sentry.testutils.cases import TestCase
from sentry.testutils.silo import all_silo_test
from sentry.utils import jwt


@all_silo_test
class TestCodecovApiClient(TestCase):
def setUp(self):
self.test_git_provider_user = "12345"
self.test_base_url = "http://example.com"
self.test_secret = "test-secret-" + "a" * 20

self.test_timestamp = datetime.datetime.now(datetime.UTC)
self._mock_now = patch("datetime.datetime.now", return_value=self.test_timestamp)

with self.options(
{
"codecov.base-url": self.test_base_url,
"codecov.api-bridge-signing-secret": self.test_secret,
}
):
self.codecov_client = CodecovApiClient(self.test_git_provider_user)

def test_raises_configuration_error_without_base_url(self):
with self.options(
{
"codecov.base-url": None,
"codecov.api-bridge-signing-secret": self.test_secret,
}
):
with pytest.raises(ConfigurationError):
CodecovApiClient(self.test_git_provider_user)

def test_raises_configuration_error_without_signing_secret(self):
with self.options(
{
"codecov.base-url": self.test_base_url,
"codecov.api-bridge-signing-secret": None,
}
):
with pytest.raises(ConfigurationError):
CodecovApiClient(self.test_git_provider_user)

def test_creates_valid_jwt(self):
encoded_jwt = self.codecov_client._create_jwt()

header = jwt.peek_header(encoded_jwt)
assert header == {
"typ": "JWT",
"alg": "HS256",
}

# Ensure the claims are what we expect, separate from verifying the
# signature and standard claims
claims = jwt.peek_claims(encoded_jwt)
expected_iat = int(self.test_timestamp.timestamp())
expected_exp = expected_iat + 300
assert claims == {
"g_u": self.test_git_provider_user,
"g_p": GitProvider.GitHub.value,
"iss": "https://sentry.io",
"iat": expected_iat,
"exp": expected_exp,
}

# Ensure we can verify the signature and whatall
jwt.decode(encoded_jwt, self.test_secret)

@patch("requests.get")
def test_sends_get_request_with_jwt_auth_header(self, mock_get):
with patch.object(self.codecov_client, "_create_jwt", return_value="test"):
self.codecov_client.get(
"/example/endpoint", {"example-param": "foo"}, {"X_TEST_HEADER": "bar"}
)
mock_get.assert_called_once_with(
"http://example.com/example/endpoint",
params={"example-param": "foo"},
headers={
"Authorization": "Bearer test",
"X_TEST_HEADER": "bar",
},
timeout=10,
)

@patch("requests.post")
def test_sends_post_request_with_jwt_auth_header(self, mock_post):
with patch.object(self.codecov_client, "_create_jwt", return_value="test"):
self.codecov_client.post(
"/example/endpoint", {"example-param": "foo"}, {"X_TEST_HEADER": "bar"}
)
mock_post.assert_called_once_with(
"http://example.com/example/endpoint",
data={"example-param": "foo"},
headers={
"Authorization": "Bearer test",
"X_TEST_HEADER": "bar",
},
timeout=10,
)
Loading