Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LintDiff][False positive] ProvisioningStateMustBeReadOnly #6191

Open
konrad-jamrozik opened this issue May 17, 2023 · 5 comments
Open

[LintDiff][False positive] ProvisioningStateMustBeReadOnly #6191

konrad-jamrozik opened this issue May 17, 2023 · 5 comments
Assignees
Labels
ARM bug This issue requires a change to an existing behavior in the product in order to be resolved. Central-EngSys This issue is owned by the Engineering System team. Spec PR Tools Tooling that runs in azure-rest-api-specs repo.

Comments

@konrad-jamrozik
Copy link
Contributor

The rule ProvisioningStateMustBeReadOnly has been disabled by this PR:

Because @bdefoy was stating it is known to cause issues (May 4th, 3:51 PM PST, in the Teams chat)

We need to re-evaluate and re-enable this rule.

@konrad-jamrozik konrad-jamrozik added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. Central-EngSys This issue is owned by the Engineering System team. Spec PR Tools Tooling that runs in azure-rest-api-specs repo. labels May 17, 2023
@konrad-jamrozik konrad-jamrozik added bug This issue requires a change to an existing behavior in the product in order to be resolved. ARM NeedsARMTriage Issue needs triaging by the ARM team and removed needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels May 17, 2023
@rkmanda
Copy link
Member

rkmanda commented May 20, 2023

Task 23293566: [LintDiff][False positive] ProvisioningStateMustBeReadOnly #6191

@rkmanda rkmanda removed the NeedsARMTriage Issue needs triaging by the ARM team label May 20, 2023
@rkmanda
Copy link
Member

rkmanda commented Jun 1, 2023

After investigation this turned out not to be a false positive. It is accurately reporting that the provisioningState property is not marked as readOnly. The reason why the property is not being detected as readOnly is because the readOnly property is placed on the envelope property and not on the leaf definition.

Envelope - "provisioningState": {
"$ref": "#/definitions/ProvisioningState",
"description": "Provisioning State of the resource",
"readOnly": true
}

Leaf def
"ProvisioningState": {
"type": "string",
"enum": [
"Succeeded",
"Failed",
"Canceled"
],
"x-ms-enum": {
"name": "ProvisioningState",
"modelAsString": true
}
},

The recommendation here is to ask the author to update the swagger and mention the readOnly property inline with the leaf definition

@rkmanda rkmanda closed this as completed Jun 1, 2023
@rkmanda rkmanda reopened this Jun 1, 2023
@rkmanda
Copy link
Member

rkmanda commented Jun 1, 2023

This will need a change in the Typespec generation of swagger. I am working with Mark Cowlishaw on this issue. Once typespec fixes this issue, we can re-enable the linter rule again for this important check

@konrad-jamrozik
Copy link
Contributor Author

This will need a change in the Typespec generation of swagger. I am working with Mark Cowlishaw on this issue. Once typespec fixes this issue, we can re-enable the linter rule again for this important check

Referenced this comment from:

@cataggar
Copy link
Member

I think this can be closed. The solution was to add use-read-only-status-schema: true in options for @azure-tools/typespec-autorest in the tspconfig.yml. More details in #637.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ARM bug This issue requires a change to an existing behavior in the product in order to be resolved. Central-EngSys This issue is owned by the Engineering System team. Spec PR Tools Tooling that runs in azure-rest-api-specs repo.
Projects
Status: 🤔 Triage
Status: 🎊 Closed
Development

No branches or pull requests

4 participants