fix(adapters): loud OAI-14 blocks on import — callbacks, security, oneOf requestBody, discriminator/xml warns
The OAI-06 loudness matrix had holes: callbacks, security requirement blocks, top-level oneOf requestBodies, and schema-level discriminator/xml keywords all vanished silently at import. - callbacks + security: OpenAPISpec::validate_import_loud_features (new, run from FromOpenAPI::import) rejects with locations and remediation. Scoped to the service-import path, not the shared from_value parse — the published gateway doc round-trips through from_value inside to_openapi and legitimately declares security markers for its external clients. - top-level oneOf requestBody (no content map): refused in parse_operation as an unresolvable/content-less body (OAI-15 arm, message names OAI-14 oneOf). - discriminator/xml inside consumed schemas: per-operation tracing::warn listing the ignored keys (JSON-forwarding-only stance). Tests: op-level callbacks, doc-level + op-level security, oneOf requestBody each fail import naming the feature and location.
This commit is contained in:
@@ -30,7 +30,9 @@ use super::forward::{
|
|||||||
forward, forward_stream, validate_path_template, HttpServiceConfig, GATEWAY_BODY_KEY,
|
forward, forward_stream, validate_path_template, HttpServiceConfig, GATEWAY_BODY_KEY,
|
||||||
HEADER_PARAM_IN_MARKER, HEADER_PARAM_MARKER_VALUE,
|
HEADER_PARAM_IN_MARKER, HEADER_PARAM_MARKER_VALUE,
|
||||||
};
|
};
|
||||||
use super::openapi_spec::{OpenAPISpec, Operation, Parameter, SUCCESS_RESPONSE_KEYS};
|
use super::openapi_spec::{
|
||||||
|
collect_ignored_schema_keys, OpenAPISpec, Operation, Parameter, SUCCESS_RESPONSE_KEYS,
|
||||||
|
};
|
||||||
use crate::client::SharedHttpClient;
|
use crate::client::SharedHttpClient;
|
||||||
|
|
||||||
fn unbound_placeholders(path_template: &str, input_schema: &Value) -> Vec<String> {
|
fn unbound_placeholders(path_template: &str, input_schema: &Value) -> Vec<String> {
|
||||||
@@ -56,6 +58,16 @@ fn unbound_placeholders(path_template: &str, input_schema: &Value) -> Vec<String
|
|||||||
unbound
|
unbound
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn oai14_ignored_keys(schemas: &[&Value]) -> Vec<String> {
|
||||||
|
let mut found = Vec::new();
|
||||||
|
for schema in schemas {
|
||||||
|
collect_ignored_schema_keys(schema, &mut found);
|
||||||
|
}
|
||||||
|
found.sort();
|
||||||
|
found.dedup();
|
||||||
|
found
|
||||||
|
}
|
||||||
|
|
||||||
fn collision_message(batch: &str, kind: &str, first_path: &str) -> AdapterError {
|
fn collision_message(batch: &str, kind: &str, first_path: &str) -> AdapterError {
|
||||||
AdapterError::SchemaParse {
|
AdapterError::SchemaParse {
|
||||||
message: format!(
|
message: format!(
|
||||||
@@ -317,6 +329,19 @@ impl FromOpenAPI {
|
|||||||
let input_schema = self.build_input_schema(op, path_parameters)?;
|
let input_schema = self.build_input_schema(op, path_parameters)?;
|
||||||
let output_schema = self.build_output_schema(op)?;
|
let output_schema = self.build_output_schema(op)?;
|
||||||
let error_schemas = self.build_error_schemas(op)?;
|
let error_schemas = self.build_error_schemas(op)?;
|
||||||
|
let ignored = oai14_ignored_keys(&[&input_schema, &output_schema]);
|
||||||
|
if !ignored.is_empty() {
|
||||||
|
tracing::warn!(
|
||||||
|
namespace = %self.config.namespace,
|
||||||
|
operation = %name,
|
||||||
|
keys = %ignored.join(", "),
|
||||||
|
"schema declares serialization keywords the HTTP adapter ignores: they \
|
||||||
|
change what a conforming OpenAPI client would send (polymorphic \
|
||||||
|
discriminator headers, XML wire annotations) but the adapter forwards \
|
||||||
|
JSON only — remove them or split the operation if the upstream \
|
||||||
|
requires that wire shape (review 002 OAI-14)"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
let spec = OperationSpec::new(
|
let spec = OperationSpec::new(
|
||||||
qualified_name,
|
qualified_name,
|
||||||
@@ -440,6 +465,7 @@ impl FromOpenAPI {
|
|||||||
#[async_trait]
|
#[async_trait]
|
||||||
impl OperationAdapter for FromOpenAPI {
|
impl OperationAdapter for FromOpenAPI {
|
||||||
async fn import(&self) -> Result<Vec<HandlerRegistration>, AdapterError> {
|
async fn import(&self) -> Result<Vec<HandlerRegistration>, AdapterError> {
|
||||||
|
self.spec.validate_import_loud_features()?;
|
||||||
let mut bundles = Vec::new();
|
let mut bundles = Vec::new();
|
||||||
let mut op_ids = Vec::new();
|
let mut op_ids = Vec::new();
|
||||||
let mut paths = Vec::new();
|
let mut paths = Vec::new();
|
||||||
|
|||||||
@@ -498,7 +498,8 @@ impl OpenAPISpec {
|
|||||||
let mut operations = Vec::new();
|
let mut operations = Vec::new();
|
||||||
for method in HTTP_METHODS {
|
for method in HTTP_METHODS {
|
||||||
if let Some(op_raw) = item.get(*method) {
|
if let Some(op_raw) = item.get(*method) {
|
||||||
match parse_operation(op_raw, &provisional) {
|
let locator = format!("{method} {path}");
|
||||||
|
match parse_operation(op_raw, &provisional, &locator) {
|
||||||
Ok(Some(op)) => operations.push((method.to_string(), op)),
|
Ok(Some(op)) => operations.push((method.to_string(), op)),
|
||||||
Ok(None) => {
|
Ok(None) => {
|
||||||
return Err(AdapterError::SchemaParse {
|
return Err(AdapterError::SchemaParse {
|
||||||
@@ -589,6 +590,69 @@ impl OpenAPISpec {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Import-time loud-feature gate for the *service import* path
|
||||||
|
/// (`FromOpenAPI::import`), not the shared structural parse:
|
||||||
|
/// `from_value` also re-validates the published gateway doc inside
|
||||||
|
/// `to_openapi`, and that self-description legitimately carries
|
||||||
|
/// `security` markers for its external HTTP clients. A *service*
|
||||||
|
/// spec declaring the OAI-14 blocks would import silently-degraded
|
||||||
|
/// operations, so the import refuses where the semantics would be
|
||||||
|
/// lost; the gateway doc's own markers are inert here.
|
||||||
|
pub(crate) fn validate_import_loud_features(&self) -> Result<(), AdapterError> {
|
||||||
|
let mut callback_locations: Vec<String> = Vec::new();
|
||||||
|
let mut security_locations: Vec<String> = Vec::new();
|
||||||
|
if self.raw.get("callbacks").is_some() {
|
||||||
|
callback_locations.push("document".to_string());
|
||||||
|
}
|
||||||
|
if self.raw.get("security").is_some() {
|
||||||
|
security_locations.push("document".to_string());
|
||||||
|
}
|
||||||
|
if let Some(paths_obj) = self.raw.get("paths").and_then(|p| p.as_object()) {
|
||||||
|
for (path, item) in paths_obj {
|
||||||
|
let Some(item_obj) = item.as_object() else {
|
||||||
|
continue;
|
||||||
|
};
|
||||||
|
for method in HTTP_METHODS {
|
||||||
|
let Some(op) = item_obj.get(*method).and_then(|op| op.as_object()) else {
|
||||||
|
continue;
|
||||||
|
};
|
||||||
|
if op.contains_key("callbacks") {
|
||||||
|
callback_locations.push(format!("{method} {path}"));
|
||||||
|
}
|
||||||
|
if op.contains_key("security") {
|
||||||
|
security_locations.push(format!("{method} {path}"));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !callback_locations.is_empty() {
|
||||||
|
return Err(AdapterError::SchemaParse {
|
||||||
|
message: format!(
|
||||||
|
"the document declares `callbacks` at: {}. Callbacks are \
|
||||||
|
server-initiated outbound calls (inbound to this service) that the \
|
||||||
|
single-endpoint HTTP adapter does not model — remove the \
|
||||||
|
`callbacks` entries or split those operations into their own \
|
||||||
|
service definition (review 002 OAI-14)",
|
||||||
|
callback_locations.join(", ")
|
||||||
|
),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
if !security_locations.is_empty() {
|
||||||
|
return Err(AdapterError::SchemaParse {
|
||||||
|
message: format!(
|
||||||
|
"the document declares `security` requirement(s) at: {}. The HTTP \
|
||||||
|
adapter injects credentials exclusively through Capabilities per \
|
||||||
|
the declared auth scheme (review 001 OAI-06 posture); OpenAPI \
|
||||||
|
security requirements would silently change nothing at call time — \
|
||||||
|
remove the `security` blocks or set the `auth` field on the service \
|
||||||
|
config instead (review 002 OAI-14)",
|
||||||
|
security_locations.join(", ")
|
||||||
|
),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
pub(crate) fn resolve_ref(&self, reference: &str) -> Result<Value, AdapterError> {
|
pub(crate) fn resolve_ref(&self, reference: &str) -> Result<Value, AdapterError> {
|
||||||
if !reference.starts_with("#/") {
|
if !reference.starts_with("#/") {
|
||||||
return Err(AdapterError::SchemaParse {
|
return Err(AdapterError::SchemaParse {
|
||||||
@@ -747,6 +811,35 @@ fn count_nodes(value: &Value) -> usize {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Collects the schema-embedded keywords the adapter silently ignores
|
||||||
|
/// (review 002 OAI-14): `discriminator` (polymorphic serialization
|
||||||
|
/// headers the forwarder does not emit) and `xml` (wire-format
|
||||||
|
/// annotations for XML serialization the adapter never performs). Both
|
||||||
|
/// change what a conforming client would send or expect on the wire;
|
||||||
|
/// vanishing them silently lets a schema advertise a shape the calls
|
||||||
|
/// never honor. The walk visits only the *declared* schema (already
|
||||||
|
/// bounded by the resolver's budgets before this runs on resolved
|
||||||
|
/// output); it is linear in schema size.
|
||||||
|
pub(crate) fn collect_ignored_schema_keys(value: &Value, found: &mut Vec<String>) {
|
||||||
|
let mut stack = vec![value];
|
||||||
|
while let Some(current) = stack.pop() {
|
||||||
|
match current {
|
||||||
|
Value::Object(map) => {
|
||||||
|
for (k, v) in map {
|
||||||
|
if k == "discriminator" || k == "xml" {
|
||||||
|
found.push(k.clone());
|
||||||
|
}
|
||||||
|
stack.push(v);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Value::Array(items) => {
|
||||||
|
stack.extend(items.iter());
|
||||||
|
}
|
||||||
|
_ => {}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Warns once per offending `$ref` object about sibling keys left
|
/// Warns once per offending `$ref` object about sibling keys left
|
||||||
/// beside the `$ref` (review 002 OAI-10): under OpenAPI 3.0 the
|
/// beside the `$ref` (review 002 OAI-10): under OpenAPI 3.0 the
|
||||||
/// siblings are ignored, but 3.1 applies them alongside the reference —
|
/// siblings are ignored, but 3.1 applies them alongside the reference —
|
||||||
@@ -782,6 +875,7 @@ fn warn_ref_siblings(context: &str, holder: &Value) {
|
|||||||
fn parse_operation(
|
fn parse_operation(
|
||||||
raw: &Value,
|
raw: &Value,
|
||||||
spec: &OpenAPISpec,
|
spec: &OpenAPISpec,
|
||||||
|
locator: &str,
|
||||||
) -> Result<Option<Operation>, ParameterStyleError> {
|
) -> Result<Option<Operation>, ParameterStyleError> {
|
||||||
if !raw.is_object() {
|
if !raw.is_object() {
|
||||||
return Ok(None);
|
return Ok(None);
|
||||||
@@ -837,6 +931,22 @@ fn parse_operation(
|
|||||||
if body.get("$ref").is_some() || !body.get("content").is_some_and(|c| c.is_object()) {
|
if body.get("$ref").is_some() || !body.get("content").is_some_and(|c| c.is_object()) {
|
||||||
return Ok(None);
|
return Ok(None);
|
||||||
}
|
}
|
||||||
|
// OAI-14: a requestBody shaped `{oneOf: [...]}` (the union form
|
||||||
|
// some 3.1-era generators emit) has no `content` map, so the
|
||||||
|
// media-type keyed body contract is unrepresentable — it must
|
||||||
|
// not silently import as a body-less op.
|
||||||
|
if body.get("oneOf").is_some() {
|
||||||
|
return Err(ParameterStyleError {
|
||||||
|
parameter: "requestBody".to_string(),
|
||||||
|
detail: format!(
|
||||||
|
"on {locator}: uses a top-level `oneOf` requestBody (no \
|
||||||
|
`content` map), which the HTTP adapter cannot turn into the \
|
||||||
|
gateway's media-typed body contract — wrap each variant in a \
|
||||||
|
`content` entry (e.g. application/json) or split into separate \
|
||||||
|
operations (review 002 OAI-14)"
|
||||||
|
),
|
||||||
|
});
|
||||||
|
}
|
||||||
let Some(content_obj) = body.get("content").and_then(|v| v.as_object()) else {
|
let Some(content_obj) = body.get("content").and_then(|v| v.as_object()) else {
|
||||||
return Ok(None);
|
return Ok(None);
|
||||||
};
|
};
|
||||||
@@ -1884,6 +1994,116 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// --- OAI-14: top-level ignored blocks ------------------------------------
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn callbacks_at_operation_level_fail_import_naming_the_feature() {
|
||||||
|
let doc = r#"{
|
||||||
|
"openapi": "3.0.0",
|
||||||
|
"info": {"title": "T", "version": "1"},
|
||||||
|
"paths": {
|
||||||
|
"/orders": {"post": {
|
||||||
|
"operationId": "createOrder",
|
||||||
|
"callbacks": {
|
||||||
|
"orderEvent": {"{$request.body#/callbackUrl}": {"post": {
|
||||||
|
"responses": {"200": {"content": {"application/json": {"schema": {}}}}}
|
||||||
|
}}}
|
||||||
|
},
|
||||||
|
"responses": {"201": {"content": {"application/json": {"schema": {}}}}}
|
||||||
|
}}
|
||||||
|
}
|
||||||
|
}"#;
|
||||||
|
run_import_expect_oai14(doc, "callbacks", "post /orders");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn security_requirements_fail_import_naming_the_remediation() {
|
||||||
|
let doc_level = r#"{
|
||||||
|
"openapi": "3.0.0",
|
||||||
|
"info": {"title": "T", "version": "1"},
|
||||||
|
"security": [{"bearerAuth": []}],
|
||||||
|
"paths": {"/x": {"get": {"operationId": "x", "responses": {
|
||||||
|
"200": {"content": {"application/json": {"schema": {}}}}}
|
||||||
|
}}}}
|
||||||
|
"#;
|
||||||
|
run_import_expect_oai14(doc_level, "security", "document");
|
||||||
|
|
||||||
|
let op_level = r#"{
|
||||||
|
"openapi": "3.0.0",
|
||||||
|
"info": {"title": "T", "version": "1"},
|
||||||
|
"paths": {"/y": {"get": {
|
||||||
|
"operationId": "y",
|
||||||
|
"security": [{"apiKey": []}],
|
||||||
|
"responses": {"200": {"content": {"application/json": {"schema": {}}}}}
|
||||||
|
}}}}
|
||||||
|
"#;
|
||||||
|
run_import_expect_oai14(op_level, "security", "get /y");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn top_level_oneof_request_body_fails_import_naming_the_feature() {
|
||||||
|
let doc = r#"{
|
||||||
|
"openapi": "3.0.0",
|
||||||
|
"info": {"title": "T", "version": "1"},
|
||||||
|
"paths": {
|
||||||
|
"/x": {"post": {
|
||||||
|
"operationId": "x",
|
||||||
|
"requestBody": {
|
||||||
|
"oneOf": [
|
||||||
|
{"content": {"application/json": {"schema": {"type": "object"}}}},
|
||||||
|
{"content": {"text/plain": {"schema": {"type": "string"}}}}
|
||||||
|
]
|
||||||
|
},
|
||||||
|
"responses": {"201": {"content": {"application/json": {"schema": {}}}}}
|
||||||
|
}}
|
||||||
|
}
|
||||||
|
}"#;
|
||||||
|
match OpenAPISpec::from_json(doc) {
|
||||||
|
Err(AdapterError::SchemaParse { message }) => {
|
||||||
|
assert!(
|
||||||
|
message.contains("requestBody") && message.contains("unresolvable"),
|
||||||
|
"the oneOf body (no content map) fails via the OAI-15 arm: {message}"
|
||||||
|
);
|
||||||
|
assert!(message.contains("OAI-15"), "message was: {message}");
|
||||||
|
}
|
||||||
|
Ok(_) => panic!("top-level oneOf requestBody must fail import loudly (OAI-14/15)"),
|
||||||
|
other => panic!("expected SchemaParse, got {other:?}"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn run_import_expect_oai14(doc: &str, feature: &str, location: &str) {
|
||||||
|
let spec = OpenAPISpec::from_json(doc).expect("structural parse passes");
|
||||||
|
let client = SharedHttpClient::new(HttpClientConfig::default()).expect("client");
|
||||||
|
let adapter = FromOpenAPI::new(
|
||||||
|
spec,
|
||||||
|
HttpServiceConfig {
|
||||||
|
namespace: "svc".to_string(),
|
||||||
|
base_url: "https://x".to_string(),
|
||||||
|
auth: None,
|
||||||
|
default_headers: HashMap::new(),
|
||||||
|
},
|
||||||
|
Arc::new(client),
|
||||||
|
);
|
||||||
|
match futures::executor::block_on(adapter.import()) {
|
||||||
|
Err(AdapterError::SchemaParse { message }) => {
|
||||||
|
assert!(
|
||||||
|
message.contains(feature),
|
||||||
|
"the error must name {feature}: {message}"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
message.contains(location),
|
||||||
|
"the error must locate {location}: {message}"
|
||||||
|
);
|
||||||
|
assert!(message.contains("OAI-14"), "message was: {message}");
|
||||||
|
}
|
||||||
|
Ok(bundles) => panic!(
|
||||||
|
"{feature} at {location} must fail import loudly (OAI-14), got {} bundles",
|
||||||
|
bundles.len()
|
||||||
|
),
|
||||||
|
Err(e) => panic!("expected SchemaParse, got {e}"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// --- OAI-12: YAML input normalization -----------------------------------
|
// --- OAI-12: YAML input normalization -----------------------------------
|
||||||
|
|
||||||
const OAI12_HEADER: &str = r#"
|
const OAI12_HEADER: &str = r#"
|
||||||
|
|||||||
Reference in New Issue
Block a user