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

Hide command when access token is provided and git_clone_project fails #9150

Merged
merged 3 commits into from
Apr 11, 2023
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
15 changes: 12 additions & 3 deletions src/prefect/projects/steps/pull.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,17 @@ def git_clone_project(
# Limit git history
cmd += ["--depth", "1"]

subprocess.check_call(
cmd, shell=sys.platform == "win32", stderr=sys.stderr, stdout=sys.stdout
)
try:
subprocess.check_call(
cmd, shell=sys.platform == "win32", stderr=sys.stderr, stdout=sys.stdout
)
except subprocess.CalledProcessError as exc:
# Hide the command used to avoid leaking the access token
exc_chain = None if access_token else exc
raise RuntimeError(
f"Failed to clone repository {repository!r} with exit code"
f" {exc.returncode}."
) from exc_chain

directory = "/".join(repository.strip().split("/")[-1:]).replace(".git", "")
return {"directory": directory}
41 changes: 37 additions & 4 deletions tests/projects/test_steps.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import pytest

from prefect import Flow
import prefect
from prefect.blocks.system import Secret
from prefect.client.orchestration import PrefectClient
from prefect.projects.steps import run_step
Expand All @@ -21,7 +21,7 @@ async def test_run_step_runs_importable_functions(self):
flow = await run_step(
{"prefect.flow": {"__fn": lambda x: 42, "name": "test-name"}}
)
assert isinstance(flow, Flow)
assert isinstance(flow, prefect.Flow)
assert flow.name == "test-name"
assert flow.fn(None) == 42

Expand All @@ -42,7 +42,7 @@ async def test_run_step_resolves_block_document_references_before_running(self):
}
}
)
assert isinstance(flow, Flow)
assert isinstance(flow, prefect.Flow)
assert flow.name == "secret-name"
assert flow.fn(None) == 42

Expand All @@ -58,6 +58,39 @@ async def test_run_step_resolves_variables_before_running(self, variables):
}
}
)
assert isinstance(flow, Flow)
assert isinstance(flow, prefect.Flow)
assert flow.name == "test_value_1:test_value_2"
assert flow.fn(None) == 42


class TestGitCloneStep:
async def test_git_clone_errors_obscure_access_token(self):
with pytest.raises(RuntimeError) as exc:
await run_step(
{
"prefect.projects.steps.git_clone_project": {
"repository": "https://github.com/PrefectHQ/prefect.git",
"branch": "definitely-does-not-exist-123",
"access_token": None,
}
}
)
# prove by default command shows up
assert (
"Command '['git', 'clone', 'https://github.com/PrefectHQ/prefect.git',"
" '-b', 'definitely-does-not-exist-123', '--depth', '1']"
in str(exc.getrepr())
)

with pytest.raises(RuntimeError) as exc:
# we uppercase the token because this test definition does show up in the exception traceback
await run_step(
{
"prefect.projects.steps.git_clone_project": {
"repository": "https://github.com/PrefectHQ/prefect.git",
"branch": "definitely-does-not-exist-123",
"access_token": "super-secret-42".upper(),
}
}
)
assert "super-secret-42".upper() not in str(exc.getrepr())