diff --git a/docs/billing-data-in-properties-bag.md b/docs/billing-data-in-properties-bag.md new file mode 100644 index 000000000..ca406b6f0 --- /dev/null +++ b/docs/billing-data-in-properties-bag.md @@ -0,0 +1,57 @@ +# BillingDataInPropertiesBag + +## Category + +ARM Error + +## Applies to + +ARM OpenAPI(swagger) specs + +## Description + +A property named `BillingData` (matched case-insensitively) must not be present in a resource's properties bag. If billing-related data is required, model it under a dedicated, appropriately named property or a separate model definition instead of placing a `BillingData` property directly in the resource properties bag. + +## How to fix the violation + +Remove the `BillingData` property from the resource properties bag. Represent the information using a differently named property or a dedicated model definition as appropriate. + +### Valid/Good Example + +```json +"Resource": { + "properties": { + "properties": { + "provisioningState": { + "type": "string" + } + } + } +} +``` + +### Invalid/Bad Example + +```json +"Resource": { + "properties": { + "properties": { + "billingData": { + "type": "string" + } + } + } +} +``` + +```json +"Resource": { + "properties": { + "properties": { + "billingData": { + "$ref": "#/definitions/BillingData" + } + } + } +} +``` diff --git a/docs/rules.md b/docs/rules.md index 7f2703807..5e738fb6b 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -145,6 +145,12 @@ This rule is to check if the tags definition of a resource conforms to the commo Please refer to [azure-resource-tags-schema.md](./azure-resource-tags-schema.md) for details. +### BillingDataInPropertiesBag + +A property named `BillingData` (matched case-insensitively) must not be present in a resource's properties bag. If billing-related data is required, model it under a dedicated, appropriately named property or a separate model definition instead of placing a `BillingData` property directly in the resource properties bag. + +Please refer to [billing-data-in-properties-bag.md](./billing-data-in-properties-bag.md) for details. + ### BodyPropertiesNamesCamelCase This violation is flagged if a request body parameter's property name (BodyPropertiesNamesCamelCase) is not in `camelCase` format. This is because the model's properties are sent across the wire and must adhere to common Json conventions for naming. This implies that every property name must start with a lower cased letter and all subsequent words within the name must start with a capital letter. In cases where there are acronyms involved, a maximum of three contiguous capital letters are allowed (acronyms should not be longer that 2 characters - see https://msdn.microsoft.com/en-us/library/141e06ef(v=vs.71).aspx, third upper case could be start of next word). Eg: `redisCache`, `publicIPAddress`, `location` are valid, but `sampleSQLQuery` is not (must be renamed as `sampleSqlQuery`). diff --git a/packages/rulesets/CHANGELOG.md b/packages/rulesets/CHANGELOG.md index cd31fd3aa..20b849d2b 100644 --- a/packages/rulesets/CHANGELOG.md +++ b/packages/rulesets/CHANGELOG.md @@ -1,5 +1,11 @@ # Change Log - @microsoft.azure/openapi-validator-rulesets +## 2.2.7 + +### Patches + +- Added rule BillingDataInPropertiesBag to disallow the reserved 'BillingData' property name (case-insensitive) in a resource's properties bag + ## 2.2.6 ### Patches diff --git a/packages/rulesets/generated/spectral/az-arm.js b/packages/rulesets/generated/spectral/az-arm.js index c400a4631..79085735f 100644 --- a/packages/rulesets/generated/spectral/az-arm.js +++ b/packages/rulesets/generated/spectral/az-arm.js @@ -1401,6 +1401,53 @@ const verifyArmPath = createRulesetFunction({ return errors; }); +const BILLING_DATA = "billingdata"; +const PROPERTIES$2 = "properties"; +const ERROR_MESSAGE$3 = "The 'BillingData' property is not allowed in the resource properties bag."; +function collectBillingDataPropertyPaths(schema, basePath, matches, visited) { + if (!_.isObject(schema) || visited.has(schema)) { + return; + } + visited.add(schema); + const s = schema; + if (_.isObject(s.properties)) { + for (const [name, propertySchema] of Object.entries(s.properties)) { + if (name.toLowerCase() === BILLING_DATA) { + matches.push([...basePath, PROPERTIES$2, name]); + } + collectBillingDataPropertyPaths(propertySchema, [...basePath, PROPERTIES$2, name], matches, visited); + } + } + for (const keyword of ["allOf", "anyOf", "oneOf"]) { + const subschemas = s[keyword]; + if (Array.isArray(subschemas)) { + subschemas.forEach((subschema, index) => { + collectBillingDataPropertyPaths(subschema, [...basePath, keyword, index], matches, visited); + }); + } + } + if (Array.isArray(s.items)) { + s.items.forEach((itemSchema, index) => { + collectBillingDataPropertyPaths(itemSchema, [...basePath, "items", index], matches, visited); + }); + } + else if (_.isObject(s.items)) { + collectBillingDataPropertyPaths(s.items, [...basePath, "items"], matches, visited); + } + if (_.isObject(s.additionalProperties)) { + collectBillingDataPropertyPaths(s.additionalProperties, [...basePath, "additionalProperties"], matches, visited); + } +} +const billingDataInPropertiesBag = (definition, _opts, ctx) => { + const bag = getProperties(definition); + const matches = []; + collectBillingDataPropertyPaths(bag, [], matches, new WeakSet()); + return matches.map((path) => ({ + message: ERROR_MESSAGE$3, + path: _.concat(ctx.path, PROPERTIES$2, path), + })); +}; + const bodyParamRepeatedInfo = (pathItem, _opts, paths) => { if (pathItem === null || typeof pathItem !== "object") { return []; @@ -4062,6 +4109,18 @@ const ruleset = { function: systemDataInPropertiesBag, }, }, + BillingDataInPropertiesBag: { + description: "The 'BillingData' property is not allowed in the resource properties bag.", + message: "{{description}}", + severity: "error", + stagingOnly: true, + resolved: true, + formats: [oas2], + given: ["$.definitions.*.properties[?(@property === 'properties')]^"], + then: { + function: billingDataInPropertiesBag, + }, + }, ReservedResourceNamesModelAsEnum: { rpcGuidelineCode: "RPC-ConstrainedCollections-V1-04", description: "Service-defined (reserved) resource names should be represented as an enum type with modelAsString set to true, not as a static string in the path.", diff --git a/packages/rulesets/package.json b/packages/rulesets/package.json index 349e69fa5..8b142d73d 100644 --- a/packages/rulesets/package.json +++ b/packages/rulesets/package.json @@ -1,6 +1,6 @@ { "name": "@microsoft.azure/openapi-validator-rulesets", - "version": "2.2.6", + "version": "2.2.7", "description": "Azure OpenAPI Validator", "main": "dist/index.js", "files": [ diff --git a/packages/rulesets/src/spectral/az-arm.ts b/packages/rulesets/src/spectral/az-arm.ts index cab09555d..f190d4ea7 100644 --- a/packages/rulesets/src/spectral/az-arm.ts +++ b/packages/rulesets/src/spectral/az-arm.ts @@ -2,6 +2,7 @@ import { oas2 } from "@stoplight/spectral-formats" import { falsy, pattern, truthy } from "@stoplight/spectral-functions" import common from "./az-common" import verifyArmPath from "./functions/arm-path-validation" +import { billingDataInPropertiesBag } from "./functions/billing-data-in-properties-bag" import bodyParamRepeatedInfo from "./functions/body-param-repeated-info" import { camelCase } from "./functions/camel-case" import collectionObjectPropertiesNaming from "./functions/collection-object-properties-naming" @@ -1026,6 +1027,22 @@ const ruleset: any = { }, }, + // A property named 'BillingData' (matched case-insensitively) must not be present in a + // resource's properties bag. + BillingDataInPropertiesBag: { + description: "The 'BillingData' property is not allowed in the resource properties bag.", + message: "{{description}}", + severity: "error", + stagingOnly: true, + resolved: true, + formats: [oas2], + // given definitions that have the properties bag + given: ["$.definitions.*.properties[?(@property === 'properties')]^"], + then: { + function: billingDataInPropertiesBag, + }, + }, + /// /// ARM RPC rules for constrained resource collections /// diff --git a/packages/rulesets/src/spectral/functions/billing-data-in-properties-bag.ts b/packages/rulesets/src/spectral/functions/billing-data-in-properties-bag.ts new file mode 100644 index 000000000..2d4d4d47a --- /dev/null +++ b/packages/rulesets/src/spectral/functions/billing-data-in-properties-bag.ts @@ -0,0 +1,74 @@ +// BillingDataInPropertiesBag +// A property named 'BillingData' (matched case-insensitively) must not be defined in a +// resource's properties bag. + +import _ from "lodash" +import { getProperties } from "./utils" + +const BILLING_DATA = "billingdata" // compared case-insensitively +const PROPERTIES = "properties" +const ERROR_MESSAGE = "The 'BillingData' property is not allowed in the resource properties bag." + +// Recursively collect the path to every property named "BillingData" (case-insensitive) that is +// defined within the given schema. Traversal is restricted to schema-structure keywords +// (properties, allOf/anyOf/oneOf, items, additionalProperties) so that only actual property +// definitions are inspected. Values inside non-structural metadata (e.g. default values, enum +// values, examples, or vendor extensions) are intentionally ignored to avoid false positives. +function collectBillingDataPropertyPaths( + schema: any, + basePath: (string | number)[], + matches: (string | number)[][], + visited: WeakSet +): void { + if (!_.isObject(schema) || visited.has(schema)) { + return + } + visited.add(schema) + + const s = schema as { [key: string]: any } + + // properties: a map of property name -> property schema + if (_.isObject(s.properties)) { + for (const [name, propertySchema] of Object.entries(s.properties as { [key: string]: any })) { + if (name.toLowerCase() === BILLING_DATA) { + matches.push([...basePath, PROPERTIES, name]) + } + collectBillingDataPropertyPaths(propertySchema, [...basePath, PROPERTIES, name], matches, visited) + } + } + + // allOf / anyOf / oneOf: arrays of subschemas + for (const keyword of ["allOf", "anyOf", "oneOf"]) { + const subschemas = s[keyword] + if (Array.isArray(subschemas)) { + subschemas.forEach((subschema: any, index: number) => { + collectBillingDataPropertyPaths(subschema, [...basePath, keyword, index], matches, visited) + }) + } + } + + // items: a single subschema or an array of subschemas + if (Array.isArray(s.items)) { + s.items.forEach((itemSchema: any, index: number) => { + collectBillingDataPropertyPaths(itemSchema, [...basePath, "items", index], matches, visited) + }) + } else if (_.isObject(s.items)) { + collectBillingDataPropertyPaths(s.items, [...basePath, "items"], matches, visited) + } + + // additionalProperties: a subschema (when it is an object rather than a boolean) + if (_.isObject(s.additionalProperties)) { + collectBillingDataPropertyPaths(s.additionalProperties, [...basePath, "additionalProperties"], matches, visited) + } +} + +export const billingDataInPropertiesBag = (definition: any, _opts: any, ctx: any) => { + const bag = getProperties(definition) + const matches: (string | number)[][] = [] + collectBillingDataPropertyPaths(bag, [], matches, new WeakSet()) + + return matches.map((path) => ({ + message: ERROR_MESSAGE, + path: _.concat(ctx.path, PROPERTIES, path), + })) +} diff --git a/packages/rulesets/src/spectral/test/billing-data-in-properties-bag.test.ts b/packages/rulesets/src/spectral/test/billing-data-in-properties-bag.test.ts new file mode 100644 index 000000000..00460a19c --- /dev/null +++ b/packages/rulesets/src/spectral/test/billing-data-in-properties-bag.test.ts @@ -0,0 +1,445 @@ +import { Spectral } from "@stoplight/spectral-core" +import linterForRule from "./utils" + +let linter: Spectral +const RULE = "BillingDataInPropertiesBag" +const ERROR_MESSAGE = "The 'BillingData' property is not allowed in the resource properties bag." + +beforeAll(async () => { + linter = await linterForRule(RULE) + return linter +}) + +test(`${RULE} should find no errors when billingData is absent from the properties bag`, async () => { + const oasDoc = { + swagger: "2.0", + paths: {}, + definitions: { + Resource: { + properties: { + properties: { + type: "object", + properties: { + provisioningState: { + type: "string", + }, + }, + }, + }, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(0) + }) +}) + +test(`${RULE} should find errors when billingData references a model definition`, async () => { + const oasDoc = { + swagger: "2.0", + paths: {}, + definitions: { + Resource: { + properties: { + properties: { + type: "object", + properties: { + billingData: { + $ref: "#/definitions/BillingData", + }, + }, + }, + }, + }, + BillingData: { + type: "object", + properties: { + amount: { + type: "number", + }, + }, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(1) + expect(results[0].path.join(".")).toBe("definitions.Resource.properties.properties.properties.billingData") + expect(results[0].message).toBe(ERROR_MESSAGE) + }) +}) + +test(`${RULE} should find errors when billingData is a primitive type`, async () => { + const oasDoc = { + swagger: "2.0", + paths: {}, + definitions: { + Resource: { + properties: { + properties: { + type: "object", + properties: { + billingData: { + type: "string", + }, + }, + }, + }, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(1) + expect(results[0].path.join(".")).toBe("definitions.Resource.properties.properties.properties.billingData") + expect(results[0].message).toBe(ERROR_MESSAGE) + }) +}) + +test(`${RULE} should find errors when billingData is an inline model`, async () => { + const oasDoc = { + swagger: "2.0", + paths: {}, + definitions: { + Resource: { + properties: { + properties: { + type: "object", + properties: { + billingData: { + type: "object", + properties: { + amount: { + type: "number", + }, + }, + }, + }, + }, + }, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(1) + expect(results[0].path.join(".")).toBe("definitions.Resource.properties.properties.properties.billingData") + expect(results[0].message).toBe(ERROR_MESSAGE) + }) +}) + +test(`${RULE} should be case-insensitive when matching the property name`, async () => { + const oasDoc = { + swagger: "2.0", + paths: {}, + definitions: { + Resource: { + properties: { + properties: { + type: "object", + properties: { + BillingData: { + type: "string", + }, + BILLINGDATA: { + type: "string", + }, + }, + }, + }, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(2) + expect(results[0].path.join(".")).toBe("definitions.Resource.properties.properties.properties.BillingData") + expect(results[1].path.join(".")).toBe("definitions.Resource.properties.properties.properties.BILLINGDATA") + expect(results[0].message).toBe(ERROR_MESSAGE) + expect(results[1].message).toBe(ERROR_MESSAGE) + }) +}) + +test(`${RULE} should not flag properties that only contain 'billingData' as a substring`, async () => { + const oasDoc = { + swagger: "2.0", + paths: {}, + definitions: { + Resource: { + properties: { + properties: { + type: "object", + properties: { + billingDataId: { + type: "string", + }, + }, + }, + }, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(0) + }) +}) + +test(`${RULE} should find errors when billingData is in a referenced properties definition`, async () => { + const oasDoc = { + swagger: "2.0", + paths: {}, + definitions: { + Resource: { + properties: { + properties: { + $ref: "#/definitions/ResourceProperties", + }, + }, + }, + ResourceProperties: { + type: "object", + properties: { + billingData: { + type: "string", + }, + }, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(1) + expect(results[0].path.join(".")).toBe("definitions.Resource.properties.properties.properties.billingData") + expect(results[0].message).toBe(ERROR_MESSAGE) + }) +}) + +test(`${RULE} should find errors when billingData is nested inside another property definition`, async () => { + const oasDoc = { + swagger: "2.0", + paths: {}, + definitions: { + Resource: { + properties: { + properties: { + type: "object", + properties: { + nested: { + type: "object", + properties: { + billingData: { + type: "string", + }, + }, + }, + }, + }, + }, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(1) + expect(results[0].path.join(".")).toBe( + "definitions.Resource.properties.properties.properties.nested.properties.billingData" + ) + expect(results[0].message).toBe(ERROR_MESSAGE) + }) +}) + +test(`${RULE} should not flag billingData defined as a top-level property outside the properties bag`, async () => { + const oasDoc = { + swagger: "2.0", + paths: {}, + definitions: { + Resource: { + properties: { + properties: { + type: "object", + properties: { + provisioningState: { + type: "string", + }, + }, + }, + billingData: { + type: "string", + }, + }, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(0) + }) +}) + +test(`${RULE} should not flag billingData keys that appear inside non-structural schema metadata`, async () => { + const oasDoc = { + swagger: "2.0", + paths: {}, + definitions: { + Resource: { + properties: { + properties: { + type: "object", + properties: { + config: { + type: "object", + default: { + billingData: "someDefaultValue", + }, + enum: [ + { + billingData: 1, + }, + ], + "x-ms-metadata": { + billingData: true, + }, + }, + }, + }, + }, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(0) + }) +}) + +test(`${RULE} should find errors when billingData is defined via allOf composition`, async () => { + const oasDoc = { + swagger: "2.0", + paths: {}, + definitions: { + Resource: { + properties: { + properties: { + allOf: [ + { + type: "object", + properties: { + billingData: { + type: "string", + }, + }, + }, + ], + }, + }, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(1) + expect(results[0].path.join(".")).toBe("definitions.Resource.properties.properties.allOf.0.properties.billingData") + expect(results[0].message).toBe(ERROR_MESSAGE) + }) +}) + +test(`${RULE} should find errors when billingData is defined in array items`, async () => { + const oasDoc = { + swagger: "2.0", + paths: {}, + definitions: { + Resource: { + properties: { + properties: { + type: "object", + properties: { + list: { + type: "array", + items: { + type: "object", + properties: { + billingData: { + type: "string", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(1) + expect(results[0].path.join(".")).toBe( + "definitions.Resource.properties.properties.properties.list.items.properties.billingData" + ) + expect(results[0].message).toBe(ERROR_MESSAGE) + }) +}) + +test(`${RULE} should find errors when billingData is defined in tuple-style array items`, async () => { + const oasDoc = { + swagger: "2.0", + paths: {}, + definitions: { + Resource: { + properties: { + properties: { + type: "object", + properties: { + tuple: { + type: "array", + items: [ + { + type: "object", + properties: { + billingData: { + type: "string", + }, + }, + }, + ], + }, + }, + }, + }, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(1) + expect(results[0].path.join(".")).toBe( + "definitions.Resource.properties.properties.properties.tuple.items.0.properties.billingData" + ) + expect(results[0].message).toBe(ERROR_MESSAGE) + }) +}) + +test(`${RULE} should find errors when billingData is defined in additionalProperties`, async () => { + const oasDoc = { + swagger: "2.0", + paths: {}, + definitions: { + Resource: { + properties: { + properties: { + type: "object", + properties: { + map: { + type: "object", + additionalProperties: { + type: "object", + properties: { + billingData: { + type: "string", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(1) + expect(results[0].path.join(".")).toBe( + "definitions.Resource.properties.properties.properties.map.additionalProperties.properties.billingData" + ) + expect(results[0].message).toBe(ERROR_MESSAGE) + }) +})