Merge pull request #83 from uptick/dev-857/delete-suspended-apps

DEV-857: treat suspended apps as removed from cluster

gitops/__init__.py

@@ -2,6 +2,8 @@
 import sys
 from pathlib import Path
 
+from gitops.utils.apps import App, get_app_details, get_apps
+
 from .utils.cli import success, warning
 
 __version__ = "0.11.5"
@@ -25,3 +27,5 @@
                 f" requirement {success(min_gitops_version)}.",
                 file=sys.stderr,
             )
+
+__all__ = ["App", "get_app_details", "get_apps", "__version__"]

gitops/common/app.py

@@ -24,8 +24,19 @@ def __init__(
         deployments: dict | None = None,
         secrets: dict | None = None,
         load_secrets: bool = True,
+        encode_secrets: bool = True,
+        # deprecated
         account_id: str = "",
     ) -> None:
+        """
+        :params name: The name of the app
+        :params path: The path to the app directory
+        :params deployments: The deployments dictionary (injecting for testing)
+        :params secrets: The secrets dictionary (injecting for testing)
+        :params load_secrets: Whether to load the secrets file
+        :params encode_secrets: Whether to base64 encode the secrets
+        """
+        self.encode_secrets = encode_secrets
         self.name = name
         self.path = path
         self.account_id = account_id
@@ -47,6 +58,9 @@ def __eq__(self, other: object) -> bool:
             and json.dumps(self.values, sort_keys=True) == json.dumps(other.values, sort_keys=True)
         )
 
+    def __repr__(self) -> str:
+        return f"App(name={self.name}, cluster={self.cluster}, tags={self.tags})"
+
     def is_inactive(self) -> bool:
         return "inactive" in self.values.get("tags", [])
 
@@ -61,11 +75,11 @@ def set_value(self, path: str, value: Any) -> None:
             current_dict = current_dict.setdefault(key, {})
         current_dict[keys[-1]] = value
 
-    def _make_values(self, deployments: dict, secrets: dict) -> dict:
-        values = {
-            **deployments,
-            "secrets": {**{k: b64encode(v.encode()).decode() for k, v in secrets.items()}},
-        }
+    def _make_values(self, deployments: dict, secrets: dict[str, str]) -> dict:
+        def encode(value: str) -> str:
+            return b64encode(str(value).encode()).decode() if self.encode_secrets else value
+
+        values = {**deployments, "secrets": {**{k: encode(v) for k, v in secrets.items()}}}
 
         image = self._make_image(deployments)
         if image:
@@ -127,6 +141,11 @@ def tags(self) -> list[str]:
     def service_account_name(self) -> str:
         return self.values.get("serviceAccount", {}).get("name") or self.values.get("serviceAccountName") or "default"
 
+    @property
+    def secrets(self) -> dict[str, str]:
+        # TODO: This should be a first class property
+        return self.values.get("secrets", {})
+
 
 class Chart:
     """Represents a Helm chart

gitops/utils/apps.py

@@ -1,5 +1,6 @@
 import sys
 from pathlib import Path
+from typing import Literal
 
 from colorama import Fore
 from tabulate import tabulate
@@ -21,13 +22,16 @@ def is_valid_app_directory(directory: Path) -> bool:
     return all(file_paths)
 
 
-def get_app_details(app_name: str, load_secrets: bool = True, exit_if_not_found: bool = True) -> App:
+def get_app_details(
+    app_name: str, load_secrets: bool = True, encode_secrets: bool = True, exit_if_not_found: bool = True
+) -> App:
     account_id = get_account_id() if load_secrets else "UNKNOWN"
     try:
         app = App(
             app_name,
             path=str(get_apps_directory() / app_name),
             load_secrets=load_secrets,
+            encode_secrets=encode_secrets,
             account_id=account_id,
         )
     except FileNotFoundError as e:
@@ -63,10 +67,11 @@ def update_app(app_name: str, **kwargs: object) -> None:
 def get_apps(  # noqa: C901
     filter: set[str] | list[str] | str = "",
     exclude: set[str] | list[str] | str = "",
-    mode: str = "PROMPT",
+    mode: Literal["PROMPT", "PREVIEW", "SILENT"] | None = "PROMPT",
     autoexclude_inactive: bool = True,
     message: str | None = None,
     load_secrets: bool = True,
+    encode_secrets: bool = True,
 ) -> list[App]:
     """Return apps that contain ALL of the tags listed in `filter` and NONE of the tags listed in
     `exclude`. The incoming filter and exclude params may come in as a list or commastring.
