Skip to content

Commit 452104f

Browse files
authored
Merge pull request #709 from obeli-sk/deployment-idempotent
refactor(api,cli): Add optional deployment ID for idempotent submission
2 parents 0e7526d + 2c9b894 commit 452104f

10 files changed

Lines changed: 221 additions & 3 deletions

File tree

assets/schemas/cli.json

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,10 @@
644644
"name": "--description",
645645
"help": "Optional human-readable description"
646646
},
647+
{
648+
"name": "--deployment-id",
649+
"help": "Optional client-supplied deployment ID for idempotent submission"
650+
},
647651
{
648652
"name": "--api-url",
649653
"short": "a",
@@ -676,6 +680,10 @@
676680
"name": "--description",
677681
"help": "Optional human-readable description for a newly submitted deployment"
678682
},
683+
{
684+
"name": "--deployment-id",
685+
"help": "Optional client-supplied deployment ID for idempotent submission"
686+
},
679687
{
680688
"name": "--api-url",
681689
"short": "a",
@@ -701,6 +709,10 @@
701709
"name": "--description",
702710
"help": "Optional human-readable description for a newly submitted deployment"
703711
},
712+
{
713+
"name": "--deployment-id",
714+
"help": "Optional client-supplied deployment ID for idempotent submission"
715+
},
704716
{
705717
"name": "--api-url",
706718
"short": "a",
@@ -1010,6 +1022,17 @@
10101022
}
10111023
]
10121024
},
1025+
{
1026+
"name": "deployment-id",
1027+
"about": "Generate a fresh random deployment ID and print it to stdout",
1028+
"options": [
1029+
{
1030+
"name": "--json",
1031+
"short": "j",
1032+
"help": "Output as JSON instead of plain text"
1033+
}
1034+
]
1035+
},
10131036
{
10141037
"name": "prompt",
10151038
"about": "Print a prompt context for authoring an Obelisk application with a coding agent",

assets/schemas/openapi.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2087,6 +2087,13 @@
20872087
"type": "string",
20882088
"description": "Deployment config as JSON string"
20892089
},
2090+
"deployment_id": {
2091+
"type": [
2092+
"string",
2093+
"null"
2094+
],
2095+
"description": "Optional client-supplied deployment ID for idempotent submission.\nIf a deployment with this ID already exists and its content digest\nmatches, the submission is a no-op; a digest mismatch is rejected."
2096+
},
20902097
"description": {
20912098
"type": [
20922099
"string",

proto/obelisk.proto

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,6 +1100,11 @@ message SubmitDeploymentRequest {
11001100
// Verify all environment variables before persisting the deployment.
11011101
bool verify = 3;
11021102
optional string description = 4;
1103+
// Optional client-supplied deployment ID for idempotent submission.
1104+
// If a deployment with this ID already exists and its content digest matches
1105+
// the submitted config, the submission is a no-op and returns this ID.
1106+
// A digest mismatch is rejected. When unset, a fresh ID is generated.
1107+
optional DeploymentId deployment_id = 5;
11031108
}
11041109
message SubmitDeploymentResponse {
11051110
DeploymentId deployment_id = 1;

src/args.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,13 @@ pub(crate) enum Deployment {
108108
/// Optional human-readable description.
109109
#[arg(long)]
110110
description: Option<String>,
111+
/// Optional client-supplied deployment ID for idempotent submission.
112+
///
113+
/// If a deployment with this ID already exists and its content digest
114+
/// matches, the submission is a no-op; a digest mismatch is an error.
115+
/// Generate one with `obelisk generate deployment-id`.
116+
#[arg(long)]
117+
deployment_id: Option<DeploymentId>,
111118
/// Address of the obelisk server
112119
#[arg(short, long, default_value = "http://127.0.0.1:5005")]
113120
api_url: String,
@@ -130,6 +137,13 @@ pub(crate) enum Deployment {
130137
/// Optional human-readable description for a newly submitted deployment.
131138
#[arg(long)]
132139
description: Option<String>,
140+
/// Optional client-supplied deployment ID for idempotent submission.
141+
///
142+
/// Only valid when submitting a file/empty deployment, not when `source`
143+
/// is an existing deployment ID. Generate one with
144+
/// `obelisk generate deployment-id`.
145+
#[arg(long)]
146+
deployment_id: Option<DeploymentId>,
133147
/// Address of the obelisk server
134148
#[arg(short, long, default_value = "http://127.0.0.1:5005")]
135149
api_url: String,
@@ -149,6 +163,13 @@ pub(crate) enum Deployment {
149163
/// Optional human-readable description for a newly submitted deployment.
150164
#[arg(long)]
151165
description: Option<String>,
166+
/// Optional client-supplied deployment ID for idempotent submission.
167+
///
168+
/// Only valid when submitting a file/empty deployment, not when `source`
169+
/// is an existing deployment ID. Generate one with
170+
/// `obelisk generate deployment-id`.
171+
#[arg(long)]
172+
deployment_id: Option<DeploymentId>,
152173
/// Address of the obelisk server
153174
#[arg(short, long, default_value = "http://127.0.0.1:5005")]
154175
api_url: String,
@@ -288,6 +309,15 @@ pub(crate) enum Generate {
288309
#[arg(short, long)]
289310
json: bool,
290311
},
312+
/// Generate a fresh random deployment ID and print it to stdout.
313+
///
314+
/// The ID can be passed to `deployment submit/enqueue/apply --deployment-id`
315+
/// for idempotent submission.
316+
DeploymentId {
317+
/// Output as JSON instead of plain text.
318+
#[arg(short, long)]
319+
json: bool,
320+
},
291321
/// Print a prompt context for authoring an Obelisk application with a coding agent.
292322
///
293323
/// Usage: obelisk generate prompt description of what to build | claude

src/command/deployment.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ impl args::Deployment {
1919
empty,
2020
verify,
2121
description,
22+
deployment_id,
2223
api_url,
2324
} => {
2425
let config_json = load_config_json_from_file_or_empty(file, empty).await?;
@@ -30,6 +31,7 @@ impl args::Deployment {
3031
created_by: Some("cli".to_string()),
3132
verify,
3233
description,
34+
deployment_id: deployment_id.map(grpc_gen::DeploymentId::from),
3335
})
3436
.await?
3537
.into_inner();
@@ -43,6 +45,7 @@ impl args::Deployment {
4345
empty,
4446
verify,
4547
description,
48+
deployment_id,
4649
api_url,
4750
} => {
4851
let channel = to_channel(&api_url).await?;
@@ -53,6 +56,7 @@ impl args::Deployment {
5356
empty,
5457
false, // will be verified in switch
5558
description,
59+
deployment_id,
5660
)
5761
.await?;
5862
switch_deployment(
@@ -68,6 +72,7 @@ impl args::Deployment {
6872
source,
6973
empty,
7074
description,
75+
deployment_id,
7176
api_url,
7277
} => {
7378
let channel = to_channel(&api_url).await?;
@@ -78,6 +83,7 @@ impl args::Deployment {
7883
empty,
7984
false, // will be verified in switch
8085
description,
86+
deployment_id,
8187
)
8288
.await?;
8389
switch_deployment(
@@ -167,13 +173,17 @@ async fn submit_deployment(
167173
empty: bool,
168174
verify: bool,
169175
description: Option<String>,
176+
deployment_id: Option<DeploymentId>,
170177
) -> anyhow::Result<DeploymentId> {
171178
assert_ne!(source.is_some(), empty);
172179
let config_json = match source {
173180
Some(DeploymentSource::Id(id)) => {
174181
if description.is_some() {
175182
bail!("--description cannot be used with an existing deployment ID");
176183
}
184+
if deployment_id.is_some() {
185+
bail!("--deployment-id cannot be used with an existing deployment ID source");
186+
}
177187
return Ok(id);
178188
}
179189
Some(DeploymentSource::File(path)) => load_config_json(path).await?,
@@ -185,6 +195,7 @@ async fn submit_deployment(
185195
created_by: Some("cli".to_string()),
186196
verify,
187197
description,
198+
deployment_id: deployment_id.map(grpc_gen::DeploymentId::from),
188199
})
189200
.await?
190201
.into_inner();

src/command/generate.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,15 @@ impl Generate {
130130
}
131131
Ok(())
132132
}
133+
Generate::DeploymentId { json } => {
134+
let deployment_id = DeploymentId::generate();
135+
if json {
136+
println!("{}", serde_json::to_string_pretty(&deployment_id)?);
137+
} else {
138+
println!("{deployment_id}");
139+
}
140+
Ok(())
141+
}
133142
Generate::Prompt { description } => {
134143
let version = format!("v{PKG_VERSION}");
135144
let description = description.join(" ");

src/command/integration_tests.rs

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -875,6 +875,26 @@ impl TestServer {
875875
.expect("webapi submit deployment: invalid deployment id")
876876
}
877877

878+
/// Submit a deployment via the Web API with an explicit (idempotency) deployment ID,
879+
/// returning the raw response so callers can assert on success or conflict.
880+
async fn webapi_submit_deployment_with_id(
881+
&self,
882+
config_json: &str,
883+
deployment_id: DeploymentId,
884+
) -> reqwest::Response {
885+
self.client
886+
.post(format!("{}/v1/deployments", self.base_url))
887+
.header("Accept", "application/json")
888+
.json(&json!({
889+
"config_json": config_json,
890+
"verify": false,
891+
"deployment_id": deployment_id.to_string(),
892+
}))
893+
.send()
894+
.await
895+
.expect("webapi submit deployment request failed")
896+
}
897+
878898
/// Hot-redeploy to the given deployment via the Web API.
879899
async fn webapi_switch_hot_redeploy(&self, deployment_id: DeploymentId) {
880900
let resp = self
@@ -939,6 +959,7 @@ impl TestDeployClient {
939959
created_by: Some("test".to_string()),
940960
verify: false,
941961
description: None,
962+
deployment_id: None,
942963
})
943964
.await
944965
.unwrap()
@@ -1657,6 +1678,79 @@ async fn list_components_grpc_filters_with_explicit_deployment_id() {
16571678
server.shutdown().await;
16581679
}
16591680

1681+
#[tokio::test]
1682+
async fn submit_deployment_is_idempotent_by_id_and_digest_webapi() {
1683+
let server = TestServer::start(test_addr!(40_104)).await;
1684+
1685+
// Read the active deployment's canonical config so we can resubmit it verbatim.
1686+
let pool = SqlitePool::new(&server.sqlite_file, SqliteConfig::default())
1687+
.await
1688+
.unwrap();
1689+
let conn = pool.external_api_conn().await.unwrap();
1690+
let active = conn.get_active_deployment().await.unwrap().unwrap();
1691+
pool.close().await;
1692+
let canonical: DeploymentCanonical = serde_json::from_str(&active.config_json).unwrap();
1693+
let config_json = crate::config::toml::compute_config_json(&canonical);
1694+
1695+
let deployment_id = DeploymentId::generate();
1696+
1697+
// First submission under the explicit ID creates the deployment.
1698+
let resp1 = server
1699+
.webapi_submit_deployment_with_id(&config_json, deployment_id)
1700+
.await;
1701+
assert!(
1702+
resp1.status().is_success(),
1703+
"first submit failed: {}",
1704+
resp1.status()
1705+
);
1706+
let body1: Value = resp1.json().await.unwrap();
1707+
let returned1: DeploymentId = body1["ok"].as_str().unwrap().parse().unwrap();
1708+
assert_eq!(deployment_id, returned1, "explicit ID must be honored");
1709+
1710+
// Resubmitting the identical config under the same ID is an idempotent no-op.
1711+
let resp2 = server
1712+
.webapi_submit_deployment_with_id(&config_json, deployment_id)
1713+
.await;
1714+
assert!(
1715+
resp2.status().is_success(),
1716+
"idempotent resubmit failed: {}",
1717+
resp2.status()
1718+
);
1719+
let body2: Value = resp2.json().await.unwrap();
1720+
let returned2: DeploymentId = body2["ok"].as_str().unwrap().parse().unwrap();
1721+
assert_eq!(
1722+
deployment_id, returned2,
1723+
"no-op resubmit must return same ID"
1724+
);
1725+
1726+
// Submitting a different config under the same ID is rejected as a digest conflict.
1727+
let mut mutated: DeploymentCanonical = serde_json::from_str(&active.config_json).unwrap();
1728+
mutated
1729+
.activities_stub
1730+
.push(ActivityStubComponentConfigCanonical::Inline(
1731+
ActivityStubExtInlineConfigCanonical {
1732+
name: ConfigName::new(concepts::StrVariant::from("idempotency_conflict_stub"))
1733+
.unwrap(),
1734+
ffqn: "testing:integration/stubs.idempotency-conflict"
1735+
.parse()
1736+
.unwrap(),
1737+
params: Some(vec![]),
1738+
return_type: Some("result<string, string>".to_string()),
1739+
},
1740+
));
1741+
let mutated_json = crate::config::toml::compute_config_json(&mutated);
1742+
let resp3 = server
1743+
.webapi_submit_deployment_with_id(&mutated_json, deployment_id)
1744+
.await;
1745+
assert_eq!(
1746+
resp3.status(),
1747+
reqwest::StatusCode::BAD_REQUEST,
1748+
"different config under same ID must be rejected as a digest conflict"
1749+
);
1750+
1751+
server.shutdown().await;
1752+
}
1753+
16601754
// ---- Activity: submit + result ----
16611755

16621756
#[tokio::test]

src/command/server.rs

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1504,6 +1504,7 @@ pub(crate) async fn submit_deployment(
15041504
verify: bool,
15051505
created_by: Option<String>,
15061506
description: Option<String>,
1507+
requested_deployment_id: Option<DeploymentId>,
15071508
prepared_dirs: &PreparedDirs,
15081509
db_pool: Arc<dyn DbPool>,
15091510
termination_watcher: &mut watch::Receiver<()>,
@@ -1513,6 +1514,26 @@ pub(crate) async fn submit_deployment(
15131514
serde_json::from_str(config_json).with_context(|| "cannot parse config_json")?;
15141515

15151516
let canonical_config = crate::config::toml::compute_config_json(&deployment);
1517+
let digest = DeploymentRecord::compute_digest(&canonical_config);
1518+
1519+
let conn = db_pool.external_api_conn().await?;
1520+
1521+
// Idempotent submission: if the caller supplied a deployment ID that already
1522+
// exists, return it as a no-op when the content digest matches (skipping the
1523+
// expensive verify/compile/link), and reject a digest mismatch as a conflict.
1524+
if let Some(requested_deployment_id) = requested_deployment_id
1525+
&& let Some(existing) = conn.get_deployment(requested_deployment_id).await?
1526+
{
1527+
if existing.digest == digest {
1528+
info!(%requested_deployment_id, "Deployment already exists with matching digest, returning existing ID");
1529+
return Ok(requested_deployment_id);
1530+
}
1531+
bail!(
1532+
"deployment {requested_deployment_id} already exists with a different content digest \
1533+
(existing {}, submitted {digest}); use a fresh deployment ID",
1534+
existing.digest
1535+
);
1536+
}
15161537

15171538
let verify_deployment_id = DeploymentId::generate();
15181539
let server_compiled = deployment_verify_config_compile_link(
@@ -1533,10 +1554,8 @@ pub(crate) async fn submit_deployment(
15331554
)
15341555
.await?;
15351556

1536-
let deployment_id = DeploymentId::generate();
1537-
let conn = db_pool.external_api_conn().await?;
1557+
let deployment_id = requested_deployment_id.unwrap_or_else(DeploymentId::generate);
15381558
let now = chrono::Utc::now();
1539-
let digest = DeploymentRecord::compute_digest(&canonical_config);
15401559

15411560
conn.insert_deployment(DeploymentRecord {
15421561
deployment_id,

0 commit comments

Comments
 (0)