-
Notifications
You must be signed in to change notification settings - Fork 405
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
feat: New module avm/ptn/authorization/pim-role-assignment #4431
base: main
Are you sure you want to change the base?
feat: New module avm/ptn/authorization/pim-role-assignment #4431
Conversation
…ncies and adding new test modules for various scopes
…ecific role assignments
… as required and improve descriptions
…oved ownership tracking
…' to 'Contributor' for consistency across documentation and tests
…nd updated metadata
…ce resource removal logic
…n e2e test configuration
…s, add start and end date parameters
…nhance resource removal logic
…ule requests with AVM test justification
…e group name is empty
…rieve PrincipalId and RoleDefinitionId
…ents during removal
…eval filter for consistency
…'asTarget()' for accuracy
/avm/ptn/authorization/role-assignment/ @Azure/avm-ptn-authorization-roleassignment-module-owners-bicep @Azure/avm-module-reviewers-bicep | ||
/avm/ptn/authorization/pim-role-assignment/ @Azure/avm-ptn-authorization-pimroleassignment-module-owners-bicep @Azure/avm-module-reviewers-bicep |
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.
I guess there one two many lines? 😄
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.
sorry I don't get the problem with this one 😄
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.
Currently shows errors as the teams are either not existing - or are not yet confirmed in the hierachy of teams (documented in the contribution guide)
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.
the teams are created, I can see them on GitHub teams. So maybe not approved yet ?
$null = New-AzRoleAssignmentScheduleRequest -Name $guid ` | ||
-Scope $scope ` | ||
-PrincipalId $pimRoleAssignmentPrinicpalId ` | ||
-RequestType AdminRemove ` | ||
-RoleDefinitionId $pimRoleAssignmentRoleDefinitionId |
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.
$null = New-AzRoleAssignmentScheduleRequest -Name $guid ` | |
-Scope $scope ` | |
-PrincipalId $pimRoleAssignmentPrinicpalId ` | |
-RequestType AdminRemove ` | |
-RoleDefinitionId $pimRoleAssignmentRoleDefinitionId | |
# To remove a scheduled request, a 'AdminRemove' request must be submitted | |
$removalInputObject = @{ | |
Name = $guid | |
Scope = $scope | |
PrincipalId = $pimRoleAssignmentPrinicpalId | |
RequestType = 'AdminRemove' | |
RoleDefinitionId = $pimRoleAssignmentRoleDefinitionId | |
} | |
$null = New-AzRoleAssignmentScheduleRequest @removalInputObject |
Splatting (using the backticks) is a bit of a 'debated' practice 😄
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.
Please also update to the other case :)
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.
Resolved
@description('Required. Principle ID of the user. This value is tenant-specific and must be stored in the CI Key Vault in a secret named \'userPrinicipalId\'.') | ||
@secure() | ||
param userPrinicipalId string = '' |
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.
@description('Required. Principle ID of the user. This value is tenant-specific and must be stored in the CI Key Vault in a secret named \'userPrinicipalId\'.') | |
@secure() | |
param userPrinicipalId string = '' | |
@description('Required. Principle ID of the user. This value is tenant-specific and must be stored in the CI Key Vault in a secret named \'CI-testUserObjectId\'.') | |
@secure() | |
param testUserObjectIdstring = '' |
Would recommend to rename the secret slightly. Also, please apply the update in the description to all test files using this secret. It must have the CI-
prefix
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.
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.
Just created the secret
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.
Resolved
Description
New pattern for PIM role assignments
Pipeline Reference
Type of Change
version.json
:version.json
.version.json
.Checklist
Set-AVMModule
locally to generate the supporting module files.