From ebd281bf72cf7243080ce5bed432356c53968e3c Mon Sep 17 00:00:00 2001
From: William Chu <william.chu@uptickhq.com>
Date: Fri, 11 Oct 2024 09:27:23 +1100
Subject: [PATCH 1/5] chore: add types to get_apps
---
gitops/utils/apps.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gitops/utils/apps.py b/gitops/utils/apps.py
index a2b18c5..2c481d5 100644
--- a/gitops/utils/apps.py
+++ b/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
@@ -63,7 +64,7 @@ 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,
From 95e72902f4cea2e84c75eb99a3165f979471069b Mon Sep 17 00:00:00 2001
From: William Chu <william.chu@uptickhq.com>
Date: Fri, 11 Oct 2024 09:51:12 +1100
Subject: [PATCH 2/5] chore: expose app.secrets and allow secrets to be
unencoded
---
gitops/common/app.py | 26 +++++++++++++++++++++-----
gitops/utils/apps.py | 8 ++++++--
tests/test_app.py | 15 +++++++++++++++
3 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/gitops/common/app.py b/gitops/common/app.py
index 898e00d..8e00094 100644
--- a/gitops/common/app.py
+++ b/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
@@ -61,11 +72,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 +138,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
diff --git a/gitops/utils/apps.py b/gitops/utils/apps.py
index 2c481d5..51c1c4f 100644
--- a/gitops/utils/apps.py
+++ b/gitops/utils/apps.py
@@ -22,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:
@@ -68,6 +71,7 @@ def get_apps( # noqa: C901
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.
@@ -102,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:
diff --git a/tests/test_app.py b/tests/test_app.py
index 525bff0..f23f46c 100644
--- a/tests/test_app.py
+++ b/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"
From 626466a5034c532995e41b0cf476f182bb3679f0 Mon Sep 17 00:00:00 2001
From: William Chu <william.chu@uptickhq.com>
Date: Fri, 11 Oct 2024 09:55:00 +1100
Subject: [PATCH 3/5] feat(gitops): expose App, get_app_details, get_apps as
top level interfaces
---
gitops/__init__.py | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/gitops/__init__.py b/gitops/__init__.py
index d51cb91..276f6c8 100644
--- a/gitops/__init__.py
+++ b/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__"]
From 743abaa231817de2903ec7a6e7c03be68a0ddfea Mon Sep 17 00:00:00 2001
From: William Chu <william.chu@uptickhq.com>
Date: Fri, 11 Oct 2024 10:27:05 +1100
Subject: [PATCH 4/5] fix(gitops_server): treat suspended servers as removed
---
gitops_server/types.py | 24 +++++++++++++++---------
gitops_server/workers/deployer/deploy.py | 3 +--
tests/conftest.py | 3 +++
tests/test_deploy.py | 16 +++++++++++++++-
tests/utils.py | 10 ++++++++--
5 files changed, 42 insertions(+), 14 deletions(-)
create mode 100644 tests/conftest.py
diff --git a/gitops_server/types.py b/gitops_server/types.py
index e2865fa..dfe9073 100644
--- a/gitops_server/types.py
+++ b/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)
diff --git a/gitops_server/workers/deployer/deploy.py b/gitops_server/workers/deployer/deploy.py
index 6ad1e64..a71fc9d 100644
--- a/gitops_server/workers/deployer/deploy.py
+++ b/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
diff --git a/tests/conftest.py b/tests/conftest.py
new file mode 100644
index 0000000..db25ff0
--- /dev/null
+++ b/tests/conftest.py
@@ -0,0 +1,3 @@
+import gitops_server.settings
+
+gitops_server.settings.CLUSTER_NAME = "test-cluster"
diff --git a/tests/test_deploy.py b/tests/test_deploy.py
index d877b89..9107d62 100644
--- a/tests/test_deploy.py
+++ b/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
diff --git a/tests/utils.py b/tests/utils.py
index 73558d4..532012d 100644
--- a/tests/utils.py
+++ b/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))
From bc0a0a1cd65bd215bba35f0a50b746b8f72bfa5d Mon Sep 17 00:00:00 2001
From: William Chu <william.chu@uptickhq.com>
Date: Fri, 11 Oct 2024 10:29:02 +1100
Subject: [PATCH 5/5] chore: add __repr__ to App
---
gitops/common/app.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/gitops/common/app.py b/gitops/common/app.py
index 8e00094..df95fb8 100644
--- a/gitops/common/app.py
+++ b/gitops/common/app.py
@@ -58,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", [])