fix: restore workers with remote workdir attachments
This commit is contained in:
@@ -192,6 +192,32 @@ impl BackendApiClient {
|
|||||||
format!("Bearer {}", self.access_token.0)
|
format!("Bearer {}", self.access_token.0)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub async fn require_success(
|
||||||
|
&self,
|
||||||
|
response: reqwest::Response,
|
||||||
|
) -> Result<reqwest::Response, BackendApiClientError> {
|
||||||
|
let status = response.status();
|
||||||
|
match status {
|
||||||
|
StatusCode::UNAUTHORIZED | StatusCode::FORBIDDEN => {
|
||||||
|
self.check_status(status)?;
|
||||||
|
}
|
||||||
|
status if !status.is_success() => {
|
||||||
|
let detail = response
|
||||||
|
.bytes()
|
||||||
|
.await
|
||||||
|
.ok()
|
||||||
|
.and_then(|body| backend_error_detail(&body));
|
||||||
|
return Err(BackendApiClientError::BackendResponse {
|
||||||
|
origin: self.origin.clone(),
|
||||||
|
status: status.as_u16(),
|
||||||
|
detail,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
_ => {}
|
||||||
|
}
|
||||||
|
Ok(response)
|
||||||
|
}
|
||||||
|
|
||||||
pub fn check_status(&self, status: StatusCode) -> Result<(), BackendApiClientError> {
|
pub fn check_status(&self, status: StatusCode) -> Result<(), BackendApiClientError> {
|
||||||
match status {
|
match status {
|
||||||
StatusCode::UNAUTHORIZED => Err(BackendApiClientError::Unauthorized {
|
StatusCode::UNAUTHORIZED => Err(BackendApiClientError::Unauthorized {
|
||||||
@@ -235,6 +261,18 @@ fn redirect_policy(origin: BackendOrigin) -> redirect::Policy {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[derive(Deserialize)]
|
||||||
|
struct BackendErrorBody {
|
||||||
|
message: String,
|
||||||
|
}
|
||||||
|
|
||||||
|
fn backend_error_detail(body: &[u8]) -> Option<String> {
|
||||||
|
serde_json::from_slice::<BackendErrorBody>(body)
|
||||||
|
.ok()
|
||||||
|
.map(|body| body.message)
|
||||||
|
.filter(|message| !message.trim().is_empty())
|
||||||
|
}
|
||||||
|
|
||||||
#[derive(Debug)]
|
#[derive(Debug)]
|
||||||
pub enum BackendApiClientError {
|
pub enum BackendApiClientError {
|
||||||
InvalidBackendOrigin(String),
|
InvalidBackendOrigin(String),
|
||||||
@@ -266,6 +304,11 @@ pub enum BackendApiClientError {
|
|||||||
origin: BackendOrigin,
|
origin: BackendOrigin,
|
||||||
status: u16,
|
status: u16,
|
||||||
},
|
},
|
||||||
|
BackendResponse {
|
||||||
|
origin: BackendOrigin,
|
||||||
|
status: u16,
|
||||||
|
detail: Option<String>,
|
||||||
|
},
|
||||||
Io {
|
Io {
|
||||||
path: PathBuf,
|
path: PathBuf,
|
||||||
source: std::io::Error,
|
source: std::io::Error,
|
||||||
@@ -312,6 +355,17 @@ impl fmt::Display for BackendApiClientError {
|
|||||||
Self::BackendStatus { origin, status } => {
|
Self::BackendStatus { origin, status } => {
|
||||||
write!(f, "Backend {origin} returned HTTP {status}")
|
write!(f, "Backend {origin} returned HTTP {status}")
|
||||||
}
|
}
|
||||||
|
Self::BackendResponse {
|
||||||
|
origin,
|
||||||
|
status,
|
||||||
|
detail,
|
||||||
|
} => {
|
||||||
|
write!(f, "Backend {origin} returned HTTP {status}")?;
|
||||||
|
if let Some(detail) = detail {
|
||||||
|
write!(f, ": {detail}")?;
|
||||||
|
}
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
Self::Io { path, source } => {
|
Self::Io { path, source } => {
|
||||||
write!(f, "failed to access {}: {source}", path.display())
|
write!(f, "failed to access {}: {source}", path.display())
|
||||||
}
|
}
|
||||||
@@ -584,6 +638,23 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn backend_error_detail_preserves_public_server_message() {
|
||||||
|
let detail = backend_error_detail(
|
||||||
|
br#"{"error":"Bad Request","message":"working_directory_runtime_mismatch: Working directory is owned by a different Runtime","diagnostics":[{"code":"working_directory_runtime_mismatch"}]}"#,
|
||||||
|
);
|
||||||
|
let error = BackendApiClientError::BackendResponse {
|
||||||
|
origin: BackendOrigin::parse("http://127.0.0.1:8787").unwrap(),
|
||||||
|
status: 400,
|
||||||
|
detail,
|
||||||
|
};
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
error.to_string(),
|
||||||
|
"Backend http://127.0.0.1:8787 returned HTTP 400: working_directory_runtime_mismatch: Working directory is owned by a different Runtime"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn backend_origin_rejects_unsafe_authority_changes() {
|
fn backend_origin_rejects_unsafe_authority_changes() {
|
||||||
for invalid in [
|
for invalid in [
|
||||||
|
|||||||
@@ -269,7 +269,7 @@ pub async fn restore_backend_worker(
|
|||||||
.json(&serde_json::json!({}))
|
.json(&serde_json::json!({}))
|
||||||
.send()
|
.send()
|
||||||
.await?;
|
.await?;
|
||||||
api.check_status(response.status())?;
|
let response = api.require_success(response).await?;
|
||||||
Ok(response.json::<BackendWorkerRestoreResponse>().await?)
|
Ok(response.json::<BackendWorkerRestoreResponse>().await?)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1909,16 +1909,18 @@ impl WorkspaceApi {
|
|||||||
.list_worker_workdir_links(&self.config.workspace_id, worker)?
|
.list_worker_workdir_links(&self.config.workspace_id, worker)?
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.find(|link| link.unlinked_at.is_none())
|
.find(|link| link.unlinked_at.is_none())
|
||||||
&& let Some(access) = repository_access_request_for_workdir(
|
{
|
||||||
|
let workdir_runtime_id = registered_workdir_runtime_id(self, &link.workdir_id)?;
|
||||||
|
if let Some(access) = repository_access_request_for_workdir(
|
||||||
self,
|
self,
|
||||||
&worker.runtime_id,
|
&workdir_runtime_id,
|
||||||
&link.workdir_id,
|
&link.workdir_id,
|
||||||
&format!("worker-restore:{}", WorkerId::now_v7()),
|
&format!("worker-restore:{}", WorkerId::now_v7()),
|
||||||
)?
|
)? {
|
||||||
{
|
self.runtime
|
||||||
self.runtime
|
.authorize_working_directory_repository_access(&workdir_runtime_id, access)
|
||||||
.authorize_working_directory_repository_access(&worker.runtime_id, access)
|
.map_err(RuntimeRegistryError::into_error)?;
|
||||||
.map_err(RuntimeRegistryError::into_error)?;
|
}
|
||||||
}
|
}
|
||||||
let binding = self
|
let binding = self
|
||||||
.runtime
|
.runtime
|
||||||
@@ -25010,7 +25012,7 @@ mod tests {
|
|||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
.unwrap();
|
.unwrap();
|
||||||
let app = build_inner_router(api);
|
let app = build_inner_router(api.clone());
|
||||||
|
|
||||||
let runtimes = get_json(app.clone(), "/api/runtimes").await;
|
let runtimes = get_json(app.clone(), "/api/runtimes").await;
|
||||||
let embedded_summary = runtimes["items"]
|
let embedded_summary = runtimes["items"]
|
||||||
@@ -25065,6 +25067,37 @@ mod tests {
|
|||||||
"embedded_worker_runtime"
|
"embedded_worker_runtime"
|
||||||
);
|
);
|
||||||
|
|
||||||
|
let workdir_id = "external-workdir";
|
||||||
|
api.store
|
||||||
|
.upsert_workdir_registry(&WorkdirRegistryRecord {
|
||||||
|
workspace_id: TEST_WORKSPACE_ID.to_string(),
|
||||||
|
workdir_id: workdir_id.to_string(),
|
||||||
|
runtime_id: "external-workdir-runtime".to_string(),
|
||||||
|
repository_id: "main".to_string(),
|
||||||
|
creation_selector: None,
|
||||||
|
creation_ref: None,
|
||||||
|
creation_tree: None,
|
||||||
|
current_selector: None,
|
||||||
|
current_ref: None,
|
||||||
|
current_tree: None,
|
||||||
|
observed_at_epoch_seconds: None,
|
||||||
|
materialization_status: "present".to_string(),
|
||||||
|
cleanliness: "clean".to_string(),
|
||||||
|
created_at: "1".to_string(),
|
||||||
|
updated_at: "1".to_string(),
|
||||||
|
})
|
||||||
|
.unwrap();
|
||||||
|
api.store
|
||||||
|
.attach_worker_workdir(&WorkerWorkdirLinkRecord {
|
||||||
|
workspace_id: TEST_WORKSPACE_ID.to_string(),
|
||||||
|
worker: RuntimeWorkerRef::new("embedded-worker-runtime", &worker_id),
|
||||||
|
workdir_id: workdir_id.to_string(),
|
||||||
|
role: "attachment".to_string(),
|
||||||
|
linked_at: "2".to_string(),
|
||||||
|
unlinked_at: None,
|
||||||
|
})
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
let worker = get_json(
|
let worker = get_json(
|
||||||
app.clone(),
|
app.clone(),
|
||||||
&format!("/api/runtimes/embedded-worker-runtime/workers/{worker_id}"),
|
&format!("/api/runtimes/embedded-worker-runtime/workers/{worker_id}"),
|
||||||
|
|||||||
Reference in New Issue
Block a user