Skip to content

Commit

Permalink
Do not delete sig/att/sbom if subject image exists (#116)
Browse files Browse the repository at this point in the history
when getting all tags in registry we are listing only active tags,
if tag was removed for the image, image still will be in registry
if it is part of image index

if manifest digest isn't present in any of active tags,
try query also manifest digest to verify its presence

also re-enable pruner

STONEBLD-2466

Signed-off-by: Robert Cerven <[email protected]>
  • Loading branch information
rcerven authored May 27, 2024
1 parent abe39f6 commit 461daea
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 11 deletions.
2 changes: 1 addition & 1 deletion config/registry_image_pruner/cronjob.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ spec:
command:
- /bin/bash
- '-c'
- echo 'The pruner is temporarily disabled, see https://github.com/konflux-ci/image-controller/pull/115'
- python /image-pruner/prune_images.py --namespace $(NAMESPACE)
volumeMounts:
- name: image-pruner-volume
mountPath: /image-pruner
Expand Down
40 changes: 35 additions & 5 deletions config/registry_image_pruner/image_pruner/prune_images.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,49 @@ def delete_image_tag(quay_token: str, namespace: str, name: str, tag: str) -> No
raise(ex)


def manifest_exists(quay_token: str, namespace: str, name: str, manifest: str) -> bool:
api_url = f"{QUAY_API_URL}/repository/{namespace}/{name}/manifest/{manifest}"
request = Request(api_url, headers={
"Authorization": f"Bearer {quay_token}",
})
resp: HTTPResponse
manifest_exists = True
try:
with urlopen(request) as resp:
if resp.status != 200 and resp.status != 204:
raise RuntimeError(resp.reason)

except HTTPError as ex:
if ex.status != 404:
raise(ex)
else:
manifest_exists = False

return manifest_exists


def remove_tags(tags: List[Dict[str, Any]], quay_token: str, namespace: str, name: str, dry_run: bool = False) -> None:
image_digests = [image["manifest_digest"] for image in tags]
tags_map = {tag_info["name"]: tag_info for tag_info in tags}
tag_regex = re.compile(r"^sha256-([0-9a-f]+)(\.sbom|\.att|\.src|\.sig)$")
manifests_checked = {}
for tag in tags:
# attestation or sbom image
if (match := tag_regex.match(tag["name"])) is not None:
if f"sha256:{match.group(1)}" not in image_digests:
if dry_run:
LOGGER.info("Tag %s from %s/%s should be removed", tag["name"], namespace, name)
else:
LOGGER.info("Removing tag %s from %s/%s", tag["name"], namespace, name)
delete_image_tag(quay_token, namespace, name, tag["name"])
# verify that manifest really doesn't exist, because if tag was removed, it won't be in tag list, but may still be in the registry
manifest_existence = manifests_checked.get(f"sha256:{match.group(1)}")
if manifest_existence is None:
manifest_existence = manifest_exists(quay_token, namespace, name, f"sha256:{match.group(1)}")
manifests_checked[f"sha256:{match.group(1)}"] = manifest_existence

if not manifest_existence:
if dry_run:
LOGGER.info("Tag %s from %s/%s should be removed", tag["name"], namespace, name)
else:
LOGGER.info("Removing tag %s from %s/%s", tag["name"], namespace, name)
delete_image_tag(quay_token, namespace, name, tag["name"])

elif tag["name"].endswith(".src"):
to_delete = False

Expand Down
74 changes: 69 additions & 5 deletions config/registry_image_pruner/image_pruner/test_prune_images.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ def test_no_image_with_expected_suffixes_is_found(self, delete_image_tag, urlope
@patch.dict(os.environ, {"QUAY_TOKEN": QUAY_TOKEN})
@patch("sys.argv", ["prune_images", "--namespace", "sample"])
@patch("prune_images.urlopen")
def test_remove_orphan_tags_with_expected_suffixes(self, urlopen):
@patch("prune_images.manifest_exists")
def test_remove_orphan_tags_with_expected_suffixes(self, manifest_exists, urlopen):
fetch_repos_rv = MagicMock()
response = MagicMock()
response.status = 200
Expand Down Expand Up @@ -187,6 +188,19 @@ def test_remove_orphan_tags_with_expected_suffixes(self, urlopen):
delete_tag_rv,
]

manifest_exists.side_effect = [
False,
False,
False,
False,
False,
False,
False,
False,
False,
False,
]

main()

def _assert_deletion_request(request: Request, tag: str) -> None:
Expand All @@ -207,7 +221,8 @@ def _assert_deletion_request(request: Request, tag: str) -> None:
@patch.dict(os.environ, {"QUAY_TOKEN": QUAY_TOKEN})
@patch("sys.argv", ["prune_images", "--namespace", "sample", "--dry-run"])
@patch("prune_images.urlopen")
def test_remove_tag_dry_run(self, urlopen):
@patch("prune_images.manifest_exists")
def test_remove_tag_dry_run(self, manifest_exists, urlopen):
fetch_repos_rv = MagicMock()
response = MagicMock()
response.status = 200
Expand Down Expand Up @@ -236,6 +251,9 @@ def test_remove_tag_dry_run(self, urlopen):
# return the repo info including tags
get_repo_rv,
]
manifest_exists.side_effect = [
False,
]

