Skip to content
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

feat: support new tokenless protocol #447

Merged
merged 7 commits into from
Jun 18, 2024
Merged
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
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) -> bool:
"""
takes in dict: pull_dict
returns true if PR is made in a fork context, false if not.
"""
return pull_dict 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
19 changes: 16 additions & 3 deletions codecov_cli/helpers/request.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import logging
from sys import exit
from time import sleep
from typing import Optional

import click
import requests
Expand All @@ -15,7 +16,7 @@
USER_AGENT = f"codecov-cli/{__version__}"


def _set_user_agent(headers: dict = None) -> dict:
def _set_user_agent(headers: Optional[dict] = None) -> dict:
headers = headers or {}
headers.setdefault("User-Agent", USER_AGENT)
return headers
Expand All @@ -37,7 +38,10 @@ def put(url: str, data: dict = None, headers: dict = None) -> requests.Response:


def post(
url: str, data: dict = None, headers: dict = None, params: dict = None
url: str,
data: Optional[dict] = None,
headers: Optional[dict] = None,
params: Optional[dict] = None,
) -> requests.Response:
headers = _set_user_agent(headers)
return requests.post(url, json=data, headers=headers, params=params)
Expand Down Expand Up @@ -82,7 +86,10 @@ def wrapper(*args, **kwargs):

@retry_request
def send_post_request(
url: str, data: dict = None, headers: dict = None, params: dict = None
url: str,
data: Optional[dict] = None,
headers: Optional[dict] = None,
params: Optional[dict] = None,
):
return request_result(post(url=url, data=data, headers=headers, params=params))

Expand All @@ -95,6 +102,12 @@ def get_token_header_or_fail(token: str) -> dict:
return {"Authorization": f"token {token}"}


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


@retry_request
def send_put_request(
url: str,
Expand Down
13 changes: 7 additions & 6 deletions codecov_cli/services/commit/__init__.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import logging
import os
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,12 @@ 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):
headers = {"X-Tokenless": pull_dict["head"]["slug"], "X-Tokenless-PR": pr}
branch = pull_dict["head"]["slug"] + ":" + branch
# this is how the CLI receives the username of the user to whom the fork belongs
# to and the branch name from the action
tokenless = os.environ.get("TOKENLESS")

Choose a reason for hiding this comment

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

What's this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gonna leave a comment explaining this but this is how the CLI receives the username of the user to whom the fork belongs to and the branch name from the action

Choose a reason for hiding this comment

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

that makes sense, a comment here would be pretty helpful thanks

if tokenless:
headers = None # 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
13 changes: 1 addition & 12 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,17 +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 = {
"X-Tokenless": pull_dict["head"]["slug"],
"X-Tokenless-PR": pull_request_number,
}
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
17 changes: 2 additions & 15 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,19 +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 = {
"X-Tokenless": pull_dict["head"]["slug"],
"X-Tokenless-PR": pull_request_number,
}
else:
headers = get_token_header_or_fail(token)
headers = get_token_header(token)

Choose a reason for hiding this comment

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

Why are some commands using get_token_header vs get_token_header_or_fail? How do we know when to use which?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

get_token_header is for supporting tokenless, if someone tries to upload using tokenless get_token_header_or_fail just fails and prevents them from doing anything

Choose a reason for hiding this comment

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

Ahh that makes sense, worth leaving another comment here? 😅

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 @@ -7,6 +7,7 @@
from codecov_cli import __version__
from codecov_cli.helpers.request import (
get,
get_token_header,
get_token_header_or_fail,
log_warnings_and_errors_if_any,
)
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
11 changes: 2 additions & 9 deletions tests/helpers/test_upload_sender.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,14 +231,8 @@ def test_upload_sender_post_called_with_right_parameters_tokenless(
mocked_storage_server,
mocker,
):
headers = {"X-Tokenless": "user-forked/repo", "X-Tokenless-PR": "pr"}
mock_get_pull = mocker.patch(
"codecov_cli.services.upload.upload_sender.get_pull",
return_value={
"head": {"slug": "user-forked/repo"},
"base": {"slug": "org/repo"},
},
)
headers = {}

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
30 changes: 2 additions & 28 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", dict(TOKENLESS="user_forked_repo/codecov-cli:branch"))
res = send_commit_data(
"commit_sha",
"parent_sha",
Expand All @@ -195,5 +169,5 @@ def mock_request(*args, headers={}, **kwargs):
"pullid": "1",
"branch": "user_forked_repo/codecov-cli:branch",
},
headers={"X-Tokenless": "user_forked_repo/codecov-cli", "X-Tokenless-PR": "1"},
headers=None,
)
Loading
Loading