-
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
[Hub Generated] Review request for Microsoft.KeyVault to update key permissions for 2021-06-01-preview. #16536
[Hub Generated] Review request for Microsoft.KeyVault to update key permissions for 2021-06-01-preview. #16536
Conversation
Swagger Validation Report
|
Rule | Message |
---|---|
R4013 - IntegerTypeMustHaveFormat |
The integer type does not have a format, please add it. Location: Microsoft.KeyVault/preview/2021-06-01-preview/secrets.json#L306 |
R4013 - IntegerTypeMustHaveFormat |
The integer type does not have a format, please add it. Location: Microsoft.KeyVault/preview/2021-06-01-preview/secrets.json#L312 |
R4013 - IntegerTypeMustHaveFormat |
The integer type does not have a format, please add it. Location: Microsoft.KeyVault/preview/2021-06-01-preview/secrets.json#L318 |
R4013 - IntegerTypeMustHaveFormat |
The integer type does not have a format, please add it. Location: Microsoft.KeyVault/preview/2021-06-01-preview/secrets.json#L324 |
R4037 - MissingTypeObject |
The schema 'CloudError' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/common.json#L10 |
R4037 - MissingTypeObject |
The schema 'CloudErrorBody' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/common.json#L19 |
R4037 - MissingTypeObject |
The schema 'SystemData' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/common.json#L33 |
R4037 - MissingTypeObject |
The schema 'PrivateLinkResourceProperties' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1947 |
R4037 - MissingTypeObject |
The schema 'PrivateLinkResource' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1932 |
R4037 - MissingTypeObject |
The schema 'PrivateLinkServiceConnectionState' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1865 |
R4037 - MissingTypeObject |
The schema 'PrivateEndpoint' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1855 |
R4037 - MissingTypeObject |
The schema 'PrivateEndpointConnectionProperties' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1838 |
R4037 - MissingTypeObject |
The schema 'PrivateEndpointConnection' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1802 |
R4037 - MissingTypeObject |
The schema 'Resource' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1627 |
R4037 - MissingTypeObject |
The schema 'CloudErrorBody' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/common.json#L19 |
R4037 - MissingTypeObject |
The schema 'CloudError' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/common.json#L10 |
R4037 - MissingTypeObject |
The schema 'Sku' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1021 |
R4037 - MissingTypeObject |
The schema 'AccessPolicyEntry' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1054 |
R4037 - MissingTypeObject |
The schema 'Permissions' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1082 |
R4037 - MissingTypeObject |
The schema 'VaultProperties' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1201 |
R4037 - MissingTypeObject |
The schema 'VaultPatchProperties' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1312 |
R4037 - MissingTypeObject |
The schema 'VaultAccessPolicyProperties' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1382 |
R4037 - MissingTypeObject |
The schema 'DeletedVaultProperties' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1397 |
R4037 - MissingTypeObject |
The schema 'VaultCreateOrUpdateParameters' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1437 |
R4037 - MissingTypeObject |
The schema 'VaultPatchParameters' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1462 |
R4037 - MissingTypeObject |
The schema 'VaultAccessPolicyParameters' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1479 |
R4037 - MissingTypeObject |
The schema 'Vault' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1512 |
R4037 - MissingTypeObject |
The schema 'DeletedVault' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1555 |
R4037 - MissingTypeObject |
The schema 'VaultListResult' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1579 |
R4037 - MissingTypeObject |
The schema 'DeletedVaultListResult' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.KeyVault/preview/2021-06-01-preview/keyvault.json#L1595 |
️️✔️
Avocado succeeded [Detail] [Expand]
Validation passes for Avocado.
️️✔️
~[Staging] ApiReadinessCheck succeeded [Detail] [Expand]
️️✔️
ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️️✔️
SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️️✔️
Cross-Version Breaking Changes succeeded [Detail] [Expand]
There are no breaking changes.
️️✔️
CredScan succeeded [Detail] [Expand]
There is no credential detected.
️️✔️
[Staging] SDK Track2 Validation succeeded [Detail] [Expand]
Validation passes for SDKTrack2Validation
- The following tags are being changed in this PR
️️✔️
[Staging] PrettierCheck succeeded [Detail] [Expand]
Validation passes for PrettierCheck.
️️✔️
[Staging] SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️️✔️
[Staging] Lint(RPaaS) succeeded [Detail] [Expand]
Validation passes for Lint(RPaaS).
Hi, @adarce 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] |
Swagger Generation Artifacts
|
[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. |
d148b04
to
7bf139c
Compare
7bf139c
to
b8512f7
Compare
@jianyexi could you help take a look at the breaking change failure, looks like unexpected and looks hard to undestand |
Approved, as it's caused by existing conflict among the swaggers in the folder: https://github.com/Azure/azure-rest-api-specs/tree/main/specification/keyvault/resource-manager/Microsoft.KeyVault/stable/2019-09-01 |
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.
Are those values actually supported with those api-versions, though? Anyone generating code could send those, and if not that would be a breaking change. It's generally not a good practice to backport changes. Was this to fix some actual problem?
@heaths I have similar concerns. We should not backport changes even for extensible enums. But is seems like that happen for soft-delete purge permissions - I don't think that was right thing to do. From Dev Experience that could be confusing when they point to specific version. |
Currently, the AKV service supports the same set of permissions across all API versions (and this PR was an attempt to fix this discrepancy). For context, back when AKV added permissions for purge/recover, the concern was potential data loss for customers using older clients. For example, consider a vault that is initially created with PurgeKey permission via the latest client, and then the customer happens to use an older client to add a GetKey permission. When parsing the GetVault response from the server, the older client would ignore the PurgeKey permission (since the older API version doesn't recognize the new value). After updating the list of key permissions in-memory, the older client would then submit a PutVault request to the server that would completely replace all permissions (so it would add GetKey, but unintentionally remove PurgeKey). One option here is to keep the ApiVersion-agnostic behaviour in the service (so that all API versions continue to support all permissions), but only update the latest API versions in this Swagger spec PR. Customers using SDKs are unlikely to notice the discrepancy (since by default SDKs only use the latest ApiVersion), but we could document the discrepancy for customers who are directly using REST APIs. @heaths and @jlichwa : What do you think? |
That scenario is still problematic if the client doesn't regenerate their client library, thus the service needs to handle that. This is also why all request/response enums should be marked See https://github.com/microsoft/api-guidelines/blob/vNext/azure/Guidelines.md#enums--sdks-client-libraries and https://github.com/microsoft/api-guidelines/blob/vNext/azure/Guidelines.md#use-extensible-enums |
b8512f7
to
33f8f27
Compare
Got it. I've updated my PR so that it only updates the most recent preview API version. |
@zhenglaizhang : Given that all concerns have been addressed, could you merge this PR please? |
Adding @lmazuel who can merge with (expected) validation errors. |
Never mind, @lmazuel. It wasn't a required check so I could. |
…Vault to update key permissions for 2021-06-01-preview. (#2038) Create to sync Azure/azure-rest-api-specs#16536 [ReCreate this PR](https://github.com/azure-resource-manager-schemas/compare/main...AzureSDKAutomation:sdkAuto/keyvault?expand=1)
This is a PR generated at OpenAPI Hub. You can view your work branch via this link.
Changelog
Add a changelog entry for this PR by answering the following questions:
What's the purpose of the update?
When are you targeting to deploy the new service/feature to public regions? Please provide the date or, if the date is not yet available, the month.
ETA: 2021-10-27
When do you expect to publish the swagger? Please provide date or, the the date is not yet available, the month.
End of October, or early November.
If updating an existing version, please select the specific langauge SDKs and CLIs that must be refreshed after the swagger is published.
Contribution checklist:
If any further question about AME onboarding or validation tools, please view the FAQ.
ARM API Review Checklist
Otherwise your PR may be subject to ARM review requirements. Complete the following:
Check this box if any of the following apply to the PR so that label “WaitForARMFeedback” will be added automatically to begin ARM API Review. Failure to comply may result in delays to the manifest.
-[ ] To review changes efficiently, ensure you copy the existing version into the new directory structure for first commit and then push new changes, including version updates, in separate commits.
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 any of the following scenarios apply to the PR, request approval from the 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.