diff --git a/docs/suggest-scope-parameter.md b/docs/suggest-scope-parameter.md new file mode 100644 index 000000000..197a56fae --- /dev/null +++ b/docs/suggest-scope-parameter.md @@ -0,0 +1,35 @@ +# SuggestScopeParameter + +## Category + +ARM Warning + +## Applies to + +ARM OpenAPI(swagger) specs + +## Related ARM Guideline Code + +- N/a + +## Description + +OpenAPI authors can use the `scope` path parameter to indicate that an API is applicable to various scopes (Tenant, +Management Group, Subscription, Resource Group, etc.) rather than explicitly defining each scope in the spec. + +## How to fix + +Remove all explicitly-scoped paths that only vary in scope and create a path with the `scope` parameter. + +Example of explicitly-scoped paths that only vary in scope: + +2. Explicitly-scoped path (by subscription) + **`/subscriptions/{subscriptionId}`**`/providers/Microsoft.Bakery/breads` + +3. Explicitly-scoped path (by resource group): + **`/subscriptions/{subscriptionId}/resourcegroups/{resourceGroupName}`**`/providers/Microsoft.Bakery/breads` + +These two paths can be replaced with a single path that uses the `scope` parameter: + +1. Path with scope parameter: + **`/{scope}`**`/providers/Microsoft.Bakery/breads` diff --git a/packages/rulesets/CHANGELOG.md b/packages/rulesets/CHANGELOG.md index 3596c6084..8346dda4f 100644 --- a/packages/rulesets/CHANGELOG.md +++ b/packages/rulesets/CHANGELOG.md @@ -5,6 +5,7 @@ ### Patches - [AllTrackedResourcesMustHaveDelete][TrackedResourcePatchOperation] Skip the api paths if it has PrivateEndpointConnectionProxy +- [SuggestScopeParameter] Added this rule to suggest using the scope parameter if paths can be combined ### Patches diff --git a/packages/rulesets/src/native/legacyRules/GetCollectionResponseSchema.ts b/packages/rulesets/src/native/legacyRules/GetCollectionResponseSchema.ts index 1ecdf7485..3e4a5825c 100644 --- a/packages/rulesets/src/native/legacyRules/GetCollectionResponseSchema.ts +++ b/packages/rulesets/src/native/legacyRules/GetCollectionResponseSchema.ts @@ -14,7 +14,7 @@ rules.push({ appliesTo_JsonQuery: "$", *run(doc, node, path, ctx) { const msg = - 'The response in the GET collection operation "{0}" does not match the response definition in the individual GET operation "{1}" .' + 'The response in the GET collection operation "{0}" does not match the response definition in the individual GET operation "{1}".' /** * 1 travel all resources and find all the resources that have a collection get * - by searching all the models return by a get operation and verify the schema diff --git a/packages/rulesets/src/spectral/az-arm.ts b/packages/rulesets/src/spectral/az-arm.ts index af8bd0b0e..104282fde 100644 --- a/packages/rulesets/src/spectral/az-arm.ts +++ b/packages/rulesets/src/spectral/az-arm.ts @@ -46,6 +46,7 @@ import resourceNameRestriction from "./functions/resource-name-restriction" import responseSchemaSpecifiedForSuccessStatusCode from "./functions/response-schema-specified-for-success-status-code" import { securityDefinitionsStructure } from "./functions/security-definitions-structure" import skuValidation from "./functions/sku-validation" +import suggestScopeParameter from "./functions/suggest-scope-parameter" 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" @@ -319,6 +320,21 @@ const ruleset: any = { }, }, + SuggestScopeParameter: { + description: + "Duplicate operations that vary only by scope can be defined with a single operation that has a scope parameter. This reduces the number of operations in the spec.", + severity: "warn", + stagingOnly: true, + message: "{{error}}", + resolved: true, + formats: [oas2], + given: "$.paths", + then: { + field: "@key", + function: suggestScopeParameter, + }, + }, + /// /// ARM RPC rules for Get patterns /// diff --git a/packages/rulesets/src/spectral/functions/suggest-scope-parameter.ts b/packages/rulesets/src/spectral/functions/suggest-scope-parameter.ts new file mode 100644 index 000000000..f57fe4402 --- /dev/null +++ b/packages/rulesets/src/spectral/functions/suggest-scope-parameter.ts @@ -0,0 +1,40 @@ +/** + * Suggests combining paths that differ only in scope by defining a scope parameter. + * + * This function checks if the given path can be combined with other paths in the Swagger document + * by introducing a scope parameter. It returns suggestions for paths that differ only in scope. + */ + +const suggestScopeParameter = (path: any, _opts: any, ctx: any) => { + const swagger = ctx?.documentInventory?.resolved + + let lowerCasePath: string + + if ( + path === null || + typeof path !== "string" || + path.length === 0 || + swagger === null || + (lowerCasePath = path.toLocaleLowerCase()).includes("{scope}") + ) { + return [] + } + + const suffix = path.substring(path.lastIndexOf("/providers")).toLocaleLowerCase() + + // Find all paths that differ only in scope + const matchingPaths = Object.keys(swagger.paths).filter((p: string) => { + const lower = p.toLocaleLowerCase() + return !lower.includes("{scope}") && lower !== lowerCasePath && lower.endsWith(suffix) + }) + + return matchingPaths.map((match: string) => { + return { + // note: only matched path is included in the message so that Spectral can deduplicate errors + message: `Path with suffix "${suffix}" differs from another path only in scope. These paths share a suffix and can be combined by defining a scope parameter.`, + path: ctx.path, + } + }) +} + +export default suggestScopeParameter diff --git a/packages/rulesets/src/spectral/test/suggest-scope-parameter.test.ts b/packages/rulesets/src/spectral/test/suggest-scope-parameter.test.ts new file mode 100644 index 000000000..41871d96e --- /dev/null +++ b/packages/rulesets/src/spectral/test/suggest-scope-parameter.test.ts @@ -0,0 +1,295 @@ +import { Spectral } from "@stoplight/spectral-core" +import linterForRule from "./utils" + +let linter: Spectral + +beforeAll(async () => { + linter = linterForRule("SuggestScopeParameter") + return linter +}) + +test("SuggestScopeParameter should find errors for subscription and resource group scopes", async () => { + const oasDoc = { + swagger: "2.0", + paths: { + "/subscriptions/{subscriptionId}/resourcegroups/{resourceGroupName}/providers/Microsoft.Bakery/breads": { + get: { + parameters: [{ $ref: "#/parameters/SubscriptionIdParameter" }, { $ref: "#/parameters/ResourceGroupParameter" }], + }, + }, + "/subscriptions/{subscriptionId}/providers/Microsoft.Bakery/breads": { + get: { + parameters: [{ $ref: "#/parameters/SubscriptionIdParameter" }], + }, + }, + }, + parameters: { + SubscriptionIdParameter: { + name: "subscriptionId", + in: "path", + required: true, + }, + ResourceGroupParameter: { + name: "resourceGroupName", + in: "path", + required: true, + }, + }, + } + + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(2) + + // all errors should have the same message + expect(new Set(results.map((r) => r.message)).size).toBe(1) + + // each path should have an error + expect(results[0].path.join(".")).toBe( + "paths./subscriptions/{subscriptionId}/resourcegroups/{resourceGroupName}/providers/Microsoft.Bakery/breads", + ) + expect(results[1].path.join(".")).toBe("paths./subscriptions/{subscriptionId}/providers/Microsoft.Bakery/breads") + }) +}) + +test("SuggestScopeParameter should find no errors with scope parameter", () => { + const oasDoc = { + swagger: "2.0", + paths: { + "/{scope}/providers/Microsoft.Bakery/breads": { + get: { + parameters: [{ $ref: "#/parameters/ScopeParameter" }], + }, + }, + }, + parameters: { + ScopeParameter: { + name: "scope", + in: "path", + required: true, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(0) + }) +}) + +test("SuggestScopeParameter should find no errors with different resourcetypes", () => { + const oasDoc = { + swagger: "2.0", + paths: { + "/subscriptions/{subscriptionId}/resourcegroups/{resourceGroupName}/providers/Microsoft.Bakery/breads": { + get: { + parameters: [{ $ref: "#/parameters/SubscriptionIdParameter" }, { $ref: "#/parameters/ResourceGroupParameter" }], + }, + }, + "/subscriptions/{subscriptionId}/providers/Microsoft.Bakery/cookies": { + get: { + parameters: [{ $ref: "#/parameters/SubscriptionIdParameter" }], + }, + }, + }, + parameters: { + SubscriptionIdParameter: { + name: "subscriptionId", + in: "path", + required: true, + }, + ResourceGroupParameter: { + name: "resourceGroupName", + in: "path", + required: true, + }, + ScopeParameter: { + name: "scope", + in: "path", + required: true, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(0) + }) +}) + +test("SuggestScopeParameter should find errors for billing scope", () => { + const oasDoc = { + swagger: "2.0", + paths: { + "/{scope}/providers/Microsoft.Bakery/breads": { + get: { + parameters: [{ $ref: "#/parameters/ScopeParameter" }], + }, + }, + "providers/Microsoft.Billing/billingAccounts/{billingAccountId}/providers/Microsoft.Bakery/breads": { + get: { + parameters: [{ $ref: "#/parameters/BillingAccountIdParameter" }], + }, + }, + "providers/Microsoft.Billing/billingAccounts/{billingAccountId}/departments/{departmentId}/providers/Microsoft.Bakery/breads": { + get: { + parameters: [{ $ref: "#/parameters/BillingAccountIdParameter" }, { $ref: "#/parameters/DepartmentIdParameter" }], + }, + }, + "providers/Microsoft.Billing/billingAccounts/{billingAccountId}/enrollmentAccounts/{enrollmentAccountId}/providers/Microsoft.Bakery/breads": + { + get: { + parameters: [{ $ref: "#/parameters/BillingAccountIdParameter" }, { $ref: "#/parameters/EnrollmentAccountIdParameter" }], + }, + }, + "providers/Microsoft.Billing/billingAccounts/{billingAccountId}/billingProfiles/{billingProfileId}/providers/Microsoft.Bakery/breads": + { + get: { + parameters: [{ $ref: "#/parameters/BillingAccountIdParameter" }, { $ref: "#/parameters/BillingProfileIdParameter" }], + }, + }, + "providers/Microsoft.Billing/billingAccounts/{billingAccountId}/invoiceSections/{invoiceSectionId}/providers/Microsoft.Bakery/breads": + { + get: { + parameters: [{ $ref: "#/parameters/BillingAccountIdParameter" }, { $ref: "#/parameters/InvoiceSectionIdParameter" }], + }, + }, + }, + parameters: { + ScopeParameter: { + name: "scope", + in: "path", + required: true, + }, + BillingAccountIdParameter: { + name: "billAcc", + in: "path", + required: true, + }, + DepartmentIdParameter: { + name: "test", + in: "path", + required: true, + }, + EnrollmentAccountIdParameter: { + name: "enrollmentAcc", + in: "path", + required: true, + }, + BillingProfileIdParameter: { + name: "billProf", + in: "path", + required: true, + }, + InvoiceSectionIdParameter: { + name: "invoiceSection", + in: "path", + required: true, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(5) + + // all errors should have the same message + expect(new Set(results.map((r) => r.message)).size).toBe(1) + + // each path besides the one with the scope parameter should have an error + expect(results[0].path.join(".")).toBe( + "paths.providers/Microsoft.Billing/billingAccounts/{billingAccountId}/providers/Microsoft.Bakery/breads", + ) + expect(results[1].path.join(".")).toBe( + "paths.providers/Microsoft.Billing/billingAccounts/{billingAccountId}/departments/{departmentId}/providers/Microsoft.Bakery/breads", + ) + expect(results[2].path.join(".")).toBe( + "paths.providers/Microsoft.Billing/billingAccounts/{billingAccountId}/enrollmentAccounts/{enrollmentAccountId}/providers/Microsoft.Bakery/breads", + ) + expect(results[3].path.join(".")).toBe( + "paths.providers/Microsoft.Billing/billingAccounts/{billingAccountId}/billingProfiles/{billingProfileId}/providers/Microsoft.Bakery/breads", + ) + expect(results[4].path.join(".")).toBe( + "paths.providers/Microsoft.Billing/billingAccounts/{billingAccountId}/invoiceSections/{invoiceSectionId}/providers/Microsoft.Bakery/breads", + ) + }) +}) + +test("SuggestScopeParameter should not find errors for list and point get paths", () => { + const oasDoc = { + swagger: "2.0", + paths: { + "/{scope}/providers/Microsoft.Bakery/breads": { + get: { + parameters: [{ $ref: "#/parameters/ScopeParameter" }], + }, + }, + "/{scope}/providers/Microsoft.Bakery/breads/{breadName}": { + get: { + parameters: [{ $ref: "#/parameters/ScopeParameter" }], + }, + }, + }, + parameters: { + ScopeParameter: { + name: "scope", + in: "path", + required: true, + }, + }, + } + + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(0) + }) +}) + +test("SuggestScopeParameter should find errors for tenant level scope", async () => { + const oasDoc = { + swagger: "2.0", + paths: { + "/providers/Microsoft.Bakery/breads": { + get: { + parameters: [ + { + name: "body_param", + in: "body", + schema: { + properties: { prop: { type: "string" } }, + }, + }, + ], + }, + }, + "/subscriptions/{subscriptionId}/resourcegroups/{resourceGroupName}/providers/Microsoft.Bakery/breads": { + get: { + parameters: [{ $ref: "#/parameters/SubscriptionIdParameter" }, { $ref: "#/parameters/ResourceGroupParameter" }], + }, + }, + "/subscriptions/{subscriptionId}/providers/Microsoft.Bakery/breads": { + get: { + parameters: [{ $ref: "#/parameters/SubscriptionIdParameter" }], + }, + }, + }, + parameters: { + SubscriptionIdParameter: { + name: "subscriptionId", + in: "path", + required: true, + }, + ResourceGroupParameter: { + name: "resourceGroupName", + in: "path", + required: true, + }, + }, + } + + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(3) + + // all errors should have the same message + expect(new Set(results.map((r) => r.message)).size).toBe(1) + + // each path should have an error + expect(results[0].path.join(".")).toBe("paths./providers/Microsoft.Bakery/breads") + expect(results[1].path.join(".")).toBe( + "paths./subscriptions/{subscriptionId}/resourcegroups/{resourceGroupName}/providers/Microsoft.Bakery/breads", + ) + expect(results[2].path.join(".")).toBe("paths./subscriptions/{subscriptionId}/providers/Microsoft.Bakery/breads") + }) +})