From 6b4266139128c3ea61a0e59292ba4ea71abac1bf Mon Sep 17 00:00:00 2001 From: Jen Sheerin <84478520+jensheerin@users.noreply.github.com> Date: Fri, 31 May 2024 16:10:22 -0400 Subject: [PATCH] Fixes lint errors after template updates (#44) * realign to template * fix linting * lint fix * fix lint * fix linting * fix lint * fix lint * fix lint * lint fix * fix lint * grp creation * fmt * recreate main * fix lint * rm extra local * update doc * template updates * template updates * lint fix * lint fix * lint fix * lint fix * fix lint * fix lint * fix lint * update doc and example * fix lint * update example * lint fix * lint fix * lint fix * fix lint * lint fix * update doc and remoteapp * fix lint * resync with new version * fix lint * fix lint * fix fmt * fix lint * fix lint * lint fix * lint fix * lint fix * lint fix --- README.md | 48 +++++++++++---------------------- examples/default/README.md | 10 +------ examples/default/main.tf | 2 +- examples/default/variables.tf | 6 ----- examples/remoteapp/README.md | 9 ------- examples/remoteapp/main.tf | 1 - examples/remoteapp/variables.tf | 6 ----- main.tf | 9 +++++++ outputs.tf | 7 +++-- variables.tf | 46 +++++++++++-------------------- 10 files changed, 47 insertions(+), 97 deletions(-) diff --git a/README.md b/README.md index 1f0464c..c41f589 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,7 @@ The following providers are used by this module: The following resources are used by this module: +- [azurerm_management_lock.this](https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/management_lock) (resource) - [azurerm_monitor_diagnostic_setting.this](https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/monitor_diagnostic_setting) (resource) - [azurerm_resource_group_template_deployment.telemetry](https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/resource_group_template_deployment) (resource) - [azurerm_role_assignment.this](https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/role_assignment) (resource) @@ -40,12 +41,6 @@ The following resources are used by this module: The following input variables are required: -### [user\_group\_name](#input\_user\_group\_name) - -Description: Microsoft Entra ID User Group for AVD users - -Type: `string` - ### [virtual\_desktop\_application\_group\_host\_pool\_id](#input\_virtual\_desktop\_application\_group\_host\_pool\_id) Description: (Required) Resource ID for a Virtual Desktop Host Pool to associate with the Virtual Desktop Application Group. Changing the name forces a new resource to be created. @@ -142,36 +137,20 @@ object({ Default: `null` -### [role\_assignment\_timeouts](#input\_role\_assignment\_timeouts) - -Description: - `create` - (Defaults to 30 minutes) Used when creating the Role Assignment. -- `delete` - (Defaults to 30 minutes) Used when deleting the Role Assignment. -- `read` - (Defaults to 5 minutes) Used when retrieving the Role Assignment. - -Type: - -```hcl -object({ - create = optional(string) - delete = optional(string) - read = optional(string) - }) -``` - -Default: `null` - ### [role\_assignments](#input\_role\_assignments) -Description: A map of role assignments to create on the resource. The map key is deliberately arbitrary to avoid issues where map keys maybe unknown at plan time. +Description: A map of role assignments to create on the . The map key is deliberately arbitrary to avoid issues where map keys maybe unknown at plan time. - - `role_definition_id_or_name` - The ID or name of the role definition to assign to the principal. - - `principal_id` - The ID of the principal to assign the role to. - - `description` - The description of the role assignment. - - `skip_service_principal_aad_check` - If set to true, skips the Azure Active Directory check for the service principal in the tenant. Defaults to false. - - `condition` - The condition which will be used to scope the role assignment. - - `condition_version` - The version of the condition syntax. Leave as `null` if you are not using a condition, if you are then valid values are '2.0'. +- `role_definition_id_or_name` - The ID or name of the role definition to assign to the principal. +- `principal_id` - The ID of the principal to assign the role to. +- `description` - (Optional) The description of the role assignment. +- `skip_service_principal_aad_check` - (Optional) If set to true, skips the Azure Active Directory check for the service principal in the tenant. Defaults to false. +- `condition` - (Optional) The condition which will be used to scope the role assignment. +- `condition_version` - (Optional) The version of the condition syntax. Leave as `null` if you are not using a condition, if you are then valid values are '2.0'. +- `delegated_managed_identity_resource_id` - (Optional) The delegated Azure Resource Id which contains a Managed Identity. Changing this forces a new resource to be created. This field is only used in cross-tenant scenario. +- `principal_type` - (Optional) The type of the `principal_id`. Possible values are `User`, `Group` and `ServicePrincipal`. It is necessary to explicitly set this attribute when creating role assignments if the principal creating the assignment is constrained by ABAC rules that filters on the PrincipalType attribute. - > Note: only set `skip_service_principal_aad_check` to true if you are assigning a role to a service principal. +> Note: only set `skip_service_principal_aad_check` to true if you are assigning a role to a service principal. Type: @@ -184,6 +163,7 @@ map(object({ condition = optional(string, null) condition_version = optional(string, null) delegated_managed_identity_resource_id = optional(string, null) + principal_type = optional(string, null) })) ``` @@ -273,6 +253,10 @@ The following outputs are exported: Description: This output is the full output for the resource to allow flexibility to reference all possible values for the resource. Example usage: module..resource.id +### [resource\_id](#output\_resource\_id) + +Description: The ID of the Azure Virtual Desktop application group + ## Modules No modules. diff --git a/examples/default/README.md b/examples/default/README.md index 18ab5cb..1eba279 100644 --- a/examples/default/README.md +++ b/examples/default/README.md @@ -82,6 +82,7 @@ data "azuread_group" "existing" { security_enabled = true } + # Assign the Azure AD group to the application group resource "azurerm_role_assignment" "this" { principal_id = data.azuread_group.existing.id @@ -103,7 +104,6 @@ module "appgroup" { virtual_desktop_application_group_resource_group_name = azurerm_resource_group.this.name virtual_desktop_application_group_name = var.virtual_desktop_application_group_name virtual_desktop_application_group_type = var.virtual_desktop_application_group_type - user_group_name = var.user_group_name } ``` @@ -161,14 +161,6 @@ Type: `string` Default: `"avdhostpool"` -### [user\_group\_name](#input\_user\_group\_name) - -Description: Microsoft Entra ID User Group for AVD users - -Type: `string` - -Default: `"avdgroup"` - ### [virtual\_desktop\_application\_group\_default\_desktop\_display\_name](#input\_virtual\_desktop\_application\_group\_default\_desktop\_display\_name) Description: (Optional) Option to set the display name for the default sessionDesktop desktop when `type` is set to `Desktop`. diff --git a/examples/default/main.tf b/examples/default/main.tf index 93571a2..caebee9 100644 --- a/examples/default/main.tf +++ b/examples/default/main.tf @@ -71,6 +71,7 @@ data "azuread_group" "existing" { security_enabled = true } + # Assign the Azure AD group to the application group resource "azurerm_role_assignment" "this" { principal_id = data.azuread_group.existing.id @@ -92,5 +93,4 @@ module "appgroup" { virtual_desktop_application_group_resource_group_name = azurerm_resource_group.this.name virtual_desktop_application_group_name = var.virtual_desktop_application_group_name virtual_desktop_application_group_type = var.virtual_desktop_application_group_type - user_group_name = var.user_group_name } diff --git a/examples/default/variables.tf b/examples/default/variables.tf index 3ecb2ec..67abaa7 100644 --- a/examples/default/variables.tf +++ b/examples/default/variables.tf @@ -14,12 +14,6 @@ variable "host_pool" { description = "The name of the AVD Host Pool to assign the application group to." } -variable "user_group_name" { - type = string - default = "avdgroup" # this should be replaced with your group. This is a sample group that is not supported. - description = "Microsoft Entra ID User Group for AVD users" -} - variable "virtual_desktop_application_group_default_desktop_display_name" { type = string default = null diff --git a/examples/remoteapp/README.md b/examples/remoteapp/README.md index bf30a6b..3cf6052 100644 --- a/examples/remoteapp/README.md +++ b/examples/remoteapp/README.md @@ -101,7 +101,6 @@ module "appgroup" { virtual_desktop_application_group_resource_group_name = azurerm_resource_group.this.name virtual_desktop_application_group_name = var.virtual_desktop_application_group_name virtual_desktop_application_group_type = var.virtual_desktop_application_group_type - user_group_name = var.user_group_name } # Sample applications @@ -187,14 +186,6 @@ Type: `string` Default: `"avdhostpool"` -### [user\_group\_name](#input\_user\_group\_name) - -Description: Microsoft Entra ID User Group for AVD users - -Type: `string` - -Default: `"avdgroup"` - ### [virtual\_desktop\_application\_group\_default\_desktop\_display\_name](#input\_virtual\_desktop\_application\_group\_default\_desktop\_display\_name) Description: (Optional) Option to set the display name for the default sessionDesktop desktop when `type` is set to `Desktop`. diff --git a/examples/remoteapp/main.tf b/examples/remoteapp/main.tf index 55624c9..a30cf2a 100644 --- a/examples/remoteapp/main.tf +++ b/examples/remoteapp/main.tf @@ -90,7 +90,6 @@ module "appgroup" { virtual_desktop_application_group_resource_group_name = azurerm_resource_group.this.name virtual_desktop_application_group_name = var.virtual_desktop_application_group_name virtual_desktop_application_group_type = var.virtual_desktop_application_group_type - user_group_name = var.user_group_name } # Sample applications diff --git a/examples/remoteapp/variables.tf b/examples/remoteapp/variables.tf index 417f798..fb810fb 100644 --- a/examples/remoteapp/variables.tf +++ b/examples/remoteapp/variables.tf @@ -14,12 +14,6 @@ variable "host_pool" { description = "The name of the AVD Host Pool to assign the application group to." } -variable "user_group_name" { - type = string - default = "avdgroup" # this should be replaced with your group. This is a sample group that is not supported. - description = "Microsoft Entra ID User Group for AVD users" -} - variable "virtual_desktop_application_group_default_desktop_display_name" { type = string default = null diff --git a/main.tf b/main.tf index 53ccea1..eded4ff 100644 --- a/main.tf +++ b/main.tf @@ -34,6 +34,15 @@ resource "azurerm_role_assignment" "this" { skip_service_principal_aad_check = each.value.skip_service_principal_aad_check } +resource "azurerm_management_lock" "this" { + count = (var.lock != null) ? 1 : 0 + + lock_level = var.lock.kind + name = coalesce(var.lock.name, "lock-${var.virtual_desktop_application_group_name}") + scope = azurerm_virtual_desktop_application_group.this.id + notes = var.lock.kind == "CanNotDelete" ? "Cannot delete the resource or its child resources." : "Cannot delete or modify the resource or its child resources." +} + # Create Diagnostic Settings for AVD application group resource "azurerm_monitor_diagnostic_setting" "this" { for_each = var.diagnostic_settings diff --git a/outputs.tf b/outputs.tf index 7387ddc..cf973c3 100644 --- a/outputs.tf +++ b/outputs.tf @@ -1,6 +1,9 @@ -# Module owners should include the full resource via a 'resource' output -# https://azure.github.io/Azure-Verified-Modules/specs/terraform/#id-tffr2---category-outputs---additional-terraform-outputs output "resource" { description = "This output is the full output for the resource to allow flexibility to reference all possible values for the resource. Example usage: module..resource.id" value = azurerm_virtual_desktop_application_group.this } + +output "resource_id" { + value = azurerm_virtual_desktop_application_group.this.id + description = "The ID of the Azure Virtual Desktop application group" +} diff --git a/variables.tf b/variables.tf index 998a1cb..3bc6776 100644 --- a/variables.tf +++ b/variables.tf @@ -1,8 +1,3 @@ -variable "user_group_name" { - type = string - description = "Microsoft Entra ID User Group for AVD users" -} - variable "virtual_desktop_application_group_host_pool_id" { type = string description = "(Required) Resource ID for a Virtual Desktop Host Pool to associate with the Virtual Desktop Application Group. Changing the name forces a new resource to be created." @@ -112,20 +107,6 @@ variable "lock" { } } -variable "role_assignment_timeouts" { - type = object({ - create = optional(string) - delete = optional(string) - read = optional(string) - }) - default = null - description = <<-EOT - - `create` - (Defaults to 30 minutes) Used when creating the Role Assignment. - - `delete` - (Defaults to 30 minutes) Used when deleting the Role Assignment. - - `read` - (Defaults to 5 minutes) Used when retrieving the Role Assignment. -EOT -} - variable "role_assignments" { type = map(object({ role_definition_id_or_name = string @@ -135,21 +116,24 @@ variable "role_assignments" { condition = optional(string, null) condition_version = optional(string, null) delegated_managed_identity_resource_id = optional(string, null) + principal_type = optional(string, null) })) default = {} - description = < Note: only set `skip_service_principal_aad_check` to true if you are assigning a role to a service principal. - DESCRIPTION nullable = false + description = <. The map key is deliberately arbitrary to avoid issues where map keys maybe unknown at plan time. + +- `role_definition_id_or_name` - The ID or name of the role definition to assign to the principal. +- `principal_id` - The ID of the principal to assign the role to. +- `description` - (Optional) The description of the role assignment. +- `skip_service_principal_aad_check` - (Optional) If set to true, skips the Azure Active Directory check for the service principal in the tenant. Defaults to false. +- `condition` - (Optional) The condition which will be used to scope the role assignment. +- `condition_version` - (Optional) The version of the condition syntax. Leave as `null` if you are not using a condition, if you are then valid values are '2.0'. +- `delegated_managed_identity_resource_id` - (Optional) The delegated Azure Resource Id which contains a Managed Identity. Changing this forces a new resource to be created. This field is only used in cross-tenant scenario. +- `principal_type` - (Optional) The type of the `principal_id`. Possible values are `User`, `Group` and `ServicePrincipal`. It is necessary to explicitly set this attribute when creating role assignments if the principal creating the assignment is constrained by ABAC rules that filters on the PrincipalType attribute. + +> Note: only set `skip_service_principal_aad_check` to true if you are assigning a role to a service principal. +DESCRIPTION } variable "tags" {