From 5a52a7a56531861f595a15b1c343bcc283292453 Mon Sep 17 00:00:00 2001 From: github-ops Date: Sun, 21 Jun 2026 09:45:08 +0000 Subject: [PATCH] Extract admin operation validation --- apps/admin-api/src/service.rs | 242 +----------------- .../src/service/operation_validation.rs | 241 +++++++++++++++++ 2 files changed, 248 insertions(+), 235 deletions(-) create mode 100644 apps/admin-api/src/service/operation_validation.rs diff --git a/apps/admin-api/src/service.rs b/apps/admin-api/src/service.rs index 6dbddcf..ee3d6ee 100644 --- a/apps/admin-api/src/service.rs +++ b/apps/admin-api/src/service.rs @@ -7,8 +7,8 @@ use crank_core::{ AuditSink, AuthProfile, CapabilityProfile, CommunityCapabilityProfile, EditionCapabilities, ExecutionMode, IdentityError, IdentityProvider, InvocationLog, InvocationLogId, NoopAuditSink, OperationSecurityLevel, OwnerOnlyPolicyEngine, PolicyEngine, ProductEdition, Protocol, - ResponseCachePolicy, Target, ToolQualityMappingRule, ToolQualityMappingSet, - ToolQualitySchemaKind, ToolQualitySchemaNode, UsagePeriod, WorkspaceId, + ToolQualityMappingRule, ToolQualityMappingSet, ToolQualitySchemaKind, ToolQualitySchemaNode, + UsagePeriod, WorkspaceId, }; use crank_mapping::{MappingRule, MappingSet}; use crank_registry::{ @@ -28,6 +28,7 @@ mod auth; mod demo; mod import_export; mod observability; +mod operation_validation; mod operations; mod samples; mod secrets; @@ -35,6 +36,9 @@ mod upstreams; mod workspaces; use crate::{auth::AuthSettings, error::ApiError, storage::LocalArtifactStorage}; +use operation_validation::{ + validate_idempotency_policy, validate_protocol_target, validate_response_cache_policy, +}; #[derive(Clone)] pub struct AdminService { @@ -431,16 +435,6 @@ fn build_request_preview( })) } -fn validate_protocol_target(protocol: Protocol, target: &Target) -> Result<(), ApiError> { - let is_match = matches!((protocol, target), (Protocol::Rest, Target::Rest(_))); - - if is_match { - return Ok(()); - } - - Err(ApiError::validation("protocol and target kind must match")) -} - fn validate_profile_display_name(value: &str) -> Result { let display_name = value.trim(); if display_name.is_empty() { @@ -686,95 +680,6 @@ fn agent_mcp_endpoint(workspace_slug: &str, agent_slug: &str) -> String { format!("/mcp/v1/{workspace_slug}/{agent_slug}") } -fn validate_response_cache_policy( - target: &Target, - execution_config: &crank_core::ExecutionConfig, -) -> Result<(), ApiError> { - let Some(ResponseCachePolicy { ttl_ms }) = execution_config.response_cache.as_ref() else { - return Ok(()); - }; - - if *ttl_ms == 0 { - return Err(ApiError::validation_with_context( - "response cache ttl must be greater than zero".to_owned(), - json!({ - "field": "execution_config.response_cache.ttl_ms", - }), - )); - } - - match target { - Target::Rest(rest_target) if rest_target.method == crank_core::HttpMethod::Get => {} - _ => { - return Err(ApiError::validation_with_context( - "response cache is supported only for REST GET operations".to_owned(), - json!({ - "field": "execution_config.response_cache", - }), - )); - } - } - - if execution_config.auth_profile_ref.is_some() { - return Err(ApiError::validation_with_context( - "response cache is not supported for operations with auth_profile_ref".to_owned(), - json!({ - "field": "execution_config.auth_profile_ref", - }), - )); - } - - Ok(()) -} - -fn validate_idempotency_policy( - target: &Target, - execution_config: &crank_core::ExecutionConfig, -) -> Result<(), ApiError> { - let Some(policy) = execution_config.idempotency.as_ref() else { - return Ok(()); - }; - - if policy.mode == crank_core::IdempotencyMode::Disabled { - return Ok(()); - } - - if policy.ttl_ms == 0 { - return Err(ApiError::validation_with_context( - "idempotency ttl must be greater than zero".to_owned(), - json!({ - "field": "execution_config.idempotency.ttl_ms", - }), - )); - } - - match target { - Target::Rest(rest_target) if rest_target.method != crank_core::HttpMethod::Get => {} - _ => { - return Err(ApiError::validation_with_context( - "idempotency is supported only for mutating REST operations".to_owned(), - json!({ - "field": "execution_config.idempotency", - }), - )); - } - } - - if policy.mode == crank_core::IdempotencyMode::Required - && policy.input_field.as_deref().is_none_or(str::is_empty) - && policy.header_name.as_deref().is_none_or(str::is_empty) - { - return Err(ApiError::validation_with_context( - "required idempotency needs input_field or header_name".to_owned(), - json!({ - "field": "execution_config.idempotency", - }), - )); - } - - Ok(()) -} - fn tool_quality_schema_node(schema: &Schema) -> ToolQualitySchemaNode { ToolQualitySchemaNode { kind: tool_quality_schema_kind(&schema.kind), @@ -827,140 +732,7 @@ fn tool_quality_mapping_rule(rule: &MappingRule) -> ToolQualityMappingRule { #[cfg(test)] #[allow(clippy::items_after_test_module)] mod tests { - use std::collections::BTreeMap; - - use crank_core::{ - ExecutionConfig, HttpMethod, IdempotencyMode, IdempotencyPolicy, ResponseCachePolicy, - RestTarget, Target, - }; - - use super::{ - validate_idempotency_policy, validate_profile_display_name, validate_profile_email, - validate_response_cache_policy, - }; - - fn cacheable_execution_config() -> ExecutionConfig { - ExecutionConfig { - timeout_ms: 1_000, - retry_policy: None, - response_cache: Some(ResponseCachePolicy { ttl_ms: 5_000 }), - idempotency: None, - safety: None, - auth_profile_ref: None, - headers: BTreeMap::new(), - } - } - - fn idempotent_execution_config(mode: IdempotencyMode) -> ExecutionConfig { - ExecutionConfig { - timeout_ms: 1_000, - retry_policy: None, - response_cache: None, - idempotency: Some(IdempotencyPolicy { - mode, - ttl_ms: 5_000, - input_field: Some("request_id".to_owned()), - header_name: Some("Idempotency-Key".to_owned()), - }), - safety: None, - auth_profile_ref: None, - headers: BTreeMap::new(), - } - } - - #[test] - fn accepts_response_cache_for_rest_get() { - let target = Target::Rest(RestTarget { - base_url: "http://example.invalid".to_owned(), - method: HttpMethod::Get, - path_template: "/catalog".to_owned(), - static_headers: BTreeMap::new(), - }); - - let result = validate_response_cache_policy(&target, &cacheable_execution_config()); - - assert!(result.is_ok()); - } - - #[test] - fn rejects_response_cache_for_non_get_rest_operation() { - let target = Target::Rest(RestTarget { - base_url: "http://example.invalid".to_owned(), - method: HttpMethod::Post, - path_template: "/catalog".to_owned(), - static_headers: BTreeMap::new(), - }); - - let error = - validate_response_cache_policy(&target, &cacheable_execution_config()).unwrap_err(); - - assert!(matches!(error, crate::error::ApiError::Validation { .. })); - assert_eq!( - error.to_string(), - "response cache is supported only for REST GET operations" - ); - } - - #[test] - fn accepts_idempotency_for_mutating_rest_operation() { - let target = Target::Rest(RestTarget { - base_url: "http://example.invalid".to_owned(), - method: HttpMethod::Post, - path_template: "/orders".to_owned(), - static_headers: BTreeMap::new(), - }); - - let result = validate_idempotency_policy( - &target, - &idempotent_execution_config(IdempotencyMode::Required), - ); - - assert!(result.is_ok()); - } - - #[test] - fn rejects_idempotency_for_rest_get() { - let target = Target::Rest(RestTarget { - base_url: "http://example.invalid".to_owned(), - method: HttpMethod::Get, - path_template: "/orders".to_owned(), - static_headers: BTreeMap::new(), - }); - - let error = validate_idempotency_policy( - &target, - &idempotent_execution_config(IdempotencyMode::Optional), - ) - .unwrap_err(); - - assert!(matches!(error, crate::error::ApiError::Validation { .. })); - assert_eq!( - error.to_string(), - "idempotency is supported only for mutating REST operations" - ); - } - - #[test] - fn rejects_required_idempotency_without_key_source() { - let target = Target::Rest(RestTarget { - base_url: "http://example.invalid".to_owned(), - method: HttpMethod::Post, - path_template: "/orders".to_owned(), - static_headers: BTreeMap::new(), - }); - let mut config = idempotent_execution_config(IdempotencyMode::Required); - let policy = config.idempotency.as_mut().unwrap(); - policy.input_field = None; - policy.header_name = None; - - let error = validate_idempotency_policy(&target, &config).unwrap_err(); - - assert!(matches!(error, crate::error::ApiError::Validation { .. })); - assert_eq!( - error.to_string(), - "required idempotency needs input_field or header_name" - ); - } + use super::{validate_profile_display_name, validate_profile_email}; #[test] fn validates_profile_identity_fields() { diff --git a/apps/admin-api/src/service/operation_validation.rs b/apps/admin-api/src/service/operation_validation.rs new file mode 100644 index 0000000..6b55069 --- /dev/null +++ b/apps/admin-api/src/service/operation_validation.rs @@ -0,0 +1,241 @@ +use crank_core::{Protocol, ResponseCachePolicy, Target}; +use serde_json::json; + +use crate::error::ApiError; + +pub(super) fn validate_protocol_target( + protocol: Protocol, + target: &Target, +) -> Result<(), ApiError> { + let is_match = matches!((protocol, target), (Protocol::Rest, Target::Rest(_))); + + if is_match { + return Ok(()); + } + + Err(ApiError::validation("protocol and target kind must match")) +} + +pub(super) fn validate_response_cache_policy( + target: &Target, + execution_config: &crank_core::ExecutionConfig, +) -> Result<(), ApiError> { + let Some(ResponseCachePolicy { ttl_ms }) = execution_config.response_cache.as_ref() else { + return Ok(()); + }; + + if *ttl_ms == 0 { + return Err(ApiError::validation_with_context( + "response cache ttl must be greater than zero".to_owned(), + json!({ + "field": "execution_config.response_cache.ttl_ms", + }), + )); + } + + match target { + Target::Rest(rest_target) if rest_target.method == crank_core::HttpMethod::Get => {} + _ => { + return Err(ApiError::validation_with_context( + "response cache is supported only for REST GET operations".to_owned(), + json!({ + "field": "execution_config.response_cache", + }), + )); + } + } + + if execution_config.auth_profile_ref.is_some() { + return Err(ApiError::validation_with_context( + "response cache is not supported for operations with auth_profile_ref".to_owned(), + json!({ + "field": "execution_config.auth_profile_ref", + }), + )); + } + + Ok(()) +} + +pub(super) fn validate_idempotency_policy( + target: &Target, + execution_config: &crank_core::ExecutionConfig, +) -> Result<(), ApiError> { + let Some(policy) = execution_config.idempotency.as_ref() else { + return Ok(()); + }; + + if policy.mode == crank_core::IdempotencyMode::Disabled { + return Ok(()); + } + + if policy.ttl_ms == 0 { + return Err(ApiError::validation_with_context( + "idempotency ttl must be greater than zero".to_owned(), + json!({ + "field": "execution_config.idempotency.ttl_ms", + }), + )); + } + + match target { + Target::Rest(rest_target) if rest_target.method != crank_core::HttpMethod::Get => {} + _ => { + return Err(ApiError::validation_with_context( + "idempotency is supported only for mutating REST operations".to_owned(), + json!({ + "field": "execution_config.idempotency", + }), + )); + } + } + + if policy.mode == crank_core::IdempotencyMode::Required + && policy.input_field.as_deref().is_none_or(str::is_empty) + && policy.header_name.as_deref().is_none_or(str::is_empty) + { + return Err(ApiError::validation_with_context( + "required idempotency needs input_field or header_name".to_owned(), + json!({ + "field": "execution_config.idempotency", + }), + )); + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use crank_core::{ + ExecutionConfig, HttpMethod, IdempotencyMode, IdempotencyPolicy, ResponseCachePolicy, + RestTarget, Target, + }; + + use super::{validate_idempotency_policy, validate_response_cache_policy}; + + fn cacheable_execution_config() -> ExecutionConfig { + ExecutionConfig { + timeout_ms: 1_000, + retry_policy: None, + response_cache: Some(ResponseCachePolicy { ttl_ms: 5_000 }), + idempotency: None, + safety: None, + auth_profile_ref: None, + headers: BTreeMap::new(), + } + } + + fn idempotent_execution_config(mode: IdempotencyMode) -> ExecutionConfig { + ExecutionConfig { + timeout_ms: 1_000, + retry_policy: None, + response_cache: None, + idempotency: Some(IdempotencyPolicy { + mode, + ttl_ms: 5_000, + input_field: Some("request_id".to_owned()), + header_name: Some("Idempotency-Key".to_owned()), + }), + safety: None, + auth_profile_ref: None, + headers: BTreeMap::new(), + } + } + + #[test] + fn accepts_response_cache_for_rest_get() { + let target = Target::Rest(RestTarget { + base_url: "http://example.invalid".to_owned(), + method: HttpMethod::Get, + path_template: "/catalog".to_owned(), + static_headers: BTreeMap::new(), + }); + + let result = validate_response_cache_policy(&target, &cacheable_execution_config()); + + assert!(result.is_ok()); + } + + #[test] + fn rejects_response_cache_for_non_get_rest_operation() { + let target = Target::Rest(RestTarget { + base_url: "http://example.invalid".to_owned(), + method: HttpMethod::Post, + path_template: "/catalog".to_owned(), + static_headers: BTreeMap::new(), + }); + + let error = + validate_response_cache_policy(&target, &cacheable_execution_config()).unwrap_err(); + + assert!(matches!(error, crate::error::ApiError::Validation { .. })); + assert_eq!( + error.to_string(), + "response cache is supported only for REST GET operations" + ); + } + + #[test] + fn accepts_idempotency_for_mutating_rest_operation() { + let target = Target::Rest(RestTarget { + base_url: "http://example.invalid".to_owned(), + method: HttpMethod::Post, + path_template: "/orders".to_owned(), + static_headers: BTreeMap::new(), + }); + + let result = validate_idempotency_policy( + &target, + &idempotent_execution_config(IdempotencyMode::Required), + ); + + assert!(result.is_ok()); + } + + #[test] + fn rejects_idempotency_for_rest_get() { + let target = Target::Rest(RestTarget { + base_url: "http://example.invalid".to_owned(), + method: HttpMethod::Get, + path_template: "/orders".to_owned(), + static_headers: BTreeMap::new(), + }); + + let error = validate_idempotency_policy( + &target, + &idempotent_execution_config(IdempotencyMode::Optional), + ) + .unwrap_err(); + + assert!(matches!(error, crate::error::ApiError::Validation { .. })); + assert_eq!( + error.to_string(), + "idempotency is supported only for mutating REST operations" + ); + } + + #[test] + fn rejects_required_idempotency_without_key_source() { + let target = Target::Rest(RestTarget { + base_url: "http://example.invalid".to_owned(), + method: HttpMethod::Post, + path_template: "/orders".to_owned(), + static_headers: BTreeMap::new(), + }); + let mut config = idempotent_execution_config(IdempotencyMode::Required); + let policy = config.idempotency.as_mut().unwrap(); + policy.input_field = None; + policy.header_name = None; + + let error = validate_idempotency_policy(&target, &config).unwrap_err(); + + assert!(matches!(error, crate::error::ApiError::Validation { .. })); + assert_eq!( + error.to_string(), + "required idempotency needs input_field or header_name" + ); + } +}