Skip to content
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
{
"parameters": {
"subscriptionId": "subid",
"resourceGroupName": "rg1",
"cacheName": "cache1",
"accessPolicyName": "accessPolicy1",
"api-version": "2023-05-01",
"parameters": {
"properties": {
"assignments": [
{
"objectId": "6497c918-11ad-41e7-1b0f-7c518a87d0b0",
"objectIdAlias": "TestAADAppRedis"
},
{
"objectId": "6497c918-11ad-41e7-1b0f-7c518a87d0b0",
"objectIdAlias": "TestAADAppRedis2123"
}
]
}
}
},
"responses": {
"200": {
"body": {
"id": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Cache/redis/cache1/accessPolicyAssignments/accessPolicy1",
"name": "cache1/accessPolicy1",
"type": "Microsoft.Cache/Redis/accessPolicyAssignments",
"properties": {
"provisioningState": "Succeeded",
"assignments": [
{
"objectId": "6497c918-11ad-41e7-1b0f-7c518a87d0b0",
"objectIdAlias": "TestAADAppRedis"
}
]
}
}
},
"201": {
"body": {
"id": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Cache/redis/cache1/accessPolicyAssignments/accessPolicy1",
"name": "cache1/accessPolicy1",
"type": "Microsoft.Cache/Redis/accessPolicyAssignments",
"properties": {
"provisioningState": "Succeeded",
"assignments": [
{
"objectId": "6497c918-11ad-41e7-1b0f-7c518a87d0b0",
"objectIdAlias": "TestAADAppRedis"
}
]
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"parameters": {
"subscriptionId": "subid",
"resourceGroupName": "rg1",
"cacheName": "cache1",
"accessPolicyName": "accessPolicy1",
"api-version": "2023-05-01"
},
"responses": {
"200": {},
"204": {},
"202": {
"headers": {
"location": "https://management.azure.com/subscriptions/subid/providers/Microsoft.Cache/...pathToOperationResult..."
},
"body": {
"id": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Cache/redis/cache1/accessPolicyAssignments/accessPolicy1",
"name": "cache1/accessPolicy1",
"type": "Microsoft.Cache/Redis/accessPolicyAssignments",
"properties": {
"provisioningState": "Deleting",
"assignments": [
{
"objectId": "6497c918-11ad-41e7-1b0f-7c518a87d0b0",
"objectIdAlias": "TestAADAppRedis"
}
]
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"parameters": {
"subscriptionId": "subid",
"resourceGroupName": "rg1",
"cacheName": "cache1",
"accessPolicyName": "accessPolicy1",
"api-version": "2023-05-01"
},
"responses": {
"200": {
"body": {
"id": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Cache/redis/cache1/accessPolicyAssignments/accessPolicy1",
"name": "cache1/accessPolicy1",
"type": "Microsoft.Cache/Redis/accessPolicyAssignments",
"properties": {
"provisioningState": "Succeeded",
"assignments": [
{
"objectId": "6497c918-11ad-41e7-1b0f-7c518a87d0b0",
"objectIdAlias": "TestAADAppRedis"
}
]
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
{
"parameters": {
"subscriptionId": "subid",
"resourceGroupName": "rg1",
"cacheName": "cache1",
"api-version": "2023-05-01"
},
"responses": {
"200": {
"body": {
"value": [
{
"id": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Cache/redis/cache1/accessPolicyAssignments/accessPolicy1",
"name": "cache1/accessPolicy1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accessPolicy1

nit, accessPolicyAssignment1 seems more appropriate here

Copy link
Member

@TimLovellSmith TimLovellSmith Apr 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Over here we're listing the assignments done for accessPolicy1, thus the naming

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. 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.
  2. 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).
  3. 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.

Copy link
Contributor

@samsaha-ms samsaha-ms Apr 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You won't be able to make breaking changes like this between public preview and stable API versions

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, then we will be able to create a completely new api-version for GA release, right? Like 2023-06-01?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

"type": "Microsoft.Cache/Redis/accessPolicyAssignments",
"properties": {
"provisioningState": "Succeeded",
"assignments": [
{
"objectId": "6497c918-11ad-41e7-1b0f-7c518a87d0b0",
"objectIdAlias": "TestAADAppRedis1"
}
]
}
},
{
"id": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Cache/redis/cache1/accessPolicyAssignments/accessPolicy2",
"name": "cache1/accessPolicy2",
"type": "Microsoft.Cache/Redis/accessPolicyAssignments",
"properties": {
"provisioningState": "Succeeded",
"assignments": [
{
"objectId": "6497c918-11ad-41e7-1b0f-7c518a87d0b1",
"objectIdAlias": "TestAADAppRedis2"
}
]
}
}
]
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
{
"parameters": {
"subscriptionId": "subid",
"resourceGroupName": "rg1",
"cacheName": "cache1",
"accessPolicyName": "accessPolicy1",
"api-version": "2023-05-01",
"parameters": {
"properties": {
"permissions": "+get +hget"
}
}
},
"responses": {
"200": {
"body": {
"id": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Cache/Redis/cache1/accessPolicies/accessPolicy1",
"name": "cache1/accessPolicy1",
"type": "Microsoft.Cache/Redis/accessPolicies",
"properties": {
"provisioningState": "Succeeded",
"permissions": "+get +hget",
"type": "Custom"
}
}
},
"201": {
"body": {
"id": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Cache/Redis/cache1/accessPolicies/accessPolicy1",
"name": "cache1/accessPolicy1",
"type": "Microsoft.Cache/Redis/accessPolicies",
"properties": {
"provisioningState": "Succeeded",
"permissions": "+get +hget",
"type": "Custom"
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{
"parameters": {
"subscriptionId": "subid",
"resourceGroupName": "rg1",
"cacheName": "cache1",
"accessPolicyName": "accessPolicy1",
"api-version": "2023-05-01"
},
"responses": {
"200": {},
"204": {},
"202": {
"headers": {
"location": "https://management.azure.com/subscriptions/subid/providers/Microsoft.Cache/...pathToOperationResult..."
},
"body": {
"id": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Cache/Redis/cache1/accessPolicies/accessPolicy1",
"name": "cache1/accessPolicy1",
"type": "Microsoft.Cache/Redis/accessPolicies",
"properties": {
"provisioningState": "Deleting",
"permissions": "+get +hget",
"type": "Custom"
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"parameters": {
"subscriptionId": "subid",
"resourceGroupName": "rg1",
"cacheName": "cache1",
"accessPolicyName": "accessPolicy1",
"api-version": "2023-05-01"
},
"responses": {
"200": {
"body": {
"id": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Cache/Redis/cache1/accessPolicies/accessPolicy1",
"name": "cache1/accessPolicy1",
"type": "Microsoft.Cache/Redis/accessPolicies",
"properties": {
"provisioningState": "Succeeded",
"permissions": "+get +hget",
"type": "Custom"
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"parameters": {
"subscriptionId": "subid",
"resourceGroupName": "rg1",
"cacheName": "cache1",
"api-version": "2023-05-01"
},
"responses": {
"200": {
"body": {
"value": [
{
"id": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Cache/Redis/cache1/accessPolicies/accessPolicyBuiltIn",
"name": "cache1/accessPolicyBuiltIn",
"type": "Microsoft.Cache/Redis/accessPolicies",
"properties": {
"provisioningState": "Succeeded",
"permissions": "+get +hget",
"type": "BuiltIn"
}
},
{
"id": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.Cache/Redis/cache1/accessPolicies/accessPolicy1",
"name": "cache1/accessPolicy1",
"type": "Microsoft.Cache/Redis/accessPolicies",
"properties": {
"provisioningState": "Succeeded",
"permissions": "+get +hget",
"type": "Custom"
}
}
]
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"parameters": {
"location": "East US",
"api-version": "2023-05-01",
"subscriptionId": "subid",
"operationId": "c7ba2bf5-5939-4d79-b037-2964ccf097da"
},
"responses": {
"200": {
"body": {
"id": "/subscriptions/subid/providers/Microsoft.Cache/locations/East US/asyncOperations/c7ba2bf5-5939-4d79-b037-2964ccf097da",
"name": "c7ba2bf5-5939-4d79-b037-2964ccf097da",
"status": "Succeeded",
"startTime": null,
"endTime": null,
"percentComplete": null,
"properties": null,
"error": null
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"parameters": {
"api-version": "2023-05-01",
"subscriptionId": "subid",
"parameters": {
"type": "Microsoft.Cache/Redis",
"name": "cacheName"
}
},
"responses": {
"200": {}
}
}
Loading