Skip to content

Commit

Permalink
fix: remove github API call from tokenless
Browse files Browse the repository at this point in the history
we no longer want to make the github API call to check if the current
ref we are uploading from is a PR to determine what to set the tokenless
header to. It's no longer necessary and all that is needed for tokenless
is to pass the correct  branch name (containing a ':') to the create
commit step so that tokenless works. The CLI will automatically check if
the TOKENLESS env var has been set by the action. The action will set
this env var when it detects it's running on a fork.

Signed-off-by: joseph-sentry <joseph.sawaya@sentry.io>
  • Loading branch information
joseph-sentry committed Jun 17, 2024
1 parent 31b70f9 commit 96ed4e6
Show file tree
Hide file tree
Showing 11 changed files with 28 additions and 198 deletions.
19 changes: 0 additions & 19 deletions codecov_cli/helpers/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,22 +95,3 @@ def parse_git_service(remote_repo_url: str):
extra=dict(remote_repo_url=remote_repo_url),
)
return None


def is_fork_pr(pull_dict: PullDict | None) -> bool:
"""
takes in dict: pull_dict
returns true if PR is made in a fork context, false if not.
"""
return pull_dict is not None and pull_dict["head"]["slug"] != pull_dict["base"]["slug"]


def get_pull(service, slug, pr_num) -> Optional[PullDict]:
"""
takes in str git service e.g. github, gitlab etc., slug in the owner/repo format, and the pull request number
returns the pull request info gotten from the git service provider if successful, None if not
"""
git_service = get_git_service(service)
if git_service:
pull_dict = git_service.get_pull_request(slug, pr_num)
return pull_dict
5 changes: 5 additions & 0 deletions codecov_cli/helpers/request.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ def get_token_header_or_fail(token: str) -> dict:
)
return {"Authorization": f"token {token}"}

def get_token_header(token: str) -> dict | None:
if token is None:
return None
return {"Authorization": f"token {token}"}


