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

Include managementGroup().name as part of role assignment GUID to avoid possible duplicates. #143

Merged
merged 3 commits into from
Feb 15, 2022

Conversation

rjygraham
Copy link
Contributor

@rjygraham rjygraham commented Feb 9, 2022

Overview/Summary

This PR updates the parRoleAssignmentNameGuid default value is used to avoid potential duplicates:

Current:

@description('A GUID representing the role assignment name.  Default:  guid(parRoleDefinitionId, parAssigneeObjectId)')
param parRoleAssignmentNameGuid string = guid(parRoleDefinitionId, parAssigneeObjectId)

Updated:

@description('A GUID representing the role assignment name.  Default:  guid(managementGroup().name, parRoleDefinitionId, parAssigneeObjectId)')
param parRoleAssignmentNameGuid string = guid(managementGroup().name, parRoleDefinitionId, parAssigneeObjectId)

This PR fixes/adds/changes/removes

  1. Possible duplicate roleAssignmentIds at Management Group scope when the parRoleAssignmentNameGuid default value is used.

Breaking Changes

  1. Existing roleAssignmentIds at Management Group scope that utilized the default parRoleAssignmentNameGuid value.

Testing Evidence

image

As part of this Pull Request I have

  • [x ] Checked for duplicate Pull Requests
  • Associated it with relevant ADO items
  • [x ] Ensured my code/branch is up-to-date with the latest changes in the main branch
  • [x ] Performed testing and provided evidence.
  • [x ] Updated relevant and associated documentation.

@ghost ghost added the Needs: Triage 🔍 Needs triaging by the team label Feb 9, 2022
@jtracey93 jtracey93 self-assigned this Feb 10, 2022
@jtracey93 jtracey93 added Area: RBAC enhancement New feature or request and removed Needs: Triage 🔍 Needs triaging by the team labels Feb 10, 2022
@jtracey93 jtracey93 self-requested a review February 10, 2022 07:26
Copy link
Contributor

@jtracey93 jtracey93 left a comment

Choose a reason for hiding this comment

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

Thanks for raising this @rjygraham, good catch. As we already do this in the "..many" version of this module 👍

Could you also update the README.md in this module folder with the updated default value and description. As you did in the actual module itself👍

@ghost ghost added the Needs: Author Feedback Awaiting feedback from the issue/PR author label Feb 10, 2022
@ghost ghost removed the Needs: Author Feedback Awaiting feedback from the issue/PR author label Feb 10, 2022
@rjygraham
Copy link
Contributor Author

@jtracey93 just updated the README.md, apologies for missing it the first time.

@jtracey93 jtracey93 added this to In progress in ALZ Bicep Public Project Feb 11, 2022
@jtracey93 jtracey93 self-requested a review February 15, 2022 11:26
@jtracey93 jtracey93 merged commit 5268a43 into Azure:main Feb 15, 2022
ALZ Bicep Public Project automation moved this from In progress to Done Feb 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: RBAC enhancement New feature or request
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants