Rule to check if Tags are defined as Top-level property (#710)

* Add TagsAreTopLevelPropertiesOnly new rule

* addressed comments

* addressed comments

* addressed the comment by removing the change log file

* addressed comments

Author: tejaswiMinnu

docs/rules.md

@@ -1220,6 +1220,12 @@ Validates that system data is not defined in the properties bag, but rather as a
 
 Please refer to [system-data-in-properties-bag.md](./system-data-in-properties-bag.md) for details.
 
+### TagsAreNotAllowedForProxyResources
+
+`tags` should not be specified in the properties bag for proxy resources. Consider using a Tracked resource instead.
+
+Please refer to [tags-are-not-allowed-for-proxy-resources.md](./tags-are-not-allowed-for-proxy-resources.md) for details.
+
 ### TenantLevelAPIsNotAllowed
 
 Tenant level APIs are strongly discouraged and subscription or resource group level APIs are preferred instead. The reason for this guidance is that tenant level APIs have a really broad scope and blast radius. We permit APIs to be at this broad scope under rare conditions. Some ARM feature sets also do not cover tenant level APIs such as the use of AFEC. Additionally, if you intend to bypass the standard RBAC constructs and make the APIs unauthorized, you will need an approval from the PAS team before the open API spec can be merged.

docs/tags-are-not-allowed-for-proxy-resources.md

@@ -0,0 +1,73 @@
+# TagsAreNotAllowedForProxyResources
+
+## Category
+
+ARM Error
+
+## Applies to
+
+ARM OpenAPI(swagger) specs
+
+## Related ARM Guideline Code
+
+- RPC-Put-V1-31
+
+## Description
+
+`tags` should not be specified in the properties bag for proxy resources. Consider using a Tracked resource instead.
+
+## How to fix the violation
+
+Either remove the `tags` from the properties bag or consider using a Tracked resource, which supports top-level `tags`.
+
+### Valid/Good Example
+
+```json
+"definitions": {
+  "Resource": {
+    "type": "object",
+    "properties": {
+      "type": "object",
+        // top-level properties
+      "tags": {
+        "type": "object",
+        "additionalProperties": {
+          "type": "object",
+          "params": {
+            "type": "boolean",
+          },
+        }
+      },
+      "location": {
+        "type": "string"
+      }
+    }
+  },
+}
+```
+
+### Invalid/Bad Example
+
+```json
+"definitions": {
+  "Resource": {
+    "type": "string",
+    "properties": {
+      "type": "object",
+      "properties": {
+        "type": "object",
+        // Nested Properties
+        "tags": {
+          "type": "object",
+          "additionalProperties": {
+            "type": "object",
+            "params": {
+              "type": "boolean",
+            },
+          }
+        },
+      },
+    }
+  }
+}
+```

packages/rulesets/generated/spectral/az-arm.js

@@ -2081,7 +2081,7 @@ const patchBodyParameters = (parameters, _opts, paths) => {
     return errors;
 };
 
-const ERROR_MESSAGE$1 = "A patch request body must only contain properties present in the corresponding put request body, and must contain at least one of the properties.";
+const ERROR_MESSAGE$2 = "A patch request body must only contain properties present in the corresponding put request body, and must contain at least one of the properties.";
 const PARAM_IN_BODY = (paramObject) => paramObject.in === "body";
 const PATCH = "patch";
 const PUT = "put";
@@ -2116,7 +2116,7 @@ const patchPropertiesCorrespondToPutProperties = (pathItem, _opts, ctx) => {
     const patchBodyPropertiesNotInPutBody = _.differenceWith(patchBodyProperties[0], putBodyProperties[0], _.isEqual);
     if (patchBodyPropertiesNotInPutBody.length > 0) {
         patchBodyPropertiesNotInPutBody.forEach((missingProperty) => errors.push({
-            message: `${Object.keys(missingProperty)[0]} property in patch body is not present in the corresponding put body. ` + ERROR_MESSAGE$1,
+            message: `${Object.keys(missingProperty)[0]} property in patch body is not present in the corresponding put body. ` + ERROR_MESSAGE$2,
             path: path,
         }));
         return errors;
@@ -2704,31 +2704,54 @@ const skuValidation = (skuSchema, opts, paths) => {
 
 const SYSTEM_DATA_CAMEL = "systemData";
 const SYSTEM_DATA_UPPER_CAMEL = "SystemData";
-const PROPERTIES = "properties";
-const ERROR_MESSAGE = "System data must be defined as a top-level property, not in the properties bag.";
+const PROPERTIES$1 = "properties";
+const ERROR_MESSAGE$1 = "System data must be defined as a top-level property, not in the properties bag.";
 const systemDataInPropertiesBag = (definition, _opts, ctx) => {
     const properties = getProperties(definition);
     const path = deepFindObjectKeyPath(properties, SYSTEM_DATA_CAMEL);
     if (path.length > 0) {
         return [
             {
-                message: ERROR_MESSAGE,
-                path: _.concat(ctx.path, PROPERTIES, path[0]),
+                message: ERROR_MESSAGE$1,
+                path: _.concat(ctx.path, PROPERTIES$1, path[0]),
             },
         ];
     }
     const pathForUpperCamelCase = deepFindObjectKeyPath(properties, SYSTEM_DATA_UPPER_CAMEL);
     if (pathForUpperCamelCase.length > 0) {
         return [
             {
-                message: ERROR_MESSAGE,
-                path: _.concat(ctx.path, PROPERTIES, pathForUpperCamelCase[0]),
+                message: ERROR_MESSAGE$1,
+                path: _.concat(ctx.path, PROPERTIES$1, pathForUpperCamelCase[0]),
             },
         ];
     }
     return [];
 };
 
+const TAGS = "tags";
+const PROPERTIES = "properties";
+const NestedPROPERTIES = "properties";
+const ERROR_MESSAGE = "Tags should not be specified in the properties bag for proxy resources. Consider using a Tracked resource instead.";
+const tagsAreNotAllowedForProxyResources = (definition, _opts, ctx) => {
+    const properties = getProperties(definition);
+    const errors = [];
+    if ("tags" in properties && !("location" in properties)) {
+        errors.push({
+            message: ERROR_MESSAGE,
+            path: _.concat(ctx.path, PROPERTIES, TAGS),
+        });
+    }
+    const deepPropertiesTags = deepFindObjectKeyPath(definition.properties.properties, TAGS);
+    if (deepPropertiesTags.length > 0) {
+        errors.push({
+            message: ERROR_MESSAGE,
+            path: _.concat(ctx.path, PROPERTIES, NestedPROPERTIES, deepPropertiesTags[0]),
+        });
+    }
+    return errors;
+};
+
 const tenantLevelAPIsNotAllowed = (pathItems, _opts, ctx) => {
     if (pathItems === null || typeof pathItems !== "object") {
         return [];
@@ -3525,6 +3548,19 @@ const ruleset = {
                 function: consistentResponseSchemaForPut,
             },
         },
+        TagsAreNotAllowedForProxyResources: {
+            rpcGuidelineCode: "RPC-Put-V1-31",
+            description: "Tags should not be specified in the properties bag for proxy resources. Consider using a Tracked resource instead.",
+            severity: "error",
+            stagingOnly: true,
+            message: "{{error}}",
+            resolved: true,
+            formats: [oas2],
+            given: ["$.definitions.*.properties^"],
+            then: {
+                function: tagsAreNotAllowedForProxyResources,
+            },
+        },
         ParametersInPost: {
             rpcGuidelineCode: "RPC-POST-V1-05",
             description: "For a POST action parameters MUST be in the payload and not in the URI.",

packages/rulesets/src/spectral/az-arm.ts

@@ -47,6 +47,7 @@ import responseSchemaSpecifiedForSuccessStatusCode from "./functions/response-sc
 import { securityDefinitionsStructure } from "./functions/security-definitions-structure"
 import skuValidation from "./functions/sku-validation"
 import { systemDataInPropertiesBag } from "./functions/system-data-in-properties-bag"
+import { tagsAreNotAllowedForProxyResources } from "./functions/tags-are-not-allowed-for-proxy-resources"
 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"
@@ -736,6 +737,21 @@ const ruleset: any = {
       },
     },
 
+    // RPC Code: RPC-Put-V1-31
+    TagsAreNotAllowedForProxyResources: {
+      rpcGuidelineCode: "RPC-Put-V1-31",
+      description: "Tags should not be specified in the properties bag for proxy resources. Consider using a Tracked resource instead.",
+      severity: "error",
+      stagingOnly: true,
+      message: "{{error}}",
+      resolved: true,
+      formats: [oas2],
+      given: ["$.definitions.*.properties^"],
+      then: {
+        function: tagsAreNotAllowedForProxyResources,
+      },
+    },
+
     ///
     /// ARM RPC rules for Post patterns
     ///

packages/rulesets/src/spectral/functions/tags-are-not-allowed-for-proxy-resources.ts

@@ -0,0 +1,29 @@
+import _ from "lodash"
+import { deepFindObjectKeyPath, getProperties } from "./utils"
+
+const TAGS = "tags"
+const PROPERTIES = "properties"
+const NestedPROPERTIES = "properties"
+const ERROR_MESSAGE = "Tags should not be specified in the properties bag for proxy resources. Consider using a Tracked resource instead."
+
+export const tagsAreNotAllowedForProxyResources = (definition: any, _opts: any, ctx: any) => {
+  const properties = getProperties(definition)
+  const errors = []
+
+  if ("tags" in properties && !("location" in properties)) {
+    errors.push({
+      message: ERROR_MESSAGE,
+      path: _.concat(ctx.path, PROPERTIES, TAGS),
+    })
+  }
+
+  const deepPropertiesTags = deepFindObjectKeyPath(definition.properties.properties, TAGS)
+  if (deepPropertiesTags.length > 0) {
+    errors.push({
+      message: ERROR_MESSAGE,
+      path: _.concat(ctx.path, PROPERTIES, NestedPROPERTIES, deepPropertiesTags[0]),
+    })
+  }
+
+  return errors
+}