Skip to content

Make Query API provisioning single-flight across processes - #493

Open
sdairs wants to merge 2 commits into
issue-455-query-input-conflictfrom
issue-451-query-provision-singleflight
Open

Make Query API provisioning single-flight across processes#493
sdairs wants to merge 2 commits into
issue-455-query-input-conflictfrom
issue-451-query-provision-singleflight

Conversation

@sdairs

@sdairs sdairs commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • serialize Query API auto-provisioning with a project-local cross-process lock
  • re-read and merge credentials under the lock, then atomically replace credentials.json; waiting processes reuse the persisted service credential
  • delete the exact management API key when post-create binding or persistence fails
  • add deterministic six-process wiremock coverage for one key creation, one endpoint binding, valid merged credentials, and atomic replacement
  • document the residual cross-project lost-update risk because endpoint upsert replaces the complete openApiKeys list without conditional updates

Closes #451

Tests

  • cargo test -p clickhousectl --test cli_request_shape_test service_query_auto_provisioning_is_single_flight_across_processes -- --exact (1 passed)
  • cargo test -p clickhousectl --test cli_request_shape_test service_query_deletes_the_exact_key_when_atomic_persistence_fails -- --exact (1 passed)
  • cargo test -p clickhousectl --bin clickhousectl service_query (21 passed)
  • cargo test -p clickhousectl --test cli_request_shape_test service_query (24 passed)
  • cargo test -p clickhousectl --bin clickhousectl (609 passed)
  • cargo test -p clickhousectl --test cli_request_shape_test (104 passed)
  • cargo fmt --all --check
  • cargo clippy -p clickhousectl --all-targets -- -D warnings

No live Cloud resources or credentials were used.

Stack

This is child 3 of the Cloud-query stack. It targets issue-455-query-input-conflict (PR #490), above issue-452-service-query-selector (PR #483). Both parent PRs remain unchanged and should merge first.

@sdairs sdairs changed the title issue 451 query provision singleflight Make Query API provisioning single-flight across processes Aug 24, 2026
creds
.service_query_keys
.insert(service_id.to_string(), stored.clone());
if let Err(error) = credentials::save_credentials(&creds) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High cloud/service_query.rs:144

save_credentials(&creds) writes the stale snapshot captured before the network awaits, so concurrent updates from remove_service_query_key or authentication flows are overwritten, potentially resurrecting removed keys or reverting newly saved credentials. Atomic replacement only prevents partial writes; make all credential writers use this lock, or re-read and merge credentials.json immediately before saving.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @crates/clickhousectl/src/cloud/service_query.rs around line 144:

`save_credentials(&creds)` writes the stale snapshot captured before the network awaits, so concurrent updates from `remove_service_query_key` or authentication flows are overwritten, potentially resurrecting removed keys or reverting newly saved credentials. Atomic replacement only prevents partial writes; make all credential writers use this lock, or re-read and merge `credentials.json` immediately before saving.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a2dd2a6. Configure here.

if let Err(error) = credentials::save_credentials(&creds) {
discard_api_key(client, org_id, &api_key_uuid).await;
return Err(error);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale credentials overwrite concurrent writers

High Severity

ensure_service_query_setup loads credentials.json once under the provision lock, then keeps that snapshot across create_api_key and bind_query_endpoint awaits before save_credentials. Auth login, logout, and remove_service_query_key still write the same file without that lock, so a later save can silently revert new API keys or resurrect deleted service_query_keys entries. Previously set_service_query_key reloaded immediately before writing.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a2dd2a6. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant