Skip to content
Open
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
23 changes: 23 additions & 0 deletions apps/piquelctl/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,19 @@ pub(crate) struct Cli {
#[arg(long, global = true, help_heading = GLOBAL)]
pub(crate) noninteractive: bool,

/// Make a change to an application, environment or preview safe to run
/// again: it is made only once.
///
/// Run the same command with the same ID, such as after losing the
/// connection, and the daemon returns the earlier result instead of
/// making the change again, within 24 hours. Secret, notification,
/// account, token and sign-in commands, and `preview prune`, ignore it,
/// so rerunning one makes its change again. Use a new ID, of up to 128 letters, digits, `-`,
/// `_`, `.` or `:`, for each command you mean to run. Defaults to
/// `PIQUELD_REQUEST_ID`, else a new ID each time.
#[arg(long, global = true, value_name = "ID", value_parser = parse_request_id, help_heading = GLOBAL)]
pub(crate) request_id: Option<String>,

#[command(subcommand)]
pub(crate) command: Command,
}
Expand Down Expand Up @@ -674,6 +687,16 @@ pub(crate) struct OperationArgs {
pub(crate) no_wait: bool,
}

/// Accepts a request ID the daemon accepts: 1 to 128 ASCII letters, digits,
/// `-`, `_`, `.` or `:`.
pub(crate) fn parse_request_id(value: &str) -> std::result::Result<String, String> {
if piqueld_client::is_request_id(value) {
Ok(value.to_owned())
} else {
Err("use 1 to 128 letters, digits, `-`, `_`, `.` or `:`".into())
}
}

/// Parses a positive integer duration with an optional unit (`ms`, `s`, `m`, `h`,
/// `d`; bare numbers are seconds). Used for `--timeout`, `PIQUELD_TIMEOUT`,
/// profiles, and the `analytics` window.
Expand Down
19 changes: 14 additions & 5 deletions apps/piquelctl/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,9 @@ async fn build_logs(

/// Builds the API client from resolved connection settings: TCP for `--url`, else the
/// Unix socket (default path if unset), plus insecure-HTTP opt-in, saved or
/// `PIQUELD_TOKEN` credentials, the per-request timeout, and a fresh request ID.
/// Fails on invalid endpoint input before any request is sent.
/// `PIQUELD_TOKEN` credentials, the per-request timeout, and the request ID:
/// `--request-id`, `PIQUELD_REQUEST_ID`, or a fresh one. Fails on invalid
/// endpoint input or request ID before any request is sent.
pub(crate) fn build_client(cli: &Cli) -> Result<Client> {
let client = match &cli.url {
Some(url) => Client::tcp(url)?,
Expand All @@ -217,9 +218,17 @@ pub(crate) fn build_client(cli: &Cli) -> Result<Client> {
client
};
let client = crate::auth::Credentials::attach(cli, client)?;
Ok(client
.with_timeout(cli.timeout)
.with_request_id(uuid::Uuid::now_v7().to_string()))
let request_id = match (&cli.request_id, std::env::var("PIQUELD_REQUEST_ID")) {
(Some(id), _) => id.clone(),
(None, Ok(id)) => crate::cli::parse_request_id(&id).map_err(|problem| {
CliError::new(
ErrorKind::Input,
format!("invalid PIQUELD_REQUEST_ID: {problem}"),
)
})?,
(None, Err(_)) => uuid::Uuid::now_v7().to_string(),
};
Ok(client.with_timeout(cli.timeout).with_request_id(request_id))
}

/// Reports daemon status together with the transport that was used.
Expand Down
24 changes: 23 additions & 1 deletion apps/piquelctl/src/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,22 @@ impl Worktree {
));
}
let message = format!("piquelctl snapshot of {branch}");
// Dated like `HEAD`, so the same worktree always makes the same
// commit, and rerunning an upload with its `--request-id` replays.
let date = Git::run(
Git::command(&self.root).args(["show", "-s", "--format=%cd", "--date=raw", &head]),
"read HEAD's date",
)
.await?;
let commit = Git::run(
snapshot
.staged(&["commit-tree", &tree, "-p", &head, "-m", &message])
.env("GIT_AUTHOR_NAME", "piquelctl")
.env("GIT_AUTHOR_EMAIL", "snapshot@piquelctl.invalid")
.env("GIT_AUTHOR_DATE", &date)
.env("GIT_COMMITTER_NAME", "piquelctl")
.env("GIT_COMMITTER_EMAIL", "snapshot@piquelctl.invalid"),
.env("GIT_COMMITTER_EMAIL", "snapshot@piquelctl.invalid")
.env("GIT_COMMITTER_DATE", &date),
"commit the snapshot",
)
.await?;
Expand Down Expand Up @@ -759,6 +768,19 @@ mod tests {
root.join("work")
}

/// The same worktree snapshots to the same commit, so a rerun can replay.
#[tokio::test]
async fn snapshots_of_an_unchanged_worktree_are_the_same_commit() {
let root = tempfile::tempdir().unwrap();
let work = fixture(root.path());
let worktree = Worktree::at(&work).await.unwrap();
let first = worktree.snapshot("feat/x", None).await.unwrap().commit;
// Commit dates have one-second resolution.
tokio::time::sleep(std::time::Duration::from_millis(1100)).await;
let second = worktree.snapshot("feat/x", None).await.unwrap().commit;
assert_eq!(first, second);
}

