Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ The user-facing CLI surface. Contains all logic for local commands, wraps `click

- Cloud handlers go through the `CloudClient` wrapper (`src/cloud/client.rs`), not `clickhouse_cloud_api::Client` directly. The wrapper handles credential precedence, error conversion, and response unwrapping.
- Cloud handlers always support `--json` output unless there is good reason not to. JSON is emitted automatically when `--json` is passed or a coding agent is detected (`is_ai_agent::detect()` via the `json_output()` helper in `main.rs`).
- `CloudError` carries a `kind: CloudErrorKind` (`Auth` for 401/403 and missing credentials, else `Generic`). It maps to `Error::AuthRequired` / `Error::Cloud` in `main.rs`. Dispatched commands exit with `0` on success or use `Error::exit_code()` for failures: `1` error, `3` cancelled, `4` auth required. Clap uses `2` for usage errors.
- `CloudError` carries a `kind: CloudErrorKind` (`Auth` for 401/403 and missing credentials, else `Generic`). It maps to `Error::AuthRequired` / `Error::Cloud` in `cloud::run`. Dispatched commands exit with `0` on success or use `Error::exit_code()` for failures: `1` error, `3` cancelled, `4` auth required. Clap uses `2` for usage errors.

Use `--help` to learn the current command surface.

Expand All @@ -22,7 +22,7 @@ The CLI does not need to have 100% coverage of endpoints exposed by the API libr

#### Adding a command

For both local and cloud commands, define the clap variant in the appropriate `cli.rs`, then wire dispatch in `src/main.rs`.
For both local and cloud commands, define the clap variant in the appropriate `cli.rs`, then wire dispatch in the owning runtime module.

**Local subcommand:**

Expand All @@ -35,7 +35,7 @@ For both local and cloud commands, define the clap variant in the appropriate `c
1. Make sure `clickhouse-cloud-api` has already been updated to support necessary endpoints & models.
2. Add the variant to the relevant sub-enum in `src/cloud/cli.rs` (or `src/cloud/postgres.rs` for Postgres). Create a new sub-enum if the surface warrants its own grouping.
3. Classify the new variant in `CloudCommands::is_write_command()` in `src/cloud/cli.rs` (Postgres variants go in the equivalent `is_write()` on the Postgres enum). OAuth (Bearer) auth is read-only; write commands require API key auth and we fail fast on OAuth + write. The match has no wildcards, so the compiler will reject a missing arm — but you still need to make the read/write call deliberately, and add a case to both the `is_write_command_read_only_commands` and `is_write_command_destructive_commands` tests.
4. Add the match arm in `run_cloud()` in `src/main.rs`.
4. Add non-Postgres match arms to `cloud::dispatch()` in `src/cloud/mod.rs`. Postgres dispatch belongs in `src/cloud/postgres.rs::run`; if another domain later gains its own runtime dispatcher, have `cloud::dispatch()` delegate to it rather than keeping that domain's match arms centrally.
5. Add a thin wrapper method on `CloudClient` in `src/cloud/client.rs`. It should delegate to `self.api().<lib_method>()`, map errors via `self.convert_error(e)`, and unwrap with `Self::unwrap_response`. Use the library's request/response types here.
6. If the command sends a request body, extract a `build_<name>_request(...)` helper in `src/cloud/commands.rs` that returns the library's request struct. Cover the helper with minimal + maximal unit tests in the `mod tests` block at the bottom of `commands.rs`, asserting directly on library struct fields.
7. Implement the handler in `src/cloud/commands.rs`. For body-sending commands the handler calls the build helper, passes the result through the `CloudClient` wrapper, and prints with the `--json` output pattern. For detail/get views (rendering a single resource), drive human output through `print_human` so it shares serde's behaviour — including deprecated-field hiding — instead of hand-writing `println!` lines:
Expand Down
11 changes: 1 addition & 10 deletions crates/clickhousectl/src/cli.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,6 @@
use clap::{Args, Parser, Subcommand};

