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
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
tejaswiMinnu marked this conversation as resolved.
Show resolved Hide resolved
{
"packageName": "@microsoft.azure/openapi-validator-rulesets",
"comment": "Add TagsAreTopLevelPropertiesOnly rule to check if tags are defined as top-level property, not in the properties bag",
"type": "patch"
}
],
"packageName": "@microsoft.azure/openapi-validator-rulesets"
}
6 changes: 6 additions & 0 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -1213,6 +1213,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.

### TagsAreTopLevelPropertiesOnly

Validates that `tags` is not defined in the properties bag, but rather as a top-level property.
tejaswiMinnu marked this conversation as resolved.
Show resolved Hide resolved

Please refer to [tags-are-top-level-properties-only.md](./tags-are-top-level-properties-only.md) for details.

### TenantLevelAPIsNotAllowed

Tenant level APIs are strongly discouraged and subscription or resource group level APIs are preferred instead.
Expand Down
57 changes: 57 additions & 0 deletions docs/tags-are-top-level-properties-only.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# TagsAreTopLevelPropertiesOnly

## Category

ARM Error

## Applies to

ARM OpenAPI(swagger) specs

## Related ARM Guideline Code

- RPC-Put-V1-30

## Description

Validates that `tags` is not defined in the properties bag, but rather as a top-level property.

## How to fix the violation

Ensure that any `tags` definitions are as top-level properties, not in the properties bag.

### Valid/Good Example

```json
"definitions": {
"tags": {
"type": "object",
"additionalProperties": {
"type": "object",
"params": {
"type": "boolean",
},
}
},
"properties": {
"type": "object",
"name": {
"type": "string"
}
}
}
```

### Invalid/Bad Example

```json
"Resource": {
"properties": {
"properties": {
"systemData": {
"$ref": "../../../../../common-types/resource-management/v2/types.json#/definitions/systemData",
}
}
}
}
```
44 changes: 37 additions & 7 deletions packages/rulesets/generated/spectral/az-arm.js
Original file line number Diff line number Diff line change
Expand Up @@ -2075,7 +2075,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 @@ -2110,7 +2110,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 @@ -2691,25 +2691,42 @@ 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$1,
path: _.concat(ctx.path, PROPERTIES$1, pathForUpperCamelCase[0]),
},
];
}
return [];
};

const TAGS = "tags";
const PROPERTIES = "properties";
const ERROR_MESSAGE = "Tags must be defined as a top-level property, not in the properties bag.";
const tagsAreTopLevelPropertiesOnly = (definition, _opts, ctx) => {
const properties = getProperties(definition);
const path = deepFindObjectKeyPath(properties, TAGS);
if (path.length > 0) {
return [
{
message: ERROR_MESSAGE,
path: _.concat(ctx.path, PROPERTIES, pathForUpperCamelCase[0]),
path: _.concat(ctx.path, PROPERTIES, path[0]),
},
];
}
Expand Down Expand Up @@ -3512,6 +3529,19 @@ const ruleset = {
function: consistentResponseSchemaForPut,
},
},
TagsAreTopLevelPropertiesOnly: {
rpcGuidelineCode: "RPC-Put-V1-30",
description: "Tags must be defined as a top-level property, not in the properties bag.",
severity: "error",
stagingOnly: true,
message: "{{error}}",
resolved: true,
formats: [oas2],
given: ["$.definitions.*.properties^"],
then: {
function: tagsAreTopLevelPropertiesOnly,
},
},
ParametersInPost: {
rpcGuidelineCode: "RPC-POST-V1-05",
description: "For a POST action parameters MUST be in the payload and not in the URI.",
Expand Down
8 changes: 4 additions & 4 deletions packages/rulesets/generated/spectral/az-dataplane.js
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ const ruleset$1 = {
description: "Operation should have a summary or description.",
message: "Operation should have a summary or description.",
severity: "warn",
disableForTypeSpec: true,
disableForTypeSpec: false,
disableForTypeSpecReason: "Covered by TSP's '@azure-tools/typespec-azure-core/documentation-required' rule.",
given: [
"$.paths[*][?( @property === 'get' && [email protected] && [email protected] )]",
Expand All @@ -663,7 +663,7 @@ const ruleset$1 = {
description: "All schemas should have a description or title.",
message: "Schema should have a description or title.",
severity: "warn",
disableForTypeSpec: true,
disableForTypeSpec: false,
disableForTypeSpecReason: "Covered by TSP's '@azure-tools/typespec-azure-core/documentation-required' rule.",
formats: [oas2, oas3],
given: ["$.definitions[?([email protected] && [email protected])]", "$.components.schemas[?([email protected] && [email protected])]"],
Expand All @@ -675,7 +675,7 @@ const ruleset$1 = {
description: "All parameters should have a description.",
message: "Parameter should have a description.",
severity: "warn",
disableForTypeSpec: true,
disableForTypeSpec: false,
disableForTypeSpecReason: "Covered by TSP's '@azure-tools/typespec-azure-core/documentation-required' rule.",
given: ["$.paths[*].parameters.*", "$.paths.*[get,put,post,patch,delete,options,head].parameters.*"],
then: {
Expand Down Expand Up @@ -991,7 +991,7 @@ const ruleset$1 = {
description: "The value of the 'description' property must be descriptive. It cannot be spaces or empty description.",
message: "'{{property}}' parameter lacks 'description' property. Consider adding a 'description' element. Accurate description is essential for maintaining reference documentation.",
severity: "error",
disableForTypeSpec: true,
disableForTypeSpec: false,
disableForTypeSpecReason: "Covered by TSP's '@azure-tools/typespec-azure-core/documentation-required' rule.",
resolved: false,
formats: [oas2],
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 { tagsAreTopLevelPropertiesOnly } from "./functions/tags-are-top-level-properties-only"
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 @@ -731,6 +732,21 @@ const ruleset: any = {
},
},

// RPC Code: RPC-Put-V1-30
TagsAreTopLevelPropertiesOnly: {
tejaswiMinnu marked this conversation as resolved.
Show resolved Hide resolved
rpcGuidelineCode: "RPC-Put-V1-30",
tejaswiMinnu marked this conversation as resolved.
Show resolved Hide resolved
description: "Tags must be defined as a top-level property, not in the properties bag.",
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: tagsAreTopLevelPropertiesOnly,
},
},

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

const TAGS = "tags"
const PROPERTIES = "properties"
const ERROR_MESSAGE = "Tags must be defined as a top-level property, not in the properties bag."

tejaswiMinnu marked this conversation as resolved.
Show resolved Hide resolved
export const tagsAreTopLevelPropertiesOnly = (definition: any, _opts: any, ctx: any) => {
const properties = getProperties(definition)
const path = deepFindObjectKeyPath(properties, TAGS)

if (path.length > 0) {
return [
{
message: ERROR_MESSAGE,
path: _.concat(ctx.path, PROPERTIES, path[0]),
},
]
}

return []
}
Loading