diff --git a/CLAUDE.md b/CLAUDE.md index 376610b79..b489e491b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -800,6 +800,24 @@ kbagent data-app create --project ALIAS --name NAME --slug SLUG (--git-repo URL # 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 update --project NAME --app-id ID [--workspace/--no-workspace] [--auto-suspend N] [--size tiny|small|medium|large] [--auth password|public] [--git-branch BRANCH] [--branch ID] [--dry-run] +# data-app update (since vNEXT, #737): the ONLY CLI path that changes Storage access or +# auto-suspend on an app that already exists -- `create` could set them, nothing could +# change them, so the documented workaround was `sync init` + `sync pull` + hand-editing +# _config.yml + `sync push`. Read-modify-write on the linked Storage config: ONLY the +# flags you pass are touched; secrets, the encrypted git `#password` and the storage +# mapping are preserved bit-identical. --workspace is TRI-STATE -- omitting it leaves +# runtime.workspace.enabled alone, it does NOT default to off, so an --auto-suspend +# change cannot silently revoke Storage access. --no-workspace DELETES the +# runtime.workspace key (byte-identical to what `create --no-workspace` writes) rather +# than writing `enabled: false`. A request whose values already match writes NOTHING: +# changed=[], deploy_required=false, no config version minted. --git-branch retargets an +# EXTERNAL repo; a Keboola-managed repo has no parameters.dataApp.git block and fails +# with DATA_APP_INVALID_GIT. Never auto-deploys -- the running container keeps its pinned +# configVersion until `data-app deploy`. `data-app detail` gained `workspace_enabled` +# ("Storage access: enabled/disabled" in human mode) in the same change: before it, +# nothing in the CLI reported whether an app could reach Storage, so the +# deploys-but-every-Storage-call-fails symptom was undiagnosable. kbagent data-app deploy --project NAME --app-id ID [--config-version N] [--wait] [--timeout SECONDS] [--branch ID] kbagent data-app start --project NAME --app-id ID [--wait] [--timeout SECONDS] kbagent data-app stop --project NAME --app-id ID [--wait] [--timeout SECONDS] diff --git a/docs/web-server-endpoints.md b/docs/web-server-endpoints.md index a7b50443b..8112bb826 100644 --- a/docs/web-server-endpoints.md +++ b/docs/web-server-endpoints.md @@ -9,7 +9,7 @@ auth, and the concepts behind these routes live in [`web-server.md`](web-server.md); a running server serves the same spec interactively at `/docs` (Swagger) and `/openapi.json`. -**235 operations** across **205 paths** and **30 routers**. +**236 operations** across **205 paths** and **30 routers**. Paths are shown as the server registers them. Reaching them through the Node BFF (or single-process `--ui` mode) prefixes every path with `/api`. @@ -292,7 +292,7 @@ Flow Notifications-tab recipients (Notification Service subscriptions) -- audit | `POST` | `/notifications/{project}` | Create a notification subscription | | `POST` | `/notifications/{project}/{subscription_id}/replace-recipient` | Replace a subscription's recipient | -### `data-apps` (18 operations) +### `data-apps` (19 operations) Streamlit / R / Python data apps -- create, deploy, start/stop, manage secrets. Mirrors `kbagent data-app *`. @@ -300,6 +300,7 @@ Streamlit / R / Python data apps -- create, deploy, start/stop, manage secrets. |---|---|---| | `GET` | `/data-apps` | List data apps across projects | | `GET` | `/data-apps/{project}/{app_id}` | Get data app detail | +| `PATCH` | `/data-apps/{project}/{app_id}` | Update data app settings | | `DELETE` | `/data-apps/{project}/{app_id}` | Delete a data app | | `POST` | `/data-apps/{project}` | Create a data app | | `POST` | `/data-apps/{project}/{app_id}/deploy` | Deploy a data app version | diff --git a/plugins/kbagent/agents/keboola-expert.md b/plugins/kbagent/agents/keboola-expert.md index dc2187046..12fe03f16 100644 --- a/plugins/kbagent/agents/keboola-expert.md +++ b/plugins/kbagent/agents/keboola-expert.md @@ -352,9 +352,13 @@ its absence is NOT a promise the entry is version-independent (see §1 Rule 6). bumping the version. - **Data app Storage access fails SILENTLY**: no `runtime.workspace.enabled` -> deploys green, reads nothing, platform logs nothing. Empty results -> read - `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. + `data-app detail` -> `workspace_enabled` FIRST (**vNEXT+**; on <= 0.91.0 read + `config detail` -> `configuration.runtime` instead -- an empty `data-app logs` + grep rules nothing out either way). `create` defaults it ON at **0.87.0+**; + <= 0.86.0 patch + redeploy. To FIX it on an existing app: `data-app update + --workspace` then `deploy` (**vNEXT+**); on <= 0.91.0 there is no update + command -- `config update --merge --set 'runtime.workspace.enabled=true'` + then `deploy`. - **`ENCRYPTION_FAILED` on an Azure stack is a VERSION GATE, not a bad token**: <= 0.85.0 rejected the Azure `KBC::ProjectSecureKV::` cipher, so private-repo `create` and `secrets-set` could not work there at all. Upgrade to 0.86.0+; do diff --git a/plugins/kbagent/skills/kbagent/SKILL.md b/plugins/kbagent/skills/kbagent/SKILL.md index 5b70f8ad0..8412a8c81 100644 --- a/plugins/kbagent/skills/kbagent/SKILL.md +++ b/plugins/kbagent/skills/kbagent/SKILL.md @@ -153,6 +153,7 @@ When working inside a git repository or project directory, run `kbagent init` (o | List the keys in parameters.dataApp.secrets, with derived runtime env-var names | `kbagent data-app secrets-list --project PROJECT --app-id APP-ID` | | Show ONE key from parameters.dataApp.secrets | `kbagent data-app secrets-get --project PROJECT --app-id APP-ID --key KEY` | | Remove one or more app-runtime secrets. | `kbagent data-app secrets-remove --project PROJECT --app-id APP-ID --key KEY` | +| Change deployment settings on an existing data app | `kbagent data-app update --project PROJECT --app-id APP-ID` | | List jobs from connected projects | `kbagent job list` | | Show detailed information about a specific job | `kbagent job detail --project PROJECT --job-id JOB-ID` | | Run a job for a component configuration | `kbagent job run --project PROJECT --component-id COMPONENT-ID --config-id CONFIG-ID` | diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index 83281d11d..cbc0ef48c 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -257,8 +257,9 @@ Bucket sharing + linking across projects in the same organization. `sharing edge ## Data Apps (Streamlit / Flask / Node deployments) Lifecycle for `keboola.data-apps`. Combines Storage API (config body, git block, encrypted secrets, runtime size) with Data Science API (`/apps` -- deployment record, state, URL, configVersion). The CLI encapsulates the §9 redeploy contract so callers cannot pin to the empty-shell v2; see `data-app-workflow.md` for the gotcha inventory and recipes. Since v0.33.0 the JSON output envelope's data-app id key is `app_id` (renamed from bare `id` for symmetry with the `--app-id` input flag); `config_id` is unchanged. - `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 detail --project NAME --app-id ID [--branch ID]` -- merged view (state, desired, url, configVersion, slug, git block with PAT redacted). Since vNEXT it also returns `workspace_enabled` (human mode: `Storage access: enabled/disabled`) -- whether `runtime.workspace.enabled` is on, i.e. whether the platform injects `KBC_TOKEN` / `WORKSPACE_ID` / `QUERY_SERVICE_URL`. Before vNEXT nothing in the CLI reported it, so an app that deploys and reports `state=running` while every Storage call fails could not be diagnosed without reading the raw config. - `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]` -- 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` mints a 20-char hex simpleAuth password (retrievable via `data-app password`). **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 is wired into the config; the minted 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 update --project NAME --app-id ID [--workspace/--no-workspace] [--auto-suspend N] [--size tiny|small|medium|large] [--auth password|public] [--git-branch BRANCH] [--branch ID] [--dry-run]` -- (since vNEXT) change deployment settings on an app that ALREADY exists. Before it, `create` was the only place these fields could be set and the documented workaround was `sync init` + `sync pull` + hand-editing `_config.yml` + `sync push`. Read-modify-write on the linked Storage config: ONLY the flags you pass are touched -- secrets, the encrypted git `#password` and the storage mapping survive bit-identical. **`--workspace` is tri-state**: omitting it leaves Storage access unchanged, so an `--auto-suspend` change can never silently revoke it; `--no-workspace` DELETES the `runtime.workspace` key (identical to what `create --no-workspace` writes) rather than writing `enabled: false`. A request whose values already match the config writes NOTHING (`changed: []`, `deploy_required: false`, no config version minted). `--git-branch` retargets an EXTERNAL repo; a Keboola-managed repo has no `parameters.dataApp.git` block and fails with `DATA_APP_INVALID_GIT`. Never auto-deploys -- the running container keeps its pinned `configVersion` until the next `data-app deploy`. - `data-app deploy --project NAME --app-id ID [--config-version N] [--wait] [--timeout SECONDS] [--branch ID]` -- the §9 redeploy contract. 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 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). diff --git a/plugins/kbagent/skills/kbagent/references/data-app-workflow.md b/plugins/kbagent/skills/kbagent/references/data-app-workflow.md index 5bbe1d8f6..64764996b 100644 --- a/plugins/kbagent/skills/kbagent/references/data-app-workflow.md +++ b/plugins/kbagent/skills/kbagent/references/data-app-workflow.md @@ -77,9 +77,13 @@ what makes the platform provision the app's ephemeral workspace and inject: and offered no option to. An app missing it **deploys successfully** -- `state=running`, `desiredState=running`, health probe green, `setup_sh` completed -- and cannot read a row. **The platform emits no diagnostic**, so -the only reliable check is the config itself: +the only reliable check is the app's own settings: ```bash +# vNEXT+ -- detail reports it directly +kbagent --json data-app detail --project P --app-id | jq '.data.workspace_enabled' + +# <= 0.91.0 -- detail did not carry the flag; read the raw config instead kbagent --json config detail --project P --component-id keboola.data-apps \ --config-id | jq '.data.configuration.runtime' ``` @@ -95,12 +99,18 @@ instead, which is at least visible. Silently serving zeros is the dangerous outcome, and it is the one you get from the DuckDB-cached read-only dashboard pattern -- the `dataapp-developer` skill's default. -**Retrofitting an app created before 0.87.0** (or by any other path): +**Retrofitting an app created before 0.87.0** (or created in the UI, or by any +other path): ```bash +# vNEXT+ -- the supported one-liner; only this field is touched +kbagent data-app update --project P --app-id --workspace +kbagent data-app deploy --project P --app-id --wait # pins the LATEST version + +# <= 0.91.0 -- no update command existed; patch the raw config kbagent config update --project P --component-id keboola.data-apps \ --config-id --merge --set 'runtime.workspace.enabled=true' -kbagent data-app deploy --project P --app-id --wait # pins the LATEST version +kbagent data-app deploy --project P --app-id --wait ``` The redeploy is required -- a config change alone does not reach the running @@ -408,7 +418,8 @@ yours at runtime. | Inventory: "what data apps does this project have?" | `data-app list` | | Inspect one: "is this app running? what's its URL?" | `data-app detail --app-id N` | | Bring a new app online from a git repo | `data-app create` (encrypts + PUTs + deploys; Storage access ON by default since 0.87.0) | -| Grant Storage access to an app that lacks it | `config update --merge --set 'runtime.workspace.enabled=true'` → `data-app deploy` | +| Grant Storage access to an app that lacks it | `data-app update --workspace` → `data-app deploy` (vNEXT+; on <= 0.91.0 `config update --merge --set 'runtime.workspace.enabled=true'` → `data-app deploy`) | +| Change auto-suspend / size / auth on an existing app | `data-app update --auto-suspend N` (vNEXT+; only the flags you pass are touched) → `data-app deploy` | | Roll out new code already pushed to git | `data-app deploy --app-id N` | | Roll out a new Storage config | `config update` (any field) → `data-app deploy` | | Wake an auto-suspended app | `data-app start --app-id N` | diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index 750daf78d..eb94ab094 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -4501,6 +4501,52 @@ the web UI. loudly when it is off. The REST surface mirrors the flag as `"workspace": true` in the `POST /data-apps/{project}` body. +## `data-app update` is the only way to change Storage access after create + +*(since vNEXT, #737)* + +`data-app create` could set `runtime.workspace.enabled` and +`parameters.autoSuspendAfterSeconds`; **nothing could change them afterwards.** +For an app created in the UI, or by an older kbagent, the documented path was +`sync init` + `sync pull` + hand-editing `_config.yml` + `sync push` -- a heavy, +schema-aware flow for a two-field change. `data-app update` closes that. + +```bash +kbagent data-app update --project P --app-id 74021026 --workspace +kbagent data-app deploy --project P --app-id 74021026 --wait +``` + +- **`--workspace` is TRI-STATE, not a boolean.** Omitting + `--workspace`/`--no-workspace` leaves Storage access exactly as it is. This + matters: a `--auto-suspend` change must never silently revoke Storage access, + which a flag defaulting to `False` would do on every invocation. +- **`--no-workspace` DELETES `runtime.workspace`** rather than writing + `enabled: false` -- byte-identical to what `create --no-workspace` produces, + so the two paths cannot diverge and `sync diff` sees no phantom drift. +- **Only the flags you pass are touched.** It is a read-modify-write over the + whole config body: `parameters.dataApp.secrets`, the encrypted git + `#password`, and the storage input/output mapping all survive bit-identical. + The Storage API's `configuration` field is a full-document overwrite, so a + hand-built body would silently drop those siblings. +- **A no-op update writes nothing.** If every requested value already matches, + the command returns `changed: []`, `deploy_required: false` and mints **no** + config version -- re-running it is free and does not force a redeploy. +- **`--git-branch` is for EXTERNAL repos only.** A Keboola-managed repo carries + no `parameters.dataApp.git` block (the Git Service owns the link), so there is + no branch to retarget: the command fails with `DATA_APP_INVALID_GIT` instead + of inventing a block. +- **It never deploys.** Per the redeploy contract the running container keeps + its pinned `configVersion` until the next `data-app deploy` -- the result + carries `deploy_required: true` and the exact next command. +- **`data-app detail` finally reports the state.** It now returns + `workspace_enabled` (`Storage access: enabled/disabled` in human mode). Before + this, `detail` showed size, auto-suspend, git and secrets but not + `runtime.workspace`, so the deploys-green-reads-nothing symptom above could + not be diagnosed from the CLI at all -- you had to read + `config detail` -> `configuration.runtime` by hand. +- **REST parity:** `PATCH /data-apps/{project}/{app_id}` with the same field + names; `null` means "leave unchanged" there too. + ## `token list --with-last-used`: `never` and `unknown` are not the same answer (since v0.88.0) `--with-last-used` derives each token's most recent activity so you can tell diff --git a/src/keboola_agent_cli/commands/_data_app_settings.py b/src/keboola_agent_cli/commands/_data_app_settings.py new file mode 100644 index 000000000..e2d3b091a --- /dev/null +++ b/src/keboola_agent_cli/commands/_data_app_settings.py @@ -0,0 +1,135 @@ +"""``data-app update`` -- change deployment settings on an existing app. + +Split out of ``data_app.py`` to respect the file-size budget +(CONTRIBUTING.md "File-size budgets"). The command attaches to the existing +``data-app`` Typer sub-app via :func:`register_settings_commands`, called at +the bottom of ``data_app.py``, so it surfaces as ``kbagent data-app update`` +with the usual ``data-app.update`` permission key. + +``create`` could already set Storage access and auto-suspend; nothing could +change them afterwards. The gap mattered most for ``runtime.workspace.enabled`` +-- the switch that makes the platform inject ``KBC_TOKEN`` / ``WORKSPACE_ID`` +/ ``QUERY_SERVICE_URL`` -- because an app missing it deploys, reports +``state=running`` and passes its health probe while every Storage call fails. +""" + +from __future__ import annotations + +import typer +from rich.console import Console + +from ..errors import ConfigError, ErrorCode, KeboolaApiError +from ._helpers import get_formatter, get_service, map_error_to_exit_code + +_REF_STORAGE_ACCESS = "https://help.keboola.com/data-apps/storage-access/" + + +def register_settings_commands(app: typer.Typer) -> None: + """Attach the ``data-app update`` command to the data-app sub-app.""" + + @app.command("update") + def data_app_update( + ctx: typer.Context, + project: str = typer.Option(..., "--project", help="Project alias"), + app_id: str = typer.Option(..., "--app-id", help="Data Science numeric app id"), + workspace: bool | None = typer.Option( + None, + "--workspace/--no-workspace", + help=( + "Grant/revoke Storage access (runtime.workspace.enabled). This is what " + "makes the platform inject KBC_TOKEN, WORKSPACE_ID, QUERY_SERVICE_URL " + "and KBC_WORKSPACE_MANIFEST_PATH. Omit to leave unchanged." + ), + ), + auto_suspend: int | None = typer.Option( + None, + "--auto-suspend", + help="Seconds of inactivity before the app is suspended.", + ), + size: str | None = typer.Option( + None, "--size", help="Backend size: tiny|small|medium|large." + ), + auth: str | None = typer.Option( + None, "--auth", help="Authentication type: password|public." + ), + git_branch: str | None = typer.Option( + None, + "--git-branch", + help="Retarget the external git repository's branch. Managed repos have no branch to set.", + ), + branch: int | None = typer.Option( + None, + "--branch", + help="Storage branch ID for the linked config (defaults to production).", + ), + dry_run: bool = typer.Option( + False, "--dry-run", help="Show the Storage PUT body without making the call." + ), + ) -> None: + """Change deployment settings on an existing data app. + + Read-modify-write on the linked Storage config: only the flags you pass + are touched, everything else (secrets, encrypted git PAT, storage + mapping) is preserved bit-identical. A request that already matches the + stored config writes nothing and reports deploy_required=false. + + The command never auto-deploys -- per the redeploy contract the running + container keeps its pinned configVersion until the next + 'kbagent data-app deploy'. + + Reference: https://help.keboola.com/data-apps/storage-access/ + """ + formatter = get_formatter(ctx) + service = get_service(ctx, "data_app_service") + + # Mirror the secrets-set precedent: catch "no field given" here so it + # exits 2 (usage) rather than falling through to the service's generic + # error exit 1. + if all(value is None for value in (workspace, auto_suspend, size, auth, git_branch)): + formatter.error( + message=( + "Nothing to update. Pass at least one of " + "--workspace/--no-workspace, --auto-suspend, --size, --auth, " + "--git-branch." + ), + error_code=ErrorCode.MISSING_PARAMETER, + ) + raise typer.Exit(code=2) from None + + try: + result = service.update_data_app( + alias=project, + app_id=app_id, + workspace=workspace, + auto_suspend_after_seconds=auto_suspend, + size=size, + auth=auth, + git_branch=git_branch, + branch_id=branch, + dry_run=dry_run, + ) + except KeboolaApiError as exc: + formatter.error( + message=exc.message, + error_code=exc.error_code, + retryable=exc.retryable, + details=exc.details, + ) + raise typer.Exit(code=map_error_to_exit_code(exc)) from None + except ConfigError as exc: + formatter.error(message=exc.message, error_code=ErrorCode.CONFIG_ERROR) + raise typer.Exit(code=5) from None + + def _human(c: Console, d: dict) -> None: + c.print(f"[bold green]Success:[/bold green] {d['message']}") + for change in d.get("changes", []): + c.print( + f" [bold]{change['field']}:[/bold] {change['before']!r} -> " + f"[cyan]{change['after']!r}[/cyan]" + ) + if d.get("changed") and "workspace" in d["changed"]: + c.print(f" [dim]Storage access reference: {_REF_STORAGE_ACCESS}[/dim]") + + formatter.output(result, _human) + if not formatter.json_mode and result.get("next_step"): + formatter.console.print(f"[dim]Next: {result['next_step']}[/dim]") diff --git a/src/keboola_agent_cli/commands/context.py b/src/keboola_agent_cli/commands/context.py index 012f64668..1035bf7b2 100644 --- a/src/keboola_agent_cli/commands/context.py +++ b/src/keboola_agent_cli/commands/context.py @@ -1274,6 +1274,11 @@ kbagent data-app detail --project NAME --app-id ID [--branch ID] Full merged view: state, desiredState, url, deployed configVersion, slug, runtime size, git settings (PAT redacted as ). + `workspace_enabled` (since vNEXT) reports whether Storage access is on -- + i.e. whether the platform injects KBC_TOKEN / WORKSPACE_ID / + QUERY_SERVICE_URL. Human mode prints it as "Storage access: + enabled/disabled". Before vNEXT nothing in `detail` showed it, so the + missing-token symptom could not be diagnosed from the CLI at all. kbagent data-app create --project ALIAS --name NAME --slug SLUG (--git-repo URL | --use-managed-git-repo) @@ -1310,6 +1315,24 @@ --config-id ID --merge --set 'runtime.workspace.enabled=true' then redeploy (deploy pins the LATEST version, so the change takes effect). + kbagent data-app update --project NAME --app-id ID + [--workspace/--no-workspace] [--auto-suspend N] [--size tiny|small|medium|large] + [--auth password|public] [--git-branch BRANCH] [--branch ID] [--dry-run] + (since vNEXT) Change deployment settings on an EXISTING app. Read-modify-write + on the linked Storage config: ONLY the flags you pass are touched, everything + else (secrets, the encrypted git #password, storage mapping) is preserved + bit-identical. Omitting --workspace/--no-workspace leaves Storage access + unchanged -- it is tri-state, not a boolean defaulting to off. + --no-workspace DELETES runtime.workspace, matching what `create + --no-workspace` writes, rather than writing `enabled: false`. + A request that already matches the stored config writes NOTHING: the result + reports changed=[] and deploy_required=false, and no config version is minted. + --git-branch retargets an EXTERNAL repo's branch; on a Keboola-managed repo + (no parameters.dataApp.git block) it fails with DATA_APP_INVALID_GIT. + Never auto-deploys -- the running container keeps its pinned configVersion + until the next `data-app deploy`. Before vNEXT the only CLI path to these + fields was `sync init` + `sync pull` + hand-editing _config.yml + `sync push`. + kbagent data-app deploy --project NAME --app-id ID [--config-version N] [--wait] [--timeout SECONDS] [--branch ID] The §9 redeploy contract. Default reads the latest Storage config version diff --git a/src/keboola_agent_cli/commands/data_app.py b/src/keboola_agent_cli/commands/data_app.py index b81700e23..2689a6638 100644 --- a/src/keboola_agent_cli/commands/data_app.py +++ b/src/keboola_agent_cli/commands/data_app.py @@ -22,6 +22,7 @@ from ..errors import ConfigError, ErrorCode, KeboolaApiError from ._data_app_git import register_git_commands from ._data_app_runtime import register_secrets_commands +from ._data_app_settings import register_settings_commands from ._helpers import ( check_cli_permission, emit_project_warnings, @@ -179,6 +180,10 @@ def data_app_detail( ), c.print(f" [bold]Size:[/bold] {d.get('size', '')}"), c.print(f" [bold]Auto-suspend:[/bold] {d.get('auto_suspend_after_seconds', '?')}s"), + c.print( + " [bold]Storage access:[/bold] " + + ("enabled" if d.get("workspace_enabled") else "[yellow]disabled[/yellow]") + ), c.print(f" [bold]URL:[/bold] {d.get('url', '')}"), c.print(f" [bold]Last started:[/bold] {d.get('last_start_timestamp', '')}"), c.print(f" [bold]Git:[/bold] {d.get('git', {})}"), @@ -999,3 +1004,4 @@ def data_app_validate_repo( # as `kbagent data-app git-*` / `secrets-*` on this sub-app. register_git_commands(data_app_app) register_secrets_commands(data_app_app) +register_settings_commands(data_app_app) diff --git a/src/keboola_agent_cli/permissions.py b/src/keboola_agent_cli/permissions.py index adfbc329d..6a0789ec9 100644 --- a/src/keboola_agent_cli/permissions.py +++ b/src/keboola_agent_cli/permissions.py @@ -194,6 +194,7 @@ "data-app.logs": "read", "data-app.runs": "read", "data-app.create": "write", + "data-app.update": "write", "data-app.deploy": "write", "data-app.start": "write", "data-app.stop": "write", diff --git a/src/keboola_agent_cli/server/_serve_command_map.py b/src/keboola_agent_cli/server/_serve_command_map.py index fffe1b31a..242c93822 100644 --- a/src/keboola_agent_cli/server/_serve_command_map.py +++ b/src/keboola_agent_cli/server/_serve_command_map.py @@ -147,6 +147,7 @@ ("GET", "/workspaces"): "workspace list", ("GET", "/workspaces/{project}/{workspace_id}"): "workspace detail", ("PATCH", "/agents/{task_id}"): "agent update", + ("PATCH", "/data-apps/{project}/{app_id}"): "data-app update", ("PATCH", "/configs/{project}/{component_id}/{config_id}"): "config update", ("PATCH", "/configs/{project}/{component_id}/{config_id}/rows/{row_id}"): "config row-update", ("PATCH", "/flows/{project}/{config_id}"): "flow update", diff --git a/src/keboola_agent_cli/server/routers/data_apps.py b/src/keboola_agent_cli/server/routers/data_apps.py index 2f39dd312..23210b8e9 100644 --- a/src/keboola_agent_cli/server/routers/data_apps.py +++ b/src/keboola_agent_cli/server/routers/data_apps.py @@ -62,6 +62,18 @@ class DataAppCreate(BaseModel): dry_run: bool = False +class DataAppUpdate(BaseModel): + """Fields changeable on an existing app. ``None`` means "leave unchanged".""" + + workspace: bool | None = None + auto_suspend_after_seconds: int | None = None + size: str | None = None + auth: str | None = None + git_branch: str | None = None + branch_id: int | None = None + dry_run: bool = False + + class SecretsSet(BaseModel): secrets: dict[str, str] branch_id: int | None = None @@ -144,6 +156,27 @@ def create( ) +@router.patch("/{project}/{app_id}", summary="Update data app settings") +def update( + project: str, + app_id: str, + body: DataAppUpdate, + registry: ServiceRegistry = Depends(get_registry), +) -> dict[str, Any]: + """Change settings on an existing data app. Mirrors `kbagent data-app update`.""" + return registry.data_app.update_data_app( + alias=project, + app_id=app_id, + workspace=body.workspace, + auto_suspend_after_seconds=body.auto_suspend_after_seconds, + size=body.size, + auth=body.auth, + git_branch=body.git_branch, + branch_id=body.branch_id, + dry_run=body.dry_run, + ) + + @router.post("/{project}/{app_id}/deploy", summary="Deploy a data app version") def deploy( project: str, diff --git a/src/keboola_agent_cli/services/_data_app_update.py b/src/keboola_agent_cli/services/_data_app_update.py new file mode 100644 index 000000000..74095d392 --- /dev/null +++ b/src/keboola_agent_cli/services/_data_app_update.py @@ -0,0 +1,373 @@ +"""In-place edits of an existing data app's Storage config (``data-app update``). + +Split out of ``data_app_service.py``: that module is grandfathered at its +current size by ``scripts/check_file_size.py`` and may only shrink, so new +functionality lands beside it as a mixin (the same composition pattern the +``client/`` package uses). :class:`DataAppUpdateMixin` is mixed into +:class:`~keboola_agent_cli.services.data_app_service.DataAppService`, so +``update_data_app`` and the shared config loader are ordinary methods on the +service from every caller's point of view. + +The read-modify-write contract mirrors the secrets path: the Storage API's +``configuration`` field is a full-document overwrite, so we GET the whole +config, mutate only the requested keys, and PUT the untouched remainder back. +""" + +from __future__ import annotations + +import json +from collections.abc import Callable +from typing import Any + +from ..errors import ErrorCode, KeboolaApiError +from ._data_app_bodies import _auth_block_for, _redact_storage_config +from .base import BaseService + +# Keys the caller may change. Order is the order they are reported in. +UPDATABLE_FIELDS = ("workspace", "auto_suspend_after_seconds", "size", "auth", "git_branch") + + +class DataAppUpdateMixin(BaseService): + """``update_data_app`` plus the Storage-config loader it shares with secrets. + + Inherits :class:`BaseService` so ``resolve_projects`` / ``_client_factory`` + type-check here as they do on the concrete service; ``_ds_client_factory`` + is annotated because ``DataAppService.__init__`` is what assigns it. + """ + + _ds_client_factory: Callable[[str, str], Any] + + def _load_data_app_storage_config( + self, + *, + ds_client: Any, + storage_client: Any, + app_id: str, + branch_id: int | None, + ) -> tuple[str, dict[str, Any], dict[str, Any]]: + """Resolve ``configId`` from the Data Science app and load the Storage config. + + Returns ``(config_id, storage_envelope, body)`` where ``body`` is a + deep-copy of ``storage_envelope.configuration`` so callers can + mutate it freely. + """ + app = ds_client.get_app(app_id) + config_id = str(app.get("configId") or "") + if not config_id: + raise KeboolaApiError( + message=f"Data app {app_id} has no associated configId", + status_code=500, + error_code=ErrorCode.API_ERROR, + retryable=False, + ) + envelope = storage_client.get_config_detail(_component_id(), config_id, branch_id=branch_id) + if not isinstance(envelope, dict): + envelope = {} + configuration = envelope.get("configuration") + if not isinstance(configuration, dict): + configuration = {} + # Deep-copy so caller mutations don't reach the cached upstream. + body = json.loads(json.dumps(configuration)) + return config_id, envelope, body + + def update_data_app( + self, + *, + alias: str, + app_id: str, + workspace: bool | None = None, + auto_suspend_after_seconds: int | None = None, + size: str | None = None, + auth: str | None = None, + git_branch: str | None = None, + branch_id: int | None = None, + dry_run: bool = False, + ) -> dict[str, Any]: + """Change deployment settings on an EXISTING data app's Storage config. + + Only the fields the caller passed are touched; every other key in the + config body -- including ``parameters.dataApp.secrets`` and the git + block's encrypted ``#password`` -- is preserved bit-identical. + + ``workspace`` is the switch behind ``runtime.workspace.enabled``: the + one that makes the platform inject ``KBC_TOKEN`` / ``WORKSPACE_ID`` / + ``QUERY_SERVICE_URL``. ``workspace=False`` deletes the key rather than + writing ``enabled: false``, matching what ``create --no-workspace`` + produces so the two paths converge on the same body. + + Never deploys: per the §9 redeploy contract the running container + keeps its pinned ``configVersion`` until the next ``data-app deploy``. + """ + requested = { + "workspace": workspace, + "auto_suspend_after_seconds": auto_suspend_after_seconds, + "size": size, + "auth": auth, + "git_branch": git_branch, + } + if all(value is None for value in requested.values()): + raise KeboolaApiError( + message=( + "Nothing to update. Pass at least one of --workspace/--no-workspace, " + "--auto-suspend, --size, --auth, --git-branch." + ), + status_code=0, + error_code=ErrorCode.MISSING_PARAMETER, + retryable=False, + ) + self._validate_update_inputs( + auto_suspend_after_seconds=auto_suspend_after_seconds, + size=size, + auth=auth, + git_branch=git_branch, + ) + + projects = self.resolve_projects([alias]) + project = projects[alias] + ds_client = self._ds_client_factory(project.stack_url, project.token) + storage_client = self._client_factory(project.stack_url, project.token) + try: + config_id, envelope, body = self._load_data_app_storage_config( + ds_client=ds_client, + storage_client=storage_client, + app_id=str(app_id), + branch_id=branch_id, + ) + new_body, changes = _apply_update(body, requested) + version_before = str(envelope.get("version", "") or "") + + base: dict[str, Any] = { + "project_alias": alias, + "app_id": str(app_id), + "config_id": config_id, + "changed": [entry["field"] for entry in changes], + "changes": changes, + "config_version_before": version_before, + } + + if not changes: + # Idempotent: every requested value already matches. No PUT, + # so no pointless config version and no redeploy prompt. + return { + **base, + "config_version_after": version_before, + "deploy_required": False, + "message": ( + f"Data app {app_id} already matches the requested settings; " + "nothing written." + ), + } + + if dry_run: + return { + **base, + "dry_run": True, + "put_storage_config_preview": _redact_storage_config( + {"configuration": new_body} + ), + "message": ( + f"Dry run -- would update {len(changes)} field(s) and PUT the " + "body above. No API call made." + ), + } + + put_response = storage_client.update_config( + component_id=_component_id(), + config_id=config_id, + configuration=new_body, + change_description=( + f"Update {', '.join(entry['field'] for entry in changes)} " + "via kbagent data-app update" + ), + branch_id=branch_id, + ) + return { + **base, + "config_version_after": str(put_response.get("version", "") or ""), + "deploy_required": True, + "next_step": ( + f"kbagent data-app deploy --project {alias} --app-id {app_id} --wait" + ), + "message": ( + f"{len(changes)} field(s) updated on data app {app_id}. " + "The running container keeps the old config until you redeploy." + ), + } + finally: + ds_client.close() + storage_client.close() + + def _validate_update_inputs( + self, + *, + auto_suspend_after_seconds: int | None, + size: str | None, + auth: str | None, + git_branch: str | None, + ) -> None: + """Service-boundary validation; the CLI validates too, this is defence in depth.""" + from .data_app_service import MAX_GIT_BRANCH_LENGTH, VALID_SIZES, _has_control_chars + + if size is not None and size not in VALID_SIZES: + raise KeboolaApiError( + message=f"Invalid --size '{size}'. Valid values: {', '.join(VALID_SIZES)}", + status_code=0, + error_code=ErrorCode.VALIDATION_ERROR, + retryable=False, + ) + if auth is not None and auth not in ("password", "public"): + raise KeboolaApiError( + message=f"Invalid --auth '{auth}'. Valid values: password, public", + status_code=0, + error_code=ErrorCode.VALIDATION_ERROR, + retryable=False, + ) + if auto_suspend_after_seconds is not None and auto_suspend_after_seconds < 0: + raise KeboolaApiError( + message="--auto-suspend must be a non-negative number of seconds.", + status_code=0, + error_code=ErrorCode.VALIDATION_ERROR, + retryable=False, + ) + if git_branch is not None: + if not git_branch or len(git_branch) > MAX_GIT_BRANCH_LENGTH: + raise KeboolaApiError( + message=( + "--git-branch must be non-empty and at most " + f"{MAX_GIT_BRANCH_LENGTH} characters." + ), + status_code=0, + error_code=ErrorCode.VALIDATION_ERROR, + retryable=False, + ) + if _has_control_chars(git_branch): + raise KeboolaApiError( + message="--git-branch contains disallowed control characters.", + status_code=0, + error_code=ErrorCode.VALIDATION_ERROR, + retryable=False, + ) + + +def _component_id() -> str: + """Late import: ``data_app_service`` imports this module, so the constants it + owns can only be reached from inside a function body.""" + from .data_app_service import DATA_APP_COMPONENT_ID + + return DATA_APP_COMPONENT_ID + + +def read_workspace_enabled(configuration: Any) -> bool: + """Return ``runtime.workspace.enabled`` from a data-app config body. + + Absent key == disabled: ``create --no-workspace`` omits the block + entirely rather than writing ``enabled: false``. + """ + if not isinstance(configuration, dict): + return False + runtime = configuration.get("runtime") + if not isinstance(runtime, dict): + return False + workspace = runtime.get("workspace") + if not isinstance(workspace, dict): + return False + return bool(workspace.get("enabled", False)) + + +def _apply_update( + body: dict[str, Any], requested: dict[str, Any] +) -> tuple[dict[str, Any], list[dict[str, Any]]]: + """Return ``(new_body, changes)`` -- a deep copy with only the asked-for edits. + + ``changes`` carries one ``{field, before, after}`` entry per field that + actually differs, so a no-op update can skip the Storage PUT entirely + instead of minting a config version that changes nothing. + """ + new_body: dict[str, Any] = json.loads(json.dumps(body)) if isinstance(body, dict) else {} + changes: list[dict[str, Any]] = [] + + def record(field: str, before: Any, after: Any) -> bool: + if before == after: + return False + changes.append({"field": field, "before": before, "after": after}) + return True + + parameters = new_body.setdefault("parameters", {}) + if not isinstance(parameters, dict): + parameters = {} + new_body["parameters"] = parameters + runtime = new_body.setdefault("runtime", {}) + if not isinstance(runtime, dict): + runtime = {} + new_body["runtime"] = runtime + + workspace = requested["workspace"] + if workspace is not None and record("workspace", read_workspace_enabled(body), workspace): + if workspace: + runtime["workspace"] = {"enabled": True} + else: + # Mirror `create --no-workspace`: drop the key, don't write false. + runtime.pop("workspace", None) + + auto_suspend = requested["auto_suspend_after_seconds"] + if auto_suspend is not None and record( + "auto_suspend_after_seconds", + parameters.get("autoSuspendAfterSeconds"), + auto_suspend, + ): + parameters["autoSuspendAfterSeconds"] = auto_suspend + + size = requested["size"] + if size is not None: + backend = runtime.get("backend") + if not isinstance(backend, dict): + backend = {} + if record("size", backend.get("size"), size): + backend["size"] = size + runtime["backend"] = backend + + auth = requested["auth"] + if auth is not None: + after_block = _auth_block_for(auth) + if record("auth", _describe_auth(new_body.get("authorization")), auth): + new_body["authorization"] = after_block + + git_branch = requested["git_branch"] + if git_branch is not None: + data_app = parameters.get("dataApp") + if not isinstance(data_app, dict): + data_app = {} + git_block = data_app.get("git") + if not isinstance(git_block, dict): + # A managed-repo app has no parameters.dataApp.git -- the Git + # Service owns the link -- so there is no branch to retarget. + raise KeboolaApiError( + message=( + "--git-branch has no effect on this app: its config carries no " + "parameters.dataApp.git block (a Keboola-managed repository, or an " + "app that has never been configured with an external repo)." + ), + status_code=0, + error_code=ErrorCode.DATA_APP_INVALID_GIT, + retryable=False, + ) + if record("git_branch", git_block.get("branch"), git_branch): + git_block["branch"] = git_branch + data_app["git"] = git_block + parameters["dataApp"] = data_app + + return new_body, changes + + +def _describe_auth(authorization: Any) -> str | None: + """Collapse an ``authorization`` block back to ``password`` / ``public``. + + Anything the two builders did not produce reports as ``None`` so the + update is treated as a real change rather than silently skipped. + """ + if not isinstance(authorization, dict): + return None + for candidate in ("password", "public"): + if authorization == _auth_block_for(candidate): + return candidate + return None diff --git a/src/keboola_agent_cli/services/data_app_service.py b/src/keboola_agent_cli/services/data_app_service.py index 33eabfa79..a319f3505 100644 --- a/src/keboola_agent_cli/services/data_app_service.py +++ b/src/keboola_agent_cli/services/data_app_service.py @@ -44,6 +44,7 @@ _redact_storage_config, _secret_fingerprint, ) +from ._data_app_update import DataAppUpdateMixin, read_workspace_enabled from .base import BaseService, ClientFactory, project_error_entry from .encrypt_service import EncryptService @@ -141,7 +142,7 @@ def _has_control_chars(value: str, *, allow_whitespace: bool = False) -> bool: SECRET_OR_PLAIN_KEY_PATTERN = re.compile(r"^#?[A-Za-z][A-Za-z0-9_-]{0,63}$") -class DataAppService(BaseService): +class DataAppService(DataAppUpdateMixin, BaseService): """Lifecycle service for Keboola data apps. Wires together the Data Science API (deployment record), the Storage @@ -311,6 +312,7 @@ def get_data_app( ), "last_start_timestamp": app.get("lastStartTimestamp"), "slug": data_app_block.get("slug", ""), + "workspace_enabled": read_workspace_enabled(configuration), "git": _redact_git_block(git_block) if git_block else {}, "raw": { "deployment": app, @@ -945,42 +947,6 @@ def get_app_logs( # Secrets lifecycle (parameters.dataApp.secrets in the Storage config) # ------------------------------------------------------------------ - def _load_data_app_storage_config( - self, - *, - ds_client: DataScienceClient, - storage_client: Any, - app_id: str, - branch_id: int | None, - ) -> tuple[str, dict[str, Any], dict[str, Any]]: - """Resolve ``configId`` from the Data Science app and load the Storage config. - - Returns ``(config_id, storage_envelope, body)`` where ``body`` is a - deep-copy of ``storage_envelope.configuration`` so callers can - mutate it freely. Mirrors the pattern at - :meth:`deploy_data_app` (data_app_service.py:586-594). - """ - app = ds_client.get_app(app_id) - config_id = str(app.get("configId") or "") - if not config_id: - raise KeboolaApiError( - message=f"Data app {app_id} has no associated configId", - status_code=500, - error_code=ErrorCode.API_ERROR, - retryable=False, - ) - envelope = storage_client.get_config_detail( - DATA_APP_COMPONENT_ID, config_id, branch_id=branch_id - ) - if not isinstance(envelope, dict): - envelope = {} - configuration = envelope.get("configuration") - if not isinstance(configuration, dict): - configuration = {} - # Deep-copy so caller mutations don't reach the cached upstream. - body = json.loads(json.dumps(configuration)) - return config_id, envelope, body - def _read_secrets_block(self, body: dict[str, Any]) -> dict[str, str]: """Return ``parameters.dataApp.secrets`` from a config body, or ``{}``.""" if not isinstance(body, dict): diff --git a/tests/test_data_app_update_cli.py b/tests/test_data_app_update_cli.py new file mode 100644 index 000000000..7a827d42f --- /dev/null +++ b/tests/test_data_app_update_cli.py @@ -0,0 +1,257 @@ +"""CLI-layer tests for ``kbagent data-app update`` (issue #737). + +Mirrors test_data_app_cli.py: patch the cli.py service factory so the runner +sees a MagicMock, then assert flag plumbing, exit codes and the JSON envelope. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from unittest.mock import MagicMock, patch + +from typer.testing import CliRunner + +from keboola_agent_cli.cli import app +from keboola_agent_cli.config_store import ConfigStore +from keboola_agent_cli.errors import ErrorCode, KeboolaApiError +from keboola_agent_cli.models import ProjectConfig +from keboola_agent_cli.services.config_service import ConfigService +from keboola_agent_cli.services.job_service import JobService +from keboola_agent_cli.services.project_service import ProjectService + +TEST_TOKEN = "901-55555-fakeTestTokenDoNotUseXXXXXXXX" + +runner = CliRunner() + + +def _setup_config(config_dir: Path) -> ConfigStore: + store = ConfigStore(config_dir=config_dir) + store.add_project( + "prod", + ProjectConfig( + stack_url="https://connection.keboola.com", + token=TEST_TOKEN, + project_name="prod", + project_id=1234, + ), + ) + return store + + +def _invoke(args: list[str], *, store: ConfigStore, data_app_mock: MagicMock): + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProj, + patch("keboola_agent_cli.cli.ConfigService") as MockCfg, + patch("keboola_agent_cli.cli.JobService") as MockJob, + patch("keboola_agent_cli.cli.DataAppService") as MockDataAppService, + ): + MockStore.return_value = store + MockProj.return_value = ProjectService(config_store=store) + MockCfg.return_value = ConfigService(config_store=store) + MockJob.return_value = JobService(config_store=store) + MockDataAppService.return_value = data_app_mock + return runner.invoke(app, args) + + +def _success_payload(**overrides): + payload = { + "project_alias": "prod", + "app_id": "74021026", + "config_id": "9001", + "changed": ["workspace"], + "changes": [{"field": "workspace", "before": False, "after": True}], + "config_version_before": "7", + "config_version_after": "8", + "deploy_required": True, + "next_step": "kbagent data-app deploy --project prod --app-id 74021026 --wait", + "message": "1 field(s) updated on data app 74021026.", + } + payload.update(overrides) + return payload + + +def test_update_workspace_flag_reaches_the_service(tmp_path: Path) -> None: + store = _setup_config(tmp_path / "cfg") + mock = MagicMock() + mock.update_data_app.return_value = _success_payload() + + result = _invoke( + [ + "--json", + "data-app", + "update", + "--project", + "prod", + "--app-id", + "74021026", + "--workspace", + ], + store=store, + data_app_mock=mock, + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["data"]["changed"] == ["workspace"] + assert mock.update_data_app.call_args.kwargs["workspace"] is True + + +def test_update_no_workspace_sends_false(tmp_path: Path) -> None: + store = _setup_config(tmp_path / "cfg") + mock = MagicMock() + mock.update_data_app.return_value = _success_payload() + + result = _invoke( + [ + "data-app", + "update", + "--project", + "prod", + "--app-id", + "74021026", + "--no-workspace", + ], + store=store, + data_app_mock=mock, + ) + + assert result.exit_code == 0, result.output + assert mock.update_data_app.call_args.kwargs["workspace"] is False + + +def test_update_omitting_workspace_leaves_it_unset(tmp_path: Path) -> None: + """No ``--workspace``/``--no-workspace`` must send ``None``, not ``False``. + + A ``False`` default would silently revoke Storage access on every + ``--auto-suspend`` change. + """ + store = _setup_config(tmp_path / "cfg") + mock = MagicMock() + mock.update_data_app.return_value = _success_payload( + changed=["auto_suspend_after_seconds"], + changes=[{"field": "auto_suspend_after_seconds", "before": 900, "after": 300}], + ) + + result = _invoke( + [ + "data-app", + "update", + "--project", + "prod", + "--app-id", + "74021026", + "--auto-suspend", + "300", + ], + store=store, + data_app_mock=mock, + ) + + assert result.exit_code == 0, result.output + kwargs = mock.update_data_app.call_args.kwargs + assert kwargs["workspace"] is None + assert kwargs["auto_suspend_after_seconds"] == 300 + + +def test_update_passes_size_auth_git_branch_and_dry_run(tmp_path: Path) -> None: + store = _setup_config(tmp_path / "cfg") + mock = MagicMock() + mock.update_data_app.return_value = _success_payload(dry_run=True) + + result = _invoke( + [ + "data-app", + "update", + "--project", + "prod", + "--app-id", + "74021026", + "--size", + "small", + "--auth", + "public", + "--git-branch", + "release", + "--branch", + "42", + "--dry-run", + ], + store=store, + data_app_mock=mock, + ) + + assert result.exit_code == 0, result.output + kwargs = mock.update_data_app.call_args.kwargs + assert kwargs["size"] == "small" + assert kwargs["auth"] == "public" + assert kwargs["git_branch"] == "release" + assert kwargs["branch_id"] == 42 + assert kwargs["dry_run"] is True + + +def test_update_without_any_flag_is_a_usage_error(tmp_path: Path) -> None: + """Caught in the command layer so it exits 2, not the generic 1.""" + store = _setup_config(tmp_path / "cfg") + mock = MagicMock() + + result = _invoke( + ["--json", "data-app", "update", "--project", "prod", "--app-id", "74021026"], + store=store, + data_app_mock=mock, + ) + + assert result.exit_code == 2 + assert json.loads(result.stdout)["error"]["code"] == "MISSING_PARAMETER" + mock.update_data_app.assert_not_called() + + +def test_update_maps_service_error_to_exit_code(tmp_path: Path) -> None: + store = _setup_config(tmp_path / "cfg") + mock = MagicMock() + mock.update_data_app.side_effect = KeboolaApiError( + message="boom", + status_code=500, + error_code=ErrorCode.API_ERROR, + retryable=False, + ) + + result = _invoke( + [ + "--json", + "data-app", + "update", + "--project", + "prod", + "--app-id", + "74021026", + "--workspace", + ], + store=store, + data_app_mock=mock, + ) + + assert result.exit_code == 1 + assert json.loads(result.stdout)["error"]["code"] == "API_ERROR" + + +def test_detail_prints_storage_access(tmp_path: Path) -> None: + store = _setup_config(tmp_path / "cfg") + mock = MagicMock() + mock.get_data_app.return_value = { + "project_alias": "prod", + "app_id": "74021026", + "name": "People Review", + "workspace_enabled": False, + "git": {}, + } + + result = _invoke( + ["data-app", "detail", "--project", "prod", "--app-id", "74021026"], + store=store, + data_app_mock=mock, + ) + + assert result.exit_code == 0, result.output + assert "Storage access:" in result.output + assert "disabled" in result.output diff --git a/tests/test_data_app_update_service.py b/tests/test_data_app_update_service.py new file mode 100644 index 000000000..cfec749d9 --- /dev/null +++ b/tests/test_data_app_update_service.py @@ -0,0 +1,271 @@ +"""Service-layer tests for ``DataAppService.update_data_app`` (issue #737). + +Covers the read-modify-write contract: only the requested fields change, +everything else in the config body survives bit-identical, and a request +that already matches the stored config writes nothing at all. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock + +import pytest + +from keboola_agent_cli.config_store import ConfigStore +from keboola_agent_cli.errors import ErrorCode, KeboolaApiError +from keboola_agent_cli.models import ProjectConfig +from keboola_agent_cli.services.data_app_service import DataAppService + +TEST_TOKEN = "901-55555-fakeTestTokenDoNotUseXXXXXXXX" + + +def _make_store(tmp_path: Path) -> ConfigStore: + config_dir = tmp_path / "config" + config_dir.mkdir() + store = ConfigStore(config_dir=config_dir) + store.add_project( + "prod", + ProjectConfig( + stack_url="https://connection.keboola.com", + token=TEST_TOKEN, + project_name="prod", + project_id=5725, + ), + ) + return store + + +def _base_configuration() -> dict[str, Any]: + """A realistic data-app config body: git + secrets + storage mapping.""" + return { + "parameters": { + "id": "74021026", + "autoSuspendAfterSeconds": 900, + "dataApp": { + "slug": "people-review", + "git": { + "repository": "https://github.com/org/repo", + "branch": "main", + "private": True, + "username": "u", + "#password": "KBC::ProjectSecureGKMS::ciphertext", + }, + "secrets": {"#API_KEY": "KBC::ProjectSecureGKMS::secret"}, + }, + }, + "runtime": {"backend": {"size": "tiny"}}, + "storage": {"input": {"tables": [{"source": "in.c-main.data"}]}}, + "authorization": { + "app_proxy": { + "auth_providers": [{"id": "simpleAuth", "type": "password"}], + "auth_rules": [ + { + "type": "pathPrefix", + "value": "/", + "auth_required": True, + "auth": ["simpleAuth"], + } + ], + } + }, + } + + +def _make_service( + store: ConfigStore, configuration: dict[str, Any] | None = None +) -> tuple[DataAppService, MagicMock, MagicMock]: + ds_mock = MagicMock() + ds_mock.get_app.return_value = {"id": "74021026", "configId": "9001"} + storage_mock = MagicMock() + storage_mock.get_config_detail.return_value = { + "id": "9001", + "version": 7, + "configuration": _base_configuration() if configuration is None else configuration, + } + storage_mock.update_config.return_value = {"version": 8} + service = DataAppService( + config_store=store, + client_factory=lambda url, token: storage_mock, + ds_client_factory=lambda url, token: ds_mock, + encrypt_service=MagicMock(), + ) + return service, ds_mock, storage_mock + + +def _put_body(storage_mock: MagicMock) -> dict[str, Any]: + return storage_mock.update_config.call_args.kwargs["configuration"] + + +# --------------------------------------------------------------------------- +# Enabling Storage access -- the issue's primary ask +# --------------------------------------------------------------------------- + + +def test_update_enables_workspace(tmp_path: Path) -> None: + service, _ds, storage = _make_service(_make_store(tmp_path)) + result = service.update_data_app(alias="prod", app_id="74021026", workspace=True) + + assert result["changed"] == ["workspace"] + assert result["deploy_required"] is True + assert result["config_version_before"] == "7" + assert result["config_version_after"] == "8" + assert _put_body(storage)["runtime"]["workspace"] == {"enabled": True} + + +def test_update_disabling_workspace_drops_the_key(tmp_path: Path) -> None: + """`--no-workspace` must omit the key, not write ``enabled: false``. + + ``create --no-workspace`` produces a body with no ``runtime.workspace`` + at all; writing ``false`` here would make the two paths diverge. + """ + configuration = _base_configuration() + configuration["runtime"]["workspace"] = {"enabled": True} + service, _ds, storage = _make_service(_make_store(tmp_path), configuration) + + result = service.update_data_app(alias="prod", app_id="74021026", workspace=False) + + assert result["changed"] == ["workspace"] + assert "workspace" not in _put_body(storage)["runtime"] + + +def test_update_preserves_every_untouched_key(tmp_path: Path) -> None: + service, _ds, storage = _make_service(_make_store(tmp_path)) + service.update_data_app(alias="prod", app_id="74021026", workspace=True) + + body = _put_body(storage) + original = _base_configuration() + assert body["storage"] == original["storage"] + assert body["authorization"] == original["authorization"] + assert body["parameters"]["dataApp"] == original["parameters"]["dataApp"] + assert body["parameters"]["id"] == "74021026" + assert body["runtime"]["backend"] == {"size": "tiny"} + + +# --------------------------------------------------------------------------- +# The other updatable fields +# --------------------------------------------------------------------------- + + +def test_update_auto_suspend_and_size(tmp_path: Path) -> None: + service, _ds, storage = _make_service(_make_store(tmp_path)) + result = service.update_data_app( + alias="prod", app_id="74021026", auto_suspend_after_seconds=300, size="small" + ) + + assert result["changed"] == ["auto_suspend_after_seconds", "size"] + body = _put_body(storage) + assert body["parameters"]["autoSuspendAfterSeconds"] == 300 + assert body["runtime"]["backend"]["size"] == "small" + + +def test_update_auth_switches_the_proxy_block(tmp_path: Path) -> None: + service, _ds, storage = _make_service(_make_store(tmp_path)) + result = service.update_data_app(alias="prod", app_id="74021026", auth="public") + + assert result["changed"] == ["auth"] + assert result["changes"][0]["before"] == "password" + rules = _put_body(storage)["authorization"]["app_proxy"]["auth_rules"] + assert rules[0]["auth_required"] is False + assert "auth" not in rules[0] + + +def test_update_git_branch(tmp_path: Path) -> None: + service, _ds, storage = _make_service(_make_store(tmp_path)) + service.update_data_app(alias="prod", app_id="74021026", git_branch="release") + + git = _put_body(storage)["parameters"]["dataApp"]["git"] + assert git["branch"] == "release" + # The encrypted PAT must survive the rewrite untouched. + assert git["#password"] == "KBC::ProjectSecureGKMS::ciphertext" + + +def test_update_git_branch_rejected_without_a_git_block(tmp_path: Path) -> None: + configuration = _base_configuration() + del configuration["parameters"]["dataApp"]["git"] + service, _ds, storage = _make_service(_make_store(tmp_path), configuration) + + with pytest.raises(KeboolaApiError) as exc: + service.update_data_app(alias="prod", app_id="74021026", git_branch="release") + assert exc.value.error_code == ErrorCode.DATA_APP_INVALID_GIT + storage.update_config.assert_not_called() + + +# --------------------------------------------------------------------------- +# No-ops, dry runs, validation +# --------------------------------------------------------------------------- + + +def test_update_with_matching_values_writes_nothing(tmp_path: Path) -> None: + service, _ds, storage = _make_service(_make_store(tmp_path)) + result = service.update_data_app( + alias="prod", app_id="74021026", auto_suspend_after_seconds=900, size="tiny" + ) + + assert result["changed"] == [] + assert result["deploy_required"] is False + assert result["config_version_after"] == "7" + storage.update_config.assert_not_called() + + +def test_update_dry_run_makes_no_put(tmp_path: Path) -> None: + service, _ds, storage = _make_service(_make_store(tmp_path)) + result = service.update_data_app(alias="prod", app_id="74021026", workspace=True, dry_run=True) + + assert result["dry_run"] is True + preview = result["put_storage_config_preview"]["configuration"] + assert preview["runtime"]["workspace"] == {"enabled": True} + storage.update_config.assert_not_called() + + +def test_update_dry_run_redacts_the_encrypted_pat(tmp_path: Path) -> None: + service, _ds, _storage = _make_service(_make_store(tmp_path)) + result = service.update_data_app(alias="prod", app_id="74021026", workspace=True, dry_run=True) + + preview = result["put_storage_config_preview"]["configuration"] + assert preview["parameters"]["dataApp"]["git"]["#password"] == "" + + +def test_update_without_any_field_is_a_usage_error(tmp_path: Path) -> None: + service, _ds, storage = _make_service(_make_store(tmp_path)) + with pytest.raises(KeboolaApiError) as exc: + service.update_data_app(alias="prod", app_id="74021026") + assert exc.value.error_code == ErrorCode.MISSING_PARAMETER + storage.get_config_detail.assert_not_called() + + +@pytest.mark.parametrize( + ("kwargs", "code"), + [ + ({"size": "enormous"}, ErrorCode.VALIDATION_ERROR), + ({"auth": "oauth"}, ErrorCode.VALIDATION_ERROR), + ({"auto_suspend_after_seconds": -1}, ErrorCode.VALIDATION_ERROR), + ({"git_branch": "bad\nbranch"}, ErrorCode.VALIDATION_ERROR), + ], +) +def test_update_rejects_invalid_values( + tmp_path: Path, kwargs: dict[str, Any], code: ErrorCode +) -> None: + service, _ds, storage = _make_service(_make_store(tmp_path)) + with pytest.raises(KeboolaApiError) as exc: + service.update_data_app(alias="prod", app_id="74021026", **kwargs) + assert exc.value.error_code == code + storage.get_config_detail.assert_not_called() + + +# --------------------------------------------------------------------------- +# detail surfaces the Storage-access state (the diagnosis half of #737) +# --------------------------------------------------------------------------- + + +def test_detail_reports_workspace_disabled_when_the_key_is_absent(tmp_path: Path) -> None: + service, _ds, _storage = _make_service(_make_store(tmp_path)) + assert service.get_data_app(alias="prod", app_id="74021026")["workspace_enabled"] is False + + +def test_detail_reports_workspace_enabled(tmp_path: Path) -> None: + configuration = _base_configuration() + configuration["runtime"]["workspace"] = {"enabled": True} + service, _ds, _storage = _make_service(_make_store(tmp_path), configuration) + assert service.get_data_app(alias="prod", app_id="74021026")["workspace_enabled"] is True diff --git a/tests/test_e2e.py b/tests/test_e2e.py index a9d7be8a8..739718c70 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -9630,6 +9630,114 @@ def test_data_app_no_workspace_omits_the_block(self) -> None: assert body["workspace"] is True assert body["requests"]["put_storage_config"]["runtime"]["workspace"] == {"enabled": True} + @skip_without_data_app_public + def test_data_app_update_toggles_storage_access(self) -> None: + """`data-app update` flips runtime.workspace.enabled on a LIVE app (#737). + + Proves the three contract points a mocked unit test cannot: the flag + actually reaches Storage, `detail` reads it back, and repeating the + same request writes nothing (no config version minted). + """ + _step(1, "Create an app with Storage access ON") + repo = os.environ[ENV_DATA_APP_GIT_REPO_PUBLIC] + body = _json_ok( + _invoke( + self.config_dir, + [ + "--json", + "data-app", + "create", + "--project", + self.alias, + "--name", + f"E2E Update {RUN_ID}", + "--slug", + f"e2e-upd-{RUN_ID}"[:60], + "--git-repo", + repo, + "--git-public", + "--auth", + "public", + "--no-deploy", # avoid waiting on a real container build + ], + ) + )["data"] + app_id = body["app_id"] + self._created_app_ids.append(app_id) + + def _detail() -> dict: + return _json_ok( + _invoke( + self.config_dir, + ["--json", "data-app", "detail", "--project", self.alias, "--app-id", app_id], + ) + )["data"] + + assert _detail()["workspace_enabled"] is True + + _step(2, "Turn Storage access OFF") + off = _json_ok( + _invoke( + self.config_dir, + [ + "--json", + "data-app", + "update", + "--project", + self.alias, + "--app-id", + app_id, + "--no-workspace", + ], + ) + )["data"] + assert off["changed"] == ["workspace"], off + assert off["deploy_required"] is True + assert _detail()["workspace_enabled"] is False + + _step(3, "Repeating the same request is a no-op -- no config version minted") + repeat = _json_ok( + _invoke( + self.config_dir, + [ + "--json", + "data-app", + "update", + "--project", + self.alias, + "--app-id", + app_id, + "--no-workspace", + ], + ) + )["data"] + assert repeat["changed"] == [] + assert repeat["deploy_required"] is False + assert repeat["config_version_after"] == repeat["config_version_before"] + + _step(4, "Turn it back on together with a new auto-suspend") + on = _json_ok( + _invoke( + self.config_dir, + [ + "--json", + "data-app", + "update", + "--project", + self.alias, + "--app-id", + app_id, + "--workspace", + "--auto-suspend", + "600", + ], + ) + )["data"] + assert set(on["changed"]) == {"workspace", "auto_suspend_after_seconds"}, on + detail = _detail() + assert detail["workspace_enabled"] is True + assert str(detail["auto_suspend_after_seconds"]) == "600", detail + @skip_without_data_app_public def test_data_app_git_repo_introspection(self) -> None: """git-repo against a deployed public app.