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.

Sign up for GitHub

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

fix: accept tokenless value from branch instead of env var #475

Merged
merged 1 commit into from
Dec 3, 2024
Merged

fix: accept tokenless value from branch instead of env var #475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 14 additions & 8 deletions codecov_cli/services/commit/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,21 @@ def send_commit_data(
enterprise_url,
args,
):
# 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")
if tokenless:
headers = None # type: ignore
branch = tokenless # type: ignore
logger.info("The PR is happening in a forked repo. Using tokenless upload.")
# Old versions of the GHA use this env var instead of the regular branch
# argument to provide an unprotected branch name
if tokenless := os.environ.get("TOKENLESS"):
branch = tokenless
Copy link
Contributor

Choose a reason for hiding this comment

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

branch = os.environ.get("TOKENLESS", None) could have worked too

Copy link
Contributor

@matt-codecov matt-codecov Dec 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

that would clobber whatever was already in branch. we want that value, unless there is a value in $TOKENLESS

Copy link
Contributor

Choose a reason for hiding this comment

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

@matt-codecov ahhh that's a good point


if branch and ":" in branch:
logger.info(f"Creating a commit for an unprotected branch: {branch}")
elif token is None:
logger.warning(
f"Branch `{branch}` is protected but no token was provided\nFor information on Codecov upload tokens, see https://docs.codecov.com/docs/codecov-tokens"
)
else:
headers = get_token_header(token)
logger.info("Using token to create a commit for protected branch `{branch}`")

headers = get_token_header(token)
Copy link
Contributor

Choose a reason for hiding this comment

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

apparently the GHA pulls the latest codecovcli by default, so users who are still on older versions of the action which use $TOKENLESS will find themselves suddenly broken if we release this version that stops reading from it

so i guess we should still read from the env var, and i think logs would be valuable if they get printed to the user so they can see hints that maybe their branch name is not what they expected

# Old versions of the GHA use this env var instead of the regular branch
# argument to provide an unprotected branch name
if tokenless := os.environ.get("TOKENLESS"):
  branch = tokenless

if branch and ":" in branch:
    logger.info(f"Creating a commit for an unprotected branch: {branch}")
elif token is None:
    logger.warning(f"Branch `{branch}` is protected but no token was provided\nFor information on Codecov upload tokens, see https://docs.codecov.com/docs/codecov-tokens")
else:
    logger.info("Using token to create a commit for protected branch `{branch}`")

headers = get_token_header(token)


data = {
"branch": branch,
Expand Down
37 changes: 32 additions & 5 deletions tests/services/commit/test_commit_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import json
import uuid

import requests
from click.testing import CliRunner
from requests import Response

from codecov_cli.services.commit import create_commit_logic, send_commit_data
from codecov_cli.types import RequestError, RequestResult, RequestResultWarning
Expand Down Expand Up @@ -155,12 +152,11 @@ def test_commit_sender_with_forked_repo(mocker):
return_value=mocker.MagicMock(status_code=200, text="success"),
)

mocker.patch("os.environ", dict(TOKENLESS="user_forked_repo/codecov-cli:branch"))
_ = send_commit_data(
"commit_sha",
"parent_sha",
"1",
"branch",
"user_forked_repo/codecov-cli:branch",
"codecov::::codecov-cli",
None,
"github",
Expand Down Expand Up @@ -208,3 +204,34 @@ def test_commit_without_token(mocker):
},
headers=None,
)


def test_commit_sender_with_forked_repo_bad_branch(mocker):
mocked_response = mocker.patch(
"codecov_cli.services.commit.send_post_request",
return_value=mocker.MagicMock(status_code=200, text="success"),
)
mocker.patch("os.environ", dict(TOKENLESS="user_forked_repo/codecov-cli:branch"))
_res = send_commit_data(
"commit_sha",
"parent_sha",
"1",
"branch",
"codecov::::codecov-cli",
None,
"github",
None,
None,
)

mocked_response.assert_called_with(
url="https://ingest.codecov.io/upload/github/codecov::::codecov-cli/commits",
data={
"branch": "user_forked_repo/codecov-cli:branch",
"cli_args": None,
"commitid": "commit_sha",
"parent_commit_id": "parent_sha",
"pullid": "1",
},
headers=None,
)
Loading