From 5ebfad2b7844b73a45b301dcd594c16efc76b186 Mon Sep 17 00:00:00 2001
From: Kimonas Sotirchos <kimwnasptd@gmail.com>
Date: Thu, 5 Sep 2024 11:42:39 +0300
Subject: [PATCH] review: Use TemporaryDirectory for cloning

---
 scripts/get-all-images.py | 73 +++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 34 deletions(-)

diff --git a/scripts/get-all-images.py b/scripts/get-all-images.py
index a42beecf..2b53967b 100644
--- a/scripts/get-all-images.py
+++ b/scripts/get-all-images.py
@@ -1,12 +1,16 @@
 import argparse
 import logging
-import shutil
 import subprocess
+import os
 import sys
+import contextlib
+import tempfile
 
 import git
 import yaml
 
+from typing import Iterator
+
 # logging
 LOG_FORMAT = "%(levelname)s \t| %(message)s"
 logging.basicConfig(format=LOG_FORMAT, level=logging.INFO)
@@ -59,7 +63,8 @@ def bundle_app_contains_gh_metadata(app: dict) -> bool:
     return False
 
 
-def clone_repo(repo_name: str, branch: str):
+@contextlib.contextmanager
+def clone_git_repo(repo_name: str, branch: str) -> Iterator[git.PathLike]:
     """
     Clones locally a repo and returns the path of the folder created.
 
@@ -69,11 +74,16 @@ def clone_repo(repo_name: str, branch: str):
     """
     repo_url = f"https://github.com/canonical/{repo_name}.git"
 
-    logging.info(f"Cloning repo {repo_url}")
-    repo = git.Repo.clone_from(repo_url, str(repo_name))
+    # we can't use the default /tmp/ dir because of
+    # https://github.com/mikefarah/yq/issues/1808
+    with tempfile.TemporaryDirectory(dir=os.getcwd()) as tmp:
+        logging.info(f"Cloning repo {repo_url}")
+        repo = git.Repo.clone_from(repo_url, tmp)
+
+        logging.info(f"Checking out to branch {branch}")
+        repo.git.checkout(branch)
 
-    logging.info(f"Checking out to branch {branch}")
-    repo.git.checkout(branch)
+        yield repo.working_dir
 
 
 def get_analytics_app_images(app: dict) -> list[str]:
@@ -87,21 +97,19 @@ def get_analytics_app_images(app: dict) -> list[str]:
     script will also fail.
     """
     images = []
-    repo_dir = app[GH_REPO_KEY]
-    clone_repo(app[GH_REPO_KEY], app[GH_BRANCH_KEY])
-
-    logging.info(f"Executing repo's {GET_IMAGES_SH} script")
-    try:
-        process = subprocess.run(["bash", "tools/get-images.sh"],
-                                 cwd=repo_dir, capture_output=True,
-                                 text=True, check=True)
-    except subprocess.CalledProcessError as exc:
-        logging.error("Script '%s' for charm '%s' failed: %s",
-                      GET_IMAGES_SH, app["charm"], exc.stderr)
-        raise exc
-    finally:
-        # cleanup
-        shutil.rmtree(repo_dir)
+    repo_name = app[GH_REPO_KEY]
+    repo_branch = app[GH_BRANCH_KEY]
+
+    with clone_git_repo(repo_name, repo_branch) as repo_dir:
+        logging.info(f"Executing repo's {GET_IMAGES_SH} script")
+        try:
+            process = subprocess.run(["bash", "tools/get-images.sh"],
+                                     cwd=repo_dir, capture_output=True,
+                                     text=True, check=True)
+        except subprocess.CalledProcessError as exc:
+            logging.error("Script '%s' for charm '%s' failed: %s",
+                          GET_IMAGES_SH, app["charm"], exc.stderr)
+            raise exc
 
     images = process.stdout.strip().split("\n")
 
@@ -120,24 +128,21 @@ def get_dependency_app_images(app: dict) -> list[str]:
     3. Delete the repo
     """
     images = []
-    repo_dir = app[GH_DEPENDENCY_REPO_KEY]
-    clone_repo(app[GH_DEPENDENCY_REPO_KEY],
-               app[GH_DEPENDENCY_BRANCH_KEY])
-    with open("%s/metadata.yaml" % repo_dir, 'r') as metadata_file:
-        metadata_dict = yaml.safe_load(metadata_file)
+    repo_name = app[GH_DEPENDENCY_REPO_KEY]
+    repo_branch = app[GH_DEPENDENCY_BRANCH_KEY]
+    with clone_git_repo(repo_name, repo_branch) as repo_dir:
+        with open("%s/metadata.yaml" % repo_dir, 'r') as metadata_file:
+            metadata_dict = yaml.safe_load(metadata_file)
 
-    for _, rsrc in metadata_dict["resources"].items():
-        if "type" in rsrc and rsrc["type"] == "oci-image":
-            img = rsrc["upstream-source"]
-            logging.info("Found image %s" % img)
-            images.append(img)
+        for _, rsrc in metadata_dict["resources"].items():
+            if rsrc.get("type", "") != "oci-image":
+                continue
 
-    # cleanup
-    shutil.rmtree(repo_dir)
+            images.append(rsrc["upstream-source"])
 
     logging.info("Found the following images:")
     for image in images:
-        logging.info(image)
+        logging.info("* " + image)
 
     return images