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: adjust for tokenless v3 #434

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions codecov_cli/commands/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"--pr",
"--pull-request-number",
"pull_request_number",
help="Specify the pull request number mannually. Used to override pre-existing CI environment variables",
help="[Deprecated], this option has no effect on the execution of this command anymore. Specify the pull request number mannually. Used to override pre-existing CI environment variables",
cls=CodecovOption,
fallback_field=FallbackFieldEnum.pull_request_number,
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps we don't need this anymore either

)
Expand Down Expand Up @@ -55,7 +55,6 @@ def create_report(
git_service,
token,
enterprise_url,
pull_request_number,
fail_on_error,
)
if not res.error:
Expand Down
10 changes: 10 additions & 0 deletions codecov_cli/helpers/request.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging
import os
from sys import exit
from time import sleep

Expand Down Expand Up @@ -95,6 +96,15 @@ def get_token_header_or_fail(token: str) -> dict:
return {"Authorization": f"token {token}"}


def get_auth_header(token):
tokenless = os.environ.get("TOKENLESS", None)
Copy link
Contributor

Choose a reason for hiding this comment

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

are we making this change for the orb and step too? Why doesn't the CLI just do this itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i don't think the CLI has access to the same context as the action but i can double check, but you are correct we should make a similar change the the orb and step as well

if tokenless:
headers = {"X-Tokenless": tokenless}
else:
headers = get_token_header_or_fail(token)
return headers


@retry_request
def send_put_request(
url: str,
Expand Down
15 changes: 3 additions & 12 deletions codecov_cli/services/commit/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@
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.encoder import encode_slug
from codecov_cli.helpers.request import (
get_token_header_or_fail,
get_auth_header,
log_warnings_and_errors_if_any,
send_post_request,
)
Expand Down Expand Up @@ -43,15 +42,7 @@ 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
Copy link
Contributor

Choose a reason for hiding this comment

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

don't we still need this?

logger.info("The PR is happening in a forked repo. Using tokenless upload.")
else:
headers = get_token_header_or_fail(token)

headers = get_auth_header(token)
data = {
"commitid": commit_sha,
"parent_commit_id": parent_sha,
Expand Down
17 changes: 3 additions & 14 deletions codecov_cli/services/report/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
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_auth_header,
get_token_header_or_fail,
log_warnings_and_errors_if_any,
request_result,
Expand All @@ -26,7 +27,6 @@ def create_report_logic(
service: str,
token: str,
enterprise_url: str,
pull_request_number: int,
fail_on_error: bool = False,
):
encoded_slug = encode_slug(slug)
Expand All @@ -37,27 +37,16 @@ def create_report_logic(
token,
encoded_slug,
enterprise_url,
pull_request_number,
)
log_warnings_and_errors_if_any(sending_result, "Report creating", fail_on_error)
return sending_result


def send_create_report_request(
commit_sha, code, service, token, encoded_slug, enterprise_url, pull_request_number
commit_sha, code, service, token, encoded_slug, enterprise_url
):
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_auth_header(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
16 changes: 2 additions & 14 deletions codecov_cli/services/upload/upload_sender.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
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_auth_header,
send_post_request,
send_put_request,
)
Expand Down Expand Up @@ -54,18 +54,7 @@ def send_upload_data(
"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_auth_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 Expand Up @@ -100,7 +89,6 @@ def send_upload_data(
extra=dict(extra_log_attributes=dict(response=resp_json_obj)),
)
put_url = resp_json_obj["raw_upload_location"]
logger.debug("Sending upload to storage")
resp_from_storage = send_put_request(put_url, data=reports_payload)
return resp_from_storage

Expand Down
7 changes: 2 additions & 5 deletions tests/commands/test_process_test_results.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ def test_process_test_results(

assert result.exit_code == 0


mocked_post.assert_called_with(
url="https://api.github.com/repos/fake/repo/issues/pull/comments",
data={
Expand All @@ -58,7 +57,6 @@ def test_process_test_results(
)



def test_process_test_results_non_existent_file(mocker, tmpdir):
tmp_file = tmpdir.mkdir("folder").join("summary.txt")

Expand Down Expand Up @@ -93,7 +91,7 @@ def test_process_test_results_non_existent_file(mocker, tmpdir):
assert result.exit_code == 1
expected_logs = [
"ci service found",
'Some files were not found',
"Some files were not found",
]
for log in expected_logs:
assert log in result.output
Expand Down Expand Up @@ -182,7 +180,6 @@ def test_process_test_results_missing_ref(mocker, tmpdir):
assert log in result.output



def test_process_test_results_missing_step_summary(mocker, tmpdir):
tmp_file = tmpdir.mkdir("folder").join("summary.txt")

Expand Down Expand Up @@ -221,4 +218,4 @@ def test_process_test_results_missing_step_summary(mocker, tmpdir):
"Error: Error getting step summary file path from environment. Can't find GITHUB_STEP_SUMMARY environment variable.",
]
for log in expected_logs:
assert log in result.output
assert log in result.output
93 changes: 84 additions & 9 deletions tests/helpers/test_network_finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,40 +12,115 @@ def test_find_files(mocker, tmp_path):
mocked_vs = MagicMock()
mocked_vs.list_relevant_files.return_value = filenames

assert NetworkFinder(versioning_system=mocked_vs, network_filter=None, network_prefix=None, network_root_folder=tmp_path).find_files() == filenames
assert NetworkFinder(versioning_system=mocked_vs, network_filter="hello", network_prefix="bello", network_root_folder=tmp_path).find_files(False) == filtered_filenames
assert NetworkFinder(versioning_system=mocked_vs, network_filter="hello", network_prefix="bello", network_root_folder=tmp_path).find_files(True) == filenames
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter=None,
network_prefix=None,
network_root_folder=tmp_path,
).find_files()
== filenames
)
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter="hello",
network_prefix="bello",
network_root_folder=tmp_path,
).find_files(False)
== filtered_filenames
)
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter="hello",
network_prefix="bello",
network_root_folder=tmp_path,
).find_files(True)
== filenames
)
mocked_vs.list_relevant_files.assert_called_with(tmp_path)


