From c9a8c7acc9d1460369ae045fb1fa16fbdc6d9ec4 Mon Sep 17 00:00:00 2001 From: jordanrburger Date: Mon, 20 Jul 2026 13:34:27 -0400 Subject: [PATCH] feat(config): add --change-description to config update and row-update Callers can now set the Keboola config version's `changeDescription` (the version-history audit line) instead of always getting the generic default ("Updated ... via kbagent config update"). This matters for auditability on shared production configs, where the version history is the paper trail. - `config update` and `config row-update` gain `--change-description TEXT`; when omitted, the previous auto-generated default is preserved unchanged. - `--dry-run` echoes the change description that would be sent (JSON + human). - The `kbagent serve` router mirrors the flag on both PATCH routes (1:1). - The client layer already supported `change_description`; the service simply threads a caller value through, falling back to a shared default helper. Creates (`config new`, `row-create`) and `flow new/update` are left as a follow-up: their client methods do not yet accept a change description, so wiring them is more than the trivial passthrough done here. --- CLAUDE.md | 4 +- .../kbagent/references/commands-reference.md | 4 +- src/keboola_agent_cli/commands/config.py | 26 ++++ src/keboola_agent_cli/commands/context.py | 4 +- .../server/routers/configs.py | 4 + .../services/config_service.py | 49 ++++-- tests/test_config_row_cli.py | 38 +++++ tests/test_config_row_service.py | 50 ++++++ tests/test_config_update.py | 143 ++++++++++++++++++ tests/test_e2e.py | 48 ++++++ tests/test_server_router_calls.py | 50 ++++++ 11 files changed, 400 insertions(+), 20 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 76c1e050..d322435e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -300,7 +300,7 @@ kbagent project member-set-role --project ALIAS --email EMAIL --role admin|guest kbagent config list [--project NAME] [--component-type TYPE] [--component-id ID] [--branch ID] [--include-rows] kbagent config detail --project NAME [--project NAME ...] --component-id ID [--config-id ID] [--branch ID] [--with-state] 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] [--allow-plaintext-on-encrypt-failure] +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] [--change-description TEXT] [--dry-run] [--branch ID] [--allow-plaintext-on-encrypt-failure] kbagent config set-default-bucket --project NAME --component-id ID --config-id ID (--bucket BUCKET_ID | --clear) [--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] @@ -312,7 +312,7 @@ kbagent config set-metadata --project NAME --component-id ID --config-id ID --ke kbagent config delete-metadata --project NAME --component-id ID --config-id ID --metadata-id ID [--branch ID] [--yes] kbagent config set-folder --project NAME --component-id ID --config-id ID --name FOLDER [--branch ID] kbagent config row-create --project NAME --component-id ID --config-id ID --name ROW_NAME [--description D] [--configuration JSON|@file|-] [--is-disabled] [--branch ID] [--allow-plaintext-on-encrypt-failure] -kbagent config row-update --project NAME --component-id ID --config-id ID --row-id ID [--name N] [--description D] [--configuration JSON|@file|-] [--is-disabled | --is-enabled] [--branch ID] [--allow-plaintext-on-encrypt-failure] +kbagent config row-update --project NAME --component-id ID --config-id ID --row-id ID [--name N] [--description D] [--configuration JSON|@file|-] [--change-description TEXT] [--is-disabled | --is-enabled] [--branch ID] [--allow-plaintext-on-encrypt-failure] kbagent config row-delete --project NAME --component-id ID --config-id ID --row-id ID [--branch ID] [--yes] kbagent config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index 80c24398..b0c15055 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -70,7 +70,7 @@ Requires a **super-admin** Manage API token (same kind as `org setup`). Same def - `config list [--project NAME] [--component-type TYPE] [--component-id ID] [--branch ID] [--include-rows]` -- list configs across projects (branch-aware). With `--include-rows` each row extends to include the full `configuration` and `rows` body (noticeably larger payload -- use only when the bodies are needed; the summary default covers name/description/component/last_modified/folder) - `config detail --project NAME [--project ...] --component-id ID [--config-id ID] [--branch ID] [--with-state]` -- **two modes.** **Single** (with `--config-id`): full config dict, shape unchanged from previous releases (callers depending on `.id`, `.configuration`, `.rows` etc. are unaffected). **Bulk** (omit `--config-id`): returns `{"configs": [...], "errors": [...]}` with every configuration of `--component-id` across one or many projects -- each row tagged with `project_alias`/`branch_id`. One HTTP request per project via `list_components_with_configs` (not one per config; a project with 100 Snowflake writers returns in a single round-trip). `--project` is repeatable in bulk mode; `--config-id` with multiple `--project` is rejected (exit 2) because a single config lives in one project. `--with-state` attaches the runtime `state` dict: single-mode triggers an extra `get_config_state` call, bulk-mode adds `include=state` to the listing call (no N+1) - `config search --query PATTERN [--project NAME] [-i] [-r] [--branch ID]` -- search config bodies for string/regex (branch-aware) -- `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] [--allow-plaintext-on-encrypt-failure]` -- update metadata and/or configuration content. **`#`-prefixed secrets auto-encrypt via the Encryption API before write (fail-closed; since 0.54.0, #378)** -- `--allow-plaintext-on-encrypt-failure` overrides, `--dry-run` keeps plaintext in the diff (ciphertext is non-deterministic). Note `--set '#password=...'` sets a *top-level* key; for a nested secret use `--set 'parameters.#password=...'`. `--set` targets a nested key (e.g. `parameters.db.host=new-host`). `--merge` deep-merges into existing config (preserves sibling keys). `--dry-run` previews changes without applying. Paths are relative to the configuration root (unlike MCP's `update_config` which uses paths relative to `parameters`). **Auto-normalize (0.28.0+; #245 / 0.31.0+; #274)**: `parameters.blocks[].codes[].script` is fixed before pushing to Storage API. **String -> array** (0.28.0+; #245): SQL transformations get statement-level split (respects `'...'` / `"..."` / `$$..$$` / `--` / `#` / `//` / `/* ... */`); Python / R / `kds-team.app-custom-python` get `[script]` wrap. **List-element re-split** (0.31.0+; #274): when `script` is already a list but an element packs multiple `;`-separated statements, each SQL element is re-run through `split_statements()` and replaced inline. Closes the ODBC `Actual statement count N did not match the desired statement count 1` (SQL state 0A000) runtime crash that survives the 0.28.0 string fix. The result envelope's `normalizations: [{path, action: "sql_split"|"wrap_array"|"sql_resplit", before_type, after_type, after_length, before_length?}]` records every change (empty when nothing was malformed; `sql_resplit` adds `before_length` and a `[E]` suffix on `path` pointing at the original element index). Bypassing kbagent (raw REST, MCP `update_sql_transformation` / `create_sql_transformation`) does NOT inherit either pass -- prefer `kbagent config update` for SQL transformation body changes. +- `config update --project NAME --component-id ID --config-id ID [--name N] [--description D] [--configuration JSON|@file|-] [--configuration-file PATH] [--set PATH=VALUE ...] [--merge] [--change-description TEXT] [--dry-run] [--branch ID] [--allow-plaintext-on-encrypt-failure]` -- update metadata and/or configuration content. `--change-description` sets the new config version's `changeDescription` (the version-history audit line); omit it to keep the auto-generated default (e.g. `Updated configuration via kbagent config update`). Distinct from `--description`, which sets the config's display description. `--dry-run` echoes the `change_description` that would be sent. **`#`-prefixed secrets auto-encrypt via the Encryption API before write (fail-closed; since 0.54.0, #378)** -- `--allow-plaintext-on-encrypt-failure` overrides, `--dry-run` keeps plaintext in the diff (ciphertext is non-deterministic). Note `--set '#password=...'` sets a *top-level* key; for a nested secret use `--set 'parameters.#password=...'`. `--set` targets a nested key (e.g. `parameters.db.host=new-host`). `--merge` deep-merges into existing config (preserves sibling keys). `--dry-run` previews changes without applying. Paths are relative to the configuration root (unlike MCP's `update_config` which uses paths relative to `parameters`). **Auto-normalize (0.28.0+; #245 / 0.31.0+; #274)**: `parameters.blocks[].codes[].script` is fixed before pushing to Storage API. **String -> array** (0.28.0+; #245): SQL transformations get statement-level split (respects `'...'` / `"..."` / `$$..$$` / `--` / `#` / `//` / `/* ... */`); Python / R / `kds-team.app-custom-python` get `[script]` wrap. **List-element re-split** (0.31.0+; #274): when `script` is already a list but an element packs multiple `;`-separated statements, each SQL element is re-run through `split_statements()` and replaced inline. Closes the ODBC `Actual statement count N did not match the desired statement count 1` (SQL state 0A000) runtime crash that survives the 0.28.0 string fix. The result envelope's `normalizations: [{path, action: "sql_split"|"wrap_array"|"sql_resplit", before_type, after_type, after_length, before_length?}]` records every change (empty when nothing was malformed; `sql_resplit` adds `before_length` and a `[E]` suffix on `path` pointing at the original element index). Bypassing kbagent (raw REST, MCP `update_sql_transformation` / `create_sql_transformation`) does NOT inherit either pass -- prefer `kbagent config update` for SQL transformation body changes. - `config set-default-bucket --project NAME --component-id ID --config-id ID (--bucket BUCKET_ID | --clear) [--dry-run] [--branch ID]` -- set or clear `configuration.storage.output.default_bucket` on a configuration. Discoverable shortcut for the raw-mode workaround at https://keboola.atlassian.net/wiki/spaces/SUP/pages/3770155030/. Read-modify-write that preserves sibling keys; returns `{"changed": false}` when the value already matches the requested state. Honored by output tables that don't pin their own `destination`. - `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 @@ -84,7 +84,7 @@ Requires a **super-admin** Manage API token (same kind as `org setup`). Same def - `config delete-metadata --project NAME --component-id ID --config-id ID --metadata-id ID [--branch ID] [--yes]` -- delete a configuration metadata entry by its numeric ID (from `metadata-list`) - `config set-folder --project NAME --component-id ID --config-id ID --name FOLDER [--branch ID]` -- set (or clear, with empty `--name`) the `KBC.configuration.folderName` metadata, which groups configs into named folders in the Keboola UI. See `config-metadata-workflow.md` - `config row-create --project NAME --component-id ID --config-id ID --name ROW_NAME [--description D] [--configuration JSON|@file|-] [--is-disabled] [--branch ID] [--allow-plaintext-on-encrypt-failure]` -- create a new configuration row. Returns the full row dict with `id`, `name`, `version`. Optional `--configuration` accepts JSON inline, `@file`, or stdin (`-`). `#`-prefixed secrets auto-encrypt before write (fail-closed; since 0.54.0, #378). -- `config row-update --project NAME --component-id ID --config-id ID --row-id ID [--name N] [--description D] [--configuration JSON|@file|-] [--is-disabled | --is-enabled] [--branch ID] [--allow-plaintext-on-encrypt-failure]` -- update an existing configuration row. Pass only the fields you want to change; omitted fields are preserved. `--is-disabled` / `--is-enabled` toggle the row's enabled state. `#`-prefixed secrets auto-encrypt before write (fail-closed; since 0.54.0, #378). +- `config row-update --project NAME --component-id ID --config-id ID --row-id ID [--name N] [--description D] [--configuration JSON|@file|-] [--change-description TEXT] [--is-disabled | --is-enabled] [--branch ID] [--allow-plaintext-on-encrypt-failure]` -- update an existing configuration row. Pass only the fields you want to change; omitted fields are preserved. `--is-disabled` / `--is-enabled` toggle the row's enabled state. `--change-description` sets the new row version's `changeDescription` audit line (default: auto-generated). `#`-prefixed secrets auto-encrypt before write (fail-closed; since 0.54.0, #378). - `config row-delete --project NAME --component-id ID --config-id ID --row-id ID [--branch ID] [--yes]` -- delete a configuration row. Destructive (gated behind `--allow-destructive`). Branch-aware. Without `--yes` and outside `--json` mode, prompts for interactive confirmation; `--json` mode auto-skips the prompt. - `config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL]` -- return the OAuth authorization URL for a component that uses OAuth authentication. **Requires a master Storage API token** (canManageTokens privilege) -- non-master tokens fail with `MISSING_MASTER_TOKEN` exit 3 on a fail-fast pre-flight check before any HTTP write happens. Open the URL in a browser to complete the OAuth flow. diff --git a/src/keboola_agent_cli/commands/config.py b/src/keboola_agent_cli/commands/config.py index 9a29bb3c..5ce7ffd2 100644 --- a/src/keboola_agent_cli/commands/config.py +++ b/src/keboola_agent_cli/commands/config.py @@ -560,6 +560,11 @@ def config_update( "--merge", help="Deep-merge into existing config instead of replacing", ), + change_description: str | None = typer.Option( + None, + "--change-description", + help="Version changeDescription for the audit trail (default: auto-generated)", + ), dry_run: bool = typer.Option( False, "--dry-run", @@ -605,6 +610,10 @@ def config_update( # Preview changes without applying kbagent config update --project P --component-id C --config-id ID \\ --set 'parameters.db.host=new-host' --dry-run + + # Set a meaningful version changeDescription for the audit trail + kbagent config update --project P --component-id C --config-id ID \\ + --set 'parameters.db.host=new-host' --change-description "AI-1234: point at new DB host" """ formatter = get_formatter(ctx) service = get_service(ctx, "config_service") @@ -666,6 +675,7 @@ def config_update( set_paths=parsed_sets, merge=effective_merge, dry_run=dry_run, + change_description=change_description, branch_id=branch, allow_plaintext_fallback=allow_plaintext, ) @@ -695,6 +705,9 @@ def config_update( for change in changes: formatter.console.print(f" {change}") formatter.console.print() + change_desc = result.get("change_description") + if change_desc: + formatter.console.print(f"[dim]changeDescription:[/dim] {change_desc}") _emit_normalizations_warning(formatter, normalizations) return @@ -2172,6 +2185,11 @@ def config_row_update( "--merge", help="Deep-merge into existing row config instead of replacing", ), + change_description: str | None = typer.Option( + None, + "--change-description", + help="Version changeDescription for the audit trail (default: auto-generated)", + ), dry_run: bool = typer.Option( False, "--dry-run", @@ -2226,6 +2244,10 @@ def config_row_update( # Disable a row (excludes it from job runs) kbagent config row-update --project P --component-id C --config-id ID --row-id R --is-disabled + + # Set a meaningful version changeDescription for the audit trail + kbagent config row-update --project P --component-id C --config-id ID --row-id R \\ + --set 'parameters.table=new_table' --change-description "AI-1234: repoint row" """ if is_disabled and is_enabled: formatter = get_formatter(ctx) @@ -2282,6 +2304,7 @@ def config_row_update( set_paths=parsed_sets, merge=effective_merge, dry_run=dry_run, + change_description=change_description, is_disabled=is_disabled_value, branch_id=branch, allow_plaintext_fallback=allow_plaintext, @@ -2309,6 +2332,9 @@ def config_row_update( for change in changes: formatter.console.print(f" {change}") formatter.console.print() + change_desc = result.get("change_description") + if change_desc: + formatter.console.print(f"[dim]changeDescription:[/dim] {change_desc}") return if formatter.json_mode: diff --git a/src/keboola_agent_cli/commands/context.py b/src/keboola_agent_cli/commands/context.py index d5cd267c..cd8d3fc5 100644 --- a/src/keboola_agent_cli/commands/context.py +++ b/src/keboola_agent_cli/commands/context.py @@ -162,7 +162,7 @@ Use storage_workspace_id with `kbagent workspace detail --workspace-id ID`, NOT parameters.id (which 404s). - 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] [--allow-plaintext-on-encrypt-failure] + 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] [--change-description TEXT] [--dry-run] [--branch ID] [--allow-plaintext-on-encrypt-failure] Update config metadata and/or configuration content. --set targets a nested key (e.g. parameters.db.host=new-host). --merge deep-merges into existing config (preserves sibling keys). --dry-run previews changes. @@ -241,7 +241,7 @@ kbagent config row-create --project NAME --component-id ID --config-id ID --name ROW_NAME [--description D] [--configuration JSON|@file|-] [--is-disabled] [--branch ID] [--allow-plaintext-on-encrypt-failure] Create a new configuration row. Returns the new row ID. Optional --configuration accepts JSON inline, @file, or stdin. #-prefixed secrets auto-encrypt before write (fail-closed; since 0.54.0). - kbagent config row-update --project NAME --component-id ID --config-id ID --row-id ID [--name N] [--description D] [--configuration JSON|@file|-] [--is-disabled | --is-enabled] [--branch ID] [--allow-plaintext-on-encrypt-failure] + kbagent config row-update --project NAME --component-id ID --config-id ID --row-id ID [--name N] [--description D] [--configuration JSON|@file|-] [--change-description TEXT] [--is-disabled | --is-enabled] [--branch ID] [--allow-plaintext-on-encrypt-failure] Update an existing configuration row. Pass only the fields you want to change. #-prefixed secrets auto-encrypt before write (fail-closed; since 0.54.0). kbagent config row-delete --project NAME --component-id ID --config-id ID --row-id ID [--branch ID] [--yes] diff --git a/src/keboola_agent_cli/server/routers/configs.py b/src/keboola_agent_cli/server/routers/configs.py index 0c5b57f0..0c21ddaa 100644 --- a/src/keboola_agent_cli/server/routers/configs.py +++ b/src/keboola_agent_cli/server/routers/configs.py @@ -20,6 +20,7 @@ class ConfigUpdate(BaseModel): set_paths: list[tuple[str, Any]] | None = None merge: bool = False dry_run: bool = False + change_description: str | None = None branch_id: int | None = None @@ -36,6 +37,7 @@ class ConfigUpdateRow(BaseModel): description: str | None = None configuration: dict[str, Any] | None = None is_disabled: bool | None = None + change_description: str | None = None branch_id: int | None = None @@ -156,6 +158,7 @@ def config_update( set_paths=body.set_paths, merge=body.merge, dry_run=body.dry_run, + change_description=body.change_description, branch_id=body.branch_id, ) @@ -385,6 +388,7 @@ def row_update( name=body.name, description=body.description, configuration=body.configuration, + change_description=body.change_description, is_disabled=body.is_disabled, branch_id=body.branch_id, ) diff --git a/src/keboola_agent_cli/services/config_service.py b/src/keboola_agent_cli/services/config_service.py index f4b9fa71..31d70f90 100644 --- a/src/keboola_agent_cli/services/config_service.py +++ b/src/keboola_agent_cli/services/config_service.py @@ -66,6 +66,23 @@ def _infer_component_type(component_id: str) -> str | None: return None +def _default_change_description(command: str, *, has_metadata: bool, has_content: bool) -> str: + """Build the default config-version ``changeDescription`` for a write. + + Used when the caller does not pass an explicit ``--change-description``. + ``command`` is the kbagent command that produced the version (e.g. + ``"config update"``), and the flags describe which parts changed so the + version-history line reads e.g. ``Updated metadata + configuration via + kbagent config update``. + """ + parts = [] + if has_metadata: + parts.append("metadata") + if has_content: + parts.append("configuration") + return f"Updated {' + '.join(parts)} via kbagent {command}" + + def _find_matches_in_json( obj: Any, match_fn: Any, @@ -663,6 +680,7 @@ def update_config( set_paths: list[tuple[str, Any]] | None = None, merge: bool = False, dry_run: bool = False, + change_description: str | None = None, branch_id: int | None = None, allow_plaintext_fallback: bool = False, ) -> dict[str, Any]: @@ -680,6 +698,9 @@ def update_config( merge: If True, deep-merge *configuration* or *set_paths* into the existing config instead of replacing. When using *set_paths* merge is always implied. + change_description: Text stored as the new config version's + ``changeDescription`` (the version-history audit line). When + ``None`` a default is generated from what changed. dry_run: If True, compute and return the diff without applying. branch_id: If set, update in a specific dev branch. If None, uses the project's active branch (if any). @@ -737,6 +758,10 @@ def update_config( component_id, final_config ) + change_desc = change_description or _default_change_description( + "config update", has_metadata=has_metadata, has_content=has_content + ) + if dry_run: current = client.get_config_detail( component_id, config_id, branch_id=effective_branch_id @@ -751,6 +776,7 @@ def update_config( "config_id": config_id, "branch_id": effective_branch_id, "changes": changes, + "change_description": change_desc, "old_configuration": old_cfg, "new_configuration": new_cfg, "normalizations": normalizations, @@ -768,13 +794,6 @@ def update_config( allow_plaintext_fallback=allow_plaintext_fallback, ) - change_parts = [] - if has_metadata: - change_parts.append("metadata") - if has_content: - change_parts.append("configuration") - change_desc = f"Updated {' + '.join(change_parts)} via kbagent config update" - result = client.update_config( component_id=component_id, config_id=config_id, @@ -1821,6 +1840,7 @@ def update_config_row( set_paths: list[tuple[str, Any]] | None = None, merge: bool = False, dry_run: bool = False, + change_description: str | None = None, is_disabled: bool | None = None, branch_id: int | None = None, allow_plaintext_fallback: bool = False, @@ -1838,6 +1858,9 @@ def update_config_row( set_paths: List of (path, value) tuples for targeted updates. merge: If True, deep-merge *configuration* into the existing row config instead of replacing. + change_description: Text stored as the new row version's + ``changeDescription``. When ``None`` a default is generated + from what changed. dry_run: If True, compute and return the diff without applying. is_disabled: When True, disable the row; when False, enable it; when None, leave the current state unchanged. @@ -1889,6 +1912,10 @@ def update_config_row( branch_id=effective_branch_id, ) + change_desc = change_description or _default_change_description( + "config row-update", has_metadata=has_metadata, has_content=has_content + ) + if dry_run: current_row = client.get_config_row( component_id, config_id, row_id, branch_id=effective_branch_id @@ -1910,17 +1937,11 @@ def update_config_row( "row_id": row_id, "branch_id": effective_branch_id, "changes": changes, + "change_description": change_desc, "old_configuration": old_cfg, "new_configuration": new_cfg, } - change_parts = [] - if has_metadata: - change_parts.append("metadata") - if has_content: - change_parts.append("configuration") - change_desc = f"Updated {' + '.join(change_parts)} via kbagent config row-update" - # Encrypt #-prefixed secrets before they reach Storage (issue #378). # Real write only -- dry-run returned above with plaintext diff. if final_config is not None: diff --git a/tests/test_config_row_cli.py b/tests/test_config_row_cli.py index 57569bcd..6f6082de 100644 --- a/tests/test_config_row_cli.py +++ b/tests/test_config_row_cli.py @@ -309,6 +309,44 @@ def test_set_flag(self, tmp_config_dir: Path) -> None: assert cfg["parameters"]["table"] == "invoices" assert cfg["parameters"]["limit"] == 1000 # sibling preserved + def test_change_description_flag_forwarded(self, tmp_config_dir: Path) -> None: + """--change-description reaches the client as change_description.""" + store = setup_single_project(tmp_config_dir) + mock_client = MagicMock() + mock_client.update_config_row.return_value = {**SAMPLE_ROW} + service = ConfigService( + config_store=store, + client_factory=lambda url, token: mock_client, + ) + + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "keboola_agent_cli.commands.config.get_service", + lambda ctx, name: service, + ) + result = _invoke( + tmp_config_dir, + "row-update", + [ + "--project", + "prod", + "--component-id", + "keboola.ex-db-snowflake", + "--config-id", + "cfg-001", + "--row-id", + "row-001", + "--name", + "New Name", + "--change-description", + "AI-1234: rename row", + ], + ) + + assert result.exit_code == 0, result.output + call_kwargs = mock_client.update_config_row.call_args.kwargs + assert call_kwargs["change_description"] == "AI-1234: rename row" + def test_dry_run_output(self, tmp_config_dir: Path) -> None: """--dry-run shows diff in JSON mode.""" store = setup_single_project(tmp_config_dir) diff --git a/tests/test_config_row_service.py b/tests/test_config_row_service.py index 6a0e7cab..cdebf26c 100644 --- a/tests/test_config_row_service.py +++ b/tests/test_config_row_service.py @@ -300,6 +300,56 @@ def test_branch_id_passed_through(self, tmp_config_dir: Path) -> None: call_kwargs = client.update_config_row.call_args.kwargs assert call_kwargs["branch_id"] == 99 + def test_custom_change_description_forwarded(self, tmp_config_dir: Path) -> None: + """An explicit change_description is sent verbatim to the client.""" + service, client = _make_service(tmp_config_dir) + + service.update_config_row( + alias="prod", + component_id="keboola.ex-db-snowflake", + config_id="cfg-001", + row_id="row-001", + name="New Name", + change_description="AI-1234: bump row limit", + ) + + call_kwargs = client.update_config_row.call_args.kwargs + assert call_kwargs["change_description"] == "AI-1234: bump row limit" + + def test_default_change_description_when_omitted(self, tmp_config_dir: Path) -> None: + """Omitting change_description preserves the generated default.""" + service, client = _make_service(tmp_config_dir) + + service.update_config_row( + alias="prod", + component_id="keboola.ex-db-snowflake", + config_id="cfg-001", + row_id="row-001", + name="New Name", + ) + + call_kwargs = client.update_config_row.call_args.kwargs + assert call_kwargs["change_description"] == ( + "Updated metadata via kbagent config row-update" + ) + + def test_dry_run_includes_change_description(self, tmp_config_dir: Path) -> None: + """dry_run surfaces the change_description that would be sent.""" + service, client = _make_service(tmp_config_dir) + + result = service.update_config_row( + alias="prod", + component_id="keboola.ex-db-snowflake", + config_id="cfg-001", + row_id="row-001", + set_paths=[("parameters.table", "orders")], + change_description="AI-1234: repoint table", + dry_run=True, + ) + + assert result["change_description"] == "AI-1234: repoint table" + client.update_config_row.assert_not_called() + # --------------------------------------------------------------------------- # delete_config_row tests diff --git a/tests/test_config_update.py b/tests/test_config_update.py index c233b031..57c09a44 100644 --- a/tests/test_config_update.py +++ b/tests/test_config_update.py @@ -457,3 +457,146 @@ def test_set_json_value(self, tmp_config_dir: Path) -> None: assert result.exit_code == 0, result.output cfg = mock_client.update_config.call_args.kwargs["configuration"] assert cfg["parameters"]["tables"] == {"new": "table"} + + +# --------------------------------------------------------------------------- +# change-description tests +# --------------------------------------------------------------------------- + + +class TestConfigServiceUpdateChangeDescription: + """Tests for the version changeDescription on ConfigService.update_config.""" + + def test_custom_change_description_forwarded(self, tmp_config_dir: Path) -> None: + """An explicit change_description is sent verbatim to the client.""" + service, client = _make_service(tmp_config_dir) + + service.update_config( + alias="prod", + component_id="keboola.ex-db-snowflake", + config_id="cfg-001", + name="New Name", + change_description="AI-1234: bump timeout", + ) + + call_kwargs = client.update_config.call_args.kwargs + assert call_kwargs["change_description"] == "AI-1234: bump timeout" + + def test_default_change_description_when_omitted(self, tmp_config_dir: Path) -> None: + """Omitting change_description preserves the generated default.""" + service, client = _make_service(tmp_config_dir) + + service.update_config( + alias="prod", + component_id="keboola.ex-db-snowflake", + config_id="cfg-001", + name="New Name", + ) + + call_kwargs = client.update_config.call_args.kwargs + assert call_kwargs["change_description"] == ("Updated metadata via kbagent config update") + + def test_dry_run_includes_change_description(self, tmp_config_dir: Path) -> None: + """dry_run surfaces the change_description that would be sent.""" + service, client = _make_service(tmp_config_dir) + + result = service.update_config( + alias="prod", + component_id="keboola.ex-db-snowflake", + config_id="cfg-001", + set_paths=[("parameters.user", "new-admin")], + change_description="AI-1234: rotate creds", + dry_run=True, + ) + + assert result["change_description"] == "AI-1234: rotate creds" + client.update_config.assert_not_called() + + +class TestConfigUpdateChangeDescriptionCli: + """CLI-level tests for --change-description on config update.""" + + def _invoke(self, tmp_config_dir: Path, args: list[str]) -> Result: + return runner.invoke( + app, + ["--json", "--config-dir", str(tmp_config_dir), "config", "update", *args], + ) + + def test_change_description_flag_forwarded(self, tmp_config_dir: Path) -> None: + """--change-description reaches the client as change_description.""" + store = setup_single_project(tmp_config_dir) + + mock_client = MagicMock() + mock_client.get_config_detail.return_value = SAMPLE_CONFIG_DETAIL + mock_client.update_config.return_value = { + "id": "cfg-001", + "name": "My Config", + "componentId": "keboola.ex-db-snowflake", + } + + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "keboola_agent_cli.commands.config.get_service", + lambda ctx, name: ConfigService( + config_store=store, + client_factory=lambda url, token: mock_client, + ), + ) + + result = self._invoke( + tmp_config_dir, + [ + "--project", + "prod", + "--component-id", + "keboola.ex-db-snowflake", + "--config-id", + "cfg-001", + "--set", + "parameters.user=new-admin", + "--change-description", + "AI-1234: rotate creds", + ], + ) + + assert result.exit_code == 0, result.output + call_kwargs = mock_client.update_config.call_args.kwargs + assert call_kwargs["change_description"] == "AI-1234: rotate creds" + + def test_dry_run_echoes_change_description(self, tmp_config_dir: Path) -> None: + """--dry-run --json output includes the change_description.""" + store = setup_single_project(tmp_config_dir) + + mock_client = MagicMock() + mock_client.get_config_detail.return_value = SAMPLE_CONFIG_DETAIL + + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "keboola_agent_cli.commands.config.get_service", + lambda ctx, name: ConfigService( + config_store=store, + client_factory=lambda url, token: mock_client, + ), + ) + + result = self._invoke( + tmp_config_dir, + [ + "--project", + "prod", + "--component-id", + "keboola.ex-db-snowflake", + "--config-id", + "cfg-001", + "--set", + "parameters.user=new-admin", + "--change-description", + "AI-1234: rotate creds", + "--dry-run", + ], + ) + + assert result.exit_code == 0, result.output + data = json.loads(result.output)["data"] + assert data["change_description"] == "AI-1234: rotate creds" + mock_client.update_config.assert_not_called() diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 2322b319..03740b25 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -1619,6 +1619,54 @@ def _test_config_update(self, config_id: str) -> None: assert db_config["port"] == 5439 assert "schema" not in db_config + # --change-description sets the version changeDescription verbatim. + change_desc = f"{RUN_ID} AI-1234: e2e change description" + dry = self._run_ok( + "config", + "update", + "--project", + self.alias, + "--component-id", + TEST_COMPONENT_ID, + "--config-id", + config_id, + "--set", + "parameters.db.timeout=15", + "--change-description", + change_desc, + "--dry-run", + ) + # --dry-run echoes the change description that would be sent. + assert dry["data"]["change_description"] == change_desc + + self._run_ok( + "config", + "update", + "--project", + self.alias, + "--component-id", + TEST_COMPONENT_ID, + "--config-id", + config_id, + "--set", + "parameters.db.timeout=15", + "--change-description", + change_desc, + ) + + # Verify the new version's changeDescription via config detail. + data = self._run_ok( + "config", + "detail", + "--project", + self.alias, + "--component-id", + TEST_COMPONENT_ID, + "--config-id", + config_id, + ) + assert data["data"]["changeDescription"] == change_desc + def _test_config_merge(self, config_id: str) -> None: """Test config update --merge: partial merge without losing existing keys.""" # Current state: host=final.example.com, port=5439, database=final_db diff --git a/tests/test_server_router_calls.py b/tests/test_server_router_calls.py index 8c2970d8..e12facdd 100644 --- a/tests/test_server_router_calls.py +++ b/tests/test_server_router_calls.py @@ -1081,3 +1081,53 @@ def test_bulk_delete_route_not_shadowed_by_alias_delete(tmp_path: Path) -> None: assert res.status_code == 200, res.text project_svc.bulk_remove_projects.assert_called_once() project_svc.remove_project.assert_not_called() + + +# --------------------------------------------------------------------------- +# configs.py PATCH /{p}/{c}/{cfg} +# Service: config.update_config(change_description=...) +# --------------------------------------------------------------------------- + + +def test_config_update_passes_change_description_kwarg(tmp_path: Path) -> None: + """Router forwards change_description= to ConfigService.update_config.""" + config_svc = MagicMock() + config_svc.update_config.return_value = {"id": CONFIG_ID, "name": "cfg"} + registry = _mock_registry(config=config_svc) + app = _make_app_with_registry(tmp_path, registry) + + with TestClient(app) as client: + res = client.patch( + f"/configs/{PROJECT}/{COMPONENT}/{CONFIG_ID}", + headers=AUTH, + json={"name": "cfg", "change_description": "AI-1234: via API"}, + ) + + assert res.status_code == 200, res.text + kwargs = config_svc.update_config.call_args.kwargs + assert kwargs["change_description"] == "AI-1234: via API" + + +# --------------------------------------------------------------------------- +# configs.py PATCH /{p}/{c}/{cfg}/rows/{row} +# Service: config.update_config_row(change_description=...) +# --------------------------------------------------------------------------- + + +def test_config_row_update_passes_change_description_kwarg(tmp_path: Path) -> None: + """Router forwards change_description= to ConfigService.update_config_row.""" + config_svc = MagicMock() + config_svc.update_config_row.return_value = {"id": ROW_ID, "name": "row"} + registry = _mock_registry(config=config_svc) + app = _make_app_with_registry(tmp_path, registry) + + with TestClient(app) as client: + res = client.patch( + f"/configs/{PROJECT}/{COMPONENT}/{CONFIG_ID}/rows/{ROW_ID}", + headers=AUTH, + json={"name": "row", "change_description": "AI-1234: via API"}, + ) + + assert res.status_code == 200, res.text + kwargs = config_svc.update_config_row.call_args.kwargs + assert kwargs["change_description"] == "AI-1234: via API"