Supporting User MSI in CMK #14048
Supporting User MSI in CMK #14048
Conversation
|
Hi, @asmaskar 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. [email protected] |
|
[Call for Action] To better understand Azure service dev/test scenario, and support Azure service developer better on Swagger and REST API related tests in early phase, please help to fill in with this survey https://aka.ms/SurveyForEarlyPhase. It will take 5 to 10 minutes. If you already complete survey, please neglect this comment. Thanks. |
Swagger Validation Report
|
| Rule | Message |
|---|---|
| Consider using x-ms-client-flatten to provide a better end user experience Location: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L4002 |
|
| Booleans are not descriptive and make them hard to use. Consider using string enums with allowed set of values defined. Property: useSystemAssignedIdentity Location: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L3989 |
|
| 'BackupResourceEncryptionConfigExtended' model/property lacks 'description' and 'title' property. Consider adding a 'description'/'title' element. Accurate description/title is essential for maintaining reference documentation. Location: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L3977 |
|
| 'BackupResourceEncryptionConfigExtendedResource' model/property lacks 'description' and 'title' property. Consider adding a 'description'/'title' element. Accurate description/title is essential for maintaining reference documentation. Location: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L3995 |
The following errors/warnings exist before current PR submission:
Only 10 items are listed, please refer to log for more details.
| Rule | Message |
|---|---|
|
Resource type naming must follow camel case. Path: '/Subscriptions/{subscriptionId}/providers/Microsoft.RecoveryServices/locations/{azureRegion}/backupPreValidateProtection' Location: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L21 |
|
Resource type naming must follow camel case. Path: '/Subscriptions/{subscriptionId}/providers/Microsoft.RecoveryServices/locations/{azureRegion}/backupStatus' Location: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L66 |
|
Resource type naming must follow camel case. Path: '/Subscriptions/{subscriptionId}/providers/Microsoft.RecoveryServices/locations/{azureRegion}/backupValidateFeatures' Location: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L111 |
|
Resource type naming must follow camel case. Path: '/Subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.RecoveryServices/vaults/{vaultName}/backupFabrics/{fabricName}/backupProtectionIntent/{intentObjectName}' Location: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L156 |
|
Resource type naming must follow camel case. Path: '/Subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.RecoveryServices/vaults/{vaultName}/backupProtectionIntents' Location: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L317 |
|
Resource type naming must follow camel case. Path: '/Subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.RecoveryServices/vaults/{vaultName}/backupUsageSummaries' Location: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L374 |
|
The response is defined but without a default error response implementation.Consider adding it.' Location: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L51 |
|
The response is defined but without a default error response implementation.Consider adding it.' Location: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L96 |
|
The response is defined but without a default error response implementation.Consider adding it.' Location: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L141 |
|
The response is defined but without a default error response implementation.Consider adding it.' Location: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L194 |
️⚠️Avocado: 1 Warnings warning [Detail]
| Rule | Message |
|---|---|
| The default tag contains multiple API versions swaggers. readme: specification/recoveryservicesbackup/resource-manager/readme.md tag: specification/recoveryservicesbackup/resource-manager/readme.md#tag-package-2021-03 |
️️✔️ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️️✔️SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️❌Cross Version BreakingChange (Base on preview version): 6 Errors, 1 Warnings failed [Detail]
- Compared Swaggers (Based on Oad v0.8.9)
- original: preview/2021-02-01-preview/bms.json <---> new: stable/2021-04-01/bms.json
| Rule | Message |
|---|---|
|
The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.RecoveryServices/vaults/{vaultName}/backupResourceGuardProxies' removed or restructured? Old: Microsoft.RecoveryServices/preview/2021-02-01-preview/bms.json#L3920:5 |
|
The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.RecoveryServices/vaults/{vaultName}/backupResourceGuardProxies/{resourceGuardProxyName}' removed or restructured? Old: Microsoft.RecoveryServices/preview/2021-02-01-preview/bms.json#L3968:5 |
|
The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.RecoveryServices/vaults/{vaultName}/backupResourceGuardProxies/{resourceGuardProxyName}/unlockDelete' removed or restructured? Old: Microsoft.RecoveryServices/preview/2021-02-01-preview/bms.json#L4117:5 |
|
The new version is missing a definition that was found in the old version. Was 'UnlockDeleteRequest' removed or renamed? New: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L3921:3 Old: Microsoft.RecoveryServices/preview/2021-02-01-preview/bms.json#L4178:3 |
|
The new version is missing a property found in the old version. Was 'resourceGuardOperationRequests' renamed or removed? New: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L8286:7 Old: Microsoft.RecoveryServices/preview/2021-02-01-preview/bms.json#L8506:7 |
|
The new version has a less constraining 'enum' value than the previous one. New: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L3946:9 Old: Microsoft.RecoveryServices/preview/2021-02-01-preview/bms.json#L4203:9 |
| The new version has a different 'allOf' property than the previous one. New: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L4002:9 Old: Microsoft.RecoveryServices/preview/2021-02-01-preview/bms.json#L4239:9 |
️❌Cross Version BreakingChange (Base on stable version): 1 Errors, 1 Warnings failed [Detail]
- Compared Swaggers (Based on Oad v0.8.9)
- original: stable/2021-03-01/bms.json <---> new: stable/2021-04-01/bms.json
| Rule | Message |
|---|---|
|
The new version has a less constraining 'enum' value than the previous one. New: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L3946:9 Old: Microsoft.RecoveryServices/stable/2021-03-01/bms.json#L3946:9 |
| The new version has a different 'allOf' property than the previous one. New: Microsoft.RecoveryServices/stable/2021-04-01/bms.json#L4002:9 Old: Microsoft.RecoveryServices/stable/2021-03-01/bms.json#L3982:9 |
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️❌[Staging] SDK Track2 Validation: 13 Errors, 0 Warnings failed [Detail]
- The following tags are being changed in this PR
Only 10 items are listed, please refer to log for more details.
| Rule | Message |
|---|---|
|
"readme":"recoveryservicesbackup/resource-manager/readme.md", "tag":"package-2021-04", "details":"Duplicate Schema named NewErrorResponse -- properties.error.$ref: undefined => "#/components/schemas/schemas:1069" ; This error can be temporarily avoided by using the 'modelerfour.lenient-model-deduplication' setting. NOTE: This setting will be removed in a future version of @autorest/modelerfour; schemas should be updated to fix this issue sooner than that." |
|
"readme":"recoveryservicesbackup/resource-manager/readme.md", "tag":"package-2021-04", "details":"Duplicate Schema named NewErrorResponse-error -- properties.details.$ref: undefined => "#/components/schemas/schemas:1073" ; This error can be temporarily avoided by using the 'modelerfour.lenient-model-deduplication' setting. NOTE: This setting will be removed in a future version of @autorest/modelerfour; schemas should be updated to fix this issue sooner than that." |
|
"readme":"recoveryservicesbackup/resource-manager/readme.md", "tag":"package-2021-04", "details":"Duplicate Schema named AzureFileshareProtectedItem -- properties.healthStatus: undefined => "$ref":"#/components/schemas/HealthStatus", "description":"backups running status for this backup item." ; This error can be temporarily avoided by using the 'modelerfour.lenient-model-deduplication' setting. NOTE: This setting will be removed in a future version of @autorest/modelerfour; schemas should be updated to fix this issue sooner than that." |
|
"readme":"recoveryservicesbackup/resource-manager/readme.md", "tag":"package-2021-04", "details":"Duplicate Schema named AzureFileShareRecoveryPoint -- properties.recoveryPointType.$ref: undefined => "#/components/schemas/schemas:1097", properties.recoveryPointType.readOnly: undefined => true, properties.recoveryPointTime.$ref: undefined => "#/components/schemas/schemas:1098", properties.recoveryPointTime.readOnly: undefined => true, properties.fileShareSnapshotUri.$ref: undefined => "#/components/schemas/schemas:1099", properties.fileShareSnapshotUri.readOnly: undefined => true, properties.recoveryPointSizeInGB.$ref: undefined => "#/components/schemas/schemas:1100", properties.recoveryPointSizeInGB.readOnly: undefined => true ; This error can be temporarily avoided by using the 'modelerfour.lenient-model-deduplication' setting. NOTE: This setting will be removed in a future version of @autorest/modelerfour; schemas should be updated to fix this issue sooner than that." |
|
"readme":"recoveryservicesbackup/resource-manager/readme.md", "tag":"package-2021-04", "details":"Duplicate Schema named AzureWorkloadPointInTimeRecoveryPoint -- allOf.0.$ref: undefined => "#/components/schemas/schemas:1234" ; This error can be temporarily avoided by using the 'modelerfour.lenient-model-deduplication' setting. NOTE: This setting will be removed in a future version of @autorest/modelerfour; schemas should be updated to fix this issue sooner than that." |
|
"readme":"recoveryservicesbackup/resource-manager/readme.md", "tag":"package-2021-04", "details":"Duplicate Schema named AzureWorkloadRecoveryPoint -- properties.recoveryPointTimeInUTC.$ref: undefined => "#/components/schemas/schemas:1235", properties.recoveryPointTimeInUTC.readOnly: undefined => true, properties.type.readOnly: undefined => true ; This error can be temporarily avoided by using the 'modelerfour.lenient-model-deduplication' setting. NOTE: This setting will be removed in a future version of @autorest/modelerfour; schemas should be updated to fix this issue sooner than that." |
|
"readme":"recoveryservicesbackup/resource-manager/readme.md", "tag":"package-2021-04", "details":"Duplicate Schema named AzureWorkloadSAPHanaPointInTimeRecoveryPoint -- allOf.0.$ref: undefined => "#/components/schemas/schemas:1230" ; This error can be temporarily avoided by using the 'modelerfour.lenient-model-deduplication' setting. NOTE: This setting will be removed in a future version of @autorest/modelerfour; schemas should be updated to fix this issue sooner than that." |
|
"readme":"recoveryservicesbackup/resource-manager/readme.md", "tag":"package-2021-04", "details":"Duplicate Schema named AzureWorkloadSAPHanaRecoveryPoint -- allOf.0.$ref: undefined => "#/components/schemas/schemas:1234" ; This error can be temporarily avoided by using the 'modelerfour.lenient-model-deduplication' setting. NOTE: This setting will be removed in a future version of @autorest/modelerfour; schemas should be updated to fix this issue sooner than that." |
|
"readme":"recoveryservicesbackup/resource-manager/readme.md", "tag":"package-2021-04", "details":"Duplicate Schema named AzureWorkloadSQLPointInTimeRecoveryPoint -- allOf.0.$ref: undefined => "#/components/schemas/schemas:1255" ; This error can be temporarily avoided by using the 'modelerfour.lenient-model-deduplication' setting. NOTE: This setting will be removed in a future version of @autorest/modelerfour; schemas should be updated to fix this issue sooner than that." |
|
"readme":"recoveryservicesbackup/resource-manager/readme.md", "tag":"package-2021-04", "details":"Duplicate Schema named AzureWorkloadSQLRecoveryPoint -- allOf.0.$ref: undefined => "#/components/schemas/schemas:1234", properties.extendedInfo.$ref: undefined => "#/components/schemas/schemas:1256" ; This error can be temporarily avoided by using the 'modelerfour.lenient-model-deduplication' setting. NOTE: This setting will be removed in a future version of @autorest/modelerfour; schemas should be updated to fix this issue sooner than that." |
️️✔️[Staging] PrettierCheck succeeded [Detail] [Expand]
Validation passes for PrettierCheck.
️️✔️[Staging] SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
Swagger Generation Artifacts
|
|
Hi, @asmaskar your PR are labelled with WaitForARMFeedback. A notification email will be sent out shortly afterwards to notify ARM review board([email protected]). cc @chunyu3 |
openapi-workflow-bot
bot
added
the
WaitForARMFeedback
PUT should return the original request at least. Please see the RPC: https://github.com/Azure/azure-resource-manager-rpc/blob/master/v1.0/resource-api-reference.md#put-resource Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2021-04-01/bms.json:692 in 129ac3e. [](commit_id = 129ac3e, deletion_comment = False) |
Why not actually extending BackupResourceEnruptionConfig using 'allof'? In reply to: 824378108 In reply to: 824378108 Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2021-04-01/bms.json:3977 in 129ac3e. [](commit_id = 129ac3e, deletion_comment = False) |
| "vaultName": "source-rsv", | ||
| "api-version": "2021-01-01", | ||
| "parameters": { | ||
| "properties": { |
There was a problem hiding this comment.
Made a fix. User is not supposed to set the fields using this API. The information will only be received as a part of GET call
j5lim
added
ARMChangesRequested
|
Hi @j5lim , I have made a fix in the PR. I made a mistake by changing the Put Request. The Put request is supposed to be the same as in customer is not supposed to set userAssignedIdentity using this API. Although in GET he will receive those information as it is modified using some other API. |
|
with respect to the other comments, the PUT call response was checked in may be more than an year ago, changing that, wont it be a breaking change? as SDK is released , is it ideal to make the change there? Regarding the other comment of using allof, i generated this swagger using a tool. I have made the changes. Please have a check if thats what you asked for. |
|
Right, the service code need to be fixed before updating the swagger. Can you please create a WI for that so that it can be fixed in the next release? In reply to: 824402975 |
|
@asmaskar There are cross version breaking changes, and it need to acquire breaking change approve. |
@chunyu3 We are making fix for the issue "Duplicate Schema named NewErrorResponse -- properties.error.$ref: undefined => "#/components/schemas/schemas:1024"". Seems like the NewErrorResponse schema in our spec and the Error Response under common json have same definition so we are making changes to use the ErrorResponse from common schema as itself. Regarding the Duplicated schema issues, I suspect it is due to the fact that in our package, we have a version 2021-03-01 that contains definitions of all APIs(all these APIs are being routed to active stamp), and another version 2018-12-20 in the same package which contains APIs which are Cross Region restore specific which need to be routed to passive stamp based on the API version, So it is expected that the same API definitions are present in both these version hence the validation issue. Since this is expected for other API definitions apart from NewErrorResponse, is there a provision to ignore this issue for the other APIs? |
|
Hi @asmaskar, Your PR has some issues. Please fix the CI sequentially by following the order of
|
|
Hi @chunyu3 , the SDK Track2 validation issue is not in our control, as in its related to some other issue. And as far as the breaking change is concerned the error there is shown due to weaker enum constraint but we havent removed any values from the enum. We have added values. Can you please let us know if there are any other issues in it |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Hi @asmaskar For the enum breaking change, yes, it may not be the issue of your PR, you can ignore this. I will confirm with the breakingchange detect owner about it. If there still issue, I will contact you. |
JeffreyRichter
added
the
Approved-BreakingChange
* Added New Api-Version 2021-04-01 * Changes for Supporting User MSI in CMK * Fixing Prettier check * Fix for Put BackupEncryptionConfig * Resolving the comments provided. * Fixing duplicate schema error * Reverting changes to error response
MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.
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.