def test_find_files_with_filter(mocker, tmp_path):
filenames = ["hello/a.txt", "hello/c.txt", "bello/b.txt"]
filtered_filenames = ["hello/a.txt", "hello/c.txt"]

mocked_vs = MagicMock()
mocked_vs.list_relevant_files.return_value = filenames

assert NetworkFinder(versioning_system=mocked_vs, network_filter="hello", network_prefix=None, network_root_folder=tmp_path).find_files() == filtered_filenames
assert NetworkFinder(versioning_system=mocked_vs, network_filter="hello", network_prefix="bello", network_root_folder=tmp_path).find_files(True) == filenames
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter="hello",
network_prefix=None,
network_root_folder=tmp_path,
).find_files()
== filtered_filenames
)
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter="hello",
network_prefix="bello",
network_root_folder=tmp_path,
).find_files(True)
== filenames
)
mocked_vs.list_relevant_files.assert_called_with(tmp_path)


def test_find_files_with_prefix(mocker, tmp_path):
filenames = ["hello/a.txt", "hello/c.txt", "bello/b.txt"]
filtered_filenames = ["hellohello/a.txt", "hellohello/c.txt", "hellobello/b.txt"]

mocked_vs = MagicMock()
mocked_vs.list_relevant_files.return_value = filenames

assert NetworkFinder(versioning_system=mocked_vs, network_filter=None, network_prefix="hello", network_root_folder=tmp_path).find_files() == filtered_filenames
assert NetworkFinder(versioning_system=mocked_vs, network_filter="hello", network_prefix="bello", network_root_folder=tmp_path).find_files(True) == filenames
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter=None,
network_prefix="hello",
network_root_folder=tmp_path,
).find_files()
== filtered_filenames
)
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter="hello",
network_prefix="bello",
network_root_folder=tmp_path,
).find_files(True)
== filenames
)
mocked_vs.list_relevant_files.assert_called_with(tmp_path)


def test_find_files_with_filter_and_prefix(mocker, tmp_path):
filenames = ["hello/a.txt", "hello/c.txt", "bello/b.txt"]
filtered_filenames = ["bellohello/a.txt", "bellohello/c.txt"]

mocked_vs = MagicMock()
mocked_vs.list_relevant_files.return_value = filenames

assert NetworkFinder(versioning_system=mocked_vs, network_filter="hello", network_prefix="bello", network_root_folder=tmp_path).find_files() == filtered_filenames
assert NetworkFinder(versioning_system=mocked_vs, network_filter="hello", network_prefix="bello", network_root_folder=tmp_path).find_files(True) == filenames
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter="hello",
network_prefix="bello",
network_root_folder=tmp_path,
).find_files()
== filtered_filenames
)
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter="hello",
network_prefix="bello",
network_root_folder=tmp_path,
).find_files(True)
== filenames
)
mocked_vs.list_relevant_files.assert_called_with(tmp_path)
12 changes: 3 additions & 9 deletions tests/helpers/test_upload_sender.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import os
import re
from pathlib import Path

Expand Down Expand Up @@ -231,19 +232,13 @@ 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 = {"X-Tokenless": "user:branch"}
mocked_legacy_upload_endpoint.match = [
matchers.json_params_matcher(request_data),
matchers.header_matcher(headers),
]

os.environ["TOKENLESS"] = "user:branch"
sending_result = UploadSender().send_upload_data(
upload_collection, random_sha, None, **named_upload_data
)
Expand All @@ -263,7 +258,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
7 changes: 5 additions & 2 deletions tests/services/commit/test_commit_service.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import os
import uuid

import requests
Expand Down Expand Up @@ -177,6 +178,8 @@ def mock_request(*args, headers={}, **kwargs):
"get",
side_effect=mock_request,
)

os.environ["TOKENLESS"] = "user:branch"
res = send_commit_data(
"commit_sha",
"parent_sha",
Expand All @@ -193,7 +196,7 @@ def mock_request(*args, headers={}, **kwargs):
"commitid": "commit_sha",
"parent_commit_id": "parent_sha",
"pullid": "1",
"branch": "user_forked_repo/codecov-cli:branch",
"branch": "branch",
},
headers={"X-Tokenless": "user_forked_repo/codecov-cli", "X-Tokenless-PR": "1"},
headers={"X-Tokenless": "user:branch"},
)