pub use crate::cloud::cli::{
ActivityCommands, AuthCommands, BackupCommands, BackupConfigCommands, ClickPipeCommands,
ClickPipeCreateCommands, ClickPipeSettingsCommands, CloudArgs, CloudCommands,
InvitationCommands, KeyCommands, MemberCommands, OrgCommands, PrivateEndpointCommands,
QueryEndpointCommands, ServiceCommands,
};
pub use crate::cloud::postgres::{
CertsCommands as PostgresCertsCommands, ConfigCommands as PostgresConfigCommands,
PostgresCommands, ReadReplicaCommands as PostgresReadReplicaCommands,
};
use crate::cloud::cli::CloudArgs;
pub use crate::local::cli::LocalArgs;

#[derive(Parser)]
Expand Down
28 changes: 1 addition & 27 deletions crates/clickhousectl/src/cloud/cli.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use chrono::{DateTime, FixedOffset, NaiveDate, NaiveTime};
use crate::cloud::shared::{parse_date_only, parse_datetime, parse_time_only};
use clap::builder::PossibleValuesParser;
use clap::{Args, Subcommand};

Expand Down Expand Up @@ -74,32 +74,6 @@ const MONGODB_READ_PREFERENCES: &[&str] = &[
"nearest",
];

fn parse_date_only(value: &str) -> Result<String, String> {
if NaiveDate::parse_from_str(value, "%Y-%m-%d").is_err() {
return Err(format!("invalid date '{}': expected YYYY-MM-DD", value));
}

Ok(value.to_string())
}

pub(super) fn parse_datetime(value: &str) -> Result<String, String> {
if DateTime::<FixedOffset>::parse_from_rfc3339(value).is_err() {
return Err(format!(
"invalid datetime '{}': expected ISO 8601 / RFC 3339",
value
));
}

Ok(value.to_string())
}

