diff --git a/.gitignore b/.gitignore index 8e18444d..c4b6be52 100644 --- a/.gitignore +++ b/.gitignore @@ -17,7 +17,7 @@ ENV/ # Environment variables .env -.env.local +.env.* # IDE .idea/ diff --git a/CLAUDE.md b/CLAUDE.md index b74c591a..83f3c263 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -250,6 +250,9 @@ kbagent config detail --project NAME --component-id ID --config-id ID [--branch kbagent config search --query PATTERN [--project NAME] [--component-type TYPE] [--ignore-case] [--regex] [--branch ID] kbagent config update --project NAME --component-id ID --config-id ID [--name N] [--description D] [--configuration JSON|@file|-] [--configuration-file PATH] [--set PATH=VALUE ...] [--merge] [--dry-run] [--branch ID] kbagent config rename --project NAME --component-id ID --config-id ID --name "New Name" [--branch ID] [--directory DIR] +kbagent config variables-set --project NAME --component-id ID --config-id ID --var KEY=VALUE [--var ...] [--replace] [--variables-id ID] [--values-id ID] [--branch ID] [--dry-run] +kbagent config variables-get --project NAME --component-id ID --config-id ID [--branch ID] +kbagent config variables-clear --project NAME --component-id ID --config-id ID [--branch ID] [--yes] kbagent job list [--project NAME] [--component-id ID] [--status STATUS] [--limit N] kbagent job detail --project NAME --job-id ID diff --git a/plugins/kbagent/.claude-plugin/plugin.json b/plugins/kbagent/.claude-plugin/plugin.json index 61b01f73..0d88b34a 100644 --- a/plugins/kbagent/.claude-plugin/plugin.json +++ b/plugins/kbagent/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "kbagent", - "version": "0.20.6", + "version": "0.21.0", "description": "AI-friendly interface to Keboola Connection projects — explore configs, jobs, lineage, call MCP tools, manage dev branches, and debug SQL in workspaces", "author": { "name": "Keboola", diff --git a/plugins/kbagent/skills/kbagent/SKILL.md b/plugins/kbagent/skills/kbagent/SKILL.md index e1b39c36..3050395d 100644 --- a/plugins/kbagent/skills/kbagent/SKILL.md +++ b/plugins/kbagent/skills/kbagent/SKILL.md @@ -93,6 +93,9 @@ When working inside a git repository or project directory, run `kbagent init` (o | Rename a configuration (update name via API + rename local sync directory) | `kbagent config rename --project PROJECT --component-id COMPONENT-ID --config-id CONFIG-ID --name NAME` | | Delete a configuration from a project | `kbagent config delete --project PROJECT --component-id COMPONENT-ID --config-id CONFIG-ID` | | Generate boilerplate configuration files for a Keboola component | `kbagent config new --component-id COMPONENT-ID` | +| Assign variables to a config (auto-creates backing keboola.variables on first call) | `kbagent config variables-set --project PROJECT --component-id COMPONENT-ID --config-id CONFIG-ID` | +| Read the current variable values attached to a config | `kbagent config variables-get --project PROJECT --component-id COMPONENT-ID --config-id CONFIG-ID` | +| Unlink variables from a config (does NOT delete the underlying keboola.variables) | `kbagent config variables-clear --project PROJECT --component-id COMPONENT-ID --config-id CONFIG-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` | @@ -204,6 +207,8 @@ For detailed response parsing rules and common pitfalls, see [gotchas](reference | Dev branches | [branch-workflow](references/branch-workflow.md) | | Encrypting secrets for MCP tools | [encrypt-workflow](references/encrypt-workflow.md) | | Sync & Git-branching (GitOps) | [sync-workflow](references/sync-workflow.md) | +| Sync row-level internals (manifest v3, hoist, encryption) | [sync-rows-workflow](references/sync-rows-workflow.md) | +| **Variables (attach to any config)** | [variables-workflow](references/variables-workflow.md) | | Reading synced data | [reading-synced-data](references/reading-synced-data.md) | | SQL migration (input mapping removal) | [sql-migration-workflow](references/sql-migration-workflow.md) | | Response parsing gotchas | [gotchas](references/gotchas.md) | diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index ef6fd86a..1825d808 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -35,6 +35,9 @@ All commands support `--json` for structured output. Multi-project flags (`--pro - `config rename --project NAME --component-id ID --config-id ID --name "New Name" [--branch ID] [--directory DIR]` -- rename a configuration (API update + local sync directory rename with git mv support) - `config delete --project NAME --component-id ID --config-id ID [--branch ID]` -- delete a configuration - `config new --component-id ID [--project NAME] [--name NAME] [--output-dir DIR]` -- scaffold new config from component schema +- `config variables-set --project NAME --component-id ID --config-id ID --var KEY=VALUE [--var ...] [--replace] [--variables-id ID] [--values-id ID] [--branch ID] [--dry-run] [--allow-plaintext-on-encrypt-failure] [--yes]` -- attach variable values to a config. Auto-creates a sibling `keboola.variables` config + default row on first use and links it via the parent's `runtime.variables_id` / `variables_values_id`. Defaults to merge; `--replace` drops keys not in `--var`. `#`-prefixed values encrypt via the Encryption API (fail-closed; exit non-zero on `ENCRYPTION_FAILED`). See `variables-workflow.md` +- `config variables-get --project NAME --component-id ID --config-id ID [--branch ID]` -- resolve `variables_id` + `values_id` from the parent config and fetch the current KEY=VALUE map. Returns `{linked: bool, variables_id, values_id, values}`; `linked=false` means the parent has no variables attached +- `config variables-clear --project NAME --component-id ID --config-id ID [--branch ID] [--yes]` -- unlink variables from the parent config (strips `variables_id` + `variables_values_id`). **Does NOT delete** the backing `keboola.variables` config -- use `config delete` explicitly if you've verified nothing else references it ## Job History - `job list [--project NAME] [--component-id ID] [--config-id ID] [--status STATUS] [--limit N]` -- list jobs (default 50, max 500) diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index 3984d7ae..7e89a169 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -1,5 +1,55 @@ # Gotchas -- Response Parsing and Common Pitfalls +## Variables: attach, don't manage (since 0.21.0) + +- `keboola.variables` is an implementation detail. Use + `kbagent config variables-set/get/clear` -- you never need to create, + list, or link variables configs manually. +- First `variables-set` auto-creates a sibling `keboola.variables` config + named `-vars` and links the parent. Subsequent sets update + the same default row. +- **`variables-clear` does NOT delete the backing variables config** -- it + may be shared across multiple configs. To actually remove it, run + `kbagent config delete --component-id keboola.variables --config-id ` + after verifying nothing else references it. +- `--var #KEY=plain` -> encrypted via Encryption API before reaching Storage. + Fail-closed: encryption failure aborts with `ENCRYPTION_FAILED`. Use + `--allow-plaintext-on-encrypt-failure` only for bootstrap/debug. +- `--replace` drops any existing keys not in the current `--var` set. + Default is merge. +- Full workflow + response shapes: see + [variables-workflow.md](variables-workflow.md). + +## Sync: row deploy & manifest v3 (since 0.21.0) + +- `sync push` **does** deploy config rows now (previously silently skipped). + Row changes in the `pushed_details` array carry `"is_row": true` and + `"parent_config_id": "..."` so you can distinguish them from parent config ops. +- For `keboola.variables` and `keboola.shared-code` rows, the row's + `configuration` keys are **hoisted** to the top level of the local YAML + (`values:`, `code_content:`, etc.) -- NOT wrapped under + `_configuration_extra`. Edit them directly at the top level. +- `.keboola/manifest.json` auto-upgrades from v2 to v3 on the next successful + pull or push. v3 adds `rows[].metadata` with per-row pull hashes. v2 + manifests still load cleanly; a downgrade to an older kbagent still reads + the file via `extra="allow"`. +- Encryption failure on a row push raises `ENCRYPTION_FAILED` from the + service. If it escapes the per-change handler it maps to CLI exit 1 + (general); if caught per-change it lands in `result["errors"][]` with the + same code. Fail-closed either way. Use + `--allow-plaintext-on-encrypt-failure` ONLY for debugging. +- **`keboola.variables` row secrets live in `{name, value}` list + elements**, not dict keys. Before 0.21.1, the encryption walker only + scanned `#`-prefixed dict keys and silently shipped plaintext for + `values: [{name: '#x', value: '...'}]`. Fixed in 0.21.1 via + `_is_secret_name_value_pair`. (`keboola.shared-code` rows carry + `code_content: [string]` and have no secrets, so the walker correctly + never fires there.) If you add a new row-hoist component with yet + another secret shape, extend the walker -- don't patch callers. +- Row-level deployment internals (manifest v3 hashes, 3-way diff, untracked + row detection, `ROW_HOIST_COMPONENTS`): see + [`sync-rows-workflow.md`](sync-rows-workflow.md). + ## Response structure varies by command Not all commands return data the same way. Key differences: diff --git a/plugins/kbagent/skills/kbagent/references/sync-rows-workflow.md b/plugins/kbagent/skills/kbagent/references/sync-rows-workflow.md new file mode 100644 index 00000000..772798f7 --- /dev/null +++ b/plugins/kbagent/skills/kbagent/references/sync-rows-workflow.md @@ -0,0 +1,303 @@ +# Sync Rows Workflow -- Row-level GitOps internals + +`sync push` deploys config rows end-to-end starting from v0.21.0. This document +is the technical reference for agents and developers who need to understand +how row tracking, hashing, and encryption work beyond the TL;DR in +[`sync-workflow.md`](./sync-workflow.md). + +If you just want to assign variable values without authoring YAML, use +`kbagent config variables-{set,get,clear}` -- see +[`variables-workflow.md`](./variables-workflow.md). + +## When to use rows vs variables-set + +Both code paths converge on the same API calls. Pick based on source-of-truth: + +| Scenario | Prefer | Why | +|---|---|---| +| Config is git-tracked, PRs review changes | `sync push` | YAML diff is the review artifact | +| Agent-driven "set this key on that config" | `variables-set` | One CLI call, no filesystem | +| Shared `keboola.variables` across many configs | `variables-set --variables-id ` | No accidental duplication | +| Bulk changes to `keboola.shared-code` snippets | `sync push` | Variables commands don't touch shared-code | +| Bootstrapping a new project | `sync push` | Pull once, diff, push -- rows land in manifest | + +`keboola.variables` and `keboola.shared-code` are the only components that +get row-hoisted payloads (see `ROW_HOIST_COMPONENTS`). All other row-bearing +components (transformations, orchestrators, etc.) round-trip via the standard +`parameters` / `storage` / `processors` shape. + +## Manifest v3 + +Pull-time state is recorded in `.keboola/manifest.json`: + +```json +{ + "version": 3, + "configurations": [ + { + "componentId": "keboola.snowflake-transformation", + "id": "15815157", + "path": "main/transformation/snowflake/my-transform", + "metadata": {"pull_hash": "...", "pull_config_hash": "..."}, + "rows": [ + { + "id": "789123", + "path": "rows/main", + "metadata": {"pull_hash": "...", "pull_config_hash": "..."} + } + ] + } + ] +} +``` + +- **`version: 3`** -- bumped from v2 when rows landed. Older manifests load + via `extra="allow"` (backward compatible); row `metadata` is seeded on the + next pull. +- **`rows[].metadata.pull_hash`** -- SHA of the raw `_config.yml` bytes at + pull time. Used to detect **local edits** (current file hash != stored). +- **`rows[].metadata.pull_config_hash`** -- stable hash of the semantic + `{name, description, configuration}` shape. Used to detect **remote drift** + without raising false positives on cosmetic changes (comment removal, + key reordering, encrypted-value nonce changes). + +The dual-hash design mirrors parent-config metadata. One hash owns +"did local change?", the other owns "did remote change?". Together they +produce the 3-way diff matrix without needing a separate `base/` snapshot +directory. + +### Hash invariants + +- A fresh pull writes both hashes; the next `sync diff` shows zero changes. +- Editing the YAML but not the semantic payload (e.g. fixing indentation) + flips `pull_hash` but leaves `pull_config_hash` equal -- diff reports the + file as unchanged. +- Re-encrypting the same secret produces different ciphertext (fresh nonce). + `config_hash` strips `KBC::ProjectSecure::` prefixes before hashing, so + this is a no-op for diff purposes. +- Windows `\r\n` line endings: `_write_config_file` opens files with + `newline=""` to pin LF output. Mixing shells on Windows won't churn hashes. + +## Row-hoisted components + +For components in `ROW_HOIST_COMPONENTS = {"keboola.variables", "keboola.shared-code"}`, +the API's `configuration` object is atypical: + +```python +# keboola.variables row API shape: +{"configuration": {"values": [{"name": "region", "value": "eu", "type": "string"}]}} + +# keboola.shared-code row API shape: +{"configuration": {"code_content": ["SELECT * FROM t WHERE region = {{ region }}"]}} +``` + +Neither fits the standard `parameters` / `storage` / `processors` promotion. +So the local YAML hoists these top-level keys: + +```yaml +# rows/main/_config.yml for keboola.variables +version: 2 +name: main +description: "" +values: + - {name: region, value: eu, type: string} +_keboola: + component_id: keboola.variables + row_id: "789123" +``` + +On push, `local_row_to_api` inverts the hoist: reserved keys (`version`, `name`, +`description`, `_keboola`, etc.) stay local, everything else is pulled into +`configuration`. The byte-for-byte round-trip invariant is locked down by +`TestVariablesRowRoundTrip` (5 tests). + +**If you add another row-bearing component with a weird shape**, add it to +`ROW_HOIST_COMPONENTS` in `src/keboola_agent_cli/sync/config_format.py`. The +round-trip hoist works automatically; whether the encryption walker covers +the secret shape depends on the payload (see next section). + +## Secret encryption in rows + +Parent configs put secrets under `#`-prefixed **dict keys**: + +```yaml +# Parent config +parameters: + "#api_token": raw-plaintext +``` + +**`keboola.variables` rows** store secrets as `{name, value}` list elements +where the `#` lives in the **`name` field**, not as a dict key: + +```yaml +# keboola.variables row +values: + - {name: "#api_key", value: raw-plaintext} +``` + +**`keboola.shared-code` rows do NOT carry secrets.** The payload is +`code_content: [string, string, ...]`, a list of SQL/Python snippets. The +encryption walker simply never fires for shared-code rows, which is correct +and intentional. If you're debugging encryption and expect a shared-code +row to encrypt something, you're confused about the shape. + +Both shapes that DO carry secrets (parent-config dict keys, and +`keboola.variables` `{name, value}` pairs) encrypt to +`KBC::ProjectSecure::...` before hitting Storage. + +The encryption walker in `services/_encryption.py` detects the +`{name, value}` shape via `_is_secret_name_value_pair()`: + +```python +def _is_secret_name_value_pair(item): + return ( + isinstance(item, dict) + and isinstance(item.get("name"), str) + and is_secret_key(item["name"]) + and isinstance(item.get("value"), str) + ) +``` + +`collect_secrets`, `apply_encrypted`, and `apply_encrypted_to_local` all +detect this shape. The emitted encryption key for such entries is +`#[].` (e.g. `#values.[0].#api_key`), which the +Encryption API accepts like any other flat secret map. + +**Fail-closed**: on encryption API failure, push aborts with `ENCRYPTION_FAILED` +and exit non-zero. Plaintext never lands on Storage. `--allow-plaintext-on-encrypt-failure` +exists as a bootstrap escape hatch; **do not use in production**. + +## Untracked row detection + +You can drop a hand-crafted row directory under a tracked config, and push +will POST it: + +```bash +# Structure after pull: +main/transformation/snowflake/my-transform/ +├── _config.yml +└── rows/ + ├── main/ # tracked (in manifest) + │ └── _config.yml + └── new-row/ # NEW -- not in manifest + └── _config.yml + +# Push picks it up: +kbagent sync push --project prod +# → POST /components/keboola.snowflake-transformation/configs/15815157/rows +# → new row gets an API-assigned id, written back into the manifest +``` + +`_find_untracked_rows` in `services/sync_service.py` (paralleling +`_find_untracked_configs`) walks each tracked config's `rows/` dir, finds +subdirs with a `_config.yml` that aren't in `manifest.configurations[].rows`, +and surfaces them as `diff` state `"added"`. Push routes them through +`_push_create_row`, which: + +1. Loads the YAML, runs `local_row_to_api` to split out `name` / + `description` / `configuration`. +2. Encrypts `#`-prefixed values (both parent-config and row-hoisted shapes). +3. POSTs to `/components/{component_id}/configs/{config_id}/rows`. +4. Writes the API-assigned row id back into the manifest along with fresh + `pull_hash` + `pull_config_hash`. +5. Writes encrypted ciphertext back into the local YAML so the next diff + sees `local == remote`. + +The `row_id` landing in `_keboola` in the local YAML matches the API response; +no collision with existing rows because the API assigns it. Test coverage: +`test_push_untracked_row_dir_calls_create_config_row`. + +## 3-way diff at the row level + +Row diff is the same 3-way engine as parent configs, just keyed by row id: + +| `pull_hash == current_local_hash`? | `pull_config_hash == remote_config_hash`? | Diff state | +|---|---|---| +| yes | yes | `unchanged` | +| no | yes | `modified` (push will update) | +| yes | no | `remote_modified` (pull to refresh) | +| no | no | `conflict` (manual resolve) | + +Added rows (filesystem-only) show as `added`; removed rows (manifest-only +after file deletion) show as `deleted` -- push DELETEs them via +`_push_delete_row`. + +Human-mode diff output prints row-level changes with the same `+`/`~`/`-`/`=` +prefixes as parent configs, e.g.: + +``` +keboola.snowflake-transformation/15815157 + ~ rows/main: values[0].value changed: '2024' -> '2025' + + rows/new-row: added (will POST on push) +``` + +## Tests you should know about + +Every row-level behavior has a named test; if you're extending this area, +add/update alongside: + +| Behavior | Test | +|---|---| +| Row create/update/delete client methods | `tests/test_client.py::TestConfigRowMethods` (9 tests) | +| Sync push row create + hash bookkeeping | `tests/test_sync_service.py::TestPushRows` (5 tests) | +| Encryption fail-closed on push | `TestPushRows::test_push_encryption_failure_aborts_fail_closed` | +| Row-hoisted secret encryption (regression) | `TestPushRows::test_push_update_row_encrypts_variables_row` | +| Untracked row directory detection | `test_push_untracked_row_dir_calls_create_config_row` | +| YAML round-trip for hoisted payloads | `TestVariablesRowRoundTrip` (5 tests) | +| Manifest v2 -> v3 backward-compat load | `tests/test_sync_manifest.py` | +| CLI-level push rows (human + JSON) | `tests/test_sync_cli.py::TestSyncPushCli` | +| Encryption walker row-hoist unit tests | `tests/test_encryption.py` | +| E2E round-trip vs live Keboola project | `test_e2e.py::TestE2ESyncWorkflow::test_sync_push_variable_row_round_trip` | + +## Response shapes (for `--json` agents) + +### `sync push` with row changes + +```json +{ + "status": "ok", + "data": { + "project_alias": "prod", + "branch_id": null, + "pushed": [ + { + "component_id": "keboola.variables", + "config_id": "15815157", + "action": "updated" + } + ], + "pushed_rows": [ + { + "component_id": "keboola.variables", + "parent_config_id": "15815157", + "row_id": "789123", + "path": "rows/main", + "action": "updated" + }, + { + "component_id": "keboola.variables", + "parent_config_id": "15815157", + "row_id": "789456", + "path": "rows/new-row", + "action": "created" + } + ], + "skipped": [], + "errors": [] + } +} +``` + +`action` is one of `created`, `updated`, `deleted`. On `ENCRYPTION_FAILED` or +API errors, the offending row lands in `errors[]` with the same shape and +an `error_code` / `message` pair; successful siblings still make it to +`pushed_rows[]` because push processes rows independently. + +## Related reading + +- [`sync-workflow.md`](./sync-workflow.md) -- general `sync pull` / `sync push` + flow, branch mapping, 3-way diff for parent configs. +- [`variables-workflow.md`](./variables-workflow.md) -- ergonomic alternative + to row YAML editing. +- [`gotchas.md`](./gotchas.md) -- quick-reference footguns, including the + row-level encryption caveat. diff --git a/plugins/kbagent/skills/kbagent/references/sync-workflow.md b/plugins/kbagent/skills/kbagent/references/sync-workflow.md index 4fc88d2f..6616ee90 100644 --- a/plugins/kbagent/skills/kbagent/references/sync-workflow.md +++ b/plugins/kbagent/skills/kbagent/references/sync-workflow.md @@ -2,6 +2,46 @@ Sync lets you manage Keboola configurations as local files with full git integration. +## Variable values deployment (since v0.21.0) + +`sync push` now deploys **config rows**, not just parent configs. This unlocks the +most common GitOps use case: deploying `keboola.variables` values through git. + +```bash +# 1. Pull a project that contains a keboola.variables config + values row. +kbagent sync pull --project prod + +# 2. Edit the row YAML directly. For keboola.variables and keboola.shared-code +# rows, the configuration keys (`values`, `code_content`, etc.) are hoisted +# to the top level of the YAML so edits are natural. +cat > main/variables//values/main/_config.yml < +EOF + +# 3. Push -- the row is written to +# PUT /v2/storage/components/keboola.variables/configs/{id}/rows/{rowId} +# with the configuration set to {"values": [...]} verbatim. +kbagent sync push --project prod +``` + +`#`-prefixed secret keys inside row YAMLs are encrypted via the Encryption API +before push, same as parent configs. Encryption failure aborts the push +(fail-closed); use `--allow-plaintext-on-encrypt-failure` only for debugging. + +For the row-level internals (manifest v3, per-row hashes, hoisted payloads, +untracked row detection, secret encryption contract), see +[`sync-rows-workflow.md`](./sync-rows-workflow.md). For the ergonomic +alternative that skips the YAML round-trip entirely, see +[`variables-workflow.md`](./variables-workflow.md). + ## All-projects workflow (recommended) ```bash diff --git a/plugins/kbagent/skills/kbagent/references/variables-workflow.md b/plugins/kbagent/skills/kbagent/references/variables-workflow.md new file mode 100644 index 00000000..573dc8d5 --- /dev/null +++ b/plugins/kbagent/skills/kbagent/references/variables-workflow.md @@ -0,0 +1,186 @@ +# Variables Workflow -- First-class attachment, not a separate resource + +Variables in Keboola are stored server-side as `keboola.variables` configurations +with rows. **That's an implementation detail.** From an agent or user perspective, +variables are just a flat KEY=VALUE dict you assign to a config. kbagent hides +the config-as-resource / row-link / schema-sync plumbing behind three commands. + +## TL;DR + +```bash +# Assign (auto-creates the backing keboola.variables config on first call). +kbagent config variables-set --project prod \ + --component-id keboola.snowflake-transformation --config-id 15815157 \ + --var year_start=2016 --var region=eu + +# Read +kbagent --json config variables-get --project prod \ + --component-id keboola.snowflake-transformation --config-id 15815157 + +# Change a single value (merges; other keys preserved) +kbagent config variables-set --project prod \ + --component-id keboola.snowflake-transformation --config-id 15815157 \ + --var region=us-west + +# Replace ALL values (drops keys not in --var) +kbagent config variables-set --project prod \ + --component-id keboola.snowflake-transformation --config-id 15815157 \ + --var only_key=only_value --replace + +# Detach (does NOT delete the backing keboola.variables config) +kbagent config variables-clear --project prod \ + --component-id keboola.snowflake-transformation --config-id 15815157 --yes +``` + +## What the CLI does behind the scenes + +### First `variables-set` on an unlinked parent (auto-create) + +1. GET the parent config → no `variables_id` set. +2. POST `/components/keboola.variables/configs` → create sibling named + `-vars` with schema derived from the `--var` keys. +3. POST `/components/keboola.variables/configs/{id}/rows` → default row named + `default` with the values payload (encrypted where `#`-prefixed). +4. PUT parent config → add `variables_id` + `variables_values_id`. + +Result: one CLI call, four API calls, hidden from the caller. + +### Subsequent `variables-set` (update existing) + +1. GET parent → `variables_id` + `variables_values_id` already set. +2. GET variables config → resolve the target row. +3. Merge (default) or replace (`--replace`) the values. +4. Encrypt any new `#`-prefixed values via the Encryption API. +5. PUT the row. +6. If new keys were introduced, extend the variables config's schema + (cosmetic; UI-only, non-fatal if it fails). + +### `variables-clear` + +Strips both `variables_id` and `variables_values_id` from the parent config. +**Does not delete the backing `keboola.variables` config** -- it may be shared +with other configs, and deletion is a distinct destructive op. Use +`kbagent config delete` explicitly to remove the backing config after verifying +nothing else references it. + +## Secrets + +Prefix the key with `#` to mark a value as a secret: + +```bash +kbagent config variables-set --project prod \ + --component-id keboola.python-transformation-v2 --config-id 42 \ + --var '#api_token=raw-plaintext-here' +``` + +Behavior: + +- kbagent sends the plaintext to the Encryption API, receives `KBC::ComponentSecure::...` + back, and writes only the ciphertext into the row's `values` array. +- **Fail-closed**: if the Encryption API is unreachable, the command aborts + with `ENCRYPTION_FAILED` (exit non-zero). The plaintext never lands on Storage. +- Escape hatch: `--allow-plaintext-on-encrypt-failure` falls back to plaintext + (**not recommended**; only for bootstrap/debugging scenarios). + +## Attaching to an existing variables config + +By default, `variables-set` auto-creates a sibling config. To attach the parent +to an **existing** `keboola.variables` config (e.g. shared across multiple +transformations in a pipeline): + +```bash +kbagent config variables-set --project prod \ + --component-id keboola.snowflake-transformation --config-id 15815157 \ + --variables-id 01kpn7sak9kwkn61h7j1y4pz3z \ + --var region=eu +``` + +Optionally pin a specific values row with `--values-id ROW_ID` (defaults to the +first row, which is the Keboola convention for the "default" row). + +## Dry-run preview + +`--dry-run` shows what would change without touching the server. The output is +a diff of current vs. proposed values: + +```bash +kbagent config variables-set --project prod \ + --component-id keboola.snowflake-transformation --config-id 15815157 \ + --var region=us-west --dry-run +``` + +Human mode prints `+ new_key`, `~ changed_key: old -> new`, `- dropped_key`, +`= unchanged_key`. JSON mode returns `{"dry_run": true, "current_values": ..., +"would_write": ..., "action": "would_create"|"would_update"}`. + +## Response shapes (for `--json` agents) + +### `variables-set` +```json +{ + "status": "ok", + "data": { + "project_alias": "prod", + "parent_component_id": "keboola.snowflake-transformation", + "parent_config_id": "15815157", + "variables_id": "01kpn7sak9kwkn61h7j1y4pz3z", + "values_id": "01kpn7sat48jmhvx20svaqdnf9", + "action": "created", + "values": {"year_start": "2016", "region": "eu"}, + "encrypted_keys": ["#api_token"] + } +} +``` + +### `variables-get` +```json +{ + "status": "ok", + "data": { + "project_alias": "prod", + "parent_component_id": "keboola.snowflake-transformation", + "parent_config_id": "15815157", + "variables_id": "01kpn7sak9kwkn61h7j1y4pz3z", + "values_id": "01kpn7sat48jmhvx20svaqdnf9", + "values": {"year_start": "2016", "region": "eu"}, + "linked": true + } +} +``` + +`linked=false` means the parent has no `variables_id` -- the other fields will +be `null` and `values` will be `{}`. + +### `variables-clear` +```json +{ + "status": "ok", + "data": { + "project_alias": "prod", + "parent_component_id": "keboola.snowflake-transformation", + "parent_config_id": "15815157", + "was_linked": true, + "unlinked_variables_id": "01kpn7sak9kwkn61h7j1y4pz3z", + "unlinked_values_id": "01kpn7sat48jmhvx20svaqdnf9" + } +} +``` + +## Relation to `sync push` + +`sync push` deploys `keboola.variables` configs + rows as regular configs via +the GitOps flow (edit YAML -> push). `variables-{set,get,clear}` is the +ergonomic alternative for when you don't want to author YAML: one-line +deployments, no manifest bookkeeping. + +Both are supported; pick whichever fits your workflow: + +| Use case | Prefer | +|---|---| +| Git-tracked config as source of truth | `sync push` | +| Quick "set a value on this transformation" | `variables-set` | +| Same variable definitions across many transformations | `variables-set --variables-id ` | +| Agent-driven pipelines with programmatic var changes | `variables-set` | + +They converge on the same API calls; `sync push` just routes through local YAML +first. diff --git a/pyproject.toml b/pyproject.toml index c8d6340e..de3fe081 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "keboola-agent-cli" -version = "0.20.6" +version = "0.21.0" description = "AI-friendly CLI for managing Keboola projects" readme = "README.md" requires-python = ">=3.12" diff --git a/src/keboola_agent_cli/changelog.py b/src/keboola_agent_cli/changelog.py index ad66ff10..57b748f0 100644 --- a/src/keboola_agent_cli/changelog.py +++ b/src/keboola_agent_cli/changelog.py @@ -8,6 +8,15 @@ # Ordered newest-first. Each value is a list of brief one-line descriptions. CHANGELOG: dict[str, list[str]] = { + "0.21.0": [ + "New: config variables-set / variables-get / variables-clear -- variables as a first-class attachment, not a resource to manage. Auto-creates the backing keboola.variables config + default row on first set, merges or replaces on update, encrypts #-prefixed values fail-closed, unlinks without deleting the backing config.", + "New: sync push now deploys config rows (create/update/delete via /rows endpoints) -- previously rows edited locally were silently skipped (FIIA P0-1)", + "New: #-prefixed secret values in row YAMLs are encrypted via the Encryption API before push, same fail-closed semantics as parent configs (FIIA P1-5)", + "New: keboola.variables / keboola.shared-code row YAMLs hoist 'values' / 'code_content' to top level (matches kbc push convention) instead of hiding under _configuration_extra", + "New: per-row 3-way diff -- sync status/diff now reports added/modified/deleted rows alongside parent configs; local row edits are preserved across pull", + "New: ManifestConfigRow.metadata with pull_hash + pull_config_hash -- manifest schema bumped to v3 (v2 manifests load cleanly and upgrade in-place on next pull)", + "Fix: _write_config_file now uses newline='' so Windows doesn't translate LF->CRLF on write, which previously caused every post-pull status to report every config as modified", + ], "0.20.6": [ "Fix: storage download-table / unload-table no longer OOM on multi-GB tables -- streamed downloads cap RAM at ~1 MiB regardless of table size (#187)", "Fix: _prepend_csv_header() no longer loads the full CSV into RAM (was the second OOM source after slice download)", diff --git a/src/keboola_agent_cli/cli.py b/src/keboola_agent_cli/cli.py index ba9dae87..aeff431e 100644 --- a/src/keboola_agent_cli/cli.py +++ b/src/keboola_agent_cli/cli.py @@ -47,6 +47,7 @@ from .services.sharing_service import SharingService from .services.storage_service import StorageService from .services.sync_service import SyncService +from .services.variables_service import VariablesService from .services.version_service import VersionService from .services.workspace_service import WorkspaceService @@ -189,6 +190,7 @@ def main( sharing_service = SharingService(config_store=config_store) storage_service = StorageService(config_store=config_store) sync_service = SyncService(config_store=config_store) + variables_service = VariablesService(config_store=config_store) encrypt_service = EncryptService(config_store=config_store) workspace_service = WorkspaceService(config_store=config_store) kai_service = KaiService(config_store=config_store) @@ -229,6 +231,7 @@ def main( ctx.obj["sharing_service"] = sharing_service ctx.obj["storage_service"] = storage_service ctx.obj["sync_service"] = sync_service + ctx.obj["variables_service"] = variables_service ctx.obj["encrypt_service"] = encrypt_service ctx.obj["workspace_service"] = workspace_service ctx.obj["kai_service"] = kai_service diff --git a/src/keboola_agent_cli/commands/config.py b/src/keboola_agent_cli/commands/config.py index c9cbaf63..705345ca 100644 --- a/src/keboola_agent_cli/commands/config.py +++ b/src/keboola_agent_cli/commands/config.py @@ -8,8 +8,10 @@ import logging import re from pathlib import Path +from typing import Any import typer +from rich.markup import escape from rich.syntax import Syntax from ..config_store import ConfigStore @@ -832,3 +834,394 @@ def config_new( ) formatter.console.print(syntax) formatter.console.print() + + +def _parse_kv_var(raw: str) -> tuple[str, str]: + """Split a ``KEY=VALUE`` token into ``(key, value)``; ``#``-prefix preserved.""" + if "=" not in raw: + raise typer.BadParameter( + f"Invalid --var: '{raw}'. Expected KEY=VALUE (use # prefix for secrets)." + ) + key, _, value = raw.partition("=") + key = key.strip() + if not key: + raise typer.BadParameter(f"Invalid --var: '{raw}'. Empty key.") + return key, value + + +@config_app.command("variables-set") +def config_variables_set( + ctx: typer.Context, + project: str = typer.Option(..., "--project", help="Project alias"), + component_id: str = typer.Option( + ..., "--component-id", help="Component ID of the config to attach variables to" + ), + config_id: str = typer.Option( + ..., "--config-id", help="Configuration ID to attach variables to" + ), + variable: list[str] | None = typer.Option( + None, + "--var", + help="Variable as KEY=VALUE (repeatable). Prefix key with # to mark as a secret (auto-encrypted).", + ), + replace: bool = typer.Option( + False, + "--replace", + help="Replace ALL variable values instead of merging (drops any keys not in --var).", + ), + variables_id: str | None = typer.Option( + None, + "--variables-id", + help="Attach parent to an existing keboola.variables config (skips auto-create).", + ), + values_id: str | None = typer.Option( + None, + "--values-id", + help="Attach to a specific values row (defaults to the first row).", + ), + branch: int | None = typer.Option(None, "--branch", help="Development branch ID (per-project)"), + dry_run: bool = typer.Option( + False, "--dry-run", help="Preview the change without writing to Keboola." + ), + allow_plaintext: bool = typer.Option( + False, + "--allow-plaintext-on-encrypt-failure", + help="Fall back to plaintext if encryption fails (NOT recommended).", + ), +) -> None: + """Assign variables to a config (auto-creates backing keboola.variables on first call). + + Variables are presented as a flat KEY=VALUE dict. The implementation detail + that Keboola stores them as a separate keboola.variables configuration with + rows is hidden: first call creates the sibling config named + -vars + default row; subsequent calls update the same row. + """ + if should_hint(ctx): + emit_hint( + ctx, + "config.variables-set", + project=project, + component_id=component_id, + config_id=config_id, + variable=variable, + replace=replace, + variables_id=variables_id, + values_id=values_id, + branch=branch, + ) + return + + formatter = get_formatter(ctx) + config_store: ConfigStore = ctx.obj["config_store"] + + raw_vars = variable or [] + if not raw_vars: + formatter.error( + message="At least one --var KEY=VALUE is required.", + error_code="INVALID_ARGUMENT", + ) + raise typer.Exit(code=2) + + variables_dict: dict[str, str] = {} + for raw in raw_vars: + try: + key, value = _parse_kv_var(raw) + except typer.BadParameter as exc: + formatter.error(message=str(exc), error_code="INVALID_ARGUMENT") + raise typer.Exit(code=2) from None + variables_dict[key] = value + + _, effective_branch = resolve_branch(config_store, formatter, project, branch) + + service = get_service(ctx, "variables_service") + + if dry_run: + try: + current = service.get_variables( + alias=project, + component_id=component_id, + config_id=config_id, + branch_id=effective_branch, + ) + except KeboolaApiError as exc: + formatter.error( + message=exc.message, + error_code=exc.error_code, + retryable=exc.retryable, + ) + raise typer.Exit(code=map_error_to_exit_code(exc)) from None + except ConfigError as exc: + formatter.error(message=exc.message, error_code="CONFIG_ERROR") + raise typer.Exit(code=5) from None + + preview_values = ( + dict(variables_dict) if replace else {**current["values"], **variables_dict} + ) + result = { + "dry_run": True, + "project_alias": project, + "parent_component_id": component_id, + "parent_config_id": config_id, + "was_linked": current["linked"], + "current_variables_id": current["variables_id"], + "current_values": current["values"], + "would_write": preview_values, + "action": "would_create" if not current["linked"] else "would_update", + } + if formatter.json_mode: + formatter.output(result) + else: + _format_variables_dry_run(formatter, result) + return + + try: + result = service.set_variables( + alias=project, + component_id=component_id, + config_id=config_id, + variables=variables_dict, + replace=replace, + variables_id=variables_id, + values_id=values_id, + branch_id=effective_branch, + allow_plaintext_fallback=allow_plaintext, + ) + except KeboolaApiError as exc: + formatter.error( + message=exc.message, + error_code=exc.error_code, + retryable=exc.retryable, + ) + raise typer.Exit(code=map_error_to_exit_code(exc)) from None + except ConfigError as exc: + formatter.error(message=exc.message, error_code="CONFIG_ERROR") + raise typer.Exit(code=5) from None + + if formatter.json_mode: + formatter.output(result) + else: + _format_variables_set(formatter, result) + + +@config_app.command("variables-get") +def config_variables_get( + ctx: typer.Context, + project: str = typer.Option(..., "--project", help="Project alias"), + component_id: str = typer.Option( + ..., "--component-id", help="Component ID of the config whose variables to read" + ), + config_id: str = typer.Option( + ..., "--config-id", help="Configuration ID whose variables to read" + ), + branch: int | None = typer.Option(None, "--branch", help="Development branch ID (per-project)"), +) -> None: + """Read the current variable values attached to a config.""" + if should_hint(ctx): + emit_hint( + ctx, + "config.variables-get", + project=project, + component_id=component_id, + config_id=config_id, + branch=branch, + ) + return + + formatter = get_formatter(ctx) + config_store: ConfigStore = ctx.obj["config_store"] + _, effective_branch = resolve_branch(config_store, formatter, project, branch) + + service = get_service(ctx, "variables_service") + try: + result = service.get_variables( + alias=project, + component_id=component_id, + config_id=config_id, + branch_id=effective_branch, + ) + except KeboolaApiError as exc: + formatter.error( + message=exc.message, + error_code=exc.error_code, + retryable=exc.retryable, + ) + raise typer.Exit(code=map_error_to_exit_code(exc)) from None + except ConfigError as exc: + formatter.error(message=exc.message, error_code="CONFIG_ERROR") + raise typer.Exit(code=5) from None + + if formatter.json_mode: + formatter.output(result) + else: + _format_variables_get(formatter, result) + + +@config_app.command("variables-clear") +def config_variables_clear( + ctx: typer.Context, + project: str = typer.Option(..., "--project", help="Project alias"), + component_id: str = typer.Option( + ..., "--component-id", help="Component ID of the config to unlink" + ), + config_id: str = typer.Option( + ..., "--config-id", help="Configuration ID to unlink variables from" + ), + branch: int | None = typer.Option(None, "--branch", help="Development branch ID (per-project)"), + yes: bool = typer.Option( + False, + "--yes", + "-y", + help="Skip confirmation prompt.", + ), +) -> None: + """Unlink variables from a config (does NOT delete the underlying keboola.variables).""" + if should_hint(ctx): + emit_hint( + ctx, + "config.variables-clear", + project=project, + component_id=component_id, + config_id=config_id, + branch=branch, + ) + return + + formatter = get_formatter(ctx) + config_store: ConfigStore = ctx.obj["config_store"] + _, effective_branch = resolve_branch(config_store, formatter, project, branch) + + if not yes and not formatter.json_mode: + confirmed = typer.confirm( + f"Unlink variables from {component_id}/{config_id}? " + "(The underlying variables config will NOT be deleted.)" + ) + if not confirmed: + formatter.console.print("[yellow]Aborted.[/yellow]") + raise typer.Exit(code=0) + + service = get_service(ctx, "variables_service") + try: + result = service.clear_variables( + alias=project, + component_id=component_id, + config_id=config_id, + branch_id=effective_branch, + ) + except KeboolaApiError as exc: + formatter.error( + message=exc.message, + error_code=exc.error_code, + retryable=exc.retryable, + ) + raise typer.Exit(code=map_error_to_exit_code(exc)) from None + except ConfigError as exc: + formatter.error(message=exc.message, error_code="CONFIG_ERROR") + raise typer.Exit(code=5) from None + + if formatter.json_mode: + formatter.output(result) + else: + _format_variables_clear(formatter, result) + + +def _format_variables_get(formatter: Any, result: dict) -> None: + if not result.get("linked"): + formatter.console.print( + "[yellow]No variables linked[/yellow] to " + f"[cyan]{escape(result['parent_component_id'])}[/cyan]/" + f"[cyan]{escape(result['parent_config_id'])}[/cyan]." + ) + return + + formatter.console.print( + f"[bold]Variables on[/bold] " + f"[cyan]{escape(result['parent_component_id'])}[/cyan]/" + f"[cyan]{escape(result['parent_config_id'])}[/cyan] " + f"[dim](variables_id={escape(result['variables_id'] or '')}, " + f"values_id={escape(result['values_id'] or '')})[/dim]" + ) + if not result["values"]: + formatter.console.print(" [dim](no values set)[/dim]") + return + for k in sorted(result["values"]): + v = result["values"][k] + display_v = "" if k.startswith("#") else escape(str(v)) + formatter.console.print(f" [green]{escape(k)}[/green] = {display_v}") + + +def _format_variables_set(formatter: Any, result: dict) -> None: + action_label = ( + "[green]created[/green]" if result["action"] == "created" else "[yellow]updated[/yellow]" + ) + formatter.console.print( + f"Variables {action_label} on " + f"[cyan]{escape(result['parent_component_id'])}[/cyan]/" + f"[cyan]{escape(result['parent_config_id'])}[/cyan]" + ) + formatter.console.print(f" variables_id: [cyan]{escape(result['variables_id'])}[/cyan]") + formatter.console.print(f" values_id: [cyan]{escape(result['values_id'])}[/cyan]") + if result.get("encrypted_keys"): + joined = ", ".join(escape(k) for k in result["encrypted_keys"]) + formatter.console.print(f" [dim]encrypted: {joined}[/dim]") + formatter.console.print("[bold]Final values:[/bold]") + for k in sorted(result["values"]): + v = result["values"][k] + display_v = "" if k.startswith("#") else escape(str(v)) + formatter.console.print(f" [green]{escape(k)}[/green] = {display_v}") + + +def _format_variables_clear(formatter: Any, result: dict) -> None: + if not result["was_linked"]: + formatter.console.print( + f"[yellow]Nothing to clear[/yellow]: " + f"[cyan]{escape(result['parent_component_id'])}[/cyan]/" + f"[cyan]{escape(result['parent_config_id'])}[/cyan] had no variables linked." + ) + return + formatter.console.print( + f"[green]Unlinked[/green] variables from " + f"[cyan]{escape(result['parent_component_id'])}[/cyan]/" + f"[cyan]{escape(result['parent_config_id'])}[/cyan] " + f"[dim](was variables_id={escape(result['unlinked_variables_id'] or '')}, " + f"values_id={escape(result['unlinked_values_id'] or '')})[/dim]" + ) + formatter.console.print( + "[dim]The underlying keboola.variables config was NOT deleted. " + "Use 'kbagent config delete' to remove it if no other config references it.[/dim]" + ) + + +def _format_variables_dry_run(formatter: Any, result: dict) -> None: + verb = "create" if result["action"] == "would_create" else "update" + formatter.console.print( + f"[yellow]DRY RUN[/yellow]: would {verb} variables on " + f"[cyan]{escape(result['parent_component_id'])}[/cyan]/" + f"[cyan]{escape(result['parent_config_id'])}[/cyan]" + ) + if result["was_linked"]: + formatter.console.print( + f" current variables_id: [cyan]{escape(result['current_variables_id'] or '')}[/cyan]" + ) + else: + formatter.console.print(" [dim]no variables currently linked[/dim]") + + current_keys = set(result["current_values"]) + proposed_keys = set(result["would_write"]) + for k in sorted(current_keys | proposed_keys): + current_v = result["current_values"].get(k) + proposed_v = result["would_write"].get(k) + if k not in proposed_keys: + display = "" if k.startswith("#") else escape(str(current_v)) + formatter.console.print(f" [red]- {escape(k)}[/red] = {display} [dim](dropped)[/dim]") + elif k not in current_keys: + display = "" if k.startswith("#") else escape(str(proposed_v)) + formatter.console.print(f" [green]+ {escape(k)}[/green] = {display}") + elif current_v != proposed_v: + display_cur = "" if k.startswith("#") else escape(str(current_v)) + display_new = "" if k.startswith("#") else escape(str(proposed_v)) + formatter.console.print( + f" [yellow]~ {escape(k)}[/yellow] = {display_cur} -> {display_new}" + ) + else: + display = "" if k.startswith("#") else escape(str(current_v)) + formatter.console.print(f" [dim]= {escape(k)} = {display}[/dim]") diff --git a/src/keboola_agent_cli/commands/context.py b/src/keboola_agent_cli/commands/context.py index 64a2f526..8f9ad5b7 100644 --- a/src/keboola_agent_cli/commands/context.py +++ b/src/keboola_agent_cli/commands/context.py @@ -119,6 +119,18 @@ kbagent config search --query PATTERN [--project NAME] [--component-type TYPE] [-i] [-r] [--branch ID] Search config bodies for string/regex. Reports match location in JSON tree. Branch-aware. + kbagent config variables-set --project NAME --component-id ID --config-id ID --var KEY=VALUE [--var ...] [--replace] [--variables-id ID] [--values-id ID] [--branch ID] [--dry-run] + Assign variables to any config. Auto-creates the backing keboola.variables + default row + on first call and links the parent; subsequent calls update the same row (merge by default; + --replace for full overwrite). Prefix KEY with # to auto-encrypt as a secret. + + kbagent config variables-get --project NAME --component-id ID --config-id ID [--branch ID] + Read variable values attached to a config. Returns linked, variables_id, values_id, values. + + kbagent config variables-clear --project NAME --component-id ID --config-id ID [--branch ID] [--yes] + Unlink variables from a config. Does NOT delete the underlying keboola.variables config + (it may be shared). Delete it explicitly via `kbagent config delete` if needed. + ### Job History kbagent job list [--project NAME] [--component-id ID] [--config-id ID] [--status STATUS] [--limit N] diff --git a/src/keboola_agent_cli/constants.py b/src/keboola_agent_cli/constants.py index 67bb0013..32118767 100644 --- a/src/keboola_agent_cli/constants.py +++ b/src/keboola_agent_cli/constants.py @@ -149,7 +149,7 @@ MANIFEST_FILENAME: str = "manifest.json" BRANCH_MAPPING_FILENAME: str = "branch-mapping.json" CONFIG_FILENAME: str = "_config.yml" -MANIFEST_VERSION: int = 2 +MANIFEST_VERSION: int = 3 DEFAULT_NAMING_BRANCH: str = "{branch_name}" DEFAULT_NAMING_CONFIG: str = "{component_type}/{component_id}/{config_name}" DEFAULT_NAMING_CONFIG_ROW: str = "rows/{config_row_name}" @@ -159,8 +159,10 @@ DEFAULT_NAMING_VARIABLES: str = "variables" DEFAULT_NAMING_VARIABLES_VALUES: str = "values/{config_row_name}" DEFAULT_NAMING_DATA_APP: str = "app/{component_id}/{config_name}" -# Aliases used by sync subsystem -CONFIG_YML_VERSION: int = MANIFEST_VERSION +# _config.yml file-format version is independent of the manifest schema version. +# Manifest v3 introduces ManifestConfigRow.metadata (row-level pull hashes) but does +# not change the on-disk YAML shape, so CONFIG_YML_VERSION stays at 2. +CONFIG_YML_VERSION: int = 2 SANITIZE_NAME_MAX_LENGTH: int = 100 # --- Sync Pull: Storage & Jobs --- diff --git a/src/keboola_agent_cli/hints/definitions/config.py b/src/keboola_agent_cli/hints/definitions/config.py index b02a713f..d019bb16 100644 --- a/src/keboola_agent_cli/hints/definitions/config.py +++ b/src/keboola_agent_cli/hints/definitions/config.py @@ -160,3 +160,129 @@ ], ) ) + +# ── config variables-set ─────────────────────────────────────────── + +HintRegistry.register( + CommandHint( + cli_command="config.variables-set", + description="Assign variables to a config (auto-creates keboola.variables if absent)", + steps=[ + HintStep( + comment="Set variable values on a config; creates or updates the backing keboola.variables", + client=ClientCall( + method="get_config_detail", + args={ + "component_id": "{component_id}", + "config_id": "{config_id}", + "branch_id": "{branch}", + }, + result_var="parent", + result_hint="dict", + ), + service=ServiceCall( + service_class="VariablesService", + service_module="variables_service", + method="set_variables", + args={ + "alias": "{project}", + "component_id": "{component_id}", + "config_id": "{config_id}", + "variables": "{variable}", + "replace": "{replace}", + "variables_id": "{variables_id}", + "values_id": "{values_id}", + "branch_id": "{branch}", + }, + ), + ), + ], + notes=[ + "--var takes KEY=VALUE, repeatable. Prefix KEY with # to auto-encrypt as secret.", + "Without --variables-id, a sibling keboola.variables config named " + "'-vars' is created on first call and the parent is linked.", + "Subsequent calls update the same default row; --replace overwrites instead of merging.", + ], + ) +) + +# ── config variables-get ─────────────────────────────────────────── + +HintRegistry.register( + CommandHint( + cli_command="config.variables-get", + description="Read variable values attached to a config", + steps=[ + HintStep( + comment="Resolve variables_id + values_id from parent, fetch the row", + client=ClientCall( + method="get_config_detail", + args={ + "component_id": "{component_id}", + "config_id": "{config_id}", + "branch_id": "{branch}", + }, + result_var="parent", + result_hint="dict", + ), + service=ServiceCall( + service_class="VariablesService", + service_module="variables_service", + method="get_variables", + args={ + "alias": "{project}", + "component_id": "{component_id}", + "config_id": "{config_id}", + "branch_id": "{branch}", + }, + ), + ), + ], + notes=[ + "Response: {'linked': bool, 'variables_id': str|None, 'values_id': str|None, " + "'values': {name: value}}.", + "linked=False means the parent has no variables_id set.", + ], + ) +) + +# ── config variables-clear ───────────────────────────────────────── + +HintRegistry.register( + CommandHint( + cli_command="config.variables-clear", + description="Unlink variables from a config (underlying keboola.variables is NOT deleted)", + steps=[ + HintStep( + comment="Strip variables_id + variables_values_id from parent config", + client=ClientCall( + method="update_config", + args={ + "component_id": "{component_id}", + "config_id": "{config_id}", + "configuration": "", + "branch_id": "{branch}", + "change_description": "Unlinked variables via kbagent", + }, + result_var="result", + result_hint="dict", + ), + service=ServiceCall( + service_class="VariablesService", + service_module="variables_service", + method="clear_variables", + args={ + "alias": "{project}", + "component_id": "{component_id}", + "config_id": "{config_id}", + "branch_id": "{branch}", + }, + ), + ), + ], + notes=[ + "Clear unlinks only -- the keboola.variables config remains in the project " + "(may be shared). Use 'kbagent config delete' to remove it explicitly.", + ], + ) +) diff --git a/src/keboola_agent_cli/permissions.py b/src/keboola_agent_cli/permissions.py index 159f2562..1955f61c 100644 --- a/src/keboola_agent_cli/permissions.py +++ b/src/keboola_agent_cli/permissions.py @@ -30,6 +30,9 @@ "config.rename": "write", "config.delete": "destructive", "config.new": "write", + "config.variables-set": "write", + "config.variables-get": "read", + "config.variables-clear": "destructive", # Job history "job.list": "read", "job.detail": "read", diff --git a/src/keboola_agent_cli/services/_encryption.py b/src/keboola_agent_cli/services/_encryption.py new file mode 100644 index 00000000..c8d2c107 --- /dev/null +++ b/src/keboola_agent_cli/services/_encryption.py @@ -0,0 +1,197 @@ +"""Shared helpers for encrypting ``#``-prefixed configuration secrets. + +Used by both :mod:`sync_service` (push-time encryption of row/parent configs) +and :mod:`variables_service` (encryption of values set via +``config variables-set``). Keeping the helpers here instead of on either +service keeps the encrypt-before-write contract in a single module and +avoids one service importing another's internals. +""" + +from __future__ import annotations + +import logging +from typing import Any + +from ..errors import KeboolaApiError + +logger = logging.getLogger(__name__) + +_ENCRYPTED_PREFIX = "KBC::" + + +def is_secret_key(key: str) -> bool: + """True if ``key`` is a Keboola secret key (starts with ``#``).""" + return isinstance(key, str) and key.startswith("#") + + +def is_already_encrypted(value: Any) -> bool: + """True if ``value`` is already encrypted (``KBC::`` prefix).""" + return isinstance(value, str) and value.startswith(_ENCRYPTED_PREFIX) + + +def _is_secret_name_value_pair(item: Any) -> bool: + """True if ``item`` is a row-hoisted secret entry like ``{"name": "#x", "value": "..."}``. + + ``keboola.variables`` rows store each variable as ``{name, value}`` inside + a ``values`` list, so a ``#secret`` lives in the ``name`` field rather + than as a dict key. This helper lets the walkers recognize that shape so + the encryption contract holds for row-hoisted components without every + caller having to pre-flatten. (``keboola.shared-code`` also hoists rows + but its payload is ``code_content`` -- no ``name``/``value`` pairs -- + so this check never fires there, which is correct.) + """ + return ( + isinstance(item, dict) + and isinstance(item.get("name"), str) + and is_secret_key(item["name"]) + and isinstance(item.get("value"), str) + ) + + +def collect_secrets(obj: Any, path_prefix: str, result: dict[str, str]) -> None: + """Recursively collect unencrypted ``#``-prefixed secret values. + + Builds a flat dict of ``{#path.key: plaintext_value}`` suitable for the + Encryption API. The Encryption API requires every key to start with + ``#``, so a nested key ``parameters.#api_token`` is flattened into + ``#parameters.#api_token`` with the outer ``#`` added. + + Also recognizes the row-hoisted ``{"name": "#x", "value": "..."}`` list + element shape used by ``keboola.variables``; see + :func:`_is_secret_name_value_pair`. + """ + if isinstance(obj, dict): + for key, value in obj.items(): + if is_secret_key(key) and isinstance(value, str) and not is_already_encrypted(value): + encrypt_key = f"#{path_prefix}{key}" if path_prefix else key + result[encrypt_key] = value + elif isinstance(value, (dict, list)): + child_prefix = f"{path_prefix}{key}." if path_prefix else f"{key}." + collect_secrets(value, child_prefix, result) + elif isinstance(obj, list): + for i, item in enumerate(obj): + if _is_secret_name_value_pair(item) and not is_already_encrypted(item["value"]): + encrypt_key = f"#{path_prefix}[{i}].{item['name']}" + result[encrypt_key] = item["value"] + else: + collect_secrets(item, f"{path_prefix}[{i}].", result) + + +def apply_encrypted(obj: Any, path_prefix: str, encrypted: dict[str, str]) -> None: + """Recursively apply encrypted values back into ``obj`` by matching flattened keys.""" + if isinstance(obj, dict): + for key in list(obj.keys()): + value = obj[key] + if is_secret_key(key) and isinstance(value, str) and not is_already_encrypted(value): + encrypt_key = f"#{path_prefix}{key}" if path_prefix else key + if encrypt_key in encrypted: + obj[key] = encrypted[encrypt_key] + elif isinstance(value, (dict, list)): + child_prefix = f"{path_prefix}{key}." if path_prefix else f"{key}." + apply_encrypted(value, child_prefix, encrypted) + elif isinstance(obj, list): + for i, item in enumerate(obj): + if _is_secret_name_value_pair(item) and not is_already_encrypted(item["value"]): + encrypt_key = f"#{path_prefix}[{i}].{item['name']}" + if encrypt_key in encrypted: + item["value"] = encrypted[encrypt_key] + else: + apply_encrypted(item, f"{path_prefix}[{i}].", encrypted) + + +def apply_encrypted_to_local(local: Any, pushed: Any) -> None: + """Copy encrypted secret values from ``pushed`` config back into ``local``. + + Walks both dicts in parallel. Where ``pushed`` has an encrypted value for a + ``#``-key, replaces the ``local`` plaintext with it so the on-disk file + matches server state after a successful push. Also handles the row-hoisted + ``{"name": "#x", "value": "..."}`` list element shape. + """ + if isinstance(local, dict) and isinstance(pushed, dict): + for key in local: + if key not in pushed: + continue + if is_secret_key(key) and is_already_encrypted(pushed[key]): + local[key] = pushed[key] + elif isinstance(local[key], dict) and isinstance(pushed[key], dict): + apply_encrypted_to_local(local[key], pushed[key]) + elif isinstance(local[key], list) and isinstance(pushed[key], list): + for i in range(min(len(local[key]), len(pushed[key]))): + lo = local[key][i] + pu = pushed[key][i] + if ( + _is_secret_name_value_pair(lo) + and isinstance(pu, dict) + and pu.get("name") == lo["name"] + and is_already_encrypted(pu.get("value")) + ): + lo["value"] = pu["value"] + else: + apply_encrypted_to_local(lo, pu) + + +def encrypt_secrets_in_config( + client: Any, + project_id: int | None, + component_id: str, + configuration: dict[str, Any], + *, + allow_plaintext_fallback: bool = False, +) -> dict[str, Any]: + """Encrypt ``#``-prefixed secret values in ``configuration`` in place. + + Walks ``configuration`` recursively via :func:`collect_secrets`, sends the + flat secret map to the Encryption API (``client.encrypt_values``), and + writes the ciphertext back into the original dict via + :func:`apply_encrypted`. + + Fail-closed by default: any encryption exception raises + :class:`KeboolaApiError` with code ``ENCRYPTION_FAILED``. Set + ``allow_plaintext_fallback=True`` to log-and-continue instead (for + bootstrap/debug scenarios only). + + Args: + client: HTTP client exposing ``encrypt_values(project_id, component_id, data)``. + project_id: Keboola project ID. When falsy, encryption is skipped entirely + (caller's responsibility to decide when that's safe). + component_id: Component ID for the Encryption API scope. + configuration: Config dict to encrypt in place (returned for chaining). + allow_plaintext_fallback: When ``False`` (default), raise on encryption + failure. When ``True``, log a warning and return the config with + plaintext intact. + """ + if not project_id: + return configuration + + secrets: dict[str, str] = {} + collect_secrets(configuration, "", secrets) + if not secrets: + return configuration + + try: + encrypted = client.encrypt_values( + project_id=project_id, + component_id=component_id, + data=secrets, + ) + apply_encrypted(configuration, "", encrypted) + logger.info("Encrypted %d secret value(s) for %s", len(encrypted), component_id) + except Exception as exc: + if allow_plaintext_fallback: + logger.warning( + "Failed to encrypt secrets for %s: %s (plaintext fallback allowed)", + component_id, + exc, + ) + else: + raise KeboolaApiError( + message=( + f"Encryption failed for {component_id}: {exc}. " + f"Refusing to push plaintext secrets. " + f"Use --allow-plaintext-on-encrypt-failure to override." + ), + status_code=0, + error_code="ENCRYPTION_FAILED", + ) from exc + + return configuration diff --git a/src/keboola_agent_cli/services/sync_service.py b/src/keboola_agent_cli/services/sync_service.py index b5725602..6f643334 100644 --- a/src/keboola_agent_cli/services/sync_service.py +++ b/src/keboola_agent_cli/services/sync_service.py @@ -40,8 +40,9 @@ api_row_to_local, classify_component_type, local_config_to_api, + local_row_to_api, ) -from ..sync.diff_engine import compute_changeset, config_hash +from ..sync.diff_engine import compute_changeset, compute_row_changeset, config_hash from ..sync.git_utils import get_default_branch, is_git_repo from ..sync.manifest import ( Manifest, @@ -55,6 +56,7 @@ save_manifest, ) from ..sync.naming import config_path, config_row_path, sanitize_name +from ._encryption import apply_encrypted_to_local, encrypt_secrets_in_config from .base import BaseService logger = logging.getLogger(__name__) @@ -302,6 +304,17 @@ def pull( existing_branch_ids: dict[str, int] = { f"{c.component_id}/{c.id}": c.branch_id for c in manifest.configurations } + # Row-level state lookup, keyed by "{component_id}/{config_id}/{row_id}". + # Values: dict with path, pull_hash, pull_config_hash (all optional). + existing_rows: dict[str, dict[str, str]] = {} + for c in manifest.configurations: + for r in c.rows: + row_key = f"{c.component_id}/{c.id}/{r.id}" + existing_rows[row_key] = { + "path": r.path, + "pull_hash": r.metadata.get("pull_hash", ""), + "pull_config_hash": r.metadata.get("pull_config_hash", ""), + } for component in components: component_id = component.get("id", "") @@ -467,33 +480,81 @@ def pull( } ) - # Handle rows -- skip writing if config is unchanged or - # locally modified (rows inherit the parent config's state). - skip_rows = locally_modified or remote_unchanged + # Row-level pull: each row gets its own 3-way diff state + # (pull_hash, pull_config_hash) so ``sync push`` can detect + # per-row local modifications and push only changed rows. row_manifests: list[ManifestConfigRow] = [] used_row_paths: set[str] = set() for row in cfg.get("rows", []): row_id = str(row.get("id", "")) row_name = row.get("name", "untitled") - row_rel_path = config_row_path( - manifest.naming.config_row, - row_name, - ) - if row_rel_path in used_row_paths: - suffix = row_id[:8] if len(row_id) > 8 else row_id - row_rel_path = f"{row_rel_path}-{suffix}" + # Reuse existing row path (stable) if already tracked. + row_lookup_key = f"{component_id}/{config_id}/{row_id}" + existing_row = existing_rows.get(row_lookup_key) + if existing_row: + row_rel_path = existing_row["path"] + else: + row_rel_path = config_row_path( + manifest.naming.config_row, + row_name, + ) + if row_rel_path in used_row_paths: + suffix = row_id[:8] if len(row_id) > 8 else row_id + row_rel_path = f"{row_rel_path}-{suffix}" used_row_paths.add(row_rel_path) row_dir = config_dir / row_rel_path - if not skip_rows: - row_local = api_row_to_local(row, component_id) + row_local = api_row_to_local(row, component_id) + row_api_cfg_hash = config_hash(row_local) + + row_file = row_dir / CONFIG_FILENAME + old_row_file_hash = existing_row["pull_hash"] if existing_row else "" + old_row_cfg_hash = existing_row["pull_config_hash"] if existing_row else "" + row_locally_modified = False + if existing_row and not force and old_row_file_hash and row_file.exists(): + row_locally_modified = self._file_hash(row_file) != old_row_file_hash + + if row_locally_modified: + # Preserve local edits; keep old hashes as the 3-way base. + row_file_hash = old_row_file_hash + row_pull_cfg_hash = old_row_cfg_hash + pull_details.append( + { + "action": "skipped", + "component_id": component_id, + "config_name": f"{config_name}/{row_name}", + "path": f"{rel_path}/{row_rel_path}", + "reason": "row locally modified", + } + ) + elif existing_row and old_row_cfg_hash and old_row_cfg_hash == row_api_cfg_hash: + # Idempotent: remote unchanged since last pull, file untouched. + row_file_hash = ( + self._file_hash(row_file) if row_file.exists() else old_row_file_hash + ) + row_pull_cfg_hash = row_api_cfg_hash + else: + # New or remote-changed row: write the file. if not dry_run: self._write_config_file(row_dir, row_local) + row_file_hash = self._file_hash(row_file) if row_file.exists() else "" + else: + row_file_hash = "" + row_pull_cfg_hash = row_api_cfg_hash files_written += 1 rows_pulled += 1 - row_manifests.append(ManifestConfigRow(id=row_id, path=row_rel_path)) + row_manifests.append( + ManifestConfigRow( + id=row_id, + path=row_rel_path, + metadata={ + "pull_hash": row_file_hash, + "pull_config_hash": row_pull_cfg_hash, + }, + ) + ) # Record in manifest (store file hash for change detection). # For skipped configs: keep existing pull_hash (file untouched) @@ -706,8 +767,11 @@ def diff( components = client.list_components_with_configs(branch_id=branch_id) self._ensure_branch_registered(manifest, branch_id, client) - # Build remote configs lookup: "{component_id}/{config_id}" -> API data + # Build remote lookups: + # remote_configs: "{component_id}/{config_id}" -> parent config data + # remote_rows: "{component_id}/{parent_config_id}/rows/{row_id}" -> row data remote_configs: dict[str, dict[str, Any]] = {} + remote_rows: dict[str, dict[str, Any]] = {} for component in components: component_id = component.get("id", "") if component_id in ALWAYS_IGNORED_COMPONENTS: @@ -715,8 +779,11 @@ def diff( for cfg in component.get("configurations", []): config_id = str(cfg.get("id", "")) key = f"{component_id}/{config_id}" - # Convert remote to local format for apples-to-apples comparison remote_configs[key] = api_config_to_local(component_id, cfg, config_id) + for row in cfg.get("rows", []): + row_id = str(row.get("id", "")) + row_key = f"{component_id}/{config_id}/rows/{row_id}" + remote_rows[row_key] = api_row_to_local(row, component_id) # Build local configs list from manifest. # For files unchanged since pull, use the stored pull_config_hash @@ -821,6 +888,59 @@ def diff( local_override_hashes or None, ) + # Row-level diff: walk manifest rows, load local YAML, feed into + # compute_row_changeset alongside remote_rows built above. + local_rows: list[dict[str, Any]] = [] + tracked_row_keys: set[str] = set() + row_base_hashes: dict[str, str] = {} + for cfg in manifest.configurations: + branch_path = self._find_branch_path(manifest, cfg.branch_id) + parent_dir = project_root / branch_path / cfg.path + for row in cfg.rows: + row_key = f"{cfg.component_id}/{cfg.id}/rows/{row.id}" + tracked_row_keys.add(row_key) + row_dir = parent_dir / row.path + row_local = self._read_config_file(row_dir) + if row_local is None: + # File missing -> "deleted" detected via tracked_row_keys. + continue + local_rows.append( + { + "component_id": cfg.component_id, + "parent_config_id": cfg.id, + "row_id": row.id, + "row_name": row_local.get("name", ""), + "path": row.path, + "data": row_local, + } + ) + pch = row.metadata.get("pull_config_hash") if row.metadata else "" + if pch: + row_base_hashes[row_key] = pch + + # Also add untracked local rows (new row dirs dropped under a tracked + # config). They have no row_id yet so compute_row_changeset flags + # them as "added" and push dispatches them via create_config_row. + for untracked in self._find_untracked_rows(project_root, manifest): + local_rows.append( + { + "component_id": untracked["component_id"], + "parent_config_id": untracked["parent_config_id"], + "row_id": "", + "row_name": untracked["row_name"], + "path": untracked["path"], + "data": untracked["data"], + } + ) + + row_changeset = compute_row_changeset( + local_rows, + remote_rows, + tracked_row_keys, + row_base_hashes or None, + ) + changeset.extend(row_changeset) + added = [c for c in changeset if c.change_type == "added"] modified = [c for c in changeset if c.change_type == "modified"] remote_modified = [c for c in changeset if c.change_type == "remote_modified"] @@ -944,8 +1064,33 @@ def push( component_id = change["component_id"] config_id = change["config_id"] config_path_str = change.get("path", "") + is_row = bool(change.get("is_row")) + parent_config_id = change.get("parent_config_id", "") try: + if is_row: + self._push_row_change( + client, + change_type=change_type, + component_id=component_id, + parent_config_id=parent_config_id, + row_id=config_id, + row_path_str=config_path_str, + project_root=project_root, + manifest=manifest, + branch_id=branch_id, + allow_plaintext_fallback=allow_plaintext_fallback, + ) + manifest_dirty = True + if change_type == "added": + created += 1 + elif change_type == "modified": + updated += 1 + elif change_type == "deleted": + deleted += 1 + pushed_details.append(change) + continue + if change_type == "added": result = self._push_create( client, @@ -1045,6 +1190,13 @@ def push( pushed_details.append(change) except Exception as exc: + # Fail-closed on encryption failures: a partial push that + # omits the failed change would leave a plaintext secret + # elsewhere or a caller believing the push "mostly succeeded". + # Surface to the CLI (exit non-zero) rather than burying in + # result["errors"]. + if isinstance(exc, KeboolaApiError) and exc.error_code == "ENCRYPTION_FAILED": + raise logger.warning( "Failed to push %s %s/%s: %s", change_type, @@ -1077,68 +1229,225 @@ def push( result_data["name_drift_warnings"] = name_drift_warnings return result_data - @staticmethod - def _encrypt_secrets_in_config( + # Kept for test compatibility: tests exercise fail-closed encryption via + # ``SyncService._encrypt_secrets_in_config(...)``. Production code uses + # :func:`encrypt_secrets_in_config` directly. + _encrypt_secrets_in_config = staticmethod(encrypt_secrets_in_config) + + def _push_row_change( + self, client: Any, - project_id: int | None, - component_id: str, - configuration: dict[str, Any], *, + change_type: str, + component_id: str, + parent_config_id: str, + row_id: str, + row_path_str: str, + project_root: Path, + manifest: Manifest, + branch_id: int | None, allow_plaintext_fallback: bool = False, - ) -> dict[str, Any]: - """Encrypt #-prefixed secret values in configuration before push. - - Walks the configuration dict recursively, collects all #-prefixed keys - with unencrypted string values, sends them to the Encryption API, - and replaces plaintext values with encrypted ones. + ) -> None: + """Dispatch a single row-level change (added/modified/deleted) to the API. - Args: - client: HTTP client with encrypt_values method. - project_id: Keboola project ID (encryption skipped if None). - component_id: Component ID for encryption context. - configuration: Config dict to encrypt in-place. - allow_plaintext_fallback: When False (default), encryption failure - raises KeboolaApiError. When True, logs a warning and continues - with plaintext values (escape hatch). + ``#``-prefixed secrets in the row's configuration are encrypted via + :func:`encrypt_secrets_in_config` before POST/PUT (same fail-closed + semantics as parent configs). Mutates ``manifest`` in place; the + caller is responsible for persisting it. """ - if not project_id: - return configuration + parent = next( + ( + c + for c in manifest.configurations + if c.component_id == component_id and c.id == parent_config_id + ), + None, + ) + if parent is None and change_type != "deleted": + raise KeboolaApiError( + message=( + f"Cannot push row {row_id}: parent config {component_id}/" + f"{parent_config_id} is not tracked in the manifest." + ), + status_code=0, + error_code="PARENT_CONFIG_NOT_TRACKED", + ) - # Collect all unencrypted secret values - secrets: dict[str, str] = {} - _collect_secrets(configuration, "", secrets) + project_id = manifest.project.id if manifest.project else None - if not secrets: - return configuration + if change_type == "deleted": + self._push_delete_row( + client, + component_id=component_id, + parent_config_id=parent_config_id, + row_id=row_id, + parent=parent, + branch_id=branch_id, + ) + return - try: - encrypted = client.encrypt_values( + # added / modified both read a local row file and encrypt-then-push. + assert parent is not None # guarded above for non-deleted change_types + row_dir = ( + project_root / self._find_branch_path(manifest, branch_id) / parent.path / row_path_str + ) + + if change_type == "added": + self._push_create_row( + client, + component_id=component_id, + parent_config_id=parent_config_id, + row_dir=row_dir, + parent=parent, + row_path_str=row_path_str, + branch_id=branch_id, project_id=project_id, + allow_plaintext_fallback=allow_plaintext_fallback, + ) + return + + if change_type == "modified": + self._push_update_row( + client, component_id=component_id, - data=secrets, + parent_config_id=parent_config_id, + row_id=row_id, + row_dir=row_dir, + parent=parent, + branch_id=branch_id, + project_id=project_id, + allow_plaintext_fallback=allow_plaintext_fallback, ) - # Apply encrypted values back into configuration - _apply_encrypted(configuration, "", encrypted) - logger.info("Encrypted %d secret value(s) for %s", len(encrypted), component_id) - except Exception as exc: - if allow_plaintext_fallback: - logger.warning( - "Failed to encrypt secrets for %s: %s (plaintext fallback allowed)", - component_id, - exc, - ) - else: - raise KeboolaApiError( - message=( - f"Encryption failed for {component_id}: {exc}. " - f"Refusing to push plaintext secrets. " - f"Use --allow-plaintext-on-encrypt-failure to override." - ), - status_code=0, - error_code="ENCRYPTION_FAILED", - ) from exc - - return configuration + return + + raise ValueError(f"Unsupported row change_type: {change_type}") + + def _push_create_row( + self, + client: Any, + *, + component_id: str, + parent_config_id: str, + row_dir: Path, + parent: ManifestConfiguration, + row_path_str: str, + branch_id: int | None, + project_id: int | None, + allow_plaintext_fallback: bool, + ) -> None: + """POST a new row; record API-assigned id + hashes in the parent's row list.""" + local_data = self._read_config_file(row_dir) + if local_data is None: + raise FileNotFoundError(f"Row file not found: {row_dir / CONFIG_FILENAME}") + + pristine_data = copy.deepcopy(local_data) + name, description, configuration = local_row_to_api(local_data) + configuration = encrypt_secrets_in_config( + client, + project_id, + component_id, + configuration, + allow_plaintext_fallback=allow_plaintext_fallback, + ) + + result = client.create_config_row( + component_id=component_id, + config_id=parent_config_id, + name=name, + configuration=configuration, + description=description, + branch_id=branch_id, + ) + new_row_id = str(result.get("id", "")) + logger.info("Created row %s/%s/%s", component_id, parent_config_id, new_row_id) + + # Write-back: encrypted secrets land in the local file so a subsequent + # diff sees local == remote. ``config_id=""`` tells the shared helper + # to skip writing a config_id into ``_keboola`` (rows use ``row_id``). + self._writeback_after_push(pristine_data, row_dir, "", configuration) + + row_file = row_dir / CONFIG_FILENAME + new_file_hash = self._file_hash(row_file) if row_file.exists() else "" + cfg_hash_value = config_hash(pristine_data) + parent.rows.append( + ManifestConfigRow( + id=new_row_id, + path=row_path_str, + metadata={"pull_hash": new_file_hash, "pull_config_hash": cfg_hash_value}, + ) + ) + + def _push_update_row( + self, + client: Any, + *, + component_id: str, + parent_config_id: str, + row_id: str, + row_dir: Path, + parent: ManifestConfiguration, + branch_id: int | None, + project_id: int | None, + allow_plaintext_fallback: bool, + ) -> None: + """PUT an existing row; refresh its hashes in the parent's row list.""" + local_data = self._read_config_file(row_dir) + if local_data is None: + raise FileNotFoundError(f"Row file not found: {row_dir / CONFIG_FILENAME}") + + pristine_data = copy.deepcopy(local_data) + name, description, configuration = local_row_to_api(local_data) + configuration = encrypt_secrets_in_config( + client, + project_id, + component_id, + configuration, + allow_plaintext_fallback=allow_plaintext_fallback, + ) + + client.update_config_row( + component_id=component_id, + config_id=parent_config_id, + row_id=row_id, + name=name, + configuration=configuration, + description=description, + change_description="Updated via kbagent sync push", + branch_id=branch_id, + ) + logger.info("Updated row %s/%s/%s", component_id, parent_config_id, row_id) + + self._writeback_after_push(pristine_data, row_dir, "", configuration) + + row_file = row_dir / CONFIG_FILENAME + new_file_hash = self._file_hash(row_file) if row_file.exists() else "" + cfg_hash_value = config_hash(pristine_data) + for r in parent.rows: + if r.id == row_id: + r.metadata["pull_hash"] = new_file_hash + r.metadata["pull_config_hash"] = cfg_hash_value + break + + def _push_delete_row( + self, + client: Any, + *, + component_id: str, + parent_config_id: str, + row_id: str, + parent: ManifestConfiguration | None, + branch_id: int | None, + ) -> None: + """DELETE a row; prune it from the parent's row list in the manifest.""" + client.delete_config_row( + component_id=component_id, + config_id=parent_config_id, + row_id=row_id, + branch_id=branch_id, + ) + if parent is not None: + parent.rows = [r for r in parent.rows if r.id != row_id] + logger.info("Deleted row %s/%s/%s", component_id, parent_config_id, row_id) def _push_create( self, @@ -1170,7 +1479,7 @@ def _push_create( # Encrypt #-prefixed secrets before sending to API project_id = manifest.project.id if manifest.project else None - configuration = self._encrypt_secrets_in_config( + configuration = encrypt_secrets_in_config( client, project_id, component_id, @@ -1230,7 +1539,7 @@ def _push_update( # Encrypt #-prefixed secrets before sending to API project_id = manifest.project.id if manifest.project else None - configuration = self._encrypt_secrets_in_config( + configuration = encrypt_secrets_in_config( client, project_id, component_id, @@ -1275,7 +1584,7 @@ def _writeback_after_push( pushed_params = pushed_configuration.get("parameters", {}) local_params = local_data.get("parameters", {}) if pushed_params and local_params: - _apply_encrypted_to_local(local_params, pushed_params) + apply_encrypted_to_local(local_params, pushed_params) self._write_config_file(config_dir, local_data) logger.debug("Updated local config at %s after push", config_dir) @@ -2070,7 +2379,13 @@ def _mask_encrypted_columns(csv_data: str) -> str: return output.getvalue() def _write_config_file(self, config_dir: Path, config_data: dict[str, Any]) -> str: - """Write a ``_config.yml`` file and return its SHA256 hash.""" + """Write a ``_config.yml`` file and return its SHA256 hash. + + Uses ``newline=""`` so Windows does NOT translate ``\\n`` into ``\\r\\n`` + on write -- without that, the in-memory ``content`` hash diverges from + the on-disk byte hash (:meth:`_file_hash`) and every post-pull + ``status`` would report the file as modified. + """ config_dir.mkdir(parents=True, exist_ok=True) config_file = config_dir / CONFIG_FILENAME content = yaml.dump( @@ -2080,7 +2395,7 @@ def _write_config_file(self, config_dir: Path, config_data: dict[str, Any]) -> s sort_keys=False, width=120, ) - config_file.write_text(content, encoding="utf-8") + config_file.write_text(content, encoding="utf-8", newline="") return hashlib.sha256(content.encode("utf-8")).hexdigest() def _file_hash(self, file_path: Path) -> str: @@ -2268,78 +2583,44 @@ def _find_untracked_configs( return added + def _find_untracked_rows(self, project_root: Path, manifest: Manifest) -> list[dict[str, Any]]: + """Scan tracked config dirs for ``rows/*/_config.yml`` not in manifest. -# --------------------------------------------------------------------------- -# Module-level helpers for secret encryption -# --------------------------------------------------------------------------- - -_ENCRYPTED_PREFIX = "KBC::" - + Paralleling :meth:`_find_untracked_configs` at the row level. A user + can drop a hand-crafted row directory under a tracked config's + ``rows/`` folder; this surfaces it so :meth:`diff` can flag it as + ``"added"`` and :meth:`push` can POST it via ``create_config_row``. -def _is_secret_key(key: str) -> bool: - """Check if a YAML key represents a secret (starts with #).""" - return isinstance(key, str) and key.startswith("#") - - -def _is_already_encrypted(value: Any) -> bool: - """Check if a value is already encrypted (KBC::*Secure::* prefix).""" - return isinstance(value, str) and value.startswith(_ENCRYPTED_PREFIX) - - -def _collect_secrets(obj: Any, path_prefix: str, result: dict[str, str]) -> None: - """Recursively collect unencrypted #-prefixed secret values. - - Builds a flat dict of {#path_key: plaintext_value} suitable for - the Encryption API. The Encryption API requires all keys to start - with '#'. - """ - if isinstance(obj, dict): - for key, value in obj.items(): - if _is_secret_key(key) and isinstance(value, str) and not _is_already_encrypted(value): - # Use the key directly for the encrypt API - encrypt_key = f"#{path_prefix}{key}" if path_prefix else key - result[encrypt_key] = value - elif isinstance(value, (dict, list)): - child_prefix = f"{path_prefix}{key}." if path_prefix else f"{key}." - _collect_secrets(value, child_prefix, result) - elif isinstance(obj, list): - for i, item in enumerate(obj): - child_prefix = f"{path_prefix}[{i}]." - _collect_secrets(item, child_prefix, result) - - -def _apply_encrypted(obj: Any, path_prefix: str, encrypted: dict[str, str]) -> None: - """Recursively apply encrypted values back into the configuration dict.""" - if isinstance(obj, dict): - for key in list(obj.keys()): - value = obj[key] - if _is_secret_key(key) and isinstance(value, str) and not _is_already_encrypted(value): - encrypt_key = f"#{path_prefix}{key}" if path_prefix else key - if encrypt_key in encrypted: - obj[key] = encrypted[encrypt_key] - elif isinstance(value, (dict, list)): - child_prefix = f"{path_prefix}{key}." if path_prefix else f"{key}." - _apply_encrypted(value, child_prefix, encrypted) - elif isinstance(obj, list): - for i, item in enumerate(obj): - child_prefix = f"{path_prefix}[{i}]." - _apply_encrypted(item, child_prefix, encrypted) - - -def _apply_encrypted_to_local(local: Any, pushed: Any) -> None: - """Copy encrypted secret values from pushed config back into local data. - - Walks both dicts in parallel. Where pushed has an encrypted value - for a #-key, replaces the local plaintext with it. - """ - if isinstance(local, dict) and isinstance(pushed, dict): - for key in local: - if key not in pushed: + Each entry contains ``component_id``, ``parent_config_id``, + ``row_name`` (from the loaded YAML), ``path`` (relative to the parent + config dir, e.g. ``rows/new-row``), and ``data`` (the loaded dict). + """ + added: list[dict[str, Any]] = [] + for cfg in manifest.configurations: + branch_path = self._find_branch_path(manifest, cfg.branch_id) + parent_dir = project_root / branch_path / cfg.path + rows_dir = parent_dir / "rows" + if not rows_dir.is_dir(): continue - if _is_secret_key(key) and _is_already_encrypted(pushed[key]): - local[key] = pushed[key] - elif isinstance(local[key], dict) and isinstance(pushed[key], dict): - _apply_encrypted_to_local(local[key], pushed[key]) - elif isinstance(local[key], list) and isinstance(pushed[key], list): - for i in range(min(len(local[key]), len(pushed[key]))): - _apply_encrypted_to_local(local[key][i], pushed[key][i]) + tracked_row_paths = {row.path for row in cfg.rows} + for row_subdir in rows_dir.iterdir(): + if not row_subdir.is_dir(): + continue + row_rel_path = f"rows/{row_subdir.name}" + if row_rel_path in tracked_row_paths: + continue + if not (row_subdir / CONFIG_FILENAME).exists(): + continue + local_data = self._read_config_file(row_subdir) + if local_data is None: + continue + added.append( + { + "component_id": cfg.component_id, + "parent_config_id": cfg.id, + "row_name": local_data.get("name", ""), + "path": row_rel_path, + "data": local_data, + } + ) + return added diff --git a/src/keboola_agent_cli/services/variables_service.py b/src/keboola_agent_cli/services/variables_service.py new file mode 100644 index 00000000..f7794824 --- /dev/null +++ b/src/keboola_agent_cli/services/variables_service.py @@ -0,0 +1,444 @@ +"""Variables service -- high-level abstraction over keboola.variables configs. + +Presents variables as a property you assign to any Keboola config, hiding the +fact that Keboola stores them server-side as separate ``keboola.variables`` +configurations with rows. Agents and users call: + + set_variables(parent_component, parent_config, {key: value, ...}) + +and the service handles create-if-missing, row update, parent linking, and +encryption of ``#``-prefixed secrets. No YAML authoring, no linking dance, no +keboola.variables-as-resource bookkeeping. +""" + +from __future__ import annotations + +import copy +import logging +from typing import Any + +from ..errors import ConfigError, KeboolaApiError +from ._encryption import encrypt_secrets_in_config +from .base import BaseService + +logger = logging.getLogger(__name__) + +VARIABLES_COMPONENT_ID = "keboola.variables" + + +class VariablesService(BaseService): + """Assign, read, and detach variables on any Keboola config. + + The backing ``keboola.variables`` configuration is an implementation detail + that callers shouldn't need to know about. On first set, a sibling config + named ``{parent_name}-vars`` is created; subsequent sets update the same + default row. ``clear`` unlinks the parent but does NOT delete the backing + config (it might be shared across configs). + """ + + def get_variables( + self, + alias: str, + component_id: str, + config_id: str, + branch_id: int | None = None, + ) -> dict[str, Any]: + """Return the current variable values assigned to ``{component_id}/{config_id}``. + + Returns a flat ``{name: value}`` dict (the ``keboola.variables`` row + structure is flattened). ``linked=False`` means the parent config has no + ``variables_id`` set -- no variables to report. + """ + projects = self.resolve_projects([alias]) + project = projects[alias] + client = self._client_factory(project.stack_url, project.token) + try: + parent = client.get_config_detail(component_id, config_id, branch_id=branch_id) + parent_configuration = parent.get("configuration") or {} + variables_id = parent_configuration.get("variables_id") + values_id = parent_configuration.get("variables_values_id") + + if not variables_id: + return { + "project_alias": alias, + "parent_component_id": component_id, + "parent_config_id": config_id, + "variables_id": None, + "values_id": None, + "values": {}, + "linked": False, + } + + vars_cfg = client.get_config_detail( + VARIABLES_COMPONENT_ID, variables_id, branch_id=branch_id + ) + target_row = self._resolve_values_row(vars_cfg, values_id) + values_dict: dict[str, str] = {} + if target_row: + for item in target_row.get("configuration", {}).get("values", []): + values_dict[item["name"]] = item["value"] + if not values_id: + values_id = target_row["id"] + + return { + "project_alias": alias, + "parent_component_id": component_id, + "parent_config_id": config_id, + "variables_id": variables_id, + "values_id": values_id, + "values": values_dict, + "linked": True, + } + finally: + client.close() + + def set_variables( + self, + alias: str, + component_id: str, + config_id: str, + variables: dict[str, str], + *, + replace: bool = False, + variables_id: str | None = None, + values_id: str | None = None, + branch_id: int | None = None, + allow_plaintext_fallback: bool = False, + ) -> dict[str, Any]: + """Assign variable values to ``{component_id}/{config_id}``. + + Creates a backing ``keboola.variables`` config + default row if the + parent has no ``variables_id`` set. Otherwise updates the already-linked + values row. With ``replace=False`` (default), values are merged with the + existing set; with ``replace=True``, the values array is overwritten + with exactly the provided dict. + + ``#``-prefixed keys are encrypted via the Encryption API before reaching + Storage (fail-closed unless ``allow_plaintext_fallback=True``). + + ``variables_id`` / ``values_id`` override the auto-discovery path and + can be used to attach the parent to a pre-existing variables config. + """ + if not variables: + raise ConfigError( + "set_variables requires at least one variable. Use --var KEY=VALUE (repeatable)." + ) + + projects = self.resolve_projects([alias]) + project = projects[alias] + client = self._client_factory(project.stack_url, project.token) + try: + parent = client.get_config_detail(component_id, config_id, branch_id=branch_id) + parent_name = parent.get("name", "") + parent_configuration = copy.deepcopy(parent.get("configuration") or {}) + + # project_id is needed for the Encryption API scope; not present on + # the config response, so fetch from verify_token. + project_id = client.verify_token().project_id + + linked_vars_id = variables_id or parent_configuration.get("variables_id") + linked_values_id = values_id or parent_configuration.get("variables_values_id") + action = "updated" + + if not linked_vars_id: + ( + linked_vars_id, + linked_values_id, + final_values, + ) = self._create_linked_variables( + client=client, + project_id=project_id, + parent_name=parent_name, + parent_component_id=component_id, + parent_config_id=config_id, + variables=variables, + branch_id=branch_id, + allow_plaintext_fallback=allow_plaintext_fallback, + ) + action = "created" + else: + linked_values_id, final_values = self._update_linked_variables( + client=client, + project_id=project_id, + variables_id=linked_vars_id, + values_id=linked_values_id, + variables=variables, + replace=replace, + branch_id=branch_id, + allow_plaintext_fallback=allow_plaintext_fallback, + ) + + # Ensure the parent config carries the link. Existing-linked path + # may no-op; auto-create path always writes. + parent_variables_id = parent_configuration.get("variables_id") + parent_values_id = parent_configuration.get("variables_values_id") + if parent_variables_id != linked_vars_id or parent_values_id != linked_values_id: + parent_configuration["variables_id"] = linked_vars_id + parent_configuration["variables_values_id"] = linked_values_id + client.update_config( + component_id=component_id, + config_id=config_id, + configuration=parent_configuration, + change_description="Linked variables via kbagent", + branch_id=branch_id, + ) + + encrypted_keys = sorted(k for k in variables if k.startswith("#")) + return { + "project_alias": alias, + "parent_component_id": component_id, + "parent_config_id": config_id, + "variables_id": linked_vars_id, + "values_id": linked_values_id, + "action": action, + "values": final_values, + "encrypted_keys": encrypted_keys, + } + finally: + client.close() + + def clear_variables( + self, + alias: str, + component_id: str, + config_id: str, + branch_id: int | None = None, + ) -> dict[str, Any]: + """Unlink variables from ``{component_id}/{config_id}``. + + Strips ``variables_id`` + ``variables_values_id`` from the parent config + and PUTs it. Does NOT delete the underlying ``keboola.variables`` config + -- it may be shared across configs, and deletion is a destructive op + the user should do explicitly via ``config delete``. + """ + projects = self.resolve_projects([alias]) + project = projects[alias] + client = self._client_factory(project.stack_url, project.token) + try: + parent = client.get_config_detail(component_id, config_id, branch_id=branch_id) + parent_configuration = copy.deepcopy(parent.get("configuration") or {}) + was_vars_id = parent_configuration.pop("variables_id", None) + was_values_id = parent_configuration.pop("variables_values_id", None) + + if was_vars_id or was_values_id: + client.update_config( + component_id=component_id, + config_id=config_id, + configuration=parent_configuration, + change_description="Unlinked variables via kbagent", + branch_id=branch_id, + ) + + return { + "project_alias": alias, + "parent_component_id": component_id, + "parent_config_id": config_id, + "was_linked": bool(was_vars_id), + "unlinked_variables_id": was_vars_id, + "unlinked_values_id": was_values_id, + } + finally: + client.close() + + # ------------------------------------------------------------------ + # internal helpers + # ------------------------------------------------------------------ + + def _create_linked_variables( + self, + *, + client: Any, + project_id: int, + parent_name: str, + parent_component_id: str, + parent_config_id: str, + variables: dict[str, str], + branch_id: int | None, + allow_plaintext_fallback: bool, + ) -> tuple[str, str, dict[str, str]]: + """Auto-create path: new variables config + default row, parent not yet linked.""" + var_name = (parent_name or parent_config_id) + "-vars" + schema = [{"name": k, "type": "string"} for k in variables] + + new_var_cfg = client.create_config( + component_id=VARIABLES_COMPONENT_ID, + name=var_name, + description=(f"Auto-created by kbagent for {parent_component_id}/{parent_config_id}"), + configuration={"variables": schema}, + branch_id=branch_id, + ) + variables_id = new_var_cfg["id"] + + row_config = self._build_encrypted_row_configuration( + client=client, + project_id=project_id, + variables=variables, + allow_plaintext_fallback=allow_plaintext_fallback, + ) + + new_row = client.create_config_row( + component_id=VARIABLES_COMPONENT_ID, + config_id=variables_id, + name="default", + configuration=row_config, + description="Auto-created default row by kbagent", + branch_id=branch_id, + ) + return variables_id, new_row["id"], dict(variables) + + def _update_linked_variables( + self, + *, + client: Any, + project_id: int, + variables_id: str, + values_id: str | None, + variables: dict[str, str], + replace: bool, + branch_id: int | None, + allow_plaintext_fallback: bool, + ) -> tuple[str, dict[str, str]]: + """Update path: parent already linked (or explicit --variables-id). Merge or replace.""" + vars_cfg = client.get_config_detail( + VARIABLES_COMPONENT_ID, variables_id, branch_id=branch_id + ) + target_row = self._resolve_values_row(vars_cfg, values_id) + + if target_row is None: + # Linked variables_id exists but no values row -- create the default. + row_config = self._build_encrypted_row_configuration( + client=client, + project_id=project_id, + variables=variables, + allow_plaintext_fallback=allow_plaintext_fallback, + ) + new_row = client.create_config_row( + component_id=VARIABLES_COMPONENT_ID, + config_id=variables_id, + name="default", + configuration=row_config, + description="Auto-created default row by kbagent", + branch_id=branch_id, + ) + self._extend_schema_if_new_keys( + client=client, + vars_cfg=vars_cfg, + variables=variables, + branch_id=branch_id, + ) + return new_row["id"], dict(variables) + + existing_values = target_row.get("configuration", {}).get("values", []) + existing_dict = {v["name"]: v["value"] for v in existing_values} + + if replace: + final_values = dict(variables) + else: + final_values = dict(existing_dict) + final_values.update(variables) + + # Encrypt only the NEW values -- existing #-keys are already KBC::- + # prefixed and collect_secrets skips already-encrypted entries. + row_config = self._build_encrypted_row_configuration( + client=client, + project_id=project_id, + variables=final_values, + allow_plaintext_fallback=allow_plaintext_fallback, + ) + + client.update_config_row( + component_id=VARIABLES_COMPONENT_ID, + config_id=variables_id, + row_id=target_row["id"], + configuration=row_config, + change_description="Updated via kbagent config variables-set", + branch_id=branch_id, + ) + + self._extend_schema_if_new_keys( + client=client, + vars_cfg=vars_cfg, + variables=final_values, + branch_id=branch_id, + ) + return target_row["id"], final_values + + @staticmethod + def _build_encrypted_row_configuration( + *, + client: Any, + project_id: int, + variables: dict[str, str], + allow_plaintext_fallback: bool, + ) -> dict[str, Any]: + """Shape a ``{values: [...]}`` row config and encrypt ``#``-prefixed entries. + + ``#``-prefixed names keep the prefix (the Encryption API and the + transformation runner both key off it). :func:`encrypt_secrets_in_config` + recognizes the ``{name, value}`` list shape directly, so no pre-flatten + dance is needed. + """ + row_config: dict[str, Any] = { + "values": [{"name": k, "value": v} for k, v in variables.items()], + } + encrypt_secrets_in_config( + client, + project_id, + VARIABLES_COMPONENT_ID, + row_config, + allow_plaintext_fallback=allow_plaintext_fallback, + ) + return row_config + + @staticmethod + def _resolve_values_row( + vars_cfg: dict[str, Any], values_id: str | None + ) -> dict[str, Any] | None: + """Return the row matching ``values_id``, or the first row (default).""" + rows = vars_cfg.get("rows", []) + if not rows: + return None + if values_id: + return next((r for r in rows if r["id"] == values_id), None) + return rows[0] + + def _extend_schema_if_new_keys( + self, + *, + client: Any, + vars_cfg: dict[str, Any], + variables: dict[str, str], + branch_id: int | None, + ) -> None: + """Add any brand-new keys to the variables config's schema. + + Keboola renders variables whose names appear in + ``configuration.variables`` (the schema) differently from stray keys in + the row ``values`` array. Keeping the schema in sync makes new vars + visible in the UI without a second round-trip. + """ + existing_schema = vars_cfg.get("configuration", {}).get("variables", []) + existing_names = {v["name"] for v in existing_schema} + new_names = {k.lstrip("#") for k in variables} - existing_names + if not new_names: + return + + updated_schema = list(existing_schema) + [ + {"name": n, "type": "string"} for n in sorted(new_names) + ] + new_configuration = copy.deepcopy(vars_cfg.get("configuration") or {}) + new_configuration["variables"] = updated_schema + + variables_id = vars_cfg["id"] + try: + client.update_config( + component_id=VARIABLES_COMPONENT_ID, + config_id=variables_id, + configuration=new_configuration, + change_description="Schema extended by kbagent", + branch_id=branch_id, + ) + except KeboolaApiError as exc: + # Schema sync is cosmetic (values still work without it). Log and + # continue rather than fail the whole operation. + logger.warning("Schema sync failed for variables config %s: %s", variables_id, exc) diff --git a/src/keboola_agent_cli/sync/config_format.py b/src/keboola_agent_cli/sync/config_format.py index 84ce9170..ef2d54fa 100644 --- a/src/keboola_agent_cli/sync/config_format.py +++ b/src/keboola_agent_cli/sync/config_format.py @@ -69,6 +69,31 @@ def _normalize_scripts(parameters: Any) -> Any: # Orchestrator-like components that have special handling ORCHESTRATOR_COMPONENTS: set[str] = {"keboola.orchestrator", "keboola.flow"} +# Row-bearing components whose `configuration` top-level keys do NOT fit the +# standard `parameters` / `storage` / `processors` shape. For these, the +# non-standard keys (e.g. `values` for variables, `code_content` for shared-code) +# are hoisted to the top level of the local YAML instead of being hidden inside +# `_configuration_extra`, so that humans and agents can edit them directly +# (matching `kbc push` convention — FIIA scaffold kit relies on this). +ROW_HOIST_COMPONENTS: set[str] = {"keboola.variables", "keboola.shared-code"} + +# Top-level keys in the local row YAML that are never part of the API +# `configuration` body. Used by `local_row_to_api` to separate editable payload +# keys from local metadata when the component is in ROW_HOIST_COMPONENTS. +_ROW_LOCAL_RESERVED_KEYS: frozenset[str] = frozenset( + { + "version", + "name", + "description", + "parameters", + "input", + "output", + "processors", + "_configuration_extra", + "_keboola", + } +) + def classify_component_type(api_type: str) -> str: """Map an API component type string to its filesystem directory name. @@ -187,7 +212,11 @@ def local_config_to_api( def api_row_to_local(row_data: dict[str, Any], component_id: str) -> dict[str, Any]: """Convert an API configuration row to a local row ``_config.yml``. - Follows the same promotion rules as :func:`api_config_to_local`. + Follows the same promotion rules as :func:`api_config_to_local`, with one + exception: for components in :data:`ROW_HOIST_COMPONENTS`, non-standard + top-level ``configuration`` keys (e.g. ``values`` for ``keboola.variables``) + are hoisted directly into the local YAML instead of being wrapped under + ``_configuration_extra``, so users can edit them naturally. """ configuration: dict[str, Any] = row_data.get("configuration") or {} @@ -212,7 +241,11 @@ def api_row_to_local(row_data: dict[str, Any], component_id: str) -> dict[str, A promoted_keys = {"parameters", "storage", "processors"} extras = {k: v for k, v in configuration.items() if k not in promoted_keys} if extras: - local["_configuration_extra"] = extras + if component_id in ROW_HOIST_COMPONENTS: + for key, value in extras.items(): + local[key] = value + else: + local["_configuration_extra"] = extras local["_keboola"] = { "component_id": component_id, @@ -227,8 +260,23 @@ def local_row_to_api( ) -> tuple[str, str, dict[str, Any]]: """Convert a local row ``_config.yml`` back to API format. + For components in :data:`ROW_HOIST_COMPONENTS`, hoisted top-level keys + (outside the reserved set) are pulled back into the API ``configuration`` + body. For all other components this behaves identically to + :func:`local_config_to_api`. + Returns: A tuple of ``(name, description, configuration_dict)``. """ - # Reuse the same logic -- the structure is identical - return local_config_to_api(row_yml) + keboola_meta: dict[str, Any] = row_yml.get("_keboola") or {} + component_id: str = keboola_meta.get("component_id", "") + + name, description, configuration = local_config_to_api(row_yml) + + if component_id in ROW_HOIST_COMPONENTS: + for key, value in row_yml.items(): + if key in _ROW_LOCAL_RESERVED_KEYS: + continue + configuration.setdefault(key, value) + + return name, description, configuration diff --git a/src/keboola_agent_cli/sync/diff_engine.py b/src/keboola_agent_cli/sync/diff_engine.py index a0adb5aa..92292112 100644 --- a/src/keboola_agent_cli/sync/diff_engine.py +++ b/src/keboola_agent_cli/sync/diff_engine.py @@ -22,27 +22,33 @@ class ConfigChange: - """Represents a single configuration change. + """Represents a single configuration or configuration-row change. ``change_type`` values: - - ``"added"`` -- new local config not yet in remote + - ``"added"`` -- new local config/row not yet in remote - ``"modified"`` -- local changed, remote unchanged (safe to push) - ``"remote_modified"`` -- remote changed, local unchanged (run pull) - ``"conflict"`` -- both sides changed since last pull - ``"deleted"`` -- local file removed, wants to delete from remote + + Row changes set ``is_row=True`` and carry ``parent_config_id`` so + ``sync push`` can dispatch them to the row-specific client methods + (``create_config_row`` / ``update_config_row`` / ``delete_config_row``). """ def __init__( self, change_type: str, component_id: str, - config_id: str, # empty string for new configs + config_id: str, # empty string for new configs/rows config_name: str, path: str, local_data: dict[str, Any] | None = None, remote_data: dict[str, Any] | None = None, details: list[str] | None = None, + is_row: bool = False, + parent_config_id: str = "", ): self.change_type = change_type self.component_id = component_id @@ -52,10 +58,12 @@ def __init__( self.local_data = local_data self.remote_data = remote_data self.details = details or [] + self.is_row = is_row + self.parent_config_id = parent_config_id def to_dict(self) -> dict[str, Any]: """Serialize to dict for JSON output.""" - return { + data: dict[str, Any] = { "change_type": self.change_type, "component_id": self.component_id, "config_id": self.config_id, @@ -63,6 +71,10 @@ def to_dict(self) -> dict[str, Any]: "path": self.path, "details": self.details, } + if self.is_row: + data["is_row"] = True + data["parent_config_id"] = self.parent_config_id + return data # --------------------------------------------------------------------------- @@ -414,3 +426,135 @@ def compute_changeset( ) return changes + + +def compute_row_changeset( + local_rows: list[dict[str, Any]], + remote_rows: dict[str, dict[str, Any]], + tracked_row_keys: set[str] | None = None, + base_hashes: dict[str, str] | None = None, +) -> list[ConfigChange]: + """3-way diff for configuration rows, parallel to :func:`compute_changeset`. + + Row keys are ``"{component_id}/{parent_config_id}/rows/{row_id}"`` so they + cannot collide with parent-config keys. Results carry ``is_row=True`` and + ``parent_config_id`` so ``sync push`` can route them to + ``client.create_config_row`` / ``update_config_row`` / ``delete_config_row``. + + Args: + local_rows: List of dicts with keys: + ``component_id``, ``parent_config_id``, ``row_id``, + ``row_name``, ``path``, ``data``. + remote_rows: Dict keyed by ``"{component_id}/{parent_config_id}/rows/{row_id}"`` + with API row data (already converted to local format via + :func:`api_row_to_local`). + tracked_row_keys: Optional set of row keys from the manifest. + Only manifest-tracked remote rows can be flagged as ``"deleted"``. + base_hashes: Optional dict of row_key -> normalized config hash at last + pull time. Enables 3-way diff (same semantics as parent configs). + + Returns: + List of :class:`ConfigChange` objects, each with ``is_row=True``. + """ + changes: list[ConfigChange] = [] + seen_remote_keys: set[str] = set() + + for entry in local_rows: + component_id: str = entry["component_id"] + parent_config_id: str = entry.get("parent_config_id", "") + row_id: str = entry.get("row_id", "") + row_name: str = entry.get("row_name", "") + path: str = entry.get("path", "") + local_data: dict[str, Any] = entry.get("data", {}) + + remote_key = ( + f"{component_id}/{parent_config_id}/rows/{row_id}" + if row_id and parent_config_id + else "" + ) + + # New row (no id yet, or not in remote) + if not row_id or remote_key not in remote_rows: + changes.append( + ConfigChange( + change_type="added", + component_id=component_id, + config_id=row_id, + config_name=row_name, + path=path, + local_data=local_data, + is_row=True, + parent_config_id=parent_config_id, + ) + ) + if remote_key: + seen_remote_keys.add(remote_key) + continue + + seen_remote_keys.add(remote_key) + remote_data: dict[str, Any] = remote_rows[remote_key] + + local_h = config_hash(local_data) + remote_h = config_hash(remote_data) + + if local_h == remote_h: + continue + + base_h = (base_hashes or {}).get(remote_key) + if base_h is not None: + local_changed = local_h != base_h + remote_changed = remote_h != base_h + else: + local_changed = True + remote_changed = False + + if local_changed and remote_changed: + change_type = "conflict" + details = deep_diff(local_data, remote_data) + elif remote_changed and not local_changed: + change_type = "remote_modified" + details = deep_diff(remote_data, local_data) + else: + change_type = "modified" + details = deep_diff(local_data, remote_data) + + changes.append( + ConfigChange( + change_type=change_type, + component_id=component_id, + config_id=row_id, + config_name=row_name, + path=path, + local_data=local_data, + remote_data=remote_data, + details=details, + is_row=True, + parent_config_id=parent_config_id, + ) + ) + + # Deleted rows: tracked in manifest but missing from local filesystem + for remote_key, remote_data in remote_rows.items(): + if remote_key in seen_remote_keys: + continue + if tracked_row_keys is not None and remote_key not in tracked_row_keys: + continue + + # remote_key shape: "{component_id}/{parent_config_id}/rows/{row_id}" + comp_and_parent, _, row_id = remote_key.rpartition("/rows/") + component_id, _, parent_config_id = comp_and_parent.partition("/") + + changes.append( + ConfigChange( + change_type="deleted", + component_id=component_id, + config_id=row_id, + config_name=remote_data.get("name", ""), + path="", + remote_data=remote_data, + is_row=True, + parent_config_id=parent_config_id, + ) + ) + + return changes diff --git a/src/keboola_agent_cli/sync/manifest.py b/src/keboola_agent_cli/sync/manifest.py index 49a48058..81749bde 100644 --- a/src/keboola_agent_cli/sync/manifest.py +++ b/src/keboola_agent_cli/sync/manifest.py @@ -70,12 +70,20 @@ class ManifestBranch(BaseModel): class ManifestConfigRow(BaseModel): - """A single configuration row reference.""" + """A single configuration row reference. + + ``metadata`` mirrors :class:`ManifestConfiguration.metadata` and stores + pull-time hashes (``pull_hash``, ``pull_config_hash``) so the row-level + diff can distinguish local-changed, remote-changed, and conflict states. + Older manifests that lack the field load with an empty dict and upgrade + on the next successful pull. + """ model_config = ConfigDict(populate_by_name=True, extra="allow") id: str path: str + metadata: dict[str, Any] = Field(default_factory=dict) class ManifestConfiguration(BaseModel): @@ -97,7 +105,7 @@ class ManifestConfiguration(BaseModel): class Manifest(BaseModel): - """Root model for .keboola/manifest.json (schema version 2).""" + """Root model for .keboola/manifest.json (schema version 3).""" model_config = ConfigDict(populate_by_name=True, extra="allow") diff --git a/tests/test_client.py b/tests/test_client.py index 72cdebbc..6926ff75 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1,7 +1,8 @@ """Tests for KeboolaClient - verify_token, retries, timeouts, error handling.""" +import json from unittest.mock import patch -from urllib.parse import quote +from urllib.parse import parse_qs, quote import httpx import pytest @@ -1647,6 +1648,206 @@ def test_delete_config_with_branch(self, httpx_mock) -> None: client.delete_config("keboola.sandboxes", "cfg-1", branch_id=200) +class TestConfigRowMethods: + """Tests for create_config_row / update_config_row / delete_config_row. + + These methods are wired into ``sync push`` for row-level deployment (FIIA's + primary use case: deploying ``keboola.variables`` values rows). They were + previously unreachable from the service layer -- these tests lock the + HTTP contract: URL, method, form-encoding, ``configuration`` JSON-stringified. + """ + + TOKEN = "901-10493007-VDtlEDWDF6Tx5V8jjE8FshFlqM0Hl0c08KHqpt0k" + + @staticmethod + def _parse_form_body(request: httpx.Request) -> dict[str, str]: + """Parse form-encoded request body to a flat str->str dict.""" + parsed = parse_qs(request.content.decode("utf-8"), keep_blank_values=True) + return {k: v[0] for k, v in parsed.items()} + + def test_create_config_row_no_branch(self, httpx_mock) -> None: + """POST to /v2/storage/components/{c}/configs/{id}/rows with configuration JSON-stringified.""" + httpx_mock.add_response( + url="https://connection.keboola.com/v2/storage/components/keboola.variables/configs/vars-1/rows", + method="POST", + json={"id": "row-new", "name": "Main", "configuration": {"values": []}}, + status_code=201, + ) + + with KeboolaClient(stack_url="https://connection.keboola.com", token=self.TOKEN) as client: + result = client.create_config_row( + component_id="keboola.variables", + config_id="vars-1", + name="Main", + configuration={"values": [{"name": "year_start", "value": "2016"}]}, + description="default values", + ) + + assert result["id"] == "row-new" + body = self._parse_form_body(httpx_mock.get_requests()[0]) + assert body["name"] == "Main" + assert body["description"] == "default values" + # configuration must be JSON-stringified, not nested form-fields + assert json.loads(body["configuration"]) == { + "values": [{"name": "year_start", "value": "2016"}] + } + + def test_create_config_row_with_branch(self, httpx_mock) -> None: + """branch_id routes the POST to /v2/storage/branch/{id}/components/.../rows.""" + httpx_mock.add_response( + url="https://connection.keboola.com/v2/storage/branch/200/components/keboola.variables/configs/vars-1/rows", + method="POST", + json={"id": "row-new"}, + status_code=201, + ) + + with KeboolaClient(stack_url="https://connection.keboola.com", token=self.TOKEN) as client: + client.create_config_row( + component_id="keboola.variables", + config_id="vars-1", + name="dev", + configuration={"values": []}, + branch_id=200, + ) + + def test_create_config_row_api_error_propagates(self, httpx_mock) -> None: + """HTTP 400 from the row-create endpoint surfaces as KeboolaApiError.""" + httpx_mock.add_response( + url="https://connection.keboola.com/v2/storage/components/keboola.variables/configs/vars-1/rows", + method="POST", + json={"error": "Validation", "code": "validation", "message": "bad row"}, + status_code=400, + ) + + with KeboolaClient(stack_url="https://connection.keboola.com", token=self.TOKEN) as client: + with pytest.raises(KeboolaApiError) as excinfo: + client.create_config_row( + component_id="keboola.variables", + config_id="vars-1", + name="bad", + configuration={"values": "not-a-list"}, + ) + assert excinfo.value.status_code == 400 + + def test_update_config_row_full_payload(self, httpx_mock) -> None: + """PUT with all fields: name, description, configuration, changeDescription.""" + httpx_mock.add_response( + url="https://connection.keboola.com/v2/storage/components/keboola.variables/configs/vars-1/rows/row-1", + method="PUT", + json={"id": "row-1", "name": "Updated"}, + status_code=200, + ) + + with KeboolaClient(stack_url="https://connection.keboola.com", token=self.TOKEN) as client: + client.update_config_row( + component_id="keboola.variables", + config_id="vars-1", + row_id="row-1", + name="Updated", + configuration={"values": [{"name": "year", "value": "2025"}]}, + description="new", + change_description="Deployed via kbagent sync push", + ) + + body = self._parse_form_body(httpx_mock.get_requests()[0]) + assert body["name"] == "Updated" + assert body["description"] == "new" + assert body["changeDescription"] == "Deployed via kbagent sync push" + assert json.loads(body["configuration"]) == {"values": [{"name": "year", "value": "2025"}]} + + def test_update_config_row_partial_omits_unset_fields(self, httpx_mock) -> None: + """None-valued fields are NOT included in the form body (partial update).""" + httpx_mock.add_response( + url="https://connection.keboola.com/v2/storage/components/keboola.variables/configs/vars-1/rows/row-1", + method="PUT", + json={"id": "row-1"}, + status_code=200, + ) + + with KeboolaClient(stack_url="https://connection.keboola.com", token=self.TOKEN) as client: + client.update_config_row( + component_id="keboola.variables", + config_id="vars-1", + row_id="row-1", + configuration={"values": []}, + # name, description, change_description intentionally omitted + ) + + body = self._parse_form_body(httpx_mock.get_requests()[0]) + assert "configuration" in body + assert "name" not in body + assert "description" not in body + assert "changeDescription" not in body + + def test_update_config_row_with_branch(self, httpx_mock) -> None: + """branch_id routes the PUT to the dev-branch endpoint.""" + httpx_mock.add_response( + url="https://connection.keboola.com/v2/storage/branch/200/components/keboola.variables/configs/vars-1/rows/row-1", + method="PUT", + json={"id": "row-1"}, + status_code=200, + ) + + with KeboolaClient(stack_url="https://connection.keboola.com", token=self.TOKEN) as client: + client.update_config_row( + component_id="keboola.variables", + config_id="vars-1", + row_id="row-1", + name="dev-version", + branch_id=200, + ) + + def test_delete_config_row_no_branch(self, httpx_mock) -> None: + """DELETE to /v2/storage/components/.../rows/{row_id} returns None.""" + httpx_mock.add_response( + url="https://connection.keboola.com/v2/storage/components/keboola.variables/configs/vars-1/rows/row-1", + method="DELETE", + status_code=204, + ) + + with KeboolaClient(stack_url="https://connection.keboola.com", token=self.TOKEN) as client: + result = client.delete_config_row( + component_id="keboola.variables", + config_id="vars-1", + row_id="row-1", + ) + assert result is None + + def test_delete_config_row_with_branch(self, httpx_mock) -> None: + """branch_id routes the DELETE to the dev-branch endpoint.""" + httpx_mock.add_response( + url="https://connection.keboola.com/v2/storage/branch/200/components/keboola.variables/configs/vars-1/rows/row-1", + method="DELETE", + status_code=204, + ) + + with KeboolaClient(stack_url="https://connection.keboola.com", token=self.TOKEN) as client: + client.delete_config_row( + component_id="keboola.variables", + config_id="vars-1", + row_id="row-1", + branch_id=200, + ) + + def test_delete_config_row_not_found_raises(self, httpx_mock) -> None: + """HTTP 404 on DELETE surfaces as KeboolaApiError with status 404.""" + httpx_mock.add_response( + url="https://connection.keboola.com/v2/storage/components/keboola.variables/configs/vars-1/rows/missing", + method="DELETE", + json={"error": "Not found", "code": "notFound"}, + status_code=404, + ) + + with KeboolaClient(stack_url="https://connection.keboola.com", token=self.TOKEN) as client: + with pytest.raises(KeboolaApiError) as excinfo: + client.delete_config_row( + component_id="keboola.variables", + config_id="vars-1", + row_id="missing", + ) + assert excinfo.value.status_code == 404 + + class TestLoadWorkspaceTablesPreserve: """Tests for load_workspace_tables() preserve parameter.""" diff --git a/tests/test_e2e.py b/tests/test_e2e.py index ba17f52c..00579306 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -2731,6 +2731,282 @@ def test_sync_workflow(self) -> None: ) assert data["status"] == "ok" + def test_sync_push_variable_row_round_trip(self) -> None: + """PR1 P0-1 acceptance: edit a keboola.variables values row, push, pull back. + + Locks the row-deploy contract: after sync push, the API's row + ``configuration`` dict must equal what we wrote locally (byte-equal + deep comparison). Creates + cleans up a dedicated ``keboola.variables`` + config + row so the test is idempotent across runs. + """ + import yaml as _yaml + + from keboola_agent_cli.client import KeboolaClient + from keboola_agent_cli.constants import CONFIG_FILENAME + + component_id = "keboola.variables" + var_cfg: dict = {} + row_id: str = "" + + # Wrap setup + body in a single try/finally so the cleanup still + # runs if create_config_row fails after create_config succeeded -- + # otherwise we leak a variables config on every failed run. + try: + with KeboolaClient(stack_url=self.url, token=self.token) as api: + var_cfg = api.create_config( + component_id=component_id, + name=f"e2e-pr1-{RUN_ID}", + description="FIIA row-push E2E fixture", + configuration={ + "variables": [ + {"name": "year_start", "type": "string"}, + {"name": "region", "type": "string"}, + ] + }, + ) + row = api.create_config_row( + component_id=component_id, + config_id=var_cfg["id"], + name="main", + configuration={ + "values": [ + {"name": "year_start", "value": "2016"}, + {"name": "region", "value": "eu"}, + ] + }, + ) + row_id = row["id"] + + # --- step A: sync init + pull --- + _step("6a", "sync init + pull (row-push setup)") + self._run_ok( + "sync", + "init", + "--project", + self.alias, + "--directory", + str(self.project_dir), + ) + self._run_ok( + "sync", + "pull", + "--project", + self.alias, + "--directory", + str(self.project_dir), + ) + + # --- step B: locate the row YAML file on disk --- + row_files = [ + p + for p in self.project_dir.rglob(CONFIG_FILENAME) + if "rows" in p.relative_to(self.project_dir).parts + and _yaml.safe_load(p.read_text(encoding="utf-8")).get("_keboola", {}).get("row_id") + == row_id + ] + assert len(row_files) == 1, f"Row YAML not found after pull. Candidates: {row_files}" + row_file = row_files[0] + + # --- step C: edit the row values locally (FIIA's primary use case) --- + _step("6b", "edit values row locally + sync push") + local_data = _yaml.safe_load(row_file.read_text(encoding="utf-8")) + # Hoisted top-level `values` key (see config_format.ROW_HOIST_COMPONENTS). + assert "values" in local_data, f"Expected hoisted 'values' key: {list(local_data)}" + local_data["values"] = [ + {"name": "year_start", "value": "2025"}, + {"name": "region", "value": "us-west"}, + ] + row_file.write_text( + _yaml.dump(local_data, default_flow_style=False, sort_keys=False), + encoding="utf-8", + ) + + push_data = self._run_ok( + "sync", + "push", + "--project", + self.alias, + "--directory", + str(self.project_dir), + ) + push_result = push_data["data"] + assert push_result["status"] == "pushed" + assert push_result["updated"] == 1, f"Expected 1 update (the row), got {push_result}" + + # --- step D: pull fresh state back and assert byte-equal --- + _step("6c", "sync pull + verify row round-trip byte-equal") + with KeboolaClient(stack_url=self.url, token=self.token) as api: + remote_row = api.get_config_detail( + component_id=component_id, + config_id=var_cfg["id"], + ) + remote_rows = remote_row.get("rows", []) + updated_row = next(r for r in remote_rows if r["id"] == row_id) + assert updated_row["configuration"] == { + "values": [ + {"name": "year_start", "value": "2025"}, + {"name": "region", "value": "us-west"}, + ] + } + finally: + # --- cleanup: delete the variables config we created --- + # Guard on var_cfg["id"] so a failure before create_config returned + # doesn't turn into a KeyError inside the cleanup handler. + cfg_id = var_cfg.get("id") if var_cfg else None + if cfg_id: + try: + with KeboolaClient(stack_url=self.url, token=self.token) as api: + api.delete_config(component_id=component_id, config_id=cfg_id) + except Exception as exc: + print(f" [cleanup] Failed to delete {component_id}/{cfg_id}: {exc}") + + def test_config_variables_round_trip(self) -> None: + """CLAUDE.md rule 16: every new CLI command needs an E2E test. + + Exercises ``config variables-{set,get,clear}`` end-to-end against a + real parent config: auto-create path, merge path, replace path, + readback, and clear. Locks the happy-path contract so agents can + trust the response shape from real Storage API responses (not just + mocks). Cleans up both the parent test config and the auto-created + ``keboola.variables`` sibling. + """ + parent_cfg: dict = {} + auto_vars_id: str | None = None + try: + with KeboolaClient(stack_url=self.url, token=self.token) as api: + parent_cfg = api.create_config( + component_id=TEST_COMPONENT_ID, + name=f"{RUN_ID}-vars-parent", + description="E2E variables round-trip parent config", + configuration={"parameters": {"db": {"host": "test.example.com"}}}, + ) + parent_id = str(parent_cfg["id"]) + + # --- step A: variables-set (AUTO-CREATE) --- + _step("7a", "config variables-set (auto-create path)") + data = self._run_ok( + "config", + "variables-set", + "--project", + self.alias, + "--component-id", + TEST_COMPONENT_ID, + "--config-id", + parent_id, + "--var", + "year_start=2016", + "--var", + "region=eu", + )["data"] + assert data["action"] == "created" + assert data["values"] == {"year_start": "2016", "region": "eu"} + auto_vars_id = data["variables_id"] + assert auto_vars_id, "auto-create path must return a variables_id" + + # --- step B: variables-get (readback) --- + _step("7b", "config variables-get (readback after set)") + data = self._run_ok( + "config", + "variables-get", + "--project", + self.alias, + "--component-id", + TEST_COMPONENT_ID, + "--config-id", + parent_id, + )["data"] + assert data["linked"] is True + assert data["values"] == {"year_start": "2016", "region": "eu"} + assert data["variables_id"] == auto_vars_id + + # --- step C: variables-set (MERGE) --- + _step("7c", "config variables-set (merge: adds year_end)") + data = self._run_ok( + "config", + "variables-set", + "--project", + self.alias, + "--component-id", + TEST_COMPONENT_ID, + "--config-id", + parent_id, + "--var", + "year_end=2024", + )["data"] + assert data["action"] == "updated" + assert data["values"] == { + "year_start": "2016", + "region": "eu", + "year_end": "2024", + } + + # --- step D: variables-set --replace --- + _step("7d", "config variables-set --replace (drops prior keys)") + data = self._run_ok( + "config", + "variables-set", + "--project", + self.alias, + "--component-id", + TEST_COMPONENT_ID, + "--config-id", + parent_id, + "--var", + "only_key=only_value", + "--replace", + )["data"] + assert data["values"] == {"only_key": "only_value"} + + # --- step E: variables-clear --- + _step("7e", "config variables-clear (unlink)") + data = self._run_ok( + "config", + "variables-clear", + "--project", + self.alias, + "--component-id", + TEST_COMPONENT_ID, + "--config-id", + parent_id, + "--yes", + )["data"] + assert data["was_linked"] is True + assert data["unlinked_variables_id"] == auto_vars_id + + # Post-clear: variables-get must report linked=False + data = self._run_ok( + "config", + "variables-get", + "--project", + self.alias, + "--component-id", + TEST_COMPONENT_ID, + "--config-id", + parent_id, + )["data"] + assert data["linked"] is False + assert data["values"] == {} + finally: + with KeboolaClient(stack_url=self.url, token=self.token) as api: + parent_id_cleanup = parent_cfg.get("id") if parent_cfg else None + if parent_id_cleanup: + try: + api.delete_config( + component_id=TEST_COMPONENT_ID, config_id=str(parent_id_cleanup) + ) + except Exception as exc: + print( + f" [cleanup] Failed to delete " + f"{TEST_COMPONENT_ID}/{parent_id_cleanup}: {exc}" + ) + if auto_vars_id: + try: + api.delete_config(component_id="keboola.variables", config_id=auto_vars_id) + except Exception as exc: + print( + f" [cleanup] Failed to delete keboola.variables/{auto_vars_id}: {exc}" + ) + # --------------------------------------------------------------------------- # Tool command tests (requires MCP server) diff --git a/tests/test_encryption.py b/tests/test_encryption.py new file mode 100644 index 00000000..2bbf7d0e --- /dev/null +++ b/tests/test_encryption.py @@ -0,0 +1,204 @@ +"""Unit tests for :mod:`keboola_agent_cli.services._encryption` helpers. + +Focus: the row-hoisted ``{"name": "#x", "value": "..."}`` list-element shape +used by ``keboola.variables`` / ``keboola.shared-code``. The dict-key shape is +already exercised via ``test_sync_encrypt.py`` and service-level tests. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock + +import pytest + +from keboola_agent_cli.errors import KeboolaApiError +from keboola_agent_cli.services._encryption import ( + apply_encrypted, + apply_encrypted_to_local, + collect_secrets, + encrypt_secrets_in_config, +) + + +class TestCollectSecretsNameValueShape: + """``collect_secrets`` must find ``#``-prefixed names in ``values: [{name,value}]``.""" + + def test_single_secret_in_values_list(self) -> None: + config = {"values": [{"name": "#api_key", "value": "plaintext-secret"}]} + result: dict[str, str] = {} + collect_secrets(config, "", result) + + assert result == {"#values.[0].#api_key": "plaintext-secret"} + + def test_mixed_secret_and_regular_entries(self) -> None: + config = { + "values": [ + {"name": "#api_key", "value": "s1"}, + {"name": "regular", "value": "public"}, + {"name": "#token", "value": "s2"}, + ] + } + result: dict[str, str] = {} + collect_secrets(config, "", result) + + assert result == { + "#values.[0].#api_key": "s1", + "#values.[2].#token": "s2", + } + + def test_skips_already_encrypted(self) -> None: + config = { + "values": [ + {"name": "#api_key", "value": "KBC::ProjectSecure::abc"}, + {"name": "#token", "value": "plaintext"}, + ] + } + result: dict[str, str] = {} + collect_secrets(config, "", result) + + assert result == {"#values.[1].#token": "plaintext"} + + def test_ignores_non_name_value_dicts_in_list(self) -> None: + """Dicts in lists that don't match the shape fall through to generic recursion.""" + config = {"rows": [{"#key": "deep-secret"}]} + result: dict[str, str] = {} + collect_secrets(config, "", result) + + assert result == {"#rows.[0].#key": "deep-secret"} + + def test_ignores_value_when_not_string(self) -> None: + config = {"values": [{"name": "#api_key", "value": 42}]} + result: dict[str, str] = {} + collect_secrets(config, "", result) + + assert result == {} + + +class TestApplyEncryptedNameValueShape: + """``apply_encrypted`` must write ciphertext back into the matching list entry.""" + + def test_replaces_plaintext_with_ciphertext(self) -> None: + config = {"values": [{"name": "#api_key", "value": "plaintext-secret"}]} + encrypted = {"#values.[0].#api_key": "KBC::ProjectSecure::xyz"} + + apply_encrypted(config, "", encrypted) + + assert config == {"values": [{"name": "#api_key", "value": "KBC::ProjectSecure::xyz"}]} + + def test_only_replaces_targeted_entries(self) -> None: + config = { + "values": [ + {"name": "#api_key", "value": "p1"}, + {"name": "regular", "value": "public"}, + {"name": "#token", "value": "p2"}, + ] + } + encrypted = { + "#values.[0].#api_key": "KBC::ProjectSecure::a", + "#values.[2].#token": "KBC::ProjectSecure::b", + } + + apply_encrypted(config, "", encrypted) + + assert config["values"][0]["value"] == "KBC::ProjectSecure::a" + assert config["values"][1]["value"] == "public" + assert config["values"][2]["value"] == "KBC::ProjectSecure::b" + + +class TestApplyEncryptedToLocalNameValueShape: + """``apply_encrypted_to_local`` must mirror ciphertext back into local state.""" + + def test_copies_ciphertext_for_matching_name(self) -> None: + local = {"values": [{"name": "#api_key", "value": "plaintext"}]} + pushed = {"values": [{"name": "#api_key", "value": "KBC::ProjectSecure::xyz"}]} + + apply_encrypted_to_local(local, pushed) + + assert local == {"values": [{"name": "#api_key", "value": "KBC::ProjectSecure::xyz"}]} + + def test_skips_when_names_diverge(self) -> None: + """Defensive: if server reordered or renamed, don't blindly overwrite.""" + local = {"values": [{"name": "#api_key", "value": "plaintext"}]} + pushed = {"values": [{"name": "#different", "value": "KBC::ProjectSecure::xyz"}]} + + apply_encrypted_to_local(local, pushed) + + assert local["values"][0]["value"] == "plaintext" + + def test_skips_when_pushed_value_is_not_encrypted(self) -> None: + local = {"values": [{"name": "#api_key", "value": "plaintext"}]} + pushed = {"values": [{"name": "#api_key", "value": "still-plaintext"}]} + + apply_encrypted_to_local(local, pushed) + + assert local["values"][0]["value"] == "plaintext" + + +class TestEncryptSecretsInConfigNameValueShape: + """End-to-end: ``encrypt_secrets_in_config`` round-trips the hoisted shape.""" + + def test_roundtrip_with_mocked_client(self) -> None: + client = MagicMock() + client.encrypt_values.return_value = { + "#values.[0].#api_key": "KBC::ProjectSecure::ciphertext" + } + + config = {"values": [{"name": "#api_key", "value": "plaintext"}]} + result = encrypt_secrets_in_config( + client=client, + project_id=901, + component_id="keboola.variables", + configuration=config, + ) + + client.encrypt_values.assert_called_once() + call_kwargs = client.encrypt_values.call_args.kwargs + assert call_kwargs["project_id"] == 901 + assert call_kwargs["component_id"] == "keboola.variables" + assert call_kwargs["data"] == {"#values.[0].#api_key": "plaintext"} + + assert result["values"][0]["value"] == "KBC::ProjectSecure::ciphertext" + + def test_fail_closed_on_encryption_failure(self) -> None: + client = MagicMock() + client.encrypt_values.side_effect = RuntimeError("api down") + + config = {"values": [{"name": "#api_key", "value": "plaintext"}]} + + with pytest.raises(KeboolaApiError) as exc_info: + encrypt_secrets_in_config( + client=client, + project_id=901, + component_id="keboola.variables", + configuration=config, + ) + + assert exc_info.value.error_code == "ENCRYPTION_FAILED" + # plaintext must not be in the config after the raise path either + # (it's in place, but the caller must not push it — that's the contract) + + def test_noop_when_no_secrets(self) -> None: + client = MagicMock() + config = {"values": [{"name": "regular", "value": "public"}]} + + encrypt_secrets_in_config( + client=client, + project_id=901, + component_id="keboola.variables", + configuration=config, + ) + + client.encrypt_values.assert_not_called() + + def test_noop_when_all_already_encrypted(self) -> None: + client = MagicMock() + config = {"values": [{"name": "#api_key", "value": "KBC::ProjectSecure::already"}]} + + encrypt_secrets_in_config( + client=client, + project_id=901, + component_id="keboola.variables", + configuration=config, + ) + + client.encrypt_values.assert_not_called() diff --git a/tests/test_sync_cli.py b/tests/test_sync_cli.py index 082079ad..3b0b7a8c 100644 --- a/tests/test_sync_cli.py +++ b/tests/test_sync_cli.py @@ -980,6 +980,220 @@ def test_sync_push_no_changes_human(self, tmp_path: Path) -> None: assert result.exit_code == 0, f"Exit code {result.exit_code}: {result.output}" assert "No changes to push" in result.output + def test_sync_push_with_row_changes_json(self, tmp_path: Path) -> None: + """JSON output reflects row-level push results (P0-1). + + Mocks a push that deploys one new row, one updated row, one deleted + row; asserts the ``pushed_details`` entries preserve ``is_row`` + + ``parent_config_id`` so ``--json`` consumers can distinguish row ops + from parent-config ops. + """ + config_dir = tmp_path / "config" + config_dir.mkdir() + + store = _setup_config( + config_dir, + {"prod": {"token": TEST_TOKEN}}, + ) + + mock_sync = _make_sync_service_mock() + mock_sync.push.return_value = { + "status": "pushed", + "created": 1, + "updated": 1, + "deleted": 1, + "errors": [], + "pushed_details": [ + { + "change_type": "added", + "component_id": "keboola.variables", + "config_id": "row-new", + "config_name": "new-row", + "path": "values/new", + "is_row": True, + "parent_config_id": "vars-1", + }, + { + "change_type": "modified", + "component_id": "keboola.variables", + "config_id": "row-1", + "config_name": "main", + "path": "values/main", + "is_row": True, + "parent_config_id": "vars-1", + }, + { + "change_type": "deleted", + "component_id": "keboola.variables", + "config_id": "row-old", + "config_name": "old", + "path": "values/old", + "is_row": True, + "parent_config_id": "vars-1", + }, + ], + } + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProjService, + patch("keboola_agent_cli.cli.SyncService") as MockSyncService, + ): + MockStore.return_value = store + MockProjService.return_value = ProjectService(config_store=store) + MockSyncService.return_value = mock_sync + + result = runner.invoke( + app, + [ + "--json", + "sync", + "push", + "--project", + "prod", + "--directory", + str(tmp_path), + ], + ) + + assert result.exit_code == 0, f"Exit code {result.exit_code}: {result.output}" + output = json.loads(result.output) + data = output["data"] + assert data["status"] == "pushed" + assert data["created"] == 1 + assert data["updated"] == 1 + assert data["deleted"] == 1 + # Row changes carry is_row + parent_config_id so downstream agents can + # distinguish row ops from parent config ops in the response. + row_details = [d for d in data["pushed_details"] if d.get("is_row")] + assert len(row_details) == 3 + assert all(d["parent_config_id"] == "vars-1" for d in row_details) + + def test_sync_push_row_encryption_failure_exits_nonzero(self, tmp_path: Path) -> None: + """Encryption failure surfaced by the service exits non-zero (exit 1). + + Bundled P1-5 contract: if the service raises ``KeboolaApiError`` with + ``ENCRYPTION_FAILED`` (e.g. Encryption API unreachable and + ``--allow-plaintext-on-encrypt-failure`` not set), the CLI maps it to + exit 1 and emits the error code in the JSON response. + """ + from keboola_agent_cli.errors import KeboolaApiError + + config_dir = tmp_path / "config" + config_dir.mkdir() + + store = _setup_config( + config_dir, + {"prod": {"token": TEST_TOKEN}}, + ) + + mock_sync = _make_sync_service_mock() + mock_sync.push.side_effect = KeboolaApiError( + message="Encryption failed for keboola.variables: network error", + status_code=0, + error_code="ENCRYPTION_FAILED", + ) + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProjService, + patch("keboola_agent_cli.cli.SyncService") as MockSyncService, + ): + MockStore.return_value = store + MockProjService.return_value = ProjectService(config_store=store) + MockSyncService.return_value = mock_sync + + result = runner.invoke( + app, + [ + "--json", + "sync", + "push", + "--project", + "prod", + "--directory", + str(tmp_path), + ], + ) + + # ENCRYPTION_FAILED is an "everything else" error_code in + # map_error_to_exit_code -> exit 1 (general). Lock that contract. + assert result.exit_code == 1 + output = json.loads(result.output) + assert output["status"] == "error" + assert output["error"]["code"] == "ENCRYPTION_FAILED" + + def test_sync_push_with_row_changes_human(self, tmp_path: Path) -> None: + """Human-mode row push prints per-row action labels + summary counts. + + Complements :meth:`test_sync_push_with_row_changes_json` so the CLI + layer has both output modes covered per best_practices.md §5. Strips + ANSI so the assertion is stable on CI. + """ + config_dir = tmp_path / "config" + config_dir.mkdir() + + store = _setup_config( + config_dir, + {"prod": {"token": TEST_TOKEN}}, + ) + + mock_sync = _make_sync_service_mock() + mock_sync.push.return_value = { + "status": "pushed", + "created": 1, + "updated": 1, + "deleted": 0, + "errors": [], + "pushed_details": [ + { + "change_type": "added", + "component_id": "keboola.variables", + "config_id": "row-new", + "config_name": "new-row", + "path": "values/new", + "is_row": True, + "parent_config_id": "vars-1", + }, + { + "change_type": "modified", + "component_id": "keboola.variables", + "config_id": "row-1", + "config_name": "main", + "path": "values/main", + "is_row": True, + "parent_config_id": "vars-1", + }, + ], + } + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProjService, + patch("keboola_agent_cli.cli.SyncService") as MockSyncService, + ): + MockStore.return_value = store + MockProjService.return_value = ProjectService(config_store=store) + MockSyncService.return_value = mock_sync + + result = runner.invoke( + app, + [ + "sync", + "push", + "--project", + "prod", + "--directory", + str(tmp_path), + ], + ) + + assert result.exit_code == 0, f"Exit code {result.exit_code}: {result.output}" + text = _strip_ansi(result.output) + assert "Pushed: 1 created, 1 updated" in text + assert "ADDED keboola.variables/values/new" in text + assert "MODIFIED keboola.variables/values/main" in text + # =================================================================== # sync branch-link / branch-unlink / branch-status CLI tests diff --git a/tests/test_sync_config_format.py b/tests/test_sync_config_format.py index 6287a406..3200bf06 100644 --- a/tests/test_sync_config_format.py +++ b/tests/test_sync_config_format.py @@ -269,3 +269,129 @@ def test_local_row_to_api(self, sample_row: dict) -> None: assert description == "A test row" assert configuration["parameters"]["query"] == "SELECT 1" assert configuration["storage"]["output"]["tables"][0]["destination"] == "out.c-main.data" + + +class TestVariablesRowRoundTrip: + """Round-trip for keboola.variables / keboola.shared-code rows. + + These components use non-standard top-level configuration keys + (``values`` for variables, ``code`` for shared-code) that the API accepts + as the row ``configuration`` body verbatim. The local YAML MUST preserve + those keys at the top level -- wrapping them under ``_configuration_extra`` + breaks the FIIA convention of direct edits (``values: [...]`` at top level) + and diverges from ``kbc push`` behaviour. + """ + + def test_variables_row_api_to_local_hoists_values_to_top_level(self) -> None: + """keboola.variables values row: ``values`` key appears at top level of YAML. + + When a user edits a variables row locally, they expect to see and write + ``values:`` directly -- not nested under ``_configuration_extra``. + """ + api_row = { + "id": "row-main", + "name": "Main", + "description": "", + "configuration": { + "values": [ + {"name": "year_start", "value": "2016", "type": "string"}, + {"name": "region", "value": "eu", "type": "string"}, + ] + }, + } + + local = api_row_to_local(api_row, "keboola.variables") + + assert local["values"] == api_row["configuration"]["values"] + assert "_configuration_extra" not in local + + def test_variables_row_local_to_api_reads_top_level_values(self) -> None: + """User-authored top-level ``values:`` in local YAML flows to API configuration. + + FIIA writes row files with ``values:`` at the top level and expects + ``sync push`` to PUT that verbatim to + ``/components/keboola.variables/configs/{id}/rows/{rowId}``. + """ + local = { + "version": 2, + "name": "Main", + "description": "", + "values": [{"name": "year_start", "value": "2016", "type": "string"}], + "_keboola": {"component_id": "keboola.variables", "row_id": "row-1"}, + } + + name, description, configuration = local_row_to_api(local) + + assert name == "Main" + assert description == "" + assert configuration == { + "values": [{"name": "year_start", "value": "2016", "type": "string"}] + } + + def test_variables_row_byte_for_byte_round_trip(self) -> None: + """api→local→api returns the identical configuration dict for a variables row. + + This is the FIIA contract: the row body the user sees in Keboola after push + must equal the row body they wrote locally, byte-for-byte (deep equality). + """ + api_row = { + "id": "row-main", + "name": "Main", + "description": "default values", + "configuration": { + "values": [ + {"name": "year_start", "value": "2016", "type": "string"}, + {"name": "flag", "value": "true", "type": "string"}, + ] + }, + } + + local = api_row_to_local(api_row, "keboola.variables") + _, _, configuration = local_row_to_api(local) + + assert configuration == api_row["configuration"] + + def test_shared_code_row_hoists_code_to_top_level(self) -> None: + """keboola.shared-code rows: ``code_content`` / ``componentId`` keys hoisted. + + Shared-code rows use ``componentId`` + ``code_content`` at top level; + they must round-trip the same way as variables ``values`` rows. + """ + api_row = { + "id": "row-1", + "name": "Reusable snippet", + "description": "", + "configuration": { + "componentId": "keboola.snowflake-transformation", + "code_content": ["SELECT * FROM my_table;"], + }, + } + + local = api_row_to_local(api_row, "keboola.shared-code") + + assert local["componentId"] == "keboola.snowflake-transformation" + assert local["code_content"] == ["SELECT * FROM my_table;"] + assert "_configuration_extra" not in local + + _, _, configuration = local_row_to_api(local) + assert configuration == api_row["configuration"] + + def test_non_hoisted_component_still_uses_configuration_extra(self) -> None: + """Non-variables/shared-code rows keep ``_configuration_extra`` wrapping. + + The hoist-to-top-level behaviour is opt-in per component. Generic + extractor/writer rows with unusual top-level keys (e.g. a component + that stores ``foo`` at the configuration root) must not regress. + """ + api_row = { + "id": "row-1", + "name": "Test", + "description": "", + "configuration": {"parameters": {"q": "SELECT 1"}, "foo": {"bar": 1}}, + } + + local = api_row_to_local(api_row, "keboola.ex-db-snowflake") + + assert "_configuration_extra" in local + assert local["_configuration_extra"] == {"foo": {"bar": 1}} + assert "foo" not in local diff --git a/tests/test_sync_manifest.py b/tests/test_sync_manifest.py index e6bff3c2..7b73e9da 100644 --- a/tests/test_sync_manifest.py +++ b/tests/test_sync_manifest.py @@ -97,6 +97,17 @@ def test_manifest_configuration_with_rows(self) -> None: assert len(config.rows) == 1 assert config.rows[0].id == "row-1" assert config.rows[0].path == "rows/my-row" + assert config.rows[0].metadata == {} + + def test_manifest_config_row_with_metadata(self) -> None: + """ManifestConfigRow stores pull-time hashes in the metadata dict.""" + row = ManifestConfigRow( + id="row-1", + path="rows/my-row", + metadata={"pull_hash": "abc123", "pull_config_hash": "def456"}, + ) + assert row.metadata["pull_hash"] == "abc123" + assert row.metadata["pull_config_hash"] == "def456" class TestManifestRoundTrip: @@ -199,6 +210,41 @@ def test_load_manifest_file_not_found(self, tmp_path) -> None: with pytest.raises(FileNotFoundError, match="Manifest not found"): load_manifest(tmp_path) + def test_load_v2_manifest_tolerates_missing_row_metadata(self, tmp_path) -> None: + """v2 manifests (rows without metadata) load cleanly and default metadata to {}. + + Covers the v2→v3 upgrade path: existing on-disk manifests pre-date the + row-level metadata field introduced for row-push hashing. + """ + keboola_dir = tmp_path / ".keboola" + keboola_dir.mkdir() + v2_manifest_data = { + "version": 2, + "project": {"id": 1, "apiHost": "connection.keboola.com"}, + "allowTargetEnv": True, + "gitBranching": {"enabled": False, "defaultBranch": "main"}, + "sortBy": "id", + "naming": {"branch": "{branch_name}"}, + "branches": [], + "configurations": [ + { + "branchId": 1, + "componentId": "keboola.variables", + "id": "vars-1", + "path": "variables", + "rows": [{"id": "row-1", "path": "values/main"}], + } + ], + } + (keboola_dir / "manifest.json").write_text(json.dumps(v2_manifest_data)) + + loaded = load_manifest(tmp_path) + + assert loaded.version == 2 + row = loaded.configurations[0].rows[0] + assert row.id == "row-1" + assert row.metadata == {} + class TestManifestExtraFields: """Tests for extra field preservation.""" diff --git a/tests/test_sync_service.py b/tests/test_sync_service.py index ec8230d4..b8d6a70f 100644 --- a/tests/test_sync_service.py +++ b/tests/test_sync_service.py @@ -412,8 +412,9 @@ def test_pull_with_rows(self, tmp_config_dir: Path, tmp_path: Path) -> None: config_files = list(project_root.rglob(CONFIG_FILENAME)) assert len(config_files) == 3 # 2 configs + 1 row _config.yml - # Find the row config file (under rows/ subdirectory relative to project_root) - row_config_files = [f for f in config_files if "/rows/" in str(f.relative_to(project_root))] + # Find the row config file (under rows/ subdirectory relative to project_root). + # Use Path.parts so the test is OS-agnostic (Windows uses '\' separators). + row_config_files = [f for f in config_files if "rows" in f.relative_to(project_root).parts] assert len(row_config_files) == 1 row_data = yaml.safe_load(row_config_files[0].read_text(encoding="utf-8")) @@ -1055,6 +1056,430 @@ def test_push_update(self, tmp_config_dir: Path, tmp_path: Path) -> None: push_client.update_config.assert_called() +class TestPushRows: + """Row-level push tests: create/update/delete + encryption. + + These lock the P0-1 + P1-5 contract: sync push must deploy variable rows + via the Storage API ``/rows`` endpoint and encrypt ``#``-prefixed secrets + before transmission. + """ + + def _init_and_pull( + self, + tmp_config_dir: Path, + project_root: Path, + ) -> tuple[ConfigStore, SyncService]: + """init + pull with SAMPLE_COMPONENTS (includes one row).""" + init_client = _make_sync_mock_client( + verify_token_response=SAMPLE_VERIFY_TOKEN, + branches_response=SAMPLE_BRANCHES, + ) + store = setup_single_project(tmp_config_dir) + init_svc = SyncService( + config_store=store, + client_factory=lambda url, token: init_client, + ) + init_svc.init_sync(alias="prod", project_root=project_root) + + pull_client = _make_sync_mock_client(components_response=SAMPLE_COMPONENTS) + pull_svc = SyncService( + config_store=store, + client_factory=lambda url, token: pull_client, + ) + pull_svc.pull(alias="prod", project_root=project_root) + return store, pull_svc + + def test_push_row_update_calls_update_config_row( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """Editing a row YAML triggers client.update_config_row with the new configuration.""" + project_root = tmp_path / "project" + project_root.mkdir() + store, _ = self._init_and_pull(tmp_config_dir, project_root) + + # Locate the row file (SAMPLE_COMPONENTS has one row under cfg-001). + config_files = list(project_root.rglob(CONFIG_FILENAME)) + row_files = [f for f in config_files if "rows" in f.relative_to(project_root).parts] + assert len(row_files) == 1, f"Expected exactly one row file, got {row_files}" + + row_file = row_files[0] + row_data = yaml.safe_load(row_file.read_text(encoding="utf-8")) + row_data["parameters"]["path"] = "/users/changed" + row_file.write_text(yaml.dump(row_data, default_flow_style=False), encoding="utf-8") + + push_client = _make_sync_mock_client(components_response=SAMPLE_COMPONENTS) + push_client.update_config_row.return_value = {"id": "row-001"} + push_svc = SyncService( + config_store=store, + client_factory=lambda url, token: push_client, + ) + + result = push_svc.push(alias="prod", project_root=project_root) + + assert result["status"] == "pushed" + assert result["updated"] == 1 + assert result["errors"] == [] + push_client.update_config_row.assert_called_once() + call_kwargs = push_client.update_config_row.call_args.kwargs + assert call_kwargs["component_id"] == "keboola.ex-http" + assert call_kwargs["config_id"] == "cfg-001" + assert call_kwargs["row_id"] == "row-001" + # configuration dict passed in has the edited value + assert call_kwargs["configuration"]["parameters"]["path"] == "/users/changed" + + def test_push_row_delete_calls_delete_config_row( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """Removing a row YAML file triggers client.delete_config_row + manifest pruning.""" + project_root = tmp_path / "project" + project_root.mkdir() + store, _ = self._init_and_pull(tmp_config_dir, project_root) + + row_files = [ + f + for f in project_root.rglob(CONFIG_FILENAME) + if "rows" in f.relative_to(project_root).parts + ] + assert len(row_files) == 1 + row_files[0].unlink() + + push_client = _make_sync_mock_client(components_response=SAMPLE_COMPONENTS) + push_client.delete_config_row.return_value = None + push_svc = SyncService( + config_store=store, + client_factory=lambda url, token: push_client, + ) + + result = push_svc.push(alias="prod", project_root=project_root) + + assert result["status"] == "pushed" + assert result["deleted"] == 1 + push_client.delete_config_row.assert_called_once() + call_kwargs = push_client.delete_config_row.call_args.kwargs + assert call_kwargs["component_id"] == "keboola.ex-http" + assert call_kwargs["config_id"] == "cfg-001" + assert call_kwargs["row_id"] == "row-001" + + # Manifest should no longer list the deleted row + manifest = load_manifest(project_root) + parent = next(c for c in manifest.configurations if c.id == "cfg-001") + assert all(r.id != "row-001" for r in parent.rows) + + def test_push_row_encrypts_hash_secrets_before_api_call( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """#-prefixed secrets in row YAML are sent through the Encryption API first. + + Locks P1-5: the PUT body contains KBC::-prefixed ciphertext, never the + plaintext the user wrote locally. + """ + project_root = tmp_path / "project" + project_root.mkdir() + store, _ = self._init_and_pull(tmp_config_dir, project_root) + + row_files = [ + f + for f in project_root.rglob(CONFIG_FILENAME) + if "rows" in f.relative_to(project_root).parts + ] + row_file = row_files[0] + row_data = yaml.safe_load(row_file.read_text(encoding="utf-8")) + row_data["parameters"]["#api_token"] = "plain-secret" + row_file.write_text(yaml.dump(row_data, default_flow_style=False), encoding="utf-8") + + push_client = _make_sync_mock_client(components_response=SAMPLE_COMPONENTS) + push_client.update_config_row.return_value = {"id": "row-001"} + + # Real encryption API returns the SAME keys the caller sent, with values + # replaced by ciphertext. _collect_secrets flattens the path, so the key + # the caller sends is "#parameters.#api_token". Mirror that here so + # _apply_encrypted successfully replaces the leaf value on round-trip. + def fake_encrypt(*, project_id, component_id, data): + return {k: "KBC::ComponentSecure::ciphertext" for k in data} + + push_client.encrypt_values.side_effect = fake_encrypt + push_svc = SyncService( + config_store=store, + client_factory=lambda url, token: push_client, + ) + + result = push_svc.push(alias="prod", project_root=project_root) + + assert result["status"] == "pushed" + assert result["updated"] == 1 + push_client.encrypt_values.assert_called_once() + enc_kwargs = push_client.encrypt_values.call_args.kwargs + assert enc_kwargs["component_id"] == "keboola.ex-http" + # Plaintext was collected and sent; key is path-prefixed so the API can + # encrypt it under the right scope. + assert any(v == "plain-secret" for v in enc_kwargs["data"].values()) + + put_kwargs = push_client.update_config_row.call_args.kwargs + assert ( + put_kwargs["configuration"]["parameters"]["#api_token"] + == "KBC::ComponentSecure::ciphertext" + ) + + def test_push_row_update_error_accumulates(self, tmp_config_dir: Path, tmp_path: Path) -> None: + """Row push failure is captured in result['errors'], other changes still pushed.""" + from keboola_agent_cli.errors import KeboolaApiError + + project_root = tmp_path / "project" + project_root.mkdir() + store, _ = self._init_and_pull(tmp_config_dir, project_root) + + row_files = [ + f + for f in project_root.rglob(CONFIG_FILENAME) + if "rows" in f.relative_to(project_root).parts + ] + row_file = row_files[0] + row_data = yaml.safe_load(row_file.read_text(encoding="utf-8")) + row_data["parameters"]["path"] = "/users/changed" + row_file.write_text(yaml.dump(row_data, default_flow_style=False), encoding="utf-8") + + push_client = _make_sync_mock_client(components_response=SAMPLE_COMPONENTS) + push_client.update_config_row.side_effect = KeboolaApiError( + message="validation failed", + status_code=400, + error_code="validation", + ) + push_svc = SyncService( + config_store=store, + client_factory=lambda url, token: push_client, + ) + + result = push_svc.push(alias="prod", project_root=project_root) + + assert result["status"] == "pushed" + assert result["updated"] == 0 + assert len(result["errors"]) == 1 + assert result["errors"][0]["config_id"] == "row-001" + assert "validation failed" in result["errors"][0]["message"] + + def test_push_encryption_failure_aborts_fail_closed( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """ENCRYPTION_FAILED on a per-change row push aborts the whole push. + + Not buried in ``result["errors"]`` -- the exception propagates so the + CLI exits non-zero. A plaintext secret must never reach the wire, and + the caller must NEVER see ``status=pushed`` when any ``#``-secret + failed to encrypt. + """ + from keboola_agent_cli.errors import KeboolaApiError + + project_root = tmp_path / "project" + project_root.mkdir() + store, _ = self._init_and_pull(tmp_config_dir, project_root) + + row_files = [ + f + for f in project_root.rglob(CONFIG_FILENAME) + if "rows" in f.relative_to(project_root).parts + ] + row_file = row_files[0] + row_data = yaml.safe_load(row_file.read_text(encoding="utf-8")) + row_data["parameters"]["#api_token"] = "plain-secret" + row_file.write_text(yaml.dump(row_data, default_flow_style=False), encoding="utf-8") + + push_client = _make_sync_mock_client(components_response=SAMPLE_COMPONENTS) + push_client.encrypt_values.side_effect = Exception("encryption service unreachable") + push_svc = SyncService( + config_store=store, + client_factory=lambda url, token: push_client, + ) + + with pytest.raises(KeboolaApiError) as excinfo: + push_svc.push(alias="prod", project_root=project_root) + assert excinfo.value.error_code == "ENCRYPTION_FAILED" + # No row mutation on the server: update_config_row must never have been called. + push_client.update_config_row.assert_not_called() + + def test_push_untracked_row_dir_calls_create_config_row( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """A hand-crafted row dir under a tracked config gets POSTed via create_config_row. + + Locks the `_find_untracked_rows` contract: dropping a + ``rows/new-row/_config.yml`` into a tracked config's ``rows/`` + directory must be detected as an added row and pushed via the + row-create endpoint. + """ + project_root = tmp_path / "project" + project_root.mkdir() + store, _ = self._init_and_pull(tmp_config_dir, project_root) + + manifest = load_manifest(project_root) + parent_cfg = next(c for c in manifest.configurations if c.id == "cfg-001") + branch_path = manifest.branches[0].path + parent_dir = project_root / branch_path / parent_cfg.path + new_row_dir = parent_dir / "rows" / "hand-crafted-row" + new_row_dir.mkdir(parents=True) + new_row_file = new_row_dir / CONFIG_FILENAME + new_row_file.write_text( + yaml.dump( + { + "name": "Hand crafted row", + "parameters": {"path": "/new/endpoint"}, + "_keboola": {"component_id": "keboola.ex-http"}, + }, + default_flow_style=False, + ), + encoding="utf-8", + ) + + push_client = _make_sync_mock_client(components_response=SAMPLE_COMPONENTS) + push_client.create_config_row.return_value = {"id": "row-new-001"} + push_svc = SyncService( + config_store=store, + client_factory=lambda url, token: push_client, + ) + + result = push_svc.push(alias="prod", project_root=project_root) + + assert result["status"] == "pushed" + assert result["created"] == 1 + assert result["errors"] == [] + push_client.create_config_row.assert_called_once() + call_kwargs = push_client.create_config_row.call_args.kwargs + assert call_kwargs["component_id"] == "keboola.ex-http" + assert call_kwargs["config_id"] == "cfg-001" + assert call_kwargs["name"] == "Hand crafted row" + assert call_kwargs["configuration"]["parameters"]["path"] == "/new/endpoint" + + # Manifest should now track the newly created row with the API-assigned id. + post_manifest = load_manifest(project_root) + parent_after = next(c for c in post_manifest.configurations if c.id == "cfg-001") + new_row_ids = [r.id for r in parent_after.rows if r.path == "rows/hand-crafted-row"] + assert new_row_ids == ["row-new-001"] + + def _build_variables_row_dir(self, tmp_path: Path) -> Path: + """Write a minimal ``keboola.variables`` row YAML with a hoisted ``values`` list.""" + from keboola_agent_cli.constants import CONFIG_FILENAME, CONFIG_YML_VERSION + + row_dir = tmp_path / "rows" / "default" + row_dir.mkdir(parents=True) + (row_dir / CONFIG_FILENAME).write_text( + yaml.dump( + { + "version": CONFIG_YML_VERSION, + "name": "default", + "description": "", + "values": [{"name": "#api_key", "value": "plain-secret-xyz"}], + "_keboola": { + "component_id": "keboola.variables", + "row_id": "vals-default", + }, + }, + default_flow_style=False, + ), + encoding="utf-8", + ) + return row_dir + + def test_push_variables_row_encrypts_hash_prefixed_name( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """``keboola.variables`` rows with ``#``-prefixed names encrypt before PUT. + + Locks the PR #190 review fix: the row-hoisted ``values: [{name, value}]`` + shape must flow through the Encryption API, not bypass it. + """ + from keboola_agent_cli.sync.manifest import ManifestConfigRow, ManifestConfiguration + + row_dir = self._build_variables_row_dir(tmp_path) + + push_client = _make_sync_mock_client() + push_client.update_config_row.return_value = {"id": "vals-default"} + + def fake_encrypt(*, project_id, component_id, data): + return {k: "KBC::ProjectSecure::ciphertext" for k in data} + + push_client.encrypt_values.side_effect = fake_encrypt + + store = setup_single_project(tmp_config_dir) + push_svc = SyncService( + config_store=store, + client_factory=lambda url, token: push_client, + ) + + parent = ManifestConfiguration( + branch_id=12345, + component_id="keboola.variables", + id="vars-001", + path="other/keboola.variables/shared-variables", + rows=[ManifestConfigRow(id="vals-default", path="rows/default", metadata={})], + ) + + push_svc._push_update_row( + push_client, + component_id="keboola.variables", + parent_config_id="vars-001", + row_id="vals-default", + row_dir=row_dir, + parent=parent, + branch_id=None, + project_id=258, + allow_plaintext_fallback=False, + ) + + push_client.encrypt_values.assert_called_once() + enc_kwargs = push_client.encrypt_values.call_args.kwargs + assert enc_kwargs["component_id"] == "keboola.variables" + assert any(v == "plain-secret-xyz" for v in enc_kwargs["data"].values()) + + put_kwargs = push_client.update_config_row.call_args.kwargs + assert put_kwargs["component_id"] == "keboola.variables" + assert put_kwargs["row_id"] == "vals-default" + assert put_kwargs["configuration"]["values"][0]["value"] == "KBC::ProjectSecure::ciphertext" + assert put_kwargs["configuration"]["values"][0]["name"] == "#api_key" + + def test_push_variables_row_encrypt_failure_aborts( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """Encryption failure on a variables row aborts fail-closed. + + ``update_config_row`` must not be called, so plaintext never hits Storage. + """ + from keboola_agent_cli.errors import KeboolaApiError + from keboola_agent_cli.sync.manifest import ManifestConfigRow, ManifestConfiguration + + row_dir = self._build_variables_row_dir(tmp_path) + + push_client = _make_sync_mock_client() + push_client.encrypt_values.side_effect = Exception("encryption service down") + + store = setup_single_project(tmp_config_dir) + push_svc = SyncService( + config_store=store, + client_factory=lambda url, token: push_client, + ) + + parent = ManifestConfiguration( + branch_id=12345, + component_id="keboola.variables", + id="vars-001", + path="other/keboola.variables/shared-variables", + rows=[ManifestConfigRow(id="vals-default", path="rows/default", metadata={})], + ) + + with pytest.raises(KeboolaApiError) as excinfo: + push_svc._push_update_row( + push_client, + component_id="keboola.variables", + parent_config_id="vars-001", + row_id="vals-default", + row_dir=row_dir, + parent=parent, + branch_id=None, + project_id=258, + allow_plaintext_fallback=False, + ) + assert excinfo.value.error_code == "ENCRYPTION_FAILED" + push_client.update_config_row.assert_not_called() + + # =================================================================== # branch_link tests # =================================================================== diff --git a/tests/test_variables_cli.py b/tests/test_variables_cli.py new file mode 100644 index 00000000..5af18dfa --- /dev/null +++ b/tests/test_variables_cli.py @@ -0,0 +1,612 @@ +"""CLI tests for config variables-set / variables-get / variables-clear. + +Mocks VariablesService to lock the CLI<->service contract: flag parsing, +--var KEY=VALUE repeatable, --replace, --dry-run, --variables-id / --values-id +overrides, --yes for clear. Verifies both --json and human output shapes. +""" + +from __future__ import annotations + +import json +import re +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 ConfigError, KeboolaApiError +from keboola_agent_cli.models import ProjectConfig +from keboola_agent_cli.services.project_service import ProjectService + + +def _strip_ansi(text: str) -> str: + return re.sub(r"\x1b\[[0-9;]*m", "", text) + + +TEST_TOKEN = "901-10493007-VDtlEDWDF6Tx5V8jjE8FshFlqM0Hl0c08KHqpt0k" +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 + + +class TestVariablesSet: + def test_parses_multiple_vars_and_passes_dict_to_service(self, tmp_path: Path) -> None: + """--var k=v (repeated) builds a dict handed to VariablesService.set_variables.""" + store = _setup_config(tmp_path / "config") + mock_vars = MagicMock() + mock_vars.set_variables.return_value = { + "project_alias": "prod", + "parent_component_id": "keboola.snowflake-transformation", + "parent_config_id": "15815157", + "variables_id": "vars-new", + "values_id": "row-new", + "action": "created", + "values": {"year_start": "2016", "region": "eu"}, + "encrypted_keys": [], + } + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProj, + patch("keboola_agent_cli.cli.VariablesService") as MockVars, + ): + MockStore.return_value = store + MockProj.return_value = ProjectService(config_store=store) + MockVars.return_value = mock_vars + + result = runner.invoke( + app, + [ + "--json", + "config", + "variables-set", + "--project", + "prod", + "--component-id", + "keboola.snowflake-transformation", + "--config-id", + "15815157", + "--var", + "year_start=2016", + "--var", + "region=eu", + ], + ) + + assert result.exit_code == 0, result.output + output = json.loads(result.output) + assert output["status"] == "ok" + assert output["data"]["action"] == "created" + + call_kwargs = mock_vars.set_variables.call_args.kwargs + assert call_kwargs["variables"] == {"year_start": "2016", "region": "eu"} + assert call_kwargs["replace"] is False + + def test_replace_flag_forwards_to_service(self, tmp_path: Path) -> None: + """--replace -> replace=True on the service call.""" + store = _setup_config(tmp_path / "config") + mock_vars = MagicMock() + mock_vars.set_variables.return_value = { + "project_alias": "prod", + "parent_component_id": "keboola.x", + "parent_config_id": "cfg-1", + "variables_id": "vars-1", + "values_id": "row-1", + "action": "updated", + "values": {"k": "v"}, + "encrypted_keys": [], + } + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProj, + patch("keboola_agent_cli.cli.VariablesService") as MockVars, + ): + MockStore.return_value = store + MockProj.return_value = ProjectService(config_store=store) + MockVars.return_value = mock_vars + + result = runner.invoke( + app, + [ + "--json", + "config", + "variables-set", + "--project", + "prod", + "--component-id", + "keboola.x", + "--config-id", + "cfg-1", + "--var", + "k=v", + "--replace", + ], + ) + + assert result.exit_code == 0 + assert mock_vars.set_variables.call_args.kwargs["replace"] is True + + def test_no_vars_exits_with_invalid_argument(self, tmp_path: Path) -> None: + """Missing --var exits 2 with INVALID_ARGUMENT; service NOT called.""" + store = _setup_config(tmp_path / "config") + mock_vars = MagicMock() + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProj, + patch("keboola_agent_cli.cli.VariablesService") as MockVars, + ): + MockStore.return_value = store + MockProj.return_value = ProjectService(config_store=store) + MockVars.return_value = mock_vars + + result = runner.invoke( + app, + [ + "--json", + "config", + "variables-set", + "--project", + "prod", + "--component-id", + "keboola.x", + "--config-id", + "cfg-1", + ], + ) + + assert result.exit_code == 2 + output = json.loads(result.output) + assert output["error"]["code"] == "INVALID_ARGUMENT" + mock_vars.set_variables.assert_not_called() + + def test_malformed_var_exits_with_invalid_argument(self, tmp_path: Path) -> None: + """--var missing = sign is rejected before the service is called.""" + store = _setup_config(tmp_path / "config") + mock_vars = MagicMock() + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProj, + patch("keboola_agent_cli.cli.VariablesService") as MockVars, + ): + MockStore.return_value = store + MockProj.return_value = ProjectService(config_store=store) + MockVars.return_value = mock_vars + + result = runner.invoke( + app, + [ + "--json", + "config", + "variables-set", + "--project", + "prod", + "--component-id", + "keboola.x", + "--config-id", + "cfg-1", + "--var", + "no_equals_sign", + ], + ) + + assert result.exit_code == 2 + output = json.loads(result.output) + assert output["error"]["code"] == "INVALID_ARGUMENT" + assert "KEY=VALUE" in output["error"]["message"] + + def test_dry_run_does_not_call_set_and_shows_preview(self, tmp_path: Path) -> None: + """--dry-run calls get_variables + prints preview, never calls set_variables.""" + store = _setup_config(tmp_path / "config") + mock_vars = MagicMock() + mock_vars.get_variables.return_value = { + "project_alias": "prod", + "parent_component_id": "keboola.x", + "parent_config_id": "cfg-1", + "variables_id": "vars-1", + "values_id": "row-1", + "values": {"region": "eu", "year_start": "2016"}, + "linked": True, + } + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProj, + patch("keboola_agent_cli.cli.VariablesService") as MockVars, + ): + MockStore.return_value = store + MockProj.return_value = ProjectService(config_store=store) + MockVars.return_value = mock_vars + + result = runner.invoke( + app, + [ + "--json", + "config", + "variables-set", + "--project", + "prod", + "--component-id", + "keboola.x", + "--config-id", + "cfg-1", + "--var", + "region=us-west", + "--dry-run", + ], + ) + + assert result.exit_code == 0 + output = json.loads(result.output) + data = output["data"] + assert data["dry_run"] is True + assert data["action"] == "would_update" + assert data["would_write"] == {"region": "us-west", "year_start": "2016"} + mock_vars.set_variables.assert_not_called() + + def test_dry_run_replace_masks_dropped_hash_keys(self, tmp_path: Path) -> None: + """``--replace --dry-run`` must mask ``#``-prefixed dropped rows as . + + Other branches (``+``/``~``/``=``) already mask; the ``-`` branch was + missing the same mask and leaked full ``KBC::ProjectSecure::...`` + ciphertext. Flagged in PR #190 review. + """ + store = _setup_config(tmp_path / "config") + mock_vars = MagicMock() + mock_vars.get_variables.return_value = { + "project_alias": "prod", + "parent_component_id": "keboola.x", + "parent_config_id": "cfg-1", + "variables_id": "vars-1", + "values_id": "row-1", + "values": { + "#old_secret": "KBC::ProjectSecure::eJwOld", + "region": "eu", + }, + "linked": True, + } + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProj, + patch("keboola_agent_cli.cli.VariablesService") as MockVars, + ): + MockStore.return_value = store + MockProj.return_value = ProjectService(config_store=store) + MockVars.return_value = mock_vars + + result = runner.invoke( + app, + [ + "config", + "variables-set", + "--project", + "prod", + "--component-id", + "keboola.x", + "--config-id", + "cfg-1", + "--var", + "region=us-west", + "--replace", + "--dry-run", + ], + ) + + assert result.exit_code == 0 + stripped = _strip_ansi(result.output) + # Dropped #-key must be masked, never leak ciphertext. + assert "KBC::" not in stripped + assert "- #old_secret" in stripped + assert "" in stripped + + def test_human_output_shows_action_and_values(self, tmp_path: Path) -> None: + """Human mode prints 'created'/'updated' + final values (ANSI stripped).""" + store = _setup_config(tmp_path / "config") + mock_vars = MagicMock() + mock_vars.set_variables.return_value = { + "project_alias": "prod", + "parent_component_id": "keboola.x", + "parent_config_id": "cfg-1", + "variables_id": "vars-1", + "values_id": "row-1", + "action": "created", + "values": {"year_start": "2016"}, + "encrypted_keys": [], + } + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProj, + patch("keboola_agent_cli.cli.VariablesService") as MockVars, + ): + MockStore.return_value = store + MockProj.return_value = ProjectService(config_store=store) + MockVars.return_value = mock_vars + + result = runner.invoke( + app, + [ + "config", + "variables-set", + "--project", + "prod", + "--component-id", + "keboola.x", + "--config-id", + "cfg-1", + "--var", + "year_start=2016", + ], + ) + + assert result.exit_code == 0, result.output + output = _strip_ansi(result.output) + assert "created" in output + assert "year_start" in output + assert "2016" in output + + +class TestVariablesGet: + def test_json_returns_linked_payload(self, tmp_path: Path) -> None: + store = _setup_config(tmp_path / "config") + mock_vars = MagicMock() + mock_vars.get_variables.return_value = { + "project_alias": "prod", + "parent_component_id": "keboola.x", + "parent_config_id": "cfg-1", + "variables_id": "vars-1", + "values_id": "row-1", + "values": {"region": "eu"}, + "linked": True, + } + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProj, + patch("keboola_agent_cli.cli.VariablesService") as MockVars, + ): + MockStore.return_value = store + MockProj.return_value = ProjectService(config_store=store) + MockVars.return_value = mock_vars + + result = runner.invoke( + app, + [ + "--json", + "config", + "variables-get", + "--project", + "prod", + "--component-id", + "keboola.x", + "--config-id", + "cfg-1", + ], + ) + + assert result.exit_code == 0 + output = json.loads(result.output) + assert output["data"]["linked"] is True + assert output["data"]["values"] == {"region": "eu"} + + def test_human_shows_no_variables_when_unlinked(self, tmp_path: Path) -> None: + store = _setup_config(tmp_path / "config") + mock_vars = MagicMock() + mock_vars.get_variables.return_value = { + "project_alias": "prod", + "parent_component_id": "keboola.x", + "parent_config_id": "cfg-1", + "variables_id": None, + "values_id": None, + "values": {}, + "linked": False, + } + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProj, + patch("keboola_agent_cli.cli.VariablesService") as MockVars, + ): + MockStore.return_value = store + MockProj.return_value = ProjectService(config_store=store) + MockVars.return_value = mock_vars + + result = runner.invoke( + app, + [ + "config", + "variables-get", + "--project", + "prod", + "--component-id", + "keboola.x", + "--config-id", + "cfg-1", + ], + ) + + assert result.exit_code == 0 + assert "No variables linked" in _strip_ansi(result.output) + + +class TestVariablesClear: + def test_clear_with_yes_skips_prompt(self, tmp_path: Path) -> None: + store = _setup_config(tmp_path / "config") + mock_vars = MagicMock() + mock_vars.clear_variables.return_value = { + "project_alias": "prod", + "parent_component_id": "keboola.x", + "parent_config_id": "cfg-1", + "was_linked": True, + "unlinked_variables_id": "vars-1", + "unlinked_values_id": "row-1", + } + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProj, + patch("keboola_agent_cli.cli.VariablesService") as MockVars, + ): + MockStore.return_value = store + MockProj.return_value = ProjectService(config_store=store) + MockVars.return_value = mock_vars + + result = runner.invoke( + app, + [ + "--json", + "config", + "variables-clear", + "--project", + "prod", + "--component-id", + "keboola.x", + "--config-id", + "cfg-1", + "--yes", + ], + ) + + assert result.exit_code == 0 + output = json.loads(result.output) + assert output["data"]["was_linked"] is True + mock_vars.clear_variables.assert_called_once() + + def test_api_error_propagates_with_correct_exit_code(self, tmp_path: Path) -> None: + """ENCRYPTION_FAILED on set_variables maps to non-zero exit.""" + store = _setup_config(tmp_path / "config") + mock_vars = MagicMock() + mock_vars.set_variables.side_effect = KeboolaApiError( + message="Encryption failed: network", + status_code=0, + error_code="ENCRYPTION_FAILED", + ) + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProj, + patch("keboola_agent_cli.cli.VariablesService") as MockVars, + ): + MockStore.return_value = store + MockProj.return_value = ProjectService(config_store=store) + MockVars.return_value = mock_vars + + result = runner.invoke( + app, + [ + "--json", + "config", + "variables-set", + "--project", + "prod", + "--component-id", + "keboola.x", + "--config-id", + "cfg-1", + "--var", + "#secret=x", + ], + ) + + assert result.exit_code != 0 + output = json.loads(result.output) + assert output["error"]["code"] == "ENCRYPTION_FAILED" + + def test_clear_human_mode_shows_unlinked_summary(self, tmp_path: Path) -> None: + """Human mode prints 'Unlinked' + component/config identifiers (ANSI stripped). + + Completes the §5 'both --json and human' coverage for variables-clear -- + the other clear test only exercises the JSON path. + """ + store = _setup_config(tmp_path / "config") + mock_vars = MagicMock() + mock_vars.clear_variables.return_value = { + "project_alias": "prod", + "parent_component_id": "keboola.snowflake-transformation", + "parent_config_id": "cfg-1", + "was_linked": True, + "unlinked_variables_id": "vars-1", + "unlinked_values_id": "row-1", + } + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProj, + patch("keboola_agent_cli.cli.VariablesService") as MockVars, + ): + MockStore.return_value = store + MockProj.return_value = ProjectService(config_store=store) + MockVars.return_value = mock_vars + + result = runner.invoke( + app, + [ + "config", + "variables-clear", + "--project", + "prod", + "--component-id", + "keboola.snowflake-transformation", + "--config-id", + "cfg-1", + "--yes", + ], + ) + + assert result.exit_code == 0, result.output + output = _strip_ansi(result.output) + assert "Unlinked" in output + assert "cfg-1" in output + assert "keboola.snowflake-transformation" in output + + def test_config_error_exits_with_config_error_code(self, tmp_path: Path) -> None: + """best_practices.md §5: ConfigError from the service maps to exit 5.""" + store = _setup_config(tmp_path / "config") + mock_vars = MagicMock() + mock_vars.clear_variables.side_effect = ConfigError("Project 'ghost' not found.") + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProj, + patch("keboola_agent_cli.cli.VariablesService") as MockVars, + ): + MockStore.return_value = store + MockProj.return_value = ProjectService(config_store=store) + MockVars.return_value = mock_vars + + result = runner.invoke( + app, + [ + "--json", + "config", + "variables-clear", + "--project", + "prod", + "--component-id", + "keboola.x", + "--config-id", + "cfg-1", + "--yes", + ], + ) + + assert result.exit_code == 5 + output = json.loads(result.output) + assert output["error"]["code"] == "CONFIG_ERROR" diff --git a/tests/test_variables_service.py b/tests/test_variables_service.py new file mode 100644 index 00000000..da39e1aa --- /dev/null +++ b/tests/test_variables_service.py @@ -0,0 +1,461 @@ +"""Tests for VariablesService -- high-level variables-as-attachment UX. + +Covers the 3 service verbs (get, set, clear) and the three set sub-paths: +auto-create, merge, replace. Encryption + fail-closed + close() semantics are +inherited from sync_service's _encrypt_secrets_in_config (tested separately). +""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from helpers import setup_single_project +from keboola_agent_cli.errors import ConfigError, KeboolaApiError +from keboola_agent_cli.models import TokenVerifyResponse +from keboola_agent_cli.services.variables_service import ( + VARIABLES_COMPONENT_ID, + VariablesService, +) + +SAMPLE_VERIFY = TokenVerifyResponse( + token_id="tok-1", + token_description="kbagent", + project_id=258, + project_name="Test", + owner_name="Me", +) + + +def _mock_client() -> MagicMock: + client = MagicMock() + client.__enter__ = MagicMock(return_value=client) + client.__exit__ = MagicMock(return_value=False) + client.verify_token.return_value = SAMPLE_VERIFY + return client + + +def _service(tmp_config_dir: Path, client: MagicMock) -> VariablesService: + store = setup_single_project(tmp_config_dir) + return VariablesService( + config_store=store, + client_factory=lambda url, token: client, + ) + + +class TestGetVariables: + def test_returns_linked_false_when_no_variables_id(self, tmp_config_dir: Path) -> None: + """A parent config with no variables_id reports linked=False and empty values.""" + client = _mock_client() + client.get_config_detail.return_value = { + "id": "cfg-1", + "name": "my-transform", + "configuration": {"parameters": {"some": "thing"}}, + } + svc = _service(tmp_config_dir, client) + + result = svc.get_variables(alias="prod", component_id="keboola.x", config_id="cfg-1") + + assert result["linked"] is False + assert result["values"] == {} + assert result["variables_id"] is None + assert result["values_id"] is None + client.close.assert_called_once() + + def test_returns_values_from_default_row(self, tmp_config_dir: Path) -> None: + """With variables_id but no values_id, resolves to first row (default convention).""" + client = _mock_client() + client.get_config_detail.side_effect = [ + { + "id": "cfg-1", + "configuration": {"variables_id": "vars-1"}, + }, + { + "id": "vars-1", + "configuration": {"variables": [{"name": "region", "type": "string"}]}, + "rows": [ + { + "id": "row-1", + "configuration": {"values": [{"name": "region", "value": "eu"}]}, + } + ], + }, + ] + svc = _service(tmp_config_dir, client) + + result = svc.get_variables(alias="prod", component_id="keboola.x", config_id="cfg-1") + + assert result["linked"] is True + assert result["values"] == {"region": "eu"} + assert result["values_id"] == "row-1" + + def test_closes_client_on_api_error(self, tmp_config_dir: Path) -> None: + """best_practices.md §3: client.close() is called even when the API raises.""" + client = _mock_client() + client.get_config_detail.side_effect = KeboolaApiError( + message="not found", status_code=404, error_code="NOT_FOUND" + ) + svc = _service(tmp_config_dir, client) + + with pytest.raises(KeboolaApiError): + svc.get_variables(alias="prod", component_id="keboola.x", config_id="cfg-1") + client.close.assert_called_once() + + +class TestSetVariablesAutoCreate: + def test_creates_backing_config_and_row_when_parent_not_linked( + self, tmp_config_dir: Path + ) -> None: + """First call on an unlinked parent: create config + row + patch parent.""" + client = _mock_client() + client.get_config_detail.return_value = { + "id": "cfg-1", + "name": "my-transform", + "configuration": {"parameters": {}}, + } + client.create_config.return_value = {"id": "vars-auto-1"} + client.create_config_row.return_value = {"id": "row-auto-1"} + svc = _service(tmp_config_dir, client) + + result = svc.set_variables( + alias="prod", + component_id="keboola.snowflake-transformation", + config_id="cfg-1", + variables={"year_start": "2016", "region": "eu"}, + ) + + assert result["action"] == "created" + assert result["variables_id"] == "vars-auto-1" + assert result["values_id"] == "row-auto-1" + assert result["values"] == {"year_start": "2016", "region": "eu"} + + # create_config was called with schema derived from var keys + create_kwargs = client.create_config.call_args.kwargs + assert create_kwargs["component_id"] == VARIABLES_COMPONENT_ID + assert create_kwargs["name"] == "my-transform-vars" + assert {v["name"] for v in create_kwargs["configuration"]["variables"]} == { + "year_start", + "region", + } + + # Row was created with values payload + row_kwargs = client.create_config_row.call_args.kwargs + assert {v["name"] for v in row_kwargs["configuration"]["values"]} == { + "year_start", + "region", + } + + # Parent config was patched to link variables_id + values_id + parent_update_kwargs = client.update_config.call_args.kwargs + assert parent_update_kwargs["component_id"] == "keboola.snowflake-transformation" + assert parent_update_kwargs["configuration"]["variables_id"] == "vars-auto-1" + assert parent_update_kwargs["configuration"]["variables_values_id"] == "row-auto-1" + client.close.assert_called_once() + + def test_auto_create_with_empty_parent_name_falls_back_to_config_id( + self, tmp_config_dir: Path + ) -> None: + """Parent without a name gets `-vars` as the auto-created name.""" + client = _mock_client() + client.get_config_detail.return_value = { + "id": "cfg-999", + "name": "", + "configuration": {}, + } + client.create_config.return_value = {"id": "vars-auto"} + client.create_config_row.return_value = {"id": "row-auto"} + svc = _service(tmp_config_dir, client) + + svc.set_variables( + alias="prod", + component_id="keboola.x", + config_id="cfg-999", + variables={"k": "v"}, + ) + + assert client.create_config.call_args.kwargs["name"] == "cfg-999-vars" + + +class TestSetVariablesUpdate: + def _linked_parent(self) -> dict: + return { + "id": "cfg-1", + "name": "my-transform", + "configuration": { + "variables_id": "vars-existing", + "variables_values_id": "row-existing", + "parameters": {"irrelevant": True}, + }, + } + + def _linked_vars_cfg( + self, values: list[dict] | None = None, schema: list[dict] | None = None + ) -> dict: + return { + "id": "vars-existing", + "configuration": { + "variables": schema + if schema is not None + else [{"name": "region", "type": "string"}], + }, + "rows": [ + { + "id": "row-existing", + "configuration": { + "values": values + if values is not None + else [{"name": "region", "value": "eu"}], + }, + } + ], + } + + def test_merge_keeps_existing_keys(self, tmp_config_dir: Path) -> None: + """Default merge: new values overlay existing ones; other keys survive.""" + client = _mock_client() + client.get_config_detail.side_effect = [ + self._linked_parent(), + self._linked_vars_cfg( + values=[ + {"name": "region", "value": "eu"}, + {"name": "year_start", "value": "2016"}, + ] + ), + ] + client.update_config_row.return_value = {"id": "row-existing"} + svc = _service(tmp_config_dir, client) + + result = svc.set_variables( + alias="prod", + component_id="keboola.x", + config_id="cfg-1", + variables={"region": "us-west"}, # only region changes + ) + + assert result["action"] == "updated" + assert result["values"] == {"region": "us-west", "year_start": "2016"} + + put_kwargs = client.update_config_row.call_args.kwargs + values_sent = {v["name"]: v["value"] for v in put_kwargs["configuration"]["values"]} + assert values_sent == {"region": "us-west", "year_start": "2016"} + + def test_replace_overwrites_entire_values(self, tmp_config_dir: Path) -> None: + """--replace drops existing keys not in the new set.""" + client = _mock_client() + client.get_config_detail.side_effect = [ + self._linked_parent(), + self._linked_vars_cfg( + values=[ + {"name": "region", "value": "eu"}, + {"name": "year_start", "value": "2016"}, + ] + ), + ] + client.update_config_row.return_value = {"id": "row-existing"} + svc = _service(tmp_config_dir, client) + + result = svc.set_variables( + alias="prod", + component_id="keboola.x", + config_id="cfg-1", + variables={"region": "us-west"}, + replace=True, + ) + + assert result["values"] == {"region": "us-west"} + + def test_new_key_extends_schema(self, tmp_config_dir: Path) -> None: + """A brand-new key appears in the schema after set (cosmetic sync).""" + client = _mock_client() + client.get_config_detail.side_effect = [ + self._linked_parent(), + self._linked_vars_cfg( + schema=[{"name": "region", "type": "string"}], + values=[{"name": "region", "value": "eu"}], + ), + ] + client.update_config_row.return_value = {"id": "row-existing"} + svc = _service(tmp_config_dir, client) + + svc.set_variables( + alias="prod", + component_id="keboola.x", + config_id="cfg-1", + variables={"year_start": "2016"}, # new key + ) + + # The schema-extension call to update_config should have been made on + # the variables config itself (distinct from the parent update). + # Two update_config calls may fire: schema + parent. The variables one + # carries a `variables` key in its configuration. + schema_calls = [ + c + for c in client.update_config.call_args_list + if c.kwargs.get("component_id") == VARIABLES_COMPONENT_ID + ] + assert len(schema_calls) == 1 + schema_names = {v["name"] for v in schema_calls[0].kwargs["configuration"]["variables"]} + assert schema_names == {"region", "year_start"} + + def test_schema_sync_failure_is_logged_not_raised(self, tmp_config_dir: Path) -> None: + """If schema update fails, the row update already succeeded -- don't bubble up.""" + client = _mock_client() + client.get_config_detail.side_effect = [ + self._linked_parent(), + self._linked_vars_cfg(), + ] + client.update_config_row.return_value = {"id": "row-existing"} + + def update_config_side_effect(**kwargs): + if kwargs.get("component_id") == VARIABLES_COMPONENT_ID: + raise KeboolaApiError( + message="schema update failed", + status_code=500, + error_code="INTERNAL", + ) + return {"id": kwargs["config_id"]} + + client.update_config.side_effect = update_config_side_effect + svc = _service(tmp_config_dir, client) + + result = svc.set_variables( + alias="prod", + component_id="keboola.x", + config_id="cfg-1", + variables={"new_key": "x"}, + ) + # The operation succeeds from the caller's view even though the + # cosmetic schema update failed. + assert result["action"] == "updated" + assert "new_key" in result["values"] + + +class TestSetVariablesEncryption: + def test_hash_prefixed_keys_are_encrypted_before_row_put(self, tmp_config_dir: Path) -> None: + """#-prefixed keys go through encrypt_values before the PUT body is built.""" + client = _mock_client() + client.get_config_detail.side_effect = [ + { + "id": "cfg-1", + "name": "my-transform", + "configuration": { + "variables_id": "vars-1", + "variables_values_id": "row-1", + }, + }, + { + "id": "vars-1", + "configuration": {"variables": []}, + "rows": [{"id": "row-1", "configuration": {"values": []}}], + }, + ] + client.update_config_row.return_value = {"id": "row-1"} + client.encrypt_values.side_effect = lambda *, project_id, component_id, data: { + k: "KBC::ComponentSecure::cipher" for k in data + } + svc = _service(tmp_config_dir, client) + + result = svc.set_variables( + alias="prod", + component_id="keboola.x", + config_id="cfg-1", + variables={"#api_token": "plain-secret"}, + ) + + assert result["encrypted_keys"] == ["#api_token"] + put_kwargs = client.update_config_row.call_args.kwargs + values_sent = {v["name"]: v["value"] for v in put_kwargs["configuration"]["values"]} + assert values_sent["#api_token"] == "KBC::ComponentSecure::cipher" + + def test_encryption_failure_fail_closed(self, tmp_config_dir: Path) -> None: + """Encryption failure raises ENCRYPTION_FAILED (no plaintext PUT).""" + client = _mock_client() + client.get_config_detail.side_effect = [ + { + "id": "cfg-1", + "name": "t", + "configuration": { + "variables_id": "vars-1", + "variables_values_id": "row-1", + }, + }, + { + "id": "vars-1", + "configuration": {"variables": []}, + "rows": [{"id": "row-1", "configuration": {"values": []}}], + }, + ] + client.encrypt_values.side_effect = Exception("encryption unavailable") + svc = _service(tmp_config_dir, client) + + with pytest.raises(KeboolaApiError) as excinfo: + svc.set_variables( + alias="prod", + component_id="keboola.x", + config_id="cfg-1", + variables={"#api_token": "plain"}, + ) + assert excinfo.value.error_code == "ENCRYPTION_FAILED" + client.update_config_row.assert_not_called() + # best_practices.md §5: try/finally close() must still fire on error. + client.close.assert_called_once() + + +class TestSetVariablesValidation: + def test_empty_variables_dict_raises(self, tmp_config_dir: Path) -> None: + """set_variables requires at least one var (CLI layer validates first, service is a belt).""" + client = _mock_client() + svc = _service(tmp_config_dir, client) + + with pytest.raises(ConfigError, match="at least one variable"): + svc.set_variables( + alias="prod", + component_id="keboola.x", + config_id="cfg-1", + variables={}, + ) + + +class TestClearVariables: + def test_strips_link_from_parent_and_keeps_backing_config(self, tmp_config_dir: Path) -> None: + """Clear removes both fields from parent configuration, leaves variables config alive.""" + client = _mock_client() + client.get_config_detail.return_value = { + "id": "cfg-1", + "configuration": { + "variables_id": "vars-1", + "variables_values_id": "row-1", + "parameters": {"unrelated": "value"}, + }, + } + svc = _service(tmp_config_dir, client) + + result = svc.clear_variables(alias="prod", component_id="keboola.x", config_id="cfg-1") + + assert result["was_linked"] is True + assert result["unlinked_variables_id"] == "vars-1" + assert result["unlinked_values_id"] == "row-1" + + put_kwargs = client.update_config.call_args.kwargs + assert "variables_id" not in put_kwargs["configuration"] + assert "variables_values_id" not in put_kwargs["configuration"] + assert put_kwargs["configuration"]["parameters"] == {"unrelated": "value"} + + # The backing keboola.variables config MUST NOT be deleted. + client.delete_config.assert_not_called() + + def test_no_op_when_parent_not_linked(self, tmp_config_dir: Path) -> None: + """Clearing an already-unlinked config is a no-op (no update_config call).""" + client = _mock_client() + client.get_config_detail.return_value = { + "id": "cfg-1", + "configuration": {"parameters": {}}, + } + svc = _service(tmp_config_dir, client) + + result = svc.clear_variables(alias="prod", component_id="keboola.x", config_id="cfg-1") + + assert result["was_linked"] is False + client.update_config.assert_not_called() diff --git a/uv.lock b/uv.lock index 74824b07..5951974b 100644 --- a/uv.lock +++ b/uv.lock @@ -423,7 +423,7 @@ wheels = [ [[package]] name = "keboola-agent-cli" -version = "0.20.6" +version = "0.21.0" source = { editable = "." } dependencies = [ { name = "httpx" },