Skip to content

Commit

Permalink
fix(docker): fix crash when scanning a non-local image
Browse files Browse the repository at this point in the history
If we run `ggshield scan docker <image>` and `<image>` is not available
locally, `ggshield` downloads it first.

I broke this feature when I made other Docker fixes: ggshield currently
raises an exception at the end of the download.

Fix that and add tests to cover that case to ensure that cannot go
undetected anymore.

Fixes #181.
  • Loading branch information
agateau-gg committed Mar 29, 2022
1 parent e844671 commit cfbaf17
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 4 deletions.
7 changes: 4 additions & 3 deletions ggshield/docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ def docker_save_to_tmp(image_name: str, destination_path: Path, timeout: int) ->
docker_pull_image(image_name, timeout)

docker_save_to_tmp(image_name, destination_path, timeout)
raise click.ClickException(
f"Unable to save docker archive:\nError: {err_string}"
)
else:
raise click.ClickException(
f"Unable to save docker archive:\nError: {err_string}"
)
except subprocess.TimeoutExpired:
raise click.ClickException('Command "{}" timed out'.format(" ".join(command)))

Expand Down
46 changes: 45 additions & 1 deletion tests/test_docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def test_docker_save_image_success(self):
timeout=DOCKER_TIMEOUT,
)

def test_docker_save_image_non_exist(self):
def test_docker_save_image_does_not_exist(self):
with patch(
"subprocess.run",
side_effect=subprocess.CalledProcessError(
Expand All @@ -89,6 +89,50 @@ def test_docker_save_image_non_exist(self):
"ggshield-non-existant", self.TMP_ARCHIVE, DOCKER_TIMEOUT
)

def test_docker_save_image_need_pull(self):
"""
GIVEN a Docker image we do not have locally
WHEN we try to save it
THEN we first pull it and then save it
This test expects the following calls to `docker` commands:
- docker save <image_name> -o <something>
-> Fake failure
- docker pull <image_name>
-> Fake success
- docker save <image_name> -o <something>
-> Fake success
"""
with patch(
"subprocess.run",
side_effect=[
subprocess.CalledProcessError(
1, cmd=[], stderr="reference does not exist".encode("utf-8")
),
None,
None,
],
):
docker_save_to_tmp(
"ggshield-non-existant", self.TMP_ARCHIVE, DOCKER_TIMEOUT
)

def test_docker_save_image_fails(self):
with patch(
"subprocess.run",
side_effect=subprocess.CalledProcessError(
1, cmd=[], stderr="docker failed weirdly".encode("utf-8")
),
):
with pytest.raises(
click.exceptions.ClickException,
match="Unable to save docker archive:",
):
docker_save_to_tmp(
"ggshield-non-existant", self.TMP_ARCHIVE, DOCKER_TIMEOUT
)

def test_docker_save_image_timeout(self):
with patch(
"subprocess.run",
Expand Down

0 comments on commit cfbaf17

Please sign in to comment.