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

Proposal: Make "name" optional on Microsoft.Authorization/roleAssignments #1771

Closed
oliverroer opened this issue Jun 18, 2021 · 3 comments
Closed

Comments

@oliverroer
Copy link

Note: I'm not sure this is the appropriate place to put this. If not, please guide me to the right place to direct this input.

This particular part of the design is a huge pain point when working with role assignments in ARM.

In the ARM template reference, the name property is marked as required must be a valid GUID.

If you do a few role assignments in ARM, that might work just fine for you, but once you really get going and start using ARM to create role assignments left and right, you really start to feel the pain. Soon enough, you will may run into either of these two issues:

  1. The name you provided already matches an existing assignment, but other properties such as the roleDefinitionId, principalId or scope differ. This will be interpreted as an attempt at modifying these properties, which is not allowed, resulting in the following error:
{
    "status": "Failed",
    "error": {
        "code": "RoleAssignmentUpdateNotPermitted",
        "message": "Tenant ID, application ID, principal ID, and scope are not allowed to be updated."
    }
}
  1. The name you provided is unique, but there is already an existing assignment with the same roleDefinitionId, principalId and scope, but a different name. Creating an identical role assignment with a different name is not allowed, and will give the following error:
{
    "status": "Failed",
    "error": {
        "code": "RoleAssignmentExists",
        "message": "The role assignment already exists."
    }
}

Here's a scenario that will give you issue 1:
You have made an ARM template which assigns the Contributor role on a Storage account to an AD group called Contoso Storage Contributors.
This template is regularly maintained and deployed, as it also manages a variety of other resources.
One day, one of these parameters have to be changed for various reasons. For example, it could be that you realize that a different role is more appropriate, or you want to assign it to a different AD group.
If you naively just go and change one of these properties in your template, your next deployment will fail, as you've now run into issue 1. To resolve it, you will now have to change how you generate the GUID for the name. Perhaps, in order to avoid this issue in the future, you use the ID of the AD group and the role definition as seeds to the guid function, and for good measure, you use the resource ID too, to make sure you never accidentally generate a name that is the same as another unrelated role assignment:

...
    "name": [guid(variables('principalId'), variables('roleId'), variables('resourceId'))]
...

Note: These three variables are a simplification. In a real scenario, you might need significantly larger expressions to get these three values, resulting in a huge ugly one-liner.
Note: Also, after fixing this, you may have to manually delete the old role assignment which might still be hanging around in Azure - but that's a different story.

From my experience, this is the best-practice approach to coming up with a GUID for the role assignment, as it incorporates the three properties that uniquely identifies the role assignment, namely the principal, the role and the scope of the assignment. You should never have this issue if you apply this approach everywhere.

Here's a scenario that will give you issue 2:
After having learned your lesson from issue 1, you want to go back to other role assignments you've implemented in ARM, and apply the same principles there as well, to avoid the same issue in the future.

After applying this method of generating the name for one of the other role assignments, you will hit issue 2, as you are now generating a role assignment with a new name, but there's still an identical role assignment in Azure with the old name.

The solution here is either:
A) Don't change it. Just keep it as it is and pray that it won't become an issue, or wait with changing it until you update one of the properties - and now you have tech debt.
B) Remove the role assignment with the old name before deploying role assignment with the new name. This does not scale well if you've heavily embraced automation, and generated many assignments in many subscriptions. Also, you have no easy way of keeping track of which assignments have to be cleaned up and which ones don't. Additionally, you may have services that are dependent on these assignments to work. In the period between the old assignment being removed and the new assignment being created, the service might be missing the permissions it needs to carry out an important job.

Hence my proposal: Make the name parameter optional so that people don't have to deal with it. If a name is not specified Azure should just generate a name based on the properties that uniquely identify a role assignment. In Azure Blueprints, you can create role assignments on a subscription and resource group level, without having to give it a name - Blueprints just magically handles that for you. It would be great if ARM had that same kind of magic built in. Sadly, Blueprints has other problems, such as this and the fact that it can't natively do role assignments on the resource level - for that, you have to resort to ARM. ☹️

@ghost
Copy link

ghost commented Sep 9, 2021

Hello @darshanhs90, @AshishGargMicrosoft! It looks like there is a schemas issue that needs your attention. Please investigate and confirm it is on your end. Thanks 😄

Issue Details

