From c89e8cc695703f87403364c2ec228b8e65361662 Mon Sep 17 00:00:00 2001 From: akhilailla Date: Wed, 1 Nov 2023 14:21:48 -0500 Subject: [PATCH 1/3] Exclude get verb from LroExtension rule validation --- packages/rulesets/src/spectral/az-common.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rulesets/src/spectral/az-common.ts b/packages/rulesets/src/spectral/az-common.ts index 1a6fa1d50..6205866d3 100644 --- a/packages/rulesets/src/spectral/az-common.ts +++ b/packages/rulesets/src/spectral/az-common.ts @@ -98,7 +98,7 @@ const ruleset: any = { message: "Operations with a 202 response must specify `x-ms-long-running-operation: true`.", severity: "error", formats: [oas2], - given: "$.paths[*][*].responses[?(@property == '202')]^^", + given: "$.paths[*][put,patch,post,delete].responses[?(@property == '202')]^^", then: { field: "x-ms-long-running-operation", function: truthy, From 290a9a6f31d9474a343fc745328dbb4318136f47 Mon Sep 17 00:00:00 2001 From: akhilailla Date: Thu, 2 Nov 2023 10:25:09 -0500 Subject: [PATCH 2/3] Update RPC-GET-V1-01(GetOperation200) to allow GET with 202 response code and RPC-Post-V1-09(LroExtension) to exclude GET from x-ms-long-running-operation presence validation --- ...09_and_RPC-Get-V1-01_2023-11-02-15-20.json | 10 ++++++ docs/get-operation200.md | 33 ----------------- docs/get-response-codes.md | 36 +++++++++++++++++++ docs/rules.md | 5 +-- .../rulesets/generated/spectral/az-arm.js | 12 +++---- .../rulesets/generated/spectral/az-common.js | 2 +- .../generated/spectral/az-dataplane.js | 2 +- packages/rulesets/src/spectral/az-arm.ts | 16 +++++---- packages/rulesets/src/spectral/az-common.ts | 1 + .../functions/patch-response-codes.ts | 2 +- 10 files changed, 68 insertions(+), 51 deletions(-) create mode 100644 common/changes/@microsoft.azure/openapi-validator-rulesets/akhilailla-fix_RPC-Post-V1-09_and_RPC-Get-V1-01_2023-11-02-15-20.json delete mode 100644 docs/get-operation200.md create mode 100644 docs/get-response-codes.md diff --git a/common/changes/@microsoft.azure/openapi-validator-rulesets/akhilailla-fix_RPC-Post-V1-09_and_RPC-Get-V1-01_2023-11-02-15-20.json b/common/changes/@microsoft.azure/openapi-validator-rulesets/akhilailla-fix_RPC-Post-V1-09_and_RPC-Get-V1-01_2023-11-02-15-20.json new file mode 100644 index 000000000..d058276c9 --- /dev/null +++ b/common/changes/@microsoft.azure/openapi-validator-rulesets/akhilailla-fix_RPC-Post-V1-09_and_RPC-Get-V1-01_2023-11-02-15-20.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft.azure/openapi-validator-rulesets", + "comment": "Update RPC-GET-V1-01(GetOperation200) to allow GET with 202 response code and RPC-Post-V1-09(LroExtension) to exclude GET from x-ms-long-running-operation presence validation", + "type": "patch" + } + ], + "packageName": "@microsoft.azure/openapi-validator-rulesets" +} \ No newline at end of file diff --git a/docs/get-operation200.md b/docs/get-operation200.md deleted file mode 100644 index 36ade184c..000000000 --- a/docs/get-operation200.md +++ /dev/null @@ -1,33 +0,0 @@ -# GetOperation200 - -## Category - -ARM Error - -## Applies to - -ARM OpenAPI(swagger) specs - -## Related ARM Guideline Code - -- RPC-Get-V1-01 - -## Output Message - -The get operation should only return 200. - -## Description - -The get operation should only return 200, also it should not be a long running operation. - -## CreatedAt - -July 07, 2022 - -## LastModifiedAt - -July 07, 2022 - -## How to fix the violation - -Considering removing the other response codes except 200. diff --git a/docs/get-response-codes.md b/docs/get-response-codes.md new file mode 100644 index 000000000..dec8cdb4e --- /dev/null +++ b/docs/get-response-codes.md @@ -0,0 +1,36 @@ +# GetResponseCodes + +## Category + +ARM Error + +## Applies to + +ARM OpenAPI(swagger) specs + +## Related ARM Guideline Code + +- RPC-Get-V1-01 + +## Output Message + +The get operation should only return 200. +In addition, it can return 202 only if it has "Location" header defined (i.e, if it is a polling action). + +## Description + +The get operation should only return 200, also it should not be a long running operation. +In addition, it can return 202 only if it has location header defined (i.e, if it is a polling action). + +## CreatedAt + +July 07, 2022 + +## LastModifiedAt + +July 07, 2022 + +## How to fix the violation + +Remove all the other response codes except 200 and 202 with "Location" header defined +i.e, remove response codes 201, 202(if no "Location" header defined), 203, 204. diff --git a/docs/rules.md b/docs/rules.md index 132588853..55642a470 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -380,11 +380,12 @@ The GET calls are synchronous and it MUST NOT have Please refer to [get-operation-must-not-be-long-running.md](./get-operation-must-not-be-long-running.md) for details. -### GetOperation200 +### GetResponseCodes The get operation should only return 200, also it should not be a long running operation. +In addition, it can return 202 only if it has location header defined (i.e, if it is a polling action). -Please refer to [get-operation200.md](./get-operation200.md) for details. +Please refer to [get-response-codes.md](./get-response-codes.md) for details. ### GuidUsage diff --git a/packages/rulesets/generated/spectral/az-arm.js b/packages/rulesets/generated/spectral/az-arm.js index 2c31f350a..ff3f1b01b 100644 --- a/packages/rulesets/generated/spectral/az-arm.js +++ b/packages/rulesets/generated/spectral/az-arm.js @@ -892,7 +892,7 @@ const ruleset$1 = { message: "Operations with a 202 response must specify `x-ms-long-running-operation: true`.", severity: "error", formats: [oas2], - given: "$.paths[*][*].responses[?(@property == '202')]^^", + given: "$.paths[*][put,patch,post,delete].responses[?(@property == '202')]^^", then: { field: "x-ms-long-running-operation", function: truthy, @@ -2058,7 +2058,7 @@ const LR_PATCH_RESPONSES = ["200", "202", "default"]; const SYNC_ERROR$1 = "Synchronous PATCH operations must have responses with 200 and default return codes. They also must not have other response codes."; const LR_ERROR$1 = "Long-running PATCH operations must have responses with 200, 202 and default return codes. They also must not have other response codes."; const EmptyResponse_ERROR$2 = "PATCH operation response codes must be non-empty. It must have response codes 200 and default if it is sync or 200, 202 and default if it is long running."; -const PatchResponseCodes = (patchOp, _opts, ctx) => { +const patchResponseCodes = (patchOp, _opts, ctx) => { var _a; if (patchOp === null || typeof patchOp !== "object") { return []; @@ -3033,13 +3033,13 @@ const ruleset = { function: falsy, }, }, - GetOperation200: { - description: "The get operation should only return 200.", + GetResponseCodes: { + description: "The GET operation should only return 200. In addition, it can return 202 only if it has \"Location\" header defined", message: "{{description}}", severity: "error", resolved: true, formats: [oas2], - given: ["$[paths,'x-ms-paths'].*[get].responses['201','202','203','204']"], + given: ["$[paths,'x-ms-paths'].*[get].responses['201','203','204']"], then: { function: falsy, }, @@ -3150,7 +3150,7 @@ const ruleset = { formats: [oas2], given: ["$[paths,'x-ms-paths'].*[patch]"], then: { - function: PatchResponseCodes, + function: patchResponseCodes, }, }, LroPatch202: { diff --git a/packages/rulesets/generated/spectral/az-common.js b/packages/rulesets/generated/spectral/az-common.js index 94fe71694..8d66338f7 100644 --- a/packages/rulesets/generated/spectral/az-common.js +++ b/packages/rulesets/generated/spectral/az-common.js @@ -662,7 +662,7 @@ const ruleset = { message: "Operations with a 202 response must specify `x-ms-long-running-operation: true`.", severity: "error", formats: [oas2], - given: "$.paths[*][*].responses[?(@property == '202')]^^", + given: "$.paths[*][put,patch,post,delete].responses[?(@property == '202')]^^", then: { field: "x-ms-long-running-operation", function: truthy, diff --git a/packages/rulesets/generated/spectral/az-dataplane.js b/packages/rulesets/generated/spectral/az-dataplane.js index 3f94710a3..d0db8bfe0 100644 --- a/packages/rulesets/generated/spectral/az-dataplane.js +++ b/packages/rulesets/generated/spectral/az-dataplane.js @@ -692,7 +692,7 @@ const ruleset$1 = { message: "Operations with a 202 response must specify `x-ms-long-running-operation: true`.", severity: "error", formats: [oas2], - given: "$.paths[*][*].responses[?(@property == '202')]^^", + given: "$.paths[*][put,patch,post,delete].responses[?(@property == '202')]^^", then: { field: "x-ms-long-running-operation", function: truthy, diff --git a/packages/rulesets/src/spectral/az-arm.ts b/packages/rulesets/src/spectral/az-arm.ts index 21811b026..a8ac8fd10 100644 --- a/packages/rulesets/src/spectral/az-arm.ts +++ b/packages/rulesets/src/spectral/az-arm.ts @@ -28,7 +28,7 @@ import { ParametersInPointGet } from "./functions/parameters-in-point-get" import { ParametersInPost } from "./functions/parameters-in-post" import pathBodyParameters from "./functions/patch-body-parameters" import { patchPropertiesCorrespondToPutProperties } from "./functions/patch-properties-correspond-to-put-properties" -import { PatchResponseCodes } from "./functions/patch-response-codes" +import { patchResponseCodes } from "./functions/patch-response-codes" import pathForTrackedResourceTypes from "./functions/path-for-tracked-resource-types" import pathSegmentCasing from "./functions/path-segment-casing" import { PostResponseCodes } from "./functions/post-response-codes" @@ -302,16 +302,18 @@ const ruleset: any = { }, // github issue https://github.com/Azure/azure-openapi-validator/issues/331 // Get operation should return 200 - // already have rule to check if operation returns non 2XX, it should mark it as 'x-ms-error-response' explicitly, - // so here on check if the 200 return '201','202','203' + // already have rule to check if operation returns non 2XX, it should mark it as 'x-ms-error-response' explicitly + // https://github.com/Azure/azure-openapi-validator/issues/549 + // GET can return 202 only if it is a polling action & has Location header defined. LroLocationHeader rule already checks if 202 response has Location header + // so here just check for non 200, 202 response codes i.e, '201','203','204' // RPC Code: RPC-Get-V1-01 - GetOperation200: { - description: "The get operation should only return 200.", + GetResponseCodes: { + description: "The GET operation should only return 200. In addition, it can return 202 only if it has \"Location\" header defined", message: "{{description}}", severity: "error", resolved: true, formats: [oas2], - given: ["$[paths,'x-ms-paths'].*[get].responses['201','202','203','204']"], + given: ["$[paths,'x-ms-paths'].*[get].responses['201','203','204']"], then: { function: falsy, }, @@ -447,7 +449,7 @@ const ruleset: any = { formats: [oas2], given: ["$[paths,'x-ms-paths'].*[patch]"], then: { - function: PatchResponseCodes, + function: patchResponseCodes, }, }, diff --git a/packages/rulesets/src/spectral/az-common.ts b/packages/rulesets/src/spectral/az-common.ts index 6205866d3..ead89ee09 100644 --- a/packages/rulesets/src/spectral/az-common.ts +++ b/packages/rulesets/src/spectral/az-common.ts @@ -98,6 +98,7 @@ const ruleset: any = { message: "Operations with a 202 response must specify `x-ms-long-running-operation: true`.", severity: "error", formats: [oas2], + // doesn't need to be validated for GET as GET will have 202 only if it is a polling action & hence x-ms-long-running-operation wouldn't be defined given: "$.paths[*][put,patch,post,delete].responses[?(@property == '202')]^^", then: { field: "x-ms-long-running-operation", diff --git a/packages/rulesets/src/spectral/functions/patch-response-codes.ts b/packages/rulesets/src/spectral/functions/patch-response-codes.ts index 848c3ede3..a53167b9b 100644 --- a/packages/rulesets/src/spectral/functions/patch-response-codes.ts +++ b/packages/rulesets/src/spectral/functions/patch-response-codes.ts @@ -10,7 +10,7 @@ const LR_ERROR = const EmptyResponse_ERROR = "PATCH operation response codes must be non-empty. It must have response codes 200 and default if it is sync or 200, 202 and default if it is long running." -export const PatchResponseCodes = (patchOp: any, _opts: any, ctx: any) => { +export const patchResponseCodes = (patchOp: any, _opts: any, ctx: any) => { if (patchOp === null || typeof patchOp !== "object") { return [] } From 52500370f2bf65652b6031e6ae3bc9cac060fa01 Mon Sep 17 00:00:00 2001 From: akhilailla Date: Thu, 9 Nov 2023 11:29:07 -0600 Subject: [PATCH 3/3] updates based on commits --- ...09_and_RPC-Get-V1-01_2023-11-02-15-20.json | 2 +- docs/get-response-codes.md | 127 +++++++++++++-- docs/lro-extension.md | 63 ++++++-- docs/rules.md | 3 +- .../rulesets/generated/spectral/az-arm.js | 50 +++++- .../rulesets/generated/spectral/az-common.js | 4 +- .../generated/spectral/az-dataplane.js | 4 +- packages/rulesets/src/spectral/az-arm.ts | 11 +- packages/rulesets/src/spectral/az-common.ts | 6 +- .../spectral/functions/get-response-codes.ts | 44 ++++++ .../spectral/test/get-response-codes.test.ts | 149 ++++++++++++++++++ 11 files changed, 418 insertions(+), 45 deletions(-) create mode 100644 packages/rulesets/src/spectral/functions/get-response-codes.ts create mode 100644 packages/rulesets/src/spectral/test/get-response-codes.test.ts diff --git a/common/changes/@microsoft.azure/openapi-validator-rulesets/akhilailla-fix_RPC-Post-V1-09_and_RPC-Get-V1-01_2023-11-02-15-20.json b/common/changes/@microsoft.azure/openapi-validator-rulesets/akhilailla-fix_RPC-Post-V1-09_and_RPC-Get-V1-01_2023-11-02-15-20.json index d058276c9..3fb0268ce 100644 --- a/common/changes/@microsoft.azure/openapi-validator-rulesets/akhilailla-fix_RPC-Post-V1-09_and_RPC-Get-V1-01_2023-11-02-15-20.json +++ b/common/changes/@microsoft.azure/openapi-validator-rulesets/akhilailla-fix_RPC-Post-V1-09_and_RPC-Get-V1-01_2023-11-02-15-20.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "@microsoft.azure/openapi-validator-rulesets", - "comment": "Update RPC-GET-V1-01(GetOperation200) to allow GET with 202 response code and RPC-Post-V1-09(LroExtension) to exclude GET from x-ms-long-running-operation presence validation", + "comment": "Update RPC-GET-V1-01(GetResponseCodes) to allow GET with 202 response code if the GET represents the location header polling url. Update RPC-Post-V1-09(LroExtension) to exclude GET from x-ms-long-running-operation presence validation", "type": "patch" } ], diff --git a/docs/get-response-codes.md b/docs/get-response-codes.md index dec8cdb4e..af8d1d990 100644 --- a/docs/get-response-codes.md +++ b/docs/get-response-codes.md @@ -12,25 +12,126 @@ ARM OpenAPI(swagger) specs - RPC-Get-V1-01 -## Output Message - -The get operation should only return 200. -In addition, it can return 202 only if it has "Location" header defined (i.e, if it is a polling action). - ## Description The get operation should only return 200, also it should not be a long running operation. In addition, it can return 202 only if it has location header defined (i.e, if it is a polling action). -## CreatedAt - -July 07, 2022 - -## LastModifiedAt - -July 07, 2022 - ## How to fix the violation Remove all the other response codes except 200 and 202 with "Location" header defined i.e, remove response codes 201, 202(if no "Location" header defined), 203, 204. + +## Good Examples + +```json +... +"/providers/Microsoft.Music/songs/{songName}" { + "get" { + "responses": { + "200": { + "description": "Successful completion of the asynchronous operation", + "schema": { + "$ref": "#/definitions/SongName" + } + }, + "default": { + "description": "Error response describing why the operation failed.", + "schema": { + "$ref": "#/definitions/ErrorResponse" + } + } + } + } +} +... +``` + +```json +... +"/providers/Microsoft.Music/songOperations/{operationId}" { + "get" { + "responses": { + "200": { + "description": "Successful completion of the asynchronous operation", + "schema": { + "$ref": "#/definitions/SongName" + } + }, + "202": { + "description": "Accepted", + "headers": { + "Location": { + "description": "The URL where the status of the asynchronous operation can be checked.", + "type": "string" + }, + }, + }, + "default": { + "description": "Error response describing why the operation failed.", + "schema": { + "$ref": "#/definitions/ErrorResponse" + } + } + } + } +} +... +``` + +## Bad Examples + +```json +... +"/providers/Microsoft.Music/songOperations/{operationId}" { + "get" { + "responses": { + "200": { + "description": "Successful completion of the asynchronous operation", + "schema": { + "$ref": "#/definitions/SongName" + } + }, + "201": { + "description": "Created", + }, + "203": { + "description": "Non authoritative", + }, + "default": { + "description": "Error response describing why the operation failed.", + "schema": { + "$ref": "#/definitions/ErrorResponse" + } + } + } + } +} +... +``` + +```json +... +"/providers/Microsoft.Music/songOperations/{operationId}" { + "get" { + "responses": { + "202": { + "description": "Accepted", + "headers": { + "Location": { + "description": "The URL where the status of the asynchronous operation can be checked.", + "type": "string" + }, + }, + }, + "default": { + "description": "Error response describing why the operation failed.", + "schema": { + "$ref": "#/definitions/ErrorResponse" + } + } + } + } +} +... +``` \ No newline at end of file diff --git a/docs/lro-extension.md b/docs/lro-extension.md index 63d4b5081..f8fdc6b73 100644 --- a/docs/lro-extension.md +++ b/docs/lro-extension.md @@ -9,24 +9,65 @@ ARM & Data Plane Error ARM & Data Plane OpenAPI specs ## Related ARM Guideline Code -- RPC-Post-V1-09 - -## Output Message -Operations with a 202 response should specify `x-ms-long-running-operation: true`. +- RPC-Post-V1-09 ## Description -Operations with a 202 response should specify `x-ms-long-running-operation: true`. +Operations with a 202 response should specify `x-ms-long-running-operation: true`. +GET operation is excluded from the validation as GET will have 202 only if it is a polling action & hence x-ms-long-running-operation wouldn't be defined -## CreatedAt +## How to fix the violation -June 18, 2022 +Add the `x-ms-long-running-operation: true` extension to PUT, PATCH, POST, DELETE operationS. -## LastModifiedAt +## Good Examples -June 18, 2022 +```json +... +"/providers/Microsoft.Music/songs/{songName}" { + "put" { + "responses": { + "202": { + "description": "Accepted", + "schema": { + "$ref": "#/definitions/SongName" + } + }, + "default": { + "description": "Error response describing why the operation failed.", + "schema": { + "$ref": "#/definitions/ErrorResponse" + } + } + }, + "x-ms-long-running-operation": true, + } +} +... +``` -## How to fix the violation +## Bad Examples -Add the `x-ms-long-running-operation: true` extension to the operation. +```json +... +"/providers/Microsoft.Music/songs/{songName}" { + "put" { + "responses": { + "202": { + "description": "Accepted", + "schema": { + "$ref": "#/definitions/SongName" + } + }, + "default": { + "description": "Error response describing why the operation failed.", + "schema": { + "$ref": "#/definitions/ErrorResponse" + } + } + }, + } +} +... +``` \ No newline at end of file diff --git a/docs/rules.md b/docs/rules.md index 55642a470..0d4975c1c 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -495,7 +495,8 @@ Please refer to [lro-error-content.md](./lro-error-content.md) for details. ### LroExtension -Operations with a 202 response should specify `x-ms-long-running-operation: true`. +Operations with a 202 response should specify `x-ms-long-running-operation: true`. +GET operation is excluded from the validation as GET will have 202 only if it is a polling action & hence x-ms-long-running-operation wouldn't be defined Please refer to [lro-extension.md](./lro-extension.md) for details. diff --git a/packages/rulesets/generated/spectral/az-arm.js b/packages/rulesets/generated/spectral/az-arm.js index ff3f1b01b..a835130d2 100644 --- a/packages/rulesets/generated/spectral/az-arm.js +++ b/packages/rulesets/generated/spectral/az-arm.js @@ -888,8 +888,8 @@ const ruleset$1 = { }, }, LroExtension: { - description: "Operations with a 202 response must specify `x-ms-long-running-operation: true`.", - message: "Operations with a 202 response must specify `x-ms-long-running-operation: true`.", + description: "Operations with a 202 response must specify `x-ms-long-running-operation: true`. GET operation is excluded from the validation as GET will have 202 only if it is a polling action & hence x-ms-long-running-operation wouldn't be defined", + message: "Operations with a 202 response must specify `x-ms-long-running-operation: true`. GET operation is excluded from the validation as GET will have 202 only if it is a polling action & hence x-ms-long-running-operation wouldn't be defined", severity: "error", formats: [oas2], given: "$.paths[*][put,patch,post,delete].responses[?(@property == '202')]^^", @@ -1476,7 +1476,7 @@ const SYNC_DELETE_RESPONSES = ["200", "204", "default"]; const LR_DELETE_RESPONSES = ["202", "204", "default"]; const SYNC_ERROR$2 = "Synchronous delete operations must have responses with 200, 204 and default return codes. They also must have no other response codes."; const LR_ERROR$2 = "Long-running delete operations must have responses with 202, 204 and default return codes. They also must have no other response codes."; -const EmptyResponse_ERROR$3 = "Delete operation response codes must be non-empty. It must have response codes 200, 204 and default if it is sync or 202, 204 and default if it is long running."; +const EmptyResponse_ERROR$4 = "Delete operation response codes must be non-empty. It must have response codes 200, 204 and default if it is sync or 202, 204 and default if it is long running."; const DeleteResponseCodes = (deleteOp, _opts, ctx) => { var _a; if (deleteOp === null || typeof deleteOp !== "object") { @@ -1487,7 +1487,7 @@ const DeleteResponseCodes = (deleteOp, _opts, ctx) => { const responses = Object.keys((_a = deleteOp === null || deleteOp === void 0 ? void 0 : deleteOp.responses) !== null && _a !== void 0 ? _a : {}); if (responses.length == 0) { errors.push({ - message: EmptyResponse_ERROR$3, + message: EmptyResponse_ERROR$4, path: path, }); return errors; @@ -1549,6 +1549,40 @@ const getCollectionOnlyHasValueAndNextLink = (properties, _opts, ctx) => { return []; }; +const GET_RESPONSES = ["200", "202", "default"]; +const GET_RESPONSE_ERROR = "GET operation must have response codes 200 and default. In addition, can have 202 if the GET represents the location header polling url."; +const EmptyResponse_ERROR$3 = "GET operation response codes must be non-empty. It must have response codes 200 and default. In addition, can have 202 if the GET represents the location header polling url."; +const getResponseCodes = (getOp, _opts, ctx) => { + var _a; + if (getOp === null || typeof getOp !== "object") { + return []; + } + const path = ctx.path; + const errors = []; + const responses = Object.keys((_a = getOp === null || getOp === void 0 ? void 0 : getOp.responses) !== null && _a !== void 0 ? _a : {}); + if (responses.length == 0) { + errors.push({ + message: EmptyResponse_ERROR$3, + path: path, + }); + return errors; + } + if (!responses.includes("200")) { + errors.push({ + message: GET_RESPONSE_ERROR, + path: path, + }); + return errors; + } + if (!responses.every((response) => GET_RESPONSES.includes(response))) { + errors.push({ + message: GET_RESPONSE_ERROR, + path: path, + }); + } + return errors; +}; + function checkApiVersion(param) { if (param.in !== "query") { return false; @@ -3034,14 +3068,14 @@ const ruleset = { }, }, GetResponseCodes: { - description: "The GET operation should only return 200. In addition, it can return 202 only if it has \"Location\" header defined", - message: "{{description}}", + description: 'The GET operation should only return 200. In addition, it can return 202 only if it has "Location" header defined', + message: "{{error}}", severity: "error", resolved: true, formats: [oas2], - given: ["$[paths,'x-ms-paths'].*[get].responses['201','203','204']"], + given: ["$[paths,'x-ms-paths'].*[get]"], then: { - function: falsy, + function: getResponseCodes, }, }, GetCollectionOnlyHasValueAndNextLink: { diff --git a/packages/rulesets/generated/spectral/az-common.js b/packages/rulesets/generated/spectral/az-common.js index 8d66338f7..6c79ab70c 100644 --- a/packages/rulesets/generated/spectral/az-common.js +++ b/packages/rulesets/generated/spectral/az-common.js @@ -658,8 +658,8 @@ const ruleset = { }, }, LroExtension: { - description: "Operations with a 202 response must specify `x-ms-long-running-operation: true`.", - message: "Operations with a 202 response must specify `x-ms-long-running-operation: true`.", + description: "Operations with a 202 response must specify `x-ms-long-running-operation: true`. GET operation is excluded from the validation as GET will have 202 only if it is a polling action & hence x-ms-long-running-operation wouldn't be defined", + message: "Operations with a 202 response must specify `x-ms-long-running-operation: true`. GET operation is excluded from the validation as GET will have 202 only if it is a polling action & hence x-ms-long-running-operation wouldn't be defined", severity: "error", formats: [oas2], given: "$.paths[*][put,patch,post,delete].responses[?(@property == '202')]^^", diff --git a/packages/rulesets/generated/spectral/az-dataplane.js b/packages/rulesets/generated/spectral/az-dataplane.js index d0db8bfe0..27463958f 100644 --- a/packages/rulesets/generated/spectral/az-dataplane.js +++ b/packages/rulesets/generated/spectral/az-dataplane.js @@ -688,8 +688,8 @@ const ruleset$1 = { }, }, LroExtension: { - description: "Operations with a 202 response must specify `x-ms-long-running-operation: true`.", - message: "Operations with a 202 response must specify `x-ms-long-running-operation: true`.", + description: "Operations with a 202 response must specify `x-ms-long-running-operation: true`. GET operation is excluded from the validation as GET will have 202 only if it is a polling action & hence x-ms-long-running-operation wouldn't be defined", + message: "Operations with a 202 response must specify `x-ms-long-running-operation: true`. GET operation is excluded from the validation as GET will have 202 only if it is a polling action & hence x-ms-long-running-operation wouldn't be defined", severity: "error", formats: [oas2], given: "$.paths[*][put,patch,post,delete].responses[?(@property == '202')]^^", diff --git a/packages/rulesets/src/spectral/az-arm.ts b/packages/rulesets/src/spectral/az-arm.ts index a8ac8fd10..795f8e067 100644 --- a/packages/rulesets/src/spectral/az-arm.ts +++ b/packages/rulesets/src/spectral/az-arm.ts @@ -8,6 +8,7 @@ import collectionObjectPropertiesNaming from "./functions/collection-object-prop import { consistentPatchProperties } from "./functions/consistent-patch-properties" import { DeleteResponseCodes } from "./functions/delete-response-codes" import { getCollectionOnlyHasValueAndNextLink } from "./functions/get-collection-only-has-value-and-next-link" +import { getResponseCodes } from "./functions/get-response-codes" import hasApiVersionParameter from "./functions/has-api-version-parameter" import hasheader from "./functions/has-header" import httpsSupportedScheme from "./functions/https-supported-scheme" @@ -303,19 +304,19 @@ const ruleset: any = { // github issue https://github.com/Azure/azure-openapi-validator/issues/331 // Get operation should return 200 // already have rule to check if operation returns non 2XX, it should mark it as 'x-ms-error-response' explicitly - // https://github.com/Azure/azure-openapi-validator/issues/549 + // https://github.com/Azure/azure-openapi-validator/issues/549 // GET can return 202 only if it is a polling action & has Location header defined. LroLocationHeader rule already checks if 202 response has Location header // so here just check for non 200, 202 response codes i.e, '201','203','204' // RPC Code: RPC-Get-V1-01 GetResponseCodes: { - description: "The GET operation should only return 200. In addition, it can return 202 only if it has \"Location\" header defined", - message: "{{description}}", + description: 'The GET operation should only return 200. In addition, it can return 202 only if it has "Location" header defined', + message: "{{error}}", severity: "error", resolved: true, formats: [oas2], - given: ["$[paths,'x-ms-paths'].*[get].responses['201','203','204']"], + given: ["$[paths,'x-ms-paths'].*[get]"], then: { - function: falsy, + function: getResponseCodes, }, }, diff --git a/packages/rulesets/src/spectral/az-common.ts b/packages/rulesets/src/spectral/az-common.ts index ead89ee09..f68bce539 100644 --- a/packages/rulesets/src/spectral/az-common.ts +++ b/packages/rulesets/src/spectral/az-common.ts @@ -94,8 +94,10 @@ const ruleset: any = { }, // RPC Code: RPC-Post-V1-09 LroExtension: { - description: "Operations with a 202 response must specify `x-ms-long-running-operation: true`.", - message: "Operations with a 202 response must specify `x-ms-long-running-operation: true`.", + description: + "Operations with a 202 response must specify `x-ms-long-running-operation: true`. GET operation is excluded from the validation as GET will have 202 only if it is a polling action & hence x-ms-long-running-operation wouldn't be defined", + message: + "Operations with a 202 response must specify `x-ms-long-running-operation: true`. GET operation is excluded from the validation as GET will have 202 only if it is a polling action & hence x-ms-long-running-operation wouldn't be defined", severity: "error", formats: [oas2], // doesn't need to be validated for GET as GET will have 202 only if it is a polling action & hence x-ms-long-running-operation wouldn't be defined diff --git a/packages/rulesets/src/spectral/functions/get-response-codes.ts b/packages/rulesets/src/spectral/functions/get-response-codes.ts new file mode 100644 index 000000000..1ce405a91 --- /dev/null +++ b/packages/rulesets/src/spectral/functions/get-response-codes.ts @@ -0,0 +1,44 @@ +// Verifies that GET has 200 response code defined. Also, checks if there is any additional response code defined it is 202 and nothing else + +const GET_RESPONSES = ["200", "202", "default"] +const GET_RESPONSE_ERROR = + "GET operation must have response codes 200 and default. In addition, can have 202 if the GET represents the location header polling url." +const EmptyResponse_ERROR = + "GET operation response codes must be non-empty. It must have response codes 200 and default. In addition, can have 202 if the GET represents the location header polling url." + +export const getResponseCodes = (getOp: any, _opts: any, ctx: any) => { + if (getOp === null || typeof getOp !== "object") { + return [] + } + const path = ctx.path + const errors = [] + + const responses = Object.keys(getOp?.responses ?? {}) + + if (responses.length == 0) { + errors.push({ + message: EmptyResponse_ERROR, + path: path, + }) + return errors + } + + // flag error if 200 is not defined + if (!responses.includes("200")) { + errors.push({ + message: GET_RESPONSE_ERROR, + path: path, + }) + return errors + } + + // check if the responses defined are the accepted ones only + if (!responses.every((response) => GET_RESPONSES.includes(response))) { + errors.push({ + message: GET_RESPONSE_ERROR, + path: path, + }) + } + + return errors +} diff --git a/packages/rulesets/src/spectral/test/get-response-codes.test.ts b/packages/rulesets/src/spectral/test/get-response-codes.test.ts new file mode 100644 index 000000000..52abe505e --- /dev/null +++ b/packages/rulesets/src/spectral/test/get-response-codes.test.ts @@ -0,0 +1,149 @@ +import { Spectral } from "@stoplight/spectral-core" +import linterForRule from "./utils" + +let linter: Spectral + +beforeAll(async () => { + linter = await linterForRule("GetResponseCodes") + return linter +}) + +const GET_RESPONSE_ERROR = + "GET operation must have response codes 200 and default. In addition, can have 202 if the GET represents the location header polling url." +const EmptyResponse_ERROR = + "GET operation response codes must be non-empty. It must have response codes 200 and default. In addition, can have 202 if the GET represents the location header polling url." + + +test("GetResponseCodes should find no errors", () => { + const myOpenApiDocument = { + swagger: "2.0", + paths: { + "/foo": { + get: { + operationId: "foo_post", + responses: { + 200: { + description: "Success", + }, + default: { + description: "Error", + }, + }, + }, + }, + "/foo1/operations": { + get: { + operationId: "foo_post", + responses: { + 200: { + description: "Success", + }, + 202: { + description: "Accepted", + "headers": { + "Location": { + "description": "The URL where the status of the asynchronous operation can be checked.", + "type": "string" + }, + }, + }, + default: { + description: "Error", + }, + }, + }, + }, + } + } + return linter.run(myOpenApiDocument).then((results) => { + expect(results.length).toBe(0) + }) +}) + +test("GetResponseCodes should find errors if responses are not defined", () => { + const myOpenApiDocument = { + swagger: "2.0", + paths: { + "/foo": { + get: { + operationId: "foo_post", + responses:{ + + }, + }, + }, + } + } + return linter.run(myOpenApiDocument).then((results) => { + expect(results.length).toBe(1) + expect(results[0].path.join(".")).toBe("paths./foo.get") + expect(results[0].message).toContain(EmptyResponse_ERROR) + }) +}) + +test("GetResponseCodes should find errors if 200 is not defined", () => { + const myOpenApiDocument = { + swagger: "2.0", + paths: { + "/foo/operations": { + get: { + operationId: "foo_post", + responses: { + 202: { + description: "Accepted", + headers: { + Location: { + description: "The URL where the status of the asynchronous operation can be checked.", + type: "string", + }, + }, + }, + default: { + description: "Error", + }, + }, + }, + }, + } + } + return linter.run(myOpenApiDocument).then((results) => { + expect(results.length).toBe(1) + expect(results[0].path.join(".")).toBe("paths./foo/operations.get") + expect(results[0].message).toContain(GET_RESPONSE_ERROR) + }) +}) + +test("GetResponseCodes should find errors if non-accepted response codes are defined", () => { + const myOpenApiDocument = { + swagger: "2.0", + paths: { + "/foo/operations": { + get: { + operationId: "foo_post", + responses: { + 200: { + description: "Success", + }, + 204: { + description: "Accepted", + headers: { + Location: { + description: "The URL where the status of the asynchronous operation can be checked.", + type: "string", + }, + }, + }, + default: { + description: "Error", + }, + }, + }, + }, + }, + } + return linter.run(myOpenApiDocument).then((results) => { + expect(results.length).toBe(1) + expect(results[0].path.join(".")).toBe("paths./foo/operations.get") + expect(results[0].message).toContain(GET_RESPONSE_ERROR) + }) +})