From 58c1ebc868511092d9109218a4f21c447c73f79a Mon Sep 17 00:00:00 2001 From: sdairs Date: Fri, 15 May 2026 16:58:39 +0100 Subject: [PATCH 1/2] Cover instance_password_update + instance_scaling_update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #159. Adds two phases to the live service-CRUD integration test: - **Vertical scaling (deprecated endpoint)** — exercises `instance_scaling_update` (PATCH /scaling), distinct from `instance_replica_scaling_update` (PATCH /replicaScaling). Captures the service's pre-state `minTotalMemoryGb`/`maxTotalMemoryGb` at the end of the existing scaling phase (1 replica @ 8 GB so totals map cleanly to per-replica), round-trips 8 → 16 → 8 GB via the deprecated body, polling for each change to land. Post-condition equals pre-condition so downstream phases see the expected base state. Non-blocking — the endpoint is deprecated and not depended on. - **Password rotation** — new "Password" phase exercises `instance_password_update` with an empty body so the server generates the password. Asserts the response surfaces a non-empty password. The query path used elsewhere in the suite is openapi-key based, so the rotated password is not consumed; no re-rotation needed because the service is deleted immediately after. Non-blocking. Also rewrites four pre-existing nested `if let` blocks in `spec_coverage_test.rs` as let-chains so `cargo clippy --tests -- -D warnings` passes — CI's clippy invocation omits `--tests`, so these had drifted past the lint floor. Semantics unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../tests/integration_test.rs | 188 +++++++++++++++++- .../tests/spec_coverage_test.rs | 110 +++++----- 2 files changed, 242 insertions(+), 56 deletions(-) diff --git a/crates/clickhouse-cloud-api/tests/integration_test.rs b/crates/clickhouse-cloud-api/tests/integration_test.rs index c271922..f8327ce 100644 --- a/crates/clickhouse-cloud-api/tests/integration_test.rs +++ b/crates/clickhouse-cloud-api/tests/integration_test.rs @@ -1213,7 +1213,142 @@ async fn cloud_service_crud_lifecycle() -> TestResult<()> { }) .await?; - // ── 7. Delete ──────────────────────────────────────────────── + // Vertical scaling round-trip via the deprecated + // `instance_scaling_update` endpoint (PATCH /scaling). This is + // distinct from `instance_replica_scaling_update` (PATCH + // /replicaScaling) exercised above: the deprecated endpoint takes + // `minTotalMemoryGb` / `maxTotalMemoryGb` and only the vertical + // axis. We enter this phase at 1 replica @ base_memory_gb so the + // total-memory body maps directly to per-replica memory; round-trip + // back to the entry state so subsequent phases see the expected + // base. + let pre_vertical = failures + .run( + &ctx, + StepKind::Blocking, + "capture pre-state for deprecated vertical scaling", + || { + let client = client.clone(); + let org_id = ctx.org_id.clone(); + let service_id = service_id.clone(); + async move { + let resp = client.instance_get(&org_id, &service_id).await?; + resp.result + .ok_or_else(|| "service get returned no result".into()) + } + }, + ) + .await? + .expect("blocking steps always return a value"); + // Sanity: the deprecated body's totals only equal per-replica when + // num_replicas == 1. We rely on the previous step landing us there. + assert_eq!(pre_vertical.num_replicas, base_replicas); + let pre_min_total = pre_vertical.min_total_memory_gb; + let pre_max_total = pre_vertical.max_total_memory_gb; + + failures + .run( + &ctx, + StepKind::NonBlocking, + "deprecated vertical scale up to 16 GB", + || { + let client = client.clone(); + let org_id = ctx.org_id.clone(); + let service_id = service_id.clone(); + let timeout = ctx.steady_state_timeout; + let interval = ctx.poll_interval; + async move { + scale_service_vertical_and_wait( + &client, + &org_id, + &service_id, + Some(scaled_memory_gb), + Some(scaled_memory_gb), + "deprecated vertical scale up", + timeout, + interval, + ) + .await + } + }, + ) + .await?; + + failures + .run( + &ctx, + StepKind::NonBlocking, + "deprecated vertical scale back to pre-state", + || { + let client = client.clone(); + let org_id = ctx.org_id.clone(); + let service_id = service_id.clone(); + let timeout = ctx.steady_state_timeout; + let interval = ctx.poll_interval; + async move { + scale_service_vertical_and_wait( + &client, + &org_id, + &service_id, + Some(pre_min_total), + Some(pre_max_total), + "deprecated vertical scale restore", + timeout, + interval, + ) + .await + } + }, + ) + .await?; + + // ── 7. Password ────────────────────────────────────────────── + // + // `instance_password_update` rotates the service password. The + // query path used by the rest of the suite is openapi-key-based, + // so the rotated password is not consumed anywhere — the pass + // condition is just a successful response that surfaces a fresh + // password. We pass an empty body so the server generates a new + // password and returns it; no re-rotation is needed because the + // service is about to be deleted. + + log_phase("Password"); + failures + .run( + &ctx, + StepKind::NonBlocking, + "rotate service password", + || { + let client = client.clone(); + let org_id = ctx.org_id.clone(); + let service_id = service_id.clone(); + let run_id = ctx.run_id.clone(); + async move { + let resp = client + .instance_password_update( + &org_id, + &service_id, + &ServicePasswordPatchRequest::default(), + ) + .await?; + let result = resp + .result + .ok_or("password update returned no result")?; + if result.password.is_empty() { + return Err("password update response had empty password".into()); + } + eprintln!( + " password rotated (length={}, run_id={})", + result.password.len(), + run_id + ); + Ok(()) + } + }, + ) + .await?; + + // ── 8. Delete ──────────────────────────────────────────────── log_phase("Delete"); @@ -1366,6 +1501,7 @@ async fn poll_for_ip_presence( Ok(()) } +#[allow(clippy::too_many_arguments)] async fn scale_service_and_wait( client: &Client, org_id: &str, @@ -1416,3 +1552,53 @@ async fn scale_service_and_wait( Ok(()) } + +#[allow(deprecated)] +#[allow(clippy::too_many_arguments)] +async fn scale_service_vertical_and_wait( + client: &Client, + org_id: &str, + service_id: &str, + min_total_memory_gb: Option, + max_total_memory_gb: Option, + description: &str, + timeout: std::time::Duration, + interval: std::time::Duration, +) -> TestResult<()> { + client + .instance_scaling_update( + org_id, + service_id, + &ServiceScalingPatchRequest { + min_total_memory_gb, + max_total_memory_gb, + ..Default::default() + }, + ) + .await?; + + poll_until( + &format!("{description} visibility"), + timeout, + interval, + || { + let client = client.clone(); + let org_id = org_id.to_string(); + let service_id = service_id.to_string(); + async move { + let resp = client.instance_get(&org_id, &service_id).await?; + let svc = resp.result.ok_or("service get returned no result")?; + if min_total_memory_gb.is_none_or(|v| svc.min_total_memory_gb == v) + && max_total_memory_gb.is_none_or(|v| svc.max_total_memory_gb == v) + { + Ok(Some(())) + } else { + Ok(None) + } + } + }, + ) + .await?; + + Ok(()) +} diff --git a/crates/clickhouse-cloud-api/tests/spec_coverage_test.rs b/crates/clickhouse-cloud-api/tests/spec_coverage_test.rs index ab6abf0..54e4484 100644 --- a/crates/clickhouse-cloud-api/tests/spec_coverage_test.rs +++ b/crates/clickhouse-cloud-api/tests/spec_coverage_test.rs @@ -202,10 +202,10 @@ fn collect_schema_refs(value: &Value) -> BTreeSet { fn collect_schema_refs_inner(value: &Value, refs: &mut BTreeSet) { match value { Value::Object(map) => { - if let Some(reference) = map.get("$ref").and_then(Value::as_str) { - if let Some(schema_name) = reference.strip_prefix("#/components/schemas/") { - refs.insert(schema_name.to_string()); - } + if let Some(reference) = map.get("$ref").and_then(Value::as_str) + && let Some(schema_name) = reference.strip_prefix("#/components/schemas/") + { + refs.insert(schema_name.to_string()); } for child in map.values() { @@ -595,17 +595,17 @@ fn resolve_required_fields<'a>(schema_name: &str, schema: &'a Value) -> BTreeSet fn is_field_nullable(prop: &Value) -> bool { // type: ["string", "null"] - if let Some(types) = prop.get("type").and_then(Value::as_array) { - if types.iter().any(|t| t.as_str() == Some("null")) { - return true; - } + if let Some(types) = prop.get("type").and_then(Value::as_array) + && types.iter().any(|t| t.as_str() == Some("null")) + { + return true; } // oneOf/anyOf with a null variant for key in &["oneOf", "anyOf"] { - if let Some(variants) = prop.get(*key).and_then(Value::as_array) { - if variants.iter().any(|v| v.get("type").and_then(Value::as_str) == Some("null")) { - return true; - } + if let Some(variants) = prop.get(*key).and_then(Value::as_array) + && variants.iter().any(|v| v.get("type").and_then(Value::as_str) == Some("null")) + { + return true; } } false @@ -627,52 +627,52 @@ fn parse_model_fields(source: &str) -> HashMap = HashMap::new(); - let mut pending_rename: Option = None; - - while i < lines.len() { - let line = lines[i].trim(); - - if line == "}" { - break; - } - - // Extract rename from serde attribute - if line.starts_with("#[serde(") { - if let Some(rename) = extract_serde_rename(line) { - pending_rename = Some(rename.to_string()); - } - } - - // Extract field definition - if let Some(rest) = line.strip_prefix("pub ") { - if let Some(colon_pos) = rest.find(':') { - let rust_field_name = rest[..colon_pos].trim(); - let type_str = rest[colon_pos + 1..].trim().trim_end_matches(','); - let is_option = type_str.starts_with("Option<"); - - // Use rename as spec name, or fall back to rust field name - // Strip r# prefix from raw identifiers (e.g., r#type -> type) - let spec_name = pending_rename.take().unwrap_or_else(|| { - rust_field_name - .strip_prefix("r#") - .unwrap_or(rust_field_name) - .to_string() - }); - - fields.insert(spec_name, FieldInfo { is_option }); - } - } - - i += 1; + if let Some(rest) = line.strip_prefix("pub struct ") + && let Some(struct_name) = identifier_prefix(rest) + { + let struct_name = struct_name.to_string(); + i += 1; + let mut fields: HashMap = HashMap::new(); + let mut pending_rename: Option = None; + + while i < lines.len() { + let line = lines[i].trim(); + + if line == "}" { + break; } - result.insert(struct_name, fields); + // Extract rename from serde attribute + if line.starts_with("#[serde(") + && let Some(rename) = extract_serde_rename(line) + { + pending_rename = Some(rename.to_string()); + } + + // Extract field definition + if let Some(rest) = line.strip_prefix("pub ") + && let Some(colon_pos) = rest.find(':') + { + let rust_field_name = rest[..colon_pos].trim(); + let type_str = rest[colon_pos + 1..].trim().trim_end_matches(','); + let is_option = type_str.starts_with("Option<"); + + // Use rename as spec name, or fall back to rust field name + // Strip r# prefix from raw identifiers (e.g., r#type -> type) + let spec_name = pending_rename.take().unwrap_or_else(|| { + rust_field_name + .strip_prefix("r#") + .unwrap_or(rust_field_name) + .to_string() + }); + + fields.insert(spec_name, FieldInfo { is_option }); + } + + i += 1; } + + result.insert(struct_name, fields); } i += 1; From c2051ad374deef92c187fce0a1c1bcabe99e3b2e Mon Sep 17 00:00:00 2001 From: sdairs Date: Fri, 15 May 2026 17:39:26 +0100 Subject: [PATCH 2/2] Fix deprecated vertical scaling step: multiples of 12 only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deprecated `instance_scaling_update` endpoint validates `minTotalMemoryGb`/`maxTotalMemoryGb` as multiples of 12 (the modern `instance_replica_scaling_update` endpoint accepts multiples of 4). The test was using 16 GB and hit `BAD_REQUEST: The value must be a multiple of 12 `. Add a "land on multiple-of-12 memory" Blocking step that moves the service from the modern phase's 8 GB to 12 GB via the modern endpoint before entering the deprecated round-trip, then round-trip 12 → 24 → 12 via the deprecated endpoint. Capture pre-state remains in place so the restore step adapts if the API ever returns a different starting value. --- .../tests/integration_test.rs | 51 ++++++++++++++++--- 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/crates/clickhouse-cloud-api/tests/integration_test.rs b/crates/clickhouse-cloud-api/tests/integration_test.rs index f8327ce..a657754 100644 --- a/crates/clickhouse-cloud-api/tests/integration_test.rs +++ b/crates/clickhouse-cloud-api/tests/integration_test.rs @@ -16,6 +16,13 @@ async fn cloud_service_crud_lifecycle() -> TestResult<()> { let mut failures = FailureRecorder::default(); let base_memory_gb = 8.0_f64; let scaled_memory_gb = 16.0_f64; + // The deprecated `instance_scaling_update` endpoint validates + // `minTotalMemoryGb`/`maxTotalMemoryGb` as multiples of 12, unlike + // the modern `instance_replica_scaling_update` endpoint which + // accepts multiples of 4. Use a dedicated pair of values for the + // deprecated round-trip to satisfy that constraint. + let deprecated_base_total_memory_gb = 12.0_f64; + let deprecated_scaled_total_memory_gb = 24.0_f64; let base_replicas = 1.0_f64; let scaled_replicas = 3.0_f64; let primary_ip = "203.0.113.10/32"; @@ -1218,10 +1225,40 @@ async fn cloud_service_crud_lifecycle() -> TestResult<()> { // distinct from `instance_replica_scaling_update` (PATCH // /replicaScaling) exercised above: the deprecated endpoint takes // `minTotalMemoryGb` / `maxTotalMemoryGb` and only the vertical - // axis. We enter this phase at 1 replica @ base_memory_gb so the - // total-memory body maps directly to per-replica memory; round-trip - // back to the entry state so subsequent phases see the expected - // base. + // axis. The deprecated endpoint additionally requires the totals to + // be multiples of 12, so we first move via the modern endpoint to + // `deprecated_base_total_memory_gb` before the round-trip. We stay + // at 1 replica so the total-memory body maps directly to + // per-replica memory. + failures + .run( + &ctx, + StepKind::Blocking, + "land on multiple-of-12 memory before deprecated phase", + || { + let client = client.clone(); + let org_id = ctx.org_id.clone(); + let service_id = service_id.clone(); + let timeout = ctx.steady_state_timeout; + let interval = ctx.poll_interval; + async move { + scale_service_and_wait( + &client, + &org_id, + &service_id, + Some(deprecated_base_total_memory_gb), + Some(deprecated_base_total_memory_gb), + Some(base_replicas), + "modern scale to deprecated base", + timeout, + interval, + ) + .await + } + }, + ) + .await?; + let pre_vertical = failures .run( &ctx, @@ -1250,7 +1287,7 @@ async fn cloud_service_crud_lifecycle() -> TestResult<()> { .run( &ctx, StepKind::NonBlocking, - "deprecated vertical scale up to 16 GB", + "deprecated vertical scale up to 24 GB", || { let client = client.clone(); let org_id = ctx.org_id.clone(); @@ -1262,8 +1299,8 @@ async fn cloud_service_crud_lifecycle() -> TestResult<()> { &client, &org_id, &service_id, - Some(scaled_memory_gb), - Some(scaled_memory_gb), + Some(deprecated_scaled_total_memory_gb), + Some(deprecated_scaled_total_memory_gb), "deprecated vertical scale up", timeout, interval,