From 453eaacac221b8ad365ac68ee6ca942a5e445142 Mon Sep 17 00:00:00 2001 From: Mark Hughes Date: Tue, 24 Oct 2023 08:32:02 +0100 Subject: [PATCH] Add role definition read, use cloudaccess submodule in install (#13) * Add permission for roles to inspect themselves * Add delay between role definition and role assignment in cloudaccess submodule to improve reliability * Use sub-module to provision cloud access in the install * Remove now-defunct waits from main module --- .../workflows/terraform-module-validation.yml | 1 + README.md | 2 +- main.tf | 16 ---- modules/cloudaccess/variables.tf | 26 +++++++ modules/cloudaccess/wf_cloud_info.tf | 18 ++++- .../cloudaccess/wf_cloud_info_definition.json | 2 + modules/cloudaccess/wf_cluster_manager.tf | 16 ++++ modules/cloudaccess/wf_dnszone_manager.tf | 16 ++++ modules/cloudaccess/wf_network_manager.tf | 16 ++++ .../wf_network_manager_definition.json | 2 + variables.tf | 2 +- wayfinder-cloudaccess.tf | 75 ++++--------------- 12 files changed, 113 insertions(+), 79 deletions(-) diff --git a/.github/workflows/terraform-module-validation.yml b/.github/workflows/terraform-module-validation.yml index 3ca02e6..f8623e3 100644 --- a/.github/workflows/terraform-module-validation.yml +++ b/.github/workflows/terraform-module-validation.yml @@ -6,6 +6,7 @@ on: pull_request: branches: - main + - vnext jobs: terraform: diff --git a/README.md b/README.md index 401889e..ee2812c 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,7 @@ The `terraform-docs` utility is used to generate this README. Follow the below s | [clusterissuer\_email](#input\_clusterissuer\_email) | The email address to use for the cert-manager cluster issuer. | `string` | n/a | yes | | [create\_duration\_delay](#input\_create\_duration\_delay) | Used to tune terraform apply when faced with errors caused by API caching or eventual consistency. Sets a custom delay period after creation of the specified resource type. | 
object({
    azurerm_role_definition         = optional(string, "180s")
    kubectl_manifest_cloud_identity = optional(string, "30s")
  })
| `{}` | no | | [create\_localadmin\_user](#input\_create\_localadmin\_user) | Whether to create a localadmin user for access to the Wayfinder Portal and API | `bool` | `true` | no | -| [destroy\_duration\_delay](#input\_destroy\_duration\_delay) | Used to tune terraform deploy when faced with errors caused by API caching or eventual consistency. Sets a custom delay period after destruction of the specified resource type. | 
object({
    azurerm_role_definition         = optional(string, "0s")
    kubectl_manifest_cloud_identity = optional(string, "60s")
  })
| `{}` | no | +| [destroy\_duration\_delay](#input\_destroy\_duration\_delay) | Used to tune terraform destroy when faced with errors caused by API caching or eventual consistency. Sets a custom delay period after destruction of the specified resource type. | 
object({
    azurerm_role_definition         = optional(string, "0s")
    kubectl_manifest_cloud_identity = optional(string, "60s")
  })
| `{}` | no | | [disable\_internet\_access](#input\_disable\_internet\_access) | Whether to disable internet access for AKS and the Wayfinder ingress controller | `bool` | `false` | no | | [disable\_local\_login](#input\_disable\_local\_login) | Whether to disable local login for Wayfinder. Note: An IDP must be configured within Wayfinder, otherwise you will not be able to log in. | `bool` | `false` | no | | [dns\_resource\_group\_name](#input\_dns\_resource\_group\_name) | The name of the resource group where the DNS Zone exists, if different to Wayfinder's resource group. | `string` | `""` | no | diff --git a/main.tf b/main.tf index ab28eeb..ec0e679 100644 --- a/main.tf +++ b/main.tf @@ -24,22 +24,6 @@ resource "time_sleep" "after_azurerm_role_definition_main" { destroy_duration = var.destroy_duration_delay["azurerm_role_definition"] } -resource "time_sleep" "after_azurerm_role_definition" { - count = var.enable_wf_cloudaccess ? 1 : 0 - depends_on = [ - azurerm_role_definition.wayfinder_cloud_info[0], - azurerm_role_definition.wayfinder_dns_zone_manager[0], - ] - - triggers = { - "azurerm_role_definition_wayfinder_cloud_info" = jsonencode(keys(azurerm_role_definition.wayfinder_cloud_info[0])) - "azurerm_role_definition_wayfinder_dns_zone_manager" = jsonencode(keys(azurerm_role_definition.wayfinder_dns_zone_manager[0])) - } - - create_duration = var.create_duration_delay["azurerm_role_definition"] - destroy_duration = var.destroy_duration_delay["azurerm_role_definition"] -} - resource "time_sleep" "after_kubectl_manifest_cloud_identity" { count = var.enable_k8s_resources && var.enable_wf_cloudaccess ? 1 : 0 depends_on = [ diff --git a/modules/cloudaccess/variables.tf b/modules/cloudaccess/variables.tf index 33845f8..f93aab3 100644 --- a/modules/cloudaccess/variables.tf +++ b/modules/cloudaccess/variables.tf @@ -75,3 +75,29 @@ variable "enable_cloud_info" { description = "Whether to create the Cloud Info IAM Role" type = bool } + +variable "create_duration_delay" { + type = object({ + azurerm_role_definition = optional(string, "30s") + }) + description = "Used to tune terraform apply when faced with errors caused by API caching or eventual consistency. Sets a custom delay period after creation of the specified resource type." + default = {} + + validation { + condition = can([for v in values(var.create_duration_delay) : regex("^[0-9]{1,6}(s|m|h)$", v)]) + error_message = "The create_duration_delay values must be a string containing the duration in numbers (1-6 digits) followed by the measure of time represented by s (seconds), m (minutes), or h (hours)." + } +} + +variable "destroy_duration_delay" { + type = object({ + azurerm_role_definition = optional(string, "0s") + }) + description = "Used to tune terraform destroy when faced with errors caused by API caching or eventual consistency. Sets a custom delay period after destruction of the specified resource type." + default = {} + + validation { + condition = can([for v in values(var.destroy_duration_delay) : regex("^[0-9]{1,6}(s|m|h)$", v)]) + error_message = "The destroy_duration_delay values must be a string containing the duration in numbers (1-6 digits) followed by the measure of time represented by s (seconds), m (minutes), or h (hours)." + } +} diff --git a/modules/cloudaccess/wf_cloud_info.tf b/modules/cloudaccess/wf_cloud_info.tf index 8a0e2d2..b66c9df 100644 --- a/modules/cloudaccess/wf_cloud_info.tf +++ b/modules/cloudaccess/wf_cloud_info.tf @@ -13,6 +13,20 @@ resource "azurerm_role_definition" "cloudinfo" { } } +resource "time_sleep" "after_azurerm_role_definition_cloudinfo" { + count = var.enable_cloud_info ? 1 : 0 + depends_on = [ + azurerm_role_definition.cloudinfo[0], + ] + + triggers = { + "azurerm_role_definition_cloudinfo" = jsonencode(keys(azurerm_role_definition.cloudinfo[0])) + } + + create_duration = var.create_duration_delay["azurerm_role_definition"] + destroy_duration = var.destroy_duration_delay["azurerm_role_definition"] +} + resource "azurerm_role_assignment" "cloudinfo" { count = var.enable_cloud_info && var.wayfinder_identity_azure_principal_id != "" ? 1 : 0 @@ -21,7 +35,8 @@ resource "azurerm_role_assignment" "cloudinfo" { principal_id = var.wayfinder_identity_azure_principal_id depends_on = [ - azurerm_role_definition.cloudinfo[0] + time_sleep.after_azurerm_role_definition_cloudinfo[0], + azurerm_role_definition.cloudinfo[0], ] } @@ -33,6 +48,7 @@ resource "azurerm_role_assignment" "cloudinfo_federated" { principal_id = azurerm_user_assigned_identity.federated_identity[0].principal_id depends_on = [ + time_sleep.after_azurerm_role_definition_cloudinfo[0], azurerm_role_definition.cloudinfo[0], azurerm_user_assigned_identity.federated_identity[0], ] diff --git a/modules/cloudaccess/wf_cloud_info_definition.json b/modules/cloudaccess/wf_cloud_info_definition.json index 1c14df9..2e6e094 100644 --- a/modules/cloudaccess/wf_cloud_info_definition.json +++ b/modules/cloudaccess/wf_cloud_info_definition.json @@ -1,5 +1,7 @@ { "actions": [ + "Microsoft.Authorization/roleAssignments/read", + "Microsoft.Authorization/roleDefinitions/read", "Microsoft.Commerce/RateCard/read", "Microsoft.Compute/virtualMachines/vmSizes/read", "Microsoft.ContainerService/containerServices/read", diff --git a/modules/cloudaccess/wf_cluster_manager.tf b/modules/cloudaccess/wf_cluster_manager.tf index d989e59..05ae538 100644 --- a/modules/cloudaccess/wf_cluster_manager.tf +++ b/modules/cloudaccess/wf_cluster_manager.tf @@ -13,6 +13,20 @@ resource "azurerm_role_definition" "clustermanager" { } } +resource "time_sleep" "after_azurerm_role_definition_clustermanager" { + count = var.enable_cluster_manager ? 1 : 0 + depends_on = [ + azurerm_role_definition.clustermanager[0], + ] + + triggers = { + "azurerm_role_definition_clustermanager" = jsonencode(keys(azurerm_role_definition.clustermanager[0])) + } + + create_duration = var.create_duration_delay["azurerm_role_definition"] + destroy_duration = var.destroy_duration_delay["azurerm_role_definition"] +} + resource "azurerm_role_assignment" "clustermanager" { count = var.enable_cluster_manager && var.wayfinder_identity_azure_principal_id != "" ? 1 : 0 @@ -21,6 +35,7 @@ resource "azurerm_role_assignment" "clustermanager" { principal_id = var.wayfinder_identity_azure_principal_id depends_on = [ + time_sleep.after_azurerm_role_definition_clustermanager[0], azurerm_role_definition.clustermanager[0] ] } @@ -33,6 +48,7 @@ resource "azurerm_role_assignment" "clustermanager_federated" { principal_id = azurerm_user_assigned_identity.federated_identity[0].principal_id depends_on = [ + time_sleep.after_azurerm_role_definition_clustermanager[0], azurerm_role_definition.clustermanager[0], azurerm_user_assigned_identity.federated_identity[0], ] diff --git a/modules/cloudaccess/wf_dnszone_manager.tf b/modules/cloudaccess/wf_dnszone_manager.tf index 215361f..3bea02f 100644 --- a/modules/cloudaccess/wf_dnszone_manager.tf +++ b/modules/cloudaccess/wf_dnszone_manager.tf @@ -13,6 +13,20 @@ resource "azurerm_role_definition" "dnszonemanager" { } } +resource "time_sleep" "after_azurerm_role_definition_dnszonemanager" { + count = var.enable_dns_zone_manager ? 1 : 0 + depends_on = [ + azurerm_role_definition.dnszonemanager[0], + ] + + triggers = { + "azurerm_role_definition_dnszonemanager" = jsonencode(keys(azurerm_role_definition.dnszonemanager[0])) + } + + create_duration = var.create_duration_delay["azurerm_role_definition"] + destroy_duration = var.destroy_duration_delay["azurerm_role_definition"] +} + resource "azurerm_role_assignment" "dnszonemanager" { count = var.enable_dns_zone_manager && var.wayfinder_identity_azure_principal_id != "" ? 1 : 0 @@ -21,6 +35,7 @@ resource "azurerm_role_assignment" "dnszonemanager" { principal_id = var.wayfinder_identity_azure_principal_id depends_on = [ + time_sleep.after_azurerm_role_definition_dnszonemanager[0], azurerm_role_definition.dnszonemanager[0] ] } @@ -33,6 +48,7 @@ resource "azurerm_role_assignment" "dnszonemanager_federated" { principal_id = azurerm_user_assigned_identity.federated_identity[0].principal_id depends_on = [ + time_sleep.after_azurerm_role_definition_dnszonemanager[0], azurerm_role_definition.dnszonemanager[0], azurerm_user_assigned_identity.federated_identity[0], ] diff --git a/modules/cloudaccess/wf_network_manager.tf b/modules/cloudaccess/wf_network_manager.tf index fe24464..208aee7 100644 --- a/modules/cloudaccess/wf_network_manager.tf +++ b/modules/cloudaccess/wf_network_manager.tf @@ -13,6 +13,20 @@ resource "azurerm_role_definition" "networkmanager" { } } +resource "time_sleep" "after_azurerm_role_definition_networkmanager" { + count = var.enable_network_manager ? 1 : 0 + depends_on = [ + azurerm_role_definition.networkmanager[0], + ] + + triggers = { + "azurerm_role_definition_networkmanager" = jsonencode(keys(azurerm_role_definition.networkmanager[0])) + } + + create_duration = var.create_duration_delay["azurerm_role_definition"] + destroy_duration = var.destroy_duration_delay["azurerm_role_definition"] +} + resource "azurerm_role_assignment" "networkmanager" { count = var.enable_network_manager && var.wayfinder_identity_azure_principal_id != "" ? 1 : 0 @@ -21,6 +35,7 @@ resource "azurerm_role_assignment" "networkmanager" { principal_id = var.wayfinder_identity_azure_principal_id depends_on = [ + time_sleep.after_azurerm_role_definition_networkmanager[0], azurerm_role_definition.networkmanager[0] ] } @@ -33,6 +48,7 @@ resource "azurerm_role_assignment" "networkmanager_federated" { principal_id = azurerm_user_assigned_identity.federated_identity[0].principal_id depends_on = [ + time_sleep.after_azurerm_role_definition_networkmanager[0], azurerm_role_definition.dnszonemanager[0], azurerm_user_assigned_identity.federated_identity[0], ] diff --git a/modules/cloudaccess/wf_network_manager_definition.json b/modules/cloudaccess/wf_network_manager_definition.json index 891c208..d8788e6 100644 --- a/modules/cloudaccess/wf_network_manager_definition.json +++ b/modules/cloudaccess/wf_network_manager_definition.json @@ -1,5 +1,7 @@ { "actions": [ + "Microsoft.Authorization/roleAssignments/read", + "Microsoft.Authorization/roleDefinitions/read", "Microsoft.Authorization/*/read", "Microsoft.Insights/alertRules/*", "Microsoft.Network/*", diff --git a/variables.tf b/variables.tf index 35397fd..769e3cc 100644 --- a/variables.tf +++ b/variables.tf @@ -86,7 +86,7 @@ variable "destroy_duration_delay" { azurerm_role_definition = optional(string, "0s") kubectl_manifest_cloud_identity = optional(string, "60s") }) - description = "Used to tune terraform deploy when faced with errors caused by API caching or eventual consistency. Sets a custom delay period after destruction of the specified resource type." + description = "Used to tune terraform destroy when faced with errors caused by API caching or eventual consistency. Sets a custom delay period after destruction of the specified resource type." default = {} validation { diff --git a/wayfinder-cloudaccess.tf b/wayfinder-cloudaccess.tf index 53a380e..839052a 100644 --- a/wayfinder-cloudaccess.tf +++ b/wayfinder-cloudaccess.tf @@ -1,63 +1,18 @@ -resource "azurerm_role_definition" "wayfinder_dns_zone_manager" { - count = var.enable_wf_cloudaccess ? 1 : 0 - name = "WayfinderDNSZoneManager-${var.wayfinder_instance_id}" - scope = data.azurerm_subscription.current.id - description = "Wayfinder managed access to create DNS Zones in Azure" - - permissions { - actions = [ - "Microsoft.Network/dnszones/delete", - "Microsoft.Network/dnszones/NS/delete", - "Microsoft.Network/dnszones/NS/read", - "Microsoft.Network/dnszones/NS/write", - "Microsoft.Network/dnszones/read", - "Microsoft.Network/dnszones/recordsets/read", - "Microsoft.Network/dnszones/TXT/delete", - "Microsoft.Network/dnszones/TXT/read", - "Microsoft.Network/dnszones/TXT/write", - "Microsoft.Network/dnszones/write", - "Microsoft.Resources/subscriptions/providers/read", - "Microsoft.Resources/subscriptions/resourceGroups/delete", - "Microsoft.Resources/subscriptions/resourceGroups/read", - "Microsoft.Resources/subscriptions/resourceGroups/write" - ] - not_actions = [] - } -} - -resource "azurerm_role_assignment" "wayfinder_dns_zone_manager" { - count = var.enable_wf_cloudaccess ? 1 : 0 - depends_on = [time_sleep.after_azurerm_role_definition[0]] - scope = data.azurerm_subscription.current.id - role_definition_name = azurerm_role_definition.wayfinder_dns_zone_manager[0].name - principal_id = azurerm_user_assigned_identity.wayfinder_main.principal_id -} - -resource "azurerm_role_definition" "wayfinder_cloud_info" { - count = var.enable_wf_cloudaccess ? 1 : 0 - name = "WayfinderCloudInfo-${var.wayfinder_instance_id}" - scope = data.azurerm_subscription.current.id - description = "Wayfinder managed access to obtain cloud metadata like prices" - - permissions { - actions = [ - "Microsoft.Commerce/RateCard/read", - "Microsoft.Compute/virtualMachines/vmSizes/read", - "Microsoft.ContainerService/containerServices/read", - "Microsoft.Resources/providers/read", - "Microsoft.Resources/subscriptions/locations/read", - "Microsoft.Resources/subscriptions/providers/read" - ] - not_actions = [] - } -} - -resource "azurerm_role_assignment" "wayfinder_cloud_info" { - count = var.enable_wf_cloudaccess ? 1 : 0 - depends_on = [time_sleep.after_azurerm_role_definition[0]] - scope = data.azurerm_subscription.current.id - role_definition_name = azurerm_role_definition.wayfinder_cloud_info[0].name - principal_id = azurerm_user_assigned_identity.wayfinder_main.principal_id +module "wayfinder_azure_cloudaccess" { + source = "./modules/cloudaccess" + count = var.enable_wf_cloudaccess ? 1 : 0 + + resource_suffix = var.wayfinder_instance_id + wayfinder_identity_azure_principal_id = azurerm_user_assigned_identity.wayfinder_main.principal_id + wayfinder_identity_azure_tenant_id = data.azurerm_subscription.current.tenant_id + region = var.location + create_duration_delay = { azurerm_role_definition = var.create_duration_delay.azurerm_role_definition } + destroy_duration_delay = { azurerm_role_definition = var.destroy_duration_delay.azurerm_role_definition } + + enable_dns_zone_manager = true + enable_cloud_info = true + enable_cluster_manager = false + enable_network_manager = false } resource "kubectl_manifest" "wayfinder_cloud_identity_main" {