Add new rule ValidQueryParametersForPointOperations (#746)

Co-authored-by: akhilailla <[email protected]>
Azure · Sep 18, 2024 · c26c69a · c26c69a
1 parent a954384
commit c26c69a
Show file tree

Hide file tree

Showing 9 changed files with 615 additions and 3 deletions.
diff --git a/docs/rules.md b/docs/rules.md
@@ -1250,7 +1250,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.
 
@@ -1350,6 +1350,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.

diff --git a/docs/tracked-extension-resources-are-not-allowed.md b/docs/tracked-extension-resources-are-not-allowed.md
@@ -1,4 +1,4 @@
-# trackedExtensionResourcesAreNotAllowed
+# TrackedExtensionResourcesAreNotAllowed
 
 ## Category
 

diff --git a/docs/valid-query-parameters-for-point-operations.md b/docs/valid-query-parameters-for-point-operations.md
@@ -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"
+          },
+        ],
+      },
+    },
+```
diff --git a/packages/rulesets/generated/spectral/az-arm.js b/packages/rulesets/generated/spectral/az-arm.js
@@ -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) {
@@ -3013,6 +3025,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: {
@@ -3917,6 +3960,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.",

diff --git a/packages/rulesets/src/spectral/az-arm.ts b/packages/rulesets/src/spectral/az-arm.ts
@@ -52,6 +52,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"
@@ -958,6 +959,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

diff --git a/packages/rulesets/src/spectral/functions/utils.ts b/packages/rulesets/src/spectral/functions/utils.ts
@@ -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) {

diff --git a/packages/rulesets/src/spectral/functions/valid-query-parameters-for-point-operations.ts b/packages/rulesets/src/spectral/functions/valid-query-parameters-for-point-operations.ts
@@ -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
+}
diff --git a/packages/rulesets/src/spectral/test/latest-version-of-common-types-must-be-used.test.ts b/packages/rulesets/src/spectral/test/latest-version-of-common-types-must-be-used.test.ts
@@ -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: {