diff --git a/CLAUDE.md b/CLAUDE.md index af707b045..cb5a9fc79 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -958,10 +958,17 @@ kbagent data-app create --project ALIAS --name NAME --slug SLUG (--git-repo URL # (1) create --use-managed-git-repo -> (2) `git-credentials-create --type http_token --permissions # readWrite` + `git push` code to the managed repo URL (from `git-repo`) -> (3) `data-app deploy`. # The platform injects the clone credentials at deploy time, so no credential wiring is needed. -# deploy pins the LATEST configVersion when a git block is present and omits it for a PURE managed -# repo (deploys from managedGitRepoId). Use `data-app runs` to debug a deploy that reverts to -# stopped (setup-phase failures produce no container logs). -kbagent data-app deploy --project NAME --app-id ID [--config-version N] [--wait] [--timeout SECONDS] [--branch ID] [--copy] [--reveal] +# deploy pins the LATEST configVersion. For a PURE managed repo (no git block yet), it first +# backfills parameters.dataApp.git from the managed repo's URL, then pins the resulting version +# (CLI-15) -- omitting configVersion alone does NOT provision the app's workspace; +# grant provisioning is gated on that block's presence, not on configVersion. The deploy result +# reports it (`git_backfilled`, `git_branch`, `warnings[]`). The branch written is `--git-branch`; +# without it, `main` for an app with no runs, and a VALIDATION_ERROR before any write for an app +# with runs (branch unknown: pass `--git-branch` with the branch the code is on). It has no effect +# on a deploy that does not backfill. Push the managed repo's code to that branch. +# Version gate lives in gotchas.md (no `(since vNEXT)` on `# ` lines). Use `data-app runs` +# to debug a deploy that reverts to stopped (setup-phase failures produce no container logs). +kbagent data-app deploy --project NAME --app-id ID [--config-version N] [--wait] [--timeout SECONDS] [--branch ID] [--git-branch BRANCH] [--copy] [--reveal] kbagent data-app start --project NAME --app-id ID [--wait] [--timeout SECONDS] kbagent data-app stop --project NAME --app-id ID [--wait] [--timeout SECONDS] kbagent data-app delete --project NAME --app-id ID [--yes] diff --git a/plugins/kbagent/agents/keboola-expert.md b/plugins/kbagent/agents/keboola-expert.md index 58984aea1..20cd9147f 100644 --- a/plugins/kbagent/agents/keboola-expert.md +++ b/plugins/kbagent/agents/keboola-expert.md @@ -389,6 +389,13 @@ its absence is NOT a promise the entry is version-independent (see §1 Rule 6). `config detail` -> `configuration.runtime` FIRST (an empty `data-app logs` grep rules nothing out). `create` defaults it ON at **0.87.0+**; <= 0.86.0 patch + redeploy. +- **Managed-repo app `running` with no `WORKSPACE_ID`**: before vNEXT (CLI-15), + `deploy` on a pure `--use-managed-git-repo` app wrote no + `parameters.dataApp.git`, and the workspace grant is gated on that block -- + `data-app detail` shows `Git: {}`. vNEXT+ backfills it on deploy (managed repo + must be pushed to the `deploy --git-branch` branch; without the flag `main` + for an app with no runs, and an error before any write for an app with runs -- + pass `--git-branch` with the branch the code is on); older: merge the block by hand, then redeploy. - **`data-app password` keeps the password out of the chat (0.96.1+)**: it never prints it without `--reveal`; the user copies it with `c` in their own terminal, or you run `--copy`. Project token only -- no Manage token. Below diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index 880753173..ba3663f7a 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -288,7 +288,7 @@ Lifecycle for `keboola.data-apps`. Combines Storage API (config body, git block, - `data-app list [--project NAME ...] [--branch ID]` -- list data apps across projects (Data Science index merged with Storage names). Since v0.43.9 filters out workspace/sandbox deployments (`componentId=keboola.sandboxes`, `type=snowflake`/`bigquery`) that the Data Science `/apps` collection also returns, so the listing matches the Apps UI. Envelope carries `component_id` per app. - `data-app detail --project NAME --app-id ID [--branch ID]` -- merged view (state, desired, url, configVersion, slug, git block with PAT redacted) - `data-app create --project ALIAS --name NAME --slug SLUG (--git-repo URL | --use-managed-git-repo) [--git-public/--no-git-public] [--git-username USER] [--git-pat-env VAR | --git-pat-file PATH | --git-pat-encrypted KBC::Project...] [--auth password|public] [--size tiny|small|medium|large] [--auto-suspend SECONDS] [--type python-js|python|streamlit|r|...] [--workspace/--no-workspace] [--branch ID] [--no-deploy] [--wait] [--timeout SECONDS] [--keep-on-failure] [--dry-run] [--copy] [--reveal]` -- POST shell + encrypt PAT + PUT Storage config (with auto-injected `parameters.id`) + PATCH deploy with the §9 trio. Cleanup-in-finally on failure unless `--keep-on-failure`. Default `--auth password` makes the platform generate a 20-char hex simpleAuth password during the deploy. With `--wait` the password is delivered like `data-app password` once the app runs (since 0.96.1); `--copy` / `--reveal` need `--wait` and a deploy (exit 2 with `--no-deploy` or `--use-managed-git-repo`). `--dry-run` accepts and refuses the same flags and adds `password_delivery` (`prompt` / `clipboard` / `stdout`) plus the warning the real run would give. **Exactly one git source required.** `--use-managed-git-repo` provisions an EMPTY Keboola-hosted repo (POST `useManagedGitRepo:true`), writes NO `parameters.dataApp.git` block, and forces `--no-deploy` (empty repo, nothing to run); mutually exclusive with `--git-repo` and all `--git-*`/PAT flags. Managed-repo deploy WORKS with no credential wiring (verified live -- tic-tac-toe deployed and serving from a Keboola-managed repo). Full flow to a RUNNING app: `git-credentials-create --type http_token --permissions readWrite` -> `git push` your code to the managed repo URL (`data-app git-repo` shows it) -> `data-app deploy`. The platform injects the clone credentials at deploy time, so nothing extra goes into the config; the created credential is only used to authenticate YOUR push. **`--workspace` (since 0.87.0) is ON by default** and writes `runtime.workspace.enabled: true` -- the single switch that makes the platform provision the app's ephemeral workspace and inject `WORKSPACE_ID` / `QUERY_SERVICE_URL` / `KBC_WORKSPACE_MANIFEST_PATH`. Every app that reads Storage needs it; before 0.87.0 kbagent never wrote it, so such an app deployed, reported `state=running`, passed its health probe and read nothing, with NO platform-side diagnostic (check `config detail` -> `configuration.runtime`; a `Missing env vars: WORKSPACE_ID` log line, if any, comes from the app's own code, so its absence rules nothing out). Pass `--no-workspace` only for an app that never touches Storage -- it omits the key entirely (no `enabled: false`), leaving the body identical to 0.86.0. The block is a sibling of `runtime.backend`. Not gated on any project feature. Retrofit an existing app with `config update --merge --set 'runtime.workspace.enabled=true'` then `data-app deploy`. -- `data-app deploy --project NAME --app-id ID [--config-version N] [--wait] [--timeout SECONDS] [--branch ID] [--copy] [--reveal]` -- the §9 redeploy contract. With `--wait` on a password app (since 0.96.1) the password is delivered like `data-app password`: the `c` prompt in a terminal (the command then ends after Enter or 120 s), `--copy`, or `--reveal`; both flags need `--wait` (exit 2 otherwise, before any API call). With no flag and no prompt (no terminal, a background job, `--json`) the password is not read and the output is as before. With a flag, `--json` adds only `ui_url`, `password_delivered_to` and (`--reveal`) `password`; a non-password app with a flag, or any failure to read the password after the deploy, is a `warnings[]` entry with exit 0. The `kbagent serve` create / deploy routes do not deliver the password. Default reads latest Storage version; `--config-version` pins an older version (rollback). Since 0.65.0: omits `configVersion` for a PURE managed repo (no git block -- deploys from `app.managedGitRepoId`, and the platform injects the clone credentials) and pins the LATEST Storage `configVersion` when a git block is present (external repos). An explicit `--config-version` always wins. +- `data-app deploy --project NAME --app-id ID [--config-version N] [--wait] [--timeout SECONDS] [--branch ID] [--git-branch BRANCH] [--copy] [--reveal]` -- the §9 redeploy contract. With `--wait` on a password app (since 0.96.1) the password is delivered like `data-app password`: the `c` prompt in a terminal (the command then ends after Enter or 120 s), `--copy`, or `--reveal`; both flags need `--wait` (exit 2 otherwise, before any API call). With no flag and no prompt (no terminal, a background job, `--json`) the password is not read and the output is as before. With a flag, `--json` adds only `ui_url`, `password_delivered_to` and (`--reveal`) `password`; a non-password app with a flag, or any failure to read the password after the deploy, is a `warnings[]` entry with exit 0. The `kbagent serve` create / deploy routes do not deliver the password. Default reads latest Storage version; `--config-version` pins an older version (rollback). Since 0.65.0 the platform injects clone credentials at deploy time, so no manual wiring is needed. Since vNEXT (CLI-15): for a PURE managed repo with no git block yet, deploy first resolves the managed repo's URL and backfills `parameters.dataApp.git`, then pins the resulting Storage `configVersion` -- omitting configVersion alone does NOT provision the app's workspace (`WORKSPACE_ID`/`KBC_WORKSPACE_MANIFEST_PATH`); grant provisioning is gated on that block's presence, not on `configVersion`. A git block already present (external repos, or a managed repo already backfilled) pins the LATEST Storage `configVersion` directly. The result then has `git_backfilled: true`, `git_branch` and a `warnings[]` entry. The branch written is `--git-branch` (since vNEXT); it is used only for this backfill and has no effect on any other deploy. Without `--git-branch`, deploy writes `main` for an app with no runs. For an app that has runs (it deployed before from the managed repo's own default branch, which no API returns), deploy fails with `VALIDATION_ERROR` before any write and asks for `--git-branch` with the branch the code is on. Push the managed repo's code to that branch. An explicit `--config-version` always wins and skips the backfill. - `data-app start --project NAME --app-id ID [--wait] [--timeout SECONDS]` -- wake an auto-suspended app at the currently-pinned version. Distinct from deploy: does NOT bump configVersion. - `data-app stop --project NAME --app-id ID [--wait] [--timeout SECONDS]` -- stop a running app (URL and Storage config preserved). - `data-app delete --project NAME --app-id ID [--yes]` -- destructive, cascades to Storage config; URL retired permanently. diff --git a/plugins/kbagent/skills/kbagent/references/data-app-workflow.md b/plugins/kbagent/skills/kbagent/references/data-app-workflow.md index 5c7f95378..55b8cfe4f 100644 --- a/plugins/kbagent/skills/kbagent/references/data-app-workflow.md +++ b/plugins/kbagent/skills/kbagent/references/data-app-workflow.md @@ -338,7 +338,9 @@ kbagent --json data-app git-repo --project prod --app-id 12345678 # -> https_u git push "https://@/keboola/app-12345678.git" HEAD:main # 4. Deploy. The platform injects the clone credentials at deploy time, so no -# credential is wired into the config. +# credential is wired into the config. Since vNEXT (CLI-15), this step also +# backfills parameters.dataApp.git from the managed repo's URL before +# pinning a version -- required for the app to get a workspace at all. kbagent data-app deploy --project prod --app-id 12345678 --wait ``` @@ -353,6 +355,25 @@ contract), so `data-app deploy` on a pure managed repo deploys straight from `app.managedGitRepoId`. If a deploy ever reverts to stopped, diagnose it with `data-app runs` (`failure_reason` + `startup_logs`). +**Workspace access is a separate concern from cloning (since vNEXT; CLI-15).** +Cloning needs no credential wiring, as above -- but an app that also reads +Storage (`runtime.workspace.enabled: true`, the `--workspace` default) needs a +workspace grant, and that grant is gated on `parameters.dataApp.git` being +present in Storage config, independent of `managedGitRepoId`. Step 4 above now +backfills that block automatically before the first deploy, so this flow needs +no extra manual step. **Push to the backfilled branch:** the block sets +`branch` from `data-app deploy --git-branch` (since vNEXT), because the +managed repo's `git-repo` lookup returns no branch to read. Without the flag, +deploy writes `main` for an app with no runs. An app that has runs deployed +before from the repo's own default branch, so deploy fails with +`VALIDATION_ERROR` before any write; pass `--git-branch` with the branch the +code is on. The deploy result reports the branch it wrote (`git_branch`). `--git-branch` has +no effect on a deploy that does not backfill. Code pushed only to another +branch is never deployed. On an older kbagent version, the tell for the +unpatched gap is `data-app detail` showing `Git: {}` (empty) on a `running` +app with no `WORKSPACE_ID` -- see the gotchas reference for the manual +workaround. + ### Manage app-runtime secrets ```bash diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index c9149ba70..5aeb5401d 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -4144,7 +4144,12 @@ Other behaviors of this family: `git-credentials` list never returns it. `--type ssh_key` requires a `--public-key` / `--public-key-file` and returns no secret. -## `data-app` managed-repo deploy: omit configVersion (the platform injects clone creds) (since v0.65.0; guidance corrected v0.65.1 -- no credential wiring needed) +## `data-app` managed-repo deploy: the platform injects clone creds; deploy backfills the git block (since v0.65.0; guidance corrected v0.65.1 -- no credential wiring needed) + +*(since vNEXT)* `data-app deploy` no longer omits configVersion for a pure +managed repo (CLI-15). It backfills `parameters.dataApp.git` and pins the +resulting version. Read the bullets below with that in mind; the last bullet +has the details. `--use-managed-git-repo` provisions an **empty** Keboola-hosted git repo (POST `useManagedGitRepo:true`) linked to the app via `app.managedGitRepoId`. It @@ -4172,14 +4177,15 @@ The end-to-end flow that is **verified to deploy and serve** (tic-tac-toe on The actual bug fixed in 0.65.0 was **kbagent always-pinning `configVersion`**: -- **configVersion at deploy depends on the source location.** `data-app deploy` - **omits** configVersion for a *pure* managed repo (no git block), which deploys - from `managedGitRepoId`; it pins the **latest** Storage configVersion only when - a git block is present (external repos). Pinning a managed app's no-git-block - Storage config made the runtime demand `dataApp.git.repository` and the deploy - reverted to `stopped` -- that was the misdiagnosed "could not read Username" - symptom, NOT a missing credential. Fixed in 0.65.0 by omitting configVersion for - pure managed repos. There is **no** cross-stack "platform doesn't inject +- **configVersion at deploy depends on the source location.** In 0.65.0 through + the release before vNEXT, `data-app deploy` **omitted** configVersion for a + *pure* managed repo (no git block) and pinned the **latest** Storage + configVersion only when a git block was present (external repos). Pinning a + managed app's no-git-block Storage config made the runtime demand + `dataApp.git.repository` and the deploy reverted to `stopped` -- that was the + misdiagnosed "could not read Username" symptom, NOT a missing credential. + Omitting configVersion fixed that crash. Since vNEXT, deploy backfills the git + block instead (last bullet). There is **no** cross-stack "platform doesn't inject credentials" gap (issue #454 closed as not-a-bug). - **Diagnose a stopped-reverting deploy with `data-app runs`**, not `data-app logs`. `runs` returns each attempt's `failure_reason` + `startup_logs` -- @@ -4189,6 +4195,28 @@ The actual bug fixed in 0.65.0 was **kbagent always-pinning `configVersion`**: command on the misdiagnosis that a credential had to be wired into the config. It was removed in 0.65.1 -- it is unnecessary because no platform-injection gap exists. +- **Correction (since vNEXT, CLI-15): omitting configVersion alone does NOT + provision a workspace.** The 0.65.0 fix above solves the "could not read Username" crash, + but a *pure* managed repo (no git block yet) still deployed with no + `WORKSPACE_ID`/`KBC_WORKSPACE_MANIFEST_PATH`, even with + `runtime.workspace.enabled: true` and `state: running` -- the same + no-platform-diagnostic failure mode as the `--workspace` gotcha above. Root + cause: workspace grant provisioning is gated on `parameters.dataApp.git` being + present in Storage config, independent of `managedGitRepoId`/ + `hasManagedGitRepo`/`configVersion`. Fixed since vNEXT: `data-app deploy` now + resolves the managed repo's URL and backfills that git block (repository URL, + branch from `--git-branch`) *before* pinning a version, for any pure managed + repo -- no manual `config update --merge` step needed. No API returns the + managed repo's default branch. So without `--git-branch`, deploy writes + `main` only for an app with no runs (`data-app runs` empty). An app with + runs deployed before from the repo's own default branch, which may not be + `main`: deploy fails with `VALIDATION_ERROR` before any write and asks for + `--git-branch` with the branch the code is on. + `--git-branch` has no effect on a deploy that does not backfill. The deploy + result reports it with `git_backfilled: true`, `git_branch` and a `warnings[]` + entry. Push your code to that branch of the managed repo. `data-app detail`'s `Git: {}` (empty) on a `running` app is the tell + for this on an older kbagent version; the workaround there is to merge the + block manually, then redeploy. ## Core platform gotchas (version-independent) diff --git a/scripts/file_size_baseline.json b/scripts/file_size_baseline.json index 74d7f7390..4bd0e1620 100644 --- a/scripts/file_size_baseline.json +++ b/scripts/file_size_baseline.json @@ -4,7 +4,7 @@ "commands/config.py": 2007, "commands/lineage.py": 1271, "commands/storage.py": 2221, - "services/data_app_service.py": 1633, + "services/data_app_service.py": 1600, "services/storage_service.py": 1684, "services/sync_service.py": 1655 } diff --git a/src/keboola_agent_cli/commands/context.py b/src/keboola_agent_cli/commands/context.py index 17fbca623..70f65db4e 100644 --- a/src/keboola_agent_cli/commands/context.py +++ b/src/keboola_agent_cli/commands/context.py @@ -1440,12 +1440,21 @@ Cleanup-in-finally if PUT or initial deploy fails (orphan shell deleted by default; --keep-on-failure preserves it for forensics). --use-managed-git-repo (0.65.0+) provisions an EMPTY Keboola-hosted repo - instead of cloning an external one; writes no git block, forces --no-deploy, - mutually exclusive with --git-repo and all --git-*/PAT flags. Managed deploy - works via: create --use-managed-git-repo -> git-credentials-create - --type http_token --permissions readWrite + push code to the managed repo - URL -> deploy. The platform injects the clone credentials at deploy time, - so no credential wiring is needed. + instead of cloning an external one; create writes no git block, forces + --no-deploy, mutually exclusive with --git-repo and all --git-*/PAT flags. + Managed deploy works via: create --use-managed-git-repo -> + git-credentials-create --type http_token --permissions readWrite + push + code to the `main` branch of the managed repo URL -> deploy. The platform + injects the clone credentials at deploy time, so no credential wiring is + needed. Deploy writes the git block itself (vNEXT+, CLI-15): for a managed + app with no parameters.dataApp.git it backfills repository URL + branch + into the Storage config, then pins the new version. The branch is + deploy --git-branch; without it, `main` for an app with no runs, and a + VALIDATION_ERROR before any write for an app that has runs (its branch + is unknown -- pass --git-branch with the branch the code is on). The + platform provisions the workspace only when that block exists. The result + has git_backfilled: true, git_branch and a warnings[] entry; code on any + other branch is not deployed. --workspace (0.87.0+, DEFAULT ON) writes runtime.workspace.enabled=true -- the single switch that makes the platform provision the ephemeral workspace and inject WORKSPACE_ID / QUERY_SERVICE_URL / KBC_WORKSPACE_MANIFEST_PATH. @@ -1461,7 +1470,8 @@ then redeploy (deploy pins the LATEST version, so the change takes effect). kbagent data-app deploy --project NAME --app-id ID [--config-version N] - [--wait] [--timeout SECONDS] [--branch ID] [--copy] [--reveal] + [--wait] [--timeout SECONDS] [--branch ID] [--git-branch BRANCH] + [--copy] [--reveal] With --wait on a password app (0.96.1+): the password is delivered like `data-app password` (the c prompt in a terminal, which then waits for Enter or 120 s; --copy / --reveal need --wait). Without a flag and @@ -1470,7 +1480,9 @@ (+ password with --reveal); a failed password read or a non-password app with --copy is a `warnings[]` entry, exit 0. The §9 redeploy contract. Default reads the latest Storage config version - and pins to it; --config-version pins an older version (rollback). + and pins to it; --config-version pins an older version (rollback) and + skips the git-block backfill. --git-branch only sets the branch of + that backfill; it has no effect otherwise. Always sends {{desiredState=running, configVersion, restartIfRunning=true}} together -- HTTP 422 otherwise. diff --git a/src/keboola_agent_cli/commands/data_app.py b/src/keboola_agent_cli/commands/data_app.py index a07a23015..4f3652b46 100644 --- a/src/keboola_agent_cli/commands/data_app.py +++ b/src/keboola_agent_cli/commands/data_app.py @@ -565,6 +565,15 @@ def data_app_deploy( "--branch", help="Storage branch for reading the latest version (defaults to production).", ), + git_branch: str | None = typer.Option( + None, + "--git-branch", + help=( + "Git branch to clone. Used only when deploy writes the git block of a " + "pure managed repo; otherwise it has no effect. Without it, deploy writes " + "main for a never-deployed app and stops with an error for a deployed one." + ), + ), copy: CopyOption = False, reveal: RevealOption = False, ) -> None: @@ -584,7 +593,7 @@ def data_app_deploy( app_id=app_id, wait=wait, timeout=timeout, - extra={"config_version": config_version, "branch_id": branch}, + extra={"config_version": config_version, "branch_id": branch, "git_branch": git_branch}, delivery=flags, ) diff --git a/src/keboola_agent_cli/constants.py b/src/keboola_agent_cli/constants.py index 99b8bfc04..2d6148cff 100644 --- a/src/keboola_agent_cli/constants.py +++ b/src/keboola_agent_cli/constants.py @@ -664,6 +664,7 @@ def _resolve_app_name() -> str: # --- Job Run --- DEFAULT_JOB_RUN_TIMEOUT: float = 300.0 # 5 min default for --wait polling +DEFAULT_GIT_BRANCH = "main" # data-app deploy backfill branch for a never-deployed managed app # --- Job Terminate --- # States where POST /jobs/{id}/kill returns HTTP 200; any other state yields 400. diff --git a/src/keboola_agent_cli/data_science_client.py b/src/keboola_agent_cli/data_science_client.py index f214226da..650d37f83 100644 --- a/src/keboola_agent_cli/data_science_client.py +++ b/src/keboola_agent_cli/data_science_client.py @@ -212,13 +212,10 @@ def patch_app( + ``restart_if_running=True`` together when bumping the deployed config version; sending ``config_version`` alone yields HTTP 422. - EXCEPTION (python-js managed repos): a *pure* managed-repo app - (``useManagedGitRepo``, no ``parameters.dataApp.git`` block) deploys its - source from ``app.managedGitRepoId``, not from a Storage configVersion, - so the caller passes ``config_version=None`` and this PATCH omits the - field entirely -- payload ``{desiredState, restartIfRunning}``. This - matches keboola-mcp-server / Kai behavior; see - ``DataAppService.deploy_data_app`` for the source-location branch. + ``config_version=None`` omits the field -- payload + ``{desiredState, restartIfRunning}``. ``DataAppService.deploy_data_app`` + no longer takes this path for a pure managed repo: it backfills + ``parameters.dataApp.git`` and pins the resulting version (CLI-15). """ payload: dict[str, Any] = {} if desired_state is not None: diff --git a/src/keboola_agent_cli/server/routers/data_apps.py b/src/keboola_agent_cli/server/routers/data_apps.py index cf0a10b5a..9cb3f7ba9 100644 --- a/src/keboola_agent_cli/server/routers/data_apps.py +++ b/src/keboola_agent_cli/server/routers/data_apps.py @@ -152,6 +152,7 @@ def deploy( wait: bool = False, timeout_seconds: float = 600.0, branch_id: int | None = None, + git_branch: str | None = None, registry: ServiceRegistry = Depends(get_registry), ) -> dict[str, Any]: """Deploy the configured version of a data app. Mirrors `kbagent data-app deploy`.""" @@ -162,6 +163,7 @@ def deploy( wait=wait, timeout_seconds=timeout_seconds, branch_id=branch_id, + git_branch=git_branch, ) diff --git a/src/keboola_agent_cli/services/_data_app_bodies.py b/src/keboola_agent_cli/services/_data_app_bodies.py index aeb30b53b..ff7dc0edf 100644 --- a/src/keboola_agent_cli/services/_data_app_bodies.py +++ b/src/keboola_agent_cli/services/_data_app_bodies.py @@ -1,17 +1,31 @@ -"""Pure body-builders and redaction helpers for :mod:`data_app_service`. +"""Body-builders, redaction helpers, and git/version resolution for +:mod:`data_app_service`. Extracted from ``data_app_service.py`` (which is over its file-size budget) -so the service module holds orchestration only. Everything here is a pure -function of its arguments: no HTTP, no config store, no I/O. The service -re-exports these names, so existing ``from ...data_app_service import _x`` -call sites keep working. +so the service module holds orchestration only. Most functions here are pure +functions of their arguments -- no HTTP, no config store, no I/O. The +exceptions are ``backfill_managed_git`` and ``resolve_effective_version``, +which call the pre-constructed client objects they are passed (no new I/O +wiring of their own); they live here anyway because they are the direct +continuation of ``deploy_data_app``'s git-block/version-resolution logic, not +a separate concern. The service re-exports these names, so existing +``from ...data_app_service import _x`` call sites keep working. """ from __future__ import annotations import json +import logging +from dataclasses import dataclass from typing import Any +from ..constants import DEFAULT_GIT_BRANCH +from ..errors import ErrorCode, KeboolaApiError + +DATA_APP_COMPONENT_ID = "keboola.data-apps" + +logger = logging.getLogger(__name__) + # Encrypted-secret prefixes produced by the Encryption API for PROJECT-scoped # ciphertext -- one variant per cloud, and exactly these three exist: # ``KBC::ProjectSecure::`` (AWS KMS), ``KBC::ProjectSecureGKMS::`` (Google KMS) @@ -197,6 +211,257 @@ def _build_runtime_block(*, size: str, workspace: bool) -> dict[str, Any]: return runtime +def _has_control_chars(value: str, *, allow_whitespace: bool = False) -> bool: + """Return True if ``value`` contains any ASCII control byte (0x00-0x1f / 0x7f). + + Set ``allow_whitespace=True`` to permit ``\\t \\n \\r`` (description markdown); + everything else under 0x20 + 0x7f is still rejected. + """ + allowed = {0x09, 0x0A, 0x0D} if allow_whitespace else set() + for ch in value: + code = ord(ch) + if code in allowed: + continue + if code < 0x20 or code == 0x7F: + return True + return False + + +def _check_text_field( + field_name: str, value: Any, max_len: int, allow_ws: bool, *, required: bool = False +) -> None: + """Reject a too-long or control-char-bearing string; non-strings are skipped. + + ``required=True`` also rejects an empty or whitespace-only string. + """ + if not isinstance(value, str): + return + if required and not value.strip(): + raise KeboolaApiError( + message=f"{field_name} must not be empty.", + status_code=0, + error_code=ErrorCode.VALIDATION_ERROR, + retryable=False, + ) + if len(value) > max_len: + raise KeboolaApiError( + message=( + f"{field_name} exceeds the {max_len}-character limit " + "enforced at the service boundary." + ), + status_code=0, + error_code=ErrorCode.VALIDATION_ERROR, + retryable=False, + ) + # Description allows tab/LF/CR (markdown); other fields reject any + # control char including CR/LF (would break URL host derivation, + # JSON serialization, or audit-log change descriptions). + if _has_control_chars(value, allow_whitespace=allow_ws): + raise KeboolaApiError( + message=(f"{field_name} contains disallowed control characters."), + status_code=0, + error_code=ErrorCode.VALIDATION_ERROR, + retryable=False, + ) + + +def _build_managed_git_block(repo: dict[str, Any], app_id: str, git_branch: str) -> dict[str, Any]: + """Build ``parameters.dataApp.git`` for a managed-repo app from a + ``get_git_repo`` response. + + See CLI-15: a managed-repo app's first deploy does not get a workspace + just from omitting ``configVersion`` -- provisioning is gated on this + block being present in Storage config, independent of + ``managedGitRepoId``/``hasManagedGitRepo``. ``deploy_data_app`` calls this + to backfill it before deploying, same as an external-git app. + + Raises if the lookup returned neither URL -- fail loudly rather than + deploy with no source pointer. + """ + git_url = repo.get("httpsUrl") or repo.get("sshUrl") + if not git_url: + raise KeboolaApiError( + error_code=ErrorCode.API_ERROR, + message=f"App {app_id} is managed but git-repo lookup returned no URL", + status_code=500, + retryable=False, + ) + # get_git_repo carries no branch field, so the caller (``deploy --git-branch``) + # decides which branch the platform clones. + return {"repository": git_url, "branch": git_branch, "private": True} + + +def git_backfill_warning(git_branch: str) -> str: + return ( + f"Deploy wrote parameters.dataApp.git (managed repository URL, branch {git_branch}) " + f"into the configuration. Push your code to {git_branch} of the managed repository." + ) + + +@dataclass(frozen=True) +class EffectiveVersion: + """The Storage ``configVersion`` deploy pins, and whether deploy backfilled the git block.""" + + version: str + git_backfilled: bool + git_branch: str | None = None + + +@dataclass(frozen=True) +class ManagedGitBackfillTarget: + """The identifying context :func:`backfill_managed_git` needs, + grouped so the call site at ``deploy_data_app`` stays one line.""" + + app_id: str + config_id: str + branch_id: int | None + latest_version: str + git_branch: str + + +def backfill_managed_git( + ds_client: Any, + storage_client: Any, + configuration: dict[str, Any], + ctx: ManagedGitBackfillTarget, +) -> str: + """Resolve + persist ``parameters.dataApp.git`` for a managed-repo app with + no git block yet; returns the resulting configVersion to pin. + + See CLI-15: ``deploy_data_app``'s configVersion-omission branch alone does + not get the app a workspace -- provisioning is gated on this block being + present. Takes pre-constructed clients (not a service instance) so + ``deploy_data_app`` can call it inline with no new constructor wiring. + """ + git_block = _build_managed_git_block( + ds_client.get_git_repo(ctx.app_id), ctx.app_id, ctx.git_branch + ) + # Match the read in resolve_effective_version: a non-dict value counts as empty. + parameters = configuration.get("parameters") + if not isinstance(parameters, dict): + parameters = configuration["parameters"] = {} + data_app = parameters.get("dataApp") + if not isinstance(data_app, dict): + data_app = parameters["dataApp"] = {} + data_app["git"] = git_block + updated = storage_client.update_config( + component_id=DATA_APP_COMPONENT_ID, + config_id=ctx.config_id, + configuration=configuration, + change_description="Auto-backfill managed-repo git block (workspace provisioning fix)", + branch_id=ctx.branch_id, + ) + new_version = str(updated.get("version", "") or "") + if not new_version: + # Falling back to ctx.latest_version would pin the pre-backfill config -- + # the exact no-workspace state this backfill exists to fix. Fail loudly, + # mirroring create_data_app's handling of the same missing-version case. + raise KeboolaApiError( + message=( + "Storage API did not return a version after backfilling the " + "managed-repo git block; cannot pin configVersion for deploy." + ), + status_code=500, + error_code=ErrorCode.API_ERROR, + retryable=False, + ) + logger.info( + "Backfilled parameters.dataApp.git for managed-repo app %s (config %s); " + "Storage version %s -> %s", + ctx.app_id, + ctx.config_id, + ctx.latest_version, + new_version, + ) + return new_version + + +def _backfill_branch(ds_client: Any, app_id: str, git_branch: str | None) -> str: + """Choose the branch to write into a backfilled managed-repo git block. + + An explicit ``--git-branch`` always wins. No API returns the managed + repo's default branch (``get_git_repo`` has no branch field). Before + vNEXT a pure managed repo deployed from ``managedGitRepoId`` on the + repo's own default branch, so an app with any run history may serve code + from a branch other than ``main``. Writing ``main`` for it would repoint + it for good. So ``main`` is used only for an app with no runs; otherwise + deploy stops before any write and asks for ``--git-branch``. + """ + if git_branch is not None: + return git_branch + if ds_client.list_app_runs(app_id, limit=1): + raise KeboolaApiError( + message=( + f"Data app {app_id} uses a managed git repository and was deployed " + "before, but its configuration has no git block. Deploy must write " + "the branch into the configuration, and the branch of the managed " + "repository is unknown. Pass --git-branch BRANCH (the branch your " + "code is on). Nothing was changed." + ), + status_code=0, + error_code=ErrorCode.VALIDATION_ERROR, + retryable=False, + ) + return DEFAULT_GIT_BRANCH + + +def resolve_effective_version( + ds_client: Any, + storage_client: Any, + app: dict[str, Any], + app_id: str, + config_id: str, + branch_id: int | None, + git_branch: str | None = None, +) -> EffectiveVersion: + """Resolve the Storage ``configVersion`` for ``deploy_data_app`` to pin. + + ``git_backfilled`` in the result is True when the git block was written + into the Storage config; ``git_branch`` then names the branch it wrote. + ``git_branch`` is used only for that backfill and ignored otherwise. + ``None`` means the caller did not pass ``--git-branch``: see + :func:`_backfill_branch` for how the branch is then chosen. + + configVersion resolution depends on where the app's *source* lives: + + * Streamlit / external-git (``parameters.dataApp.git`` present) -- pin the + latest Storage version so the operator reads the current git block. + * A pure managed repo (``hasManagedGitRepo``, no git block yet) -- backfill + the git block first (CLI-15: omitting configVersion alone does not get + the app a workspace; see :func:`backfill_managed_git`), then pin the + version that backfill wrote. + + Raises if neither a git block can be backfilled nor a Storage version is + resolvable. + """ + storage_config = storage_client.get_config_detail( + DATA_APP_COMPONENT_ID, + config_id, + branch_id=branch_id, + ) + latest_version = str(storage_config.get("version", "") or "") + configuration = _coerce_config_dict(storage_config.get("configuration")) + data_app_cfg = (configuration.get("parameters") or {}).get("dataApp") or {} + is_managed = bool(app.get("hasManagedGitRepo")) + has_git_block = bool(data_app_cfg.get("git")) + if is_managed and not has_git_block: + branch = _backfill_branch(ds_client, app_id, git_branch) + ctx = ManagedGitBackfillTarget(app_id, config_id, branch_id, latest_version, branch) + version = backfill_managed_git(ds_client, storage_client, configuration, ctx) + return EffectiveVersion(version=version, git_backfilled=True, git_branch=branch) + if not latest_version: + raise KeboolaApiError( + message=( + f"Cannot resolve a Storage configVersion for app {app_id}; " + "Storage config returned no version." + ), + status_code=500, + error_code=ErrorCode.API_ERROR, + retryable=False, + ) + return EffectiveVersion(version=latest_version, git_backfilled=False) + + def _redact_secret(value: Any) -> Any: """Replace encrypted ``#`` values with a placeholder for human output.""" if isinstance(value, str) and value.startswith("KBC::"): diff --git a/src/keboola_agent_cli/services/data_app_service.py b/src/keboola_agent_cli/services/data_app_service.py index ea27d99df..021deae0f 100644 --- a/src/keboola_agent_cli/services/data_app_service.py +++ b/src/keboola_agent_cli/services/data_app_service.py @@ -35,15 +35,20 @@ # Re-exported so existing `from ...data_app_service import _x` call sites # (tests included) keep resolving after the split into _data_app_bodies.py. from ._data_app_bodies import ( + DATA_APP_COMPONENT_ID, ENCRYPTED_PASSWORD_PREFIXES, RESERVED_RUNTIME_ENV_VARS, _auth_block_for, _build_runtime_block, + _check_text_field, _coerce_config_dict, _derive_runtime_env_var_name, + _has_control_chars, _redact_git_block, _redact_storage_config, _secret_fingerprint, + git_backfill_warning, + resolve_effective_version, ) from ._data_app_password import ( DataAppPassword, @@ -71,8 +76,6 @@ def make_default_ds_client_factory(config_store: Any) -> DataScienceClientFactor # Constants encoded from the writeup # --------------------------------------------------------------------------- -DATA_APP_COMPONENT_ID = "keboola.data-apps" - VALID_TYPES: tuple[str, ...] = ( "python-js", "python", @@ -108,22 +111,6 @@ def make_default_ds_client_factory(config_store: Any) -> DataScienceClientFactor STOPPED_STATE = "stopped" -def _has_control_chars(value: str, *, allow_whitespace: bool = False) -> bool: - """Return True if ``value`` contains any ASCII control byte (0x00-0x1f / 0x7f). - - Set ``allow_whitespace=True`` to permit ``\\t \\n \\r`` (description markdown); - everything else under 0x20 + 0x7f is still rejected. - """ - allowed = {0x09, 0x0A, 0x0D} if allow_whitespace else set() - for ch in value: - code = ord(ch) - if code in allowed: - continue - if code < 0x20 or code == 0x7F: - return True - return False - - # URL schemes accepted for ``--git-repo``. Anything else (file://, gopher://, # bare ssh syntax like ``git@host:path``) is rejected at the service # boundary -- the data-app runner only ever talks https / http / git / ssh. @@ -584,6 +571,7 @@ def deploy_data_app( wait: bool = False, timeout_seconds: float = DEFAULT_JOB_RUN_TIMEOUT, branch_id: int | None = None, + git_branch: str | None = None, ) -> dict[str, Any]: """Encapsulate the §9 redeploy contract. @@ -591,7 +579,14 @@ def deploy_data_app( ``config_version`` to deploy an older version. ALWAYS sends ``restartIfRunning=true`` together with ``configVersion`` -- the server returns HTTP 422 for any other shape. + + ``git_branch`` is the branch written into ``parameters.dataApp.git`` + when deploy backfills a pure managed repo; it has no effect otherwise. + ``None`` (not given) means ``main`` for a never-deployed app and a + VALIDATION_ERROR before any write for an app with run history. """ + # None (not given) is skipped: _check_text_field ignores non-strings. + _check_text_field("--git-branch", git_branch, MAX_GIT_BRANCH_LENGTH, False, required=True) projects = self.resolve_projects([alias]) project = projects[alias] @@ -608,52 +603,22 @@ def deploy_data_app( retryable=False, ) - # configVersion resolution depends on where the app's *source* - # lives, which we infer from the latest Storage config: - # - # * Streamlit / external-git (parameters.dataApp.git present) -> the - # source pointer lives IN the Storage config, so we PIN the latest - # version so the operator reads the current git block. - # * A managed repo (useManagedGitRepo, NO git block) -> the source - # resolves via app.managedGitRepoId and the platform injects the - # clone credentials at deploy time, so we OMIT configVersion - # (matches keboola-mcp-server / Kai and the sandboxes-service - # `testManagedGitRepo.sh` contract). Pinning a managed app's - # no-git-block config instead makes the runtime demand - # `dataApp.git.repository` and the deploy fails -- that was the - # pre-0.65.0 bug this branch fixes. - # - # An explicit --config-version always wins as an escape hatch. + # An explicit --config-version always wins as an escape hatch; see + # resolve_effective_version's docstring for the resolution rules. effective_version: str | None = config_version + backfilled_branch: str | None = None if config_version is None: storage_client = self._client_factory(project.stack_url, project.token) - storage_config = storage_client.get_config_detail( - DATA_APP_COMPONENT_ID, config_id, branch_id=branch_id + resolved = resolve_effective_version( + ds_client, storage_client, app, app_id, config_id, branch_id, git_branch ) - latest_version = str(storage_config.get("version", "") or "") - configuration = _coerce_config_dict(storage_config.get("configuration")) - data_app_cfg = (configuration.get("parameters") or {}).get("dataApp") or {} - is_managed = bool(app.get("hasManagedGitRepo")) - has_git_block = bool(data_app_cfg.get("git")) - if is_managed and not has_git_block: - effective_version = None # deploy from managedGitRepoId, no pin - else: - if not latest_version: - raise KeboolaApiError( - message=( - f"Cannot resolve a Storage configVersion for app {app_id}; " - "Storage config returned no version." - ), - status_code=500, - error_code=ErrorCode.API_ERROR, - retryable=False, - ) - effective_version = latest_version + # git_branch is None unless the backfill wrote the git block. + effective_version, backfilled_branch = resolved.version, resolved.git_branch deployed = ds_client.patch_app( app_id, desired_state=RUNNING_STATE, - config_version=effective_version, # None only for pure managed repos + config_version=effective_version, restart_if_running=True, ) poll_result: dict[str, Any] | None = None @@ -664,7 +629,7 @@ def deploy_data_app( target_desired_state=RUNNING_STATE, timeout_seconds=timeout_seconds, ) - return self._format_lifecycle_result( + result = self._format_lifecycle_result( alias=alias, app_id=app_id, action="deploy", @@ -672,6 +637,11 @@ def deploy_data_app( poll_result=poll_result, config_version=effective_version or "", ) + if backfilled_branch is not None: + result["git_backfilled"] = True + result["git_branch"] = backfilled_branch + result["warnings"] = [git_backfill_warning(backfilled_branch)] + return result finally: ds_client.close() if storage_client is not None: @@ -1596,28 +1566,7 @@ def _validate_create_inputs( ("--git-branch", git_branch, MAX_GIT_BRANCH_LENGTH, False), ("--git-username", git_username or "", MAX_GIT_USERNAME_LENGTH, False), ): - if not isinstance(field_value, str): - continue - if len(field_value) > max_len: - raise KeboolaApiError( - message=( - f"{field_name} exceeds the {max_len}-character limit " - "enforced at the service boundary." - ), - status_code=0, - error_code=ErrorCode.VALIDATION_ERROR, - retryable=False, - ) - # Description allows tab/LF/CR (markdown); other fields reject any - # control char including CR/LF (would break URL host derivation, - # JSON serialization, or audit-log change descriptions). - if _has_control_chars(field_value, allow_whitespace=allow_ws): - raise KeboolaApiError( - message=(f"{field_name} contains disallowed control characters."), - status_code=0, - error_code=ErrorCode.VALIDATION_ERROR, - retryable=False, - ) + _check_text_field(field_name, field_value, max_len, allow_ws) # Reject git_repo URLs that don't use a known clone scheme. The # data-app runner only handles https / http / git / ssh; anything diff --git a/tests/test_data_app_cli.py b/tests/test_data_app_cli.py index 825c2307d..cdf03f1df 100644 --- a/tests/test_data_app_cli.py +++ b/tests/test_data_app_cli.py @@ -538,6 +538,51 @@ def test_deploy_success(self, tmp_path: Path) -> None: assert body["data"]["config_version"] == "5" mock.deploy_data_app.assert_called_once() + @pytest.mark.parametrize(("extra", "branch"), [(["--git-branch", "dev"], "dev"), ([], None)]) + def test_deploy_passes_git_branch_to_service( + self, tmp_path: Path, extra: list[str], branch: str | None + ) -> None: + config_dir = tmp_path / "config" + config_dir.mkdir() + store = _setup_config(config_dir, {"prod": {"token": TEST_TOKEN}}) + mock = MagicMock() + mock.deploy_data_app.return_value = {"app_id": "42", "action": "deploy"} + + result = _invoke( + ["data-app", "deploy", "--project", "prod", "--app-id", "42", *extra], + store=store, + data_app_mock=mock, + ) + + assert result.exit_code == 0 + assert mock.deploy_data_app.call_args.kwargs["git_branch"] == branch + + def test_deploy_reports_git_backfill(self, tmp_path: Path) -> None: + config_dir = tmp_path / "config" + config_dir.mkdir() + store = _setup_config(config_dir, {"prod": {"token": TEST_TOKEN}}) + mock = MagicMock() + mock.deploy_data_app.return_value = { + "project_alias": "prod", + "app_id": "42", + "action": "deploy", + "state": "starting", + "desired_state": "running", + "config_version": "5", + "url": "", + "message": "Data app 42 deploy requested.", + "git_backfilled": True, + "warnings": ["Deploy wrote parameters.dataApp.git into the configuration."], + } + args = ["data-app", "deploy", "--project", "prod", "--app-id", "42"] + + as_json = _invoke(["--json", *args], store=store, data_app_mock=mock) + assert json.loads(as_json.output)["data"]["git_backfilled"] is True + + human = _invoke(args, store=store, data_app_mock=mock) + assert human.exit_code == 0 + assert "Deploy wrote parameters.dataApp.git into the configuration." in human.output + def test_api_error_exit_code(self, tmp_path: Path) -> None: config_dir = tmp_path / "config" config_dir.mkdir() diff --git a/tests/test_data_app_service.py b/tests/test_data_app_service.py index 79909a4d8..9c8c028b1 100644 --- a/tests/test_data_app_service.py +++ b/tests/test_data_app_service.py @@ -11,6 +11,7 @@ from __future__ import annotations +import json import logging from pathlib import Path from typing import Any, ClassVar @@ -63,7 +64,10 @@ def _make_service( storage_mock: MagicMock | None = None, encrypt_mock: MagicMock | None = None, ) -> tuple[DataAppService, MagicMock, MagicMock, MagicMock]: - ds_mock = ds_mock or MagicMock() + if ds_mock is None: + ds_mock = MagicMock() + # No run history: a never-deployed app (a bare MagicMock would be truthy). + ds_mock.list_app_runs.return_value = [] storage_mock = storage_mock or MagicMock() if encrypt_mock is None: encrypt_mock = MagicMock() @@ -1100,9 +1104,13 @@ def test_deploy_never_sends_config_block(self, tmp_path: Path) -> None: kwargs = ds_mock.patch_app.call_args.kwargs assert "config" not in kwargs - def test_deploy_pure_managed_omits_config_version(self, tmp_path: Path) -> None: - """A managed-repo app with NO git block deploys from managedGitRepoId, - so configVersion is omitted (matches keboola-mcp-server).""" + def test_deploy_pure_managed_backfills_git_block_and_pins_new_version( + self, tmp_path: Path + ) -> None: + """A managed-repo app with NO git block does not get a workspace just from + omitting configVersion (CLI-15) -- provisioning is gated on + parameters.dataApp.git being present in Storage config. Backfill it from + the managed repo's URL and pin the resulting version.""" store = _make_store(tmp_path) service, ds_mock, storage_mock, _enc = _make_service(store) ds_mock.get_app.return_value = {"configId": "ulid", "hasManagedGitRepo": True} @@ -1110,12 +1118,306 @@ def test_deploy_pure_managed_omits_config_version(self, tmp_path: Path) -> None: "version": 4, "configuration": {"parameters": {"dataApp": {"slug": "x"}}}, # no git block } + ds_mock.get_git_repo.return_value = { + "sshUrl": None, + "httpsUrl": "https://git.example.com/keboola/app-42.git", + "isManagedGitRepo": True, + } + storage_mock.update_config.return_value = {"version": "5"} ds_mock.patch_app.return_value = {"state": "starting"} service.deploy_data_app(alias="prod", app_id="42") - kwargs = ds_mock.patch_app.call_args.kwargs - assert kwargs["config_version"] is None # omitted + ds_mock.get_git_repo.assert_called_once_with("42") + update_kwargs = storage_mock.update_config.call_args.kwargs + git_block = update_kwargs["configuration"]["parameters"]["dataApp"]["git"] + assert git_block == { + "repository": "https://git.example.com/keboola/app-42.git", + "branch": "main", + "private": True, + } + # slug survives the merge -- we mutate the fetched config, not replace it + assert update_kwargs["configuration"]["parameters"]["dataApp"]["slug"] == "x" + + patch_kwargs = ds_mock.patch_app.call_args.kwargs + assert patch_kwargs["config_version"] == "5" # the newly-written version, pinned + + def test_deploy_pure_managed_falls_back_to_ssh_url(self, tmp_path: Path) -> None: + """httpsUrl is preferred but sshUrl works when it's the only one set.""" + store = _make_store(tmp_path) + service, ds_mock, storage_mock, _enc = _make_service(store) + ds_mock.get_app.return_value = {"configId": "ulid", "hasManagedGitRepo": True} + storage_mock.get_config_detail.return_value = { + "version": 1, + "configuration": {"parameters": {"dataApp": {"slug": "x"}}}, + } + ds_mock.get_git_repo.return_value = { + "sshUrl": "ssh://git@git.example.com/keboola/app-42.git", + "httpsUrl": None, + "isManagedGitRepo": True, + } + storage_mock.update_config.return_value = {"version": "2"} + ds_mock.patch_app.return_value = {"state": "starting"} + + service.deploy_data_app(alias="prod", app_id="42") + + update_kwargs = storage_mock.update_config.call_args.kwargs + git_block = update_kwargs["configuration"]["parameters"]["dataApp"]["git"] + assert git_block["repository"] == "ssh://git@git.example.com/keboola/app-42.git" + + def test_deploy_pure_managed_raises_when_git_repo_lookup_empty(self, tmp_path: Path) -> None: + """Neither URL set on the git-repo lookup -- fail loudly rather than + deploy with no source pointer.""" + store = _make_store(tmp_path) + service, ds_mock, storage_mock, _enc = _make_service(store) + ds_mock.get_app.return_value = {"configId": "ulid", "hasManagedGitRepo": True} + storage_mock.get_config_detail.return_value = { + "version": 1, + "configuration": {"parameters": {"dataApp": {"slug": "x"}}}, + } + ds_mock.get_git_repo.return_value = { + "sshUrl": None, + "httpsUrl": None, + "isManagedGitRepo": True, + } + + with pytest.raises(KeboolaApiError): + service.deploy_data_app(alias="prod", app_id="42") + + storage_mock.update_config.assert_not_called() + ds_mock.patch_app.assert_not_called() + + def test_deploy_pure_managed_raises_when_backfill_returns_no_version( + self, tmp_path: Path + ) -> None: + """The backfill PUT returned no version -- falling back to the pre-backfill + version would deploy the no-git-block config (the CLI-15 bug), so fail + loudly instead of deploying.""" + store = _make_store(tmp_path) + service, ds_mock, storage_mock, _enc = _make_service(store) + ds_mock.get_app.return_value = {"configId": "ulid", "hasManagedGitRepo": True} + storage_mock.get_config_detail.return_value = { + "version": 4, + "configuration": {"parameters": {"dataApp": {"slug": "x"}}}, + } + ds_mock.get_git_repo.return_value = { + "sshUrl": None, + "httpsUrl": "https://git.example.com/keboola/app-42.git", + "isManagedGitRepo": True, + } + storage_mock.update_config.return_value = {} + + with pytest.raises(KeboolaApiError, match="did not return a version"): + service.deploy_data_app(alias="prod", app_id="42") + + storage_mock.update_config.assert_called_once() + ds_mock.patch_app.assert_not_called() + + def test_deploy_pure_managed_result_reports_backfill(self, tmp_path: Path) -> None: + store = _make_store(tmp_path) + service, ds_mock, storage_mock, _enc = _make_service(store) + ds_mock.get_app.return_value = {"configId": "ulid", "hasManagedGitRepo": True} + storage_mock.get_config_detail.return_value = { + "version": 4, + "configuration": {"parameters": {"dataApp": {"slug": "x"}}}, + } + ds_mock.get_git_repo.return_value = {"httpsUrl": "https://git.example.com/app-42.git"} + storage_mock.update_config.return_value = {"version": "5"} + ds_mock.patch_app.return_value = {"state": "starting"} + + result = service.deploy_data_app(alias="prod", app_id="42") + + assert result["git_backfilled"] is True + assert len(result["warnings"]) == 1 + assert "parameters.dataApp.git" in result["warnings"][0] + assert "main" in result["warnings"][0] + + @pytest.mark.parametrize("branch", ["dev", "release/1.0"]) + def test_deploy_backfill_writes_and_reports_git_branch( + self, tmp_path: Path, branch: str + ) -> None: + store = _make_store(tmp_path) + service, ds_mock, storage_mock, _enc = _make_service(store) + ds_mock.get_app.return_value = {"configId": "ulid", "hasManagedGitRepo": True} + storage_mock.get_config_detail.return_value = { + "version": 4, + "configuration": {"parameters": {"dataApp": {"slug": "x"}}}, + } + ds_mock.get_git_repo.return_value = {"httpsUrl": "https://git.example.com/app-42.git"} + storage_mock.update_config.return_value = {"version": "5"} + ds_mock.patch_app.return_value = {"state": "starting"} + + result = service.deploy_data_app(alias="prod", app_id="42", git_branch=branch) + + written = storage_mock.update_config.call_args.kwargs["configuration"] + assert written["parameters"]["dataApp"]["git"]["branch"] == branch + assert result["git_branch"] == branch + assert f"branch {branch})" in result["warnings"][0] + assert f"Push your code to {branch} " in result["warnings"][0] + + def test_deploy_backfill_defaults_to_main(self, tmp_path: Path) -> None: + store = _make_store(tmp_path) + service, ds_mock, storage_mock, _enc = _make_service(store) + ds_mock.get_app.return_value = {"configId": "ulid", "hasManagedGitRepo": True} + storage_mock.get_config_detail.return_value = { + "version": 4, + "configuration": {"parameters": {"dataApp": {"slug": "x"}}}, + } + ds_mock.get_git_repo.return_value = {"httpsUrl": "https://git.example.com/app-42.git"} + storage_mock.update_config.return_value = {"version": "5"} + ds_mock.patch_app.return_value = {"state": "starting"} + + result = service.deploy_data_app(alias="prod", app_id="42") + + ds_mock.list_app_runs.assert_called_once_with("42", limit=1) + written = storage_mock.update_config.call_args.kwargs["configuration"] + assert written["parameters"]["dataApp"]["git"]["branch"] == "main" + assert result["git_branch"] == "main" + + def test_deploy_backfill_without_git_branch_fails_for_deployed_app( + self, tmp_path: Path + ) -> None: + """An app with run history deployed before from the managed repo's own + default branch, which no API returns. Writing main could repoint it for + good, so deploy stops before any write and asks for --git-branch.""" + store = _make_store(tmp_path) + service, ds_mock, storage_mock, _enc = _make_service(store) + ds_mock.get_app.return_value = {"configId": "ulid", "hasManagedGitRepo": True} + storage_mock.get_config_detail.return_value = { + "version": 4, + "configuration": {"parameters": {"dataApp": {"slug": "x"}}}, + } + ds_mock.list_app_runs.return_value = [{"state": "running"}] + + with pytest.raises(KeboolaApiError) as exc: + service.deploy_data_app(alias="prod", app_id="42") + + assert exc.value.error_code == ErrorCode.VALIDATION_ERROR + assert "--git-branch BRANCH" in exc.value.message + ds_mock.get_git_repo.assert_not_called() + storage_mock.update_config.assert_not_called() + ds_mock.patch_app.assert_not_called() + + def test_deploy_backfill_explicit_git_branch_skips_run_check(self, tmp_path: Path) -> None: + store = _make_store(tmp_path) + service, ds_mock, storage_mock, _enc = _make_service(store) + ds_mock.get_app.return_value = {"configId": "ulid", "hasManagedGitRepo": True} + storage_mock.get_config_detail.return_value = { + "version": 4, + "configuration": {"parameters": {"dataApp": {"slug": "x"}}}, + } + ds_mock.list_app_runs.return_value = [{"state": "running"}] + ds_mock.get_git_repo.return_value = {"httpsUrl": "https://git.example.com/app-42.git"} + storage_mock.update_config.return_value = {"version": "5"} + ds_mock.patch_app.return_value = {"state": "starting"} + + result = service.deploy_data_app(alias="prod", app_id="42", git_branch="master") + + ds_mock.list_app_runs.assert_not_called() + written = storage_mock.update_config.call_args.kwargs["configuration"] + assert written["parameters"]["dataApp"]["git"]["branch"] == "master" + assert result["git_branch"] == "master" + + def test_deploy_git_branch_is_ignored_without_backfill(self, tmp_path: Path) -> None: + store = _make_store(tmp_path) + service, ds_mock, storage_mock, _enc = _make_service(store) + ds_mock.get_app.return_value = {"configId": "ulid", "hasManagedGitRepo": True} + git_block = {"repository": "https://g/r", "branch": "main"} + storage_mock.get_config_detail.return_value = { + "version": 9, + "configuration": {"parameters": {"dataApp": {"git": git_block}}}, + } + ds_mock.patch_app.return_value = {"state": "starting"} + + result = service.deploy_data_app(alias="prod", app_id="42", git_branch="dev") + + storage_mock.update_config.assert_not_called() + assert "git_branch" not in result + assert "git_backfilled" not in result + + @pytest.mark.parametrize("git_branch", ["x" * 256, "", " "]) + def test_deploy_rejects_invalid_git_branch(self, tmp_path: Path, git_branch: str) -> None: + store = _make_store(tmp_path) + service, ds_mock, _storage_mock, _enc = _make_service(store) + + with pytest.raises(KeboolaApiError) as exc: + service.deploy_data_app(alias="prod", app_id="42", git_branch=git_branch) + + assert exc.value.error_code == ErrorCode.VALIDATION_ERROR + ds_mock.get_app.assert_not_called() + + def test_deploy_with_git_block_does_not_report_backfill(self, tmp_path: Path) -> None: + store = _make_store(tmp_path) + service, ds_mock, storage_mock, _enc = _make_service(store) + ds_mock.get_app.return_value = {"configId": "ulid", "hasManagedGitRepo": True} + storage_mock.get_config_detail.return_value = { + "version": 9, + "configuration": {"parameters": {"dataApp": {"git": {"repository": "https://g/r"}}}}, + } + ds_mock.patch_app.return_value = {"state": "starting"} + + result = service.deploy_data_app(alias="prod", app_id="42") + + assert "git_backfilled" not in result + assert "warnings" not in result + + @pytest.mark.parametrize( + "parameters", + [[], {"dataApp": None}, {"dataApp": []}], + ids=["parameters-list", "dataApp-null", "dataApp-list"], + ) + def test_deploy_pure_managed_backfill_tolerates_non_dict_shapes( + self, tmp_path: Path, parameters: Any + ) -> None: + """The read treats a non-dict parameters / dataApp as empty; the write must too.""" + store = _make_store(tmp_path) + service, ds_mock, storage_mock, _enc = _make_service(store) + ds_mock.get_app.return_value = {"configId": "ulid", "hasManagedGitRepo": True} + storage_mock.get_config_detail.return_value = { + "version": 4, + "configuration": {"parameters": parameters}, + } + ds_mock.get_git_repo.return_value = {"httpsUrl": "https://git.example.com/app-42.git"} + storage_mock.update_config.return_value = {"version": "5"} + ds_mock.patch_app.return_value = {"state": "starting"} + + service.deploy_data_app(alias="prod", app_id="42") + + written = storage_mock.update_config.call_args.kwargs["configuration"] + assert written["parameters"]["dataApp"]["git"]["repository"] == ( + "https://git.example.com/app-42.git" + ) + + def test_deploy_pure_managed_backfill_targets_deploy_branch(self, tmp_path: Path) -> None: + store = _make_store(tmp_path) + service, ds_mock, storage_mock, _enc = _make_service(store) + ds_mock.get_app.return_value = {"configId": "ulid", "hasManagedGitRepo": True} + storage_mock.get_config_detail.return_value = { + "version": 4, + "configuration": {"parameters": {"dataApp": {"slug": "x"}}}, + } + ds_mock.get_git_repo.return_value = {"httpsUrl": "https://git.example.com/app-42.git"} + storage_mock.update_config.return_value = {"version": "5"} + ds_mock.patch_app.return_value = {"state": "starting"} + + service.deploy_data_app(alias="prod", app_id="42", branch_id=777) + + assert storage_mock.get_config_detail.call_args.kwargs["branch_id"] == 777 + assert storage_mock.update_config.call_args.kwargs["branch_id"] == 777 + + def test_deploy_explicit_version_on_pure_managed_skips_backfill(self, tmp_path: Path) -> None: + """An explicit --config-version bypasses resolve_effective_version entirely.""" + store = _make_store(tmp_path) + service, ds_mock, storage_mock, _enc = _make_service(store) + ds_mock.get_app.return_value = {"configId": "ulid", "hasManagedGitRepo": True} + ds_mock.patch_app.return_value = {"state": "starting"} + + result = service.deploy_data_app(alias="prod", app_id="42", config_version="3") + + ds_mock.get_git_repo.assert_not_called() + storage_mock.update_config.assert_not_called() + assert ds_mock.patch_app.call_args.kwargs["config_version"] == "3" + assert "git_backfilled" not in result def test_deploy_managed_with_git_block_pins_latest(self, tmp_path: Path) -> None: """Once a credential is wired (parameters.dataApp.git present), the source @@ -1433,6 +1735,21 @@ def test_ui_url_and_config_read_follow_the_app_branch( "keboola.data-apps", "cfg-1", branch_id=config_branch ) + def test_auth_is_read_from_a_configuration_echoed_as_json_string(self, tmp_path: Path) -> None: + """Some Storage payloads echo ``configuration`` as a JSON string; the auth + check must parse it, not crash on ``str.get`` or report ``missing``.""" + store = _make_store(tmp_path) + service, ds_mock, storage_mock, _enc = _make_service(store) + _stub_password_app(ds_mock, storage_mock, authorization=_build_simple_auth_block()) + storage_mock.get_config_detail.return_value = { + "id": "cfg-1", + "configuration": json.dumps({"authorization": _build_simple_auth_block()}), + } + + result = service.get_data_app_password(alias="prod", app_id="42") + + assert result.password == "deadbeefcafe" + # --------------------------------------------------------------------------- # Delete diff --git a/tests/test_e2e.py b/tests/test_e2e.py index b354002e4..71bb91dd9 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -13769,7 +13769,7 @@ def _run_ok(self, *args: str) -> dict[str, Any]: return _json_ok(self._run(*args)) def test_managed_repo_lifecycle(self) -> None: - """create --use-managed-git-repo -> git-repo -> runs, then delete.""" + """create --use-managed-git-repo -> git-repo -> deploy -> runs, then delete.""" slug = f"e2e-managed-{int(time.time())}" _step(1, "create a managed-repo data app (empty repo, forced --no-deploy)") @@ -13801,7 +13801,20 @@ def test_managed_repo_lifecycle(self) -> None: assert repo["is_managed_git_repo"] is True assert repo["https_url"], f"expected an https clone URL, got {repo}" - _step(3, "runs lists deployment attempts (empty for a never-deployed app)") + _step( + 3, + "deploy backfills parameters.dataApp.git before pinning (CLI-15) -- " + "the repo is still empty, so this only proves the Storage write, not " + "a successful container start", + ) + self._run_ok("data-app", "deploy", "--project", self.alias, "--app-id", app_id) + detail = self._run_ok("data-app", "detail", "--project", self.alias, "--app-id", app_id)[ + "data" + ] + assert detail["git"], f"expected deploy to backfill the git block, got {detail['git']!r}" + assert detail["git"]["repository"] == repo["https_url"] + + _step(4, "runs lists the deploy attempt just made") runs = self._run_ok( "data-app", "runs", "--project", self.alias, "--app-id", app_id, "--limit", "5" )["data"] @@ -13809,7 +13822,7 @@ def test_managed_repo_lifecycle(self) -> None: assert isinstance(runs["runs"], list) assert runs["count"] == len(runs["runs"]) - _step(4, "delete the managed app (cascades the managed repo)") + _step(5, "delete the managed app (cascades the managed repo)") self._run_ok("data-app", "delete", "--project", self.alias, "--app-id", app_id, "--yes") self._created_app_ids.remove(app_id) diff --git a/tests/test_server_router_calls.py b/tests/test_server_router_calls.py index e7a9d3714..bf9e8afa7 100644 --- a/tests/test_server_router_calls.py +++ b/tests/test_server_router_calls.py @@ -1197,6 +1197,23 @@ def test_data_app_create_passes_use_managed_git_repo(tmp_path: Path) -> None: assert kwargs.get("git_repo") == "" +@pytest.mark.parametrize(("query", "branch"), [({"git_branch": "dev"}, "dev"), ({}, None)]) +def test_data_app_deploy_passes_git_branch( + tmp_path: Path, query: dict[str, str], branch: str | None +) -> None: + """POST /data-apps/{p}/{app}/deploy forwards git_branch; not given is None.""" + data_app_svc = MagicMock() + data_app_svc.deploy_data_app.return_value = {"app_id": APP_ID} + registry = _mock_registry(data_app=data_app_svc) + app = _make_app_with_registry(tmp_path, registry) + + with TestClient(app) as client: + res = client.post(f"/data-apps/{PROJECT}/{APP_ID}/deploy", params=query, headers=AUTH) + + assert res.status_code == 200, res.text + assert data_app_svc.deploy_data_app.call_args.kwargs["git_branch"] == branch + + def test_data_app_create_workspace_defaults_on_over_rest(tmp_path: Path) -> None: """The REST surface must default Storage access ON, like the CLI.""" data_app_svc = MagicMock()