Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
changeKind: feature
packages:
- "@azure-tools/typespec-azure-resource-manager"
- "@azure-tools/typespec-azure-rulesets"
---

Add the `no-unsupported-patch-properties` ARM linter rule, which reports writable resource identity, location, and provisioning state properties in PATCH request bodies. Register the rule as disabled by default in the ARM ruleset.
1 change: 1 addition & 0 deletions packages/typespec-azure-resource-manager/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ Available ruleSets:
| [`@azure-tools/typespec-azure-resource-manager/no-empty-model`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-empty-model) | ARM Properties with type:object that don't reference a model definition are not allowed. ARM doesn't allow generic type definitions as this leads to bad customer experience. |
| [`@azure-tools/typespec-azure-resource-manager/no-reserved-resource-property`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-reserved-resource-property) | Reserved property names (for example 'billingData') must not be present in a resource's property bag. The property name is matched case-insensitively. |
| [`@azure-tools/typespec-azure-resource-manager/no-query-in-point-op`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-query-in-point-op) | Point operations must not declare query parameters beyond api-version. |
| [`@azure-tools/typespec-azure-resource-manager/no-unsupported-patch-properties`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-unsupported-patch-properties) | ARM PATCH request bodies must not contain writable resource identity, location, or provisioning state properties. |

## Decorators

Expand Down
2 changes: 2 additions & 0 deletions packages/typespec-azure-resource-manager/src/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { noQueryInPointOpRule } from "./rules/no-query-in-point-op.js";
import { noReservedResourcePropertyRule } from "./rules/no-reserved-resource-property.js";
import { deleteOperationMissingRule } from "./rules/no-resource-delete-operation.js";
import { noResponseBodyRule } from "./rules/no-response-body.js";
import { noUnsupportedPatchPropertiesRule } from "./rules/no-unsupported-patch-properties.js";
import { operationsInterfaceMissingRule } from "./rules/operations-interface-missing.js";
import { patchEnvelopePropertiesRules } from "./rules/patch-envelope-properties.js";
import { resourceNameRule } from "./rules/resource-name.js";
Expand Down Expand Up @@ -95,6 +96,7 @@ const rules = [
noEmptyModel,
noReservedResourcePropertyRule,
noQueryInPointOpRule,
noUnsupportedPatchPropertiesRule,
];

