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

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

Merged
merged 13 commits into from
Aug 20, 2024
Merged
6 changes: 6 additions & 0 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -1214,6 +1214,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
tejaswiMinnu marked this conversation as resolved.
Show resolved Hide resolved

`tags` should not be specified in the properties bag for proxy resources. Consider using a Tracked resource instead.
tejaswiMinnu marked this conversation as resolved.
Show resolved Hide resolved

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.
Expand Down
73 changes: 73 additions & 0 deletions docs/tags-are-not-allowed-for-proxy-resources.md
Original file line number Diff line number Diff line change
@@ -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`.

tejaswiMinnu marked this conversation as resolved.
Show resolved Hide resolved
### Valid/Good Example

```json
"definitions": {
"Resource": {
"type": "object",
"properties": {
"type": "object",
tejaswiMinnu marked this conversation as resolved.
Show resolved Hide resolved
// 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",
},
}
},
},
}
}
}
```
52 changes: 44 additions & 8 deletions packages/rulesets/generated/spectral/az-arm.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 [];
Expand Down Expand Up @@ -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.",
Expand Down
16 changes: 16 additions & 0 deletions packages/rulesets/src/spectral/az-arm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
tejaswiMinnu marked this conversation as resolved.
Show resolved Hide resolved
stagingOnly: true,
message: "{{error}}",
resolved: true,
formats: [oas2],
given: ["$.definitions.*.properties^"],
then: {
function: tagsAreNotAllowedForProxyResources,
},
},

///
/// ARM RPC rules for Post patterns
///
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import _ from "lodash"
import { deepFindObjectKeyPath, getProperties } from "./utils"

const TAGS = "tags"
const PROPERTIES = "properties"
const NestedPROPERTIES = "properties"
tejaswiMinnu marked this conversation as resolved.
Show resolved Hide resolved
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
}
Loading