diff --git a/.github/workflows/cloud-integration.yml b/.github/workflows/cloud-integration.yml index f37f197..e4eed58 100644 --- a/.github/workflows/cloud-integration.yml +++ b/.github/workflows/cloud-integration.yml @@ -36,6 +36,7 @@ jobs: CLICKHOUSE_CLOUD_TEST_ORG_ID: ${{ secrets.CLICKHOUSE_CLOUD_TEST_ORG_ID }} CLICKHOUSE_CLOUD_TEST_PROVIDER: ${{ secrets.CLICKHOUSE_CLOUD_TEST_PROVIDER }} CLICKHOUSE_CLOUD_TEST_REGION: ${{ secrets.CLICKHOUSE_CLOUD_TEST_REGION }} + CLICKHOUSE_CLOUD_TEST_SECONDARY_USER_ID: ${{ secrets.CLICKHOUSE_CLOUD_TEST_SECONDARY_USER_ID }} CLICKHOUSE_CLOUD_TEST_TIMEOUT_CREATE_SECS: ${{ vars.CLICKHOUSE_CLOUD_TEST_TIMEOUT_CREATE_SECS || '1800' }} CLICKHOUSE_CLOUD_TEST_TIMEOUT_DELETE_SECS: ${{ vars.CLICKHOUSE_CLOUD_TEST_TIMEOUT_DELETE_SECS || '900' }} CLICKHOUSE_CLOUD_TEST_TIMEOUT_STEADY_STATE_SECS: ${{ vars.CLICKHOUSE_CLOUD_TEST_TIMEOUT_STEADY_STATE_SECS || '1800' }} @@ -68,6 +69,9 @@ jobs: - name: Run cloud Postgres integration suite run: cargo test -p clickhouse-cloud-api --test integration_postgres_test -- --ignored --nocapture + - name: Run cloud Org integration suite + run: cargo test -p clickhouse-cloud-api --test integration_org_test -- --ignored --nocapture + - name: Run ClickPipe Postgres CDC integration test run: cargo test -p clickhouse-cloud-api --test clickpipe_postgres_cdc_test -- --ignored --nocapture diff --git a/CLAUDE.md b/CLAUDE.md index ddbf2d4..3b5bed5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -149,6 +149,6 @@ cargo run -p clickhousectl -- local client --query "SELECT 1" ## Testing model -- **`clickhouse-cloud-api`**: real cloud integration tests, target 100% OpenAPI spec coverage. Call `Client` directly from Rust — never via the CLI. Cost is not a reason to skip a test. Top-level binaries `tests/integration_*.rs`, shared handlers in `tests/common/`, ClickPipes-specific modules in `tests/clickpipes/`. +- **`clickhouse-cloud-api`**: real cloud integration tests, target 100% OpenAPI spec coverage. Call `Client` directly from Rust — never via the CLI. Cost is not a reason to skip a test. Top-level binaries `tests/integration_*.rs` (`integration_test` = ClickHouse service CRUD, `integration_postgres_test` = Postgres CRUD, `integration_org_test` = org-scoped endpoints — the org suite needs `CLICKHOUSE_CLOUD_TEST_SECONDARY_USER_ID`), shared handlers in `tests/common/`, ClickPipes-specific modules in `tests/clickpipes/`. - **`clickhousectl`**: request-shape tests via `wiremock` (`tests/cli_request_shape_test.rs`). Assert the JSON the CLI sends matches the library's request models — this is the CLI↔lib contract. Every new subcommand that builds a request body needs one. - **`spec_coverage_test.rs`**: structural floor — every spec operation/field has a matching client method/model field. diff --git a/README.md b/README.md index b52f3af..c75a71b 100644 --- a/README.md +++ b/README.md @@ -728,7 +728,11 @@ The CLI also checks for updates in the background (at most once per 24 hours) an ## Cloud integration testing -Cloud API integration is tested against a real ClickHouse Cloud workspace via the library crate. All changes to cloud commands must pass CI testing before merge. Tests are in [`crates/clickhouse-cloud-api/tests/integration_test.rs`](crates/clickhouse-cloud-api/tests/integration_test.rs). +Cloud API integration is tested against a real ClickHouse Cloud workspace via the library crate. All changes to cloud commands must pass CI testing before merge. Tests live in three binaries, each a single `#[tokio::test]` lifecycle: + +- [`tests/integration_test.rs`](crates/clickhouse-cloud-api/tests/integration_test.rs) — ClickHouse service CRUD + service-scoped endpoints +- [`tests/integration_postgres_test.rs`](crates/clickhouse-cloud-api/tests/integration_postgres_test.rs) — Postgres service CRUD +- [`tests/integration_org_test.rs`](crates/clickhouse-cloud-api/tests/integration_org_test.rs) — org-scoped endpoints (members, invitations, roles, activity, prometheus, private endpoint config) Required environment variables: @@ -738,12 +742,17 @@ export CLICKHOUSE_CLOUD_API_SECRET=... export CLICKHOUSE_CLOUD_TEST_ORG_ID=... export CLICKHOUSE_CLOUD_TEST_PROVIDER=aws export CLICKHOUSE_CLOUD_TEST_REGION=us-east-1 +# Required for the org integration suite (members + invitations need a +# second user in the test org); optional otherwise. +export CLICKHOUSE_CLOUD_TEST_SECONDARY_USER_ID=... ``` -Run the integration test: +Run a suite: ```bash -cargo test -p clickhouse-cloud-api --test integration_test -- --ignored --nocapture +cargo test -p clickhouse-cloud-api --test integration_test -- --ignored --nocapture +cargo test -p clickhouse-cloud-api --test integration_postgres_test -- --ignored --nocapture +cargo test -p clickhouse-cloud-api --test integration_org_test -- --ignored --nocapture ``` By default, any failed check fails the run. To keep going after `non-blocking` capability failures and collect them in a summary at the end, set: diff --git a/crates/clickhouse-cloud-api/tests/common/support.rs b/crates/clickhouse-cloud-api/tests/common/support.rs index 15f13c1..ee3f568 100644 --- a/crates/clickhouse-cloud-api/tests/common/support.rs +++ b/crates/clickhouse-cloud-api/tests/common/support.rs @@ -26,6 +26,11 @@ pub struct TestContext { pub provider: String, pub region: String, pub run_id: String, + /// Existing second user in the test org. Only required by the org + /// integration suite (members, invitations); other suites leave it unset. + /// Use [`TestContext::secondary_user_id`] to get the value with a + /// suite-specific error message. + pub secondary_user_id: Option, pub create_timeout: Duration, pub delete_timeout: Duration, pub steady_state_timeout: Duration, @@ -40,6 +45,10 @@ impl fmt::Debug for TestContext { .field("provider", &self.provider) .field("region", &self.region) .field("run_id", &self.run_id) + .field( + "secondary_user_id", + &self.secondary_user_id.as_ref().map(|_| ""), + ) .field("create_timeout", &self.create_timeout) .field("delete_timeout", &self.delete_timeout) .field("steady_state_timeout", &self.steady_state_timeout) @@ -75,6 +84,7 @@ impl TestContext { provider: required_env("CLICKHOUSE_CLOUD_TEST_PROVIDER")?, region: required_env("CLICKHOUSE_CLOUD_TEST_REGION")?, run_id, + secondary_user_id: optional_env("CLICKHOUSE_CLOUD_TEST_SECONDARY_USER_ID"), create_timeout: duration_from_env( "CLICKHOUSE_CLOUD_TEST_TIMEOUT_CREATE_SECS", DEFAULT_CREATE_TIMEOUT_SECS, @@ -157,6 +167,43 @@ impl TestContext { ] } + /// Returns the secondary user id, erroring if the env var is not set. + /// Call this from the org integration suite (members, invitations) where + /// the fixture is mandatory. + pub fn secondary_user_id(&self) -> TestResult<&str> { + self.secondary_user_id.as_deref().ok_or_else(|| { + "CLICKHOUSE_CLOUD_TEST_SECONDARY_USER_ID is required for the org integration \ + suite — set it to the id of an existing second user in the test org" + .into() + }) + } + + /// Tags applied to org-scoped resources that support tagging. + pub fn org_run_tags(&self) -> Vec { + vec![ + ResourceTagsV1 { + key: "managed_by".to_string(), + value: Some("clickhousectl_it".to_string()), + }, + ResourceTagsV1 { + key: "suite".to_string(), + value: Some("org".to_string()), + }, + ResourceTagsV1 { + key: "run_id".to_string(), + value: Some(self.run_id.clone()), + }, + ] + } + + pub fn org_run_tag_filters(&self) -> Vec { + vec![ + "tag:managed_by=clickhousectl_it".to_string(), + "tag:suite=org".to_string(), + format!("tag:run_id={}", self.run_id), + ] + } + pub fn clickpipe_service_name(&self) -> String { format!("clickhousectl-it-cp-{}", self.run_id) } @@ -372,6 +419,8 @@ pub struct CleanupRegistry { /// being deleted, table drops are redundant but harmless. tables: Vec, api_key_ids: Vec, + role_ids: Vec, + invitation_ids: Vec, } impl CleanupRegistry { @@ -433,6 +482,23 @@ impl CleanupRegistry { .retain(|registered| registered != key_id); } + pub fn register_role(&mut self, role_id: impl Into) { + self.role_ids.push(role_id.into()); + } + + pub fn unregister_role(&mut self, role_id: &str) { + self.role_ids.retain(|registered| registered != role_id); + } + + pub fn register_invitation(&mut self, invitation_id: impl Into) { + self.invitation_ids.push(invitation_id.into()); + } + + pub fn unregister_invitation(&mut self, invitation_id: &str) { + self.invitation_ids + .retain(|registered| registered != invitation_id); + } + pub async fn cleanup( &mut self, client: &Client, @@ -443,7 +509,25 @@ impl CleanupRegistry { ) -> Result<(), String> { let mut failures = Vec::new(); - // API keys are cleaned up first; they belong to the org, not a + // Invitations and roles are org-scoped and cheap to delete; clear + // them first so a botched service teardown doesn't leak them. + while let Some(invitation_id) = self.invitation_ids.pop() { + match client.invitation_delete(org_id, &invitation_id).await { + Ok(_) => {} + Err(clickhouse_cloud_api::Error::Api { status: 404, .. }) => {} + Err(e) => failures.push(format!("invitation {invitation_id}: {e}")), + } + } + + while let Some(role_id) = self.role_ids.pop() { + match client.organization_role_delete(org_id, &role_id).await { + Ok(_) => {} + Err(clickhouse_cloud_api::Error::Api { status: 404, .. }) => {} + Err(e) => failures.push(format!("role {role_id}: {e}")), + } + } + + // API keys are cleaned up next; they belong to the org, not a // specific service, so they outlive service deletion if leaked. while let Some(key_id) = self.api_key_ids.pop() { match client.openapi_key_delete(org_id, &key_id).await { @@ -1055,3 +1139,10 @@ impl ClickHouseQuery { } } } + +fn optional_env(name: &str) -> Option { + match env::var(name) { + Ok(value) if !value.is_empty() => Some(value), + _ => None, + } +} diff --git a/crates/clickhouse-cloud-api/tests/integration_org_test.rs b/crates/clickhouse-cloud-api/tests/integration_org_test.rs new file mode 100644 index 0000000..ee095fe --- /dev/null +++ b/crates/clickhouse-cloud-api/tests/integration_org_test.rs @@ -0,0 +1,142 @@ +mod common; + +use common::support::*; + +/// Org-scoped live integration suite. +/// +/// Single `#[tokio::test]` lifecycle that exercises org-level endpoints +/// (members, invitations, custom roles, activity, prometheus, private +/// endpoint config, openapi keys) without provisioning a ClickHouse or +/// Postgres service. The shape mirrors `integration_test.rs` and +/// `integration_postgres_test.rs`: +/// +/// - `TestContext::from_env()` builds the shared run config. +/// - `FailureRecorder` accumulates non-blocking failures so one CI run +/// reports every broken endpoint, not just the first. +/// - `CleanupRegistry` records every created resource so teardown runs +/// even if the test body panics. +/// +/// Test phases land in issues #153 through #157 — this file currently +/// holds only the lifecycle scaffold and an org access sanity check. +#[tokio::test] +#[ignore = "requires live ClickHouse Cloud credentials and a secondary user fixture"] +async fn cloud_org_lifecycle() -> TestResult<()> { + let ctx = TestContext::from_env()?; + let client = create_client()?; + let mut cleanup = CleanupRegistry::default(); + + let test_result = async { + log_run_header("cloud_org_lifecycle", &ctx); + let mut failures = FailureRecorder::default(); + + // ── Org Access ────────────────────────────────────────────── + // + // Confirm the API key can reach the configured org before any + // downstream phase tries to mutate org-scoped resources. Phase + // bodies for members, invitations, roles, activity, prometheus + // and private endpoint config are added in #153–#157. + + log_phase("Org Access"); + let org = failures + .run(&ctx, StepKind::Blocking, "verify org access", || { + let client = client.clone(); + let org_id = ctx.org_id.clone(); + async move { + let resp = client.organization_get(&org_id).await?; + resp.result + .ok_or_else(|| "org get returned no result".into()) + } + }) + .await? + .expect("blocking steps always return a value"); + assert_eq!(org.id.to_string(), ctx.org_id); + + // ── Org Observability ─────────────────────────────────────── + // + // Read-only checks against org-scoped endpoints that don't + // require any fixture beyond the org itself. Both steps are + // NonBlocking — they exist purely to detect live API drift. + + log_phase("Org Observability"); + + failures + .run(&ctx, StepKind::NonBlocking, "organization prometheus", || { + let client = client.clone(); + let org_id = ctx.org_id.clone(); + async move { + // The org-level prometheus exporter returns empty + // output when no service in the org is emitting metrics + // at request time. This suite deliberately does not + // provision a service, so empty output is a valid + // response — coverage here is that the call succeeds. + // The service-level prometheus endpoint is covered with + // a non-empty assertion in integration_test.rs. + let _metrics = client.organization_prometheus_get(&org_id, None).await?; + Ok(()) + } + }) + .await?; + + failures + .run( + &ctx, + StepKind::NonBlocking, + "organization private endpoint config list", + || { + let client = client.clone(); + let org_id = ctx.org_id.clone(); + let cloud_provider = ctx.provider.clone(); + let region_id = ctx.region.clone(); + async move { + // Deprecated endpoint. The API requires an existing + // instance in the requested provider+region before + // it returns a config, but this suite is + // deliberately service-less. Treat the + // "no created instances" 400 as the expected + // response: it still proves auth, routing and the + // 400 deserialization path. Any other response + // (including a 200) is fine too — the integration + // service suite covers the populated path with a + // real instance. + #[allow(deprecated)] + let result = client + .organization_private_endpoint_config_get_list( + &org_id, + &cloud_provider, + ®ion_id, + ) + .await; + match result { + Ok(_) => Ok(()), + Err(clickhouse_cloud_api::Error::Api { status: 400, message }) + if message.contains("no created instances") => + { + eprintln!( + " expected 400 (no instances in region) — endpoint reachable" + ); + Ok(()) + } + Err(e) => Err(e.into()), + } + } + }, + ) + .await?; + + failures.finish() + } + .await; + + let cleanup_result = cleanup + .cleanup(&client, &ctx.org_id, ctx.delete_timeout, ctx.poll_interval, None) + .await; + + match (test_result, cleanup_result) { + (Ok(()), Ok(())) => Ok(()), + (Err(error), Ok(())) => Err(error), + (Ok(()), Err(cleanup_error)) => Err(cleanup_error.into()), + (Err(error), Err(cleanup_error)) => { + Err(format!("{error}\ncleanup failed:\n{cleanup_error}").into()) + } + } +}