fn parse_time_only(value: &str) -> Result<String, String> {
if NaiveTime::parse_from_str(value, "%H:%M").is_err() {
return Err(format!("invalid time '{}': expected HH:MM", value));
}

Ok(value.to_string())
}
#[derive(Subcommand)]
pub enum AuthCommands {
/// Log in to ClickHouse Cloud
Expand Down
103 changes: 7 additions & 96 deletions crates/clickhousectl/src/cloud/commands.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
use crate::cloud::client::{CloudClient, CloudError};
use crate::cloud::credentials;
use crate::cloud::output::{ABSENT, or_absent, print_human};
use crate::cloud::shared::{parse_serde_enum, parse_tags, resolve_org_id};
use clickhouse_cloud_api::models::{
ApiKeyPatchRequest, ApiKeyPatchRequestState, ApiKeyPostRequest, ApiKeyPostRequestState,
AutoscalingMode, BackupConfigurationPatchRequest, InstancePrivateEndpointsPatch,
InstanceServiceQueryApiEndpointsPostRequest, InstanceTagsPatch, IpAccessListEntry,
IpAccessListPatch, OrganizationPatchPrivateEndpoint,
OrganizationPatchPrivateEndpointCloudprovider, OrganizationPatchPrivateEndpointRegion,
OrganizationPatchRequest, OrganizationPrivateEndpointsPatch, ResourceTagsV1,
ServicPrivateEndpointePostRequest, Service, ServiceEndpoint, ServiceEndpointChange,
ServiceEndpointChangeProtocol, ServicePasswordPatchRequest, ServicePatchRequest,
ServicePatchRequestReleasechannel, ServicePostRequest, ServicePostRequestCompliancetype,
ServicePostRequestProfile, ServicePostRequestProvider, ServicePostRequestRegion,
ServicePostRequestReleasechannel, ServiceReplicaScalingPatchRequest, ServiceState,
ServiceStatePatchRequestCommand,
OrganizationPatchRequest, OrganizationPrivateEndpointsPatch, ServicPrivateEndpointePostRequest,
Service, ServiceEndpoint, ServiceEndpointChange, ServiceEndpointChangeProtocol,
ServicePasswordPatchRequest, ServicePatchRequest, ServicePatchRequestReleasechannel,
ServicePostRequest, ServicePostRequestCompliancetype, ServicePostRequestProfile,
ServicePostRequestProvider, ServicePostRequestRegion, ServicePostRequestReleasechannel,
ServiceReplicaScalingPatchRequest, ServiceState, ServiceStatePatchRequestCommand,
};
use std::io::{IsTerminal, Write};
use tabled::{Table, Tabled, settings::Style};
Expand Down Expand Up @@ -46,17 +46,6 @@ fn first_endpoint(endpoints: Option<&[ServiceEndpoint]>) -> String {
.unwrap_or_else(|| ABSENT.to_string())
}

/// Resolve org ID from explicit arg or auto-detect
pub(super) async fn resolve_org_id(
client: &CloudClient,
org_id: Option<&str>,
) -> Result<String, Box<dyn std::error::Error>> {
match org_id {
Some(id) => Ok(id.to_string()),
None => Ok(client.get_default_org_id().await?),
}
}

/// Resolve a service by name or ID within the given org.
/// Exactly one of `name` or `id` must be provided.
async fn resolve_service(
Expand Down Expand Up @@ -88,69 +77,6 @@ async fn resolve_service(
}
}

/// Parse a string into a library enum via serde deserialization, with client-side
/// validation against a known-values list. Library enums have an `Unknown(String)`
/// catch-all that prevents serde from ever failing, so we validate first.
pub(super) fn parse_serde_enum<T: serde::de::DeserializeOwned>(
value: &str,
field: &str,
known_values: &[&str],
) -> Result<T, Box<dyn std::error::Error>> {
if !known_values.contains(&value) {
return Err(format!(
"invalid {}: unknown value '{}', expected one of: {}",
field,
value,
known_values.join(", ")
)
.into());
}
serde_json::from_value(serde_json::Value::String(value.to_string()))
.map_err(|e| format!("invalid {}: {}", field, e).into())
}

pub(super) fn parse_tag(value: &str) -> Result<ResourceTagsV1, Box<dyn std::error::Error>> {
match value.split_once('=') {
Some((key, tag_value)) => {
let key = key.trim();
if key.is_empty() {
Err(format!("invalid tag '{}': tag key cannot be empty", value).into())
} else {
Ok(ResourceTagsV1 {
key: key.to_string(),
value: Some(tag_value.to_string()),
})
}
}
None => {
let key = value.trim();
if key.is_empty() {
Err(format!("invalid tag '{}': tag key cannot be empty", value).into())
} else {
Ok(ResourceTagsV1 {
key: key.to_string(),
value: None,
})
}
}
}
}

pub(super) fn parse_tags(
values: &[String],
) -> Result<Option<Vec<ResourceTagsV1>>, Box<dyn std::error::Error>> {
if values.is_empty() {
Ok(None)
} else {
Ok(Some(
values
.iter()
.map(|value| parse_tag(value))
.collect::<Result<Vec<_>, _>>()?,
))
}
}

fn parse_ip_access_entries(values: &[String]) -> Option<Vec<IpAccessListEntry>> {
(!values.is_empty()).then(|| {
values
Expand Down Expand Up @@ -3869,21 +3795,6 @@ mod tests {
);
}

#[test]
fn parse_tag_rejects_empty_keys() {
let err = parse_tag("=value").unwrap_err();
assert_eq!(
err.to_string(),
"invalid tag '=value': tag key cannot be empty"
);

let err = parse_tag(" ").unwrap_err();
assert_eq!(
err.to_string(),
"invalid tag ' ': tag key cannot be empty"
);
}

#[test]
fn build_create_service_request_supports_ga_optional_fields() {
let opts = CreateServiceOptions {
Expand Down
Loading