Release redis microsoft.cache 2023-05-01 preview #23406
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… to version 2023-05-01
* Add examples for access policy resource apis * Add examples for access policy assignment APIs * Fix formatting * Add access policy path to swagger specification * Add example for Access policy list api * Add path for get access policies of a cache * Add access policy assignment path * Add aad enabled property to redis configuration * Push prettier fixes * Semantic validation fix * FIx response in examples * Add location header to delete responses * Add proxy resource to get responses * Rewrite access policy definitions * Fix definition name * Fix access policy definition references * Add schema for access policy delete operation * Remove request body from access policy assignment delete operation * Rewrite access policy assignment definitions * Fix access policy assignment example * Add provisioning state and type enums * Add cache name regex * Update new response codes and access policy pattern matching * Update provisioning state enum with terminal states * Add object types to resources * Add identifier to access policy assignments * Add object type to assignments * Add location header to 202 responses * Add access policy type enum options * Add long running operation options to PUT and DELETE * Add schema for PUT 200 * Rename username property to name * Add access policy list operation * Minor fix * Enum rename * Enum removal * Add enum back * Use single definition for access policy request and response * Use single definition for access policy assignment request and response * Use common cache name parameter * Remove cache name parameter pattern * Remove common cache parameter for new APIs, use common types API version for all operations * Prettier cleanup * Add common type subscription id * Use common types vs everywhere * Combine access policyassignments, properties and requestresponse def * Combine access policyand access policy properties * Make permissions only field for access policies * Rename access policy assignment name property to objectIdAlias and update description * Add type object to access policy assignment request response * Remove obj type * Remove property from access policy assignment definition * Test * Update access policy assignment description * Update aad description * Add method extension and pattern to global cacheNameParameter * Rename access policy list definition * Rename access policy assignment definitions * Rename permissions desc * Rename permissions desc * Update permissions desc * Update permissions desc * Update with suggestions * Update Put response schema description * Remove global params update linked server operations --------- Co-authored-by: Kanchan Joshi <kanchanjoshi@microsoft.com>
|
Hi, @koderjoker 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. vscswagger@microsoft.com |
ghost
added
the
Swagger Validation Report
|
| compared swaggers (via Oad v0.10.4)] | new version | base version |
|---|---|---|
| redis.json | 2023-05-01(036139d) | 2022-06-01(main) |
| redis.json | 2023-05-01(036139d) | 2019-07-01(main) |
The following breaking changes are detected by comparison with the latest stable version:
The following breaking changes are detected by comparison with the latest preview version:
| Rule | Message |
|---|---|
| The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Cache/Redis/{name}/listUpgradeNotifications' removed or restructured? Old: Microsoft.Cache/preview/2019-07-01/redis.json#L103:5 |
|
| The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Cache/Redis/{name}' removed or restructured? Old: Microsoft.Cache/preview/2019-07-01/redis.json#L155:5 |
|
| The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Cache/Redis' removed or restructured? Old: Microsoft.Cache/preview/2019-07-01/redis.json#L354:5 |
|
| The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/providers/Microsoft.Cache/Redis' removed or restructured? Old: Microsoft.Cache/preview/2019-07-01/redis.json#L394:5 |
|
| The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Cache/Redis/{name}/listKeys' removed or restructured? Old: Microsoft.Cache/preview/2019-07-01/redis.json#L427:5 |
|
| The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Cache/Redis/{name}/regenerateKey' removed or restructured? Old: Microsoft.Cache/preview/2019-07-01/redis.json#L471:5 |
|
| The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Cache/Redis/{name}/forceReboot' removed or restructured? Old: Microsoft.Cache/preview/2019-07-01/redis.json#L524:5 |
|
| The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Cache/Redis/{name}/import' removed or restructured? Old: Microsoft.Cache/preview/2019-07-01/redis.json#L577:5 |
|
| The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Cache/Redis/{name}/export' removed or restructured? Old: Microsoft.Cache/preview/2019-07-01/redis.json#L634:5 |
|
| The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Cache/Redis/{cacheName}/firewallRules' removed or restructured? Old: Microsoft.Cache/preview/2019-07-01/redis.json#L691:5 |
|
| The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Cache/Redis/{cacheName}/firewallRules/{ruleName}' removed or restructured? Old: Microsoft.Cache/preview/2019-07-01/redis.json#L739:5 |
|
| The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Cache/Redis/{cacheName}/patchSchedules' removed or restructured? Old: Microsoft.Cache/preview/2019-07-01/redis.json#L906:5 |
|
| The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Cache/Redis/{name}/patchSchedules/{default}' removed or restructured? Old: Microsoft.Cache/preview/2019-07-01/redis.json#L954:5 |
|
| The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Cache/Redis/{name}/linkedServers/{linkedServerName}' removed or restructured? Old: Microsoft.Cache/preview/2019-07-01/redis.json#L1142:5 |
|
| The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Cache/Redis/{name}/linkedServers' removed or restructured? Old: Microsoft.Cache/preview/2019-07-01/redis.json#L1304:5 |
|
| The new version adds a response code 'default'. New: Microsoft.Cache/preview/2023-05-01/redis.json#L61:11 |
|
| The new version adds a response code 'default'. New: Microsoft.Cache/preview/2023-05-01/redis.json#L106:11 |
|
| The new version has a different type '' than the previous one 'string'. New: Microsoft.Cache/preview/2023-05-01/redis.json#L2278:11 Old: Microsoft.Cache/preview/2019-07-01/redis.json#L1397:11 |
|
| The new version has a different type '' than the previous one 'string'. New: Microsoft.Cache/preview/2023-05-01/redis.json#L2278:11 Old: Microsoft.Cache/preview/2019-07-01/redis.json#L1397:11 |
|
| The new version has a different type '' than the previous one 'string'. New: Microsoft.Cache/preview/2023-05-01/redis.json#L2278:11 Old: Microsoft.Cache/preview/2019-07-01/redis.json#L1397:11 |
|
| The new version has a different type '' than the previous one 'string'. New: Microsoft.Cache/preview/2023-05-01/redis.json#L2278:11 Old: Microsoft.Cache/preview/2019-07-01/redis.json#L1397:11 |
|
| The new version has a different default value than the previous one. New: Microsoft.Cache/preview/2023-05-01/redis.json#L2285:9 Old: Microsoft.Cache/preview/2019-07-01/redis.json#L1402:9 |
|
| The new version has a different default value than the previous one. New: Microsoft.Cache/preview/2023-05-01/redis.json#L2285:9 Old: Microsoft.Cache/preview/2019-07-01/redis.json#L1402:9 |
|
| The new version has a different default value than the previous one. New: Microsoft.Cache/preview/2023-05-01/redis.json#L2285:9 Old: Microsoft.Cache/preview/2019-07-01/redis.json#L1402:9 |
|
| The new version has a different default value than the previous one. New: Microsoft.Cache/preview/2023-05-01/redis.json#L2285:9 Old: Microsoft.Cache/preview/2019-07-01/redis.json#L1402:9 |
|
| The read only property has changed from 'true' to 'false'. New: Microsoft.Cache/preview/2023-05-01/redis.json#L2281:9 Old: Microsoft.Cache/preview/2019-07-01/redis.json#L1485:9 |
|
| The new version has a different 'allOf' property than the previous one. New: Microsoft.Cache/preview/2023-05-01/redis.json#L2566:5 Old: Microsoft.Cache/preview/2019-07-01/redis.json#L1708:5 |
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️❌LintDiff: 2 Errors, 6 Warnings failed [Detail]
| compared tags (via openapi-validator v2.0.0) | new version | base version |
|---|---|---|
| package-preview-2023-05 | package-preview-2023-05(036139d) | default(main) |
[must fix]The following errors/warnings are introduced by current PR:
| Rule | Message | Related RPC [For API reviewers] |
|---|---|---|
|
An async PUT operation must set long running operation options 'x-ms-long-running-operation-options' Location: Microsoft.Cache/preview/2023-05-01/redis.json#L1708 |
RPC-Async-V1-01 |
|
An async PUT operation must set long running operation options 'x-ms-long-running-operation-options' Location: Microsoft.Cache/preview/2023-05-01/redis.json#L1934 |
RPC-Async-V1-01 |
| The response of operation:'AccessPolicy_CreateUpdate' is defined without 'systemData'. Consider adding the systemData to the response. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L1708 |
||
| The x-ms-long-running-operation-options should be specified explicitly to indicate the type of response header to track the async operation. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L1708 |
RPC-Async-V1-06 | |
| The response of operation:'AccessPolicy_Get' is defined without 'systemData'. Consider adding the systemData to the response. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L1837 |
||
| The response of operation:'AccessPolicyAssignment_CreateUpdate' is defined without 'systemData'. Consider adding the systemData to the response. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L1934 |
||
| The x-ms-long-running-operation-options should be specified explicitly to indicate the type of response header to track the async operation. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L1934 |
RPC-Async-V1-06 | |
| The response of operation:'AccessPolicyAssignment_Get' is defined without 'systemData'. Consider adding the systemData to the response. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L2063 |
The following errors/warnings exist before current PR submission:
Only 30 items are listed, please refer to log for more details.
| Rule | Message |
|---|---|
|
The response schema of operations API '/providers/Microsoft.Cache/operations' does not match the ARM specification. Please standardize the schema. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L57 |
|
Resource type naming must follow camel case. Path: '/subscriptions/{subscriptionId}/providers/Microsoft.Cache/CheckNameAvailability' Location: Microsoft.Cache/preview/2023-05-01/redis.json#L73 |
|
An async PUT operation must set long running operation options 'x-ms-long-running-operation-options' Location: Microsoft.Cache/preview/2023-05-01/redis.json#L173 |
|
A 202 response should include an Location response header. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L285 |
|
An async DELETE operation must set long running operation options 'x-ms-long-running-operation-options' Location: Microsoft.Cache/preview/2023-05-01/redis.json#L299 |
|
A 202 response should include an Location response header. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L333 |
|
An async POST operation must set long running operation options 'x-ms-long-running-operation-options' Location: Microsoft.Cache/preview/2023-05-01/redis.json#L630 |
|
A 202 response should include an Location response header. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L670 |
|
An async POST operation must set long running operation options 'x-ms-long-running-operation-options' Location: Microsoft.Cache/preview/2023-05-01/redis.json#L689 |
|
A 202 response should include an Location response header. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L729 |
|
The resource name parameter 'ruleName' should be defined with a 'pattern' restriction. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L793 |
|
Tracked resource 'RedisPatchSchedule' must have patch operation that at least supports the update of tags. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L1000 |
|
The resource name parameter 'linkedServerName' should be defined with a 'pattern' restriction. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L1194 |
|
An async PUT operation must set long running operation options 'x-ms-long-running-operation-options' Location: Microsoft.Cache/preview/2023-05-01/redis.json#L1195 |
|
An async DELETE operation must set long running operation options 'x-ms-long-running-operation-options' Location: Microsoft.Cache/preview/2023-05-01/redis.json#L1262 |
|
The response in the GET collection operation 'PrivateEndpointConnections_List' does not match the response definition in the individual GET operation 'PrivateEndpointConnections_Get' . Location: Microsoft.Cache/preview/2023-05-01/redis.json#L1420 |
|
The resource name parameter 'privateEndpointConnectionName' should be defined with a 'pattern' restriction. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L1465 |
|
An async PUT operation must set long running operation options 'x-ms-long-running-operation-options' Location: Microsoft.Cache/preview/2023-05-01/redis.json#L1509 |
|
The schema 'Sku' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L2161 |
|
The schema 'RedisCommonProperties' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L2201 |
|
The schema 'RedisCreateProperties' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L2342 |
|
The schema 'RedisUpdateProperties' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L2374 |
|
The schema 'RedisProperties' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L2389 |
|
ProvisioningState must have terminal states: Succeeded, Failed and Canceled. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L2391 |
|
The schema 'RedisCreateParameters' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L2472 |
|
The schema 'RedisUpdateParameters' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L2512 |
|
The schema 'RedisAccessKeys' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L2533 |
|
The schema 'RedisFirewallRule' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L2548 |
|
The schema 'RedisFirewallRuleProperties' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L2574 |
|
The schema 'RedisFirewallRuleListResult' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.Cache/preview/2023-05-01/redis.json#L2591 |
️❌Avocado: 15 Errors, 0 Warnings failed [Detail]
| Rule | Message |
|---|---|
|
The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.Cache/Redis/{}/listUpgradeNotifications is not in the default tag. Please make sure the missing API swaggers are in the default tag.readme: specification/redis/resource-manager/readme.md json: Microsoft.Cache/preview/2019-07-01/redis.json |
|
The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.Cache/Redis/{} is not in the default tag. Please make sure the missing API swaggers are in the default tag.readme: specification/redis/resource-manager/readme.md json: Microsoft.Cache/preview/2019-07-01/redis.json |
|
The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.Cache/Redis is not in the default tag. Please make sure the missing API swaggers are in the default tag.readme: specification/redis/resource-manager/readme.md json: Microsoft.Cache/preview/2019-07-01/redis.json |
|
The default tag should contain all APIs. The API path /subscriptions/{}/providers/Microsoft.Cache/Redis is not in the default tag. Please make sure the missing API swaggers are in the default tag.readme: specification/redis/resource-manager/readme.md json: Microsoft.Cache/preview/2019-07-01/redis.json |
|
The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.Cache/Redis/{}/listKeys is not in the default tag. Please make sure the missing API swaggers are in the default tag.readme: specification/redis/resource-manager/readme.md json: Microsoft.Cache/preview/2019-07-01/redis.json |
|
The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.Cache/Redis/{}/regenerateKey is not in the default tag. Please make sure the missing API swaggers are in the default tag.readme: specification/redis/resource-manager/readme.md json: Microsoft.Cache/preview/2019-07-01/redis.json |
|
The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.Cache/Redis/{}/forceReboot is not in the default tag. Please make sure the missing API swaggers are in the default tag.readme: specification/redis/resource-manager/readme.md json: Microsoft.Cache/preview/2019-07-01/redis.json |
|
The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.Cache/Redis/{}/import is not in the default tag. Please make sure the missing API swaggers are in the default tag.readme: specification/redis/resource-manager/readme.md json: Microsoft.Cache/preview/2019-07-01/redis.json |
|
The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.Cache/Redis/{}/export is not in the default tag. Please make sure the missing API swaggers are in the default tag.readme: specification/redis/resource-manager/readme.md json: Microsoft.Cache/preview/2019-07-01/redis.json |
|
The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.Cache/Redis/{}/firewallRules is not in the default tag. Please make sure the missing API swaggers are in the default tag.readme: specification/redis/resource-manager/readme.md json: Microsoft.Cache/preview/2019-07-01/redis.json |
|
The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.Cache/Redis/{}/firewallRules/{} is not in the default tag. Please make sure the missing API swaggers are in the default tag.readme: specification/redis/resource-manager/readme.md json: Microsoft.Cache/preview/2019-07-01/redis.json |
|
The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.Cache/Redis/{}/patchSchedules is not in the default tag. Please make sure the missing API swaggers are in the default tag.readme: specification/redis/resource-manager/readme.md json: Microsoft.Cache/preview/2019-07-01/redis.json |
|
The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.Cache/Redis/{}/patchSchedules/{} is not in the default tag. Please make sure the missing API swaggers are in the default tag.readme: specification/redis/resource-manager/readme.md json: Microsoft.Cache/preview/2019-07-01/redis.json |
|
The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.Cache/Redis/{}/linkedServers/{} is not in the default tag. Please make sure the missing API swaggers are in the default tag.readme: specification/redis/resource-manager/readme.md json: Microsoft.Cache/preview/2019-07-01/redis.json |
|
The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.Cache/Redis/{}/linkedServers is not in the default tag. Please make sure the missing API swaggers are in the default tag.readme: specification/redis/resource-manager/readme.md json: Microsoft.Cache/preview/2019-07-01/redis.json |
️❌ApiReadinessCheck: 1 Errors, 0 Warnings failed [Detail]
| Rule | Message |
|---|---|
|
"code: InvalidResourceType, message: The resource type 'operations' could not be found in the namespace 'Microsoft.Cache' for api version '2023-05-01'. The supported api-versions are '2023-03-01-preview, 2022-11-01-preview, 2022-06-01, 2022-05-01, 2021-06-01, 2021-03-01, 2021-02-01-preview, 2020-12-01, 2020-10-01-preview, 2020-06-01, 2019-07-01, 2018-03-01, 2017-10-01, 2017-02-01, 2016-04-01, 2015-08-01, 2015-03-01, 2014-04-01, 2014-04-01-preview, 2014-04-01-alpha'." |
️⚠️~[Staging] ServiceAPIReadinessTest: 0 Warnings warning [Detail]
API Test is not triggered due to precheck failure. Check pipeline log for details.
️️✔️SwaggerAPIView succeeded [Detail] [Expand]
️️✔️CadlAPIView succeeded [Detail] [Expand]
️️✔️TypeSpecAPIView succeeded [Detail] [Expand]
️️✔️ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️️✔️SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️️✔️PoliCheck succeeded [Detail] [Expand]
Validation passed for PoliCheck.
️️✔️PrettierCheck succeeded [Detail] [Expand]
Validation passes for PrettierCheck.
️️✔️SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️️✔️Lint(RPaaS) succeeded [Detail] [Expand]
Validation passes for Lint(RPaaS).
️️✔️CadlValidation succeeded [Detail] [Expand]
Validation passes for CadlValidation.
️️✔️TypeSpec Validation succeeded [Detail] [Expand]
Validation passes for TypeSpec Validation.
️️✔️PR Summary succeeded [Detail] [Expand]
Validation passes for Summary.
Swagger Generation Artifacts
|
Generated ApiView
|
openapi-workflow-bot
bot
added
ARMReview
WaitForARMFeedback
|
Hi, @koderjoker your PR are labelled with WaitForARMFeedback. A notification email will be sent out shortly afterwards to notify ARM review board(armapireview@microsoft.com). |
Member
Author
|
Changes already approved as part of #22946, including SDK and swagger breaking changes |
AzureRestAPISpecReview
added
BreakingChangeReviewRequired
|
Hi @koderjoker, one or multiple breaking change(s) is detected in your PR. Please check out the breaking change(s), and provide business justification in the PR comment and @ PR assignee why you must have these change(s), and how external customer impact can be mitigated. Please ensure to follow breaking change policy to request breaking change review and approval before proceeding swagger PR review. |
|
Hi @koderjoker, Your PR has some issues. Please fix the CI sequentially by following the order of
|
|
Hi, @koderjoker, For review efficiency consideration, when creating a new api version, it is required to place API specs of the base version in the first commit, and push new version updates into successive commits. You can use OpenAPIHub to initialize the PR for adding a new version. For more details refer to the wiki. Or you could onboard API spec pipeline |
This was referenced Apr 3, 2023
| "value": [ | ||
| { | ||
| "id": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Cache/redis/cache1/accessPolicyAssignments/accessPolicy1", | ||
| "name": "cache1/accessPolicy1", |
Member
Member
There was a problem hiding this comment.
Oh I see, it is actually aliasing to the accessPolicy1 resource...? Which has to already exist? This is a kind of confusing difference to other ARM APIs.
Member
Author
There was a problem hiding this comment.
Over here we're listing the assignments done for accessPolicy1, thus the naming
Member
There was a problem hiding this comment.
Right but the URL for doing that is a bit surprising. Compared to e.g.
""/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Cache/redis/cache1/accessPolicies/accessPolicy1/assignments",
or
"/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Cache/redis/cache1/accessPolicyAssignments?$filter='accessPolicy eq accessPolicy1'"
Contributor
There was a problem hiding this comment.
I agree with Tim that this is very unorthodox. Resources aren't referenced by name, they should be referenced by ID. In this case its even more unusual in that the reference is implied by the name of the "assignment" that is being created being equal to the name of the policy that is being assigned.
I think this is not only unorthodox but problematic in your scenario because:
- It forces all objectId assignments for a specific policy to exist as an array inside 1 resource. If there are a lot that resource is going to be large. That resource is also not going to be safe to update concurrently because PATCH doesn't work well on arrays (if it was implemented in this API) and peoples changes will stomp on one another.
- It eliminates the possibility of doing linked access checks on the access policy you are assigning (since there is no resourceid in the body to do the access check on).
- RBAC becomes much less granular. An "owner" access policy has 1 and only 1 policy assignment associated with it and anyone that has access to it has access to add/remove any objectId from that assignment. If they were individual assignments that referenced the policy in their body I could give someone access to a specific instance of the owner assignment eliminating the potential for them messing up the other owner assignments.
Contributor
There was a problem hiding this comment.
@pilor , @TimLovellSmith , Thanks for pointing this out, we are open to changing our api to incorporate this suggestion in our stable GA release of the api version, but this is too late for this preview version of API as we have public preview release next week. Could you please help in merging this preview version with the exception? We will update our API before we publish GA/stable version of the corresponding api version. Hope that will be ok.
Contributor
There was a problem hiding this comment.
You won't be able to make breaking changes like this between public preview and stable API versions
Contributor
There was a problem hiding this comment.
I see, then we will be able to create a completely new api-version for GA release, right? Like 2023-06-01?
Contributor
There was a problem hiding this comment.
New API versions can still not have breaking changes. New API versions are for expanding functionality. The policy is here: https://eng.ms/docs/cloud-ai-platform/azure-core/azure-core-pm-and-design/trusted-platform-pm-karimb/service-lifecycle-and-actions-team/service-lifecycle-actions-team/apex/media/overview_breakingchanges
...manager/Microsoft.Cache/preview/2023-05-01/examples/RedisCacheAccessPolicyAssignmentGet.json
Show resolved
Hide resolved
Member
|
@koderjoker - https://github.com/Azure/azure-rest-api-specs/pull/23406/checks?check_run_id=12886661549 Also, this PR has breaking changes. Please review all the breaking changes via the link https://github.com/Azure/azure-rest-api-specs/pull/23406/checks?check_run_id=12886661236 If the breaking changes are intended, please reach out to ARM Breaking Changes reviewers azbreakchangereview@microsoft.com to get a sign off. Please remember to remove ARMChangesRequested tag once you address all the issues and get a sign off for breaking changes. |
openapi-workflow-bot
bot
removed
the
WaitForARMFeedback
Contributor
@sjanamma, for PUT api x-ms-long-running-operation-options, is not mandatory, @rkmanda suggested us to remove it in one of previous comment. There is an issue in automation check that needs to be fixed. |
koderjoker
changed the title
Member
Member
Author
We allow spaces and punctuation as well here eg. in case of user names
I've fixed this now as part of #23656. @ihsanali01 @sjanamma @qiaozha could you help us get this quickly merged in? |
koderjoker
added
the
WaitForARMFeedback
Member
I think you probably just want to remove this and ignore any linter errors - my understanding was that this LRO 'options' shouldn't be required in most cases as the SDK will do the right thing by default. Refers to: specification/redis/resource-manager/Microsoft.Cache/preview/2023-05-01/redis.json:2014 in c363e11. [](commit_id = c363e11, deletion_comment = False) |
Member
Member
@rkmanda just to check, is it only PUTs its not required on? I don't think its required for DELETE either, with the usual implementation pattern? In reply to: 1496788714 Refers to: specification/redis/resource-manager/Microsoft.Cache/preview/2023-05-01/redis.json:1716 in f960faa. [](commit_id = f960faa, deletion_comment = False) |
Member
Member
would address In reply to: 1516835971 Refers to: specification/redis/resource-manager/Microsoft.Cache/preview/2023-05-01/redis.json:2735 in c363e11. [](commit_id = c363e11, deletion_comment = False) |
Member
Please add 'pattern' specifier. In reply to: 1516579332 Refers to: specification/redis/resource-manager/Microsoft.Cache/preview/2023-05-01/redis.json:3095 in c363e11. [](commit_id = c363e11, deletion_comment = False) |
JeffreyRichter
added
the
Approved-BreakingChange
Yes, long-running-operations is not required for PUT. Please ignore that |
* Remove x-ms-long-running-operation-options * Address comments --------- Co-authored-by: Kanchan Joshi <kanchanjoshi@microsoft.com>
pilor
reviewed
Apr 24, 2023
| "value": [ | ||
| { | ||
| "id": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Cache/redis/cache1/accessPolicyAssignments/accessPolicy1", | ||
| "name": "cache1/accessPolicy1", |
Contributor
There was a problem hiding this comment.
The "name" of a nested resource is just the name of the resource, not the concatenated name of the parent + child. In this case it should be "accessPolicy1".
openapi-workflow-bot
bot
removed
the
WaitForARMFeedback
Member
Author
|
All comments addressed in fresh PR for reviewing and changing ease, please help with review here #23745. Closing this PR |
Merged
11 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ARM API Information (Control Plane)
MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.
Azure 1st Party Service can try out the Shift Left experience to initiate API design review from ADO code repo. If you are interested, may request engineering support by filling in with the form https://aka.ms/ShiftLeftSupportForm.
Changelog
Add a changelog entry for this PR by answering the following questions:
Contribution checklist (MS Employees Only):
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 the label "ARMReview" and "WaitForARMFeedback" will be added by bot to kick off ARM API Review. Missing to check this box in the following scenario may result in delays to the ARM manifest review and deployment.
-[ ] 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. You can use OpenAPIHub to initialize the PR for adding a new version. For more details refer to the wiki.
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 you have any breaking changes as defined in the Breaking Change Policy, request approval from the Breaking Change Review Board.
Action: to initiate an evaluation of the breaking change, create a new intake using the template for breaking changes. Additional details on the process and office hours are on the Breaking Change Wiki.
NOTE: To update API(s) in public preview for over 1 year (refer to Retirement of Previews)
Please follow the link to find more details on PR review process.