fix(openapi): harden story 2.1 production lifecycle
This commit is contained in:
@@ -29,6 +29,10 @@ use crate::{
|
||||
};
|
||||
|
||||
const IMPORT_JOB_TTL_HOURS: i64 = 24;
|
||||
const OPENAPI_PARSE_DEADLINE: std::time::Duration = std::time::Duration::from_secs(30);
|
||||
const OPENAPI_PARSE_CONCURRENCY: usize = 4;
|
||||
static OPENAPI_PARSE_SLOTS: tokio::sync::Semaphore =
|
||||
tokio::sync::Semaphore::const_new(OPENAPI_PARSE_CONCURRENCY);
|
||||
|
||||
impl AdminService {
|
||||
#[instrument(skip(self, upload), fields(workspace_id = %workspace_id.as_str()))]
|
||||
@@ -38,7 +42,6 @@ impl AdminService {
|
||||
upload: OpenApiUpload,
|
||||
) -> Result<OpenApiImportPreviewResponse, ApiError> {
|
||||
self.ensure_workspace_exists(workspace_id).await?;
|
||||
self.registry.delete_expired_import_jobs().await?;
|
||||
|
||||
validate_openapi_upload(&upload)?;
|
||||
let OpenApiUpload {
|
||||
@@ -72,19 +75,38 @@ impl AdminService {
|
||||
source_id.clone(),
|
||||
source.updated_at,
|
||||
);
|
||||
let verified = self
|
||||
let verified = match self
|
||||
.registry
|
||||
.read_artifact_source(&self.artifact_store, workspace_id, &source_id)
|
||||
.await?;
|
||||
.read_artifact_source(
|
||||
std::sync::Arc::clone(&self.artifact_store),
|
||||
workspace_id,
|
||||
&source_id,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(verified) => verified,
|
||||
Err(error) => {
|
||||
detach_guard.detach_now().await;
|
||||
return Err(ApiError::from(error));
|
||||
}
|
||||
};
|
||||
if verified.source.blob.artifact_ref != *artifact.artifact_ref() {
|
||||
detach_guard.detach_now().await;
|
||||
return Err(ApiError::openapi_upload(locale, "source_integrity"));
|
||||
}
|
||||
let preview = parse_verified_preview(verified.bytes, locale).await?;
|
||||
let preview = match parse_verified_preview(verified.bytes, locale).await {
|
||||
Ok(preview) => preview,
|
||||
Err(error) => {
|
||||
detach_guard.detach_now().await;
|
||||
return Err(error);
|
||||
}
|
||||
};
|
||||
if preview
|
||||
.groups
|
||||
.iter()
|
||||
.all(|group| group.operations.is_empty())
|
||||
{
|
||||
detach_guard.detach_now().await;
|
||||
return Err(ApiError::openapi_upload(locale, "no_methods"));
|
||||
}
|
||||
let source_envelope = ImportJobSourceEnvelope {
|
||||
@@ -93,15 +115,18 @@ impl AdminService {
|
||||
};
|
||||
let preview_value = serde_json::to_value(&preview)
|
||||
.map_err(|error| ApiError::internal(error.to_string()))?;
|
||||
let preview_digest = preview_digest(&preview_value)?;
|
||||
let preview_payload = json!({
|
||||
"source": {
|
||||
"source_id": source_envelope.source_id.as_str(),
|
||||
"digest": source_envelope.digest.as_str(),
|
||||
},
|
||||
"preview": preview_value,
|
||||
"preview_digest": preview_digest,
|
||||
});
|
||||
|
||||
self.registry
|
||||
if let Err(error) = self
|
||||
.registry
|
||||
.create_import_job(CreateImportJobRequest {
|
||||
id: &job_id,
|
||||
workspace_id,
|
||||
@@ -114,7 +139,11 @@ impl AdminService {
|
||||
created_at: &now,
|
||||
expires_at: &expires_at,
|
||||
})
|
||||
.await?;
|
||||
.await
|
||||
{
|
||||
detach_guard.detach_now().await;
|
||||
return Err(ApiError::from(error));
|
||||
}
|
||||
detach_guard.disarm();
|
||||
|
||||
Ok(OpenApiImportPreviewResponse {
|
||||
@@ -132,9 +161,24 @@ impl AdminService {
|
||||
workspace_id: &WorkspaceId,
|
||||
job_id: &ImportJobId,
|
||||
payload: OpenApiImportCreatePayload,
|
||||
) -> Result<OpenApiImportCreateResponse, ApiError> {
|
||||
self.create_openapi_import_with_locale(
|
||||
workspace_id,
|
||||
job_id,
|
||||
payload,
|
||||
OpenApiUploadLocale::En,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
pub async fn create_openapi_import_with_locale(
|
||||
&self,
|
||||
workspace_id: &WorkspaceId,
|
||||
job_id: &ImportJobId,
|
||||
payload: OpenApiImportCreatePayload,
|
||||
locale: OpenApiUploadLocale,
|
||||
) -> Result<OpenApiImportCreateResponse, ApiError> {
|
||||
self.ensure_workspace_exists(workspace_id).await?;
|
||||
self.registry.delete_expired_import_jobs().await?;
|
||||
|
||||
if !matches!(payload.conflict_mode.as_str(), "skip" | "rename") {
|
||||
return Err(ApiError::validation(
|
||||
@@ -185,22 +229,56 @@ impl AdminService {
|
||||
application_key: &application_key,
|
||||
conflict_mode,
|
||||
operations: &[],
|
||||
pre_skipped: &[],
|
||||
finished_at: &finished_at,
|
||||
})
|
||||
.await?;
|
||||
return Ok(openapi_import_response(applied, Vec::new()));
|
||||
return Ok(openapi_import_response(applied));
|
||||
}
|
||||
|
||||
let source = import_job_source(&job.preview_payload)?;
|
||||
let verified = self
|
||||
let source = import_job_source(&job.preview_payload, locale)?;
|
||||
let verified = match self
|
||||
.registry
|
||||
.read_artifact_source(&self.artifact_store, workspace_id, &source.source_id)
|
||||
.read_artifact_source(
|
||||
std::sync::Arc::clone(&self.artifact_store),
|
||||
workspace_id,
|
||||
&source.source_id,
|
||||
)
|
||||
.await
|
||||
.map_err(openapi_source_error)?;
|
||||
{
|
||||
Ok(verified) => verified,
|
||||
Err(
|
||||
error @ (RegistryError::SourceNotFound { .. } | RegistryError::SourceUnavailable),
|
||||
) => {
|
||||
// Another request may have finished and detached the source
|
||||
// after our unlocked Pending read. Re-read the job and ask
|
||||
// the registry for its canonical, locked replay instead of
|
||||
// exposing a false `source_unavailable` outcome.
|
||||
let latest = self.registry.get_import_job(workspace_id, job_id).await?;
|
||||
if latest.is_some_and(|latest| latest.status == ImportJobStatus::Completed) {
|
||||
let applied = self
|
||||
.registry
|
||||
.apply_import_job(ApplyImportJobRequest {
|
||||
id: job_id,
|
||||
workspace_id,
|
||||
application_key: &application_key,
|
||||
conflict_mode,
|
||||
operations: &[],
|
||||
pre_skipped: &[],
|
||||
finished_at: &finished_at,
|
||||
})
|
||||
.await?;
|
||||
return Ok(openapi_import_response(applied));
|
||||
}
|
||||
return Err(openapi_source_error(locale, error));
|
||||
}
|
||||
Err(error) => return Err(openapi_source_error(locale, error)),
|
||||
};
|
||||
if verified.source.blob.artifact_ref != source.digest {
|
||||
return Err(ApiError::source_integrity());
|
||||
return Err(ApiError::openapi_upload(locale, "source_integrity"));
|
||||
}
|
||||
let preview = parse_verified_preview(verified.bytes, OpenApiUploadLocale::En).await?;
|
||||
let preview = parse_verified_preview(verified.bytes, locale).await?;
|
||||
verify_preview_contract(&job.preview_payload, &preview, locale)?;
|
||||
let mut candidates = BTreeMap::new();
|
||||
for group in &preview.groups {
|
||||
for operation in &group.operations {
|
||||
@@ -213,7 +291,7 @@ impl AdminService {
|
||||
|
||||
for operation_key in selected {
|
||||
let Some(candidate) = candidates.get(&operation_key) else {
|
||||
skipped.push(OpenApiImportSkippedOperation {
|
||||
skipped.push(crank_registry::SkippedImportOperation {
|
||||
operation_key,
|
||||
name: String::new(),
|
||||
reason: "operation was not found in import preview".to_owned(),
|
||||
@@ -261,22 +339,21 @@ impl AdminService {
|
||||
application_key: &application_key,
|
||||
conflict_mode,
|
||||
operations: &operations,
|
||||
pre_skipped: &skipped,
|
||||
finished_at: &finished_at,
|
||||
})
|
||||
.await?;
|
||||
|
||||
Ok(openapi_import_response(applied, skipped))
|
||||
Ok(openapi_import_response(applied))
|
||||
}
|
||||
}
|
||||
|
||||
fn openapi_import_response(
|
||||
applied: ImportJobApplyResult,
|
||||
mut skipped: Vec<OpenApiImportSkippedOperation>,
|
||||
) -> OpenApiImportCreateResponse {
|
||||
fn openapi_import_response(applied: ImportJobApplyResult) -> OpenApiImportCreateResponse {
|
||||
let created = applied
|
||||
.created
|
||||
.iter()
|
||||
.map(|operation| OpenApiImportCreatedOperation {
|
||||
operation_key: operation.operation_key.clone(),
|
||||
operation_id: operation.operation_id.as_str().to_owned(),
|
||||
name: operation.name.clone(),
|
||||
version: operation.version,
|
||||
@@ -300,22 +377,29 @@ fn openapi_import_response(
|
||||
})
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
for operation in applied.skipped {
|
||||
skipped.push(OpenApiImportSkippedOperation {
|
||||
operation_key: operation.operation_key.clone(),
|
||||
name: operation.name.clone(),
|
||||
reason: "operation with this name already exists".to_owned(),
|
||||
});
|
||||
findings.push(ImportFinding {
|
||||
code: operation.reason,
|
||||
severity: ImportFindingSeverity::Warning,
|
||||
message: format!(
|
||||
"Операция {} уже существует и была пропущена.",
|
||||
operation.name
|
||||
),
|
||||
operation_key: Some(operation.operation_key),
|
||||
});
|
||||
}
|
||||
let skipped = applied
|
||||
.skipped
|
||||
.into_iter()
|
||||
.map(|operation| {
|
||||
let reason = operation.reason.clone();
|
||||
let name = operation.name.clone();
|
||||
findings.push(ImportFinding {
|
||||
code: reason.clone(),
|
||||
severity: ImportFindingSeverity::Warning,
|
||||
message: if name.is_empty() {
|
||||
"Выбранная операция отсутствует в исходном preview и была пропущена.".to_owned()
|
||||
} else {
|
||||
format!("Операция {name} уже существует и была пропущена.")
|
||||
},
|
||||
operation_key: Some(operation.operation_key.clone()),
|
||||
});
|
||||
OpenApiImportSkippedOperation {
|
||||
operation_key: operation.operation_key.clone(),
|
||||
name: operation.name,
|
||||
reason,
|
||||
}
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
info!(
|
||||
name: "admin.openapi_import.completed",
|
||||
created = created.len(),
|
||||
@@ -361,14 +445,28 @@ async fn parse_verified_preview(
|
||||
bytes: Vec<u8>,
|
||||
locale: OpenApiUploadLocale,
|
||||
) -> Result<crank_import::rest::ImportPreview, ApiError> {
|
||||
tokio::task::spawn_blocking(move || {
|
||||
let started = tokio::time::Instant::now();
|
||||
let permit = tokio::time::timeout(OPENAPI_PARSE_DEADLINE, OPENAPI_PARSE_SLOTS.acquire())
|
||||
.await
|
||||
.map_err(|_| ApiError::openapi_upload(locale, "parser_unavailable"))?
|
||||
.map_err(|_| ApiError::openapi_upload(locale, "parser_unavailable"))?;
|
||||
let parsing = tokio::task::spawn_blocking(move || {
|
||||
// Keep the permit inside the blocking task. Timing out the caller
|
||||
// cannot cancel CPU work already running, but abandoned parsers remain
|
||||
// globally bounded and release capacity when they actually finish.
|
||||
let _permit = permit;
|
||||
let document = std::str::from_utf8(&bytes)
|
||||
.map_err(|_| ApiError::openapi_upload(locale, "invalid_utf8"))?;
|
||||
crank_import::rest::preview_document(document)
|
||||
.map_err(|_| ApiError::openapi_upload(locale, "invalid_document"))
|
||||
})
|
||||
.await
|
||||
.map_err(|_| ApiError::openapi_upload(locale, "parser_unavailable"))?
|
||||
});
|
||||
let remaining = OPENAPI_PARSE_DEADLINE
|
||||
.checked_sub(started.elapsed())
|
||||
.unwrap_or(std::time::Duration::ZERO);
|
||||
tokio::time::timeout(remaining, parsing)
|
||||
.await
|
||||
.map_err(|_| ApiError::openapi_upload(locale, "parser_unavailable"))?
|
||||
.map_err(|_| ApiError::openapi_upload(locale, "parser_unavailable"))?
|
||||
}
|
||||
|
||||
fn artifact_error(locale: OpenApiUploadLocale, error: ArtifactError) -> ApiError {
|
||||
@@ -383,30 +481,63 @@ fn artifact_error(locale: OpenApiUploadLocale, error: ArtifactError) -> ApiError
|
||||
}
|
||||
}
|
||||
|
||||
fn openapi_source_error(error: RegistryError) -> ApiError {
|
||||
fn openapi_source_error(locale: OpenApiUploadLocale, error: RegistryError) -> ApiError {
|
||||
match error {
|
||||
RegistryError::SourceNotFound { .. } | RegistryError::SourceUnavailable => {
|
||||
ApiError::source_unavailable()
|
||||
ApiError::openapi_upload(locale, "source_unavailable")
|
||||
}
|
||||
RegistryError::SourceIntegrity => ApiError::source_integrity(),
|
||||
RegistryError::SourceIntegrity => ApiError::openapi_upload(locale, "source_integrity"),
|
||||
other => ApiError::from(other),
|
||||
}
|
||||
}
|
||||
|
||||
fn import_job_source(payload: &serde_json::Value) -> Result<ImportJobSourceEnvelope, ApiError> {
|
||||
fn preview_digest(preview: &serde_json::Value) -> Result<String, ApiError> {
|
||||
let canonical =
|
||||
serde_json::to_vec(preview).map_err(|error| ApiError::internal(error.to_string()))?;
|
||||
Ok(format!("{:x}", Sha256::digest(canonical)))
|
||||
}
|
||||
|
||||
fn verify_preview_contract(
|
||||
payload: &serde_json::Value,
|
||||
preview: &crank_import::rest::ImportPreview,
|
||||
locale: OpenApiUploadLocale,
|
||||
) -> Result<(), ApiError> {
|
||||
// Pre-fingerprint jobs are legacy rolling-upgrade records. They retain the
|
||||
// old reparse behavior; new jobs fail closed if parser output drifts or a
|
||||
// persisted preview has been changed.
|
||||
let Some(expected) = payload
|
||||
.get("preview_digest")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
else {
|
||||
return Ok(());
|
||||
};
|
||||
let actual = preview_digest(
|
||||
&serde_json::to_value(preview).map_err(|error| ApiError::internal(error.to_string()))?,
|
||||
)?;
|
||||
if actual == expected {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(ApiError::openapi_upload(locale, "source_integrity"))
|
||||
}
|
||||
}
|
||||
|
||||
fn import_job_source(
|
||||
payload: &serde_json::Value,
|
||||
locale: OpenApiUploadLocale,
|
||||
) -> Result<ImportJobSourceEnvelope, ApiError> {
|
||||
let source = payload
|
||||
.get("source")
|
||||
.ok_or_else(ApiError::source_unavailable)?;
|
||||
.ok_or_else(|| ApiError::openapi_upload(locale, "source_unavailable"))?;
|
||||
let source_id = source
|
||||
.get("source_id")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.filter(|value| value.len() <= 132)
|
||||
.ok_or_else(ApiError::source_unavailable)?;
|
||||
.ok_or_else(|| ApiError::openapi_upload(locale, "source_unavailable"))?;
|
||||
let digest = source
|
||||
.get("digest")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.and_then(|value| value.parse().ok())
|
||||
.ok_or_else(ApiError::source_integrity)?;
|
||||
.ok_or_else(|| ApiError::openapi_upload(locale, "source_integrity"))?;
|
||||
Ok(ImportJobSourceEnvelope {
|
||||
source_id: ArtifactSourceId::new(source_id),
|
||||
digest,
|
||||
@@ -440,6 +571,25 @@ impl SourceDetachGuard {
|
||||
fn disarm(&mut self) {
|
||||
self.armed = false;
|
||||
}
|
||||
|
||||
async fn detach_now(&mut self) {
|
||||
if !self.armed {
|
||||
return;
|
||||
}
|
||||
self.armed = false;
|
||||
// This is the normal error path: await the database transition so a
|
||||
// caller receives a completed cleanup before it can retry. `Drop`
|
||||
// remains only the cancellation/shutdown safety net.
|
||||
let _ = self
|
||||
.registry
|
||||
.detach_artifact_source(DetachArtifactSourceRequest {
|
||||
workspace_id: &self.workspace_id,
|
||||
source_id: &self.source_id,
|
||||
expected_updated_at: Some(self.expected_updated_at),
|
||||
detached_at: OffsetDateTime::now_utc(),
|
||||
})
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for SourceDetachGuard {
|
||||
|
||||
Reference in New Issue
Block a user