Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ kbagent sync status [--directory DIR]
kbagent sync diff --project ALIAS [--all-projects] [--directory DIR] [--branch ID]
kbagent sync push --project ALIAS [--all-projects] [--dry-run] [--force] [--allow-plaintext-on-encrypt-failure] [--branch ID] [--no-name-drift-warnings]
kbagent sync clone --source DIR --target ALIAS --target-dir DIR [--bucket-map FILE] [--variable-values FILE] [--instance-rename FILE] [--dry-run] [--branch ID]
# `sync clone` (0.63.0+) copies a reference synced tree into a fresh target project + parameterizes it: applies bucket_map / variable_values / instance_rename overrides (JSON/YAML files), then pushes so every config CREATEs fresh -- keboola.flow task configIds and transformation variable links are remapped reference->ULID by push Phase C/D. Idempotent: re-run with an existing --target-dir reports no_changes. Fails fast if the target already contains the reference's configs (clone needs a fresh target).
# `sync clone` (0.63.0+) copies a reference synced tree into a fresh target project + parameterizes it: applies bucket_map / variable_values / instance_rename overrides (JSON/YAML files), then pushes so every config CREATEs fresh -- keboola.flow task configIds and transformation variable links are remapped reference->ULID by push Phase C/D. Idempotent: re-run with an existing --target-dir reports no_changes. Fails fast if the target already contains the reference's configs (clone needs a fresh target). Override files must be flat {id: scalar} mappings (0.89.0+): a nested mapping/list/null value is rejected with CONFIG_ERROR naming the key + actual type, instead of being silently stringified into a bogus ID.
kbagent sync branch-link --project ALIAS (--branch-id ID | --branch-name NAME) [--directory DIR]
kbagent sync branch-unlink [--directory DIR]
kbagent sync branch-status [--directory DIR]
Expand Down
6 changes: 6 additions & 0 deletions plugins/kbagent/skills/kbagent/references/gotchas.md
Original file line number Diff line number Diff line change
Expand Up @@ -3514,6 +3514,12 @@ things to internalise:
new/empty target. Re-running clone with the same `--target-dir` is idempotent
(`no_changes`), because after the first push the local manifest carries the new
ULIDs that match the target remote.
- Override files (`--bucket-map` / `--variable-values` / `--instance-rename`)
must be **flat `{id: scalar}` mappings** (since v0.89.0). A nested value —
one fat-fingered colon away from valid YAML, e.g. `in.c-old:` followed by an
indented `new: in.c-new` — is rejected with `CONFIG_ERROR` (exit 5) naming
the offending key and its actual type. Older versions silently stringified
it (`str(dict)` → `"{'new': 'in.c-new'}"`) and pushed that as a "bucket ID".

### `search --regex` matches entity names only; `matched_columns` is textual-only (since v0.67.0)

Expand Down
5 changes: 5 additions & 0 deletions plugins/kbagent/skills/kbagent/references/sync-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,11 @@ Override files are JSON or YAML objects:
- `--variable-values` → `{ "db_host": "prod-db", "api_base": "https://..." }` — overrides matching `keboola.variables` row values.
- `--instance-rename` → `{ "extractor/keboola.ex-db/Acme": "extractor/keboola.ex-db/Globex" }` — renames config dirs + manifest paths.

All three files must be **flat `{id: scalar}` mappings** (since v0.89.0) — a
nested mapping, list, or empty (`null`) value is rejected with `CONFIG_ERROR`
(exit 5) naming the offending key and its actual type, instead of being
silently stringified into a bogus ID.

