diff --git a/docs/patch-body-parameters-schema.md b/docs/patch-body-parameters-schema.md index 5681ff87..f3a4b259 100644 --- a/docs/patch-body-parameters-schema.md +++ b/docs/patch-body-parameters-schema.md @@ -12,10 +12,6 @@ ARM and Data plane OpenAPI(swagger) specs - RPC-Patch-V1-10 -## Output Message - -Properties of a PATCH request body must not be {0}. PATCH operation: '{1}' Model Definition: '{2}' Property: '{3}' - ## Description A request parameter of the Patch Operation must not have a required/default/'x-ms-mutability:"create"' value. diff --git a/packages/rulesets/CHANGELOG.md b/packages/rulesets/CHANGELOG.md index 3596c608..82a039d7 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 +- [PatchBodyParametersSchema] Skip validation for MSI (managed service identity) as it is being referenced from common-types and has the required field & is being referenced in patch body parameter schema by several RP's who are having to get an exception. ### Patches diff --git a/packages/rulesets/generated/spectral/az-arm.js b/packages/rulesets/generated/spectral/az-arm.js index 785b381e..9088d6bf 100644 --- a/packages/rulesets/generated/spectral/az-arm.js +++ b/packages/rulesets/generated/spectral/az-arm.js @@ -2036,7 +2036,7 @@ const parametersInPost = (postParameters, _opts, ctx) => { return errors; }; -const patchBodyParameters = (parameters, _opts, paths) => { +const patchBodyParameters = (parameters, _opts, paths, isTopLevel = true) => { if (parameters === null || parameters.schema === undefined || parameters.in !== "body") { return []; } @@ -2045,6 +2045,9 @@ const patchBodyParameters = (parameters, _opts, paths) => { const requiredProperties = getRequiredProperties(parameters.schema); const errors = []; for (const prop of Object.keys(properties)) { + if (isTopLevel && prop.toLowerCase() === "identity") { + continue; + } if (properties[prop].default) { errors.push({ message: `Properties of a PATCH request body must not have default value, property:${prop}.`, @@ -2068,7 +2071,7 @@ const patchBodyParameters = (parameters, _opts, paths) => { errors.push(...patchBodyParameters({ schema: properties[prop], in: "body", - }, _opts, { path: [...path, "schema", "properties", prop] })); + }, _opts, { path: [...path, "schema", "properties", prop] }, false)); } } return errors; diff --git a/packages/rulesets/src/spectral/functions/patch-body-parameters.ts b/packages/rulesets/src/spectral/functions/patch-body-parameters.ts index d7eec132..6a464831 100644 --- a/packages/rulesets/src/spectral/functions/patch-body-parameters.ts +++ b/packages/rulesets/src/spectral/functions/patch-body-parameters.ts @@ -2,28 +2,37 @@ import type { IFunctionResult } from "@stoplight/spectral-core" import { getProperties, getRequiredProperties } from "./utils" //This rule appears if in the patch body parameters have properties which is marked as required or x-ms-mutability:["create"] or have default -const patchBodyParameters = (parameters: any, _opts: any, paths: any): IFunctionResult[] => { +const patchBodyParameters = (parameters: any, _opts: any, paths: any, isTopLevel: boolean = true): IFunctionResult[] => { if (parameters === null || parameters.schema === undefined || parameters.in !== "body") { return [] } + const path = paths.path || [] const properties: object = getProperties(parameters.schema) const requiredProperties = getRequiredProperties(parameters.schema) const errors = [] for (const prop of Object.keys(properties)) { + // skip validation for identity property only at the top level + // as it refers MSI(managed service identity) from common-types + if (isTopLevel && prop.toLowerCase() === "identity") { + continue + } + if (properties[prop].default) { errors.push({ message: `Properties of a PATCH request body must not have default value, property:${prop}.`, path: [...path, "schema"], }) } + if (requiredProperties.includes(prop)) { errors.push({ message: `Properties of a PATCH request body must not be required, property:${prop}.`, path: [...path, "schema"], }) } + const xmsMutability = properties[prop]["x-ms-mutability"] if (xmsMutability && xmsMutability.length === 1 && xmsMutability[0] === "create") { errors.push({ @@ -40,8 +49,9 @@ const patchBodyParameters = (parameters: any, _opts: any, paths: any): IFunction in: "body", }, _opts, - { path: [...path, "schema", "properties", prop] } - ) + { path: [...path, "schema", "properties", prop] }, + false, + ), ) } } diff --git a/packages/rulesets/src/spectral/test/patch-body-parameters.test.ts b/packages/rulesets/src/spectral/test/patch-body-parameters.test.ts index 4d73fd7a..5f4b3ff4 100644 --- a/packages/rulesets/src/spectral/test/patch-body-parameters.test.ts +++ b/packages/rulesets/src/spectral/test/patch-body-parameters.test.ts @@ -108,6 +108,184 @@ test("PatchBodyParametersSchema should find errors for required/create value", ( }) }) +test("PatchBodyParametersSchema should skip errors for MSI though has required value only at top level", () => { + const oasDoc = { + swagger: "2.0", + paths: { + "/foo": { + patch: { + tags: ["SampleTag"], + operationId: "Foo_Update", + description: "Test Description", + parameters: [ + { + name: "foo_patch", + in: "body", + schema: { + $ref: "#/definitions/OpenShiftClusterUpdate", + }, + }, + ], + responses: {}, + }, + }, + }, + definitions: { + OpenShiftClusterUpdate: { + description: "OpenShiftCluster represents an Azure Red Hat OpenShift cluster.", + type: "object", + properties: { + tags: { + $ref: "#/definitions/Tags", + description: "The resource tags.", + }, + identity: { + $ref: "#/definitions/ManagedServiceIdentity", + description: "Identity stores information about the cluster MSI(s) in a workload identity cluster.", + }, + }, + }, + ManagedServiceIdentity: { + description: "Managed service identity (system assigned and/or user assigned identities)", + type: "object", + properties: { + principalId: { + readOnly: true, + format: "uuid", + type: "string", + description: + "The service principal ID of the system assigned identity. This property will only be provided for a system assigned identity.", + }, + tenantId: { + readOnly: true, + format: "uuid", + type: "string", + description: + "The tenant ID of the system assigned identity. This property will only be provided for a system assigned identity.", + }, + type: { + description: "Type of managed service identity (where both SystemAssigned and UserAssigned types are allowed).", + type: "string", + }, + userAssignedIdentities: { + description: + "The set of user assigned identities associated with the resource. The userAssignedIdentities dictionary keys will be ARM resource ids in the form: '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ManagedIdentity/userAssignedIdentities/{identityName}. The dictionary values can be empty objects ({}) in requests.", + type: "object", + }, + }, + required: ["type"], + }, + Tags: { + description: "Tags represents an OpenShift cluster's tags.", + type: "object", + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(0) + }) +}) + +test("PatchBodyParametersSchema should not skip validation for non top level identity property", () => { + const oasDoc = { + swagger: "2.0", + paths: { + "/foo": { + patch: { + tags: ["SampleTag"], + operationId: "Foo_Update", + description: "Test Description", + parameters: [ + { + name: "foo_patch", + in: "body", + schema: { + $ref: "#/definitions/OpenShiftClusterUpdate", + }, + }, + ], + responses: {}, + }, + }, + }, + definitions: { + OpenShiftClusterUpdate: { + description: "OpenShiftCluster represents an Azure Red Hat OpenShift cluster.", + type: "object", + properties: { + identity: { + $ref: "#/definitions/ManagedServiceIdentity", + description: "Identity stores information about the cluster MSI(s) in a workload identity cluster.", + }, + testIdentity: { + $ref: "#/definitions/foo", + }, + }, + }, + ManagedServiceIdentity: { + description: "Managed service identity (system assigned and/or user assigned identities)", + type: "object", + properties: { + principalId: { + readOnly: true, + format: "uuid", + type: "string", + description: + "The service principal ID of the system assigned identity. This property will only be provided for a system assigned identity.", + }, + tenantId: { + readOnly: true, + format: "uuid", + type: "string", + description: + "The tenant ID of the system assigned identity. This property will only be provided for a system assigned identity.", + }, + type: { + description: "Type of managed service identity (where both SystemAssigned and UserAssigned types are allowed).", + type: "string", + }, + userAssignedIdentities: { + description: + "The set of user assigned identities associated with the resource. The userAssignedIdentities dictionary keys will be ARM resource ids in the form: '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ManagedIdentity/userAssignedIdentities/{identityName}. The dictionary values can be empty objects ({}) in requests.", + type: "object", + }, + }, + required: ["type"], + }, + TestManagedServiceIdentity: { + type: "object", + properties: { + type: { + description: "Type of managed service identity (where both SystemAssigned and UserAssigned types are allowed).", + type: "string", + }, + userAssignedIdentities: { + description: + "The set of user assigned identities associated with the resource. The userAssignedIdentities dictionary keys will be ARM resource ids in the form: '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ManagedIdentity/userAssignedIdentities/{identityName}. The dictionary values can be empty objects ({}) in requests.", + type: "object", + }, + }, + required: ["type"], + }, + foo: { + description: "OpenShiftCluster represents an Azure Red Hat OpenShift cluster.", + type: "object", + properties: { + identity: { + $ref: "#/definitions/TestManagedServiceIdentity", + description: "Identity stores information about the cluster MSI(s) in a workload identity cluster.", + }, + }, + }, + }, + } + return linter.run(oasDoc).then((results) => { + expect(results.length).toBe(1) + expect(results[0].path.join(".")).toBe("paths./foo.patch.parameters.0.schema.properties.testIdentity") + expect(results[0].message).toContain("Properties of a PATCH request body must not be required, property:type.") + }) +}) + test("PatchBodyParametersSchema should find errors for default value in nested body parameter", () => { const oasDoc = { swagger: "2.0", @@ -178,7 +356,7 @@ test("PatchBodyParametersSchema should find errors for default value in nested b } return linter.run(oasDoc).then((results) => { expect(results.length).toBe(2) - results.sort((a, b) => a.path.join('.').localeCompare(b.path.join('.'))); + results.sort((a, b) => a.path.join(".").localeCompare(b.path.join("."))) expect(results[0].path.join(".")).toBe("paths./bar.patch.parameters.0.schema.properties.properties") expect(results[0].message).toContain("Properties of a PATCH request body must not have default value, property:prop0.") expect(results[1].path.join(".")).toBe("paths./foo.patch.parameters.0.schema.properties.properties")