-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Adding resource property to decision model in access review APIs #13026
Conversation
Hi, @shubhamguptacal Thanks for your PR. I am workflow bot for review process. Here are some small tips. Any feedback about review process or workflow bot, pls contact swagger and tools team. vsswagger@microsoft.com |
Swagger Validation Report
|
Rule | Message |
---|---|
'PUT' operation 'AccessReviewDefaultSettings_Put' should use method name 'Create'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Authorization/preview/2021-03-01-preview/authorization-AccessReviewCalls.json#L655 |
|
'PATCH' operation 'AccessReviewInstanceMyDecisions_Patch' should use method name 'Update'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Authorization/preview/2021-03-01-preview/authorization-AccessReviewCalls.json#L972 |
|
A PUT operation request body schema should be the same as its 200 response schema, to allow reusing the same entity between GET and PUT. If the schema of the PUT request body is a superset of the GET response body, make sure you have a PATCH operation to make the resource updatable. Operation: 'AccessReviewScheduleDefinitions_CreateOrUpdateById' Request Model: 'AccessReviewScheduleDefinitionProperties' Response Model: 'AccessReviewScheduleDefinition' Location: Microsoft.Authorization/preview/2021-03-01-preview/authorization-AccessReviewCalls.json#L188 |
|
A PUT operation request body schema should be the same as its 200 response schema, to allow reusing the same entity between GET and PUT. If the schema of the PUT request body is a superset of the GET response body, make sure you have a PATCH operation to make the resource updatable. Operation: 'AccessReviewDefaultSettings_Put' Request Model: 'AccessReviewScheduleSettings' Response Model: 'AccessReviewDefaultSettings' Location: Microsoft.Authorization/preview/2021-03-01-preview/authorization-AccessReviewCalls.json#L651 |
|
OperationId has a noun that conflicts with one of the model names in definitions section. The model name will be disambiguated to 'AccessReviewInstanceModel'. Consider using the plural form of 'AccessReviewInstance' to avoid this. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Authorization/preview/2021-03-01-preview/authorization-AccessReviewCalls.json#L380 |
|
OperationId has a noun that conflicts with one of the model names in definitions section. The model name will be disambiguated to 'AccessReviewInstanceModel'. Consider using the plural form of 'AccessReviewInstance' to avoid this. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Authorization/preview/2021-03-01-preview/authorization-AccessReviewCalls.json#L427 |
|
OperationId has a noun that conflicts with one of the model names in definitions section. The model name will be disambiguated to 'AccessReviewInstanceModel'. Consider using the plural form of 'AccessReviewInstance' to avoid this. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Authorization/preview/2021-03-01-preview/authorization-AccessReviewCalls.json#L474 |
|
OperationId has a noun that conflicts with one of the model names in definitions section. The model name will be disambiguated to 'AccessReviewInstanceModel'. Consider using the plural form of 'AccessReviewInstance' to avoid this. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Authorization/preview/2021-03-01-preview/authorization-AccessReviewCalls.json#L521 |
|
OperationId has a noun that conflicts with one of the model names in definitions section. The model name will be disambiguated to 'AccessReviewDefaultSettingsModel'. Consider using the plural form of 'AccessReviewDefaultSettings' to avoid this. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Authorization/preview/2021-03-01-preview/authorization-AccessReviewCalls.json#L621 |
|
OperationId has a noun that conflicts with one of the model names in definitions section. The model name will be disambiguated to 'AccessReviewDefaultSettingsModel'. Consider using the plural form of 'AccessReviewDefaultSettings' to avoid this. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Authorization/preview/2021-03-01-preview/authorization-AccessReviewCalls.json#L655 |
️️✔️
Avocado succeeded [Detail] [Expand]
Validation passes for Avocado.
️️✔️
ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️️✔️
SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️⚠️
[Staging] Cross Version BreakingChange (Base on preview version): 2 Warnings warning [Detail]
- Compared Swaggers (Based on Oad v0.8.6)
Rule | Message |
---|---|
The new version is missing a definition that was found in the old version. Was 'AccessReviewDecisionTarget' removed or renamed? New: Microsoft.Authorization/preview/2021-03-01-preview/authorization-AccessReviewCalls.json#L1031:3 Old: Microsoft.Authorization/preview/2018-05-01-preview/authorization-AccessReviewCalls.json#L1031:3 |
|
The new version is missing a property found in the old version. Was 'target' renamed or removed? New: Microsoft.Authorization/preview/2021-03-01-preview/authorization-AccessReviewCalls.json#L1442:7 Old: Microsoft.Authorization/preview/2018-05-01-preview/authorization-AccessReviewCalls.json#L1434:7 |
️️✔️
[Staging] Cross Version BreakingChange (Base on stable version) succeeded [Detail] [Expand]
There are no breaking changes.
️️✔️
CredScan succeeded [Detail] [Expand]
There is no credential detected.
️⚠️
[Staging] SDK Track2 Validation: 400 Warnings warning [Detail]
Only 10 items are listed, please refer to log for more details.
- The following tags are being changed in this PR
- authorization/resource-manager/readme.md#package-2020-10-01-preview
- authorization/resource-manager/readme.md#package-2020-04-01-preview
- authorization/resource-manager/readme.md#package-2020-04-01-preview-only
- authorization/resource-manager/readme.md#package-2020-03-01-preview
- authorization/resource-manager/readme.md#package-2018-09-01-preview
- authorization/resource-manager/readme.md#package-2018-09-01-preview-only
- authorization/resource-manager/readme.md#package-2018-07-01-preview
- authorization/resource-manager/readme.md#package-2018-07-01-preview-only
- authorization/resource-manager/readme.md#package-2018-05-01-preview
- authorization/resource-manager/readme.md#package-2018-01-01-preview
- authorization/resource-manager/readme.md#package-2018-01-01-preview-only
- authorization/resource-manager/readme.md#package-2017-10-01-preview
- authorization/resource-manager/readme.md#package-2017-10-01-preview-only
- authorization/resource-manager/readme.md#package-2015-07-01
- authorization/resource-manager/readme.md#package-2015-06-01-preview
- authorization/resource-manager/readme.md#profile-hybrid-2020-09-01
- authorization/resource-manager/readme.md#profile-hybrid-2019-03-01
- authorization/resource-manager/readme.md#package-2021-01-01-preview-only
- authorization/resource-manager/readme.md#package-2021-03-01-preview-only
Rule | Message |
---|---|
"readme":"authorization/resource-manager/readme.md", "tag":"package-2020-10-01-preview", "details":"The schema 'RoleAssignmentScheduleFilter' with an undefined type and decalared properties is a bit ambigious. This has been auto-corrected to 'type:object'" |
|
"readme":"authorization/resource-manager/readme.md", "tag":"package-2020-10-01-preview", "details":"The schema 'RoleAssignmentScheduleProperties' with an undefined type and decalared properties is a bit ambigious. This has been auto-corrected to 'type:object'" |
|
"readme":"authorization/resource-manager/readme.md", "tag":"package-2020-10-01-preview", "details":"The schema 'RoleAssignmentSchedule' with an undefined type and decalared properties is a bit ambigious. This has been auto-corrected to 'type:object'" |
|
"readme":"authorization/resource-manager/readme.md", "tag":"package-2020-10-01-preview", "details":"The schema 'RoleAssignmentScheduleListResult' with an undefined type and decalared properties is a bit ambigious. This has been auto-corrected to 'type:object'" |
|
"readme":"authorization/resource-manager/readme.md", "tag":"package-2020-10-01-preview", "details":"The schema 'CloudError' with an undefined type and decalared properties is a bit ambigious. This has been auto-corrected to 'type:object'" |
|
"readme":"authorization/resource-manager/readme.md", "tag":"package-2020-10-01-preview", "details":"The schema 'CloudErrorBody' with an undefined type and decalared properties is a bit ambigious. This has been auto-corrected to 'type:object'" |
|
"readme":"authorization/resource-manager/readme.md", "tag":"package-2020-10-01-preview", "details":"The schema 'RoleAssignmentScheduleInstanceFilter' with an undefined type and decalared properties is a bit ambigious. This has been auto-corrected to 'type:object'" |
|
"readme":"authorization/resource-manager/readme.md", "tag":"package-2020-10-01-preview", "details":"The schema 'RoleAssignmentScheduleInstanceProperties' with an undefined type and decalared properties is a bit ambigious. This has been auto-corrected to 'type:object'" |
|
"readme":"authorization/resource-manager/readme.md", "tag":"package-2020-10-01-preview", "details":"The schema 'RoleAssignmentScheduleInstance' with an undefined type and decalared properties is a bit ambigious. This has been auto-corrected to 'type:object'" |
|
"readme":"authorization/resource-manager/readme.md", "tag":"package-2020-10-01-preview", "details":"The schema 'RoleAssignmentScheduleInstanceListResult' with an undefined type and decalared properties is a bit ambigious. This has been auto-corrected to 'type:object'" |
️️✔️
[Staging] PrettierCheck succeeded [Detail] [Expand]
Validation passes for PrettierCheck.
️️✔️
[Staging] SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
Swagger Generation Artifacts
|
Hi @shubhamguptacal, Your PR has some issues. Please fix the CI sequentially by following the order of
|
0a0d038
to
cb16667
Compare
cb16667
to
f956df9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed off from ARM side.
f956df9
to
0fec5e4
Compare
0fec5e4
to
466025b
Compare
…re#13026) * Cloning version 2021-03-01-preview from version 2018-05-01-preview * Added resource property to decision model
Context
Today, we have existing APIs exposed for access reviews with version 2018-05-01-preview. We now want to make enhancements to these APIs to allow support of new scenarios. We're doing so by introducing a new version 2021-03-01-preview which is cloned from 2018-05-01-preview. There are two commits in this PR - the first commit is just a clone of the existing version and the second commit includes changes in model between these two versions.
In access reviews, today, most of the scenarios revolve around reviewing access of a user to a single role. We're working on new scenarios where one can review all roles in a single review. We also want to be able to more scenarios in the future where we can review combination of "principal" and "resources". To do so, we're augmenting the decision model to allow support for "resource" property in the decision model. In addition to this change, we're also renaming "target" to "principal" to clarify the intent here.
The following illustration helps showcase the Before and After changes in model :
Changelog
Please ensure to add changelog with this PR by answering the following questions.
Contribution checklist:
If any further question about AME onboarding or validation tools, please view the FAQ.
ARM API Review Checklist
Ensure to check this box if one of the following scenarios meet updates in the PR, so that label “WaitForARMFeedback” will be added automatically to involve ARM API Review. Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs, all “removals” and “adding a new property” no more require ARM API review.
Please ensure you've reviewed following guidelines including ARM resource provider contract and REST guidelines. Estimated time (4 hours). This is required before you can request review from ARM API Review board.
If you are blocked on ARM review and want to get the PR merged with urgency, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
Breaking Change Review Checklist
If there are following updates in the PR, ensure to request an approval from Breaking Change Review Board as defined in the Breaking Change Policy.
Action: to initiate an evaluation of the breaking change, create a new intake using the template for breaking changes. Addition details on the process and office hours are on the Breaking change Wiki.
Please follow the link to find more details on PR review process.