Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new rule ValidQueryParametersForPointOperations #746

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -1244,7 +1244,7 @@ Per [ARM guidelines](https://github.com/Azure/azure-resource-manager-rpc/blob/ma

Please refer to [top-level-resources-list-by-subscription.md](./top-level-resources-list-by-subscription.md) for details.

### trackedExtensionResourcesAreNotAllowed
### TrackedExtensionResourcesAreNotAllowed

Extension resources are always considered to be proxy and must not be of the type tracked.

Expand Down Expand Up @@ -1344,6 +1344,12 @@ Only valid types are allowed for properties.

Please refer to [valid-formats.md](./valid-formats.md) for details.

### ValidQueryParametersForPointOperations

Point operations (GET, PUT, PATCH, DELETE) must not include any query parameters other than api-version.

Please refer to [valid-query-parameters-for-point-operations.md](./valid-query-parameters-for-point-operations.md) for details.

### ValidResponseCodeRequired

Every operation response must contain a valid code like "200","201","202" or "204" which indicates the operation is succeed and it's not allowed that a response schema just contains a "default" code.
Expand Down
2 changes: 1 addition & 1 deletion docs/tracked-extension-resources-are-not-allowed.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# trackedExtensionResourcesAreNotAllowed
# TrackedExtensionResourcesAreNotAllowed

## Category

Expand Down
107 changes: 107 additions & 0 deletions docs/valid-query-parameters-for-point-operations.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
# ValidQueryParametersForPointOperations

## Category

ARM Error

## Applies to

ARM OpenAPI(swagger) specs

## Related ARM Guideline Code

- RPC-Uri-V1-13

## Description

Point operations (GET, PUT, PATCH, DELETE) must not include any query parameters other than api-version.

## How to fix the violation

Remove all query params other than api-version for point operations (GET, PUT, PATCH, DELETE).

## Good Examples

```json
"Microsoft.Music/Songs": {
"get": {
"operationId": "Foo_Get",
"description": "Test Description",
"parameters": [
{
"name": "api-version",
"in": "query"
},
],
},
"put": {
"operationId": "Foo_Update",
"description": "Test Description",
"parameters": [
{
"name": "api-version",
"in": "query"
},
],
},
"patch": {
"operationId": "Foo_Update",
"description": "Test Description",
"parameters": [
{
"name": "api-version",
"in": "query"
},
],
},
"delete": {
"operationId": "Foo_Update",
"description": "Test Description",
"parameters": [
{
"name": "api-version",
"in": "query"
},
],
},
},
```

## Bad Examples

```json
"Microsoft.Music/Songs": {
"get": {
"operationId": "Foo_get",
"description": "Test Description",
"parameters": [
{
"name": "name",
"in": "query",
"required": false,
"type": "string",
},
],
},
"put": {
"operationId": "Foo_Create",
"description": "Test Description",
"parameters": [
{
"name": "$filter",
"in": "query"
},
],
},
"patch": {
"operationId": "Foo_Update",
"description": "Test Description",
"parameters": [
{
"name": "name",
"in": "query"
},
],
},
},
```
56 changes: 56 additions & 0 deletions packages/rulesets/generated/spectral/az-arm.js
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,18 @@ const providerAndNamespace = "/providers/[^/]+";
const resourceTypeAndResourceName = "(?:/\\w+/default|/\\w+/{[^/]+})";
const queryParam = "(?:\\?\\w+)";
const resourcePathRegEx = new RegExp(`${providerAndNamespace}${resourceTypeAndResourceName}+${queryParam}?$`, "gi");
function isPointOperation(path) {
const index = path.lastIndexOf("/providers/");
if (index === -1) {
return false;
}
const lastProvider = path.substr(index);
const matches = lastProvider.match(resourcePathRegEx);
if (matches) {
return true;
}
return false;
}
function getResourcesPathHierarchyBasedOnResourceType(path) {
const index = path.lastIndexOf("/providers/");
if (index === -1) {
Expand Down Expand Up @@ -2846,6 +2858,37 @@ const trackedResourceTagsPropertyInRequest = (pathItem, _opts, paths) => {
return errors;
};

const validQueryParametersForPointOperations = (pathItem, _opts, ctx) => {
if (pathItem === null || typeof pathItem !== "object") {
return [];
}
const path = ctx.path || [];
const uris = Object.keys(pathItem);
if (uris.length < 1) {
return [];
}
const pointOperations = new Set(["get", "put", "patch", "delete"]);
const errors = [];
for (const uri of uris) {
if (isPointOperation(uri)) {
const verbs = Object.keys(pathItem[uri]);
for (const verb of verbs) {
if (pointOperations.has(verb)) {
const params = pathItem[uri][verb]["parameters"];
const queryParams = params === null || params === void 0 ? void 0 : params.filter((param) => param.in === "query" && param.name !== "api-version");
queryParams === null || queryParams === void 0 ? void 0 : queryParams.map((param) => {
errors.push({
message: `Query parameter ${param.name} should be removed. Point operation '${verb}' MUST not have query parameters other than api-version.`,
path: [path, uri, verb, "parameters"],
});
});
}
}
}
}
return errors;
};

const validatePatchBodyParamProperties = createRulesetFunction({
input: null,
options: {
Expand Down Expand Up @@ -3878,6 +3921,19 @@ const ruleset = {
function: trackedExtensionResourcesAreNotAllowed,
},
},
ValidQueryParametersForPointOperations: {
rpcGuidelineCode: "RPC-Uri-V1-13",
description: "Point operations (GET, PUT, PATCH, DELETE) must not include any query parameters other than api-version.",
message: "{{error}}",
stagingOnly: true,
severity: "error",
resolved: true,
formats: [oas2],
given: "$[paths,'x-ms-paths']",
then: {
function: validQueryParametersForPointOperations,
},
},
SystemDataDefinitionsCommonTypes: {
rpcGuidelineCode: "RPC-SystemData-V1-01, RPC-SystemData-V1-02",
description: "System data references must utilize common types.",
Expand Down
15 changes: 15 additions & 0 deletions packages/rulesets/src/spectral/az-arm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import { tagsAreNotAllowedForProxyResources } from "./functions/tags-are-not-all
import { tenantLevelAPIsNotAllowed } from "./functions/tenant-level-apis-not-allowed"
import { trackedExtensionResourcesAreNotAllowed } from "./functions/tracked-extension-resources-are-not-allowed"
import trackedResourceTagsPropertyInRequest from "./functions/trackedresource-tags-property-in-request"
import { validQueryParametersForPointOperations } from "./functions/valid-query-parameters-for-point-operations"
import { validatePatchBodyParamProperties } from "./functions/validate-patch-body-param-properties"
import withXmsResource from "./functions/with-xms-resource"
import verifyXMSLongRunningOperationProperty from "./functions/xms-long-running-operation-property"
Expand Down Expand Up @@ -943,6 +944,20 @@ const ruleset: any = {
function: trackedExtensionResourcesAreNotAllowed,
},
},
// RPC Code: RPC-Uri-V1-13
ValidQueryParametersForPointOperations: {
rpcGuidelineCode: "RPC-Uri-V1-13",
description: "Point operations (GET, PUT, PATCH, DELETE) must not include any query parameters other than api-version.",
message: "{{error}}",
stagingOnly: true,
severity: "error",
resolved: true,
formats: [oas2],
given: "$[paths,'x-ms-paths']",
then: {
function: validQueryParametersForPointOperations,
},
},

///
/// ARM RPC rules for SystemData
Expand Down
19 changes: 19 additions & 0 deletions packages/rulesets/src/spectral/functions/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,25 @@ const providerAndNamespace = "/providers/[^/]+"
const resourceTypeAndResourceName = "(?:/\\w+/default|/\\w+/{[^/]+})"
const queryParam = "(?:\\?\\w+)"
const resourcePathRegEx = new RegExp(`${providerAndNamespace}${resourceTypeAndResourceName}+${queryParam}?$`, "gi")
/**
* Checks if the provided path is a point operation
* i.e, if its a path that can have point GET, PUT, PATCH, DELETE
* @param path path/uri
* @returns true or false
*/
export function isPointOperation(path: string) {
const index = path.lastIndexOf("/providers/")
if (index === -1) {
return false
}
const lastProvider = path.substr(index)
const matches = lastProvider.match(resourcePathRegEx)
if(matches){
return true
}
return false
}

export function getResourcesPathHierarchyBasedOnResourceType(path: string) {
const index = path.lastIndexOf("/providers/")
if (index === -1) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { isPointOperation } from "./utils"

export const validQueryParametersForPointOperations = (pathItem: any, _opts: any, ctx: any) => {
if (pathItem === null || typeof pathItem !== "object") {
return []
}

const path = ctx.path || []
const uris = Object.keys(pathItem)
if (uris.length < 1) {
return []
}
const pointOperations = new Set(["get", "put", "patch", "delete"])
const errors: any[] = []

for (const uri of uris) {
//check if the path is a point operation
if (isPointOperation(uri)) {
const verbs = Object.keys(pathItem[uri])
for (const verb of verbs) {
//check query params only for point operations/verbs
if (pointOperations.has(verb)) {
const params = pathItem[uri][verb]["parameters"]
const queryParams = params?.filter((param: { in: string; name: string }) => param.in === "query" && param.name !== "api-version")
queryParams?.map((param: { name: any }) => {
errors.push({
message: `Query parameter ${param.name} should be removed. Point operation '${verb}' MUST not have query parameters other than api-version.`,
path: [path, uri, verb, "parameters"],
})
})
}
}
}
}

return errors
}
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ test("LatestVersionOfCommonTypesMustBeUsed should find no errors", async () => {
expect(results.length).toBe(0)
})
})
test("ParametersInPointGet should find no errors when common-types ref is not present", async () => {
test("LatestVersionOfCommonTypesMustBeUsed should find no errors when common-types ref is not present", async () => {
const myOpenApiDocument = {
swagger: "2.0",
paths: {
Expand Down
Loading