From 1df15ce3406dc0c34b813428f64114763efa29cd Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Thu, 27 Mar 2025 17:27:37 +0000 Subject: [PATCH 01/10] CLOUDP-304960: Errors (APIs must return ApiError when errors occur) --- ...PA114ErrorResponsesReferToApiError.test.js | 203 ++++++++++++++++++ tools/spectral/ipa/ipa-spectral.yaml | 1 + tools/spectral/ipa/rulesets/IPA-114.yaml | 18 ++ tools/spectral/ipa/rulesets/README.md | 14 ++ .../IPA114ErrorResponsesReferToApiError.js | 82 +++++++ 5 files changed, 318 insertions(+) create mode 100644 tools/spectral/ipa/__tests__/IPA114ErrorResponsesReferToApiError.test.js create mode 100644 tools/spectral/ipa/rulesets/IPA-114.yaml create mode 100644 tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js diff --git a/tools/spectral/ipa/__tests__/IPA114ErrorResponsesReferToApiError.test.js b/tools/spectral/ipa/__tests__/IPA114ErrorResponsesReferToApiError.test.js new file mode 100644 index 0000000000..6b4b3d3da2 --- /dev/null +++ b/tools/spectral/ipa/__tests__/IPA114ErrorResponsesReferToApiError.test.js @@ -0,0 +1,203 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const components = { + responses: { + badRequest: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/ApiError', + }, + }, + }, + description: 'Bad Request.', + }, + notFound: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/ApiError', + }, + }, + }, + description: 'Not Found.', + }, + internalServerError: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/ApiError', + }, + }, + }, + description: 'Internal Error.', + }, + }, + schemas: { + ApiError: { + type: 'object', + properties: { + error: { + type: 'string', + }, + }, + }, + }, +}; + +testRule('xgen-IPA-114-error-responses-refer-to-api-error', [ + { + name: 'valid error responses with ApiError schema', + document: { + paths: { + '/resources': { + get: { + responses: { + 400: { + $ref: '#/components/responses/badRequest', + }, + 404: { + $ref: '#/components/responses/notFound', + }, + 500: { + $ref: '#/components/responses/internalServerError', + }, + }, + }, + }, + }, + components: components, + }, + errors: [], + }, + { + name: 'invalid error responses missing schema', + document: { + paths: { + '/resources': { + get: { + responses: { + 400: { + content: { + 'application/json': {}, + }, + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-114-error-responses-refer-to-api-error', + message: '400 response must define a schema referencing ApiError', + path: ['paths', '/resources', 'get', 'responses', '400', 'content', 'application/json'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid error responses referencing wrong schema', + document: { + paths: { + '/resources': { + get: { + responses: { + 500: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Error', + }, + }, + }, + }, + }, + }, + }, + }, + components: { + schemas: { + Error: { + type: 'object', + properties: { + message: { + type: 'string', + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-114-error-responses-refer-to-api-error', + message: '500 response must reference ApiError schema', + path: ['paths', '/resources', 'get', 'responses', '500', 'content', 'application/json'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid error responses with inline schema', + document: { + paths: { + '/resources': { + get: { + responses: { + 404: { + content: { + 'application/json': { + schema: { + type: 'object', + properties: { + error: { + type: 'string', + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-114-error-responses-refer-to-api-error', + message: '404 response must reference ApiError schema', + path: ['paths', '/resources', 'get', 'responses', '404', 'content', 'application/json'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'error responses with exception', + document: { + paths: { + '/resources': { + get: { + responses: { + 400: { + content: { + 'application/json': { + schema: { + type: 'object', + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-114-error-responses-refer-to-api-error': 'Reason', + }, + }, + }, + }, + }, + }, + }, + }, + }, + errors: [], + }, +]); diff --git a/tools/spectral/ipa/ipa-spectral.yaml b/tools/spectral/ipa/ipa-spectral.yaml index 3bc7d2371b..8007b39ad5 100644 --- a/tools/spectral/ipa/ipa-spectral.yaml +++ b/tools/spectral/ipa/ipa-spectral.yaml @@ -10,6 +10,7 @@ extends: - ./rulesets/IPA-110.yaml - ./rulesets/IPA-112.yaml - ./rulesets/IPA-113.yaml + - ./rulesets/IPA-114.yaml - ./rulesets/IPA-117.yaml - ./rulesets/IPA-123.yaml - ./rulesets/IPA-125.yaml diff --git a/tools/spectral/ipa/rulesets/IPA-114.yaml b/tools/spectral/ipa/rulesets/IPA-114.yaml new file mode 100644 index 0000000000..c04a38c04b --- /dev/null +++ b/tools/spectral/ipa/rulesets/IPA-114.yaml @@ -0,0 +1,18 @@ +# IPA-114: Errors +# http://go/ipa/114 + +functions: + - IPA114ErrorResponsesReferToApiError + +rules: + xgen-IPA-114-error-responses-refer-to-api-error: + description: | + APIs must return ApiError when errors occur + + ##### Implementation details + This rule checks that all 4xx and 5xx error responses reference the ApiError schema. + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-114-error-responses-refer-to-api-error' + given: '$.paths[*][*].responses[?(@property.match(/^[45]\d\d$/))]' + severity: warn + then: + function: 'IPA114ErrorResponsesReferToApiError' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index e0e3827ce3..02f1ee5746 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -603,6 +603,20 @@ Rule checks for the following conditions: +### IPA-114 + +Rules are based on [http://go/ipa/IPA-114](http://go/ipa/IPA-114). + +#### xgen-IPA-114-error-responses-refer-to-api-error + + ![warn](https://img.shields.io/badge/warning-yellow) +APIs must return ApiError when errors occur + +##### Implementation details +This rule checks that all 4xx and 5xx error responses reference the ApiError schema. + + + ### IPA-117 Rules are based on [http://go/ipa/IPA-117](http://go/ipa/IPA-117). diff --git a/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js new file mode 100644 index 0000000000..cdfe3c8623 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js @@ -0,0 +1,82 @@ +import { hasException } from './utils/exceptions.js'; +import { + collectAdoption, + collectAndReturnViolation, + collectException, + handleInternalError, +} from './utils/collectionUtils.js'; +import { resolveObject } from './utils/componentUtils.js'; +import { getSchemaNameFromRef } from './utils/methodUtils.js'; + +const RULE_NAME = 'xgen-IPA-114-error-responses-refer-to-api-error'; + +/** + * Verifies that 4xx and 5xx responses reference the ApiError schema + * + * @param {object} input - The response object to check + * @param {object} _ - Rule options (unused) + * @param {{ path: string[], documentInventory: object}} context - The context object containing the path and document + * @returns {object|void} - Violation object if any errors found, otherwise undefined + */ +export default (input, _, { path, documentInventory }) => { + const oas = documentInventory.unresolved; + const apiResponseObject = resolveObject(oas, path); + const errorCode = path[path.length - 1]; // e.g., "400", "404", "500" + + const errors = checkViolationsAndReturnErrors(apiResponseObject, oas, path, errorCode); + + if (errors.length !== 0) { + return collectAndReturnViolation(path, RULE_NAME, errors); + } + + collectAdoption(path, RULE_NAME); +}; + +function checkViolationsAndReturnErrors(apiResponseObject, oas, path, errorCode) { + try { + const errors = []; + let content; + + if (apiResponseObject.content) { + content = apiResponseObject.content; + } else if (apiResponseObject.$ref) { + const schemaName = getSchemaNameFromRef(apiResponseObject.$ref); + const responseSchema = resolveObject(oas, ['components', 'responses', schemaName]); + content = responseSchema.content; + } + + for (const [mediaType, mediaTypeObj] of Object.entries(content)) { + if (!mediaType.endsWith('json')) { + continue; + } + + if (hasException(mediaTypeObj, RULE_NAME)) { + collectException(mediaTypeObj, RULE_NAME, [...path, 'content', mediaType]); + continue; + } + + const contentPath = [...path, 'content', mediaType]; + + // Check if schema exists + if (!mediaTypeObj || !mediaTypeObj.schema) { + errors.push({ + path: contentPath, + message: `${errorCode} response must define a schema referencing ApiError`, + }); + continue; + } + + // Check if schema references ApiError + const schema = mediaTypeObj.schema; + if (!schema.$ref || !schema.$ref.endsWith('/ApiError')) { + errors.push({ + path: contentPath, + message: `${errorCode} response must reference ApiError schema`, + }); + } + } + return errors; + } catch (e) { + handleInternalError(RULE_NAME, path, e); + } +} From 2d43dbc701a55dc901848f5412da3bf0ca5ea5b4 Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Thu, 27 Mar 2025 17:33:28 +0000 Subject: [PATCH 02/10] docs fix --- .../rulesets/functions/IPA114ErrorResponsesReferToApiError.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js index cdfe3c8623..b16ad907ee 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js +++ b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js @@ -15,8 +15,7 @@ const RULE_NAME = 'xgen-IPA-114-error-responses-refer-to-api-error'; * * @param {object} input - The response object to check * @param {object} _ - Rule options (unused) - * @param {{ path: string[], documentInventory: object}} context - The context object containing the path and document - * @returns {object|void} - Violation object if any errors found, otherwise undefined + * @param {object} context - The context object containing path and document information */ export default (input, _, { path, documentInventory }) => { const oas = documentInventory.unresolved; From be645e76bc3e2736e31bbb62e961f877d89a1757 Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Fri, 28 Mar 2025 08:58:05 +0000 Subject: [PATCH 03/10] additional checks --- ...PA114ErrorResponsesReferToApiError.test.js | 33 +++++++++++++++++-- .../IPA114ErrorResponsesReferToApiError.js | 33 +++++++++++++------ 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/tools/spectral/ipa/__tests__/IPA114ErrorResponsesReferToApiError.test.js b/tools/spectral/ipa/__tests__/IPA114ErrorResponsesReferToApiError.test.js index 6b4b3d3da2..91a83a313f 100644 --- a/tools/spectral/ipa/__tests__/IPA114ErrorResponsesReferToApiError.test.js +++ b/tools/spectral/ipa/__tests__/IPA114ErrorResponsesReferToApiError.test.js @@ -91,12 +91,34 @@ testRule('xgen-IPA-114-error-responses-refer-to-api-error', [ errors: [ { code: 'xgen-IPA-114-error-responses-refer-to-api-error', - message: '400 response must define a schema referencing ApiError', + message: '400 response must define a schema referencing ApiError.', path: ['paths', '/resources', 'get', 'responses', '400', 'content', 'application/json'], severity: DiagnosticSeverity.Warning, }, ], }, + { + name: 'invalid error responses missing content', + document: { + paths: { + '/resources': { + get: { + responses: { + 400: {}, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-114-error-responses-refer-to-api-error', + message: '400 response must define content with ApiError schema reference.', + path: ['paths', '/resources', 'get', 'responses', '400'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, { name: 'invalid error responses referencing wrong schema', document: { @@ -133,7 +155,7 @@ testRule('xgen-IPA-114-error-responses-refer-to-api-error', [ errors: [ { code: 'xgen-IPA-114-error-responses-refer-to-api-error', - message: '500 response must reference ApiError schema', + message: '500 response must reference ApiError schema.', path: ['paths', '/resources', 'get', 'responses', '500', 'content', 'application/json'], severity: DiagnosticSeverity.Warning, }, @@ -168,7 +190,7 @@ testRule('xgen-IPA-114-error-responses-refer-to-api-error', [ errors: [ { code: 'xgen-IPA-114-error-responses-refer-to-api-error', - message: '404 response must reference ApiError schema', + message: '404 response must reference ApiError schema.', path: ['paths', '/resources', 'get', 'responses', '404', 'content', 'application/json'], severity: DiagnosticSeverity.Warning, }, @@ -193,6 +215,11 @@ testRule('xgen-IPA-114-error-responses-refer-to-api-error', [ }, }, }, + 500: { + 'x-xgen-IPA-exception': { + 'xgen-IPA-114-error-responses-refer-to-api-error': 'Reason', + }, + }, }, }, }, diff --git a/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js index b16ad907ee..8d0a05d771 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js +++ b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js @@ -18,17 +18,27 @@ const RULE_NAME = 'xgen-IPA-114-error-responses-refer-to-api-error'; * @param {object} context - The context object containing path and document information */ export default (input, _, { path, documentInventory }) => { - const oas = documentInventory.unresolved; - const apiResponseObject = resolveObject(oas, path); - const errorCode = path[path.length - 1]; // e.g., "400", "404", "500" + try { + const oas = documentInventory.unresolved; + const apiResponseObject = resolveObject(oas, path); + const errorCode = path[path.length - 1]; + + // Check for exception at response level + if (hasException(apiResponseObject, RULE_NAME)) { + collectException(apiResponseObject, RULE_NAME, path); + return; + } - const errors = checkViolationsAndReturnErrors(apiResponseObject, oas, path, errorCode); + const errors = checkViolationsAndReturnErrors(apiResponseObject, oas, path, errorCode); + if (errors.length !== 0) { + return collectAndReturnViolation(path, RULE_NAME, errors); + } - if (errors.length !== 0) { - return collectAndReturnViolation(path, RULE_NAME, errors); + collectAdoption(path, RULE_NAME); + } catch(e) { + handleInternalError(RULE_NAME, path, e); } - collectAdoption(path, RULE_NAME); }; function checkViolationsAndReturnErrors(apiResponseObject, oas, path, errorCode) { @@ -42,6 +52,8 @@ function checkViolationsAndReturnErrors(apiResponseObject, oas, path, errorCode) const schemaName = getSchemaNameFromRef(apiResponseObject.$ref); const responseSchema = resolveObject(oas, ['components', 'responses', schemaName]); content = responseSchema.content; + } else { + return [{ path, message: `${errorCode} response must define content with ApiError schema reference.` }]; } for (const [mediaType, mediaTypeObj] of Object.entries(content)) { @@ -57,20 +69,21 @@ function checkViolationsAndReturnErrors(apiResponseObject, oas, path, errorCode) const contentPath = [...path, 'content', mediaType]; // Check if schema exists - if (!mediaTypeObj || !mediaTypeObj.schema) { + if (!mediaTypeObj.schema) { errors.push({ path: contentPath, - message: `${errorCode} response must define a schema referencing ApiError`, + message: `${errorCode} response must define a schema referencing ApiError.`, }); continue; } // Check if schema references ApiError const schema = mediaTypeObj.schema; + if (!schema.$ref || !schema.$ref.endsWith('/ApiError')) { errors.push({ path: contentPath, - message: `${errorCode} response must reference ApiError schema`, + message: `${errorCode} response must reference ApiError schema.`, }); } } From c2eb28802e1f145ff459e8d85bb2aaa28d714499 Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Fri, 28 Mar 2025 08:59:43 +0000 Subject: [PATCH 04/10] additional checks --- .../functions/IPA114ErrorResponsesReferToApiError.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js index 8d0a05d771..228aa03186 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js +++ b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js @@ -18,7 +18,6 @@ const RULE_NAME = 'xgen-IPA-114-error-responses-refer-to-api-error'; * @param {object} context - The context object containing path and document information */ export default (input, _, { path, documentInventory }) => { - try { const oas = documentInventory.unresolved; const apiResponseObject = resolveObject(oas, path); const errorCode = path[path.length - 1]; @@ -35,10 +34,6 @@ export default (input, _, { path, documentInventory }) => { } collectAdoption(path, RULE_NAME); - } catch(e) { - handleInternalError(RULE_NAME, path, e); - } - }; function checkViolationsAndReturnErrors(apiResponseObject, oas, path, errorCode) { From d2ff6fa2b2544fc93a17f951fcd82150571ff309 Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Fri, 28 Mar 2025 09:01:25 +0000 Subject: [PATCH 05/10] test log --- .../functions/IPA114ErrorResponsesReferToApiError.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js index 228aa03186..af98831eb2 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js +++ b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js @@ -18,6 +18,7 @@ const RULE_NAME = 'xgen-IPA-114-error-responses-refer-to-api-error'; * @param {object} context - The context object containing path and document information */ export default (input, _, { path, documentInventory }) => { + try { const oas = documentInventory.unresolved; const apiResponseObject = resolveObject(oas, path); const errorCode = path[path.length - 1]; @@ -34,6 +35,11 @@ export default (input, _, { path, documentInventory }) => { } collectAdoption(path, RULE_NAME); + } catch(e) { + handleInternalError(RULE_NAME, path, e); + console.log(e); + } + }; function checkViolationsAndReturnErrors(apiResponseObject, oas, path, errorCode) { From 46f534ea4bc581a65b6e33c93d38566f73297227 Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Fri, 28 Mar 2025 09:06:27 +0000 Subject: [PATCH 06/10] test log --- tools/spectral/ipa/rulesets/IPA-114.yaml | 2 +- .../rulesets/functions/IPA114ErrorResponsesReferToApiError.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/spectral/ipa/rulesets/IPA-114.yaml b/tools/spectral/ipa/rulesets/IPA-114.yaml index c04a38c04b..1fe9d56b58 100644 --- a/tools/spectral/ipa/rulesets/IPA-114.yaml +++ b/tools/spectral/ipa/rulesets/IPA-114.yaml @@ -12,7 +12,7 @@ rules: ##### Implementation details This rule checks that all 4xx and 5xx error responses reference the ApiError schema. message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-114-error-responses-refer-to-api-error' - given: '$.paths[*][*].responses[?(@property.match(/^[45]\d\d$/))]' severity: warn + given: '$.paths[*][*].responses[?(@property.match(/^[45]\d\d$/))]' then: function: 'IPA114ErrorResponsesReferToApiError' diff --git a/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js index af98831eb2..72c42e282a 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js +++ b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js @@ -38,6 +38,8 @@ export default (input, _, { path, documentInventory }) => { } catch(e) { handleInternalError(RULE_NAME, path, e); console.log(e); + console.log(RULE_NAME); + console.log(path); } }; From 408e83675c8c90e40b70f826e8c03f9b606287e8 Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Fri, 28 Mar 2025 09:07:58 +0000 Subject: [PATCH 07/10] test log --- .../functions/IPA114ErrorResponsesReferToApiError.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js index 72c42e282a..44aabfe23b 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js +++ b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js @@ -18,11 +18,14 @@ const RULE_NAME = 'xgen-IPA-114-error-responses-refer-to-api-error'; * @param {object} context - The context object containing path and document information */ export default (input, _, { path, documentInventory }) => { - try { const oas = documentInventory.unresolved; const apiResponseObject = resolveObject(oas, path); const errorCode = path[path.length - 1]; + console.log(RULE_NAME); + console.log(path); + console.log(input); + // Check for exception at response level if (hasException(apiResponseObject, RULE_NAME)) { collectException(apiResponseObject, RULE_NAME, path); @@ -35,13 +38,6 @@ export default (input, _, { path, documentInventory }) => { } collectAdoption(path, RULE_NAME); - } catch(e) { - handleInternalError(RULE_NAME, path, e); - console.log(e); - console.log(RULE_NAME); - console.log(path); - } - }; function checkViolationsAndReturnErrors(apiResponseObject, oas, path, errorCode) { From 7cc8e068073afbe333de137e078b718377beb8aa Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Fri, 28 Mar 2025 09:10:28 +0000 Subject: [PATCH 08/10] remove test logs --- .../rulesets/functions/IPA114ErrorResponsesReferToApiError.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js index 44aabfe23b..228aa03186 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js +++ b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js @@ -22,10 +22,6 @@ export default (input, _, { path, documentInventory }) => { const apiResponseObject = resolveObject(oas, path); const errorCode = path[path.length - 1]; - console.log(RULE_NAME); - console.log(path); - console.log(input); - // Check for exception at response level if (hasException(apiResponseObject, RULE_NAME)) { collectException(apiResponseObject, RULE_NAME, path); From 281b54c878a749519cd0fff42c543655b75fabc8 Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Fri, 28 Mar 2025 09:12:34 +0000 Subject: [PATCH 09/10] prettier fix --- .../IPA114ErrorResponsesReferToApiError.js | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js index 228aa03186..d040a5eec7 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js +++ b/tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js @@ -18,22 +18,22 @@ const RULE_NAME = 'xgen-IPA-114-error-responses-refer-to-api-error'; * @param {object} context - The context object containing path and document information */ export default (input, _, { path, documentInventory }) => { - const oas = documentInventory.unresolved; - const apiResponseObject = resolveObject(oas, path); - const errorCode = path[path.length - 1]; + const oas = documentInventory.unresolved; + const apiResponseObject = resolveObject(oas, path); + const errorCode = path[path.length - 1]; - // Check for exception at response level - if (hasException(apiResponseObject, RULE_NAME)) { - collectException(apiResponseObject, RULE_NAME, path); - return; - } + // Check for exception at response level + if (hasException(apiResponseObject, RULE_NAME)) { + collectException(apiResponseObject, RULE_NAME, path); + return; + } - const errors = checkViolationsAndReturnErrors(apiResponseObject, oas, path, errorCode); - if (errors.length !== 0) { - return collectAndReturnViolation(path, RULE_NAME, errors); - } + const errors = checkViolationsAndReturnErrors(apiResponseObject, oas, path, errorCode); + if (errors.length !== 0) { + return collectAndReturnViolation(path, RULE_NAME, errors); + } - collectAdoption(path, RULE_NAME); + collectAdoption(path, RULE_NAME); }; function checkViolationsAndReturnErrors(apiResponseObject, oas, path, errorCode) { From 331eea2ec0a99043b486557063c96e3afb56b4c4 Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Fri, 28 Mar 2025 13:04:41 +0000 Subject: [PATCH 10/10] CLOUDP-304961: IPA-114: Errors (ApiError schema properties) --- .../IPA114ApiErrorHasBadRequestDetail.test.js | 168 ++++++++++++++++++ tools/spectral/ipa/rulesets/IPA-114.yaml | 16 ++ tools/spectral/ipa/rulesets/README.md | 12 ++ .../IPA114ApiErrorHasBadRequestDetail.js | 68 +++++++ 4 files changed, 264 insertions(+) create mode 100644 tools/spectral/ipa/__tests__/IPA114ApiErrorHasBadRequestDetail.test.js create mode 100644 tools/spectral/ipa/rulesets/functions/IPA114ApiErrorHasBadRequestDetail.js diff --git a/tools/spectral/ipa/__tests__/IPA114ApiErrorHasBadRequestDetail.test.js b/tools/spectral/ipa/__tests__/IPA114ApiErrorHasBadRequestDetail.test.js new file mode 100644 index 0000000000..ea853a382b --- /dev/null +++ b/tools/spectral/ipa/__tests__/IPA114ApiErrorHasBadRequestDetail.test.js @@ -0,0 +1,168 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const validDocument = { + components: { + schemas: { + ApiError: { + properties: { + badRequestDetail: { + $ref: '#/components/schemas/BadRequestDetail', + }, + detail: { type: 'string' }, + }, + }, + BadRequestDetail: { + properties: { + fields: { + type: 'array', + items: { + $ref: '#/components/schemas/FieldViolation', + }, + }, + }, + }, + FieldViolation: { + properties: { + description: { type: 'string' }, + field: { type: 'string' }, + }, + required: ['description', 'field'], + }, + }, + }, +}; + +testRule('xgen-IPA-114-api-error-has-bad-request-detail', [ + { + name: 'valid ApiError schema passes validation', + document: validDocument, + errors: [], + }, + { + name: 'missing properties in ApiError fails', + document: { + components: { + schemas: { + ApiError: {}, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-114-api-error-has-bad-request-detail', + message: 'ApiError schema must have badRequestDetail field.', + path: ['components', 'schemas', 'ApiError'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'missing badRequestDetail field fails', + document: { + components: { + schemas: { + ApiError: { + properties: { + detail: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-114-api-error-has-bad-request-detail', + message: 'ApiError schema must have badRequestDetail field.', + path: ['components', 'schemas', 'ApiError'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'badRequestDetail without fields property fails', + document: { + components: { + schemas: { + ApiError: { + properties: { + badRequestDetail: { + properties: { + someOtherProperty: { type: 'string' }, + }, + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-114-api-error-has-bad-request-detail', + message: 'badRequestDetail must include an array of fields.', + path: ['components', 'schemas', 'ApiError'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'fields not being an array fails', + document: { + components: { + schemas: { + ApiError: { + properties: { + badRequestDetail: { + properties: { + fields: { + type: 'object', + }, + }, + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-114-api-error-has-bad-request-detail', + message: 'badRequestDetail must include an array of fields.', + path: ['components', 'schemas', 'ApiError'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'missing description or field properties fails', + document: { + components: { + schemas: { + ApiError: { + properties: { + badRequestDetail: { + properties: { + fields: { + type: 'array', + properties: { + // Missing description and field properties + otherProperty: { type: 'string' }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-114-api-error-has-bad-request-detail', + message: 'Each field must include description and field properties.', + path: ['components', 'schemas', 'ApiError'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, +]); diff --git a/tools/spectral/ipa/rulesets/IPA-114.yaml b/tools/spectral/ipa/rulesets/IPA-114.yaml index 1fe9d56b58..e6fc26815f 100644 --- a/tools/spectral/ipa/rulesets/IPA-114.yaml +++ b/tools/spectral/ipa/rulesets/IPA-114.yaml @@ -3,6 +3,7 @@ functions: - IPA114ErrorResponsesReferToApiError + - IPA114ApiErrorHasBadRequestDetail rules: xgen-IPA-114-error-responses-refer-to-api-error: @@ -16,3 +17,18 @@ rules: given: '$.paths[*][*].responses[?(@property.match(/^[45]\d\d$/))]' then: function: 'IPA114ErrorResponsesReferToApiError' + xgen-IPA-114-api-error-has-bad-request-detail: + description: | + ApiError schema should have badRequestDetail field with proper structure. + + ##### Implementation details + Rule checks that: + - ApiError schema has badRequestDetail field + - badRequestDetail must include an array of fields + - Each field must include description and field properties + - This rule does not allow exceptions + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-114-api-error-has-bad-request-detail' + severity: warn + given: $.components.schemas.ApiError + then: + function: 'IPA114ApiErrorHasBadRequestDetail' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index e5036fde09..48d8505100 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -615,6 +615,18 @@ APIs must return ApiError when errors occur ##### Implementation details This rule checks that all 4xx and 5xx error responses reference the ApiError schema. +#### xgen-IPA-114-api-error-has-bad-request-detail + + ![warn](https://img.shields.io/badge/warning-yellow) +ApiError schema should have badRequestDetail field with proper structure. + +##### Implementation details +Rule checks that: +- ApiError schema has badRequestDetail field +- badRequestDetail must include an array of fields +- Each field must include description and field properties +- This rule does not allow exceptions + ### IPA-117 diff --git a/tools/spectral/ipa/rulesets/functions/IPA114ApiErrorHasBadRequestDetail.js b/tools/spectral/ipa/rulesets/functions/IPA114ApiErrorHasBadRequestDetail.js new file mode 100644 index 0000000000..b1ac069135 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/IPA114ApiErrorHasBadRequestDetail.js @@ -0,0 +1,68 @@ +import { collectAdoption, collectAndReturnViolation, handleInternalError } from './utils/collectionUtils.js'; + +const RULE_NAME = 'xgen-IPA-114-api-error-has-bad-request-detail'; + +/** + * Verifies that ApiError schema has badRequestDetail field with proper structure + * + * @param {object} input - The ApiError schema object + * @param {object} _ - Rule options (unused) + * @param {object} context - The context object containing path and document information + */ +export default (input, _, { path, documentInventory }) => { + const errors = checkViolationsAndReturnErrors(input, documentInventory, path); + if (errors.length > 0) { + return collectAndReturnViolation(path, RULE_NAME, errors); + } + + collectAdoption(path, RULE_NAME); +}; + +/** + * Check for violations in ApiError schema structure + * + * @param {object} apiErrorSchema - The ApiError schema object to validate + * @param {object} documentInventory - Contains document information + * @param {Array} path - Path to the schema in the document + * @returns {Array} - Array of error objects + */ +function checkViolationsAndReturnErrors(apiErrorSchema, documentInventory, path) { + try { + // ApiError should have badRequestDetail property + if (!apiErrorSchema.properties?.badRequestDetail) { + return [ + { + path, + message: 'ApiError schema must have badRequestDetail field.', + }, + ]; + } + + //badRequestDetail must include an array of fields + const badRequestDetail = apiErrorSchema.properties.badRequestDetail; + if (badRequestDetail.properties?.fields?.type !== 'array') { + return [ + { + path, + message: 'badRequestDetail must include an array of fields.', + }, + ]; + } + + //Each field must include description and field properties + const fields = badRequestDetail.properties.fields; + if (!fields.items?.properties?.description && !fields.items?.properties?.field) { + return [ + { + path, + message: 'Each field must include description and field properties.', + }, + ]; + } + + return []; + } catch (e) { + handleInternalError(RULE_NAME, path, e); + return [{ path, message: `Internal error during validation: ${e.message}` }]; + } +}