export const $linter = defineLinter({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
ARM PATCH request bodies must not make resource identity, location, or provisioning state properties writable. A PATCH operation cannot change top-level `id`, `name`, `type`, or `location`, or `properties.provisioningState`.

Remove these properties from the PATCH body, mark identity and provisioning state properties with read-only visibility, and mark location as read-only and create-only.

## Impact

- **Area:** API

Writable system-managed properties let generated SDKs offer updates that the service cannot safely apply and violate the ARM RPC contract.

## ❌ Incorrect

```tsp
model WidgetPatch {
id?: string;
location?: string;
properties?: WidgetPatchProperties;
}

model WidgetPatchProperties {
provisioningState?: string;
}
```

## ✅ Correct

```tsp
model WidgetPatch {
@visibility(Lifecycle.Read)
id?: string;

@visibility(Lifecycle.Read, Lifecycle.Create)
location?: string;

properties?: WidgetPatchProperties;
}

model WidgetPatchProperties {
@visibility(Lifecycle.Read)
provisioningState?: ResourceProvisioningState;
}
```

## LintDiff Equivalent

This rule corresponds to the Swagger validator rule [UnSupportedPatchProperties](https://github.com/Azure/azure-openapi-validator/blob/main/docs/un-supported-patch-properties.md).
Original file line number Diff line number Diff line change
@@ -0,0 +1,227 @@
import {
createRule,
fileRef,
getLifecycleVisibilityEnum,
getLocationContext,
getVisibilityForClass,
isNeverType,
isNullType,
paramMessage,
resolveEncodedName,
type DiagnosticTarget,
type Model,
type ModelProperty,
type Operation,
type Program,
type Type,
} from "@typespec/compiler";
import {
createMetadataInfo,
getHttpOperation,
resolveRequestVisibility,
Visibility,
type MetadataInfo,
} from "@typespec/http";

import { resolveProviderNamespace } from "../namespace.js";

const unsupportedPatchProperties = new Set(["id", "name", "type", "location"]);

export const noUnsupportedPatchPropertiesRule = createRule({
name: "no-unsupported-patch-properties",
docs: fileRef.fromPackageRoot("src/rules/no-unsupported-patch-properties.md"),
description:
"ARM PATCH request bodies must not contain writable resource identity, location, or provisioning state properties.",
severity: "warning",
url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-unsupported-patch-properties",
messages: {
default: paramMessage`PATCH request body property '${"propertyName"}' is not patchable and should be removed or made read-only or immutable.`,
},
create(context) {
return {
operation: (operation) => {
const namespace = operation.interface?.namespace ?? operation.namespace;
if (resolveProviderNamespace(context.program, namespace) === undefined) {
return;
}
Comment thread
msyyc marked this conversation as resolved.
Outdated

const [httpOperation] = getHttpOperation(context.program, operation);
if (httpOperation.verb !== "patch" || httpOperation.parameters.body === undefined) {
return;
}

for (const violation of findViolations(
context.program,
httpOperation.parameters.body.type,
operation,
)) {
context.reportDiagnostic({
target: violation.target,
format: {
propertyName: violation.propertyName,
},
});
}
},
};
},
});

type Violation = {
target: DiagnosticTarget;
propertyName: string;
};

function findViolations(program: Program, patchBody: Type, operation: Operation): Violation[] {
const patchModel = getModelType(patchBody);
if (patchModel === undefined) {
return [];
}

const metadataInfo = createMetadataInfo(program, {
canonicalVisibility: Visibility.Read,
canShareProperty: (property) => canSharePropertyUsingReadonlyOrXmsMutability(program, property),
Comment thread
msyyc marked this conversation as resolved.
});
const visibility = resolveRequestVisibility(program, operation, "patch");
const schemaVisibility = getSchemaVisibility(metadataInfo, patchModel, visibility);
const violations: Violation[] = [];

for (const { property, jsonName } of getModelProperties(program, patchModel)) {
if (!metadataInfo.isPayloadProperty(property, schemaVisibility) || isNeverType(property.type)) {
continue;
}

if (unsupportedPatchProperties.has(jsonName) && isWritableProperty(program, property)) {
violations.push({
target: getDiagnosticTarget(program, property, operation),
propertyName: jsonName,
});
}

if (jsonName === "properties") {
collectProvisioningStateViolation(
program,
property.type,
operation,
metadataInfo,
schemaVisibility,
violations,
);
}
}

return violations;
}

function collectProvisioningStateViolation(
program: Program,
type: Type,
operation: Operation,
metadataInfo: MetadataInfo,
visibility: Visibility,
violations: Violation[],
) {
const propertiesModel = getModelType(type);
if (propertiesModel === undefined) {
return;
}

const schemaVisibility = getSchemaVisibility(metadataInfo, propertiesModel, visibility);
for (const { property, jsonName } of getModelProperties(program, propertiesModel)) {
if (
jsonName === "provisioningState" &&
metadataInfo.isPayloadProperty(property, schemaVisibility) &&
!isNeverType(property.type) &&
isWritableProperty(program, property)
) {
violations.push({
target: getDiagnosticTarget(program, property, operation),
propertyName: `properties.${jsonName}`,
});
}
}
}

function getModelType(type: Type): Model | undefined {
if (type.kind === "Model") {
return type;
}
if (type.kind !== "Union") {
return undefined;
}

const nonNullVariants = [...type.variants.values()]
.map((variant) => variant.type)
.filter((variant) => !isNullType(variant));
return nonNullVariants.length === 1 && nonNullVariants[0].kind === "Model"
? nonNullVariants[0]
: undefined;
}

function getSchemaVisibility(
metadataInfo: MetadataInfo,
model: Model,
visibility: Visibility,
): Visibility {
return metadataInfo.isTransformed(model, visibility) ? visibility : Visibility.Read;
}

function isWritableProperty(program: Program, property: ModelProperty): boolean {
const lifecycle = getLifecycleVisibilityEnum(program);
const visibility = getVisibilityForClass(program, property, lifecycle);
const read = lifecycle.members.get("Read");
const update = lifecycle.members.get("Update");
if (read !== undefined && visibility.size === 1 && visibility.has(read)) {
return false;
}

const emittedMutability = [...visibility].filter((member) =>
["Read", "Create", "Update"].includes(member.name),
);
return (
visibility.size === lifecycle.members.size ||
emittedMutability.length === 0 ||
(update !== undefined && visibility.has(update))
);
}

function canSharePropertyUsingReadonlyOrXmsMutability(
program: Program,
property: ModelProperty,
): boolean {
const lifecycle = getLifecycleVisibilityEnum(program);
const visibility = getVisibilityForClass(program, property, lifecycle);
if (visibility.size === lifecycle.members.size) {
return true;
}
return (
visibility.size > 0 &&
[...visibility].every((member) => ["Read", "Create", "Update"].includes(member.name))
);
}

function getDiagnosticTarget(
program: Program,
property: ModelProperty,
operation: Operation,
): DiagnosticTarget {
return getLocationContext(program, property).type === "project" ? property : operation;
}

function getModelProperties(
program: Program,
model: Model,
): { property: ModelProperty; jsonName: string }[] {
const properties = new Map<string, ModelProperty>();

for (let current: Model | undefined = model; current !== undefined; current = current.baseModel) {
for (const property of current.properties.values()) {
const jsonName = resolveEncodedName(program, property, "application/json");
if (!properties.has(jsonName)) {
properties.set(jsonName, property);
}
}
}

return [...properties].map(([jsonName, property]) => ({ property, jsonName }));
}
Loading
Loading