-
Notifications
You must be signed in to change notification settings - Fork 4k
fix set-azurermroledefinition bug #5032
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@darshanhs90 would you mind updating the Resources change log with a snippet of the changes being made in this PR?
PSRoleDefinition fetchedRoleDefinition = null; | ||
foreach (String scope in roleDefinition.AssignableScopes) | ||
{ | ||
try |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@darshanhs90 why is the try-catch being added?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cormacpayne Customers can add new scopes as part of the assignable scopes[irrespective of the position,either at the start or the end].
Assuming they add a new scope at the start,the getroledefinition call tries to get the roledefinition at the new scope and results in a Roledefinition doesnt exist. Hence when we add the try catch,we can check through all the assigned scopes if the Role defintiion exists or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@darshanhs90 thanks for the explanation. This PR looks good once the change log is updated.
@darshanhs90 looks like there are some merge conflicts after your other PR was merged. Would you mind taking a look? |
Description
This checklist is used to make sure that common guidelines for a pull request are followed. You can find a more complete discussion of PowerShell cmdlet best practices here.
General Guidelines
Testing Guidelines
Cmdlet Signature Guidelines
ShouldProcess
and haveSupportShouldProcess=true
specified in the cmdlet attribute. You can find more information onShouldProcess
here.OutputType
attribute if any output is produced - if the cmdlet produces no output, it should implement aPassThru
parameter.Cmdlet Parameter Guidelines