From 3731dc7c3d0e9e07957dded431828b21d68266b3 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:49:54 +0100 Subject: [PATCH 1/2] fix(apply): refuse an unknown --build-id before tofu apply `fluid apply --mode amend-and-build --build-id ` (and replace-and-build) checked the id only in the build runner, which keeps the builds whose id equals it and, with none left, logs "Build not found" and returns 1. On the OpenTofu engine the runner starts after `tofu apply`, so a mistyped id provisioned the product and then failed with a log line and exit 1. apply now runs the runner's match in a build mode, right after the build-id grammar gate, ahead of the apply hooks, tofu and the runner, on both engines. The refusal is a typed CLIError, apply_build_id_not_found, that names the id, lists the contract's build ids and says nothing was applied. Ids match exactly, as in the runner. An empty --build-id and a contract without builds are not checked, as before. The runner keeps its own check as the backstop. The Command Center run now reports apply_build_id_not_found, where it reported build_not_found: after the infrastructure was applied. --- fluid_build/_error_catalog.py | 8 + fluid_build/build_runners/base.py | 6 +- fluid_build/cli/apply.py | 47 ++++- tests/cli/test_apply_build_ids_before_tofu.py | 175 +++++++++++++++++- .../test_apply_reports_to_command_center.py | 12 +- 5 files changed, 241 insertions(+), 7 deletions(-) diff --git a/fluid_build/_error_catalog.py b/fluid_build/_error_catalog.py index 8918e49d..405eb3ba 100644 --- a/fluid_build/_error_catalog.py +++ b/fluid_build/_error_catalog.py @@ -276,6 +276,14 @@ def slug_for(event: str) -> str: ], None, ), + "apply_build_id_not_found": ( + [ + "Pass one of the available build ids as --build-id: ids match exactly, case included", + "Run 'fluid apply --mode amend-and-build' without --build-id to run " + + "every build", + ], + None, + ), "output_write_failed": ( ["Check that the --out path is writable and the parent directory exists"], None, diff --git a/fluid_build/build_runners/base.py b/fluid_build/build_runners/base.py index 3fd4e3af..78da7ebb 100644 --- a/fluid_build/build_runners/base.py +++ b/fluid_build/build_runners/base.py @@ -1411,7 +1411,11 @@ def run_builds_from_args( report = current_apply_run() product_id = str(contract.get("id") or "") - # Filter builds if specific ID requested + # Filter builds if specific ID requested. ``fluid apply`` refuses an id no + # build has before it applies anything + # (``cli/apply.py::_gate_selected_build_for_apply``), so on the OpenTofu + # engine it is not refused here after ``tofu apply``; this one stays as the + # backstop for any caller that skips it. if args.build_id: builds = [b for b in builds if b.get("id") == args.build_id] if not builds: diff --git a/fluid_build/cli/apply.py b/fluid_build/cli/apply.py index 40b80600..c702ea26 100644 --- a/fluid_build/cli/apply.py +++ b/fluid_build/cli/apply.py @@ -346,6 +346,47 @@ def _gate_build_ids_for_apply(contract: Dict[str, Any]) -> None: ) from exc +def _gate_selected_build_for_apply(contract: Dict[str, Any], build_id: Optional[str]) -> None: + """Refuse, before anything is applied, a ``--build-id`` that names no build. + + The build runner keeps the builds whose id equals ``--build-id`` and, with + none left, logs "Build not found" and returns 1. On the OpenTofu engine it + runs after ``tofu apply``, so a mistyped id provisioned the product and + then failed. This is the runner's own match + (``build_runners.base.run_builds_from_args``), run ahead of the hooks, + ``tofu`` and the runner on both engines. Like the runner, it checks nothing + for an empty ``--build-id``, which filters nothing, or for a contract with + no builds, where the runner returns before it filters. + + Raises: + CLIError: ``apply_build_id_not_found``. + """ + builds = contract.get("builds") or [] + if not (isinstance(build_id, str) and build_id) or not builds: + return + # The runner's filter: an exact match, case and spaces included. + if any(isinstance(build, dict) and build.get("id") == build_id for build in builds): + return + available = [build["id"] for build in builds if isinstance(build, dict) and build.get("id")] + if available: + why, fix = "no build of the contract has that id", "Pass one of the available ids" + else: + why, fix = "none of the contract's builds declares an id", "Give the build you mean an id" + raise CLIError( + 1, + "apply_build_id_not_found", + { + "build_id": repr(build_id), + "available": available, + "hint": ( + "--build-id selects the build whose id equals it exactly, case and spaces " + f"included, and {why}. {fix}, or drop --build-id to run every build. " + "Nothing was applied." + ), + }, + ) + + def _log_apply_failure(logger: logging.Logger, start_time: float, error: Exception) -> None: """Record a pre-dispatch ``apply_contract`` failure for the structured log.""" log_operation_failure(logger, "apply_contract", error=error, duration=time.time() - start_time) @@ -1962,12 +2003,14 @@ def run(args, logger: logging.Logger) -> int: # skip it, so a schema-invalid contract reached ``tofu`` or the build # runner and could exit 0. One call per run, on the one loaded copy. # A build mode also checks the ids the build phase turns into paths, which - # the schema admits more of: on the OpenTofu engine the runner would only - # refuse one after ``tofu apply``. + # the schema admits more of, and then that ``--build-id`` names a build, in + # the runner's order: on the OpenTofu engine the runner would only refuse + # either after ``tofu apply``. try: _gate_contract_for_apply(source_contract, logger) if needs_build(resolved_mode): _gate_build_ids_for_apply(source_contract) + _gate_selected_build_for_apply(source_contract, resolved_build_id) except CLIError as exc: _log_apply_failure(logger, start_time, exc) raise diff --git a/tests/cli/test_apply_build_ids_before_tofu.py b/tests/cli/test_apply_build_ids_before_tofu.py index a68f329d..af4086f0 100644 --- a/tests/cli/test_apply_build_ids_before_tofu.py +++ b/tests/cli/test_apply_build_ids_before_tofu.py @@ -22,6 +22,10 @@ ``--mode amend-and-build`` provisioned the product and then failed with "Unexpected error", because ``IdentifierViolation`` had no typed mapping. +A ``--build-id`` that names no build failed late the same way: the runner keeps the +builds whose id equals it and, with none left, logs ``Build not found`` and returns 1, +after ``tofu apply`` on the OpenTofu engine. + These tests drive ``run()`` through the real argparse surface. ``tofu`` and the apply hooks are recorders; the build runner is the real one, behind a spy, so "refused first" is asserted as "``tofu``, the hooks and the runner never ran". @@ -33,7 +37,7 @@ import json import logging from pathlib import Path -from typing import Any, Dict, Iterator +from typing import Any, Dict, Iterator, Optional from unittest import mock import pytest @@ -355,3 +359,172 @@ def test_the_runner_still_refuses_on_its_own(self, tmp_path: Path) -> None: ) def test_what_it_does_not_check(self, contract: Dict[str, Any]) -> None: _ids.validate_build_ids(contract) + + +# ``--build-id`` values that name no build of ``_CONTRACT``. The runner compares ids +# exactly, so a near miss is as unknown as a typo. +_UNKNOWN_BUILD_IDS = [ + pytest.param("make_row", id="typo"), + pytest.param("Make_Rows", id="case"), + pytest.param("make_rows ", id="trailing-space"), + pytest.param("make", id="prefix"), +] + + +def _with_build_ids(*build_ids: Optional[str]) -> Dict[str, Any]: + """``_CONTRACT`` with one build per id; ``None`` is a build that declares no id.""" + contract = copy.deepcopy(_CONTRACT) + template = contract["builds"][0] + contract["builds"] = [] + for build_id in build_ids: + build = copy.deepcopy(template) + if build_id is None: + del build["id"] + else: + build["id"] = build_id + contract["builds"].append(build) + return contract + + +@pytest.mark.usefixtures("workspace") +class TestAnUnknownBuildIdIsRefusedFirst: + """``--build-id`` names no build of the contract. + + The runner keeps the builds whose id equals ``--build-id`` and, with none left, + logs ``Build not found`` and returns 1. On the OpenTofu engine that came after + ``tofu apply``: a mistyped id provisioned the product, then the run failed. + """ + + @pytest.mark.parametrize("form", ["yaml", "plan"]) + @pytest.mark.parametrize("build_id", _UNKNOWN_BUILD_IDS) + @pytest.mark.parametrize("mode,extra", _BUILD_MODES) + def test_refused_before_tofu( + self, + tofu: mock.Mock, + hooks: mock.Mock, + runner: mock.Mock, + form: str, + build_id: str, + mode: str, + extra: Any, + ) -> None: + src = _source(form, _CONTRACT, mode=mode) + with pytest.raises(CLIError) as exc: + _apply(src, "--mode", mode, *extra, "--build-id", build_id) + tofu.assert_not_called() + hooks.assert_not_called() + runner.assert_not_called() + assert exc.value.event == "apply_build_id_not_found" + assert exc.value.context["build_id"] == repr(build_id) + assert exc.value.context["available"] == ["make_rows"] + + @pytest.mark.usefixtures("native") + def test_native_engine_refuses_with_the_same_typed_error(self, runner: mock.Mock) -> None: + """The native engine applies nothing before its builds, but its refusal was a + log line and exit 1.""" + src = _source("yaml", _CONTRACT, mode="amend-and-build") + with pytest.raises(CLIError) as exc: + _apply(src, "--mode", "amend-and-build", "--build-id", "make_row") + runner.assert_not_called() + assert exc.value.event == "apply_build_id_not_found" + + def test_it_lists_the_build_ids_in_contract_order(self, tofu: mock.Mock) -> None: + src = _source("yaml", _with_build_ids("make_rows", "load_orders"), mode="amend-and-build") + with pytest.raises(CLIError) as exc: + _apply(src, "--mode", "amend-and-build", "--build-id", "load_order") + assert exc.value.context["available"] == ["make_rows", "load_orders"] + + def test_builds_that_declare_no_id_match_no_build_id( + self, tofu: mock.Mock, runner: mock.Mock + ) -> None: + """``builds[].id`` is optional, and the runner matches no ``--build-id`` to a + build without one.""" + src = _source("yaml", _with_build_ids(None), mode="amend-and-build") + with pytest.raises(CLIError) as exc: + _apply(src, "--mode", "amend-and-build", "--build-id", "make_rows") + tofu.assert_not_called() + runner.assert_not_called() + assert exc.value.context["available"] == [] + assert "none of the contract's builds declares an id" in exc.value.context["hint"] + + def test_an_unusable_id_is_refused_first(self, tofu: mock.Mock) -> None: + """The runner checks the ids before it filters, and apply keeps that order.""" + src = _source("yaml", _with("contract.id", "1orders"), mode="amend-and-build") + with pytest.raises(CLIError) as exc: + _apply(src, "--mode", "amend-and-build", "--build-id", "make_row") + assert exc.value.event == "apply_build_identifier_invalid" + + def test_the_refusal_says_what_to_change(self, tofu: mock.Mock) -> None: + src = _source("yaml", _CONTRACT, mode="amend-and-build") + with pytest.raises(CLIError) as exc: + _apply(src, "--mode", "amend-and-build", "--build-id", "make_row") + hint = exc.value.context["hint"] + assert "Nothing was applied" in hint + assert "drop --build-id" in hint + assert exc.value.suggestions, "the catalog gives the event its suggestions" + + def test_the_cli_renders_it_as_a_typed_error( + self, tofu: mock.Mock, capsys: pytest.CaptureFixture[str] + ) -> None: + """Through ``main()``: the slug, the id asked for and the ids there are.""" + src = _source("yaml", _CONTRACT, mode="amend-and-build") + argv = ["apply", src, "--mode", "amend-and-build", "--build-id", "make_row", "--yes"] + assert cli_mod.main(argv) == 1 + out = capsys.readouterr().out + assert "ERR_APPLY_BUILD_ID_NOT_FOUND" in out + assert "'make_row'" in out + assert "['make_rows']" in out + tofu.assert_not_called() + + @pytest.mark.parametrize("build_id", _UNKNOWN_BUILD_IDS) + def test_the_runner_still_refuses_on_its_own( + self, tmp_path: Path, caplog: pytest.LogCaptureFixture, build_id: str + ) -> None: + """Kept as the backstop for a caller that skips ``fluid apply``. It refuses the + same near misses: both compare ids exactly.""" + import argparse + + path = tmp_path / "contract.fluid.yaml" + path.write_text("{}", encoding="utf-8") + args = argparse.Namespace(contract=str(path), env=None, build_id=build_id) + with caplog.at_level(logging.ERROR, logger=LOG.name): + rc = runner_base.run_builds_from_args(args, LOG, contract=copy.deepcopy(_CONTRACT)) + assert rc == 1 + assert f"Build not found: {build_id}" in caplog.text + + +@pytest.mark.usefixtures("workspace") +class TestAKnownBuildIdStillRuns: + """The check refuses exactly the ``--build-id`` values the runner would refuse.""" + + @pytest.mark.parametrize("form", ["yaml", "plan"]) + @pytest.mark.parametrize("build_id", ["make_rows", "load_orders"]) + def test_it_reaches_tofu_then_the_runner( + self, tofu: mock.Mock, form: str, build_id: str + ) -> None: + src = _source(form, _with_build_ids("make_rows", "load_orders"), mode="amend-and-build") + with mock.patch("fluid_build.build_runners.run_builds_from_args", return_value=0) as run: + assert _apply(src, "--mode", "amend-and-build", "--build-id", build_id) == 0 + tofu.assert_called_once() + run.assert_called_once() + assert run.call_args.args[0].build_id == build_id + + def test_an_empty_build_id_filters_nothing(self, tofu: mock.Mock) -> None: + """The runner runs every build for ``--build-id ''``, so it is not refused.""" + src = _source("yaml", _CONTRACT, mode="amend-and-build") + with mock.patch("fluid_build.build_runners.run_builds_from_args", return_value=0) as run: + assert _apply(src, "--mode", "amend-and-build", "--build-id", "") == 0 + tofu.assert_called_once() + run.assert_called_once() + + def test_a_contract_without_builds_is_not_checked( + self, tofu: mock.Mock, runner: mock.Mock + ) -> None: + """With no builds the runner returns 0 before it filters, so a ``--build-id`` is + not refused there either.""" + contract = copy.deepcopy(_CONTRACT) + del contract["builds"] + src = _source("yaml", contract, mode="amend-and-build") + assert _apply(src, "--mode", "amend-and-build", "--build-id", "make_rows") == 0 + tofu.assert_called_once() + runner.assert_called_once() diff --git a/tests/cli/test_apply_reports_to_command_center.py b/tests/cli/test_apply_reports_to_command_center.py index c60e76a4..80782468 100644 --- a/tests/cli/test_apply_reports_to_command_center.py +++ b/tests/cli/test_apply_reports_to_command_center.py @@ -592,15 +592,21 @@ def test_a_failed_build_is_reported_with_its_build_id(product, tofu, build, cc, def test_a_build_id_that_is_not_in_the_contract_is_the_reason( product, tofu, build, cc, monkeypatch ): + """Refused before ``tofu`` runs: the run reports the refusal and no infrastructure.""" url, recorder = cc _configure(monkeypatch, url) _with_build(product) - assert _apply(product, "--mode", "amend-and-build", "--build-id", "no_such_build") == 1 + with pytest.raises(CLIError) as exc: + _apply(product, "--mode", "amend-and-build", "--build-id", "no_such_build") + assert exc.value.event == "apply_build_id_not_found" + (post,) = _by(recorder, "POST") (patch,) = _by(recorder, "PATCH") - assert patch["body"]["result"]["error_event"] == "build_not_found:no_such_build" - assert patch["body"]["current_phase"] == "build" + assert post["body"]["metadata"]["product_id"] == "bronze.customer_subscriptions" + result = patch["body"]["result"] + assert result["error_event"] == "apply_build_id_not_found" + assert "planned_changes" not in result assert build["calls"] == 0 From 6ad9351e5fe71eda7582bd611f8b802317c032a1 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:49:55 +0100 Subject: [PATCH 2/2] docs(changelog): an unknown --build-id is refused before tofu apply --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f08f2503..3637d9f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -144,6 +144,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 engines, and refuses with `ERR_APPLY_BUILD_IDENTIFIER_INVALID`, which names the id, says nothing was applied and suggests a rename. A mode without a build phase, and a contract without builds, are not checked, as before. +- **A mistyped `--build-id` provisioned the product, then found no build.** The build runner keeps + the builds whose id equals `--build-id` and, with none left, logged `Build not found` and exited + 1, but on the OpenTofu engine it starts after `tofu apply`, so the infrastructure was created + first. A build mode now checks `--build-id` against the contract's builds before the apply + hooks, `tofu` and the runner, on both engines, and refuses with `ERR_APPLY_BUILD_ID_NOT_FOUND`, + which names the id, lists the ids the contract has and says nothing was applied. Ids match + exactly, as in the runner. An empty `--build-id` and a contract without builds are not checked, + as before. The Command Center run now reports `apply_build_id_not_found`, where it reported + `build_not_found:`. - **`fluid diff --baseline` reported nothing for column changes in a 0.7.x contract.** It read columns from `exposes[].schema`, but a 0.7.x contract keeps them in `exposes[].contract.schema`