@retry_request
def send_put_request(
Expand Down
9 changes: 4 additions & 5 deletions codecov_cli/services/commit/__init__.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import os
import logging
import typing

from codecov_cli.helpers.config import CODECOV_API_URL
from codecov_cli.helpers.encoder import decode_slug, encode_slug
from codecov_cli.helpers.git import get_pull, is_fork_pr
from codecov_cli.helpers.request import (
get_token_header_or_fail,
log_warnings_and_errors_if_any,
Expand Down Expand Up @@ -43,11 +43,10 @@ def create_commit_logic(
def send_commit_data(
commit_sha, parent_sha, pr, branch, slug, token, service, enterprise_url
):
decoded_slug = decode_slug(slug)
pull_dict = get_pull(service, decoded_slug, pr) if not token else None
if is_fork_pr(pull_dict):
tokenless = os.environ.get
if tokenless:
headers = None # type: ignore
branch = pull_dict["head"]["slug"] + ":" + branch # type: ignore
branch = tokenless # type: ignore
logger.info("The PR is happening in a forked repo. Using tokenless upload.")
else:
headers = get_token_header_or_fail(token)
Expand Down
10 changes: 1 addition & 9 deletions codecov_cli/services/report/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from codecov_cli.helpers import request
from codecov_cli.helpers.config import CODECOV_API_URL
from codecov_cli.helpers.encoder import decode_slug, encode_slug
from codecov_cli.helpers.git import get_pull, is_fork_pr
from codecov_cli.helpers.request import (
get_token_header_or_fail,
log_warnings_and_errors_if_any,
Expand Down Expand Up @@ -47,14 +46,7 @@ def send_create_report_request(
commit_sha, code, service, token, encoded_slug, enterprise_url, pull_request_number
):
data = {"code": code}
decoded_slug = decode_slug(encoded_slug)
pull_dict = (
get_pull(service, decoded_slug, pull_request_number) if not token else None
)
if is_fork_pr(pull_dict):
headers = None
else:
headers = get_token_header_or_fail(token)
headers = get_token_header_or_fail(token)
upload_url = enterprise_url or CODECOV_API_URL
url = f"{upload_url}/upload/{service}/{encoded_slug}/commits/{commit_sha}/reports"
return send_post_request(url=url, headers=headers, data=data)
Expand Down
14 changes: 2 additions & 12 deletions codecov_cli/services/upload/upload_sender.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@
from codecov_cli import __version__ as codecov_cli_version
from codecov_cli.helpers.config import CODECOV_API_URL
from codecov_cli.helpers.encoder import encode_slug
from codecov_cli.helpers.git import get_pull, is_fork_pr
from codecov_cli.helpers.request import (
get_token_header_or_fail,
get_token_header,
send_post_request,
send_put_request,
)
Expand Down Expand Up @@ -53,16 +52,7 @@ def send_upload_data(
"version": codecov_cli_version,
"ci_service": ci_service,
}

# Data to upload to Codecov
pull_dict = (
get_pull(git_service, slug, pull_request_number) if not token else None
)

if is_fork_pr(pull_dict):
headers = None
else:
headers = get_token_header_or_fail(token)
headers = get_token_header(token)
encoded_slug = encode_slug(slug)
upload_url = enterprise_url or CODECOV_API_URL
url, data = self.get_url_and_possibly_update_data(
Expand Down
4 changes: 2 additions & 2 deletions codecov_cli/services/upload_completion/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from codecov_cli.helpers.config import CODECOV_API_URL
from codecov_cli.helpers.encoder import encode_slug
from codecov_cli.helpers.request import (
get_token_header_or_fail,
get_token_header,
log_warnings_and_errors_if_any,
send_post_request,
)
Expand All @@ -16,7 +16,7 @@ def upload_completion_logic(
commit_sha, slug, token, git_service, enterprise_url, fail_on_error=False
):
encoded_slug = encode_slug(slug)
headers = get_token_header_or_fail(token)
headers = get_token_header(token)
upload_url = enterprise_url or CODECOV_API_URL
url = f"{upload_url}/upload/{git_service}/{encoded_slug}/commits/{commit_sha}/upload-complete"
sending_result = send_post_request(url=url, headers=headers)
Expand Down
79 changes: 0 additions & 79 deletions tests/helpers/test_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,82 +135,3 @@ def test_get_git_service_class():
assert git.get_git_service("bitbucket") == None


def test_pr_is_not_fork_pr(mocker):
def mock_request(*args, headers={}, **kwargs):
assert headers["X-GitHub-Api-Version"] == "2022-11-28"
res = {
"url": "https://api.github.com/repos/codecov/codecov-cli/pulls/1",
"head": {
"sha": "123",
"label": "codecov-cli:branch",
"ref": "branch",
"repo": {"full_name": "codecov/codecov-cli"},
},
"base": {
"sha": "123",
"label": "codecov-cli:main",
"ref": "main",
"repo": {"full_name": "codecov/codecov-cli"},
},
}
response = Response()
response.status_code = 200
response._content = json.dumps(res).encode("utf-8")
return response

mocker.patch.object(
requests,
"get",
side_effect=mock_request,
)
pull_dict = git.get_pull("github", "codecov/codecov-cli", 1)
assert not git.is_fork_pr(pull_dict)


def test_pr_is_fork_pr(mocker):
def mock_request(*args, headers={}, **kwargs):
assert headers["X-GitHub-Api-Version"] == "2022-11-28"
res = {
"url": "https://api.github.com/repos/codecov/codecov-cli/pulls/1",
"head": {
"sha": "123",
"label": "codecov-cli:branch",
"ref": "branch",
"repo": {"full_name": "user_forked_repo/codecov-cli"},
},
"base": {
"sha": "123",
"label": "codecov-cli:main",
"ref": "main",
"repo": {"full_name": "codecov/codecov-cli"},
},
}
response = Response()
response.status_code = 200
response._content = json.dumps(res).encode("utf-8")
return response

mocker.patch.object(
requests,
"get",
side_effect=mock_request,
)
pull_dict = git.get_pull("github", "codecov/codecov-cli", 1)
assert git.is_fork_pr(pull_dict)


def test_pr_not_found(mocker):
def mock_request(*args, headers={}, **kwargs):
assert headers["X-GitHub-Api-Version"] == "2022-11-28"
response = Response()
response.status_code = 404
response._content = b"not-found"
return response

mocker.patch.object(
requests,
"get",
side_effect=mock_request,
)
pull_dict = git.get_pull("github", "codecov/codecov-cli", 1)
assert not git.is_fork_pr(pull_dict)
12 changes: 12 additions & 0 deletions tests/helpers/test_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from codecov_cli.helpers.request import (
get,
get_token_header_or_fail,
get_token_header,
log_warnings_and_errors_if_any,
)
from codecov_cli.helpers.request import logger as req_log
Expand Down Expand Up @@ -69,6 +70,17 @@ def test_get_token_header_or_fail():
== "Codecov token not found. Please provide Codecov token with -t flag."
)

def test_get_token_header():
# Test with a valid UUID token
token = uuid.uuid4()
result = get_token_header(token)
assert result == {"Authorization": f"token {str(token)}"}

# Test with a None token
token = None
result = get_token_header(token)
assert result is None


def test_request_retry(mocker, valid_response):
expected_response = request_result(valid_response)
Expand Down
9 changes: 1 addition & 8 deletions tests/helpers/test_upload_sender.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,13 +232,7 @@ def test_upload_sender_post_called_with_right_parameters_tokenless(
mocker,
):
headers = {}
mock_get_pull = mocker.patch(
"codecov_cli.services.upload.upload_sender.get_pull",
return_value={
"head": {"slug": "user-forked/repo"},
"base": {"slug": "org/repo"},
},
)

mocked_legacy_upload_endpoint.match = [
matchers.json_params_matcher(request_data),
matchers.header_matcher(headers),
Expand All @@ -263,7 +257,6 @@ def test_upload_sender_post_called_with_right_parameters_tokenless(
assert (
post_req_made.headers.items() >= headers.items()
) # test dict is a subset of the other
mock_get_pull.assert_called()

def test_upload_sender_put_called_with_right_parameters(
self, mocked_responses, mocked_legacy_upload_endpoint, mocked_storage_server
Expand Down
28 changes: 1 addition & 27 deletions tests/services/commit/test_commit_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,33 +150,7 @@ def test_commit_sender_with_forked_repo(mocker):
return_value=mocker.MagicMock(status_code=200, text="success"),
)

def mock_request(*args, headers={}, **kwargs):
assert headers["X-GitHub-Api-Version"] == "2022-11-28"
res = {
"url": "https://api.github.com/repos/codecov/codecov-cli/pulls/1",
"head": {
"sha": "123",
"label": "codecov-cli:branch",
"ref": "branch",
"repo": {"full_name": "user_forked_repo/codecov-cli"},
},
"base": {
"sha": "123",
"label": "codecov-cli:main",
"ref": "main",
"repo": {"full_name": "codecov/codecov-cli"},
},
}
response = Response()
response.status_code = 200
response._content = json.dumps(res).encode("utf-8")
return response

mocker.patch.object(
requests,
"get",
side_effect=mock_request,
)
mocker.patch("os.environ.get", "user_forked_repo/codecov-cli:branch")
res = send_commit_data(
"commit_sha",
"parent_sha",
Expand Down
37 changes: 0 additions & 37 deletions tests/services/report/test_report_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,43 +26,6 @@ def test_send_create_report_request_200(mocker):
mocked_response.assert_called_once()


def test_send_create_report_request_200_tokneless(mocker):
mocked_response = mocker.patch(
"codecov_cli.services.report.send_post_request",
return_value=RequestResult(
status_code=200,
error=None,
warnings=[],
text="mocked response",
),
)

mocked_get_pull = mocker.patch(
"codecov_cli.services.report.get_pull",
return_value={
"head": {"slug": "user-forked/repo"},
"base": {"slug": "org/repo"},
},
)
res = send_create_report_request(
"commit_sha",
"code",
"github",
None,
"owner::::repo",
"enterprise_url",
1,
)
assert res.error is None
assert res.warnings == []
mocked_response.assert_called_with(
url=f"enterprise_url/upload/github/owner::::repo/commits/commit_sha/reports",
headers=None,
data={"code": "code"},
)
mocked_get_pull.assert_called()


def test_send_create_report_request_403(mocker):
mocked_response = mocker.patch(
"codecov_cli.services.report.requests.post",
Expand Down

0 comments on commit 96ed4e6

Please sign in to comment.