@@ -101,7 +106,7 @@ def get_apps(  # noqa: C901
             continue
         elif not is_valid_app_directory(entry):
             continue
-        app = get_app_details(entry.name, load_secrets=load_secrets)
+        app = get_app_details(entry.name, load_secrets=load_secrets, encode_secrets=encode_secrets)
 
         pseudotags = [app.name, app.cluster]
         if app.image and app.image_prefix:

gitops_server/types.py

@@ -19,16 +19,22 @@ class UpdateAppResult(RunOutput):
 
 
 class AppDefinitions:
-    def __init__(self, name, apps: dict | None = None):
+    def __init__(self, name, apps: dict[str, App] | None = None, path: str | None = None):
         self.name = name
         self.apps = apps or {}
 
-    def from_path(self, path: str):
-        path = os.path.join(path, "apps")
-        for entry in os.listdir(path):
-            entry_path = os.path.join(path, entry)
-            if entry[0] != "." and not os.path.isfile(entry_path):
-                app = App(entry, entry_path, account_id=settings.ACCOUNT_ID)
-                # We only care for apps pertaining to our current cluster.
-                if app.values["cluster"] == settings.CLUSTER_NAME:
+        if path:
+            path = os.path.join(path, "apps")
+
+            for entry in os.listdir(path):
+                entry_path = os.path.join(path, entry)
+                if entry[0] != "." and not os.path.isfile(entry_path):
+                    app = App(entry, entry_path, account_id=settings.ACCOUNT_ID)
                     self.apps[entry] = app
+
+        # Removing apps that are suspended or not part of this cluster
+        for app in list(self.apps.values()):
+            # We only care for apps pertaining to our current cluster.
+            if app.values["cluster"] != settings.CLUSTER_NAME or "suspended" in app.tags:
+                # and suspended apps are considered removed from the cluster.
+                self.apps.pop(app.name)

gitops_server/workers/deployer/deploy.py

@@ -67,8 +67,7 @@ async def post_result_summary(source: str, results: list[UpdateAppResult]):
 async def load_app_definitions(url: str, sha: str) -> AppDefinitions:
     logger.info(f'Loading app definitions at "{sha}".')
     async with temp_repo(url, ref=sha) as repo:
-        app_definitions = AppDefinitions(name=get_repo_name_from_url(url))
-        app_definitions.from_path(repo)
+        app_definitions = AppDefinitions(name=get_repo_name_from_url(url), path=repo)
         return app_definitions
 
 

tests/conftest.py

@@ -0,0 +1,3 @@
+import gitops_server.settings
+
+gitops_server.settings.CLUSTER_NAME = "test-cluster"

tests/test_app.py

@@ -105,3 +105,18 @@ def test_local_repo_is_parsed_properly(self):
 
         assert chart.type == "local"
         assert chart.path == "."
+
+    def test_app_namespace_property(self):
+        path = create_test_yaml()
+        app = App("test", path, encode_secrets=True)
+
+        assert app.namespace
+
+    def test_app_encode_secrets_works(self):
+        path = create_test_yaml()
+        app = App("test", path, encode_secrets=True)
+
+        assert app.secrets["SNAPE"] != "KILLS_DUMBLEDORE"
+
+        app_decoded = App("test", path, encode_secrets=False)
+        assert app_decoded.secrets["SNAPE"] == "KILLS_DUMBLEDORE"

tests/test_deploy.py

@@ -4,10 +4,11 @@
 import pytest
 
 from gitops.common.app import App
+from gitops_server.types import AppDefinitions
 from gitops_server.workers.deployer import Deployer
 
 from .sample_data import SAMPLE_GITHUB_PAYLOAD, SAMPLE_GITHUB_PAYLOAD_SKIP_MIGRATIONS
-from .utils import mock_load_app_definitions
+from .utils import create_test_yaml, mock_load_app_definitions
 
 # Patch gitops_server.git.run & check correct commands + order
 # Patch command that reads yaml from cluster repo +
@@ -120,3 +121,16 @@ async def test_deployer_skip_migrations_in_commit_message_should_run_helm_withou
         ]
         for where, check in check_in_run_mock:
             assert check in post_mock.call_args_list[where][0][0]
+
+
+class TestLoadAppDefinitions:
+    def test_load_app_definitions_ignores_suspended_apps(self):
+        app_definitions = AppDefinitions(
+            "mock-repo",
+            apps={
+                "in-cluster": App("in-cluster", path=create_test_yaml(tags=[])),
+                "suspended": App("suspended", path=create_test_yaml(tags=["suspended"])),
+                "not-in-cluster": App("not-in-cluster", path=create_test_yaml(tags=[], cluster="other-cluster")),
+            },
+        )
+        assert len(app_definitions.apps) == 1

tests/utils.py

@@ -1,3 +1,5 @@
+from typing import Any
+
 import yaml
 
 from gitops.common.app import App
@@ -20,14 +22,14 @@ async def mock_load_app_definitions(url, sha):
     return app_definitions
 
 
-def create_test_yaml(fg=4, bg=2):
+def create_test_yaml(fg=4, bg=2, **kwargs: Any):
     data = {
         "chart": "https://github.com/some/chart",
         "images": {"template": "template-tag-{tag}"},
         "namespace": "mynamespace",
         "tags": ["tag1", "tag2"],
         "image-tag": "myimagetag",
-        "cluster": "UNKNOWN",
+        "cluster": "test-cluster",
         "containers": {"fg": {"replicas": fg}, "bg": {"replicas": bg}},
         "environment": {
             "DJANGO_SETTINGS_MODULE": "my.settings.module",
@@ -36,6 +38,10 @@ def create_test_yaml(fg=4, bg=2):
             "MEDIA_CLOUDINARY_PREFIX": "cloudinaryprefix",
         },
     }
+
+    for k, v in kwargs.items():
+        data[k] = v
+
     with open("/tmp/deployment.yml", "w+") as fh:
         fh.write(yaml.dump(data))