Conversation
…and to be flexible, verifying all necessary directories and creating them if missing
…multicloud perspective
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR refactors the macro command implementation into a dedicated Babylon/commands/macro/helpers/ package, migrates Babylon “remote state” storage from Azure Blob Storage to Kubernetes Secrets, and adjusts webapp destroy behavior to rely on Terraform state rather than Babylon’s state file.
Changes:
- Extracted shared macro logic (ACL/security diffing, workspace K8s resources + Postgres schema jobs, Terraform webapp operations) into new helper modules.
- Reworked namespace/state handling to remove
state-idand persist/list remote states via Kubernetes Secrets. - Updated templates and test scripts to match the new CLI behavior (
babylon namespace useno longer takes-s,babylon initnow requires a provider argument).
Reviewed changes
Copilot reviewed 29 out of 32 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Dependency lock updates (incl. cosmotech-api bump) to support the refactor and environment changes. |
tests/unit/test_macro.py |
Updates unit test imports to point to the new shared macro helper module. |
tests/integration/test_api_endpoints.sh |
Adjusts namespace CLI usage to remove -s/--state-id. |
tests/e2e/test_e2e.sh |
Adjusts namespace CLI usage and updates babylon init invocation to require a provider argument. |
Babylon/utils/kubernetes_state.py |
Adds new Kubernetes Secret-based state persistence/retrieval utility. |
Babylon/utils/environment.py |
Removes Azure Blob state handling, adds Kubernetes-backed remote state + updated namespace persistence. |
Babylon/utils/decorators.py |
Removes --state-id injection and updates context/tenant option docs. |
Babylon/templates/working_dir/terraform_cloud/tfc_workspace_create.yaml |
Removes Terraform Cloud workspace template (no longer used). |
Babylon/templates/working_dir/terraform_cloud/tfc_variables_create.yaml |
Removes Terraform Cloud variables template (no longer used). |
Babylon/templates/working_dir/.templates/yaml/kob/Webapp.yaml |
Adds KOB/on-prem webapp manifest template. |
Babylon/templates/working_dir/.templates/yaml/kob/variables.yaml |
Adds KOB/on-prem variables template. |
Babylon/templates/working_dir/.templates/yaml/dataset/customers.csv |
Removes dataset CSV template. |
Babylon/templates/working_dir/.templates/yaml/azure/Webapp.yaml |
Adds Azure webapp manifest template. |
Babylon/templates/working_dir/.templates/yaml/azure/variables.yaml |
Expands Azure variables template with clearer guidance/comments. |
Babylon/templates/working_dir/.templates/webapp/webapp_details.yaml |
Removes legacy webapp details template. |
Babylon/templates/working_dir/.templates/webapp/webapp_config.yaml |
Removes legacy webapp config template. |
Babylon/templates/working_dir/.templates/webapp/app_insight.json |
Removes legacy Application Insights template. |
Babylon/commands/namespace/get_contexts.py |
Updates output to remove STATE ID column. |
Babylon/commands/namespace/get_all_states.py |
Refactors local/remote state listing; remote now points to Kubernetes Secrets. |
Babylon/commands/macro/init.py |
Reworks project scaffolding; init now requires a provider argument and uses provider-scoped templates. |
Babylon/commands/macro/helpers/workspace.py |
New module consolidating workspace CRUD, K8s Secret/ConfigMap lifecycle, and Postgres schema jobs. |
Babylon/commands/macro/helpers/webapp.py |
New module consolidating Terraform webapp apply/destroy helpers. |
Babylon/commands/macro/helpers/common.py |
New module consolidating include/exclude resolution and ACL/security update helpers. |
Babylon/commands/macro/helpers/__init__.py |
Adds helpers package init. |
Babylon/commands/macro/destroy.py |
Converts macro destroy into a thin orchestrator that calls helper modules; switches remote state sync to Kubernetes. |
Babylon/commands/macro/deploy.py |
Removes the old monolithic deploy helper module (split into helpers). |
Babylon/commands/macro/deploy_workspace.py |
Converts workspace deploy into an orchestrator calling the new workspace helpers; remote state sync to Kubernetes. |
Babylon/commands/macro/deploy_webapp.py |
Switches Terraform helper import to the new webapp helper module; tightens chmod. |
Babylon/commands/macro/deploy_solution.py |
Switches shared ACL/security helper import and remote state sync to Kubernetes. |
Babylon/commands/macro/deploy_organization.py |
Switches shared ACL/security helper import and remote state sync to Kubernetes. |
Babylon/commands/macro/apply.py |
Updates include/exclude helper import to the new shared module. |
.github/workflows/ci.yml |
Removes Azure Blob env secrets and clarifies step names for Azure login/AKS context. |
Comments suppressed due to low confidence (1)
Babylon/utils/environment.py:282
- The guidance printed when
namespace.yamlis missing is out of date: it suggestsbabylon namespace use <tenant-name>, but thenamespace usecommand now requires-c/--contextand-t/--tenant. Updating this message will prevent user confusion when they hit this error path.
logger.error(f" [bold red]✘[/bold red] [cyan]{ns_file}[/cyan] not found")
logger.info(" Run the following command to set your active namespace:")
logger.info(" [cyan]babylon namespace use <tenant-name>[/cyan]")
sys.exit(1)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| v1.read_namespaced_secret(name=secret_name, namespace=namespace) | ||
| # Secret exists → replace it. |
There was a problem hiding this comment.
replace_namespaced_secret is called with a freshly built Secret object that lacks metadata.resourceVersion. The Kubernetes API typically requires resourceVersion for replace/PUT updates, so this path can fail when the secret already exists. Consider using patch_namespaced_secret, or read the existing Secret first and set secret.metadata.resource_version = existing.metadata.resource_version before replacing.
| v1.read_namespaced_secret(name=secret_name, namespace=namespace) | |
| # Secret exists → replace it. | |
| existing_secret = v1.read_namespaced_secret(name=secret_name, namespace=namespace) | |
| # Secret exists → replace it. Kubernetes replace requests require | |
| # the current resourceVersion to be included. | |
| if existing_secret.metadata and secret.metadata: | |
| secret.metadata.resource_version = existing_secret.metadata.resource_version |
There was a problem hiding this comment.
I think we need to add a version check in the run terraform, initial init and destroy webapp. Babylon is always cloning the latest webapp-terraform so with the next release of webapp-terraform current versions of Babylon might get into trouble. Same for a webapp that we want to destroy but it was created with a previous version of webapp-terraform. Better to check for version compatibility and throw a warning or let the user do it manually if the versions are not aligned
| export TENANT="tenant-sphinx" | ||
| export STATE="teststate" | ||
|
|
||
| babylon namespace use -c ${CONTEXT} -t ${TENANT} -s $STATE |
| except Exception as e: | ||
| logger.error(f" [bold red]✘[/bold red] Failed to list remote states: {e}") | ||
| return [] | ||
| def get_state_from_kubernetes(self, namespace: str = "", secret_name: str = "") -> dict: |
There was a problem hiding this comment.
There is a function with the exact same name still in the environment.py, do we still need both?
| import subprocess | ||
| from base64 import b64encode | ||
| from logging import getLogger | ||
| from pathlib import Path as PathlibPath |
There was a problem hiding this comment.
the as PathlibPath changes the name of the imported library, this is helpful when we want to avoid a naming conflict. Here in this file I don't see another similarly named module so I don't see why we rename Path as Pathlib, unless we do this in all files to be consistent so we don't need to think each time if we import Click or not
| """ | ||
| project_path = Path(getcwd()) / project_folder | ||
| variables_path = Path(getcwd()) / variables_file | ||
| cwd = Path(getcwd()) |
There was a problem hiding this comment.
pathlib has the cwd method (https://docs.python.org/3/library/pathlib.html#pathlib.Path.cwd) so we don't need to import os getcwd (line 3) since we have already imported Path
There was a problem hiding this comment.
I don't think we use anymore the inquirer module from pyproject.toml which would remove a few dependencies here. Perhaps it is worth looking again into the list of dependencie sin pyrpoject.toml and cleanup a few that we don't need
Hi new PR