**Why it just works on a fresh target:** the reference's config ids do not exist
in the target project, so the push diff classifies every config as `added` and
assigns new ULIDs. Because the push's `created_id_map` is keyed by the reference
Expand Down
9 changes: 9 additions & 0 deletions src/keboola_agent_cli/changelog.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@
# Ordered newest-first. Each value is a list of brief one-line descriptions.
CHANGELOG: dict[str, list[str]] = {
"0.89.0": [
"Fix (#646): `sync clone` now rejects a malformed override file instead of "
"silently pushing garbage IDs. The `--bucket-map` / `--variable-values` / "
"`--instance-rename` loader used to coerce every YAML value with bare "
"`str(value)`, so a nested mapping -- one fat-fingered colon away from valid "
"input -- was used as the literal string `\"{'new': 'in.c-new'}\"` in the "
"target project. A mapping, list, or empty (`null`) value now fails fast with "
"`CONFIG_ERROR` (exit 5) naming the offending key and its actual type in YAML "
"vocabulary (mapping/list/null). Scalar values (string, number, boolean) keep "
"the existing string coercion.",
"Fix: `config delete` can no longer permanently purge a configuration by being run "
"twice. The Storage API overloads DELETE -- on a live configuration it soft-deletes "
"into the trash, but on a configuration ALREADY in the trash the same call purges it "
Expand Down
18 changes: 5 additions & 13 deletions src/keboola_agent_cli/commands/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,21 +52,13 @@ def _resolve_project_root(directory: Path, alias: str | None = None) -> Path:
def _load_override_file(path: Path) -> dict[str, str]:
"""Load a clone override map (JSON or YAML) as a flat ``{str: str}`` dict.

YAML's loader also parses JSON, so a single path handles both. Every value
is coerced to ``str`` (bucket ids, variable values, and path prefixes are
all strings).
Values must be scalars (bucket ids, variable values, and path prefixes are
all strings); a nested mapping or list raises instead of being silently
stringified into a bogus ID.
"""
import yaml
from ..yaml_input import load_flat_scalar_mapping

if not path.exists():
raise ConfigError(f"Override file not found: {path}")
try:
data = yaml.safe_load(path.read_text(encoding="utf-8"))
except yaml.YAMLError as exc:
raise ConfigError(f"Cannot parse override file {path}: {exc}") from exc
if not isinstance(data, dict):
raise ConfigError(f"Override file {path} must contain a JSON/YAML object (mapping).")
return {str(key): str(value) for key, value in data.items()}
return load_flat_scalar_mapping(path, label="override file")


def _change_label(change: dict) -> str:
Expand Down
19 changes: 2 additions & 17 deletions src/keboola_agent_cli/services/_describe_batch_input.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,7 @@
from pathlib import Path
from typing import Any, NamedTuple

# YAML-flavoured names for the types a section can wrongly be: the author is
# reading their own YAML, not Python, so "string"/"number" beat "str"/"int".
_TYPE_NAMES: dict[type, str] = {
bool: "boolean",
dict: "mapping",
float: "number",
int: "number",
list: "list",
str: "string",
type(None): "null",
}
from ..yaml_input import yaml_type_name


class _Section(NamedTuple):
Expand Down Expand Up @@ -102,15 +92,10 @@ def total(self) -> int:
return len(self.buckets) + len(self.tables) + len(self.columns)


def _type_name(value: Any) -> str:
"""Return ``value``'s type named in YAML vocabulary."""
return _TYPE_NAMES.get(type(value), type(value).__name__)


def _shape_error(key: str, expected: str, value: Any, example: str) -> ValueError:
"""Build the ValueError for one wrongly-shaped key."""
return ValueError(
f"'{key}' must be {expected}, got a {_type_name(value)}. Expected:\n{example}"
f"'{key}' must be {expected}, got a {yaml_type_name(value)}. Expected:\n{example}"
)


Expand Down
77 changes: 77 additions & 0 deletions src/keboola_agent_cli/yaml_input.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
"""Shared shape validation for flat ``{id: scalar}`` YAML/JSON input files.

Several commands accept a small user-authored mapping file -- ``sync clone``'s
``--bucket-map`` / ``--variable-values`` / ``--instance-rename`` are the
canonical case. Coercing every value with bare ``str(value)`` silently turns a
nested mapping (one fat-fingered colon away from valid input) into the literal
string ``"{'new': 'in.c-new'}"``, which then lands in the target project as a
"bucket ID". This module rejects non-scalar values (and ``None``) up front,
naming the offending key and its actual type -- same approach as the
``storage describe-batch --from-file`` validation in
``services/_describe_batch_input.py``.

Type names come out in YAML vocabulary ("mapping", "list", "null"), not
Python's -- the author is reading their own YAML file, not a traceback.
"""

from __future__ import annotations

from pathlib import Path
from typing import Any

from .errors import ConfigError

_TYPE_NAMES: dict[type, str] = {
bool: "boolean",
dict: "mapping",
float: "number",
int: "number",
list: "list",
str: "string",
type(None): "null",
}


def yaml_type_name(value: Any) -> str:
"""Return ``value``'s type named in YAML vocabulary."""
return _TYPE_NAMES.get(type(value), type(value).__name__)


def load_flat_scalar_mapping(path: Path, *, label: str = "input file") -> dict[str, str]:
"""Load a JSON/YAML file as a flat ``{str: str}`` mapping, or raise.

YAML's loader also parses JSON, so a single path handles both. Scalar
values (string, number, boolean) are coerced to ``str``; a container or
``None`` value raises instead of being stringified.

Args:
path: The file to load.
label: What the file is, for error messages (e.g. "override file").

Raises:
ConfigError: The file is missing, is not valid YAML/JSON, is not a
mapping at the top level, or any value is not a scalar.
"""
import yaml

if not path.exists():
raise ConfigError(f"{label.capitalize()} not found: {path}")
try:
data = yaml.safe_load(path.read_text(encoding="utf-8"))
except yaml.YAMLError as exc:
raise ConfigError(f"Cannot parse {label} {path}: {exc}") from exc
if not isinstance(data, dict):
raise ConfigError(
f"{label.capitalize()} {path} must contain a JSON/YAML object (mapping), "
f"got a {yaml_type_name(data)}."
)
result: dict[str, str] = {}
for key, value in data.items():
if value is None or isinstance(value, dict | list):
raise ConfigError(
f"'{key}' in {label} {path} must be a single scalar value "
f"(string, number or boolean), got a {yaml_type_name(value)}. "
f"Check the file for a stray colon or missing value."
)
result[str(key)] = str(value)
return result
37 changes: 37 additions & 0 deletions tests/test_sync_clone.py
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,43 @@ def test_forwards_args_and_overrides(self, tmp_path: Path) -> None:
assert call["overrides"]["bucket_map"] == {"in.c-ref": "in.c-prod"}
assert "Cloned into target" in result.output

def test_nested_override_value_errors(self, tmp_path: Path) -> None:
"""A non-scalar override value (fat-fingered colon) is rejected, not stringified."""
from unittest.mock import patch

from typer.testing import CliRunner

from keboola_agent_cli.cli import app

runner = CliRunner()
source = tmp_path / "golden"
_golden_source(source)
bmap = tmp_path / "buckets.yaml"
bmap.write_text("in.c-ref:\n new: in.c-prod\n", encoding="utf-8")

with patch("keboola_agent_cli.cli.SyncService") as MockSync:
svc = MagicMock()
MockSync.return_value = svc
result = runner.invoke(
app,
[
"sync",
"clone",
"--source",
str(source),
"--target",
"target",
"--target-dir",
str(tmp_path / "clone"),
"--bucket-map",
str(bmap),
],
)
assert result.exit_code == 5
assert "in.c-ref" in result.output
assert "mapping" in result.output
svc.clone_project.assert_not_called()

def test_missing_override_file_errors(self, tmp_path: Path) -> None:
from unittest.mock import patch

Expand Down
93 changes: 93 additions & 0 deletions tests/test_yaml_input.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
"""Unit tests for the shared flat {id: scalar} YAML-mapping loader (yaml_input.py).

The loader backs `sync clone --bucket-map/--variable-values/--instance-rename`.
Before it existed, `commands/sync.py` coerced every value with bare
``str(value)``, so a nested mapping (a fat-fingered colon in the YAML) was
silently used as the literal string ``"{'new': 'in.c-new'}"`` -- a wrong
bucket ID in the target project instead of a rejection.
"""

from pathlib import Path

import pytest

from keboola_agent_cli.errors import ConfigError
from keboola_agent_cli.yaml_input import load_flat_scalar_mapping, yaml_type_name


class TestYamlTypeName:
"""Type names come out in YAML vocabulary, not Python's."""

@pytest.mark.parametrize(
("value", "expected"),
[
({"a": 1}, "mapping"),
(["a"], "list"),
(None, "null"),
("x", "string"),
(42, "number"),
(4.2, "number"),
(True, "boolean"),
],
)
def test_known_types(self, value: object, expected: str) -> None:
assert yaml_type_name(value) == expected

def test_unknown_type_falls_back_to_python_name(self) -> None:
class Weird:
pass

assert yaml_type_name(Weird()) == "Weird"


class TestLoadFlatScalarMapping:
def _write(self, tmp_path: Path, content: str) -> Path:
path = tmp_path / "map.yaml"
path.write_text(content, encoding="utf-8")
return path

def test_valid_json(self, tmp_path: Path) -> None:
path = self._write(tmp_path, '{"in.c-old": "in.c-new"}')
assert load_flat_scalar_mapping(path) == {"in.c-old": "in.c-new"}

def test_valid_yaml_scalars_coerced_to_str(self, tmp_path: Path) -> None:
path = self._write(tmp_path, "retries: 3\nname: prod\n")
assert load_flat_scalar_mapping(path) == {"retries": "3", "name": "prod"}

def test_nested_mapping_value_rejected(self, tmp_path: Path) -> None:
path = self._write(tmp_path, "in.c-old:\n new: in.c-new\n")
with pytest.raises(ConfigError, match=r"'in\.c-old'.*mapping"):
load_flat_scalar_mapping(path)

def test_list_value_rejected(self, tmp_path: Path) -> None:
path = self._write(tmp_path, "in.c-old:\n - in.c-new\n")
with pytest.raises(ConfigError, match=r"'in\.c-old'.*list"):
load_flat_scalar_mapping(path)

def test_null_value_rejected(self, tmp_path: Path) -> None:
path = self._write(tmp_path, "in.c-old:\n")
with pytest.raises(ConfigError, match=r"'in\.c-old'.*null"):
load_flat_scalar_mapping(path)

def test_top_level_list_rejected(self, tmp_path: Path) -> None:
path = self._write(tmp_path, "- in.c-old\n- in.c-new\n")
with pytest.raises(ConfigError, match="mapping"):
load_flat_scalar_mapping(path)

def test_empty_file_rejected(self, tmp_path: Path) -> None:
path = self._write(tmp_path, "")
with pytest.raises(ConfigError, match="mapping"):
load_flat_scalar_mapping(path)

def test_missing_file(self, tmp_path: Path) -> None:
with pytest.raises(ConfigError, match="not found"):
load_flat_scalar_mapping(tmp_path / "nope.yaml")

def test_invalid_yaml(self, tmp_path: Path) -> None:
path = self._write(tmp_path, "key: [unclosed\n")
with pytest.raises(ConfigError, match=r"[Cc]annot parse"):
load_flat_scalar_mapping(path)

def test_label_used_in_messages(self, tmp_path: Path) -> None:
with pytest.raises(ConfigError, match=r"[Oo]verride file not found"):
load_flat_scalar_mapping(tmp_path / "nope.yaml", label="override file")