Skip to content
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

Automatic naming for role assignments #5105

Open
aelij opened this issue Nov 8, 2021 · 11 comments
Open

Automatic naming for role assignments #5105

aelij opened this issue Nov 8, 2021 · 11 comments
Labels
enhancement New feature or request Needs: Upvote This issue requires more votes to be considered provider improvement

Comments

@aelij
Copy link
Member

aelij commented Nov 8, 2021

Is your feature request related to a problem? Please describe.
Role assignment naming is hard to get right in ARM. I think the best way of doing it is using the guid function to hash all the parameters of the role assignment. This way templates remain fully idempotent, and avoid "role assignment exists" errors.

Describe the solution you'd like

resource roleAssignment 'Microsoft.Authorization/roleAssignments@2020-10-01-preview' = {
  name: auto
  properties: {
    roleDefinitionId: roleDefinitionId
    principalId: principalId
    principalType: 'ServicePrincipal'
  }
}

name would be equal to guid(scope, roleDefinitionId, principalId, principalType) (or perhaps generalized as, guid(scope, prop1, prop2, ...). This also forces principalId (and other parameters) to be statically-known - which is great since using dynamic values makes the name non-idempotent.

Note this can also be used for Cosmos's Microsoft.DocumentDB/databaseAccounts/sqlRoleAssignments (and possibly others).

@aelij aelij added the enhancement New feature or request label Nov 8, 2021
@ghost ghost added the Needs: Triage 🔍 label Nov 8, 2021
@alex-frankel
Copy link
Collaborator

Definitely get the desire for this, but I don't think this is something likely to change. It would be up to the Authorization RP team to make a change like this, so it's not up to us.

If the name is auto, how would we know the name for future updates of the same role assignment? Personally, I'd prefer roleAssignments relax the GUID requirement and allow any name (that is unique to the tenant). I think the biggest challenge is having to guess and pray that the GUID you are generating is not taken.

@aelij
Copy link
Member Author

aelij commented Nov 9, 2021

how would we know the name for future updates

I meant that auto would compile into a guid expression from the resource's data. Then, if you change one of the parameters, it would create a new role assignment.

@alex-frankel
Copy link
Collaborator

It would be difficult to justify doing something specifically for roleAssignments, but it might be interesting to explore a more generic "auto-naming" capability for all resource types.

@alex-frankel alex-frankel added Needs: Author Feedback Awaiting feedback from the author of the issue and removed Needs: Triage 🔍 labels Nov 17, 2021
@aelij
Copy link
Member Author

aelij commented Nov 18, 2021

As I said above, it could be generalized by using the guid function on all properties, guid(scope, string(p1), string(p2), ...)

@alex-frankel
Copy link
Collaborator

The question is what properties to choose automatically? Does there need to be some sort of mapping that says something like:

  • for roleAssignment use roleDefinitionId, principalId, etc.
  • for virtualMachine use propertyA, propertyB, etc.

At that point it becomes a question of who maintains that list and what would the fallback behavior be? We also need to ask how useful is a guid for a resource name when it's not explicitly required?

I'd much prefer the Authorization RP figures out how to relax the guid requirement.

@aelij
Copy link
Member Author

aelij commented Nov 21, 2021

Why not use all properties for any resource?

I doubt it can be solved by the auth RP - the ARM contract requires operations to be idempotent, which means a PUT must include the resource identifier. And the identifier needs to be globally unique, which is why a GUID is a good choice. Plus the fact the RP doesn't allow duplicate entries (that is ones that differ by ID but have the same role assignment properties) - which is why this is difficult.

@slavizh
Copy link
Contributor

slavizh commented Nov 22, 2021

@aelij you cannot use all properties as some of these can be changed. For example if you change the description that does not result in new name and thus new role created.

@aelij
Copy link
Member Author

aelij commented Nov 22, 2021

Hmm, yes. Then perhaps an API spec extension that defines those properties (e.g. x-ms-id-property: true). As for fallback, if this extension is not defined, then Bicep should display an error that auto is not support for the type.

This could also be used by Azure SDKs which now produce a random GUID for role assignments. That makes it difficult to migrate an existing assignment to Bicep.

@aelij
Copy link
Member Author

aelij commented Apr 23, 2023

API spec extension that defines those properties (e.g. x-ms-id-property: true).

What about this idea?

@ghost
Copy link

ghost commented May 19, 2023

Hi aelij, this issue has been marked as stale because it was labeled as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. Thanks for contributing to bicep! 😄 🦾

@github-project-automation github-project-automation bot moved this to Todo in Bicep May 19, 2023
@aelij
Copy link
Member Author

aelij commented May 21, 2023

Not stale

@ghost ghost added Needs: Attention 👋 and removed Needs: Author Feedback Awaiting feedback from the author of the issue Status: No Recent Activity labels May 21, 2023
@alex-frankel alex-frankel added Needs: Triage 🔍 Needs: Upvote This issue requires more votes to be considered and removed Needs: Triage 🔍 Needs: Attention 👋 labels May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Needs: Upvote This issue requires more votes to be considered provider improvement
Projects
Status: Todo
Development

No branches or pull requests

3 participants