From 4074c3cdd5386c7c1b7e84224854660ca6adf5d8 Mon Sep 17 00:00:00 2001 From: Mark Hughes Date: Mon, 23 Oct 2023 15:46:45 +0100 Subject: [PATCH 1/9] Add permission for roles to inspect themselves --- modules/cloudaccess/wf_cloud_info_definition.json | 2 ++ modules/cloudaccess/wf_network_manager_definition.json | 2 ++ 2 files changed, 4 insertions(+) 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_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/*", From fcacbc212314c4539f70150b3d7427732fc56ea6 Mon Sep 17 00:00:00 2001 From: Mark Hughes Date: Mon, 23 Oct 2023 15:47:01 +0100 Subject: [PATCH 2/9] Use sub-module to provision cloud access in the installer --- wayfinder-cloudaccess.tf | 66 ++++++---------------------------------- 1 file changed, 9 insertions(+), 57 deletions(-) diff --git a/wayfinder-cloudaccess.tf b/wayfinder-cloudaccess.tf index 53a380e..9fec9ac 100644 --- a/wayfinder-cloudaccess.tf +++ b/wayfinder-cloudaccess.tf @@ -1,63 +1,15 @@ -resource "azurerm_role_definition" "wayfinder_dns_zone_manager" { +module "wayfinder_azure_cloudaccess" { + source = "./modules/cloudaccess" 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_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 -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 + enable_dns_zone_manager = true + enable_cloud_info = true + enable_cluster_manager = false + enable_network_manager = false } resource "kubectl_manifest" "wayfinder_cloud_identity_main" { From 281866a63fc96f6230ec7710e585b7f98258c821 Mon Sep 17 00:00:00 2001 From: Mark Hughes Date: Mon, 23 Oct 2023 15:48:24 +0100 Subject: [PATCH 3/9] Fmt, pass region --- wayfinder-cloudaccess.tf | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/wayfinder-cloudaccess.tf b/wayfinder-cloudaccess.tf index 9fec9ac..5c1dfad 100644 --- a/wayfinder-cloudaccess.tf +++ b/wayfinder-cloudaccess.tf @@ -1,15 +1,16 @@ module "wayfinder_azure_cloudaccess" { source = "./modules/cloudaccess" - count = var.enable_wf_cloudaccess ? 1 : 0 + count = var.enable_wf_cloudaccess ? 1 : 0 - resource_suffix = var.wayfinder_instance_id + 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 - enable_dns_zone_manager = true - enable_cloud_info = true - enable_cluster_manager = false - enable_network_manager = false + enable_dns_zone_manager = true + enable_cloud_info = true + enable_cluster_manager = false + enable_network_manager = false } resource "kubectl_manifest" "wayfinder_cloud_identity_main" { From 39d5330614556ebad46539c217ab9e1ac7f61183 Mon Sep 17 00:00:00 2001 From: Mark Hughes Date: Mon, 23 Oct 2023 17:02:17 +0100 Subject: [PATCH 4/9] Run workflow on PRs to vnext --- .github/workflows/terraform-module-validation.yml | 1 + 1 file changed, 1 insertion(+) 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: From 15841fbe0a977a887d111a65a787fa037eeb0735 Mon Sep 17 00:00:00 2001 From: Mark Hughes Date: Mon, 23 Oct 2023 20:06:38 +0100 Subject: [PATCH 5/9] Remove now-defunct wait --- main.tf | 16 ---------------- 1 file changed, 16 deletions(-) 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 = [ From aa966d2083ec8ab8510cc6967288fe88550742a9 Mon Sep 17 00:00:00 2001 From: Mark Hughes Date: Mon, 23 Oct 2023 20:15:43 +0100 Subject: [PATCH 6/9] Add delay between role definition and role assignment to improve reliability --- modules/cloudaccess/variables.tf | 13 +++++++++++++ modules/cloudaccess/wf_cloud_info.tf | 18 +++++++++++++++++- modules/cloudaccess/wf_cluster_manager.tf | 16 ++++++++++++++++ modules/cloudaccess/wf_dnszone_manager.tf | 16 ++++++++++++++++ modules/cloudaccess/wf_network_manager.tf | 16 ++++++++++++++++ 5 files changed, 78 insertions(+), 1 deletion(-) diff --git a/modules/cloudaccess/variables.tf b/modules/cloudaccess/variables.tf index 33845f8..634be96 100644 --- a/modules/cloudaccess/variables.tf +++ b/modules/cloudaccess/variables.tf @@ -75,3 +75,16 @@ 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)." + } +} 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_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], ] From c86602df55028f35a0fa66820ff56c1a11af6d58 Mon Sep 17 00:00:00 2001 From: Mark Hughes Date: Mon, 23 Oct 2023 20:18:07 +0100 Subject: [PATCH 7/9] missing var --- modules/cloudaccess/variables.tf | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/modules/cloudaccess/variables.tf b/modules/cloudaccess/variables.tf index 634be96..5859907 100644 --- a/modules/cloudaccess/variables.tf +++ b/modules/cloudaccess/variables.tf @@ -88,3 +88,16 @@ variable "create_duration_delay" { 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 deploy 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)." + } +} From d8a0c61c2ad9fea6f549dbeb46bb2abf697b44d5 Mon Sep 17 00:00:00 2001 From: Mark Hughes Date: Mon, 23 Oct 2023 20:28:26 +0100 Subject: [PATCH 8/9] Pass delays from install down to cloudaccess submodule --- modules/cloudaccess/variables.tf | 2 +- variables.tf | 2 +- wayfinder-cloudaccess.tf | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/cloudaccess/variables.tf b/modules/cloudaccess/variables.tf index 5859907..f93aab3 100644 --- a/modules/cloudaccess/variables.tf +++ b/modules/cloudaccess/variables.tf @@ -93,7 +93,7 @@ variable "destroy_duration_delay" { type = object({ azurerm_role_definition = optional(string, "0s") }) - 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/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 5c1dfad..839052a 100644 --- a/wayfinder-cloudaccess.tf +++ b/wayfinder-cloudaccess.tf @@ -6,6 +6,8 @@ module "wayfinder_azure_cloudaccess" { 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 From d46dd146d35bd9b1763e56d2de50cae76ceedbd8 Mon Sep 17 00:00:00 2001 From: Mark Hughes Date: Mon, 23 Oct 2023 20:29:19 +0100 Subject: [PATCH 9/9] docs --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 550accc..e8a55dd 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 |