diff --git a/.gitignore b/.gitignore index b0f2d9f..5a49eb8 100644 --- a/.gitignore +++ b/.gitignore @@ -1,7 +1,7 @@ /target stackql /.stackql -stackql*.sh +stackql*shell.sh stackql*.zip stackql*.pkg stackql_history.txt @@ -13,4 +13,5 @@ stackql-deploy nohup.out contributors.csv .claude/ -nohup.out \ No newline at end of file +nohup.out +tmp/ \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index d999b9b..3cb9675 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,21 @@ # Changelog +## 2.0.7 (2026-04-19) + +### Fixes + +- Fixed post-deploy failure for `createorupdate` resources whose `exports` anchor is a literal `SELECT` with no `FROM` clause (a supported and common pattern when the values to export are already known and an extra API round-trip is wasteful). Previously the exports query was executed as a statecheck proxy, and a FROM-less result caused the proxy check to report the resource was not in the desired state, aborting the run. With `createorupdate`, the DML is authoritative, so the exports-as-statecheck proxy is now skipped entirely - `exports` still runs to populate the global context for downstream resources. +- Fixed `stackql-deploy upgrade` downloading the stackql binary twice when the binary was missing: the pre-command binary check triggered a download, and the subcommand dispatch then triggered a second download. `upgrade` is now exempt from the pre-command binary check. +- Server-side notices from provider HTTP 4xx/5xx responses are now detected even when stackql wraps them as a generic `a notice level event has occurred` message with the real status code in the `DETAIL:` payload. Previously these escaped the error check and the `create`/`update`/`delete` operation silently appeared to succeed while the post-deploy `exists` check spun through its retries. +- Collapsed duplicate lines within a single notice's `DETAIL:` payload so repeated provider error bodies are printed once. +- Teardown now tolerates resources with unresolved template variables. If an `exists`, `exports`, or `delete` query references a variable that was never populated (because an upstream resource doesn't exist), the resource is treated as already torn down and skipped, instead of aborting the run. Stacks in a half-baked state can now be torn down cleanly. +- During teardown, `RETURNING` clauses are stripped from rendered `delete` DML when `return_vals.delete` is not configured for the resource. Some providers reject `RETURNING *` on `DELETE`, and teardown has no consumer for the returned data unless the manifest explicitly opts in via `return_vals.delete`. When `return_vals.delete` is configured the `RETURNING` clause is preserved and mapped fields are captured as `this.*` with non-fatal warnings if a mapping cannot be satisfied. +- Stale provider notices are no longer re-surfaced on subsequent queries. stackql emits a cumulative `NoticeResponse` on every query that includes every provider notice observed earlier in the session; the pgwire client now tracks each notice line already surfaced and drops byte-identical re-emissions. Dedup is exact-match (no canonicalization) so two distinct provider errors — which always differ in their embedded request/serving IDs — are never conflated. Fixes spurious `create`/`update`/`delete` failures where a 4xx provider response from an earlier `exists` SELECT was attributed to a later DML. + +### Features + +- When retries are exhausted on a `statecheck`, `exports` proxy, or post-deploy `exists` check, the last rendered query is now logged at `warn` level so the failing SQL is visible without needing `--show-queries` or `--log-level debug`. Pre-create exists checks (which fast-fail by design) stay silent. + ## 2.0.6 (2026-03-28) ### Fixes diff --git a/Cargo.lock b/Cargo.lock index 8410613..42be9f4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1809,7 +1809,7 @@ checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" [[package]] name = "stackql-deploy" -version = "2.0.6" +version = "2.0.7" dependencies = [ "base64", "chrono", diff --git a/Cargo.toml b/Cargo.toml index b8adf41..594abf1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "stackql-deploy" -version = "2.0.6" +version = "2.0.7" edition = "2021" rust-version = "1.75" description = "Infrastructure-as-code framework for declarative cloud resource management using StackQL" diff --git a/internal-stackql-deploy-build-install.sh b/internal-stackql-deploy-build-install.sh new file mode 100644 index 0000000..85110fc --- /dev/null +++ b/internal-stackql-deploy-build-install.sh @@ -0,0 +1,9 @@ +rm -rf stackql-deploy +rm -rf stackql +rm -rf stackql-aws-cloud-shell.sh +rm -rf stackql-azure-cloud-shell.sh +rm -rf stackql-google-cloud-shell.sh +rm -rf stackql-databricks-shell.sh +cargo build --release +cp target/release/stackql-deploy stackql-deploy +./stackql-deploy upgrade \ No newline at end of file diff --git a/public-stackql-deploy-install.sh b/public-stackql-deploy-install.sh new file mode 100644 index 0000000..39a7096 --- /dev/null +++ b/public-stackql-deploy-install.sh @@ -0,0 +1,7 @@ +rm -rf stackql-deploy +rm -rf stackql +rm -rf stackql-aws-cloud-shell.sh +rm -rf stackql-azure-cloud-shell.sh +rm -rf stackql-google-cloud-shell.sh +rm -rf stackql-databricks-shell.sh +curl -L https://get-stackql-deploy.io | tar xzf - \ No newline at end of file diff --git a/src/app.rs b/src/app.rs index 331e139..c2ac59c 100644 --- a/src/app.rs +++ b/src/app.rs @@ -75,7 +75,7 @@ pub const STACKQL_BINARY_NAME: &str = "stackql"; pub const STACKQL_RELEASE_BASE_URL: &str = "https://releases.stackql.io/stackql/latest"; /// Commands exempt from binary check -pub const EXEMPT_COMMANDS: [&str; 1] = ["init"]; +pub const EXEMPT_COMMANDS: [&str; 2] = ["init", "upgrade"]; /// The base URL for GitHub template repository pub const GITHUB_TEMPLATE_BASE: &str = diff --git a/src/commands/build.rs b/src/commands/build.rs index 7a6c948..b923b8b 100644 --- a/src/commands/build.rs +++ b/src/commands/build.rs @@ -904,6 +904,12 @@ fn run_build( ); is_correct_state = true; } + } else if has_createorupdate { + info!( + "createorupdate for [{}] is authoritative, skipping exports-as-statecheck proxy", + resource.name + ); + is_correct_state = true; } else if let Some(ref eq_str) = exports_query_str { info!( "using exports query as post-deploy statecheck for [{}]", diff --git a/src/commands/teardown.rs b/src/commands/teardown.rs index 42455f5..a1ed05e 100644 --- a/src/commands/teardown.rs +++ b/src/commands/teardown.rs @@ -8,7 +8,7 @@ use std::time::Instant; use clap::{ArgMatches, Command}; -use log::{debug, info}; +use log::{debug, info, warn}; use crate::commands::base::CommandRunner; use crate::commands::common_args::{ @@ -16,6 +16,7 @@ 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::utils::connection::create_client; use crate::utils::display::{print_unicode_box, BorderColor}; use crate::utils::server::{check_and_start_server, stop_local_server}; @@ -106,17 +107,19 @@ fn collect_exports(runner: &mut CommandRunner, show_queries: bool, dry_run: bool continue; } - 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) - } 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") { - let rendered = - runner.render_query(&resource.name, "exists", &eq.template, &full_context); + 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) + } 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) + { let (_exists, fields) = runner.check_if_resource_exists( resource, &rendered, @@ -131,17 +134,35 @@ 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") { - let rendered = - runner.render_query(&resource.name, "exports", &eq.template, &full_context); + } + 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), 1u32, 0u32) - } else { - (None, 1u32, 0u32) + Some(rendered) => (Some(rendered), 1u32, 0u32), + None => { + info!( + "[{}] exports query has unresolved variables, skipping exports collection", + resource.name + ); + (None, 1u32, 0u32) + } } - }; + } else { + (None, 1u32, 0u32) + } + }; if let Some(ref eq_str) = exports_query { runner.process_exports( @@ -227,9 +248,17 @@ fn run_teardown(runner: &mut CommandRunner, dry_run: bool, show_queries: bool, _ let (exists_query_str, exists_retries, exists_retry_delay) = if let Some(eq) = resource_queries.get("exists") { - let rendered = - runner.render_query(&resource.name, "exists", &eq.template, &full_context); - (rendered, eq.options.retries, eq.options.retry_delay) + if let Some(rendered) = + runner.try_render_query(&resource.name, "exists", &eq.template, &full_context) + { + (rendered, eq.options.retries, eq.options.retry_delay) + } else { + info!( + "[{}] exists query has unresolved variables, assuming resource does not exist, skipping...", + resource.name + ); + continue; + } } else if let Some(sq) = resource_queries.get("statecheck") { info!( "exists query not defined for [{}], trying statecheck query as exists query.", @@ -290,15 +319,52 @@ fn run_teardown(runner: &mut CommandRunner, dry_run: bool, show_queries: bool, _ exists }; - // Render the delete query now (after exists fields are available). - let dq = resource_queries.get("delete").unwrap(); - let delete_query = - runner.render_query(&resource.name, "delete", &dq.template, &full_context); - let delete_retries = dq.options.retries; - let delete_retry_delay = dq.options.retry_delay; - // 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, + "delete", + &dq.template, + &full_context, + ) { + Some(rendered) => rendered, + None => { + info!( + "[{}] delete query has unresolved variables, assuming resource does not exist, skipping...", + resource.name + ); + continue; + } + }; + let delete_retries = dq.options.retries; + let delete_retry_delay = dq.options.retry_delay; + + // Only keep a RETURNING clause when return_vals.delete is configured + // for this resource. Otherwise strip it — teardown has no use for + // return values, and some providers reject RETURNING * on DELETE. + let delete_return_mappings = resource.get_return_val_mappings("delete"); + let delete_query = if delete_return_mappings.is_empty() { + if has_returning_clause(&rendered_delete) { + debug!( + "[{}] stripping RETURNING clause from delete query (no return_vals.delete configured)", + resource.name + ); + strip_returning_clause(&rendered_delete) + } else { + rendered_delete + } + } else if !has_returning_clause(&rendered_delete) { + warn!( + "return_vals.delete specified for [{}] but delete query has no RETURNING clause; capture will be skipped", + resource.name + ); + rendered_delete + } else { + rendered_delete + }; + let (returning_row, delete_confirmed) = runner.delete_and_confirm( resource, &delete_query, @@ -312,7 +378,39 @@ fn run_teardown(runner: &mut CommandRunner, dry_run: bool, show_queries: bool, _ // 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() { + for (src, tgt) in &delete_return_mappings { + if let Some(val) = row.get(src.as_str()) { + if !val.is_empty() && val != "null" { + info!( + "RETURNING [{}] for [{}] captured as [this.{}] = [{}]", + src, resource.name, tgt, val + ); + full_context + .insert(format!("{}.{}", resource.name, tgt), val.clone()); + } else { + warn!( + "return_vals.delete for [{}]: field [{}] in RETURNING result is null or empty", + resource.name, src + ); + } + } else { + warn!( + "return_vals.delete for [{}]: expected field [{}] not found in RETURNING result", + resource.name, src + ); + } + } + } + } 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. diff --git a/src/core/utils.rs b/src/core/utils.rs index 616c19c..d18211b 100644 --- a/src/core/utils.rs +++ b/src/core/utils.rs @@ -11,7 +11,7 @@ use std::process; use std::thread; use std::time::{Duration, Instant}; -use log::{debug, error, info}; +use log::{debug, error, info, warn}; use crate::core::errors::check_fatal_error; use crate::utils::pgwire::PgwireLite; @@ -245,8 +245,8 @@ pub fn run_stackql_command( continue; } else { catch_error_and_exit(&format!( - "Error during stackql command execution:\n\n{}\n", - notice + "Error during stackql command execution:\n\n{}\n\nlast rendered query:\n\n{}\n", + notice, processed_command )); } } @@ -326,10 +326,16 @@ pub fn run_stackql_command( } /// Check if a notice/message indicates an error. +/// +/// Patterns can appear either at the start of the notice message or inside +/// the `DETAIL:` payload (stackql wraps provider errors as a generic "a +/// notice level event has occurred" message with the real HTTP status in +/// the detail), so match against the whole notice string. fn error_detected_in_notice(msg: &str) -> bool { - msg.starts_with("http response status code: 4") - || msg.starts_with("http response status code: 5") + msg.contains("http response status code: 4") + || msg.contains("http response status code: 5") || msg.starts_with("error:") + || msg.contains("\nDETAIL: error:") || msg.starts_with("disparity in fields to insert") || msg.starts_with("cannot find matching operation") } @@ -496,6 +502,17 @@ pub fn perform_retries_with_fields( attempt += 1; } + // Pre-create exists checks run with retries == 1 as a "fast fail" where + // a negative result is the expected outcome (the resource does not yet + // exist and needs to be created). Only surface the query when the + // caller configured real retries — i.e. a statecheck, exports proxy, + // or post-deploy exists check where exhaustion signals a stack failure. + if retries > 1 { + warn!( + "retries exhausted for [{}], last rendered query:\n\n{}\n", + resource_name, query + ); + } (false, None) } @@ -729,6 +746,24 @@ pub fn has_returning_clause(query: &str) -> bool { query.to_uppercase().contains("RETURNING") } +/// Remove a trailing `RETURNING ...` clause from a DML query. +/// +/// Matches case-insensitively on the last `RETURNING` keyword occurrence and +/// truncates from there, preserving any trailing `;`. +pub fn strip_returning_clause(query: &str) -> String { + let upper = query.to_uppercase(); + if let Some(idx) = upper.rfind("RETURNING") { + let trailing_semi = query.trim_end().ends_with(';'); + let mut out = query[..idx].trim_end().to_string(); + if trailing_semi { + out.push(';'); + } + out + } else { + query.to_string() + } +} + /// Execute a DML command (INSERT / UPDATE / DELETE), optionally capturing /// the `RETURNING *` result as the first row. /// @@ -771,8 +806,8 @@ pub fn run_stackql_dml_returning( break; } else { catch_error_and_exit(&format!( - "Error during stackql DML execution:\n\n{}\n", - notice + "Error during stackql DML execution:\n\n{}\n\nlast rendered query:\n\n{}\n", + notice, command )); } } diff --git a/src/utils/pgwire.rs b/src/utils/pgwire.rs index 5064212..e0d32fc 100644 --- a/src/utils/pgwire.rs +++ b/src/utils/pgwire.rs @@ -6,7 +6,7 @@ //! to a local StackQL server using the PostgreSQL simple query protocol (v3). //! No native dependencies (replaces pgwire-lite → libpq-sys). -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::io::{Read, Write}; use std::net::TcpStream; @@ -37,6 +37,12 @@ pub struct PgQueryResult { /// Minimal PostgreSQL wire-protocol client. pub struct PgwireLite { stream: TcpStream, + /// Canonical signatures of every notice line surfaced earlier in this + /// session. stackql emits each new query's NoticeResponse with a + /// cumulative `detail` field containing every provider notice seen so + /// far. Any detail line already present in this set is stale and + /// dropped from subsequent query results. + seen_notice_sigs: HashSet, } impl PgwireLite { @@ -49,7 +55,10 @@ impl PgwireLite { let stream = TcpStream::connect(&addr) .map_err(|e| format!("Connection to {} failed: {}", addr, e))?; - let mut client = PgwireLite { stream }; + let mut client = PgwireLite { + stream, + seen_notice_sigs: HashSet::new(), + }; client.startup()?; Ok(client) } @@ -118,6 +127,12 @@ impl PgwireLite { /// Execute a simple (non-prepared) SQL query and return structured results. pub fn query(&mut self, sql: &str) -> Result { + // Drain any bytes the server may have emitted outside a prior query's + // response window. stackql has been observed to re-emit stale + // NoticeResponse frames from earlier statements, which would + // otherwise be attributed to this query. + self.drain_pending(); + // Send Query message: 'Q' | int32(len) | sql\0 let sql_bytes = sql.as_bytes(); let payload_len = 4 + sql_bytes.len() + 1; // length field + sql + null @@ -187,14 +202,43 @@ impl PgwireLite { } } + // Drop any detail lines we've already surfaced earlier in this + // session; stackql keeps re-emitting them on every subsequent + // query's NoticeResponse. + let kept = filter_stale_notices(notices, &mut self.seen_notice_sigs); + Ok(PgQueryResult { column_names, rows, - notices, + notices: kept, row_count, }) } + /// Discard any bytes the server sent outside a query response window. + /// + /// The server is expected to stay silent between queries (the prior + /// response ended with `ReadyForQuery` / `'Z'`). stackql has been + /// observed to emit stray `NoticeResponse` frames referring to + /// previously-executed statements; if left in the socket buffer those + /// frames would be read at the start of the next query and + /// misattributed. Best-effort non-blocking read; any I/O error is + /// swallowed since the normal path is zero bytes available. + fn drain_pending(&mut self) { + if self.stream.set_nonblocking(true).is_err() { + return; + } + let mut buf = [0u8; 4096]; + loop { + match self.stream.read(&mut buf) { + Ok(0) => break, // EOF + Ok(_) => continue, // keep draining + Err(_) => break, // WouldBlock or other — nothing more pending + } + } + let _ = self.stream.set_nonblocking(false); + } + // ------------------------------------------------------------------ // Low-level I/O helpers // ------------------------------------------------------------------ @@ -224,6 +268,84 @@ impl PgwireLite { } } +// ------------------------------------------------------------------ +// Stale notice filtering +// ------------------------------------------------------------------ + +/// Build a signature for a notice line. Dedup uses exact byte-match (after +/// trimming) so two genuine provider errors — which always differ in their +/// embedded request/serving IDs — are never conflated. A re-emitted stale +/// notice is byte-identical to its first emission and dedups cleanly. +fn canonical_line(line: &str) -> String { + line.trim().to_string() +} + +/// Drop detail lines whose canonical signature was already surfaced earlier +/// in the session. `seen` is mutated in place: every line that survives +/// filtering is added so it counts as stale on subsequent queries. A notice +/// whose message is stale and whose detail becomes empty after filtering is +/// dropped entirely. +fn filter_stale_notices(notices: Vec, seen: &mut HashSet) -> Vec { + let mut kept = Vec::with_capacity(notices.len()); + + for n in notices { + let message_stale = n + .fields + .get("message") + .map(|m| seen.contains(&format!("M|{}", canonical_line(m)))) + .unwrap_or(false); + + let filtered_detail = n.fields.get("detail").map(|detail| { + detail + .lines() + .filter(|line| { + let c = canonical_line(line); + c.is_empty() || !seen.contains(&format!("D|{}", c)) + }) + .collect::>() + .join("\n") + }); + + let has_detail_content = filtered_detail + .as_deref() + .map(|d| !d.trim().is_empty()) + .unwrap_or(false); + + if message_stale && !has_detail_content { + continue; + } + + // Mark the surviving signatures as seen so they're filtered from + // future queries. + if let Some(m) = n.fields.get("message") { + let c = canonical_line(m); + if !c.is_empty() { + seen.insert(format!("M|{}", c)); + } + } + if let Some(d) = filtered_detail.as_deref() { + for line in d.lines() { + let c = canonical_line(line); + if !c.is_empty() { + seen.insert(format!("D|{}", c)); + } + } + } + + let mut new_fields = n.fields.clone(); + if let Some(d) = filtered_detail { + if d.trim().is_empty() { + new_fields.remove("detail"); + } else { + new_fields.insert("detail".to_string(), d); + } + } + kept.push(Notice { fields: new_fields }); + } + + kept +} + // ------------------------------------------------------------------ // Message parsers (free functions for readability) // ------------------------------------------------------------------ diff --git a/src/utils/query.rs b/src/utils/query.rs index b75bf6d..58aaf34 100644 --- a/src/utils/query.rs +++ b/src/utils/query.rs @@ -94,10 +94,25 @@ pub fn execute_query(query: &str, client: &mut PgwireLite) -> Result = detail + .lines() + .filter(|line| { + let canonical = volatile_ids.replace_all(line, "").to_string(); + seen.insert(canonical) + }) + .collect(); notice_text.push_str("\nDETAIL: "); - notice_text.push_str(detail); + notice_text.push_str(&deduped.join("\n")); } // Add hint if available diff --git a/sync.sh b/sync.sh new file mode 100644 index 0000000..a6a69e3 --- /dev/null +++ b/sync.sh @@ -0,0 +1,2 @@ +git fetch origin +git merge origin/main \ No newline at end of file