with self.assertLogs(LOGGER) as logs:
main()
Expand Down Expand Up @@ -297,7 +315,8 @@ def test_handle_image_repos_pagination(self, urlopen):
class TestRemoveTags(unittest.TestCase):

@patch("prune_images.delete_image_tag")
def test_remove_tags(self, delete_image_tag):
@patch("prune_images.manifest_exists")
def test_remove_tags(self, manifest_exists, delete_image_tag):
tags = [
{
"name": "sha256-502c8c35e31459e8774f88e115d50d2ad33ba0e9dfd80429bc70ed4c1fd9e0cd.att",
Expand All @@ -309,6 +328,11 @@ def test_remove_tags(self, delete_image_tag):
},
]

manifest_exists.side_effect = [
False,
False,
]

with self.assertLogs(LOGGER) as logs:
remove_tags(tags, QUAY_TOKEN, "some", "repository")
logs_output = "\n".join(logs.output)
Expand All @@ -320,7 +344,8 @@ def test_remove_tags(self, delete_image_tag):
delete_image_tag.assert_has_calls(calls)

@patch("prune_images.delete_image_tag")
def test_remove_tags_dry_run(self, delete_image_tag):
@patch("prune_images.manifest_exists")
def test_remove_tags_dry_run(self, manifest_exists, delete_image_tag):
tags = [
{
"name": "sha256-502c8c35e31459e8774f88e115d50d2ad33ba0e9dfd80429bc70ed4c1fd9e0cd.att",
Expand All @@ -332,6 +357,11 @@ def test_remove_tags_dry_run(self, delete_image_tag):
}
]

manifest_exists.side_effect = [
False,
False,
]

with self.assertLogs(LOGGER) as logs:
remove_tags(tags, QUAY_TOKEN, "some", "repository", dry_run=True)
logs_output = "\n".join(logs.output)
Expand Down Expand Up @@ -364,7 +394,34 @@ def test_remove_tags_nothing_to_remove(self, delete_image_tag):
delete_image_tag.assert_not_called()

@patch("prune_images.delete_image_tag")
def test_remove_tags_multiple_tags(self, delete_image_tag):
@patch("prune_images.manifest_exists")
def test_remove_tags_nothing_to_remove_digest_exists(self, manifest_exists, delete_image_tag):
tags = [
{
"name": "sha256-502c8c35e31459e8774f88e115d50d2ad33ba0e9dfd80429bc70ed4c1fd9e0cd.att",
"manifest_digest": "sha256:125c1d18ee1c3b9bde0c7810fcb0d4ffbc67e9b0c5b88bb8df9ca039bc1c9457",
},
{
"name": "sha256-502c8c35e31459e8774f88e115d50d2ad33ba0e9dfd80429bc70ed4c1fd9e0cd.sbom",
"manifest_digest": "sha256:351326f899759a9a7ae3ca3c1cbdadcc8012f43231c145534820a68bdf36d55b",
},
]

manifest_exists.side_effect = [
True,
True,
]

with self.assertRaisesRegex(AssertionError, expected_regex="no logs of level INFO"):
with self.assertLogs(LOGGER) as logs:
remove_tags(tags, QUAY_TOKEN, "some", "repository", dry_run=True)

delete_image_tag.assert_not_called()


@patch("prune_images.delete_image_tag")
@patch("prune_images.manifest_exists")
def test_remove_tags_multiple_tags(self, manifest_exists, delete_image_tag):
tags = [
{
"name": "sha256-502c8c35e31459e8774f88e115d50d2ad33ba0e9dfd80429bc70ed4c1fd9e0cd.att",
Expand All @@ -384,6 +441,13 @@ def test_remove_tags_multiple_tags(self, delete_image_tag):
},
]

manifest_exists.side_effect = [
False,
False,
False,
False,
]

with self.assertLogs(LOGGER) as logs:
remove_tags(tags, QUAY_TOKEN, "some", "repository")
logs_output = "\n".join(logs.output)
Expand Down

0 comments on commit 461daea

Please sign in to comment.