[ProvisioningStateMustBeReadOnly] Rule ignores siblings of $ref

[cataggar]

I am working on https://github.com/Azure/azure-rest-api-specs-pr/pull/15631 and there are many instances of ProvisioningStateMustBeReadOnly errors that are false positives.
https://github.com/Azure/azure-rest-api-specs-pr/pull/15631/checks?check_run_id=19650955197

All of the provisioningStates are marked as readOnly, for example:

        "provisioningState": {
          "$ref": "#/definitions/AddonProvisioningState",
          "description": "The state of the addon provisioning",
          "readOnly": true
        }

I see this test was considered flaky before in #561. cc @AkhilaIlla

[cataggar]

I saw this discussed in Azure/azure-sdk-tools#6191. The solution was to add use-read-only-status-schema: true to the tspconfig.yml:

options:
  "@azure-tools/typespec-autorest":
    azure-resource-provider-folder: "resource-manager"
    emitter-output-dir: "{project-root}/.."
    examples-directory: examples
    output-file: "{azure-resource-provider-folder}/{service-name}/{version-status}/{version}/vmware.json"
    omit-unreachable-types: true
    use-read-only-status-schema: true

[mikeharder]

This should be fixed in coordination wtih Azure/oav#848.

Overview

The rule is a "false positive" from the perspective of autorest, but it's correct from the perspective of OpenAPI 2.0. Siblings of $ref are allowed in the former, but ignored in the latter (#706).

Spectral

ProvisioningStateMustBeReadOnly is a "spectral" rule:

azure-openapi-validator/packages/rulesets/src/spectral/az-arm.ts

Lines 208 to 221 in d372289

	ProvisioningStateMustBeReadOnly: {
	rpcGuidelineCode: "RPC-Async-V1-16",
	description: "This is a rule introduced to validate if provisioningState property is set to readOnly or not.",
	message: "{{error}}",
	severity: "error",
	disableForTypeSpec: false,
	disableForTypeSpecReason: "Covered by TSP's '@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state' rule.",
	resolved: true,
	formats: [oas2],
	given: ["$[paths,'x-ms-paths'].*.*.responses.*.schema"],
	then: {
	function: provisioningStateMustBeReadOnly,
	},
	},

It's probably ignoring the readonly property, because the OpenAPI 2.0 spec says it should be ignored. It might be possible to configure spectral allow siblings of $ref, but I'm not sure if or how yet, other than converting the autorest to OpenAPI 3.1 which allows siblings of $ref.

Relevant unit test from spectral:

https://github.com/stoplightio/spectral/blob/9e906eaa0e792d229e1fd553f018ab8e6afef12e/packages/rulesets/src/oas/__tests__/no-%24ref-siblings.test.ts

Convert to Native

Otherwise, this rule could be converted from spectral to native, which should be able to allow siblings of $ref.

Documentation

Until fixed, we should improve the documentation:

1. Add a bullet to https://github.com/Azure/azure-rest-api-specs/blob/main/documentation/ci-fix.md#swagger-lintdiff-for-typespec-troubleshooting-guides
2. Add a suggestion to add use-read-only-status-schema: true to the rule error message.

[mikeharder]

@markcowl: How would you characterize setting use-read-only-status-schema: true in tspconfig.yaml? Is this a "workaround" for the bugs in azure-openapi-validator and oav? Or should specs configure this for other reasons, and the errors are just a good reminder?

Put another way, if the bugs in azure-openapi-validator and oav are fixed, is TypeSpec setting use-read-only-status-schema still useful? Ignoring our tooling, how does this setting impact the spec, the SDKs, or customers?