devopshq / artifactory-cleanup

Extended cleanup tool for JFrog Artifactory
MIT License
112 stars 61 forks source link

Bug: KeepLatestNVersionImagesByProperty - ValueError: Can not find version in [..] for multi-arch builds (buildx) #137

Open macgeneral opened 5 months ago

macgeneral commented 5 months ago

Hey there,

Thank you for maintaining this great project.

I stumbled across an issue in regards of how I build and tag my docker images.

I first build and push them by digest (so without a tag/label) to build them in a GitHub matrix action for multiple architectures. In a second step I join those images into one manifest using docker buildx imagetools create.

If artifactory-cleanup stumbles upon such images it crashes if there's a KeepLatestNVersionImagesByProperty rule.

- rule: KeepLatestNVersionImagesByProperty
  count: 3
  number_of_digits_in_version: 3
  custom_regexp: "(^v?(\\d+)+(\\.\\d+)*|(_\\d{4}\\-\\d{2})*$)"

ValueError: Can not find version in [..]

{
    "repo": "[..]",
    "path": "[..]",
    "name": "sha256:72958d1c85825ab3a53db3f4fa620a67f4ab6f9e8146ac2c7e299ae3885b6b4d",
    "type": "file",
    "created": "2024-03-19T13:28:05.099Z",
    "created_by": "[..]",
    "modified": "2024-03-19T13:28:05.065Z",
    "modified_by": "[..]",
    "updated": "2024-03-19T13:28:05.102Z",
    "depth": 4,
    "actual_md5": "d54d8029f9a6f4d76a18696c3e01c8bf",
    "actual_sha1": "e1c95c82ba22a7c9dc689adf86bf9d1c32bc7b0c",
    "properties": {
        "docker.repoName": "[..]",
        "docker.label.description": "[..]",
        "docker.manifest.type": "application/vnd.oci.image.manifest.v1+json",
        "docker.label.org.opencontainers.image.created": "2024-03-19T13:26:41.394Z",
        "docker.label.org.opencontainers.image.licenses": None,
        "docker.label.version": "1.0",
        "docker.label.org.opencontainers.image.revision": "133037faaf57c9b28e7d449072598faef83f10d7",
        "docker.label.org.opencontainers.image.url": "[..]",
        "docker.manifest.digest": "sha256:72958d1c85825ab3a53db3f4fa620a67f4ab6f9e8146ac2c7e299ae3885b6b4d",
        "artifactory.content-type": "application/vnd.oci.image.manifest.v1+json",
        "docker.label.org.opencontainers.image.title": "[..]",
        "sha256": "72958d1c85825ab3a53db3f4fa620a67f4ab6f9e8146ac2c7e299ae3885b6b4d",
        "docker.label.org.opencontainers.image.version": "main",
        "oci.artifact.type": "application/vnd.oci.image.config.v1+json",
        "docker.label.maintainer": "[..]",
        "docker.label.org.opencontainers.image.description": None,
        "docker.manifest": "sha256:72958d1c85825ab3a53db3f4fa620a67f4ab6f9e8146ac2c7e299ae3885b6b4d",
        "docker.label.org.opencontainers.image.source": "[..]",
    },
    "repo_path_checksum": "94d553bb399d1d974b98fd33a1f8abb8c25db77b",
    "sha256": "72958d1c85825ab3a53db3f4fa620a67f4ab6f9e8146ac2c7e299ae3885b6b4d",
    "stats": {
        "downloaded": "2024-03-19T13:38:14.093Z",
        "downloaded_by": "[..]",
        "downloads": 1,
        "remote_downloads": 0,
    },
    "virtual_repos": [],
    "size": 48070401,
}

Note: I did redact some values with [..], also the docker.label.version property is manually defined in the Dockerfile and independent from tags etc.

A quick and easy fix for this would be to add those two lines after https://github.com/devopshq/artifactory-cleanup/blob/7e5ba2a75c5bd547d0c4df6c099069715ff1ce10/artifactory_cleanup/rules/docker.py#L216

def get_version(self, artifact) -> Tuple:
    """Parse property and get version from it"""
    value = artifact["properties"][self.property]
+   if value.startswith("sha256:"):
+       value = artifact["properties"].get("docker.label.org.opencontainers.image.version")
    match = re.match(self.custom_regexp, value)
    if not match:
        raise ValueError(f"Can not find version in '{artifact}'")
    version_str = match.group()
    if version_str.startswith("v"):
        version_str = version_str[1:]
        return tuple(["v"] + list(map(int, version_str.split("."))))
    version = tuple(map(int, version_str.split(".")))
    return version

Unfortunately this still results in a crash if it's something completely unexpected like a branch name etc.

Is it possible to not raise an error in this case and simply log a warning to stdout? I would expect artifactory-cleanup to treat those images according to the remaining rules (e.g. DeleteNotUsedSince et al).

allburov commented 5 months ago

Is it possible to not raise an error in this case and simply log a warning to stdout?

It's a solution. We should either explicitly filter out not valid artifacts by using other rules before passing it to the KeepLatestNFilesInFolder.

KeepLatestNFilesInFolder was written when there was "docker buildx imagetools create" so feel free to adjust and contiribute the solution if anyone find it!