From 23e376a854d4bcb933ece6f1f85d4d9a574531ca Mon Sep 17 00:00:00 2001 From: Jeffrey Aven Date: Thu, 10 Sep 2026 11:57:43 +1000 Subject: [PATCH 1/2] Add skip_on_delete, harden teardown, wire --on-failure ignore, add integration tests, bump to 2.2.0 --- .github/workflows/ci.yml | 4 + .github/workflows/integration-tests.yml | 60 +++ .github/workflows/test-demo.yml | 68 ---- CHANGELOG.md | 21 ++ Cargo.lock | 2 +- Cargo.toml | 16 +- ci-scripts/integration-test.sh | 37 ++ ci-scripts/test.sh | 16 +- docs/flows.md | 14 +- src/commands/base.rs | 100 +++-- src/commands/build.rs | 10 +- src/commands/teardown.rs | 346 +++++++++++++----- src/commands/test.rs | 17 +- src/core/config.rs | 5 +- src/core/templating.rs | 82 +++++ src/core/utils.rs | 92 ++++- src/lib.rs | 21 ++ src/main.rs | 16 +- src/resource/manifest.rs | 44 +++ src/resource/validation.rs | 1 + tests/README.md | 96 +++++ tests/build.rs | 127 +++++++ tests/common/cli.rs | 267 ++++++++++++++ tests/common/fake_cloud.rs | 271 ++++++++++++++ tests/common/mock_server.rs | 260 +++++++++++++ tests/common/mod.rs | 152 ++++++++ .../manifest_skip_query.yml | 56 +++ .../manifest_skip_resource.yml | 56 +++ .../manifest_with_condition.yml | 56 +++ .../manifest_with_delete_callback.yml | 58 +++ .../manifest_with_script.yml | 61 +++ .../resources/current_user.iql | 4 + .../resources/storage_credential.iql | 31 ++ .../resources/workspace.iql | 32 ++ .../resources/workspace_with_callback.iql | 39 ++ .../databricks_workspace/stackql_manifest.yml | 58 +++ tests/live.rs | 317 ++++++++++++++++ .../aws_ssm/resources/app_config_tags.json | 4 + .../aws_ssm/resources/aws_get_parameter.iql | 7 + .../aws_ssm/resources/aws_parameter.iql | 21 ++ .../aws_ssm/resources/awscc_parameter.iql | 80 ++++ .../live_stacks/aws_ssm/stackql_manifest.yml | 191 ++++++++++ .../aws_ssm_onfailure/resources/ghost.iql | 10 + .../resources/ghost_parameter.iql | 17 + .../aws_ssm_onfailure/stackql_manifest.yml | 49 +++ .../github_labels/resources/label.iql | 57 +++ .../github_labels/stackql_manifest.yml | 63 ++++ tests/teardown.rs | 307 ++++++++++++++++ tests/test_command.rs | 102 ++++++ website/docs/cli-reference/teardown.md | 9 + website/docs/manifest-file.md | 6 + website/docs/manifest_fields/index.js | 1 + .../resources/skipondelete.mdx | 35 ++ 53 files changed, 3633 insertions(+), 239 deletions(-) create mode 100644 .github/workflows/integration-tests.yml delete mode 100644 .github/workflows/test-demo.yml create mode 100644 ci-scripts/integration-test.sh create mode 100644 src/lib.rs create mode 100644 tests/README.md create mode 100644 tests/build.rs create mode 100644 tests/common/cli.rs create mode 100644 tests/common/fake_cloud.rs create mode 100644 tests/common/mock_server.rs create mode 100644 tests/common/mod.rs create mode 100644 tests/fixtures/databricks_workspace/manifest_skip_query.yml create mode 100644 tests/fixtures/databricks_workspace/manifest_skip_resource.yml create mode 100644 tests/fixtures/databricks_workspace/manifest_with_condition.yml create mode 100644 tests/fixtures/databricks_workspace/manifest_with_delete_callback.yml create mode 100644 tests/fixtures/databricks_workspace/manifest_with_script.yml create mode 100644 tests/fixtures/databricks_workspace/resources/current_user.iql create mode 100644 tests/fixtures/databricks_workspace/resources/storage_credential.iql create mode 100644 tests/fixtures/databricks_workspace/resources/workspace.iql create mode 100644 tests/fixtures/databricks_workspace/resources/workspace_with_callback.iql create mode 100644 tests/fixtures/databricks_workspace/stackql_manifest.yml create mode 100644 tests/live.rs create mode 100644 tests/live_stacks/aws_ssm/resources/app_config_tags.json create mode 100644 tests/live_stacks/aws_ssm/resources/aws_get_parameter.iql create mode 100644 tests/live_stacks/aws_ssm/resources/aws_parameter.iql create mode 100644 tests/live_stacks/aws_ssm/resources/awscc_parameter.iql create mode 100644 tests/live_stacks/aws_ssm/stackql_manifest.yml create mode 100644 tests/live_stacks/aws_ssm_onfailure/resources/ghost.iql create mode 100644 tests/live_stacks/aws_ssm_onfailure/resources/ghost_parameter.iql create mode 100644 tests/live_stacks/aws_ssm_onfailure/stackql_manifest.yml create mode 100644 tests/live_stacks/github_labels/resources/label.iql create mode 100644 tests/live_stacks/github_labels/stackql_manifest.yml create mode 100644 tests/teardown.rs create mode 100644 tests/test_command.rs create mode 100644 website/docs/manifest_fields/resources/skipondelete.mdx diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eb547bd..1ca5a1f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,6 +5,8 @@ on: branches: [main] paths: - 'src/**' + - 'tests/**' + - 'ci-scripts/**' - 'build.rs' - 'Cargo.toml' - 'Cargo.lock' @@ -13,6 +15,8 @@ on: branches: [main] paths: - 'src/**' + - 'tests/**' + - 'ci-scripts/**' - 'build.rs' - 'Cargo.toml' - 'Cargo.lock' diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml new file mode 100644 index 0000000..ab73545 --- /dev/null +++ b/.github/workflows/integration-tests.yml @@ -0,0 +1,60 @@ +name: Integration Tests + +# Live provider tests: the real stackql-deploy binary against AWS (SSM +# Parameter Store) and GitHub (repository labels), using free resources only. +# Intended as a required status check on pull requests to main. +# +# Repository secrets: AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY (an IAM +# principal allowed to manage SSM parameters under /stackql-deploy/ and to +# call s3:DeleteBucket, which is expected to fail). The GitHub stack uses the +# workflow's own GITHUB_TOKEN. + +on: + pull_request: + branches: [main] + paths: + - 'src/**' + - 'tests/**' + - 'ci-scripts/**' + - 'build.rs' + - 'Cargo.toml' + - 'Cargo.lock' + - '.github/workflows/integration-tests.yml' + workflow_dispatch: + +# One live run at a time: the stacks use a per-run name suffix, but serial +# execution keeps provider rate limits and the label namespace predictable. +concurrency: + group: integration-tests + cancel-in-progress: false + +permissions: + contents: read + issues: write + +jobs: + live: + name: Live provider tests + # Secrets are not exposed to pull requests from forks; skip rather than fail. + if: github.event_name == 'workflow_dispatch' || github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-latest + timeout-minutes: 30 + env: + AWS_REGION: us-east-1 + AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} + AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} + STACKQL_GITHUB_USERNAME: github-actions + STACKQL_GITHUB_PASSWORD: ${{ secrets.GITHUB_TOKEN }} + GITHUB_OWNER: ${{ github.repository_owner }} + GITHUB_REPO: ${{ github.event.repository.name }} + STACKQL_DEPLOY_LIVE_RUN_ID: gh${{ github.run_id }}-${{ github.run_attempt }} + steps: + - uses: actions/checkout@v6 + - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + - name: Install stackql + uses: stackql/setup-stackql@v2 + - name: Show stackql version + run: stackql --version + - name: Run live integration tests + run: bash ci-scripts/integration-test.sh diff --git a/.github/workflows/test-demo.yml b/.github/workflows/test-demo.yml deleted file mode 100644 index b383b83..0000000 --- a/.github/workflows/test-demo.yml +++ /dev/null @@ -1,68 +0,0 @@ -name: Test Demo - -on: - push: - branches: [main] - paths: - - 'demo/**' - - '.github/workflows/test-demo.yml' - pull_request: - branches: [main] - paths: - - 'demo/**' - - '.github/workflows/test-demo.yml' - -jobs: - test-demo: - name: Test Demo Projects - runs-on: ubuntu-latest - - steps: - - name: Placeholder deployment - run: | - echo "šŸš€ Testing Demos (placeholder)" - echo "This is a placeholder step - actual implementation pending" - # - name: Checkout repository - # uses: actions/checkout@v6 - - # - name: Install Rust toolchain - # uses: dtolnay/rust-toolchain@stable - - # - name: Build CLI - # run: cargo build --release - - # - name: Configure AWS credentials - # if: github.event_name == 'push' && github.ref == 'refs/heads/main' - # uses: aws-actions/configure-aws-credentials@v2 - # with: - # aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} - # aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} - # aws-region: us-east-1 - - # - name: Configure Azure credentials - # if: github.event_name == 'push' && github.ref == 'refs/heads/main' - # uses: azure/login@v1 - # with: - # creds: ${{ secrets.AZURE_CREDENTIALS }} - - # - name: Setup Google Cloud SDK - # if: github.event_name == 'push' && github.ref == 'refs/heads/main' - # uses: google-github-actions/setup-gcloud@v1 - # with: - # service_account_key: ${{ secrets.GCP_SA_KEY }} - # project_id: ${{ secrets.GCP_PROJECT_ID }} - - # - name: Run AWS demo tests - # run: | - # cd demo/aws - # ../../target/release/stackql-deploy test demo-stack dev --dry-run - - # - name: Run Azure demo tests - # run: | - # cd demo/azure - # ../../target/release/stackql-deploy test demo-stack dev --dry-run - - # - name: Run GCP demo tests - # run: | - # cd demo/gcp - # ../../target/release/stackql-deploy test demo-stack dev --dry-run \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 870729f..2e6b1a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,26 @@ # Changelog +## 2.2.0 (2026-09-10) + +### Features + +- `teardown --on-failure ignore` is now honoured. A `delete` statement the provider rejects is logged at `warn` level, the resource is reported as not confirmed deleted, and the teardown continues with the next resource; the default (`error`) still aborts at the first failure. Fatal errors (network, auth, planner) abort in both modes. Every teardown now ends with a summary of resources whose delete could not be confirmed. `rollback` is not meaningful for teardown and is treated as `error`. +- Added a live integration test suite (`tests/live.rs`, `tests/live_stacks/`) that runs the real binary against real providers using free resources only: AWS SSM Parameter Store parameters (Cloud Control and native API) and GitHub repository labels. It covers create with `RETURNING *`, `return_vals`, `callback`, `statecheck`, `PatchDocument` updates, `createorupdate`, `query`/`command`/`script` resources, conditions, `file()`, `merge`, per-environment values, protected masking, stack exports, `test` pass and fail, `teardown` with `skip_on_delete`, `--on-failure`, and idempotent re-runs. The tests are `#[ignore]`d for `cargo test`; `ci-scripts/integration-test.sh` runs them, and the new `Integration Tests` workflow runs them on pull requests to `main` as a merge gate (replacing the `Test Demo` placeholder workflow). See `tests/README.md`. +- Added `skip_on_delete: true` for resources ([#56](https://github.com/stackql/stackql-deploy-rs/issues/56)). A `query` resource with this flag is not executed during `teardown` and its declared exports are set to ``; a `resource` or `multi` resource with this flag still has its exports collected (a downstream `delete` may need them) but its `delete` query is not executed, so the resource is retained. The flag has no effect on `build` or `test`. +- Added an integration test suite (`tests/`) that drives the `build` and `teardown` flows against an in-process mock stackql server speaking the PostgreSQL wire protocol, with fixture stacks under `tests/fixtures/`. Tests assert on the exact statements sent and need no stackql binary, provider registry, network access, or cloud credentials. `ci-scripts/test.sh` now runs `cargo test`, and the CI workflow triggers on `tests/**` and `ci-scripts/**` changes. To support this, the crate now has a library target alongside the binary; it is not a public API. + +### Fixes + +- Teardown no longer executes queries that interpolate the `` export placeholder. When an upstream resource had already been deleted (for example by an earlier, partially successful teardown), its exports were set to `` and then substituted into downstream `exists`, `exports`, `delete`, and inline `sql` queries. For a Databricks workspace this produced `https://.cloud.databricks.com/...`, a fatal `dial tcp ... no such host` error, and an aborted teardown that could never complete. Such queries are now skipped with a log line naming the resource and anchor, and the dependent resource's exports are themselves marked `` so the skip propagates consistently. +- Teardown now tolerates a non-fatal provider error on an `exports` query (fatal network and auth errors still abort). The resource's exports are marked `` with a warning and the teardown continues; previously the error aborted the run. +- Teardown no longer aborts on stacks that contain a `script` resource. Export collection tried to load a `.iql` file for the script and exited when it was not found; script exports are now marked `` (scripts are never executed on teardown). +- Inline `sql` on `query` resources is rendered tolerantly during teardown: a missing template variable skips the query instead of exiting the process. +- The `test` command now evaluates `if` conditions (it previously processed every resource regardless) and runs `script` resources the same way `build` does (it previously exited with `unknown resource type: script`). +- A `--dry-run` teardown now renders and logs each `delete` statement. Previously the dry-run exists check reported every resource as not found, so the run only showed "skipping delete". +- The `` and `` export placeholders are no longer registered for log redaction when the export is `protected`. Previously a protected export in a dry run registered `` as a secret, which then masked every other placeholder in the run as `********`. +- Command failures that are ignored (`multi` resources, and now `--on-failure ignore`) are logged at `warn` level instead of `debug`. +- A `callback:delete` (or generic `callback`) anchor no longer aborts a `--dry-run` teardown, or a teardown whose delete returned no `RETURNING *` row. Callbacks poll the handle returned by `RETURNING *`, so they are now skipped with a log line when there is nothing to poll, matching `build`. + ## 2.1.1 (2026-08-24) ### Fixes diff --git a/Cargo.lock b/Cargo.lock index 1e219cd..11bb664 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1809,7 +1809,7 @@ checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" [[package]] name = "stackql-deploy" -version = "2.1.1" +version = "2.2.0" dependencies = [ "base64", "chrono", diff --git a/Cargo.toml b/Cargo.toml index b032b24..8c5b271 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "stackql-deploy" -version = "2.1.1" +version = "2.2.0" edition = "2021" rust-version = "1.75" description = "Infrastructure-as-code framework for declarative cloud resource management using StackQL" @@ -12,11 +12,23 @@ keywords = ["stackql", "infrastructure", "iac", "cloud", "devops"] categories = ["command-line-utilities", "development-tools"] readme = "README.md" -# docs.rs: this is a binary-only crate ā€” no public library API to document. +# docs.rs: the library target exists only so the integration tests in +# tests/ can drive the command runner in-process; it is not a public API. # The README (above) is what crates.io renders on the package page. [package.metadata.docs.rs] no-default-features = true +# Module-level doc comments contain illustrative snippets that reference +# crate-internal paths; they are not compilable examples, so doctests are off. +[lib] +name = "stackql_deploy" +path = "src/lib.rs" +doctest = false + +[[bin]] +name = "stackql-deploy" +path = "src/main.rs" + [dependencies] clap = { version = "4.3", features = ["derive"] } colored = "2.0" diff --git a/ci-scripts/integration-test.sh b/ci-scripts/integration-test.sh new file mode 100644 index 0000000..a6b37c0 --- /dev/null +++ b/ci-scripts/integration-test.sh @@ -0,0 +1,37 @@ +#!/bin/bash +set -euo pipefail + +# Runs the live integration tests: the real stackql-deploy binary against +# real providers, using free resources only (AWS SSM Parameter Store standard +# parameters and GitHub repository labels). See tests/README.md. +# +# Required in the environment: +# AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY (AWS stacks) +# STACKQL_GITHUB_USERNAME, STACKQL_GITHUB_PASSWORD (GitHub stack; the +# password is a token with issues:write on GITHUB_OWNER/GITHUB_REPO) +# Optional: +# AWS_REGION (default us-east-1), GITHUB_OWNER, GITHUB_REPO, +# STACKQL_DEPLOY_LIVE_RUN_ID (unique suffix for resource names; a local +# timestamp is used when unset), STACKQL_DEPLOY_LIVE_FILTER (run only +# tests whose name contains this string). +# +# The stackql binary must be on PATH (or in the working directory). + +echo "===============================================" +echo " Running live integration tests" +echo "===============================================" + +if ! command -v stackql >/dev/null 2>&1 && [ ! -x ./stackql ]; then + echo "error: the stackql binary was not found on PATH" >&2 + echo " install it (https://stackql.io/downloads) or run 'stackql-deploy upgrade'" >&2 + exit 1 +fi + +export STACKQL_DEPLOY_LIVE_RUN_ID="${STACKQL_DEPLOY_LIVE_RUN_ID:-local-$(date +%s)}" +echo "run id: ${STACKQL_DEPLOY_LIVE_RUN_ID}" + +# Each live stack starts its own stackql server on a dedicated port, but the +# provider cache under ~/.stackql is shared, so run the stacks one at a time. +cargo test --test live -- --ignored --test-threads=1 --nocapture "${STACKQL_DEPLOY_LIVE_FILTER:-}" + +echo -e "\nāœ… Live integration tests passed" diff --git a/ci-scripts/test.sh b/ci-scripts/test.sh index 384063a..bfc3a80 100644 --- a/ci-scripts/test.sh +++ b/ci-scripts/test.sh @@ -6,16 +6,14 @@ echo "===============================================" echo " Running Tests for stackql-deploy" echo "===============================================" -# Run unit tests +# Run unit tests (in-module #[cfg(test)] tests in src/) echo "Running unit tests..." -# cargo test --lib +cargo test --lib -# Run integration tests if they exist +# Run integration tests (tests/*.rs). These drive the build and teardown +# flows against an in-process mock stackql server - no stackql binary, +# provider registry, network access, or cloud credentials required. echo -e "\nRunning integration tests..." -# cargo test --test '*' +cargo test --test '*' -# Run doc tests -echo -e "\nRunning documentation tests..." -# cargo test --doc - -echo -e "\nāœ… All tests passed successfully!" \ No newline at end of file +echo -e "\nāœ… All tests passed successfully!" diff --git a/docs/flows.md b/docs/flows.md index d742a69..b3881b0 100644 --- a/docs/flows.md +++ b/docs/flows.md @@ -194,7 +194,19 @@ graph LR H -->|still there| I ``` -During teardown export collection, missing exports are set to `` rather than failing ā€” the stack may be partially deployed. +During teardown export collection, exports that cannot be collected are set to `` rather than failing - the stack may be partially deployed. An export ends up `` when its query returns no rows, returns a non-fatal provider error, cannot be rendered, belongs to a `script` resource (never executed on teardown), or belongs to a resource with `skip_on_delete: true`. + +Any teardown query (`exists`, `statecheck`, `exports`, `delete`, or inline `sql`) whose rendered text contains `` is skipped rather than executed. Running it would at best match nothing and at worst put the placeholder into a hostname or identifier (for example `https://.cloud.databricks.com/...`), which fails with a fatal `dial tcp` error and aborts the whole teardown. The resource is logged as skipped and processing continues with the next one. + +A `delete` statement the provider rejects aborts the teardown by default (`--on-failure error`). With `--on-failure ignore` the failure is logged, the resource is reported as not confirmed deleted, and the next resource is processed; fatal network, auth and planner errors abort in both modes. Every teardown ends with a summary of resources whose delete could not be confirmed. + +`skip_on_delete: true` opts a resource out of teardown explicitly: + +| Type | Effect on teardown | +|------|--------------------| +| `query` | Query not executed; declared exports set to `` | +| `resource` / `multi` | Exports still collected (downstream deletes may need them); `delete` not executed, resource retained | +| `command` / `script` | No change (never executed on teardown) | --- diff --git a/src/commands/base.rs b/src/commands/base.rs index 6e20195..7fe37cf 100644 --- a/src/commands/base.rs +++ b/src/commands/base.rs @@ -19,7 +19,8 @@ use crate::core::utils::{ catch_error_and_exit, check_exports_as_statecheck_proxy, check_short_circuit, export_vars, flatten_returning_row, has_returning_clause, perform_retries, perform_retries_with_fields, pull_providers, run_callback_poll, run_ext_script, run_stackql_command, - run_stackql_dml_returning, run_stackql_query, show_query, + run_stackql_dml_returning, run_stackql_query, show_query, unknown_exports_for, + DRY_RUN_EXPORT_PLACEHOLDER, }; use crate::resource::manifest::{Manifest, Resource}; use crate::resource::validation::validate_manifest; @@ -170,6 +171,18 @@ impl CommandRunner { templating::render_inline_template(&self.engine, resource_name, sql, full_context) } + /// Tolerant variant of [`Self::render_inline_template`]: returns `None` + /// instead of exiting when the inline SQL references variables that are + /// not in the context. Used during teardown. + pub fn try_render_inline_template( + &self, + resource_name: &str, + sql: &str, + full_context: &HashMap, + ) -> Option { + templating::try_render_inline_template(&self.engine, resource_name, sql, full_context) + } + /// Render a single query template JIT with the current context. pub fn render_query( &self, @@ -934,7 +947,8 @@ impl CommandRunner { if let Some(map) = item.as_mapping() { for (_, val) in map { if let Some(v) = val.as_str() { - export_data.insert(v.to_string(), "".to_string()); + export_data + .insert(v.to_string(), DRY_RUN_EXPORT_PLACEHOLDER.to_string()); } } } @@ -942,7 +956,7 @@ impl CommandRunner { } else { for item in expected_exports { if let Some(s) = item.as_str() { - export_data.insert(s.to_string(), "".to_string()); + export_data.insert(s.to_string(), DRY_RUN_EXPORT_PLACEHOLDER.to_string()); } } } @@ -969,27 +983,9 @@ impl CommandRunner { if exports.is_empty() { if ignore_missing_exports { - // During teardown, set all expected exports to so - // downstream queries can still render (the resource may - // already be partially deleted). - let mut fallback = HashMap::new(); - for item in expected_exports { - if let Some(s) = item.as_str() { - fallback.insert(s.to_string(), "".to_string()); - } else if let Some(map) = item.as_mapping() { - for (_, val) in map { - if let Some(v) = val.as_str() { - fallback.insert(v.to_string(), "".to_string()); - } - } - } - } - export_vars( - &mut self.global_context, - &resource.name, - &fallback, - protected_exports, - ); + // During teardown the resource may already be partially + // deleted; mark its exports as unknown and carry on. + self.set_exports_unknown(resource); return; } show_query(true, exports_query); @@ -997,23 +993,28 @@ impl CommandRunner { } // Check for errors - if !exports.is_empty() { - if exports[0].contains_key("_stackql_deploy_error") { - let err_msg = exports[0].get("_stackql_deploy_error").unwrap(); - show_query(true, exports_query); - catch_error_and_exit(&format!( - "Exports query failed for {}\n\nError details:\n{}", - resource.name, err_msg - )); - } - if exports[0].contains_key("error") { - let err_msg = exports[0].get("error").unwrap(); - show_query(true, exports_query); - catch_error_and_exit(&format!( - "Exports query failed for {}\n\nError details:\n{}", + if let Some(err_msg) = exports[0] + .get("_stackql_deploy_error") + .or_else(|| exports[0].get("error")) + { + if ignore_missing_exports { + // During teardown a provider error on an exports query is + // not fatal (fatal network/auth errors already exited in + // run_stackql_query): the resource may be mid-deletion or + // unreachable. Mark the exports unknown so dependants are + // skipped, and carry on. + warn!( + "exports query for [{}] failed during teardown, marking exports as unknown:\n\n{}\n", resource.name, err_msg - )); + ); + self.set_exports_unknown(resource); + return; } + show_query(true, exports_query); + catch_error_and_exit(&format!( + "Exports query failed for {}\n\nError details:\n{}", + resource.name, err_msg + )); } if exports.len() > 1 { @@ -1032,6 +1033,27 @@ impl CommandRunner { ); } + /// Set every declared export of `resource` to the teardown + /// [`crate::core::utils::UNKNOWN_EXPORT_PLACEHOLDER`] in the global + /// context. + /// + /// Called during teardown when the exports could not be collected: the + /// exports query returned no rows, could not be rendered, or the resource + /// was skipped via `skip_on_delete`. Downstream queries that interpolate + /// the placeholder are skipped rather than executed. + pub fn set_exports_unknown(&mut self, resource: &Resource) { + if resource.exports.is_empty() { + return; + } + let fallback = unknown_exports_for(&resource.exports); + export_vars( + &mut self.global_context, + &resource.name, + &fallback, + &resource.protected, + ); + } + /// Process exports from an already-obtained result (e.g., from exports proxy). pub fn process_exports_from_result( &mut self, diff --git a/src/commands/build.rs b/src/commands/build.rs index b923b8b..f7cee04 100644 --- a/src/commands/build.rs +++ b/src/commands/build.rs @@ -18,7 +18,7 @@ use crate::commands::common_args::{ FailureAction, }; use crate::core::config::get_resource_type; -use crate::core::utils::{catch_error_and_exit, export_vars}; +use crate::core::utils::{catch_error_and_exit, export_vars, DRY_RUN_EXPORT_PLACEHOLDER}; use crate::utils::connection::create_client; use crate::utils::display::{print_unicode_box, BorderColor}; use crate::utils::server::{check_and_start_server, stop_local_server}; @@ -137,7 +137,7 @@ macro_rules! render_exports { } /// Main build workflow matching Python's StackQLProvisioner.run(). -fn run_build( +pub fn run_build( runner: &mut CommandRunner, dry_run: bool, show_queries: bool, @@ -1017,11 +1017,13 @@ fn run_build( if let Some(map) = item.as_mapping() { for (_, val) in map { if let Some(v) = val.as_str() { - placeholder_data.insert(v.to_string(), "".to_string()); + placeholder_data + .insert(v.to_string(), DRY_RUN_EXPORT_PLACEHOLDER.to_string()); } } } else if let Some(s) = item.as_str() { - placeholder_data.insert(s.to_string(), "".to_string()); + placeholder_data + .insert(s.to_string(), DRY_RUN_EXPORT_PLACEHOLDER.to_string()); } } info!( diff --git a/src/commands/teardown.rs b/src/commands/teardown.rs index 54e2c41..85a11da 100644 --- a/src/commands/teardown.rs +++ b/src/commands/teardown.rs @@ -16,7 +16,11 @@ use crate::commands::common_args::{ FailureAction, }; use crate::core::config::get_resource_type; -use crate::core::utils::{has_returning_clause, strip_returning_clause}; +use crate::core::utils::{ + has_returning_clause, references_unknown_export, strip_returning_clause, + UNKNOWN_EXPORT_PLACEHOLDER, +}; +use crate::resource::manifest::Resource; use crate::utils::connection::create_client; use crate::utils::display::{print_unicode_box, BorderColor}; use crate::utils::server::{check_and_start_server, stop_local_server}; @@ -72,12 +76,7 @@ pub fn execute(matches: &ArgMatches) { BorderColor::Yellow, ); - run_teardown( - &mut runner, - is_dry_run, - is_show_queries, - &format!("{:?}", on_failure_val), - ); + run_teardown(&mut runner, is_dry_run, is_show_queries, *on_failure_val); if is_dry_run { print_unicode_box("dry-run teardown complete", BorderColor::Green); @@ -88,7 +87,51 @@ pub fn execute(matches: &ArgMatches) { stop_local_server(); } +/// Render a query template for teardown. +/// +/// Returns `None` when the query cannot or must not be executed: +/// +/// - template variables are unresolved (an upstream export was never +/// populated), or +/// - the rendered SQL contains the [`UNKNOWN_EXPORT_PLACEHOLDER`] because an +/// upstream export could not be collected. +/// +/// In both cases `consequence` is logged so the operator can see why the +/// resource was skipped. +fn render_for_teardown( + runner: &CommandRunner, + resource: &Resource, + anchor: &str, + template: &str, + full_context: &std::collections::HashMap, + consequence: &str, +) -> Option { + match runner.try_render_query(&resource.name, anchor, template, full_context) { + Some(rendered) if references_unknown_export(&rendered) => { + info!( + "[{}] {} query references an export that could not be collected ({}), {}", + resource.name, anchor, UNKNOWN_EXPORT_PLACEHOLDER, consequence + ); + None + } + Some(rendered) => Some(rendered), + None => { + info!( + "[{}] {} query has unresolved variables, {}", + resource.name, anchor, consequence + ); + None + } + } +} + /// Collect exports for all resources before teardown. +/// +/// Exports are collected in manifest order so that a downstream resource's +/// `exists`/`delete` queries can reference upstream values. Every resource +/// ends this phase with each of its declared exports either populated or set +/// to [`UNKNOWN_EXPORT_PLACEHOLDER`]; queries that would interpolate the +/// placeholder are skipped by [`render_for_teardown`]. fn collect_exports(runner: &mut CommandRunner, show_queries: bool, dry_run: bool) { info!( "collecting exports for [{}] in [{}] environment", @@ -101,25 +144,63 @@ fn collect_exports(runner: &mut CommandRunner, show_queries: bool, dry_run: bool let res_type = get_resource_type(resource).to_string(); info!("getting exports for resource [{}]", resource.name); - let mut full_context = runner.get_full_context(resource); - + // Commands have no exports, and scripts are not executed during + // teardown; mark script exports unknown so downstream references + // are skipped rather than left unrenderable. if res_type == "command" { continue; } + if res_type == "script" { + runner.set_exports_unknown(resource); + continue; + } + + // A query resource opted out of teardown: do not run it, and mark + // its exports unknown so anything depending on them is skipped. + if res_type == "query" && resource.skip_on_delete { + info!( + "[{}] skip_on_delete is set, query not executed during teardown", + resource.name + ); + runner.set_exports_unknown(resource); + continue; + } + + let mut full_context = runner.get_full_context(resource); let (exports_query, exports_retries, exports_retry_delay) = if let Some(sql_val) = resource.sql.as_ref().filter(|_| res_type == "query") { - let iq = runner.render_inline_template(&resource.name, sql_val, &full_context); - (Some(iq), 1u32, 0u32) + match runner.try_render_inline_template(&resource.name, sql_val, &full_context) { + Some(rendered) if references_unknown_export(&rendered) => { + info!( + "[{}] inline query references an export that could not be collected ({}), skipping exports collection", + resource.name, UNKNOWN_EXPORT_PLACEHOLDER + ); + (None, 1u32, 0u32) + } + Some(rendered) => (Some(rendered), 1u32, 0u32), + None => { + info!( + "[{}] inline query has unresolved variables, skipping exports collection", + resource.name + ); + (None, 1u32, 0u32) + } + } } else { let queries = runner.get_queries(resource, &full_context); // Run exists query first to capture this.* fields needed by // exports (e.g. this.identifier). if let Some(eq) = queries.get("exists") { - if let Some(rendered) = - runner.try_render_query(&resource.name, "exists", &eq.template, &full_context) - { + if let Some(rendered) = render_for_teardown( + runner, + resource, + "exists", + &eq.template, + &full_context, + "assuming resource does not exist", + ) { let (_exists, fields) = runner.check_if_resource_exists( resource, &rendered, @@ -134,38 +215,30 @@ fn collect_exports(runner: &mut CommandRunner, show_queries: bool, dry_run: bool full_context.insert(format!("{}.{}", resource.name, k), v.clone()); } } - } else { - info!( - "[{}] exists query has unresolved variables, assuming resource does not exist", - resource.name - ); } } if let Some(eq) = queries.get("exports") { - match runner.try_render_query( - &resource.name, - "exports", - &eq.template, - &full_context, - ) { - // During teardown use minimal retries - the resource may - // already be partially deleted. - Some(rendered) => (Some(rendered), 1u32, 0u32), - None => { - info!( - "[{}] exports query has unresolved variables, skipping exports collection", - resource.name - ); - (None, 1u32, 0u32) - } - } + // During teardown use minimal retries - the resource may + // already be partially deleted. + ( + render_for_teardown( + runner, + resource, + "exports", + &eq.template, + &full_context, + "skipping exports collection", + ), + 1u32, + 0u32, + ) } else { (None, 1u32, 0u32) } }; - if let Some(ref eq_str) = exports_query { - runner.process_exports( + match exports_query { + Some(ref eq_str) => runner.process_exports( resource, &full_context, eq_str, @@ -174,13 +247,34 @@ fn collect_exports(runner: &mut CommandRunner, show_queries: bool, dry_run: bool dry_run, show_queries, true, // ignore_missing_exports - ); + ), + // The exports could not be collected (no query, unresolved, or + // upstream): mark them unknown so downstream queries + // referencing them are skipped consistently. + None => runner.set_exports_unknown(resource), } } } /// Main teardown workflow matching Python's StackQLDeProvisioner.run(). -fn run_teardown(runner: &mut CommandRunner, dry_run: bool, show_queries: bool, _on_failure: &str) { +/// +/// `on_failure` controls what happens when a `delete` statement fails at the +/// provider: +/// +/// - `Error` (default): the first failed delete aborts the run. +/// - `Ignore`: the failure is logged, the resource is reported as not +/// confirmed deleted, and the run continues with the next resource. Fatal +/// errors (network, auth, planner) still abort. +/// - `Rollback`: not meaningful for teardown; treated as `Error`. +/// +/// A delete that cannot be confirmed within its retry budget never aborts the +/// run on its own; such resources are listed in a summary at the end. +pub fn run_teardown( + runner: &mut CommandRunner, + dry_run: bool, + show_queries: bool, + on_failure: FailureAction, +) { let start_time = Instant::now(); info!( @@ -190,6 +284,24 @@ fn run_teardown(runner: &mut CommandRunner, dry_run: bool, show_queries: bool, _ if dry_run { "(dry run)" } else { "" } ); + let ignore_delete_errors = match on_failure { + FailureAction::Ignore => { + info!("on-failure=ignore: a failed delete is logged and the teardown continues with the next resource"); + true + } + FailureAction::Rollback => { + warn!( + "on-failure=rollback is not supported for teardown, treating as on-failure=error" + ); + false + } + FailureAction::Error => false, + }; + + // Resources whose delete was not confirmed (failed, or still present + // after the retry budget); reported once at the end of the run. + let mut unconfirmed: Vec = Vec::new(); + // Collect all exports first collect_exports(runner, show_queries, dry_run); @@ -215,6 +327,14 @@ fn run_teardown(runner: &mut CommandRunner, dry_run: bool, show_queries: bool, _ continue; } + if resource.skip_on_delete { + info!( + "[{}] skip_on_delete is set, resource retained (delete not executed)", + resource.name + ); + continue; + } + info!( "de-provisioning resource [{}], type: {}", resource.name, res_type @@ -245,43 +365,42 @@ fn run_teardown(runner: &mut CommandRunner, dry_run: bool, show_queries: bool, _ let resource_queries = runner.get_queries(resource, &full_context); // Get exists query (fallback to statecheck) - render JIT - let (exists_query_str, exists_retries, exists_retry_delay) = if let Some(eq) = - resource_queries.get("exists") - { - if let Some(rendered) = - runner.try_render_query(&resource.name, "exists", &eq.template, &full_context) - { - (rendered, eq.options.retries, eq.options.retry_delay) - } else { + let (exists_query_str, exists_retries, exists_retry_delay) = + if let Some(eq) = resource_queries.get("exists") { + match render_for_teardown( + runner, + resource, + "exists", + &eq.template, + &full_context, + "assuming resource does not exist, skipping...", + ) { + Some(rendered) => (rendered, eq.options.retries, eq.options.retry_delay), + None => continue, + } + } else if let Some(sq) = resource_queries.get("statecheck") { info!( - "[{}] exists query has unresolved variables, assuming resource does not exist, skipping...", + "exists query not defined for [{}], trying statecheck query as exists query.", resource.name ); - continue; - } - } else if let Some(sq) = resource_queries.get("statecheck") { - info!( - "exists query not defined for [{}], trying statecheck query as exists query.", - resource.name - ); - if let Some(rendered) = - runner.try_render_query(&resource.name, "statecheck", &sq.template, &full_context) - { - (rendered, sq.options.retries, sq.options.retry_delay) + match render_for_teardown( + runner, + resource, + "statecheck", + &sq.template, + &full_context, + "skipping...", + ) { + Some(rendered) => (rendered, sq.options.retries, sq.options.retry_delay), + None => continue, + } } else { info!( - "[{}] statecheck has unresolved variables, skipping...", + "No exists or statecheck query for [{}], skipping...", resource.name ); continue; - } - } else { - info!( - "No exists or statecheck query for [{}], skipping...", - resource.name - ); - continue; - }; + }; // Check if delete query template exists (don't render yet ā€” may need // this.* fields from the exists check). @@ -295,7 +414,7 @@ fn run_teardown(runner: &mut CommandRunner, dry_run: bool, show_queries: bool, _ } // Pre-delete check - let ignore_errors = res_type == "multi"; + let ignore_errors = res_type == "multi" || ignore_delete_errors; let resource_exists = if res_type == "multi" { info!("pre-delete check not supported for multi resources, skipping..."); true @@ -316,27 +435,34 @@ fn run_teardown(runner: &mut CommandRunner, dry_run: bool, show_queries: bool, _ full_context.insert(format!("{}.{}", resource.name, k), v.clone()); } } - exists + // A dry run executes no queries, so the exists check above only + // logged its SQL and reported "not found". Assume the resource + // exists so the delete statement is rendered and logged too. + if dry_run { + info!( + "dry run: assuming [{}] exists so the delete can be shown", + resource.name + ); + true + } else { + exists + } }; // Delete if resource_exists { // Render the delete query now (after exists fields are available). let dq = resource_queries.get("delete").unwrap(); - let rendered_delete = match runner.try_render_query( - &resource.name, + let rendered_delete = match render_for_teardown( + runner, + resource, "delete", &dq.template, &full_context, + "cannot delete, skipping...", ) { Some(rendered) => rendered, - None => { - info!( - "[{}] delete query has unresolved variables, assuming resource does not exist, skipping...", - resource.name - ); - continue; - } + None => continue, }; let delete_retries = dq.options.retries; let delete_retry_delay = dq.options.retry_delay; @@ -413,7 +539,10 @@ fn run_teardown(runner: &mut CommandRunner, dry_run: bool, show_queries: bool, _ ); } - // Run callback:delete block if present. + // Run callback:delete block if present. A callback polls the + // handle returned by RETURNING *, so there is nothing to poll + // when no row came back: a dry run, a delete without RETURNING, + // or a delete that failed and was ignored. let cb_anchor = if resource_queries.contains_key("callback:delete") { Some("callback:delete") } else if resource_queries.contains_key("callback") { @@ -422,26 +551,37 @@ fn run_teardown(runner: &mut CommandRunner, dry_run: bool, show_queries: bool, _ None }; if let Some(anchor) = cb_anchor { - if let Some(q) = resource_queries.get(anchor) { + if returning_row.is_none() { + info!( + "[{}] {} not run: the delete returned no RETURNING data{}", + resource.name, + anchor, + if dry_run { " (dry run)" } else { "" } + ); + } else if let Some(q) = resource_queries.get(anchor) { let cb_template = q.template.clone(); let cb_retries = q.options.retries; let cb_delay = q.options.retry_delay; let cb_sc_field = q.options.short_circuit_field.clone(); let cb_sc_value = q.options.short_circuit_value.clone(); let cb_ctx = runner.get_full_context(resource); - let rendered_cb = - runner.render_query(&resource.name, anchor, &cb_template, &cb_ctx); - runner.run_callback( - resource, - &rendered_cb, - cb_retries, - cb_delay, - cb_sc_field.as_deref(), - cb_sc_value.as_deref(), - "delete", - dry_run, - show_queries, - ); + match runner.try_render_query(&resource.name, anchor, &cb_template, &cb_ctx) { + Some(rendered_cb) => runner.run_callback( + resource, + &rendered_cb, + cb_retries, + cb_delay, + cb_sc_field.as_deref(), + cb_sc_value.as_deref(), + "delete", + dry_run, + show_queries, + ), + None => warn!( + "[{}] {} has unresolved variables and was not run", + resource.name, anchor + ), + } } } @@ -456,6 +596,7 @@ fn run_teardown(runner: &mut CommandRunner, dry_run: bool, show_queries: bool, _ show_queries, ); info!("[{}] delete could not be confirmed", resource.name); + unconfirmed.push(resource.name.clone()); } } else { info!( @@ -467,5 +608,18 @@ fn run_teardown(runner: &mut CommandRunner, dry_run: bool, show_queries: bool, _ } let elapsed = start_time.elapsed(); - info!("teardown completed in {:.2?}", elapsed); + if unconfirmed.is_empty() { + info!("teardown completed in {:.2?}", elapsed); + } else { + warn!( + "teardown completed in {:.2?} with {} resource(s) whose delete could not be confirmed: {}", + elapsed, + unconfirmed.len(), + unconfirmed + .iter() + .map(|n| format!("[{}]", n)) + .collect::>() + .join(", ") + ); + } } diff --git a/src/commands/test.rs b/src/commands/test.rs index aef7450..499a4cd 100644 --- a/src/commands/test.rs +++ b/src/commands/test.rs @@ -99,7 +99,7 @@ pub fn execute(matches: &ArgMatches) { } /// Main test workflow matching Python's StackQLTestRunner.run(). -fn run_test( +pub fn run_test( runner: &mut CommandRunner, dry_run: bool, show_queries: bool, @@ -125,18 +125,29 @@ fn run_test( let res_type = get_resource_type(resource).to_string(); + let mut full_context = runner.get_full_context(resource); + + // Evaluate condition (same semantics as build and teardown) + if !runner.evaluate_condition(resource, &full_context) { + continue; + } + if res_type == "query" { info!("exporting variables for [{}]", resource.name); } else if res_type == "resource" || res_type == "multi" { info!("testing resource [{}], type: {}", resource.name, res_type); } else if res_type == "command" { + // Commands are mutations with no state to test. + continue; + } else if res_type == "script" { + // Scripts are the only source of their exports, which later + // resources may reference; run them exactly as build does. + runner.process_script_resource(resource, dry_run, &full_context); continue; } else { catch_error_and_exit(&format!("unknown resource type: {}", res_type)); } - let mut full_context = runner.get_full_context(resource); - // Get test queries (templates only, not yet rendered) let (test_queries, inline_query) = if let Some(sql_val) = resource.sql.as_ref().filter(|_| res_type == "query") { diff --git a/src/core/config.rs b/src/core/config.rs index 6e05d37..5fe3d6b 100644 --- a/src/core/config.rs +++ b/src/core/config.rs @@ -465,6 +465,7 @@ mod tests { description: String::new(), r#if: None, skip_validation: None, + skip_on_delete: false, auth: None, return_vals: None, } @@ -629,8 +630,8 @@ mod tests { let ctx = get_full_context(&engine, &global_context, &resource, "dev", None); - assert!(ctx.get("idempotency_token").is_none()); - assert!(ctx.get("my_resource.idempotency_token").is_none()); + assert!(!ctx.contains_key("idempotency_token")); + assert!(!ctx.contains_key("my_resource.idempotency_token")); } #[test] diff --git a/src/core/templating.rs b/src/core/templating.rs index 4a397db..75f6dfa 100644 --- a/src/core/templating.rs +++ b/src/core/templating.rs @@ -585,11 +585,93 @@ pub fn render_inline_template( } } +/// Tolerant variant of [`render_inline_template`]: returns `None` when the +/// inline SQL references variables that are not in the context, instead of +/// exiting the process. +/// +/// Used by `teardown`, where an upstream export may legitimately be missing +/// (the resource that produced it is already gone, or was skipped via +/// `skip_on_delete`) and the correct behaviour is to skip the query. +pub fn try_render_inline_template( + engine: &TemplateEngine, + resource_name: &str, + template_string: &str, + full_context: &HashMap, +) -> Option { + let mut temp_context = prepare_query_context(full_context); + + let expanded = preprocess_this_prefix(template_string, resource_name).ok()?; + let compat = preprocess_jinja2_compat(&expanded); + let processed = preprocess_inline_dicts(&compat, &mut temp_context); + let template_name = format!("{}__inline", resource_name); + + match engine.render_with_filters(&template_name, &processed, &temp_context) { + Ok(rendered) => { + let unresolved_re = Regex::new(r"\{\{[^}]+\}\}").unwrap(); + if unresolved_re.is_match(&rendered) { + debug!( + "Unresolved variables in [{}] inline template, deferring render", + resource_name + ); + return None; + } + debug!( + "[{}] rendered inline template:\n\n{}\n", + resource_name, rendered + ); + Some(rendered) + } + Err(e) => { + debug!( + "[{}] inline template could not be rendered: {}", + resource_name, e + ); + None + } + } +} + #[cfg(test)] mod tests { use super::*; use crate::template::engine::TemplateEngine; + // ā”€ā”€ try_render_inline_template unit tests ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ + + #[test] + fn test_try_render_inline_template_renders_when_context_complete() { + let engine = TemplateEngine::new(); + let mut ctx = HashMap::new(); + ctx.insert("deployment_name".to_string(), "dbc-1234".to_string()); + let rendered = try_render_inline_template( + &engine, + "workspace_ready", + "SELECT userName FROM databricks_workspace.iam.current_user \ + WHERE deployment_name = '{{ deployment_name }}'", + &ctx, + ); + assert_eq!( + rendered.as_deref(), + Some( + "SELECT userName FROM databricks_workspace.iam.current_user \ + WHERE deployment_name = 'dbc-1234'" + ) + ); + } + + #[test] + fn test_try_render_inline_template_returns_none_when_variable_missing() { + let engine = TemplateEngine::new(); + let ctx: HashMap = HashMap::new(); + let rendered = try_render_inline_template( + &engine, + "workspace_ready", + "SELECT 1 WHERE deployment_name = '{{ deployment_name }}'", + &ctx, + ); + assert!(rendered.is_none()); + } + // ā”€ā”€ preprocess_this_prefix unit tests ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ #[test] diff --git a/src/core/utils.rs b/src/core/utils.rs index 2eff7d6..2244c83 100644 --- a/src/core/utils.rs +++ b/src/core/utils.rs @@ -234,6 +234,13 @@ pub fn run_stackql_command( } => { // Check for errors in notices for notice in ¬ices { + if error_detected_in_notice(notice) && ignore_errors { + warn!( + "Command returned an error notice (ignored):\n\n{}\n", + notice + ); + continue; + } if error_detected_in_notice(notice) && !ignore_errors { if attempt < retries { debug!( @@ -315,7 +322,7 @@ pub fn run_stackql_command( e )); } else { - debug!("Command failed (ignored): {}", e); + warn!("Command failed (ignored):\n\n{}\n", e); return String::new(); } } @@ -587,6 +594,58 @@ fn is_version_higher(installed: &str, requested: &str) -> bool { parse(installed) > parse(requested) } +/// Placeholder assigned to an export during `teardown` when its value could +/// not be collected: the exports query returned no rows (the upstream +/// resource may already be gone), the query could not be rendered, or the +/// resource was skipped via `skip_on_delete`. +/// +/// Downstream queries that would interpolate this placeholder must not be +/// executed - see [`references_unknown_export`]. +pub const UNKNOWN_EXPORT_PLACEHOLDER: &str = ""; + +/// Placeholder assigned to an export during `--dry-run`, where no query is +/// executed and the real value is not known. +pub const DRY_RUN_EXPORT_PLACEHOLDER: &str = ""; + +/// True for the framework's own export placeholders. These are never real +/// values and must not be registered for log redaction: doing so would mask +/// every other placeholder in the run as if it were a secret. +pub fn is_export_placeholder(value: &str) -> bool { + value == UNKNOWN_EXPORT_PLACEHOLDER || value == DRY_RUN_EXPORT_PLACEHOLDER +} + +/// Returns true when a rendered query contains the teardown +/// [`UNKNOWN_EXPORT_PLACEHOLDER`], i.e. it references an export whose value +/// could not be collected. +/// +/// Such a query is never useful to run: at best it matches nothing, at worst +/// the placeholder lands in a hostname or identifier and the provider call +/// fails with a fatal, non-retryable error (for example +/// `dial tcp: lookup .cloud.databricks.com: no such host`), which +/// aborts the whole teardown. +pub fn references_unknown_export(rendered_query: &str) -> bool { + rendered_query.contains(UNKNOWN_EXPORT_PLACEHOLDER) +} + +/// Build the fallback export map for a resource during teardown: every +/// declared export (plain `name` or `{ column: name }` mapping) is set to +/// [`UNKNOWN_EXPORT_PLACEHOLDER`]. +pub fn unknown_exports_for(expected_exports: &[serde_yaml::Value]) -> HashMap { + let mut fallback = HashMap::new(); + for item in expected_exports { + if let Some(s) = item.as_str() { + fallback.insert(s.to_string(), UNKNOWN_EXPORT_PLACEHOLDER.to_string()); + } else if let Some(map) = item.as_mapping() { + for (_, val) in map { + if let Some(v) = val.as_str() { + fallback.insert(v.to_string(), UNKNOWN_EXPORT_PLACEHOLDER.to_string()); + } + } + } + } + fallback +} + /// Update global context with exported values. /// /// Each export is stored under two keys: @@ -608,7 +667,7 @@ pub fn export_vars( ) { for (key, value) in export_data { let is_protected = protected_exports.contains(key); - if is_protected { + if is_protected && !is_export_placeholder(value) { // Register for global log redaction so the value is also masked // anywhere else it surfaces (e.g. interpolated into a downstream // resource's query shown via --dry-run or --show-queries). @@ -1313,4 +1372,33 @@ mod tests { "SUCCESS" )); } + + #[test] + fn test_references_unknown_export_detects_placeholder() { + let rendered = "SELECT userName FROM databricks_workspace.iam.current_user WHERE deployment_name = ''"; + assert!(references_unknown_export(rendered)); + assert!(!references_unknown_export( + "SELECT userName FROM databricks_workspace.iam.current_user WHERE deployment_name = 'dbc-1234'" + )); + } + + #[test] + fn test_unknown_exports_for_handles_plain_and_mapped_exports() { + let expected = vec![ + serde_yaml::Value::String("workspace_id".to_string()), + serde_yaml::from_str::("arn: role_arn").unwrap(), + ]; + let fallback = unknown_exports_for(&expected); + assert_eq!(fallback.len(), 2); + assert_eq!( + fallback.get("workspace_id").map(String::as_str), + Some(UNKNOWN_EXPORT_PLACEHOLDER) + ); + // Mapped exports use the target (value) name, not the source column. + assert_eq!( + fallback.get("role_arn").map(String::as_str), + Some(UNKNOWN_EXPORT_PLACEHOLDER) + ); + assert!(!fallback.contains_key("arn")); + } } diff --git a/src/lib.rs b/src/lib.rs new file mode 100644 index 0000000..a8f1d0a --- /dev/null +++ b/src/lib.rs @@ -0,0 +1,21 @@ +// lib.rs + +//! # StackQL Deploy - Library Crate +//! +//! The `stackql-deploy` binary (`src/main.rs`) is a thin CLI wrapper over the +//! modules exposed here. Exposing them as a library lets the integration +//! tests under `tests/` construct a `CommandRunner` against an in-process +//! mock StackQL server and drive the `build` / `teardown` flows directly, +//! without a real stackql binary, provider registry, or cloud credentials. +//! +//! This crate is not intended as a public API: module layout and function +//! signatures may change between releases without notice. + +pub mod app; +pub mod commands; +pub mod core; +pub mod error; +pub mod globals; +pub mod resource; +pub mod template; +pub mod utils; diff --git a/src/main.rs b/src/main.rs index c3d2cf0..c0dfad2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -22,27 +22,19 @@ //! //! For detailed help, use `--help` or `-h` flags. -mod app; -mod commands; -pub mod core; -mod error; -mod globals; -mod resource; -mod template; -mod utils; - use std::process; use clap::{Arg, ArgAction, Command}; -use error::{get_binary_path_with_error, AppError}; use log::{debug, error, info}; -use crate::app::{ +use stackql_deploy::app::{ APP_AUTHOR, APP_DESCRIPTION, APP_NAME, APP_VERSION, DEFAULT_LOG_LEVEL, DEFAULT_SERVER_HOST, DEFAULT_SERVER_PORT, DEFAULT_SERVER_PORT_STR, EXEMPT_COMMANDS, LOG_LEVELS, }; -use crate::utils::logging::initialize_logger; +use stackql_deploy::error::{get_binary_path_with_error, AppError}; +use stackql_deploy::utils::logging::initialize_logger; +use stackql_deploy::{commands, globals, print_error}; /// Main function that initializes the CLI and handles command execution. fn main() { diff --git a/src/resource/manifest.rs b/src/resource/manifest.rs index 5228c9b..261bae5 100644 --- a/src/resource/manifest.rs +++ b/src/resource/manifest.rs @@ -142,6 +142,19 @@ pub struct Resource { #[serde(default)] pub skip_validation: Option, + /// When true, the resource is not processed during `teardown`. + /// + /// - `query` resources: the query is not executed and each declared + /// export is set to the `` placeholder, so any downstream + /// query that references those exports is skipped rather than run. + /// - `resource` / `multi` resources: exports are still collected (a + /// downstream `delete` may need them) but the resource's own `delete` + /// query is not executed, so the resource is retained. + /// + /// Has no effect on `build` or `test`. + #[serde(default)] + pub skip_on_delete: bool, + /// Auth configuration for the resource #[serde(default)] pub auth: Option, @@ -771,6 +784,37 @@ resources: ); } + #[test] + fn test_skip_on_delete_defaults_false_and_parses_true() { + let dir = setup_test_dir(); + let manifest_content = r#" +version: 1 +name: test-stack +providers: + - databricks_workspace +resources: + - name: workspace + props: [] + - name: workspace_ready + type: query + skip_on_delete: true + props: [] + sql: SELECT 1 AS workspace_principal + exports: + - workspace_principal +"#; + fs::write(dir.path().join("stackql_manifest.yml"), manifest_content).unwrap(); + + let manifest = Manifest::load_from_stack_dir(dir.path()).unwrap(); + assert!(!manifest.find_resource("workspace").unwrap().skip_on_delete); + assert!( + manifest + .find_resource("workspace_ready") + .unwrap() + .skip_on_delete + ); + } + #[test] fn test_nested_file_directives() { let dir = setup_test_dir(); diff --git a/src/resource/validation.rs b/src/resource/validation.rs index bc8b3d1..e0b3a99 100644 --- a/src/resource/validation.rs +++ b/src/resource/validation.rs @@ -105,6 +105,7 @@ mod tests { description: String::new(), r#if: None, skip_validation: None, + skip_on_delete: false, auth: None, return_vals: None, }) diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 0000000..9d5a607 --- /dev/null +++ b/tests/README.md @@ -0,0 +1,96 @@ +# Tests + +Two layers, both driven by `cargo test`: + +| Layer | Files | Needs | Runs on | +|-------|-------|-------|---------| +| Mock | `build.rs`, `teardown.rs`, `test_command.rs`, `fixtures/` | nothing | every `cargo test`, `ci-scripts/test.sh`, the `CI` workflow | +| Live | `live.rs`, `live_stacks/` | stackql binary, AWS and GitHub credentials | `ci-scripts/integration-test.sh`, the `Integration Tests` workflow on pull requests | + +Unit tests live next to the code in `src/` (`#[cfg(test)]` modules). + +## Mock layer + +The crate exposes a library target so tests can build a `CommandRunner` in +process and call `run_build`, `run_test`, and `run_teardown` directly. +`common/mock_server.rs` is a small in-process server speaking the PostgreSQL +simple-query protocol, exactly what the stackql server speaks, and it records +every statement it receives. `common/fake_cloud.rs` answers those statements +from a table of present or absent provider resources, so a test can assert on +the precise SQL that was (or was not) sent and on the exports left behind. + +Fixtures under `fixtures//` are ordinary stacks. A fixture can carry +several `manifest_*.yml` variants sharing one `resources/` directory; +`TestStack::with_manifest` installs the chosen one as `stackql_manifest.yml` +in a temporary copy. + +```sh +cargo test # unit + mock tests, a few seconds, no network +``` + +## Live layer + +`live.rs` runs the real binary (`CARGO_BIN_EXE_stackql-deploy`) against the +stacks in `live_stacks/`. The binary only reads the stack directory; it runs +with `target/live/` as its working directory, which is where the stackql +server keeps its provider cache (`.stackql/`) and where +`.stackql-deploy-exports`, `stackql.log` and `--output-file` documents are +written. That directory is gitignored and persists between runs, so provider +documents are pulled once. Each stack gets its own server port (5451 to +5453), so a developer's default server on 5444 is never touched. Resource +names carry a per-run suffix (`STACKQL_DEPLOY_LIVE_RUN_ID`, or a local +timestamp), so concurrent runs do not collide, and every test tears its stack +down, including on failure. + +All resources are free: AWS Systems Manager Parameter Store standard +parameters and GitHub repository labels. + +| Stack | Provider | What it exercises | +|-------|----------|-------------------| +| `aws_ssm` | `awscc`, `aws` | The full lifecycle: dry run, create, update and delete with `RETURNING *`, `return_vals`, `callback:create` / `callback:update` / `callback:delete` polling, `troubleshoot`, `statecheck`, `PatchDocument` update via `generate_patch_document`, `createorupdate` (`REPLACE`), `query` (file and inline), `command`, `script`, `if` conditions, `file()` directives, `merge`, per-environment `values`, `protected` masking with `--show-queries`, stack `exports` with `--output-file`, `test` pass and fail, teardown with `skip_on_delete`, idempotent re-build and re-teardown | +| `aws_ssm_onfailure` | `aws`, `awscc` | Two deletes that can never succeed. S3 `DeleteBucket` on a bucket that never existed is rejected synchronously: `--on-failure error` aborts, `--on-failure ignore` logs and continues. Cloud Control `DeleteResource` on a parameter that never existed fails asynchronously: the `troubleshoot:delete` anchor surfaces the ProgressEvent error through the `RequestToken` captured by `return_vals.delete` | +| `github_labels` | `github` | Plain REST provider without `RETURNING`: 404-as-not-found exists check, create, idempotent re-build, update, `test`, teardown | + +### Running locally + +```sh +# credentials in the environment (never in the repo) +export AWS_ACCESS_KEY_ID=... AWS_SECRET_ACCESS_KEY=... AWS_REGION=us-east-1 +export STACKQL_GITHUB_USERNAME= STACKQL_GITHUB_PASSWORD= +export GITHUB_OWNER= GITHUB_REPO= # where the test label is created + +bash ci-scripts/integration-test.sh # everything +STACKQL_DEPLOY_LIVE_FILTER=aws_ssm bash ci-scripts/integration-test.sh # one stack +cargo test --test live live_teardown_on_failure_modes -- --ignored --nocapture +``` + +The `stackql` binary must be on `PATH`. Script resources run through +`sh -c`, so on Windows run from Git Bash (or another shell with `sh` on +`PATH`). Expect around four minutes for `aws_ssm`; the statecheck retry +budget before an update is most of it. + +### In CI + +`.github/workflows/integration-tests.yml` runs the live layer on every pull +request to `main` that touches `src/`, `tests/`, `ci-scripts/` or the Cargo +files, and on `workflow_dispatch`. It needs two repository secrets, +`AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY`, for an IAM principal that +may manage SSM parameters under `/stackql-deploy/` and call `s3:DeleteBucket` +(the call is expected to fail; no bucket is ever created). The GitHub stack +uses the workflow's own `GITHUB_TOKEN` with `issues: write`. Pull requests +from forks cannot see secrets, so the job skips itself for them. + +To make it a merge gate, add the `Live provider tests` job to the required +status checks of the `main` branch protection rule. + +### Adding a live stack + +1. Create `live_stacks//` with a `stackql_manifest.yml` and + `resources/`. Use only free resources, include the run id in every + resource name, and make sure `teardown` removes everything the stack + creates. Take manifest inputs with `-e` (globals are rendered from `-e` + flags and a `.env` file, not from the process environment). +2. Add a `#[test] #[ignore]` function in `live.rs` using `LiveStack`, + `TeardownGuard`, and a new port. +3. Document the credentials it needs here and in + `ci-scripts/integration-test.sh`. diff --git a/tests/build.rs b/tests/build.rs new file mode 100644 index 0000000..358c5f9 --- /dev/null +++ b/tests/build.rs @@ -0,0 +1,127 @@ +//! Integration tests for the `build` flow. +//! +//! These drive `run_build` against the same fixture stack and mock server as +//! the teardown tests. They double as a smoke test of the harness (a full +//! exists -> create -> post-create exists -> statecheck -> exports cycle) and +//! pin down that `skip_on_delete` is a teardown-only setting. + +mod common; + +use common::{export_value, FakeCloud, MockServer, TestStack}; +use stackql_deploy::commands::build::run_build; + +const PROVIDERS: &[&str] = &[ + "databricks_account::v26.07.00430", + "databricks_workspace::v26.07.00430", +]; +const WORKSPACES: &str = "databricks_account.provisioning.workspaces"; +const CURRENT_USER: &str = "databricks_workspace.iam.current_user"; +const STORAGE_CREDENTIALS: &str = "databricks_workspace.catalog.storage_credentials"; + +/// Nothing deployed yet; each table becomes present with these values once +/// its `INSERT` arrives. +fn empty_cloud() -> FakeCloud { + FakeCloud::new(PROVIDERS) + .table_absent( + WORKSPACES, + &[ + ("workspace_id", "7474649669073866"), + ("deployment_name", "dbc-d586e37e-0269"), + ("workspace_status", "RUNNING"), + ], + ) + .table_present(CURRENT_USER, &[("userName", "cicd-sp@example.com")]) + .table_absent(STORAGE_CREDENTIALS, &[("id", "07abf775-f0de-4749")]) +} + +fn mutations(server: &MockServer) -> Vec { + server + .queries() + .into_iter() + .filter(|q| { + let u = q.trim_start().to_ascii_uppercase(); + u.starts_with("INSERT") || u.starts_with("UPDATE") || u.starts_with("DELETE") + }) + .collect() +} + +#[test] +fn build_creates_missing_resources_and_chains_exports() { + let server = MockServer::start(empty_cloud().into_handler()); + let stack = TestStack::new("databricks_workspace"); + let mut runner = stack.runner(&server, "dev"); + + run_build(&mut runner, false, false, "Error", None); + + let inserts = mutations(&server); + assert_eq!(inserts.len(), 2, "expected two creates, got: {:?}", inserts); + assert!(inserts[0].contains(WORKSPACES)); + // The second create was rendered with the export of the first. + assert!(inserts[1].contains(STORAGE_CREDENTIALS)); + assert!(inserts[1].contains("'dbc-d586e37e-0269'")); + + assert_eq!( + export_value(&runner, "deployment_name").as_deref(), + Some("dbc-d586e37e-0269") + ); + assert_eq!( + export_value(&runner, "workspace_principal").as_deref(), + Some("cicd-sp@example.com") + ); + assert_eq!( + export_value(&runner, "storage_credential_id").as_deref(), + Some("07abf775-f0de-4749") + ); + assert_eq!( + export_value(&runner, "workspace_status").as_deref(), + Some("RUNNING") + ); +} + +#[test] +fn build_still_executes_queries_flagged_skip_on_delete() { + // skip_on_delete only affects teardown: on build the readiness query + // runs and its export is populated as usual. + let server = MockServer::start(empty_cloud().into_handler()); + let stack = TestStack::with_manifest("databricks_workspace", "manifest_skip_query.yml"); + let mut runner = stack.runner(&server, "dev"); + + run_build(&mut runner, false, false, "Error", None); + + assert!( + server.received(CURRENT_USER), + "skip_on_delete must not suppress the query during build" + ); + assert_eq!( + export_value(&runner, "workspace_principal").as_deref(), + Some("cicd-sp@example.com") + ); +} + +#[test] +fn build_still_creates_resources_flagged_skip_on_delete() { + let server = MockServer::start(empty_cloud().into_handler()); + let stack = TestStack::with_manifest("databricks_workspace", "manifest_skip_resource.yml"); + let mut runner = stack.runner(&server, "dev"); + + run_build(&mut runner, false, false, "Error", None); + + let inserts = mutations(&server); + assert!( + inserts.iter().any(|q| q.contains(WORKSPACES)), + "skip_on_delete must not suppress create during build, got: {:?}", + inserts + ); +} + +#[test] +fn dry_run_build_sends_no_mutations() { + let server = MockServer::start(empty_cloud().into_handler()); + let stack = TestStack::new("databricks_workspace"); + let mut runner = stack.runner(&server, "dev"); + + run_build(&mut runner, true, false, "Error", None); + + let sent = mutations(&server); + assert!(sent.is_empty(), "dry run sent mutations: {:?}", sent); +} diff --git a/tests/common/cli.rs b/tests/common/cli.rs new file mode 100644 index 0000000..addd38c --- /dev/null +++ b/tests/common/cli.rs @@ -0,0 +1,267 @@ +//! Drive the real `stackql-deploy` binary against a live stack. +//! +//! Used by the `#[ignore]`d live tests in `tests/live.rs`. The binary is +//! pointed at a stack directory under `tests/live_stacks/` (which it only +//! reads) and runs with `target/live/` as its working directory, so the +//! stackql server's provider cache (`.stackql/`), `.stackql-deploy-exports` +//! and `stackql.log` land in a gitignored place that persists between runs. +//! Sharing one working directory means the provider documents are pulled +//! once, not once per stack. + +use std::cell::RefCell; +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::Command; + +/// A live stack and the working directory the binary runs in. +pub struct LiveStack { + /// The stack directory in the source tree (read only). + pub dir: PathBuf, + /// Working directory for the binary and the stackql server. + pub work_dir: PathBuf, + pub name: String, + port: u16, + /// `-e KEY=VALUE` inputs; interior mutability lets a test change an input + /// between runs while a [`TeardownGuard`] holds a shared borrow. + inputs: RefCell>, +} + +/// Result of one `stackql-deploy` invocation. +pub struct Outcome { + pub phase: String, + pub status: Option, + pub stdout: String, + pub stderr: String, +} + +impl LiveStack { + /// Bind to `tests/live_stacks/`. `port` is the stackql server port + /// for this stack; give every stack its own so runs never collide with + /// each other or with a developer's default server. + pub fn new(name: &str, port: u16) -> Self { + let manifest_dir = Path::new(env!("CARGO_MANIFEST_DIR")); + let dir = manifest_dir.join("tests").join("live_stacks").join(name); + assert!(dir.is_dir(), "live stack not found: {}", dir.display()); + let work_dir = manifest_dir.join("target").join("live"); + fs::create_dir_all(&work_dir).expect("create target/live"); + // Leftovers from an earlier run must not satisfy this run's asserts. + let _ = fs::remove_file(work_dir.join(".stackql-deploy-exports")); + LiveStack { + dir, + work_dir, + name: name.to_string(), + port, + inputs: RefCell::new(Vec::new()), + } + } + + /// Set (or replace) a manifest input passed as `-e KEY=VALUE`. + pub fn input(&self, key: &str, value: &str) -> &Self { + let mut inputs = self.inputs.borrow_mut(); + inputs.retain(|(k, _)| k != key); + inputs.push((key.to_string(), value.to_string())); + self + } + + /// Run `stackql-deploy [extra...]`. + pub fn run(&self, command: &str, stack_env: &str, extra: &[&str]) -> Outcome { + let phase = format!("{} {} {}", command, stack_env, extra.join(" ")) + .trim() + .to_string(); + let mut cmd = Command::new(env!("CARGO_BIN_EXE_stackql-deploy")); + cmd.current_dir(&self.work_dir) + .arg("--port") + .arg(self.port.to_string()) + .arg(command) + .arg(&self.dir) + .arg(stack_env); + for (k, v) in self.inputs.borrow().iter() { + cmd.arg("-e").arg(format!("{}={}", k, v)); + } + cmd.args(extra); + + eprintln!( + " +[{}] $ stackql-deploy {}", + self.name, phase + ); + let output = cmd.output().expect("spawn stackql-deploy"); + let outcome = Outcome { + phase, + status: output.status.code(), + stdout: String::from_utf8_lossy(&output.stdout).into_owned(), + stderr: String::from_utf8_lossy(&output.stderr).into_owned(), + }; + outcome.print(); + outcome + } + + /// Path of the sourceable exports file the binary writes in its cwd. + pub fn exports_file(&self) -> PathBuf { + self.work_dir.join(".stackql-deploy-exports") + } + + /// A scratch path in the working directory (for `--output-file`), with + /// any previous run's file removed. + pub fn scratch(&self, file_name: &str) -> PathBuf { + let path = self.work_dir.join(format!("{}-{}", self.name, file_name)); + let _ = fs::remove_file(&path); + path + } +} + +impl Outcome { + /// stdout followed by stderr (the logger writes to stderr). + pub fn log(&self) -> String { + format!("{}\n{}", self.stdout, self.stderr) + } + + pub fn contains(&self, needle: &str) -> bool { + self.log().contains(needle) + } + + pub fn count(&self, needle: &str) -> usize { + self.log().matches(needle).count() + } + + pub fn assert_success(&self) -> &Self { + assert_eq!( + self.status, + Some(0), + "expected `{}` to succeed, exit status {:?}", + self.phase, + self.status + ); + self + } + + pub fn assert_failure(&self) -> &Self { + assert_ne!( + self.status, + Some(0), + "expected `{}` to fail, but it exited 0", + self.phase + ); + self + } + + pub fn assert_contains(&self, needle: &str) -> &Self { + assert!( + self.contains(needle), + "expected `{}` output to contain {:?}", + self.phase, + needle + ); + self + } + + pub fn assert_not_contains(&self, needle: &str) -> &Self { + assert!( + !self.contains(needle), + "expected `{}` output NOT to contain {:?}", + self.phase, + needle + ); + self + } + + /// Echo the run's output, folded into a group on GitHub Actions. + fn print(&self) { + let on_actions = std::env::var_os("GITHUB_ACTIONS").is_some(); + if on_actions { + eprintln!("::group::{} (exit {:?})", self.phase, self.status); + } + eprintln!("{}", self.log()); + if on_actions { + eprintln!("::endgroup::"); + } + eprintln!("[{}] exit status: {:?}", self.phase, self.status); + } +} + +/// Runs `teardown --on-failure ignore` when dropped unless disarmed, so a +/// failing test still removes what it created. +pub struct TeardownGuard<'a> { + stack: &'a LiveStack, + stack_env: String, + armed: bool, +} + +impl<'a> TeardownGuard<'a> { + pub fn new(stack: &'a LiveStack, stack_env: &str) -> Self { + TeardownGuard { + stack, + stack_env: stack_env.to_string(), + armed: true, + } + } + + /// Call once the test has torn the stack down itself. + pub fn disarm(&mut self) { + self.armed = false; + } +} + +impl Drop for TeardownGuard<'_> { + fn drop(&mut self) { + if self.armed { + eprintln!( + "[{}] test did not reach its own teardown, cleaning up", + self.stack.name + ); + let _ = self + .stack + .run("teardown", &self.stack_env, &["--on-failure", "ignore"]); + } + } +} + +/// Read a `--output-file` JSON document into a map of string values. +pub fn read_exports_json(path: &Path) -> serde_json::Map { + let text = fs::read_to_string(path) + .unwrap_or_else(|e| panic!("read exports file {}: {}", path.display(), e)); + serde_json::from_str::(&text) + .expect("exports file is valid JSON") + .as_object() + .expect("exports file is a JSON object") + .clone() +} + +/// Value of `key` in an exports JSON map as a string. +pub fn export_str(map: &serde_json::Map, key: &str) -> String { + match map.get(key) { + Some(serde_json::Value::String(s)) => s.clone(), + Some(other) => other.to_string(), + None => panic!( + "export {:?} missing from {:?}", + key, + map.keys().collect::>() + ), + } +} + +/// Environment variable or a default. +pub fn env_or(key: &str, default: &str) -> String { + std::env::var(key).unwrap_or_else(|_| default.to_string()) +} + +/// Fail fast with a clear message when a credential is missing. +pub fn require_env(key: &str, purpose: &str) { + if std::env::var_os(key).is_none() { + panic!( + "environment variable {} is required for {}; see tests/README.md", + key, purpose + ); + } +} + +/// Unique suffix for resource names: the CI run id, or a local timestamp. +pub fn run_id() -> String { + std::env::var("STACKQL_DEPLOY_LIVE_RUN_ID").unwrap_or_else(|_| { + let secs = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + format!("local-{}", secs) + }) +} diff --git a/tests/common/fake_cloud.rs b/tests/common/fake_cloud.rs new file mode 100644 index 0000000..b3f8eee --- /dev/null +++ b/tests/common/fake_cloud.rs @@ -0,0 +1,271 @@ +//! A tiny stateful stand-in for cloud providers behind the mock server. +//! +//! Each provider "table" (e.g. `databricks_account.provisioning.workspaces`) +//! holds at most one row and a present/absent flag. `SELECT` answers from +//! that row, `INSERT` marks the table present, `DELETE` marks it absent. This +//! is deliberately simple: `WHERE` clauses are ignored and the select list is +//! parsed only far enough to honour column names, `AS` aliases, `COUNT(*)` +//! and quoted / numeric literals. + +use std::collections::HashMap; + +use regex::Regex; + +use super::mock_server::MockResponse; + +#[derive(Debug, Clone, Default)] +struct TableState { + present: bool, + row: HashMap, +} + +/// Builder for a fake provider backend. Consume it with +/// [`FakeCloud::into_handler`] and pass the result to `MockServer::start`. +#[derive(Debug, Clone, Default)] +pub struct FakeCloud { + providers: Vec<(String, String)>, + tables: HashMap, + /// `(needle, message)`: any statement containing `needle` fails with + /// `message` instead of being evaluated. + errors: Vec<(String, String)>, +} + +impl FakeCloud { + /// `providers` uses the manifest form, e.g. `aws::v26.08.00444` or `aws`. + /// Every listed provider is reported as installed by `SHOW PROVIDERS`. + pub fn new(providers: &[&str]) -> Self { + let providers = providers + .iter() + .map(|p| match p.split_once("::") { + Some((name, version)) => (name.to_string(), version.to_string()), + None => (p.to_string(), "v1.0.0".to_string()), + }) + .collect(); + FakeCloud { + providers, + ..Default::default() + } + } + + /// The table exists and returns `row` for selects. + pub fn table_present(mut self, table: &str, row: &[(&str, &str)]) -> Self { + self.tables.insert( + table.to_string(), + TableState { + present: true, + row: to_row(row), + }, + ); + self + } + + /// The table does not exist. Selects return no rows; an `INSERT` makes it + /// present with `row_after_create`. + pub fn table_absent(mut self, table: &str, row_after_create: &[(&str, &str)]) -> Self { + self.tables.insert( + table.to_string(), + TableState { + present: false, + row: to_row(row_after_create), + }, + ); + self + } + + /// Any statement containing `needle` fails with `message`. + pub fn error_on(mut self, needle: &str, message: &str) -> Self { + self.errors.push((needle.to_string(), message.to_string())); + self + } + + pub fn into_handler(mut self) -> impl FnMut(&str) -> MockResponse + Send + 'static { + move |sql: &str| self.handle(sql) + } + + fn handle(&mut self, sql: &str) -> MockResponse { + for (needle, message) in &self.errors { + if sql.contains(needle.as_str()) { + return MockResponse::Error(message.clone()); + } + } + + let trimmed = sql.trim().trim_end_matches(';').trim(); + let upper = trimmed.to_ascii_uppercase(); + + if upper == "SHOW PROVIDERS" { + return MockResponse::Rows { + columns: vec!["name".to_string(), "version".to_string()], + rows: self + .providers + .iter() + .map(|(n, v)| vec![Some(n.clone()), Some(v.clone())]) + .collect(), + }; + } + if upper.starts_with("REGISTRY PULL") { + return MockResponse::Command("REGISTRY".to_string()); + } + if upper.starts_with("SELECT") { + return self.handle_select(trimmed); + } + if let Some(table) = capture(r"(?is)^\s*INSERT\s+INTO\s+([\w.]+)", trimmed) { + self.tables.entry(table).or_default().present = true; + return MockResponse::Command("INSERT 0 1".to_string()); + } + if let Some(table) = capture(r"(?is)^\s*DELETE\s+FROM\s+([\w.]+)", trimmed) { + self.tables.entry(table).or_default().present = false; + return MockResponse::Command("DELETE 1".to_string()); + } + if capture(r"(?is)^\s*UPDATE\s+([\w.]+)", trimmed).is_some() { + return MockResponse::Command("UPDATE 1".to_string()); + } + MockResponse::empty() + } + + fn handle_select(&self, sql: &str) -> MockResponse { + let table = capture(r"(?is)\bFROM\s+([\w.]+)", sql); + let select_list = select_list(sql); + let items: Vec<(String, String)> = split_top_level_commas(&select_list) + .into_iter() + .map(|item| parse_select_item(&item)) + .collect(); + + let state = table.as_deref().and_then(|t| self.tables.get(t)); + let present = match (&table, state) { + (None, _) => true, // literal SELECT with no FROM + (Some(_), Some(s)) => s.present, + (Some(_), None) => false, // unknown table behaves as empty + }; + + let columns: Vec = items.iter().map(|(_, alias)| alias.clone()).collect(); + + let is_count = items + .iter() + .any(|(expr, _)| expr.to_ascii_uppercase().starts_with("COUNT(")); + if is_count { + let cells = items + .iter() + .map(|(expr, _)| { + if expr.to_ascii_uppercase().starts_with("COUNT(") { + Some(if present { "1" } else { "0" }.to_string()) + } else { + state.and_then(|s| s.row.get(&column_of(expr)).cloned()) + } + }) + .collect(); + return MockResponse::Rows { + columns, + rows: vec![cells], + }; + } + + if !present { + return MockResponse::Rows { + columns, + rows: vec![], + }; + } + + let cells = items + .iter() + .map(|(expr, _)| { + literal_value(expr) + .or_else(|| state.and_then(|s| s.row.get(&column_of(expr)).cloned())) + }) + .collect(); + MockResponse::Rows { + columns, + rows: vec![cells], + } + } +} + +fn to_row(cells: &[(&str, &str)]) -> HashMap { + cells + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect() +} + +fn capture(pattern: &str, sql: &str) -> Option { + Regex::new(pattern) + .unwrap() + .captures(sql) + .and_then(|c| c.get(1).map(|m| m.as_str().to_string())) +} + +/// Text between `SELECT` and the top-level `FROM` (or end of statement). +fn select_list(sql: &str) -> String { + let re = Regex::new(r"(?is)^\s*SELECT\s+(.*?)(?:\s+FROM\s+.*)?$").unwrap(); + re.captures(sql) + .and_then(|c| c.get(1).map(|m| m.as_str().trim().to_string())) + .unwrap_or_default() +} + +fn split_top_level_commas(s: &str) -> Vec { + let mut items = Vec::new(); + let mut depth = 0i32; + let mut in_quote = false; + let mut current = String::new(); + for ch in s.chars() { + match ch { + '\'' => { + in_quote = !in_quote; + current.push(ch); + } + '(' if !in_quote => { + depth += 1; + current.push(ch); + } + ')' if !in_quote => { + depth -= 1; + current.push(ch); + } + ',' if !in_quote && depth == 0 => { + items.push(current.trim().to_string()); + current.clear(); + } + _ => current.push(ch), + } + } + if !current.trim().is_empty() { + items.push(current.trim().to_string()); + } + items +} + +/// `expr AS alias` -> (expr, alias); `expr` -> (expr, bare column name). +fn parse_select_item(item: &str) -> (String, String) { + let re = Regex::new(r"(?is)^(.*?)\s+AS\s+(\w+)$").unwrap(); + if let Some(c) = re.captures(item) { + return ( + c.get(1).unwrap().as_str().trim().to_string(), + c.get(2).unwrap().as_str().to_string(), + ); + } + let expr = item.trim().to_string(); + let alias = column_of(&expr); + (expr, alias) +} + +/// Bare column name of an expression such as `t.col` or `col`. +fn column_of(expr: &str) -> String { + expr.trim() + .rsplit('.') + .next() + .unwrap_or(expr) + .trim() + .to_string() +} + +/// `'text'` -> text, `123` -> 123, otherwise `None` (a column reference). +fn literal_value(expr: &str) -> Option { + let e = expr.trim(); + if e.len() >= 2 && e.starts_with('\'') && e.ends_with('\'') { + return Some(e[1..e.len() - 1].to_string()); + } + if e.parse::().is_ok() { + return Some(e.to_string()); + } + None +} diff --git a/tests/common/mock_server.rs b/tests/common/mock_server.rs new file mode 100644 index 0000000..0b56e50 --- /dev/null +++ b/tests/common/mock_server.rs @@ -0,0 +1,260 @@ +//! Minimal in-process StackQL (PostgreSQL wire protocol v3) mock server. +//! +//! Speaks just enough of the simple-query protocol for `PgwireLite`: +//! startup handshake, `Q` (Query), `X` (Terminate), and the response +//! messages `T`/`D`/`C`/`E`/`Z`. Every SQL statement received is recorded so +//! tests can assert on exactly what stackql-deploy sent. + +use std::io::{self, Read, Write}; +use std::net::{TcpListener, TcpStream}; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex}; +use std::thread::{self, JoinHandle}; + +/// What the mock server answers for one SQL statement. +#[derive(Debug, Clone)] +pub enum MockResponse { + /// A result set. `None` cells are sent as SQL NULL. + Rows { + columns: Vec, + rows: Vec>>, + }, + /// A command tag such as `DELETE 1` or `INSERT 0 1`. + Command(String), + /// An `ErrorResponse` with the given message. + Error(String), +} + +impl MockResponse { + /// Convenience: a result set with zero rows. + pub fn empty() -> Self { + MockResponse::Rows { + columns: vec![], + rows: vec![], + } + } + + /// Convenience: a single-row result set. + pub fn single_row(cells: &[(&str, &str)]) -> Self { + MockResponse::Rows { + columns: cells.iter().map(|(c, _)| c.to_string()).collect(), + rows: vec![cells.iter().map(|(_, v)| Some(v.to_string())).collect()], + } + } +} + +type Handler = Box MockResponse + Send>; + +/// A running mock server bound to an ephemeral loopback port. +pub struct MockServer { + port: u16, + queries: Arc>>, + stop: Arc, + thread: Option>, +} + +impl MockServer { + /// Start a server whose responses are produced by `handler`. + pub fn start(handler: F) -> Self + where + F: FnMut(&str) -> MockResponse + Send + 'static, + { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind mock server"); + let port = listener.local_addr().unwrap().port(); + let queries: Arc>> = Arc::new(Mutex::new(Vec::new())); + let stop = Arc::new(AtomicBool::new(false)); + + let thread_queries = Arc::clone(&queries); + let thread_stop = Arc::clone(&stop); + let mut handler: Handler = Box::new(handler); + + let thread = thread::spawn(move || { + for stream in listener.incoming() { + if thread_stop.load(Ordering::SeqCst) { + break; + } + if let Ok(stream) = stream { + // Errors here mean the client hung up; that is normal at + // the end of a test. + let _ = serve_connection(stream, &mut handler, &thread_queries); + } + } + }); + + MockServer { + port, + queries, + stop, + thread: Some(thread), + } + } + + pub fn port(&self) -> u16 { + self.port + } + + /// Every SQL statement received so far, in order. + pub fn queries(&self) -> Vec { + self.queries.lock().unwrap().clone() + } + + /// Statements containing `needle`. + pub fn queries_containing(&self, needle: &str) -> Vec { + self.queries() + .into_iter() + .filter(|q| q.contains(needle)) + .collect() + } + + /// True when at least one statement contains `needle`. + pub fn received(&self, needle: &str) -> bool { + !self.queries_containing(needle).is_empty() + } +} + +impl Drop for MockServer { + fn drop(&mut self) { + self.stop.store(true, Ordering::SeqCst); + // Wake the accept loop so the thread can observe the stop flag. + let _ = TcpStream::connect(("127.0.0.1", self.port)); + if let Some(t) = self.thread.take() { + let _ = t.join(); + } + } +} + +// --------------------------------------------------------------------------- +// Wire protocol +// --------------------------------------------------------------------------- + +const SSL_REQUEST_CODE: i32 = 80877103; +const CANCEL_REQUEST_CODE: i32 = 80877102; + +fn serve_connection( + mut stream: TcpStream, + handler: &mut Handler, + log: &Arc>>, +) -> io::Result<()> { + // Startup: length-prefixed messages without a type byte. + loop { + let len = read_i32(&mut stream)? as usize; + let mut body = vec![0u8; len.saturating_sub(4)]; + stream.read_exact(&mut body)?; + if body.len() < 4 { + return Ok(()); + } + let code = i32::from_be_bytes([body[0], body[1], body[2], body[3]]); + if code == SSL_REQUEST_CODE { + stream.write_all(b"N")?; + continue; + } + if code == CANCEL_REQUEST_CODE { + return Ok(()); + } + break; + } + + // AuthenticationOk, then ReadyForQuery (idle). + write_msg(&mut stream, b'R', &0i32.to_be_bytes())?; + write_msg(&mut stream, b'Z', b"I")?; + + loop { + let mut msg_type = [0u8; 1]; + if stream.read_exact(&mut msg_type).is_err() { + return Ok(()); // client closed the socket + } + let len = read_i32(&mut stream)? as usize; + let mut body = vec![0u8; len.saturating_sub(4)]; + stream.read_exact(&mut body)?; + + match msg_type[0] { + b'Q' => { + let sql_bytes = body.strip_suffix(&[0u8]).unwrap_or(&body); + let sql = String::from_utf8_lossy(sql_bytes).into_owned(); + log.lock().unwrap().push(sql.clone()); + let response = handler(&sql); + write_response(&mut stream, response)?; + write_msg(&mut stream, b'Z', b"I")?; + } + b'X' => return Ok(()), + _ => { + // Unsupported message: answer with an error so the client + // does not hang, then return to idle. + write_error(&mut stream, "mock server: unsupported message")?; + write_msg(&mut stream, b'Z', b"I")?; + } + } + } +} + +fn write_response(stream: &mut TcpStream, response: MockResponse) -> io::Result<()> { + match response { + MockResponse::Rows { columns, rows } => { + // RowDescription + let mut t = Vec::new(); + t.extend_from_slice(&(columns.len() as i16).to_be_bytes()); + for col in &columns { + t.extend_from_slice(col.as_bytes()); + t.push(0); + t.extend_from_slice(&0i32.to_be_bytes()); // table OID + t.extend_from_slice(&0i16.to_be_bytes()); // attribute number + t.extend_from_slice(&25i32.to_be_bytes()); // type OID (text) + t.extend_from_slice(&(-1i16).to_be_bytes()); // type size + t.extend_from_slice(&(-1i32).to_be_bytes()); // type modifier + t.extend_from_slice(&0i16.to_be_bytes()); // format (text) + } + write_msg(stream, b'T', &t)?; + + // DataRow per row + for row in &rows { + let mut d = Vec::new(); + d.extend_from_slice(&(row.len() as i16).to_be_bytes()); + for cell in row { + match cell { + Some(v) => { + d.extend_from_slice(&(v.len() as i32).to_be_bytes()); + d.extend_from_slice(v.as_bytes()); + } + None => d.extend_from_slice(&(-1i32).to_be_bytes()), + } + } + write_msg(stream, b'D', &d)?; + } + + let tag = format!("SELECT {}\0", rows.len()); + write_msg(stream, b'C', tag.as_bytes()) + } + MockResponse::Command(tag) => { + let tag = format!("{}\0", tag); + write_msg(stream, b'C', tag.as_bytes()) + } + MockResponse::Error(msg) => write_error(stream, &msg), + } +} + +fn write_error(stream: &mut TcpStream, msg: &str) -> io::Result<()> { + let mut e = Vec::new(); + e.push(b'S'); + e.extend_from_slice(b"ERROR\0"); + e.push(b'C'); + e.extend_from_slice(b"XX000\0"); + e.push(b'M'); + e.extend_from_slice(msg.as_bytes()); + e.push(0); + e.push(0); + write_msg(stream, b'E', &e) +} + +fn write_msg(stream: &mut TcpStream, msg_type: u8, payload: &[u8]) -> io::Result<()> { + let mut msg = Vec::with_capacity(5 + payload.len()); + msg.push(msg_type); + msg.extend_from_slice(&((payload.len() + 4) as i32).to_be_bytes()); + msg.extend_from_slice(payload); + stream.write_all(&msg) +} + +fn read_i32(stream: &mut TcpStream) -> io::Result { + let mut buf = [0u8; 4]; + stream.read_exact(&mut buf)?; + Ok(i32::from_be_bytes(buf)) +} diff --git a/tests/common/mod.rs b/tests/common/mod.rs new file mode 100644 index 0000000..24242ff --- /dev/null +++ b/tests/common/mod.rs @@ -0,0 +1,152 @@ +//! Shared integration-test harness. +//! +//! - [`mock_server`]: an in-process PostgreSQL-wire mock of the stackql server +//! that records every statement it receives. +//! - [`fake_cloud`]: a stateful stand-in for provider tables. +//! - [`TestStack`]: copies a fixture stack from `tests/fixtures/` into a +//! temporary directory, optionally swapping in one of the fixture's +//! alternative manifests, and builds a `CommandRunner` connected to the +//! mock server. +//! - [`cli`]: runs the real binary against the live stacks under +//! `tests/live_stacks/` (used only by the `#[ignore]`d tests in +//! `tests/live.rs`). +//! +//! The mock layer needs no stackql binary, provider registry, or cloud +//! credentials. + +// Each test binary compiles this module separately and uses a different +// subset of it. +#![allow(dead_code, unused_imports)] + +pub mod cli; +pub mod fake_cloud; +pub mod mock_server; + +use std::fs; +use std::path::{Path, PathBuf}; + +use stackql_deploy::commands::base::CommandRunner; +use stackql_deploy::core::utils::UNKNOWN_EXPORT_PLACEHOLDER; +use stackql_deploy::utils::pgwire::PgwireLite; + +pub use fake_cloud::FakeCloud; +pub use mock_server::MockServer; + +/// Route stackql-deploy's `log` output through the test harness so a failing +/// test shows the same trail an operator would see. Honour `RUST_LOG` when +/// set; default to `info`. +pub fn init_test_logging() { + let mut builder = env_logger::Builder::from_env(env_logger::Env::default()); + if std::env::var_os("RUST_LOG").is_none() { + builder.filter_level(log::LevelFilter::Info); + } + let _ = builder.is_test(true).try_init(); +} + +/// Placeholder written into exports that teardown could not collect. +pub const UNKNOWN: &str = UNKNOWN_EXPORT_PLACEHOLDER; + +/// Absolute path of `tests/fixtures/`. +pub fn fixture_dir(name: &str) -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests") + .join("fixtures") + .join(name) +} + +/// A fixture stack copied into a temporary directory. +pub struct TestStack { + /// Kept alive so the directory is removed when the test ends. + _tempdir: tempfile::TempDir, + pub dir: PathBuf, +} + +impl TestStack { + /// Copy `tests/fixtures/` into a temp dir using its default + /// `stackql_manifest.yml`. + pub fn new(fixture: &str) -> Self { + Self::with_manifest(fixture, "stackql_manifest.yml") + } + + /// Copy `tests/fixtures/` into a temp dir and install + /// `/` as the stack's `stackql_manifest.yml`. Lets one + /// set of `resources/*.iql` files be shared by several manifest variants. + pub fn with_manifest(fixture: &str, manifest: &str) -> Self { + init_test_logging(); + let source = fixture_dir(fixture); + assert!( + source.is_dir(), + "fixture directory not found: {}", + source.display() + ); + let tempdir = tempfile::tempdir().expect("create temp dir"); + let dir = tempdir.path().join(fixture); + copy_dir(&source, &dir); + + let chosen = source.join(manifest); + assert!( + chosen.is_file(), + "manifest variant not found: {}", + chosen.display() + ); + fs::copy(&chosen, dir.join("stackql_manifest.yml")).expect("install manifest"); + + TestStack { + _tempdir: tempdir, + dir, + } + } + + /// Build a `CommandRunner` for this stack connected to `server`. + pub fn runner(&self, server: &MockServer, stack_env: &str) -> CommandRunner { + // `catch_error_and_exit` tries to stop a *local* stackql server on + // the globally configured host/port before exiting. Point the + // globals at a non-local host so that path can never touch a real + // server on this machine. `init_globals` is first-write-wins, which + // is fine: every test in a binary uses the same inert host. + stackql_deploy::globals::init_globals("mock-stackql.invalid".to_string(), server.port()); + + let client = PgwireLite::new("127.0.0.1", server.port(), false, "default") + .expect("connect to mock server"); + let env_file = self.dir.join(".env"); + CommandRunner::new( + client, + self.dir.to_str().expect("utf-8 stack dir"), + stack_env, + env_file.to_str().expect("utf-8 env file"), + &[], + ) + } +} + +fn copy_dir(from: &Path, to: &Path) { + fs::create_dir_all(to).expect("create dir"); + for entry in fs::read_dir(from).expect("read fixture dir") { + let entry = entry.expect("dir entry"); + let target = to.join(entry.file_name()); + if entry.path().is_dir() { + copy_dir(&entry.path(), &target); + } else { + fs::copy(entry.path(), &target).expect("copy fixture file"); + } + } +} + +/// Value of an exported variable in the runner's global context. +pub fn export_value(runner: &CommandRunner, name: &str) -> Option { + runner.global_context.get(name).cloned() +} + +/// Assert that no statement sent to the mock server interpolated the +/// `` placeholder. This is the core regression check for issue #56: +/// such a statement turns a missing export into a bogus hostname or +/// identifier and a fatal provider error. +pub fn assert_no_unknown_placeholder_sent(server: &MockServer) { + let offenders = server.queries_containing(UNKNOWN); + assert!( + offenders.is_empty(), + "statements containing {} were sent to the server:\n{}", + UNKNOWN, + offenders.join("\n---\n") + ); +} diff --git a/tests/fixtures/databricks_workspace/manifest_skip_query.yml b/tests/fixtures/databricks_workspace/manifest_skip_query.yml new file mode 100644 index 0000000..4446ab1 --- /dev/null +++ b/tests/fixtures/databricks_workspace/manifest_skip_query.yml @@ -0,0 +1,56 @@ +# Variant of stackql_manifest.yml: workspace_ready (type: query) opts out of +# teardown with skip_on_delete: true. +version: 1 +name: dbx-fixture +description: databricks workspace fixture for integration tests +providers: + - databricks_account::v26.07.00430 + - databricks_workspace::v26.07.00430 + +globals: + - name: account_id + value: "acct-0000" + - name: prefix + value: "{{ stack_name }}-{{ stack_env }}" + +resources: + + - name: workspace + description: the account-level workspace, exports the deployment name + file: workspace.iql + props: + - name: workspace_name + value: "{{ prefix }}-workspace" + exports: + - workspace_id + - deployment_name + + - name: workspace_ready + description: wait until the workspace answers the workspace API as the deploying principal + type: query + file: current_user.iql + skip_on_delete: true + props: [] + exports: + - workspace_principal + + - name: storage_credential + description: a workspace-level resource addressed by deployment_name + file: storage_credential.iql + props: + - name: name + value: "{{ prefix }}_credential" + exports: + - storage_credential_id + + - name: estate_summary + description: inline query over the workspace as the account sees it + type: query + props: [] + sql: | + SELECT workspace_status + FROM databricks_account.provisioning.workspaces + WHERE account_id = '{{ account_id }}' + AND workspace_id = '{{ workspace_id }}'; + exports: + - workspace_status diff --git a/tests/fixtures/databricks_workspace/manifest_skip_resource.yml b/tests/fixtures/databricks_workspace/manifest_skip_resource.yml new file mode 100644 index 0000000..bca246f --- /dev/null +++ b/tests/fixtures/databricks_workspace/manifest_skip_resource.yml @@ -0,0 +1,56 @@ +# Variant of stackql_manifest.yml: the workspace resource is retained on +# teardown with skip_on_delete: true. +version: 1 +name: dbx-fixture +description: databricks workspace fixture for integration tests +providers: + - databricks_account::v26.07.00430 + - databricks_workspace::v26.07.00430 + +globals: + - name: account_id + value: "acct-0000" + - name: prefix + value: "{{ stack_name }}-{{ stack_env }}" + +resources: + + - name: workspace + description: the account-level workspace, exports the deployment name + file: workspace.iql + skip_on_delete: true + props: + - name: workspace_name + value: "{{ prefix }}-workspace" + exports: + - workspace_id + - deployment_name + + - name: workspace_ready + description: wait until the workspace answers the workspace API as the deploying principal + type: query + file: current_user.iql + props: [] + exports: + - workspace_principal + + - name: storage_credential + description: a workspace-level resource addressed by deployment_name + file: storage_credential.iql + props: + - name: name + value: "{{ prefix }}_credential" + exports: + - storage_credential_id + + - name: estate_summary + description: inline query over the workspace as the account sees it + type: query + props: [] + sql: | + SELECT workspace_status + FROM databricks_account.provisioning.workspaces + WHERE account_id = '{{ account_id }}' + AND workspace_id = '{{ workspace_id }}'; + exports: + - workspace_status diff --git a/tests/fixtures/databricks_workspace/manifest_with_condition.yml b/tests/fixtures/databricks_workspace/manifest_with_condition.yml new file mode 100644 index 0000000..8abae75 --- /dev/null +++ b/tests/fixtures/databricks_workspace/manifest_with_condition.yml @@ -0,0 +1,56 @@ +# Variant of stackql_manifest.yml: estate_summary carries an `if` condition +# that is false for every environment except prd. +version: 1 +name: dbx-fixture +description: databricks workspace fixture for integration tests +providers: + - databricks_account::v26.07.00430 + - databricks_workspace::v26.07.00430 + +globals: + - name: account_id + value: "acct-0000" + - name: prefix + value: "{{ stack_name }}-{{ stack_env }}" + +resources: + + - name: workspace + description: the account-level workspace, exports the deployment name + file: workspace.iql + props: + - name: workspace_name + value: "{{ prefix }}-workspace" + exports: + - workspace_id + - deployment_name + + - name: workspace_ready + description: wait until the workspace answers the workspace API as the deploying principal + type: query + file: current_user.iql + props: [] + exports: + - workspace_principal + + - name: storage_credential + description: a workspace-level resource addressed by deployment_name + file: storage_credential.iql + props: + - name: name + value: "{{ prefix }}_credential" + exports: + - storage_credential_id + + - name: estate_summary + description: inline query over the workspace as the account sees it; only in prd + type: query + if: "'{{ stack_env }}' == 'prd'" + props: [] + sql: | + SELECT workspace_status + FROM databricks_account.provisioning.workspaces + WHERE account_id = '{{ account_id }}' + AND workspace_id = '{{ workspace_id }}'; + exports: + - workspace_status diff --git a/tests/fixtures/databricks_workspace/manifest_with_delete_callback.yml b/tests/fixtures/databricks_workspace/manifest_with_delete_callback.yml new file mode 100644 index 0000000..7feb047 --- /dev/null +++ b/tests/fixtures/databricks_workspace/manifest_with_delete_callback.yml @@ -0,0 +1,58 @@ +# Variant of stackql_manifest.yml: the workspace delete uses RETURNING * with +# return_vals.delete and a callback:delete anchor. +version: 1 +name: dbx-fixture +description: databricks workspace fixture for integration tests +providers: + - databricks_account::v26.07.00430 + - databricks_workspace::v26.07.00430 + +globals: + - name: account_id + value: "acct-0000" + - name: prefix + value: "{{ stack_name }}-{{ stack_env }}" + +resources: + + - name: workspace + description: the account-level workspace, exports the deployment name + file: workspace_with_callback.iql + return_vals: + delete: + - workspace_id + props: + - name: workspace_name + value: "{{ prefix }}-workspace" + exports: + - workspace_id + - deployment_name + + - name: workspace_ready + description: wait until the workspace answers the workspace API as the deploying principal + type: query + file: current_user.iql + props: [] + exports: + - workspace_principal + + - name: storage_credential + description: a workspace-level resource addressed by deployment_name + file: storage_credential.iql + props: + - name: name + value: "{{ prefix }}_credential" + exports: + - storage_credential_id + + - name: estate_summary + description: inline query over the workspace as the account sees it + type: query + props: [] + sql: | + SELECT workspace_status + FROM databricks_account.provisioning.workspaces + WHERE account_id = '{{ account_id }}' + AND workspace_id = '{{ workspace_id }}'; + exports: + - workspace_status diff --git a/tests/fixtures/databricks_workspace/manifest_with_script.yml b/tests/fixtures/databricks_workspace/manifest_with_script.yml new file mode 100644 index 0000000..5b08cd2 --- /dev/null +++ b/tests/fixtures/databricks_workspace/manifest_with_script.yml @@ -0,0 +1,61 @@ +# Variant of stackql_manifest.yml: adds a script resource with an export. +version: 1 +name: dbx-fixture +description: databricks workspace fixture for integration tests +providers: + - databricks_account::v26.07.00430 + - databricks_workspace::v26.07.00430 + +globals: + - name: account_id + value: "acct-0000" + - name: prefix + value: "{{ stack_name }}-{{ stack_env }}" + +resources: + + - name: workspace + description: the account-level workspace, exports the deployment name + file: workspace.iql + props: + - name: workspace_name + value: "{{ prefix }}-workspace" + exports: + - workspace_id + - deployment_name + + - name: workspace_ready + description: wait until the workspace answers the workspace API as the deploying principal + type: query + file: current_user.iql + props: [] + exports: + - workspace_principal + + - name: storage_credential + description: a workspace-level resource addressed by deployment_name + file: storage_credential.iql + props: + - name: name + value: "{{ prefix }}_credential" + exports: + - storage_credential_id + + - name: post_deploy_hook + description: an external script; never executed during teardown + type: script + run: "echo '{\"hook_result\": \"ok\"}'" + exports: + - hook_result + + - name: estate_summary + description: inline query over the workspace as the account sees it + type: query + props: [] + sql: | + SELECT workspace_status + FROM databricks_account.provisioning.workspaces + WHERE account_id = '{{ account_id }}' + AND workspace_id = '{{ workspace_id }}'; + exports: + - workspace_status diff --git a/tests/fixtures/databricks_workspace/resources/current_user.iql b/tests/fixtures/databricks_workspace/resources/current_user.iql new file mode 100644 index 0000000..5094f28 --- /dev/null +++ b/tests/fixtures/databricks_workspace/resources/current_user.iql @@ -0,0 +1,4 @@ +/*+ exports, retries=2, retry_delay=0 */ +SELECT userName AS workspace_principal +FROM databricks_workspace.iam.current_user +WHERE deployment_name = '{{ deployment_name }}' diff --git a/tests/fixtures/databricks_workspace/resources/storage_credential.iql b/tests/fixtures/databricks_workspace/resources/storage_credential.iql new file mode 100644 index 0000000..27d52b1 --- /dev/null +++ b/tests/fixtures/databricks_workspace/resources/storage_credential.iql @@ -0,0 +1,31 @@ +/*+ exists */ +SELECT id AS storage_credential_id +FROM databricks_workspace.catalog.storage_credentials +WHERE deployment_name = '{{ deployment_name }}' +AND name = '{{ name }}' + +/*+ create */ +INSERT INTO databricks_workspace.catalog.storage_credentials ( + name, + deployment_name +) +SELECT + '{{ name }}', + '{{ deployment_name }}' + +/*+ statecheck, retries=1, retry_delay=0 */ +SELECT COUNT(*) AS count +FROM databricks_workspace.catalog.storage_credentials +WHERE deployment_name = '{{ deployment_name }}' +AND name = '{{ name }}' + +/*+ exports */ +SELECT id AS storage_credential_id +FROM databricks_workspace.catalog.storage_credentials +WHERE deployment_name = '{{ deployment_name }}' +AND name = '{{ name }}' + +/*+ delete */ +DELETE FROM databricks_workspace.catalog.storage_credentials +WHERE name = '{{ name }}' +AND deployment_name = '{{ deployment_name }}' diff --git a/tests/fixtures/databricks_workspace/resources/workspace.iql b/tests/fixtures/databricks_workspace/resources/workspace.iql new file mode 100644 index 0000000..f41d259 --- /dev/null +++ b/tests/fixtures/databricks_workspace/resources/workspace.iql @@ -0,0 +1,32 @@ +/*+ exists */ +SELECT COUNT(*) AS count +FROM databricks_account.provisioning.workspaces +WHERE account_id = '{{ account_id }}' +AND workspace_name = '{{ workspace_name }}' + +/*+ create */ +INSERT INTO databricks_account.provisioning.workspaces ( + account_id, + workspace_name +) +SELECT + '{{ account_id }}', + '{{ workspace_name }}' + +/*+ statecheck, retries=1, retry_delay=0 */ +SELECT COUNT(*) AS count +FROM databricks_account.provisioning.workspaces +WHERE account_id = '{{ account_id }}' +AND workspace_name = '{{ workspace_name }}' +AND workspace_status = 'RUNNING' + +/*+ exports */ +SELECT workspace_id, deployment_name +FROM databricks_account.provisioning.workspaces +WHERE account_id = '{{ account_id }}' +AND workspace_name = '{{ workspace_name }}' + +/*+ delete */ +DELETE FROM databricks_account.provisioning.workspaces +WHERE account_id = '{{ account_id }}' +AND workspace_id = '{{ workspace_id }}' diff --git a/tests/fixtures/databricks_workspace/resources/workspace_with_callback.iql b/tests/fixtures/databricks_workspace/resources/workspace_with_callback.iql new file mode 100644 index 0000000..5e9cc26 --- /dev/null +++ b/tests/fixtures/databricks_workspace/resources/workspace_with_callback.iql @@ -0,0 +1,39 @@ +/*+ exists */ +SELECT COUNT(*) AS count +FROM databricks_account.provisioning.workspaces +WHERE account_id = '{{ account_id }}' +AND workspace_name = '{{ workspace_name }}' + +/*+ create */ +INSERT INTO databricks_account.provisioning.workspaces ( + account_id, + workspace_name +) +SELECT + '{{ account_id }}', + '{{ workspace_name }}' + +/*+ statecheck, retries=1, retry_delay=0 */ +SELECT COUNT(*) AS count +FROM databricks_account.provisioning.workspaces +WHERE account_id = '{{ account_id }}' +AND workspace_name = '{{ workspace_name }}' +AND workspace_status = 'RUNNING' + +/*+ exports */ +SELECT workspace_id, deployment_name +FROM databricks_account.provisioning.workspaces +WHERE account_id = '{{ account_id }}' +AND workspace_name = '{{ workspace_name }}' + +/*+ delete */ +DELETE FROM databricks_account.provisioning.workspaces +WHERE account_id = '{{ account_id }}' +AND workspace_id = '{{ workspace_id }}' +RETURNING * + +/*+ callback:delete, retries=2, retry_delay=0 */ +SELECT workspace_status = 'DELETED' AS success +FROM databricks_account.provisioning.workspaces +WHERE account_id = '{{ account_id }}' +AND workspace_id = '{{ callback.workspace_id }}' diff --git a/tests/fixtures/databricks_workspace/stackql_manifest.yml b/tests/fixtures/databricks_workspace/stackql_manifest.yml new file mode 100644 index 0000000..c02fcd7 --- /dev/null +++ b/tests/fixtures/databricks_workspace/stackql_manifest.yml @@ -0,0 +1,58 @@ +# Baseline fixture: a Databricks-on-AWS style stack where a query resource +# (workspace_ready) and a workspace-level resource (storage_credential) both +# depend on an export (deployment_name) produced by the account-level +# workspace resource. Used by the integration tests in tests/teardown.rs and +# tests/build.rs; the manifest_*.yml siblings are variants of this file. +version: 1 +name: dbx-fixture +description: databricks workspace fixture for integration tests +providers: + - databricks_account::v26.07.00430 + - databricks_workspace::v26.07.00430 + +globals: + - name: account_id + value: "acct-0000" + - name: prefix + value: "{{ stack_name }}-{{ stack_env }}" + +resources: + + - name: workspace + description: the account-level workspace, exports the deployment name + file: workspace.iql + props: + - name: workspace_name + value: "{{ prefix }}-workspace" + exports: + - workspace_id + - deployment_name + + - name: workspace_ready + description: wait until the workspace answers the workspace API as the deploying principal + type: query + file: current_user.iql + props: [] + exports: + - workspace_principal + + - name: storage_credential + description: a workspace-level resource addressed by deployment_name + file: storage_credential.iql + props: + - name: name + value: "{{ prefix }}_credential" + exports: + - storage_credential_id + + - name: estate_summary + description: inline query over the workspace as the account sees it + type: query + props: [] + sql: | + SELECT workspace_status + FROM databricks_account.provisioning.workspaces + WHERE account_id = '{{ account_id }}' + AND workspace_id = '{{ workspace_id }}'; + exports: + - workspace_status diff --git a/tests/live.rs b/tests/live.rs new file mode 100644 index 0000000..e7c6d2a --- /dev/null +++ b/tests/live.rs @@ -0,0 +1,317 @@ +//! Live integration tests: the real `stackql-deploy` binary against real +//! providers, using only free resources (SSM Parameter Store standard +//! parameters, GitHub repository labels) and a unique name suffix per run. +//! +//! These tests are `#[ignore]`d so that a plain `cargo test` never touches a +//! cloud account. Run them with credentials in the environment: +//! +//! ```text +//! cargo test --test live -- --ignored --test-threads=1 --nocapture +//! ``` +//! +//! or via `bash ci-scripts/integration-test.sh`, which is what the +//! pre-merge CI workflow runs. See `tests/README.md` for the environment +//! variables each stack needs. + +mod common; + +use common::cli::{ + env_or, export_str, read_exports_json, require_env, run_id, LiveStack, TeardownGuard, +}; + +const ENV: &str = "ci"; + +/// Every stackql-deploy code path reachable from a manifest, on AWS SSM +/// Parameter Store: create with RETURNING / return_vals / callback, statecheck, +/// PatchDocument update, createorupdate, query / command / script resources, +/// conditions, file() and merge, protected values, stack exports, dry runs, +/// test pass and fail, teardown with skip_on_delete, idempotent re-runs. +#[test] +#[ignore = "live AWS test; run with --ignored and AWS credentials in the environment"] +fn live_aws_ssm_full_lifecycle() { + require_env("AWS_ACCESS_KEY_ID", "the AWS SSM live test"); + require_env("AWS_SECRET_ACCESS_KEY", "the AWS SSM live test"); + let region = env_or("AWS_REGION", "us-east-1"); + let run = run_id(); + let secret_v1 = format!("sqld-secret-{}-v1", run); + let secret_v2 = format!("sqld-secret-{}-v2", run); + + let stack = LiveStack::new("aws_ssm", 5451); + stack + .input("AWS_REGION", ®ion) + .input("RUN_ID", &run) + .input("APP_CONFIG_VALUE", &secret_v1); + let mut guard = TeardownGuard::new(&stack, ENV); + + let app_config_name = format!("/stackql-deploy/{}/{}/app-config", ENV, run); + let feature_flags_name = format!("/stackql-deploy/{}/{}/feature-flags", ENV, run); + let deployed_by = format!("stackql-deploy-live-{}", run); + + // 1. Dry run: every query rendered, nothing executed. + stack + .run("build", ENV, &["--dry-run"]) + .assert_success() + .assert_contains("dry run create for [app_config]") + .assert_contains("dry run script for [deploy_marker]") + .assert_contains("dry-run build complete") + .assert_not_contains("creating [app_config]..."); + + // 2. Build from nothing. + let exports_json = stack.scratch("exports.json"); + let out = stack.run( + "build", + ENV, + &[ + "--show-queries", + "--output-file", + exports_json.to_str().unwrap(), + ], + ); + out.assert_success() + // Cloud Control create with RETURNING *, return_vals and callback + .assert_contains("creating [app_config]...") + .assert_contains("RETURNING [Identifier] for [app_config] captured as [this.identifier]") + .assert_contains("[app_config] create callback completed successfully") + .assert_contains("[app_config] is in the desired state") + .assert_contains("successfully deployed app_config") + // query resource with skip_on_delete runs normally on build + .assert_contains("successfully exported variables for query in app_config_lookup") + // createorupdate resource + .assert_contains("creating [feature_flags]...") + .assert_contains("createorupdate for [feature_flags] is authoritative") + // conditions + .assert_contains("Skipping resource [prd_only_check] due to condition") + // command and script resources + .assert_contains("running command...") + .assert_contains("Exported variables from script") + // protected input is masked everywhere, including --show-queries + .assert_not_contains(&secret_v1) + .assert_contains("set protected variable [app_config_value]"); + assert!(stack.exports_file().is_file(), "exports file written"); + let exports = read_exports_json(&exports_json); + assert_eq!(export_str(&exports, "app_config_name"), app_config_name); + assert_eq!(export_str(&exports, "app_config_version"), "1"); + assert!( + export_str(&exports, "app_config_arn").ends_with(&format!(":parameter{}", app_config_name)) + ); + assert_eq!( + export_str(&exports, "feature_flags_name"), + feature_flags_name + ); + assert_eq!( + export_str(&exports, "feature_flags_deployed_by"), + deployed_by + ); + assert_eq!(export_str(&exports, "param_count"), "2"); + assert_eq!(export_str(&exports, "deployed_by"), deployed_by); + assert_eq!(export_str(&exports, "stack_env"), ENV); + + // 3. Test: everything is in the desired state. + stack + .run("test", ENV, &[]) + .assert_success() + .assert_contains("test passed for app_config") + .assert_contains("test passed for feature_flags") + .assert_contains("Skipping resource [prd_only_check] due to condition") + .assert_contains("Exported variables from script") + .assert_not_contains(&secret_v1); + + // 4. Build again: idempotent, no create or update. + stack + .run("build", ENV, &[]) + .assert_success() + .assert_contains("[app_config] is in the desired state") + .assert_not_contains("creating [app_config]...") + .assert_not_contains("updating [app_config]..."); + + // 5. Change an input: statecheck fails, PatchDocument update, statecheck passes. + stack.input("APP_CONFIG_VALUE", &secret_v2); + let out = stack.run( + "build", + ENV, + &["--output-file", exports_json.to_str().unwrap()], + ); + out.assert_success() + .assert_contains("[app_config] is not in the desired state") + .assert_contains("updating [app_config]...") + .assert_contains("[app_config] update callback completed successfully") + .assert_contains("successfully deployed app_config") + .assert_not_contains(&secret_v2); + let exports = read_exports_json(&exports_json); + assert_eq!(export_str(&exports, "app_config_version"), "2"); + + // 6. Test with a stale input fails. + stack.input("APP_CONFIG_VALUE", &secret_v1); + stack + .run("test", ENV, &[]) + .assert_failure() + .assert_contains("[app_config] is not in the desired state") + .assert_contains("test failed for app_config"); + stack.input("APP_CONFIG_VALUE", &secret_v2); + + // 7. Teardown dry run shows the deletes and the skipped query. + stack + .run("teardown", ENV, &["--dry-run"]) + .assert_success() + .assert_contains("[app_config_lookup] skip_on_delete is set") + .assert_contains("dry run delete for [feature_flags]") + .assert_contains("dry run delete for [app_config]") + .assert_contains("dry-run teardown complete"); + + // 8. Teardown. + stack + .run("teardown", ENV, &[]) + .assert_success() + .assert_contains( + "[app_config_lookup] skip_on_delete is set, query not executed during teardown", + ) + .assert_contains("successfully deleted feature_flags") + .assert_contains( + "RETURNING [RequestToken] for [app_config] captured as [this.RequestToken]", + ) + .assert_contains("[app_config] delete callback completed successfully") + .assert_contains("successfully deleted app_config") + .assert_not_contains("could not be confirmed") + .assert_contains("teardown complete"); + guard.disarm(); + + // 9. Teardown again: nothing left, still succeeds. + stack + .run("teardown", ENV, &[]) + .assert_success() + .assert_contains("resource [feature_flags] does not exist, skipping delete") + .assert_contains("resource [app_config] does not exist, skipping delete"); +} + +/// Two deletes that can never succeed. S3 DeleteBucket on a bucket that +/// never existed is rejected synchronously (the exact failure from issue +/// #56's teardown log): the default aborts the run, `--on-failure ignore` +/// reports and continues. A Cloud Control DeleteResource on a parameter that +/// never existed fails asynchronously: the statement succeeds, the +/// post-delete check fails, and the `troubleshoot:delete` anchor surfaces the +/// ProgressEvent error through the RequestToken captured by +/// `return_vals.delete`. +#[test] +#[ignore = "live AWS test; run with --ignored and AWS credentials in the environment"] +fn live_teardown_on_failure_modes() { + require_env("AWS_ACCESS_KEY_ID", "the AWS on-failure live test"); + require_env("AWS_SECRET_ACCESS_KEY", "the AWS on-failure live test"); + let region = env_or("AWS_REGION", "us-east-1"); + let run = run_id(); + + let stack = LiveStack::new("aws_ssm_onfailure", 5452); + stack.input("AWS_REGION", ®ion).input("RUN_ID", &run); + + // ghost_parameter is processed first (reverse manifest order), so its + // troubleshoot diagnostics appear before the S3 failure aborts the run. + stack + .run("teardown", ENV, &[]) + .assert_failure() + .assert_contains( + "RETURNING [RequestToken] for [ghost_parameter] captured as [this.RequestToken]", + ) + .assert_contains("[ghost_parameter] troubleshoot diagnostics (delete)") + .assert_contains("\"ErrorCode\": \"NotFound\"") + .assert_contains("[ghost_parameter] delete could not be confirmed") + .assert_contains("deleting [ghost]...") + .assert_contains("Exception during stackql command execution") + .assert_contains("stackql-deploy operation failed"); + + stack + .run("teardown", ENV, &["--on-failure", "ignore"]) + .assert_success() + .assert_contains("on-failure=ignore") + .assert_contains("[ghost_parameter] troubleshoot diagnostics (delete)") + .assert_contains("Command failed (ignored)") + .assert_contains("[ghost] delete could not be confirmed") + .assert_contains("whose delete could not be confirmed: [ghost_parameter], [ghost]") + .assert_contains("teardown complete"); +} + +/// Plain REST provider (no RETURNING, 404-as-not-found exists check) on +/// GitHub repository labels: create, idempotent re-build, update, test, +/// teardown, idempotent re-teardown. +#[test] +#[ignore = "live GitHub test; run with --ignored and a GitHub token with issues:write in the environment"] +fn live_github_labels_lifecycle() { + require_env("STACKQL_GITHUB_USERNAME", "the GitHub labels live test"); + require_env("STACKQL_GITHUB_PASSWORD", "the GitHub labels live test"); + let (default_owner, default_repo) = match std::env::var("GITHUB_REPOSITORY") { + Ok(full) if full.contains('/') => { + let (o, r) = full.split_once('/').unwrap(); + (o.to_string(), r.to_string()) + } + _ => ("stackql".to_string(), "stackql-deploy-rs".to_string()), + }; + let owner = env_or("GITHUB_OWNER", &default_owner); + let repo = env_or("GITHUB_REPO", &default_repo); + let run = run_id(); + let label_name = format!("stackql-deploy-live-{}", run); + + let stack = LiveStack::new("github_labels", 5453); + stack + .input("GITHUB_OWNER", &owner) + .input("GITHUB_REPO", &repo) + .input("RUN_ID", &run) + .input("LABEL_COLOR", "0e8a16"); + let mut guard = TeardownGuard::new(&stack, ENV); + + // Build from nothing: 404 on the exists check means "create". + let exports_json = stack.scratch("exports.json"); + stack + .run( + "build", + ENV, + &["--output-file", exports_json.to_str().unwrap()], + ) + .assert_success() + .assert_contains("[run_label] does not exist") + .assert_contains("creating [run_label]...") + .assert_contains("[run_label] is in the desired state") + .assert_contains("successfully deployed run_label"); + let exports = read_exports_json(&exports_json); + assert!( + export_str(&exports, "run_label_id").parse::().is_ok(), + "run_label_id is numeric" + ); + assert!(export_str(&exports, "run_label_url").ends_with(&format!("/labels/{}", label_name))); + assert!( + export_str(&exports, "label_count") + .parse::() + .unwrap_or(0) + >= 1 + ); + + // Idempotent re-build. + stack + .run("build", ENV, &[]) + .assert_success() + .assert_not_contains("creating [run_label]...") + .assert_not_contains("updating [run_label]..."); + + // Change the colour: update path. + stack.input("LABEL_COLOR", "d73a4a"); + stack + .run("build", ENV, &[]) + .assert_success() + .assert_contains("[run_label] is not in the desired state") + .assert_contains("updating [run_label]...") + .assert_contains("successfully deployed run_label"); + + stack + .run("test", ENV, &[]) + .assert_success() + .assert_contains("test passed for run_label"); + + stack + .run("teardown", ENV, &[]) + .assert_success() + .assert_contains("successfully deleted run_label") + .assert_contains("teardown complete"); + guard.disarm(); + + stack + .run("teardown", ENV, &[]) + .assert_success() + .assert_contains("resource [run_label] does not exist, skipping delete"); +} diff --git a/tests/live_stacks/aws_ssm/resources/app_config_tags.json b/tests/live_stacks/aws_ssm/resources/app_config_tags.json new file mode 100644 index 0000000..15d73d4 --- /dev/null +++ b/tests/live_stacks/aws_ssm/resources/app_config_tags.json @@ -0,0 +1,4 @@ +{ + "stackql:resource": "app_config", + "stackql:purpose": "live-integration-test" +} diff --git a/tests/live_stacks/aws_ssm/resources/aws_get_parameter.iql b/tests/live_stacks/aws_ssm/resources/aws_get_parameter.iql new file mode 100644 index 0000000..3903921 --- /dev/null +++ b/tests/live_stacks/aws_ssm/resources/aws_get_parameter.iql @@ -0,0 +1,7 @@ +/*+ exports, retries=3, retry_delay=5 */ +SELECT + JSON_EXTRACT(parameters, '$[0].Version') AS app_config_version, + JSON_EXTRACT(parameters, '$[0].ARN') AS app_config_arn +FROM aws.ssm.parameters +WHERE region = '{{ region }}' +AND Names = '{{ lookup_names }}' diff --git a/tests/live_stacks/aws_ssm/resources/aws_parameter.iql b/tests/live_stacks/aws_ssm/resources/aws_parameter.iql new file mode 100644 index 0000000..a5897d2 --- /dev/null +++ b/tests/live_stacks/aws_ssm/resources/aws_parameter.iql @@ -0,0 +1,21 @@ +/*+ exists, postdelete_retries=6, postdelete_retry_delay=5 */ +SELECT COUNT(*) AS count +FROM awscc.ssm.parameters_list_only +WHERE region = '{{ region }}' +AND name = '{{ param_name }}' + +/*+ createorupdate */ +REPLACE aws.ssm.parameters +SET Name = '{{ param_name }}', + Type = 'StringList', + Value = '{{ param_value }}', + Overwrite = 'true' +WHERE region = '{{ region }}' + +/*+ exports */ +SELECT '{{ param_name }}' AS feature_flags_name + +/*+ delete */ +DELETE FROM aws.ssm.parameters +WHERE Name = '{{ param_name }}' +AND region = '{{ region }}' diff --git a/tests/live_stacks/aws_ssm/resources/awscc_parameter.iql b/tests/live_stacks/aws_ssm/resources/awscc_parameter.iql new file mode 100644 index 0000000..2e1c157 --- /dev/null +++ b/tests/live_stacks/aws_ssm/resources/awscc_parameter.iql @@ -0,0 +1,80 @@ +/*+ exists, postdelete_retries=6, postdelete_retry_delay=5 */ +SELECT COUNT(*) AS count +FROM awscc.ssm.parameters_list_only +WHERE region = '{{ region }}' +AND name = '{{ param_name }}' + +/*+ create */ +INSERT INTO awscc.ssm.parameters ( + Name, + Type, + Value, + Description, + Tags, + region +) +SELECT + '{{ param_name }}', + 'String', + '{{ param_value }}', + '{{ param_description }}', + '{{ param_tags }}', + '{{ region }}' +RETURNING * + +/*+ callback:create, retries=6, retry_delay=5, short_circuit_field=OperationStatus, short_circuit_value=SUCCESS */ +SELECT OperationStatus = 'SUCCESS' AS success +FROM awscc.cloud_control.resource_request +WHERE region = '{{ region }}' +AND RequestToken = '{{ callback.RequestToken }}' + +/*+ statecheck, retries=3, retry_delay=5 */ +SELECT COUNT(*) AS count FROM +( + SELECT JSON_EQUAL(tags, '{{ param_tags }}') AS tags_match + FROM awscc.ssm.parameters + WHERE region = '{{ region }}' + AND Identifier = '{{ param_name }}' + AND value = '{{ param_value }}' +) t +WHERE tags_match = 1 + +/*+ update */ +UPDATE awscc.ssm.parameters +SET PatchDocument = string('{{ { + "Value": param_value, + "Tags": param_tags + } | generate_patch_document }}') +WHERE Identifier = '{{ param_name }}' +AND region = '{{ region }}' +RETURNING * + +/*+ callback:update, retries=6, retry_delay=5, short_circuit_field=OperationStatus, short_circuit_value=SUCCESS */ +SELECT OperationStatus = 'SUCCESS' AS success +FROM awscc.cloud_control.resource_request +WHERE region = '{{ region }}' +AND RequestToken = '{{ callback.RequestToken }}' + +/*+ exports */ +SELECT name AS app_config_name, value AS app_config_value +FROM awscc.ssm.parameters +WHERE region = '{{ region }}' +AND Identifier = '{{ param_name }}' + +/*+ delete */ +DELETE FROM awscc.ssm.parameters +WHERE Identifier = '{{ param_name }}' +AND region = '{{ region }}' +RETURNING * + +/*+ callback:delete, retries=6, retry_delay=5, short_circuit_field=OperationStatus, short_circuit_value=SUCCESS */ +SELECT OperationStatus = 'SUCCESS' AS success +FROM awscc.cloud_control.resource_request +WHERE region = '{{ region }}' +AND RequestToken = '{{ callback.RequestToken }}' + +/*+ troubleshoot */ +SELECT OperationStatus, ErrorCode, StatusMessage +FROM awscc.cloud_control.resource_request +WHERE region = '{{ region }}' +AND RequestToken = '{{ this.RequestToken }}' diff --git a/tests/live_stacks/aws_ssm/stackql_manifest.yml b/tests/live_stacks/aws_ssm/stackql_manifest.yml new file mode 100644 index 0000000..ca6c67c --- /dev/null +++ b/tests/live_stacks/aws_ssm/stackql_manifest.yml @@ -0,0 +1,191 @@ +version: 1 +name: sqld-live-ssm +description: | + Live integration stack for stackql-deploy built on AWS Systems Manager + Parameter Store (standard tier parameters are free). Exercises the full + resource lifecycle against two DML styles - Cloud Control (awscc: INSERT and + UPDATE with RETURNING, callbacks, PatchDocument) and the native API (aws: + REPLACE as createorupdate) - plus query, command and script resource types, + file() directives, per-environment values, merged tags, conditions, + return_vals, protected exports, skip_on_delete and stack-level exports. + + Note: this stackql release does not support LIKE against the Cloud Control + *_list_only views (it works on ordinary list tables), so prefix_summary + filters with INSTR instead. + + Inputs (pass with -e): AWS_REGION, RUN_ID, APP_CONFIG_VALUE. + Every parameter is created under /stackql-deploy/// so + concurrent runs never collide. +providers: + - awscc::v26.07.00426 + - aws::v26.08.00444 + +globals: + - name: region + value: "{{ AWS_REGION }}" + - name: run_id + description: unique suffix per run (for example the CI run id) + value: "{{ RUN_ID }}" + - name: param_prefix + value: "/stackql-deploy/{{ stack_env }}/{{ run_id }}" + - name: app_config_value + description: changed between builds by the test harness to drive the update path; masked in all output + value: "{{ APP_CONFIG_VALUE }}" + protected: true + - name: global_tags + value: + stackql:stack-name: "{{ stack_name }}" + stackql:stack-env: "{{ stack_env }}" + stackql:run-id: "{{ run_id }}" + +resources: + + # --------------------------------------------------------------------------- + # Cloud Control resource: INSERT ... RETURNING *, return_vals, callback, + # PatchDocument update, protected export + # --------------------------------------------------------------------------- + - name: app_config + description: application configuration parameter managed through the Cloud Control API + file: awscc_parameter.iql + props: + - name: param_name + value: "{{ param_prefix }}/app-config" + - name: param_value + value: "{{ app_config_value }}" + - name: param_description + values: + ci: + value: "stackql-deploy live test parameter (ci)" + dev: + value: "stackql-deploy live test parameter (dev)" + - name: param_tags + merge: + - global_tags + value: file(app_config_tags.json) + return_vals: + create: + - Identifier: identifier + - OperationStatus + - RequestToken + update: + - RequestToken + delete: + - RequestToken + - OperationStatus + exports: + - app_config_name + - app_config_value + protected: + - app_config_value + + # --------------------------------------------------------------------------- + # Query resource reading the same parameter through the native API, with + # JSON extraction. Opts out of teardown. + # --------------------------------------------------------------------------- + - name: app_config_lookup + description: version and ARN of app_config via the native SSM API + type: query + file: aws_get_parameter.iql + skip_on_delete: true + props: + - name: lookup_names + value: + - "{{ app_config_name }}" + exports: + - app_config_version + - app_config_arn + + # --------------------------------------------------------------------------- + # Native API resource using createorupdate (REPLACE) and a FROM-less + # exports query + # --------------------------------------------------------------------------- + - name: feature_flags + description: feature flag parameter managed with the native PutParameter API + file: aws_parameter.iql + props: + - name: param_name + value: "{{ param_prefix }}/feature-flags" + - name: param_value + value: "beta=on,dark_mode=off" + exports: + - feature_flags_name + + # --------------------------------------------------------------------------- + # Conditions: one query runs, one is skipped + # --------------------------------------------------------------------------- + - name: prefix_summary + description: number of parameters created by this run + type: query + if: "'{{ stack_env }}' != 'prd'" + props: [] + sql: | + SELECT COUNT(*) AS param_count + FROM awscc.ssm.parameters_list_only + WHERE region = '{{ region }}' + AND INSTR(name, '{{ param_prefix }}/') = 1 + exports: + - param_count + + - name: prd_only_check + description: never runs outside prd, exercises the skip path of `if` + type: query + if: "'{{ stack_env }}' == 'prd'" + props: [] + sql: | + SELECT COUNT(*) AS prd_param_count + FROM awscc.ssm.parameters_list_only + WHERE region = '{{ region }}' + exports: + - prd_param_count + + # --------------------------------------------------------------------------- + # Command resource: a mutation with no lifecycle of its own (tags the + # parameter above; the tags disappear with the parameter on teardown), + # followed by a query that reads the result back + # --------------------------------------------------------------------------- + - name: tag_feature_flags + description: tag the feature flag parameter with the run that deployed it + type: command + props: + - name: resource_tags + value: + - Key: stackql:deployed-by + Value: "stackql-deploy-live-{{ run_id }}" + sql: | + UPDATE aws.ssm.tags + SET Tags = '{{ resource_tags }}' + WHERE ResourceType = 'Parameter' + AND ResourceId = '{{ feature_flags_name }}' + AND region = '{{ region }}' + + - name: feature_flags_tags + description: read back the tag applied by the command above + type: query + props: [] + sql: | + SELECT JSON_EXTRACT(tag_list, '$[0].Value') AS feature_flags_deployed_by + FROM aws.ssm.tags + WHERE ResourceType = 'Parameter' + AND ResourceId = '{{ feature_flags_name }}' + AND region = '{{ region }}' + exports: + - feature_flags_deployed_by + + # --------------------------------------------------------------------------- + # Script resource: external command exporting JSON + # --------------------------------------------------------------------------- + - name: deploy_marker + description: records which run deployed the stack + type: script + run: 'echo "{\"deployed_by\": \"stackql-deploy-live-{{ run_id }}\"}"' + exports: + - deployed_by + +exports: + - app_config_name + - app_config_version + - app_config_arn + - feature_flags_name + - feature_flags_deployed_by + - param_count + - deployed_by diff --git a/tests/live_stacks/aws_ssm_onfailure/resources/ghost.iql b/tests/live_stacks/aws_ssm_onfailure/resources/ghost.iql new file mode 100644 index 0000000..86ffa01 --- /dev/null +++ b/tests/live_stacks/aws_ssm_onfailure/resources/ghost.iql @@ -0,0 +1,10 @@ +/*+ exists */ +SELECT 1 AS count + +/*+ create */ +SELECT 1 AS count + +/*+ delete, retries=1, retry_delay=0 */ +DELETE FROM aws.s3.buckets +WHERE bucket = '{{ bucket_name }}' +AND region = '{{ region }}' diff --git a/tests/live_stacks/aws_ssm_onfailure/resources/ghost_parameter.iql b/tests/live_stacks/aws_ssm_onfailure/resources/ghost_parameter.iql new file mode 100644 index 0000000..d8a0922 --- /dev/null +++ b/tests/live_stacks/aws_ssm_onfailure/resources/ghost_parameter.iql @@ -0,0 +1,17 @@ +/*+ exists */ +SELECT 1 AS count + +/*+ create */ +SELECT 1 AS count + +/*+ delete, retries=1, retry_delay=0 */ +DELETE FROM awscc.ssm.parameters +WHERE Identifier = '{{ param_name }}' +AND region = '{{ region }}' +RETURNING * + +/*+ troubleshoot:delete */ +SELECT OperationStatus, ErrorCode, StatusMessage +FROM awscc.cloud_control.resource_request +WHERE region = '{{ region }}' +AND RequestToken = '{{ this.RequestToken }}' diff --git a/tests/live_stacks/aws_ssm_onfailure/stackql_manifest.yml b/tests/live_stacks/aws_ssm_onfailure/stackql_manifest.yml new file mode 100644 index 0000000..110235d --- /dev/null +++ b/tests/live_stacks/aws_ssm_onfailure/stackql_manifest.yml @@ -0,0 +1,49 @@ +version: 1 +name: sqld-live-onfailure +description: | + Live integration stack whose resources always fail to delete; nothing is + ever created. Each exists check is a literal that reports the resource as + present, so teardown always attempts the delete: + + ghost S3 DeleteBucket on a bucket that never existed. S3 + rejects the call synchronously, which is what + --on-failure decides about: error aborts the run, ignore + logs the failure and carries on. + ghost_parameter Cloud Control DeleteResource on a parameter that never + existed. Cloud Control accepts the request and fails it + asynchronously, so the delete statement itself succeeds; + the post-delete check then fails and the troubleshoot + anchor surfaces the ProgressEvent error (NotFound) using + the RequestToken captured from RETURNING * via + return_vals.delete. + + Resources are torn down in reverse manifest order, so ghost_parameter is + processed first. + + Inputs (pass with -e): AWS_REGION, RUN_ID. +providers: + - aws::v26.08.00444 + - awscc::v26.07.00426 + +globals: + - name: region + value: "{{ AWS_REGION }}" + - name: run_id + value: "{{ RUN_ID }}" + +resources: + - name: ghost + description: reports itself as existing; S3 rejects its delete synchronously + props: + - name: bucket_name + value: "stackql-deploy-{{ stack_env }}-{{ run_id }}-ghost-does-not-exist" + + - name: ghost_parameter + description: reports itself as existing; Cloud Control fails its delete asynchronously + props: + - name: param_name + value: "/stackql-deploy/{{ stack_env }}/{{ run_id }}/ghost-does-not-exist" + return_vals: + delete: + - RequestToken + - OperationStatus diff --git a/tests/live_stacks/github_labels/resources/label.iql b/tests/live_stacks/github_labels/resources/label.iql new file mode 100644 index 0000000..a765295 --- /dev/null +++ b/tests/live_stacks/github_labels/resources/label.iql @@ -0,0 +1,57 @@ +/*+ exists */ +SELECT COUNT(*) AS count +FROM github.issues.labels +WHERE owner = '{{ owner }}' +AND repo = '{{ repo }}' +AND name = '{{ label_name }}' + +/*+ create */ +INSERT INTO github.issues.labels ( + owner, + repo, + name, + color, + description +) +SELECT + '{{ owner }}', + '{{ repo }}', + '{{ label_name }}', + '{{ label_color }}', + '{{ label_description }}' + +/*+ statecheck, retries=3, retry_delay=3 */ +SELECT COUNT(*) AS count FROM +( + SELECT name, color, description + FROM github.issues.labels + WHERE owner = '{{ owner }}' + AND repo = '{{ repo }}' +) t +WHERE name = '{{ label_name }}' +AND color = '{{ label_color }}' +AND description = '{{ label_description }}' + +/*+ update */ +UPDATE github.issues.labels +SET color = '{{ label_color }}', + description = '{{ label_description }}' +WHERE owner = '{{ owner }}' +AND repo = '{{ repo }}' +AND name = '{{ label_name }}' + +/*+ exports */ +SELECT id AS run_label_id, url AS run_label_url FROM +( + SELECT id, url, name + FROM github.issues.labels + WHERE owner = '{{ owner }}' + AND repo = '{{ repo }}' +) t +WHERE name = '{{ label_name }}' + +/*+ delete */ +DELETE FROM github.issues.labels +WHERE owner = '{{ owner }}' +AND repo = '{{ repo }}' +AND name = '{{ label_name }}' diff --git a/tests/live_stacks/github_labels/stackql_manifest.yml b/tests/live_stacks/github_labels/stackql_manifest.yml new file mode 100644 index 0000000..718ed05 --- /dev/null +++ b/tests/live_stacks/github_labels/stackql_manifest.yml @@ -0,0 +1,63 @@ +version: 1 +name: sqld-live-github +description: | + Live integration stack for stackql-deploy built on GitHub repository labels + (free, and removed again on teardown). Exercises the plain REST provider + path - INSERT / UPDATE / DELETE without RETURNING, a 404-as-not-found + exists check, and an exists -> statecheck -> update cycle driven by an + input that the test harness changes between builds. + + Inputs (pass with -e): GITHUB_OWNER, GITHUB_REPO, RUN_ID, LABEL_COLOR. + Credentials: STACKQL_GITHUB_USERNAME / STACKQL_GITHUB_PASSWORD (a token + with issues:write on the repository). +providers: + - github::v26.05.00393 + +globals: + - name: owner + value: "{{ GITHUB_OWNER }}" + - name: repo + value: "{{ GITHUB_REPO }}" + - name: run_id + description: unique suffix per run (for example the CI run id) + value: "{{ RUN_ID }}" + - name: label_color + description: changed between builds by the test harness to drive the update path + value: "{{ LABEL_COLOR }}" + +resources: + + - name: run_label + description: a temporary label identifying this run + file: label.iql + props: + - name: label_name + value: "stackql-deploy-live-{{ run_id }}" + - name: label_color + value: "{{ label_color }}" + - name: label_description + values: + ci: + value: "temporary label created by the stackql-deploy live test suite" + dev: + value: "temporary label created by a stackql-deploy developer run" + exports: + - run_label_id + - run_label_url + + - name: repo_label_count + description: total labels on the repository, including the one above + type: query + props: [] + sql: | + SELECT COUNT(*) AS label_count + FROM github.issues.labels + WHERE owner = '{{ owner }}' + AND repo = '{{ repo }}' + exports: + - label_count + +exports: + - run_label_id + - run_label_url + - label_count diff --git a/tests/teardown.rs b/tests/teardown.rs new file mode 100644 index 0000000..32ae90a --- /dev/null +++ b/tests/teardown.rs @@ -0,0 +1,307 @@ +//! Integration tests for the `teardown` flow. +//! +//! Each test drives `run_teardown` against a fixture stack and an in-process +//! mock stackql server, then asserts on the exact statements that were sent +//! and on the exports left in the runner's global context. +//! +//! Background (issue #56): tearing down a partially deleted stack used to +//! interpolate the `` export placeholder into downstream queries, +//! e.g. `https://.cloud.databricks.com/...`, which failed with a +//! fatal `dial tcp` error and aborted the whole teardown. + +mod common; + +use common::{ + assert_no_unknown_placeholder_sent, export_value, FakeCloud, MockServer, TestStack, UNKNOWN, +}; +use stackql_deploy::commands::common_args::FailureAction; +use stackql_deploy::commands::teardown::run_teardown; + +const PROVIDERS: &[&str] = &[ + "databricks_account::v26.07.00430", + "databricks_workspace::v26.07.00430", +]; +const WORKSPACES: &str = "databricks_account.provisioning.workspaces"; +const CURRENT_USER: &str = "databricks_workspace.iam.current_user"; +const STORAGE_CREDENTIALS: &str = "databricks_workspace.catalog.storage_credentials"; + +/// A fully deployed estate: every table present with realistic values. +fn healthy_cloud() -> FakeCloud { + FakeCloud::new(PROVIDERS) + .table_present( + WORKSPACES, + &[ + ("workspace_id", "7474649669073866"), + ("deployment_name", "dbc-d586e37e-0269"), + ("workspace_status", "RUNNING"), + ], + ) + .table_present(CURRENT_USER, &[("userName", "cicd-sp@example.com")]) + .table_present(STORAGE_CREDENTIALS, &[("id", "07abf775-f0de-4749")]) +} + +#[test] +fn teardown_skips_queries_that_depend_on_a_deleted_upstream_resource() { + // The workspace was deleted by an earlier, partially successful teardown. + // Everything that needs its deployment_name must be skipped, not run. + let cloud = FakeCloud::new(PROVIDERS) + .table_absent(WORKSPACES, &[]) + .table_present(CURRENT_USER, &[("userName", "cicd-sp@example.com")]) + .table_present(STORAGE_CREDENTIALS, &[("id", "07abf775-f0de-4749")]); + let server = MockServer::start(cloud.into_handler()); + let stack = TestStack::new("databricks_workspace"); + let mut runner = stack.runner(&server, "dev"); + + run_teardown(&mut runner, false, false, FailureAction::Error); + + assert_no_unknown_placeholder_sent(&server); + + // The run did real work: it checked whether the workspace still exists. + assert!(server.received(WORKSPACES)); + + // Nothing that interpolates deployment_name was sent to the provider. + assert!( + !server.received(CURRENT_USER), + "workspace_ready query must not run when deployment_name is unknown" + ); + assert!( + !server.received(STORAGE_CREDENTIALS), + "storage_credential exists/delete must not run when deployment_name is unknown" + ); + + // Exports that could not be collected are marked unknown, not missing. + assert_eq!( + export_value(&runner, "deployment_name").as_deref(), + Some(UNKNOWN) + ); + assert_eq!( + export_value(&runner, "workspace_id").as_deref(), + Some(UNKNOWN) + ); + assert_eq!( + export_value(&runner, "workspace_principal").as_deref(), + Some(UNKNOWN) + ); + assert_eq!( + export_value(&runner, "storage_credential_id").as_deref(), + Some(UNKNOWN) + ); + assert_eq!( + export_value(&runner, "workspace_status").as_deref(), + Some(UNKNOWN) + ); +} + +#[test] +fn teardown_deletes_everything_when_the_estate_is_healthy() { + // Control case for the test above: with all exports collectable, every + // dependent query runs with real values and every resource is deleted. + let server = MockServer::start(healthy_cloud().into_handler()); + let stack = TestStack::new("databricks_workspace"); + let mut runner = stack.runner(&server, "dev"); + + run_teardown(&mut runner, false, false, FailureAction::Error); + + assert_no_unknown_placeholder_sent(&server); + + assert_eq!( + export_value(&runner, "deployment_name").as_deref(), + Some("dbc-d586e37e-0269") + ); + assert_eq!( + export_value(&runner, "workspace_principal").as_deref(), + Some("cicd-sp@example.com") + ); + assert_eq!( + export_value(&runner, "workspace_status").as_deref(), + Some("RUNNING") + ); + + let deletes = server.queries_containing("DELETE FROM"); + assert_eq!(deletes.len(), 2, "expected two deletes, got: {:?}", deletes); + // Reverse manifest order: the workspace-level resource goes first. + assert!(deletes[0].contains(STORAGE_CREDENTIALS)); + assert!(deletes[0].contains("deployment_name = 'dbc-d586e37e-0269'")); + assert!(deletes[1].contains(WORKSPACES)); + assert!(deletes[1].contains("workspace_id = '7474649669073866'")); +} + +#[test] +fn skip_on_delete_query_is_not_executed_during_teardown() { + let server = MockServer::start(healthy_cloud().into_handler()); + let stack = TestStack::with_manifest("databricks_workspace", "manifest_skip_query.yml"); + let mut runner = stack.runner(&server, "dev"); + + run_teardown(&mut runner, false, false, FailureAction::Error); + + assert_no_unknown_placeholder_sent(&server); + + // The flagged query never reaches the provider, even though it would + // have succeeded, and its export is reported as unknown. + assert!( + !server.received(CURRENT_USER), + "skip_on_delete query must not be executed during teardown" + ); + assert_eq!( + export_value(&runner, "workspace_principal").as_deref(), + Some(UNKNOWN) + ); + + // Everything else still tears down normally with real values. + let deletes = server.queries_containing("DELETE FROM"); + assert_eq!(deletes.len(), 2, "expected two deletes, got: {:?}", deletes); + assert!(deletes[0].contains(STORAGE_CREDENTIALS)); + assert!(deletes[1].contains(WORKSPACES)); + assert_eq!( + export_value(&runner, "workspace_status").as_deref(), + Some("RUNNING") + ); +} + +#[test] +fn skip_on_delete_resource_is_retained_but_its_exports_still_flow_downstream() { + let server = MockServer::start(healthy_cloud().into_handler()); + let stack = TestStack::with_manifest("databricks_workspace", "manifest_skip_resource.yml"); + let mut runner = stack.runner(&server, "dev"); + + run_teardown(&mut runner, false, false, FailureAction::Error); + + assert_no_unknown_placeholder_sent(&server); + + // The retained resource is never deleted... + let deletes = server.queries_containing("DELETE FROM"); + assert!( + deletes.iter().all(|q| !q.contains(WORKSPACES)), + "workspace must not be deleted, got: {:?}", + deletes + ); + // ...but its exports were collected and used by dependants. + assert_eq!( + export_value(&runner, "deployment_name").as_deref(), + Some("dbc-d586e37e-0269") + ); + assert!(server.received(CURRENT_USER)); + assert_eq!(deletes.len(), 1, "expected one delete, got: {:?}", deletes); + assert!(deletes[0].contains(STORAGE_CREDENTIALS)); + assert!(deletes[0].contains("deployment_name = 'dbc-d586e37e-0269'")); +} + +#[test] +fn teardown_tolerates_a_failing_exports_query() { + // A non-fatal provider error on an exports query (e.g. the workspace API + // rejecting the call while the workspace is being deleted) must not abort + // the teardown; the export is marked unknown and dependants are skipped. + let cloud = healthy_cloud().error_on( + CURRENT_USER, + "Query execution failed: query returns error: http response status code: 400, \ + response body: {\"error_code\":\"BAD_REQUEST\"}", + ); + let server = MockServer::start(cloud.into_handler()); + let stack = TestStack::new("databricks_workspace"); + let mut runner = stack.runner(&server, "dev"); + + run_teardown(&mut runner, false, false, FailureAction::Error); + + assert_no_unknown_placeholder_sent(&server); + assert!(server.received(CURRENT_USER)); + assert_eq!( + export_value(&runner, "workspace_principal").as_deref(), + Some(UNKNOWN) + ); + + let deletes = server.queries_containing("DELETE FROM"); + assert_eq!(deletes.len(), 2, "expected two deletes, got: {:?}", deletes); +} + +#[test] +fn on_failure_ignore_continues_past_a_failed_delete() { + // The provider rejects the storage credential delete (a non-fatal 409). + // With --on-failure ignore the run logs it, reports the resource as not + // confirmed deleted, and still tears down the workspace. With the + // default (error) the same failure aborts the process, which is why only + // the ignore path is covered in-process; the live suite covers the exit + // code of the error path. + let cloud = healthy_cloud().error_on( + "DELETE FROM databricks_workspace.catalog.storage_credentials", + "Query execution failed: query returns error: http response status code: 409, response body: {\"error_code\":\"RESOURCE_CONFLICT\"}", + ); + let server = MockServer::start(cloud.into_handler()); + let stack = TestStack::new("databricks_workspace"); + let mut runner = stack.runner(&server, "dev"); + + run_teardown(&mut runner, false, false, FailureAction::Ignore); + + assert_no_unknown_placeholder_sent(&server); + let deletes = server.queries_containing("DELETE FROM"); + assert!( + deletes.iter().any(|q| q.contains(STORAGE_CREDENTIALS)), + "the failing delete must have been attempted" + ); + assert!( + deletes.iter().any(|q| q.contains(WORKSPACES)), + "teardown must continue to the workspace after the ignored failure, got: {:?}", + deletes + ); +} + +#[test] +fn teardown_does_not_abort_on_script_resources() { + // Scripts have no .iql file; collecting their exports used to try to + // load one and exit. They are simply marked unknown. + let server = MockServer::start(healthy_cloud().into_handler()); + let stack = TestStack::with_manifest("databricks_workspace", "manifest_with_script.yml"); + let mut runner = stack.runner(&server, "dev"); + + run_teardown(&mut runner, false, false, FailureAction::Error); + + assert_no_unknown_placeholder_sent(&server); + assert_eq!( + export_value(&runner, "hook_result").as_deref(), + Some(UNKNOWN) + ); + let deletes = server.queries_containing("DELETE FROM"); + assert_eq!(deletes.len(), 2, "expected two deletes, got: {:?}", deletes); +} + +#[test] +fn delete_callback_is_skipped_when_no_returning_row_was_captured() { + // The mock answers DELETE with a bare command tag, so no RETURNING row + // exists to poll. Both a dry run and a real run must skip the callback + // rather than fail to render `callback.*` variables. + let stack = + TestStack::with_manifest("databricks_workspace", "manifest_with_delete_callback.yml"); + + let server = MockServer::start(healthy_cloud().into_handler()); + let mut runner = stack.runner(&server, "dev"); + run_teardown(&mut runner, true, false, FailureAction::Error); + assert!(!server.received("workspace_status = 'DELETED'")); + + let server = MockServer::start(healthy_cloud().into_handler()); + let mut runner = stack.runner(&server, "dev"); + run_teardown(&mut runner, false, false, FailureAction::Error); + assert!(server.received("DELETE FROM databricks_account.provisioning.workspaces")); + assert!(!server.received("workspace_status = 'DELETED'")); +} + +#[test] +fn dry_run_teardown_sends_no_mutations() { + let server = MockServer::start(healthy_cloud().into_handler()); + let stack = TestStack::new("databricks_workspace"); + let mut runner = stack.runner(&server, "dev"); + + run_teardown(&mut runner, true, false, FailureAction::Error); + + let mutations: Vec = server + .queries() + .into_iter() + .filter(|q| { + let u = q.trim_start().to_ascii_uppercase(); + u.starts_with("DELETE") || u.starts_with("INSERT") || u.starts_with("UPDATE") + }) + .collect(); + assert!( + mutations.is_empty(), + "dry run sent mutations: {:?}", + mutations + ); +} diff --git a/tests/test_command.rs b/tests/test_command.rs new file mode 100644 index 0000000..6fcf2b8 --- /dev/null +++ b/tests/test_command.rs @@ -0,0 +1,102 @@ +//! Integration tests for the `test` command against the mock server. + +mod common; + +use common::{export_value, FakeCloud, MockServer, TestStack}; +use stackql_deploy::commands::build::run_build; +use stackql_deploy::commands::test::run_test; + +const PROVIDERS: &[&str] = &[ + "databricks_account::v26.07.00430", + "databricks_workspace::v26.07.00430", +]; +const WORKSPACES: &str = "databricks_account.provisioning.workspaces"; +const CURRENT_USER: &str = "databricks_workspace.iam.current_user"; +const STORAGE_CREDENTIALS: &str = "databricks_workspace.catalog.storage_credentials"; + +fn healthy_cloud() -> FakeCloud { + FakeCloud::new(PROVIDERS) + .table_present( + WORKSPACES, + &[ + ("workspace_id", "7474649669073866"), + ("deployment_name", "dbc-d586e37e-0269"), + ("workspace_status", "RUNNING"), + ], + ) + .table_present(CURRENT_USER, &[("userName", "cicd-sp@example.com")]) + .table_present(STORAGE_CREDENTIALS, &[("id", "07abf775-f0de-4749")]) +} + +#[test] +fn test_command_validates_a_healthy_estate_without_mutations() { + let server = MockServer::start(healthy_cloud().into_handler()); + let stack = TestStack::new("databricks_workspace"); + let mut runner = stack.runner(&server, "dev"); + + run_test(&mut runner, false, false, "Error", None); + + let mutations: Vec = server + .queries() + .into_iter() + .filter(|q| { + let u = q.trim_start().to_ascii_uppercase(); + u.starts_with("INSERT") || u.starts_with("UPDATE") || u.starts_with("DELETE") + }) + .collect(); + assert!(mutations.is_empty(), "test sent mutations: {:?}", mutations); + assert_eq!( + export_value(&runner, "workspace_principal").as_deref(), + Some("cicd-sp@example.com") + ); + assert_eq!( + export_value(&runner, "workspace_status").as_deref(), + Some("RUNNING") + ); +} + +#[test] +fn test_command_honours_if_conditions() { + // estate_summary is gated on stack_env == prd; in dev it must not run. + let server = MockServer::start(healthy_cloud().into_handler()); + let stack = TestStack::with_manifest("databricks_workspace", "manifest_with_condition.yml"); + let mut runner = stack.runner(&server, "dev"); + + run_test(&mut runner, false, false, "Error", None); + + assert!( + !server.received("SELECT workspace_status"), + "conditional query must not run when its condition is false" + ); + assert!(export_value(&runner, "workspace_status").is_none()); +} + +#[test] +fn build_honours_if_conditions() { + let server = MockServer::start(healthy_cloud().into_handler()); + let stack = TestStack::with_manifest("databricks_workspace", "manifest_with_condition.yml"); + let mut runner = stack.runner(&server, "dev"); + + run_build(&mut runner, false, false, "Error", None); + + assert!(!server.received("SELECT workspace_status")); + assert!(export_value(&runner, "workspace_status").is_none()); +} + +/// Scripts run through `sh -c`, which is not on PATH in a default Windows +/// shell; the live suite covers scripts on Linux CI. +#[cfg(unix)] +#[test] +fn test_command_runs_script_resources_and_exports_their_output() { + let server = MockServer::start(healthy_cloud().into_handler()); + let stack = TestStack::with_manifest("databricks_workspace", "manifest_with_script.yml"); + let mut runner = stack.runner(&server, "dev"); + + run_test(&mut runner, false, false, "Error", None); + + assert_eq!(export_value(&runner, "hook_result").as_deref(), Some("ok")); + assert_ne!( + export_value(&runner, "hook_result").as_deref(), + Some(UNKNOWN) + ); +} diff --git a/website/docs/cli-reference/teardown.md b/website/docs/cli-reference/teardown.md index 14bcb2b..579c28a 100644 --- a/website/docs/cli-reference/teardown.md +++ b/website/docs/cli-reference/teardown.md @@ -52,6 +52,7 @@ Command used to deprovision and remove resources in a specified stack in a given | `-e` `--env` | Set additional environment variables (can be used multiple times) | `--env DB_USER=admin` | | `--dry-run` | Perform a dry run of the operation. No changes will be made | | | `--show-queries` | Display the queries executed in the output logs | | +| `--on-failure` | What to do when a `delete` statement is rejected by the provider: `error` (default) aborts the run at the first failure, `ignore` logs the failure, reports the resource as not confirmed deleted, and continues with the next resource | `--on-failure ignore` | :::tip @@ -59,6 +60,14 @@ Exported variables specified as `protected` in the respective resource definitio ::: +:::info + +With `--on-failure ignore`, fatal errors (network, authentication, and stackql planner errors) still abort the run. A delete that is dispatched but cannot be confirmed within its retry budget never aborts the run on its own; resources whose delete could not be confirmed are listed in a summary at the end of the teardown, and the exit status is still `0`. + +Resources that are skipped during teardown - because they carry `skip_on_delete: true`, or because a query they depend on could not be rendered with real values - are logged and left in place. + +::: + * * * ## Examples diff --git a/website/docs/manifest-file.md b/website/docs/manifest-file.md index 2af4b9f..9afa73e 100644 --- a/website/docs/manifest-file.md +++ b/website/docs/manifest-file.md @@ -141,6 +141,12 @@ the fields within the __`stackql_manifest.yml`__ file are described in further d *** +### `resource.skip_on_delete` + + + +*** + ### `resource.callback` There is no `callback` section in the manifest schema. Callback behaviour ā€” including what to poll, retry counts, retry delays, and short-circuit conditions ā€” is configured entirely within the resource's `.iql` file using the `/*+ callback */` anchor. This prevents confusion for users who might expect a manifest entry by analogy with `props` and `exports`. diff --git a/website/docs/manifest_fields/index.js b/website/docs/manifest_fields/index.js index 45a3d99..18b36b1 100644 --- a/website/docs/manifest_fields/index.js +++ b/website/docs/manifest_fields/index.js @@ -18,6 +18,7 @@ export { default as ResourceAuth } from "./resources/auth.mdx"; export { default as ResourceIf } from "./resources/if.mdx"; export { default as ResourceSql } from "./resources/sql.mdx"; export { default as ResourceSkipValidation } from "./resources/skipvalidation.mdx"; +export { default as ResourceSkipOnDelete } from "./resources/skipondelete.mdx"; export { default as ResourcePropName } from "./resources/props/name.mdx"; export { default as ResourcePropDescription } from "./resources/props/description.mdx"; export { default as ResourcePropValue } from "./resources/props/value.mdx"; diff --git a/website/docs/manifest_fields/resources/skipondelete.mdx b/website/docs/manifest_fields/resources/skipondelete.mdx new file mode 100644 index 0000000..00dec23 --- /dev/null +++ b/website/docs/manifest_fields/resources/skipondelete.mdx @@ -0,0 +1,35 @@ +import File from '@site/src/components/File'; +import LeftAlignedTable from '@site/src/components/LeftAlignedTable'; + + + +When set to `true`, the resource is not processed by the `teardown` command. It has no effect on `build` or `test`. + +What "not processed" means depends on the resource type: + +- **`query`** - the query is not executed during teardown. Each of its declared `exports` is set to the `` placeholder, and any downstream query that would interpolate one of those exports is skipped rather than run. +- **`resource`** and **`multi`** - the resource's `exports` are still collected (a downstream `delete` may need them), but its own `delete` query is not executed, so the resource is retained. + + + +```yaml {5} +resources: + - name: workspace_ready + description: wait until the new workspace answers the workspace API as the deploying principal + type: query + skip_on_delete: true + file: databricks_workspace/current_user.iql + props: [] + exports: + - workspace_principal +``` + + + +:::info + +- Use `skip_on_delete: true` on a `query` resource whose only purpose is to gate a `build` (readiness polls, post-deploy assertions, summaries) and which has nothing to contribute to a teardown. This avoids running a query against a resource that is about to be, or already has been, deleted. +- Use `skip_on_delete: true` on a `resource` to retain it on teardown, for example a storage bucket that holds data, or a shared resource created out of band and only asserted by this stack. +- During teardown, any `exists`, `statecheck`, `exports`, `delete`, or inline `sql` query whose rendered text contains `` is skipped automatically, whether the placeholder came from `skip_on_delete` or from an upstream resource that no longer exists. `skip_on_delete` is therefore never required for a teardown to succeed; it is an explicit way to say a query should not run at all. + +::: From d12e43db6ee3ce39352cf224a71186e40aafa617 Mon Sep 17 00:00:00 2001 From: Jeffrey Aven Date: Thu, 10 Sep 2026 12:21:20 +1000 Subject: [PATCH 2/2] Honour postdelete_retries, run delete callback before post-delete check, fix CI - The exists anchor's postdelete_retries / postdelete_retry_delay were parsed but never used; the post-delete check ran twice within zero seconds, so an asynchronous delete (Cloud Control) was only confirmed by luck of timing. The check now polls per those options after each delete attempt. - callback:delete now runs before the post-delete check, not after it. - delete_and_confirm split into execute_delete + confirm_deleted. - Mock provider: RETURNING rows on DML, asynchronous deletes, canned answers; tests for callback ordering, postdelete polling and unconfirmed deletes. - test_command: drop a reference only compiled on unix (CI compile error). - Give the on-failure ghosts a short postdelete budget so Cloud Control has time to record the failure the troubleshoot anchor reports. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 2 + docs/flows.md | 2 + src/commands/base.rs | 283 ++++++------------ src/commands/teardown.rs | 253 +++++++++------- tests/common/fake_cloud.rs | 102 ++++++- tests/common/mod.rs | 2 +- .../resources/storage_credential.iql | 2 +- .../resources/workspace.iql | 2 +- .../resources/workspace_with_callback.iql | 2 +- .../aws_ssm_onfailure/resources/ghost.iql | 2 +- .../resources/ghost_parameter.iql | 2 +- tests/teardown.rs | 95 +++++- tests/test_command.rs | 4 - website/docs/resource-query-files.md | 4 +- 14 files changed, 437 insertions(+), 320 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e6b1a8..03616fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ - The `` and `` export placeholders are no longer registered for log redaction when the export is `protected`. Previously a protected export in a dry run registered `` as a secret, which then masked every other placeholder in the run as `********`. - Command failures that are ignored (`multi` resources, and now `--on-failure ignore`) are logged at `warn` level instead of `debug`. - A `callback:delete` (or generic `callback`) anchor no longer aborts a `--dry-run` teardown, or a teardown whose delete returned no `RETURNING *` row. Callbacks poll the handle returned by `RETURNING *`, so they are now skipped with a log line when there is nothing to poll, matching `build`. +- The `postdelete_retries` and `postdelete_retry_delay` options on the `exists` anchor are now honoured, as documented (defaults 10 and 5). They were parsed but never used: the post-delete check ran once immediately and once more after the `delete` anchor's `retry_delay`, which defaults to 0, so an asynchronous delete (Cloud Control, most SaaS APIs) could only be confirmed by luck of timing. After each delete attempt the exists query now polls until the resource is gone, up to `postdelete_retries` times, `postdelete_retry_delay` seconds apart; only then is the delete re-issued, up to the `delete` anchor's `retries`. +- The `callback:delete` anchor now runs before the post-delete check rather than after it, so a provider's asynchronous delete is polled to completion before the resource is checked for absence. ## 2.1.1 (2026-08-24) diff --git a/docs/flows.md b/docs/flows.md index b3881b0..9d287dd 100644 --- a/docs/flows.md +++ b/docs/flows.md @@ -198,6 +198,8 @@ During teardown export collection, exports that cannot be collected are set to ` Any teardown query (`exists`, `statecheck`, `exports`, `delete`, or inline `sql`) whose rendered text contains `` is skipped rather than executed. Running it would at best match nothing and at worst put the placeholder into a hostname or identifier (for example `https://.cloud.databricks.com/...`), which fails with a fatal `dial tcp` error and aborts the whole teardown. The resource is logged as skipped and processing continues with the next one. +After each `delete` (and its `callback:delete`, if any) the `exists` query is polled until the resource is gone, up to `postdelete_retries` times `postdelete_retry_delay` seconds apart (options on the `exists` anchor, defaults 10 and 5). Only then is the delete re-issued, up to the `delete` anchor's `retries`. + A `delete` statement the provider rejects aborts the teardown by default (`--on-failure error`). With `--on-failure ignore` the failure is logged, the resource is reported as not confirmed deleted, and the next resource is processed; fatal network, auth and planner errors abort in both modes. Every teardown ends with a summary of resources whose delete could not be confirmed. `skip_on_delete: true` opts a resource out of teardown explicitly: diff --git a/src/commands/base.rs b/src/commands/base.rs index 7fe37cf..df947cb 100644 --- a/src/commands/base.rs +++ b/src/commands/base.rs @@ -496,32 +496,20 @@ impl CommandRunner { } } - /// Delete a resource and confirm deletion with an interleaved - /// delete-check-retry loop. + /// Execute one `delete` statement. /// - /// When `delete_retries > 0` the loop is: - /// 1. Execute DELETE - /// 2. Run exists query ā€” count==0 ā†’ done, count==1 ā†’ continue, else ā†’ error - /// 3. Wait `delete_retry_delay` seconds - /// 4. Run exists query again ā€” count==0 ā†’ done, count==1 ā†’ re-delete - /// ... repeat up to `delete_retries` times - /// - /// When `delete_retries == 0`: single delete + single check, no retry. - /// - /// Returns the RETURNING * row (if any) from the first successful delete. - #[allow(clippy::too_many_arguments)] - pub fn delete_and_confirm( + /// Returns the `RETURNING *` row when the statement has a RETURNING + /// clause and the provider returned data, otherwise `None`. Retries are + /// the caller's business (see `run_teardown`), so the DML itself runs + /// once. In a dry run the statement is only logged. + pub fn execute_delete( &mut self, resource: &Resource, delete_query: &str, - exists_query: &str, - delete_retries: u32, - delete_retry_delay: u32, dry_run: bool, show_queries: bool, ignore_errors: bool, - ) -> (Option>, bool) { - // --- dry run path --- + ) -> Option> { if dry_run { if has_returning_clause(delete_query) { info!( @@ -534,190 +522,107 @@ impl CommandRunner { resource.name, delete_query ); } - return (None, true); + return None; } - let mut returning_row: Option> = None; - - // Helper closure: execute the DELETE statement once (no retries on the - // DML itself ā€” retries are handled by the outer loop). - let execute_delete = |client: &mut crate::utils::pgwire::PgwireLite, - query: &str, - res_name: &str, - sq: bool, - ignore: bool| { - info!("deleting [{}]...", res_name); - show_query(sq, query); - if has_returning_clause(query) { - let (msg, row) = run_stackql_dml_returning(query, client, ignore, 0, 0); - debug!("Delete response: {}", msg); - row - } else { - let msg = run_stackql_command(query, client, ignore, 0, 0); - debug!("Delete response: {}", msg); - None - } - }; - - // Helper closure: run the exists query and return the count. - // Returns Ok(count) or Err(msg) for unexpected results. - let run_exists_count = |client: &mut crate::utils::pgwire::PgwireLite, - query: &str, - res_name: &str, - sq: bool| - -> Result { - info!("running post-delete check for [{}]...", res_name); - show_query(sq, query); - let result = run_stackql_query(query, client, true, 0, 5); - if result.is_empty() { - return Ok(0); // no rows ā†’ resource gone - } - if result[0].contains_key("_stackql_deploy_error") || result[0].contains_key("error") { - return Ok(0); // error querying ā†’ treat as gone - } - if let Some(count_str) = result[0].get("count") { - if let Ok(count) = count_str.parse::() { - return Ok(count); - } - } - // No count field ā€” check if all field values are null/empty - // (resource gone) or any non-null value (resource still exists). - let row = &result[0]; - let all_null = row.values().all(|v| v == "null" || v.is_empty()); - if all_null { - Ok(0) // all null/empty ā†’ resource gone - } else { - Ok(1) // non-null value ā†’ resource still exists - } - }; - - // --- no-retry path: single delete + single check --- - if delete_retries == 0 { - let row = execute_delete( - &mut self.client, - delete_query, - &resource.name, - show_queries, - ignore_errors, - ); - if returning_row.is_none() { - returning_row = row; - } - match run_exists_count(&mut self.client, exists_query, &resource.name, show_queries) { - Ok(0) => { - info!("[{}] confirmed deleted", resource.name); - return (returning_row, true); - } - Ok(1) => { - info!( - "[{}] delete dispatched (resource may still be deleting asynchronously)", - resource.name - ); - return (returning_row, false); - } - Ok(n) => { - catch_error_and_exit(&format!( - "Post-delete exists query for [{}] returned count={} (expected 0 or 1). \ - This indicates a query or logic error.", - resource.name, n - )); - } - Err(msg) => { - catch_error_and_exit(&msg); - } - } + info!("deleting [{}]...", resource.name); + show_query(show_queries, delete_query); + if has_returning_clause(delete_query) { + let (msg, row) = + run_stackql_dml_returning(delete_query, &mut self.client, ignore_errors, 0, 0); + debug!("Delete response: {}", msg); + row + } else { + let msg = run_stackql_command(delete_query, &mut self.client, ignore_errors, 0, 0); + debug!("Delete response: {}", msg); + None } + } - // --- retry path: interleaved delete + check loop --- + /// Poll the exists query until the resource is gone. + /// + /// The first check runs immediately; if the resource is still present, + /// up to `postdelete_retries` further checks follow, each after + /// `postdelete_retry_delay` seconds (the `postdelete_*` options of the + /// `exists` anchor). Returns true once a check reports the resource gone. + /// A dry run reports success without querying. + pub fn confirm_deleted( + &mut self, + resource: &Resource, + exists_query: &str, + postdelete_retries: u32, + postdelete_retry_delay: u32, + dry_run: bool, + show_queries: bool, + ) -> bool { + if dry_run { + return true; + } let start = std::time::Instant::now(); - - for attempt in 0..delete_retries { - // Step 1: execute DELETE - let row = execute_delete( - &mut self.client, - delete_query, - &resource.name, - show_queries, - ignore_errors, - ); - if returning_row.is_none() { - returning_row = row; - } - - // Step 2: immediate post-delete check - match run_exists_count(&mut self.client, exists_query, &resource.name, show_queries) { - Ok(0) => { - info!("[{}] confirmed deleted", resource.name); - return (returning_row, true); - } - Ok(1) => { - let elapsed = start.elapsed().as_secs(); - info!( - "[{}] still exists after delete, attempt {}/{} ({} seconds elapsed)", - resource.name, - attempt + 1, - delete_retries, - elapsed - ); - } - Ok(n) => { - catch_error_and_exit(&format!( - "Post-delete exists query for [{}] returned count={} (expected 0 or 1). \ - This indicates a query or logic error.", - resource.name, n - )); - } - Err(msg) => { - catch_error_and_exit(&msg); - } - } - - // Step 3: wait retry_delay - if delete_retry_delay > 0 { + for check in 0..=postdelete_retries { + if check > 0 { info!( - "[{}] waiting {} seconds before next attempt...", - resource.name, delete_retry_delay + "[{}] still exists, waiting {} seconds before post-delete check {}/{} ({} seconds elapsed)", + resource.name, + postdelete_retry_delay, + check, + postdelete_retries, + start.elapsed().as_secs() ); - std::thread::sleep(std::time::Duration::from_secs(delete_retry_delay as u64)); + std::thread::sleep(std::time::Duration::from_secs( + postdelete_retry_delay as u64, + )); } - - // Step 4: check again after the delay (maybe it cleaned up) - match run_exists_count(&mut self.client, exists_query, &resource.name, show_queries) { - Ok(0) => { + match self.post_delete_count(resource, exists_query, show_queries) { + 0 => { info!("[{}] confirmed deleted", resource.name); - return (returning_row, true); - } - Ok(1) => { - let elapsed = start.elapsed().as_secs(); - info!( - "[{}] still exists after delay, attempt {}/{} ({} seconds elapsed), re-issuing delete...", - resource.name, - attempt + 1, - delete_retries, - elapsed - ); - // Loop continues ā†’ next iteration will re-issue DELETE - } - Ok(n) => { - catch_error_and_exit(&format!( - "Post-delete exists query for [{}] returned count={} (expected 0 or 1). \ - This indicates a query or logic error.", - resource.name, n - )); - } - Err(msg) => { - catch_error_and_exit(&msg); + return true; } + 1 => {} + n => catch_error_and_exit(&format!( + "Post-delete exists query for [{}] returned count={} (expected 0 or 1). \ + This indicates a query or logic error.", + resource.name, n + )), } } - - // Exhausted all retries - let elapsed = start.elapsed().as_secs(); info!( - "[{}] delete could not be confirmed after {} attempts ({} seconds elapsed)", - resource.name, delete_retries, elapsed + "[{}] still exists after {} post-delete check(s) ({} seconds elapsed)", + resource.name, + postdelete_retries + 1, + start.elapsed().as_secs() ); - (returning_row, false) + false + } + + /// Run the exists query once after a delete and reduce the result to a + /// count: 0 when the resource is gone (no rows, an error, a zero count, or + /// a row of nulls), 1 when it is still present, anything else verbatim. + fn post_delete_count( + &mut self, + resource: &Resource, + exists_query: &str, + show_queries: bool, + ) -> i64 { + info!("running post-delete check for [{}]...", resource.name); + show_query(show_queries, exists_query); + let result = run_stackql_query(exists_query, &mut self.client, true, 0, 5); + if result.is_empty() { + return 0; + } + let row = &result[0]; + if row.contains_key("_stackql_deploy_error") || row.contains_key("error") { + return 0; + } + if let Some(count) = row.get("count").and_then(|c| c.parse::().ok()) { + return count; + } + let all_null = row.values().all(|v| v == "null" || v.is_empty()); + if all_null { + 0 + } else { + 1 + } } // ----------------------------------------------------------------------- diff --git a/src/commands/teardown.rs b/src/commands/teardown.rs index 85a11da..4502c4c 100644 --- a/src/commands/teardown.rs +++ b/src/commands/teardown.rs @@ -365,42 +365,41 @@ pub fn run_teardown( let resource_queries = runner.get_queries(resource, &full_context); // Get exists query (fallback to statecheck) - render JIT - let (exists_query_str, exists_retries, exists_retry_delay) = - if let Some(eq) = resource_queries.get("exists") { - match render_for_teardown( - runner, - resource, - "exists", - &eq.template, - &full_context, - "assuming resource does not exist, skipping...", - ) { - Some(rendered) => (rendered, eq.options.retries, eq.options.retry_delay), - None => continue, - } - } else if let Some(sq) = resource_queries.get("statecheck") { - info!( - "exists query not defined for [{}], trying statecheck query as exists query.", - resource.name - ); - match render_for_teardown( - runner, - resource, - "statecheck", - &sq.template, - &full_context, - "skipping...", - ) { - Some(rendered) => (rendered, sq.options.retries, sq.options.retry_delay), - None => continue, - } - } else { - info!( - "No exists or statecheck query for [{}], skipping...", - resource.name - ); - continue; - }; + let (exists_query_str, exists_opts) = if let Some(eq) = resource_queries.get("exists") { + match render_for_teardown( + runner, + resource, + "exists", + &eq.template, + &full_context, + "assuming resource does not exist, skipping...", + ) { + Some(rendered) => (rendered, eq.options.clone()), + None => continue, + } + } else if let Some(sq) = resource_queries.get("statecheck") { + info!( + "exists query not defined for [{}], trying statecheck query as exists query.", + resource.name + ); + match render_for_teardown( + runner, + resource, + "statecheck", + &sq.template, + &full_context, + "skipping...", + ) { + Some(rendered) => (rendered, sq.options.clone()), + None => continue, + } + } else { + info!( + "No exists or statecheck query for [{}], skipping...", + resource.name + ); + continue; + }; // Check if delete query template exists (don't render yet ā€” may need // this.* fields from the exists check). @@ -422,8 +421,8 @@ pub fn run_teardown( let (exists, fields) = runner.check_if_resource_exists( resource, &exists_query_str, - exists_retries, - exists_retry_delay, + exists_opts.retries, + exists_opts.retry_delay, dry_run, show_queries, false, @@ -464,7 +463,7 @@ pub fn run_teardown( Some(rendered) => rendered, None => continue, }; - let delete_retries = dq.options.retries; + let delete_retries = dq.options.retries.max(1); let delete_retry_delay = dq.options.retry_delay; // Only keep a RETURNING clause when return_vals.delete is configured @@ -491,24 +490,60 @@ pub fn run_teardown( rendered_delete }; - let (returning_row, delete_confirmed) = runner.delete_and_confirm( - resource, - &delete_query, - &exists_query_str, - delete_retries, - delete_retry_delay, - dry_run, - show_queries, - ignore_errors, - ); + // Callback anchor, if any: polled after each delete attempt that + // returned a RETURNING * row, before the post-delete check. + let cb_anchor = if resource_queries.contains_key("callback:delete") { + Some("callback:delete") + } else if resource_queries.contains_key("callback") { + Some("callback") + } else { + None + }; - // Capture RETURNING * result. - if let Some(ref row) = returning_row { - debug!("RETURNING payload for [{}]: {:?}", resource.name, row); - runner.store_callback_data(&resource.name, row); + // Delete, then confirm. Each attempt is: execute the delete, + // capture RETURNING * (return_vals + callback context), run the + // delete callback, then poll the exists query using the exists + // anchor's postdelete_retries / postdelete_retry_delay. If the + // resource is still present, wait retry_delay and re-issue the + // delete, up to the delete anchor's retries. + let mut delete_confirmed = false; + for attempt in 0..delete_retries { + if attempt > 0 { + if delete_retry_delay > 0 { + info!( + "[{}] waiting {} seconds before re-issuing delete (attempt {}/{})...", + resource.name, + delete_retry_delay, + attempt + 1, + delete_retries + ); + std::thread::sleep(std::time::Duration::from_secs( + delete_retry_delay as u64, + )); + } else { + info!( + "[{}] re-issuing delete (attempt {}/{})...", + resource.name, + attempt + 1, + delete_retries + ); + } + } + + let returning_row = runner.execute_delete( + resource, + &delete_query, + dry_run, + show_queries, + ignore_errors, + ); + + // Capture RETURNING * result. + if let Some(ref row) = returning_row { + debug!("RETURNING payload for [{}]: {:?}", resource.name, row); + runner.store_callback_data(&resource.name, row); - // Apply return_vals.delete mappings from manifest. - if !delete_return_mappings.is_empty() { + // Apply return_vals.delete mappings from manifest. for (src, tgt) in &delete_return_mappings { if let Some(val) = row.get(src.as_str()) { if !val.is_empty() && val != "null" { @@ -531,58 +566,70 @@ pub fn run_teardown( ); } } + } else if !delete_return_mappings.is_empty() && !dry_run { + warn!( + "return_vals.delete specified for [{}] but no RETURNING data received", + resource.name + ); } - } else if !delete_return_mappings.is_empty() { - warn!( - "return_vals.delete specified for [{}] but no RETURNING data received", - resource.name - ); - } - // Run callback:delete block if present. A callback polls the - // handle returned by RETURNING *, so there is nothing to poll - // when no row came back: a dry run, a delete without RETURNING, - // or a delete that failed and was ignored. - let cb_anchor = if resource_queries.contains_key("callback:delete") { - Some("callback:delete") - } else if resource_queries.contains_key("callback") { - Some("callback") - } else { - None - }; - if let Some(anchor) = cb_anchor { - if returning_row.is_none() { - info!( - "[{}] {} not run: the delete returned no RETURNING data{}", - resource.name, - anchor, - if dry_run { " (dry run)" } else { "" } - ); - } else if let Some(q) = resource_queries.get(anchor) { - let cb_template = q.template.clone(); - let cb_retries = q.options.retries; - let cb_delay = q.options.retry_delay; - let cb_sc_field = q.options.short_circuit_field.clone(); - let cb_sc_value = q.options.short_circuit_value.clone(); - let cb_ctx = runner.get_full_context(resource); - match runner.try_render_query(&resource.name, anchor, &cb_template, &cb_ctx) { - Some(rendered_cb) => runner.run_callback( - resource, - &rendered_cb, - cb_retries, - cb_delay, - cb_sc_field.as_deref(), - cb_sc_value.as_deref(), - "delete", - dry_run, - show_queries, - ), - None => warn!( - "[{}] {} has unresolved variables and was not run", - resource.name, anchor - ), + // Run the delete callback. A callback polls the handle + // returned by RETURNING *, so there is nothing to poll when + // no row came back: a dry run, a delete without RETURNING, or + // a delete that failed and was ignored. + if let Some(anchor) = cb_anchor { + if returning_row.is_none() { + info!( + "[{}] {} not run: the delete returned no RETURNING data{}", + resource.name, + anchor, + if dry_run { " (dry run)" } else { "" } + ); + } else if let Some(q) = resource_queries.get(anchor) { + let cb_template = q.template.clone(); + let cb_retries = q.options.retries; + let cb_delay = q.options.retry_delay; + let cb_sc_field = q.options.short_circuit_field.clone(); + let cb_sc_value = q.options.short_circuit_value.clone(); + let cb_ctx = runner.get_full_context(resource); + match runner.try_render_query(&resource.name, anchor, &cb_template, &cb_ctx) + { + Some(rendered_cb) => runner.run_callback( + resource, + &rendered_cb, + cb_retries, + cb_delay, + cb_sc_field.as_deref(), + cb_sc_value.as_deref(), + "delete", + dry_run, + show_queries, + ), + None => warn!( + "[{}] {} has unresolved variables and was not run", + resource.name, anchor + ), + } } } + + if runner.confirm_deleted( + resource, + &exists_query_str, + exists_opts.postdelete_retries, + exists_opts.postdelete_retry_delay, + dry_run, + show_queries, + ) { + delete_confirmed = true; + break; + } + } + if !delete_confirmed { + info!( + "[{}] delete could not be confirmed after {} attempt(s)", + resource.name, delete_retries + ); } if delete_confirmed { diff --git a/tests/common/fake_cloud.rs b/tests/common/fake_cloud.rs index b3f8eee..e4acd23 100644 --- a/tests/common/fake_cloud.rs +++ b/tests/common/fake_cloud.rs @@ -17,6 +17,12 @@ use super::mock_server::MockResponse; struct TableState { present: bool, row: HashMap, + /// When set, a DELETE does not remove the table immediately: it stays + /// present for this many further reads, mimicking a provider whose + /// delete completes asynchronously. + reads_until_gone: Option, + /// Countdown armed by a DELETE when `reads_until_gone` is set. + pending_reads: Option, } /// Builder for a fake provider backend. Consume it with @@ -28,6 +34,9 @@ pub struct FakeCloud { /// `(needle, message)`: any statement containing `needle` fails with /// `message` instead of being evaluated. errors: Vec<(String, String)>, + /// `(needle, response)`: any statement containing `needle` gets this + /// canned response instead of being evaluated (checked after `errors`). + answers: Vec<(String, MockResponse)>, } impl FakeCloud { @@ -54,6 +63,7 @@ impl FakeCloud { TableState { present: true, row: to_row(row), + ..Default::default() }, ); self @@ -67,6 +77,7 @@ impl FakeCloud { TableState { present: false, row: to_row(row_after_create), + ..Default::default() }, ); self @@ -78,6 +89,22 @@ impl FakeCloud { self } + /// Any statement containing `needle` receives `response` verbatim. + pub fn answer(mut self, needle: &str, response: MockResponse) -> Self { + self.answers.push((needle.to_string(), response)); + self + } + + /// A DELETE on `table` completes asynchronously: the table still reads as + /// present for `reads` further selects, then disappears. + pub fn async_delete(mut self, table: &str, reads: u32) -> Self { + self.tables + .entry(table.to_string()) + .or_default() + .reads_until_gone = Some(reads); + self + } + pub fn into_handler(mut self) -> impl FnMut(&str) -> MockResponse + Send + 'static { move |sql: &str| self.handle(sql) } @@ -88,9 +115,15 @@ impl FakeCloud { return MockResponse::Error(message.clone()); } } + for (needle, response) in &self.answers { + if sql.contains(needle.as_str()) { + return response.clone(); + } + } let trimmed = sql.trim().trim_end_matches(';').trim(); let upper = trimmed.to_ascii_uppercase(); + let returning = upper.split_whitespace().any(|word| word == "RETURNING"); if upper == "SHOW PROVIDERS" { return MockResponse::Rows { @@ -109,20 +142,50 @@ impl FakeCloud { return self.handle_select(trimmed); } if let Some(table) = capture(r"(?is)^\s*INSERT\s+INTO\s+([\w.]+)", trimmed) { - self.tables.entry(table).or_default().present = true; - return MockResponse::Command("INSERT 0 1".to_string()); + let state = self.tables.entry(table).or_default(); + state.present = true; + return dml_response(state, returning, "INSERT 0 1"); } if let Some(table) = capture(r"(?is)^\s*DELETE\s+FROM\s+([\w.]+)", trimmed) { - self.tables.entry(table).or_default().present = false; - return MockResponse::Command("DELETE 1".to_string()); + let state = self.tables.entry(table).or_default(); + eprintln!( + "FAKE DELETE returning={} row_keys={:?} sql={:?}", + returning, + state.row.keys().collect::>(), + trimmed + ); + let response = dml_response(state, returning, "DELETE 1"); + match state.reads_until_gone { + Some(reads) => state.pending_reads = Some(reads), + None => state.present = false, + } + return response; } - if capture(r"(?is)^\s*UPDATE\s+([\w.]+)", trimmed).is_some() { - return MockResponse::Command("UPDATE 1".to_string()); + if let Some(table) = capture(r"(?is)^\s*UPDATE\s+([\w.]+)", trimmed) { + let state = self.tables.entry(table).or_default(); + return dml_response(state, returning, "UPDATE 1"); } MockResponse::empty() } - fn handle_select(&self, sql: &str) -> MockResponse { + /// Advance an asynchronous delete by one read; returns whether the table + /// is present for this read. + fn observe(&mut self, table: &str) -> bool { + let Some(state) = self.tables.get_mut(table) else { + return false; + }; + if let Some(remaining) = state.pending_reads { + if remaining == 0 { + state.present = false; + state.pending_reads = None; + } else { + state.pending_reads = Some(remaining - 1); + } + } + state.present + } + + fn handle_select(&mut self, sql: &str) -> MockResponse { let table = capture(r"(?is)\bFROM\s+([\w.]+)", sql); let select_list = select_list(sql); let items: Vec<(String, String)> = split_top_level_commas(&select_list) @@ -130,12 +193,11 @@ impl FakeCloud { .map(|item| parse_select_item(&item)) .collect(); - let state = table.as_deref().and_then(|t| self.tables.get(t)); - let present = match (&table, state) { - (None, _) => true, // literal SELECT with no FROM - (Some(_), Some(s)) => s.present, - (Some(_), None) => false, // unknown table behaves as empty + let present = match table.as_deref() { + None => true, // literal SELECT with no FROM + Some(t) => self.observe(t), }; + let state = table.as_deref().and_then(|t| self.tables.get(t)); let columns: Vec = items.iter().map(|(_, alias)| alias.clone()).collect(); @@ -180,6 +242,22 @@ impl FakeCloud { } } +/// Response to a DML statement: the table's row when `RETURNING` was +/// requested (as a real provider would return the affected object), otherwise +/// the command tag. +fn dml_response(state: &TableState, returning: bool, tag: &str) -> MockResponse { + if returning && !state.row.is_empty() { + let mut columns: Vec = state.row.keys().cloned().collect(); + columns.sort(); + let cells = columns.iter().map(|c| state.row.get(c).cloned()).collect(); + return MockResponse::Rows { + columns, + rows: vec![cells], + }; + } + MockResponse::Command(tag.to_string()) +} + fn to_row(cells: &[(&str, &str)]) -> HashMap { cells .iter() diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 24242ff..5708adf 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -30,7 +30,7 @@ use stackql_deploy::core::utils::UNKNOWN_EXPORT_PLACEHOLDER; use stackql_deploy::utils::pgwire::PgwireLite; pub use fake_cloud::FakeCloud; -pub use mock_server::MockServer; +pub use mock_server::{MockResponse, MockServer}; /// Route stackql-deploy's `log` output through the test harness so a failing /// test shows the same trail an operator would see. Honour `RUST_LOG` when diff --git a/tests/fixtures/databricks_workspace/resources/storage_credential.iql b/tests/fixtures/databricks_workspace/resources/storage_credential.iql index 27d52b1..9fa6958 100644 --- a/tests/fixtures/databricks_workspace/resources/storage_credential.iql +++ b/tests/fixtures/databricks_workspace/resources/storage_credential.iql @@ -1,4 +1,4 @@ -/*+ exists */ +/*+ exists, postdelete_retries=1, postdelete_retry_delay=0 */ SELECT id AS storage_credential_id FROM databricks_workspace.catalog.storage_credentials WHERE deployment_name = '{{ deployment_name }}' diff --git a/tests/fixtures/databricks_workspace/resources/workspace.iql b/tests/fixtures/databricks_workspace/resources/workspace.iql index f41d259..5b8fb2b 100644 --- a/tests/fixtures/databricks_workspace/resources/workspace.iql +++ b/tests/fixtures/databricks_workspace/resources/workspace.iql @@ -1,4 +1,4 @@ -/*+ exists */ +/*+ exists, postdelete_retries=1, postdelete_retry_delay=0 */ SELECT COUNT(*) AS count FROM databricks_account.provisioning.workspaces WHERE account_id = '{{ account_id }}' diff --git a/tests/fixtures/databricks_workspace/resources/workspace_with_callback.iql b/tests/fixtures/databricks_workspace/resources/workspace_with_callback.iql index 5e9cc26..d5c6718 100644 --- a/tests/fixtures/databricks_workspace/resources/workspace_with_callback.iql +++ b/tests/fixtures/databricks_workspace/resources/workspace_with_callback.iql @@ -1,4 +1,4 @@ -/*+ exists */ +/*+ exists, postdelete_retries=1, postdelete_retry_delay=0 */ SELECT COUNT(*) AS count FROM databricks_account.provisioning.workspaces WHERE account_id = '{{ account_id }}' diff --git a/tests/live_stacks/aws_ssm_onfailure/resources/ghost.iql b/tests/live_stacks/aws_ssm_onfailure/resources/ghost.iql index 86ffa01..4f5b5d7 100644 --- a/tests/live_stacks/aws_ssm_onfailure/resources/ghost.iql +++ b/tests/live_stacks/aws_ssm_onfailure/resources/ghost.iql @@ -1,4 +1,4 @@ -/*+ exists */ +/*+ exists, postdelete_retries=2, postdelete_retry_delay=3 */ SELECT 1 AS count /*+ create */ diff --git a/tests/live_stacks/aws_ssm_onfailure/resources/ghost_parameter.iql b/tests/live_stacks/aws_ssm_onfailure/resources/ghost_parameter.iql index d8a0922..2cc0781 100644 --- a/tests/live_stacks/aws_ssm_onfailure/resources/ghost_parameter.iql +++ b/tests/live_stacks/aws_ssm_onfailure/resources/ghost_parameter.iql @@ -1,4 +1,4 @@ -/*+ exists */ +/*+ exists, postdelete_retries=2, postdelete_retry_delay=3 */ SELECT 1 AS count /*+ create */ diff --git a/tests/teardown.rs b/tests/teardown.rs index 32ae90a..54f6025 100644 --- a/tests/teardown.rs +++ b/tests/teardown.rs @@ -12,7 +12,8 @@ mod common; use common::{ - assert_no_unknown_placeholder_sent, export_value, FakeCloud, MockServer, TestStack, UNKNOWN, + assert_no_unknown_placeholder_sent, export_value, FakeCloud, MockResponse, MockServer, + TestStack, UNKNOWN, }; use stackql_deploy::commands::common_args::FailureAction; use stackql_deploy::commands::teardown::run_teardown; @@ -265,9 +266,9 @@ fn teardown_does_not_abort_on_script_resources() { #[test] fn delete_callback_is_skipped_when_no_returning_row_was_captured() { - // The mock answers DELETE with a bare command tag, so no RETURNING row - // exists to poll. Both a dry run and a real run must skip the callback - // rather than fail to render `callback.*` variables. + // No RETURNING row exists to poll, either because it is a dry run or + // because the provider answered the DELETE with a bare command tag. Both + // must skip the callback rather than fail to render `callback.*`. let stack = TestStack::with_manifest("databricks_workspace", "manifest_with_delete_callback.yml"); @@ -276,13 +277,97 @@ fn delete_callback_is_skipped_when_no_returning_row_was_captured() { run_teardown(&mut runner, true, false, FailureAction::Error); assert!(!server.received("workspace_status = 'DELETED'")); - let server = MockServer::start(healthy_cloud().into_handler()); + let cloud = healthy_cloud().answer( + "DELETE FROM databricks_account.provisioning.workspaces", + MockResponse::Command("DELETE 1".to_string()), + ); + let server = MockServer::start(cloud.into_handler()); let mut runner = stack.runner(&server, "dev"); run_teardown(&mut runner, false, false, FailureAction::Error); assert!(server.received("DELETE FROM databricks_account.provisioning.workspaces")); assert!(!server.received("workspace_status = 'DELETED'")); } +#[test] +fn delete_callback_runs_before_the_post_delete_check_and_postdelete_polling_is_honoured() { + // The workspace delete returns a RETURNING row, and the delete only + // takes effect after one further read (an asynchronous provider). The + // exists anchor allows postdelete_retries=1, so the second check must + // confirm the delete; and the callback must be polled before that check. + let cloud = healthy_cloud().async_delete(WORKSPACES, 1).answer( + "workspace_status = 'DELETED'", + MockResponse::single_row(&[("success", "1")]), + ); + let server = MockServer::start(cloud.into_handler()); + let stack = + TestStack::with_manifest("databricks_workspace", "manifest_with_delete_callback.yml"); + let mut runner = stack.runner(&server, "dev"); + + run_teardown(&mut runner, false, false, FailureAction::Error); + + let queries = server.queries(); + let delete_at = queries + .iter() + .position(|q| q.starts_with("DELETE FROM databricks_account.provisioning.workspaces")) + .expect("workspace delete sent"); + let callback_at = queries + .iter() + .position(|q| q.contains("workspace_status = 'DELETED'")) + .expect("delete callback polled"); + let checks_after_delete: Vec = queries + .iter() + .enumerate() + .filter(|(i, q)| { + *i > delete_at && q.contains("SELECT COUNT(*) AS count") && q.contains(WORKSPACES) + }) + .map(|(i, _)| i) + .collect(); + assert!( + callback_at > delete_at && callback_at < checks_after_delete[0], + "callback must run between the delete and the first post-delete check: {:?}", + queries + ); + assert_eq!( + checks_after_delete.len(), + 2, + "one immediate check plus one postdelete retry, got: {:?}", + queries + ); + assert_eq!( + queries + .iter() + .filter(|q| q.starts_with("DELETE FROM databricks_account.provisioning.workspaces")) + .count(), + 1, + "only one delete was needed" + ); +} + +#[test] +fn unconfirmed_delete_does_not_stop_the_run() { + // The storage credential delete never takes effect within the + // postdelete budget, so it ends the run unconfirmed after one delete + // (the delete anchor's default retries=1); the workspace is still deleted. + let cloud = healthy_cloud().async_delete(STORAGE_CREDENTIALS, 5); + let server = MockServer::start(cloud.into_handler()); + let stack = TestStack::new("databricks_workspace"); + let mut runner = stack.runner(&server, "dev"); + + run_teardown(&mut runner, false, false, FailureAction::Error); + + let queries = server.queries(); + assert_eq!( + queries + .iter() + .filter( + |q| q.starts_with("DELETE FROM databricks_workspace.catalog.storage_credentials") + ) + .count(), + 1 + ); + assert!(server.received("DELETE FROM databricks_account.provisioning.workspaces")); +} + #[test] fn dry_run_teardown_sends_no_mutations() { let server = MockServer::start(healthy_cloud().into_handler()); diff --git a/tests/test_command.rs b/tests/test_command.rs index 6fcf2b8..b50d8d8 100644 --- a/tests/test_command.rs +++ b/tests/test_command.rs @@ -95,8 +95,4 @@ fn test_command_runs_script_resources_and_exports_their_output() { run_test(&mut runner, false, false, "Error", None); assert_eq!(export_value(&runner, "hook_result").as_deref(), Some("ok")); - assert_ne!( - export_value(&runner, "hook_result").as_deref(), - Some(UNKNOWN) - ); } diff --git a/website/docs/resource-query-files.md b/website/docs/resource-query-files.md index 62da309..c16f5a1 100644 --- a/website/docs/resource-query-files.md +++ b/website/docs/resource-query-files.md @@ -406,7 +406,9 @@ AND JSON_EXTRACT(properties, '$.provisioningState') = 'Succeeded' ### `postdelete_retries` and `postdelete_retry_delay` -The `postdelete_retries` and `postdelete_retry_delay` query options are used in `exists` queries and are implemeneted specifically for `teardown` operations, allowing time for the resource to be deleted by the provider. +The `postdelete_retries` and `postdelete_retry_delay` query options are used in `exists` queries and are implemented specifically for `teardown` operations, allowing time for the resource to be deleted by the provider. + +After each `delete` statement (and its `callback:delete`, if one is defined), the `exists` query is run immediately and, while the resource is still present, up to `postdelete_retries` more times with `postdelete_retry_delay` seconds between checks. The defaults are `postdelete_retries=10` and `postdelete_retry_delay=5`. If the resource is still present after the last check, the `delete` statement is re-issued according to the `delete` anchor's own `retries` and `retry_delay` options (default `retries=1`, a single attempt), after which the resource is reported as not confirmed deleted and the teardown moves on. ```sql /*+ exists, postdelete_retries=10, postdelete_retry_delay=5 */