#[tokio::test]
async fn snapshots_include_untracked_files_and_leave_the_repository_untouched() {
let root = tempfile::tempdir().unwrap();
Expand Down
4 changes: 4 additions & 0 deletions apps/piquelctl/tests/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,8 @@ fn apply_deploy_retries_a_dropped_request_with_the_same_identity() {
let output = run(
&server,
&[
"--request-id",
"agent-run-7",
"app",
"apply",
"--deploy",
Expand All @@ -901,6 +903,8 @@ fn apply_deploy_retries_a_dropped_request_with_the_same_identity() {
.headers
.get("idempotency-key")
.expect("command has a replay identity");
// Sent as is, so rerunning the command replays it.
assert_eq!(key, "agent-run-7");
assert_eq!(Some(key), second.headers.get("idempotency-key"));
}

Expand Down
9 changes: 1 addition & 8 deletions apps/piqueld/src/api/service/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,7 @@ impl ApplicationService {
if !force && mutation.lacks_precondition(expected_generation) {
return Err(ApplicationError::PreconditionRequired);
}
// Request IDs are idempotency keys: 1–128 chars of `[A-Za-z0-9-_.:]`.
if request_id.is_some_and(|id| {
id.is_empty()
|| id.len() > 128
|| !id
.bytes()
.all(|b| b.is_ascii_alphanumeric() || b"-_.:".contains(&b))
}) {
if request_id.is_some_and(|id| !piqueld_core::is_request_id(id)) {
return Err(StoreError::InvalidInput.into());
}
if let Mutation::Rename { name, .. } = mutation
Expand Down
8 changes: 1 addition & 7 deletions apps/piqueld/src/api/service/promotions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,7 @@ impl ApplicationService {
Self::check_submission(&request(None), expected_generation, force, request_id)?;
let replayed = self
.store
.replay(
actor,
&request(None),
expected_generation,
force,
request_id,
)
.replay(actor, &request(None), force, request_id)
.await?;
if let Some(response) = replayed {
return Self::promotion_response(response);
Expand Down
2 changes: 1 addition & 1 deletion apps/piqueld/src/api/service/restores.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl ApplicationService {
// A repeated request returns what it got, whatever changed since.
if let Some(response) = self
.store
.replay(actor, &request, None, false, request_id)
.replay(actor, &request, false, request_id)
.await?
{
return Self::restore_response(response);
Expand Down
29 changes: 29 additions & 0 deletions apps/piqueld/tests/api_contract/acceptance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,35 @@ async fn generations_deploy_reconcile_and_event_pagination_share_the_http_contra
);
}

/// Rerunning a command with its request ID returns what the first run got,
/// even though the rerun inspected the preconditions again, after the first
/// run changed them, and never applies the change twice. Other content under
/// the same ID still conflicts.
#[tokio::test]
async fn a_rerun_with_the_same_request_id_replays_its_acceptance() {
let temp = tempfile::tempdir().unwrap();
let api = AcceptanceApi::start(&temp).await;
let request = new_application();
let keyed = api.client.clone().with_request_id("agent-run-7");
let accepted = keyed.apply_and_deploy(&request).await.unwrap();
let mut rerun = request.clone();
rerun.expected_generation = Some(accepted.generation);
rerun.expected_application_id = Some(accepted.application_id.clone());
let replayed = keyed.apply_and_deploy(&rerun).await.unwrap();
assert_eq!(replayed.operation_id, accepted.operation_id);
assert_eq!(
api.client
.application(&accepted.application_id)
.await
.unwrap()
.generation,
accepted.generation
);
rerun.manifest.spec.services[0].replicas = 2.into();
let error = keyed.apply_and_deploy(&rerun).await.unwrap_err();
assert_eq!(api_error(error).code, "request_id_conflict");
}

#[tokio::test]
async fn acceptance_receipts_survive_restart_and_supersession() {
let temp = tempfile::tempdir().unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/piqueld-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ pub use piqueld_core::planner::{ActionReason, ActionRisk};
pub use piqueld_core::{
ApplicationId, ApplicationName, BuildFingerprint, BuildInputs, EnvironmentId, Release,
ReleaseAvailability, ReleaseId, ResolvedSource, RouteName, ServiceImage, ServiceName,
TomlDiagnostic, ValidatedApplication, ValidationError, ValidationErrors,
TomlDiagnostic, ValidatedApplication, ValidationError, ValidationErrors, is_request_id,
};
pub use piqueld_core::{
ApplicationState, Convergence, Event, EventActor, Operation, OperationKind, OperationState,
Expand Down
2 changes: 1 addition & 1 deletion crates/piqueld-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ pub use names::{
ApplicationName, ApplicationNameError, EnvironmentName, EnvironmentNameError, GitBranch,
GitBranchError, JobName, JobNameError, PreviewSlot, PreviewSlotError, PreviewSlug,
PreviewSlugError, RouteName, RouteNameError, ServiceName, ServiceNameError, VolumeName,
VolumeNameError,
VolumeNameError, is_request_id,
};
pub use operation::{ApplicationState, Operation, OperationKind, OperationState};
pub use planner::{
Expand Down
10 changes: 10 additions & 0 deletions crates/piqueld-core/src/names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,16 @@ pub fn is_label(value: &str, lengths: RangeInclusive<usize>) -> bool {
.all(|byte| byte.is_ascii_alphanumeric() || byte == b'-')
}

/// Whether `value` can be a request ID, the `Idempotency-Key` of a
/// mutation: 1 to 128 ASCII letters, digits, `-`, `_`, `.` or `:`.
#[must_use]
pub fn is_request_id(value: &str) -> bool {
(1..=128).contains(&value.len())
&& value
.bytes()
.all(|byte| byte.is_ascii_alphanumeric() || b"-_.:".contains(&byte))
}

/// [`is_label`] without uppercase letters: the form of piqueld's IDs, names,
/// and hostnames.
pub(crate) fn is_lowercase_label(value: &str, lengths: RangeInclusive<usize>) -> bool {
Expand Down
Loading
Loading