Note: I'm not sure this is the appropriate place to put this. If not, please guide me to the right place to direct this input.

This particular part of the design is a huge pain point when working with role assignments in ARM.

In the ARM template reference, the name property is marked as required must be a valid GUID.

If you do a few role assignments in ARM, that might work just fine for you, but once you really get going and start using ARM to create role assignments left and right, you really start to feel the pain. Soon enough, you will may run into either of these two issues:

  1. The name you provided already matches an existing assignment, but other properties such as the roleDefinitionId, principalId or scope differ. This will be interpreted as an attempt at modifying these properties, which is not allowed, resulting in the following error:
{
    "status": "Failed",
    "error": {
        "code": "RoleAssignmentUpdateNotPermitted",
        "message": "Tenant ID, application ID, principal ID, and scope are not allowed to be updated."
    }
}
  1. The name you provided is unique, but there is already an existing assignment with the same roleDefinitionId, principalId and scope, but a different name. Creating an identical role assignment with a different name is not allowed, and will give the following error:
{
    "status": "Failed",
    "error": {
        "code": "RoleAssignmentExists",
        "message": "The role assignment already exists."
    }
}

Here's a scenario that will give you issue 1:
You have made an ARM template which assigns the Contributor role on a Storage account to an AD group called Contoso Storage Contributors.
This template is regularly maintained and deployed, as it also manages a variety of other resources.
One day, one of these parameters have to be changed for various reasons. For example, it could be that you realize that a different role is more appropriate, or you want to assign it to a different AD group.
If you naively just go and change one of these properties in your template, your next deployment will fail, as you've now run into issue 1. To resolve it, you will now have to change how you generate the GUID for the name. Perhaps, in order to avoid this issue in the future, you use the ID of the AD group and the role definition as seeds to the guid function, and for good measure, you use the resource ID too, to make sure you never accidentally generate a name that is the same as another unrelated role assignment:

...
    "name": [guid(variables('principalId'), variables('roleId'), variables('resourceId'))]
...

Note: These three variables are a simplification. In a real scenario, you might need significantly larger expressions to get these three values, resulting in a huge ugly one-liner.
Note: Also, after fixing this, you may have to manually delete the old role assignment which might still be hanging around in Azure - but that's a different story.

From my experience, this is the best-practice approach to coming up with a GUID for the role assignment, as it incorporates the three properties that uniquely identifies the role assignment, namely the principal, the role and the scope of the assignment. You should never have this issue if you apply this approach everywhere.

Here's a scenario that will give you issue 2:
After having learned your lesson from issue 1, you want to go back to other role assignments you've implemented in ARM, and apply the same principles there as well, to avoid the same issue in the future.

After applying this method of generating the name for one of the other role assignments, you will hit issue 2, as you are now generating a role assignment with a new name, but there's still an identical role assignment in Azure with the old name.

The solution here is either:
A) Don't change it. Just keep it as it is and pray that it won't become an issue, or wait with changing it until you update one of the properties - and now you have tech debt.
B) Remove the role assignment with the old name before deploying role assignment with the new name. This does not scale well if you've heavily embraced automation, and generated many assignments in many subscriptions. Also, you have no easy way of keeping track of which assignments have to be cleaned up and which ones don't. Additionally, you may have services that are dependent on these assignments to work. In the period between the old assignment being removed and the new assignment being created, the service might be missing the permissions it needs to carry out an important job.

Hence my proposal: Make the name parameter optional so that people don't have to deal with it. If a name is not specified Azure should just generate a name based on the properties that uniquely identify a role assignment. In Azure Blueprints, you can create role assignments on a subscription and resource group level, without having to give it a name - Blueprints just magically handles that for you. It would be great if ARM had that same kind of magic built in. Sadly, Blueprints has other problems, such as this and the fact that it can't natively do role assignments on the resource level - for that, you have to resort to ARM. ☹️

Author: oliverroer
Assignees: -
Labels:

RP:Authorization

Milestone: -

@dagoroz
Copy link
Contributor

dagoroz commented Oct 20, 2021

Closing: won't fix
ARM templates is tightly coupled to the schema for role assignment operations
making name optional would mean reworking how the AuthN RP processes RA PUT requests.
The feature request has been noted but it doesn't align with the current feature team goals

@anthony-c-martin
Copy link
Member

Closing as per @dagoroz's comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants