-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Support for Azure resource role settings (related to/complementary to azurerm_pim_eligible_role_assignment) #23458
Comments
@MohnJadden doesn't the proposed azurerm_pim_eligible_role_settings require some properties that define the scope of the policy/settings aka a subscription or a resource group? fwiw #23295 looks to implement this as a |
@drdamour When defining PIM settings for Azure resources, I'd argue that specifying the resource in TF defines the scope automatically. Meanwhile, if you're setting a security group to have that same role for an entire resource group via azurerm_resource_group, then the scope is defined to be done at that resource group. Same thing applies for entire subscriptions, management groups, etc. If we require specifying additional scopes for settings, those scopes may or may not be managed by Terraform, and users may not want to import them into TF. I'd say the same process used to manage one or more resources as defined in TF should be used here, rather than making users do an additional step (which may require even more re-engineering to make happen in the first place for those of us onboarding TF in brownfields) |
@MohnJadden not sure what you're describing. Settings have ot apply to something, a resource group or a subscription. Your example terraform doesn't have an property that defines what that scope is. they can't just be set at root level. ie you suggested:
and i'm saying it would need to be more like
thats whatended up being done in the pr https://github.com/hashicorp/terraform-provider-azurerm/pull/23295/files#diff-2625d6c1d3f739cd95745c6745af8e56396cd922c66d94c315c7dc905e69fdf2R38 |
@drdamour Very true. Good point - my example specified a scope for the assignment, but not settings. If there's no resource to specify against, there's nothing to set. Thank you for taking the time to clarify! Good to see the PR is in progress, thanks for linking it! |
I would like to be able to use this module the same way we use "azurerm_role_assignment" - with constraints/filters applied to the roles. That would allow us to assign PIM-able "Owner" role (with constraints that only allows Owner to assign "Contributor", "Reader" and "Key Vault). A syntax somewhat like this would be very nice to have (and consistent with "azurerm_role_assignment"):
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Is there an existing issue for this?
Community Note
Description
Privileged Identity Management has a series of settings for each role assignment at each scope: management group, subscription, resource group, or resource. These PIM settings govern details such as how long PIM eligibility lasts, approvals required, etc.
Terraform recently released the ability to assign Azure resource roles to PIM via azurerm_pim_eligible_role_assignment, but there's no way to set resource role settings from Terraform. Some use cases involve PIM eligible assignments that are permanent or longer than the 1 year default, and we have to create the resource group then manually change it in order to get PIM to work. The same goes for PIM assignments where different approval or notification workflows are required.
We should be able to set these role settings via Terraform. The settings should all be optional, with separate blocks for approvals or other future workflows that have more than just bool/int/string values.
New or Affected Resource(s)/Data Source(s)
azurerm_pim_eligible_role_assignment
Potential Terraform Configuration
References
Identified via/related to #22766
#23432 is tangentially related
The text was updated successfully, but these errors were encountered: