fix: tfstate-backend cold-start regression (#1093)

Co-authored-by: Nuru <[email protected]>

Author: dudymas

modules/tfstate-backend/README.md

@@ -10,12 +10,14 @@ wish to restrict who can read the production Terraform state backend S3 bucket.
 all Terraform users require read access to the most sensitive accounts, such as `root` and `audit`, in order to read
 security configuration information, so careful planning is required when architecting backend splits.
 
-> [!TIP]
->
-> Part of cold start, so it has to initially be run with `SuperAdmin`, multiple times: to create the S3 bucket and then
-> to move the state into it. Follow the guide
-> **[here](https://docs.cloudposse.com/reference-architecture/how-to-guides/implementation/enterprise/implement-aws-cold-start/#provision-tfstate-backend-component)**
-> to get started.
+## Prerequisites
+
+- This component assumes you are using the `aws-teams` and `aws-team-roles` components.
+- Before the `account` and `account-map` components are deployed for the first time, you'll want to run this component with `access_roles_enabled` set to `false` to
+  prevent errors due to missing IAM Role ARNs. 
+  This will enable only enough access to the Terraform state for you to finish provisioning accounts and roles.
+  After those components have been deployed, you will want to
+ run this component again with `access_roles_enabled` set to `true` to provide the complete access as configured in the stacks. 
 
 ### Access Control
 
@@ -141,15 +143,18 @@ terraform:
 | Name | Type |
 |------|------|
 | [aws_iam_role.default](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role) | resource |
+| [aws_arn.cold_start_access](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/arn) | data source |
+| [aws_iam_policy_document.cold_start_assume_role](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_iam_policy_document.tfstate](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
+| [aws_partition.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/partition) | data source |
 | [awsutils_caller_identity.current](https://registry.terraform.io/providers/cloudposse/awsutils/latest/docs/data-sources/caller_identity) | data source |
 
 ## Inputs
 
 | Name | Description | Type | Default | Required |
 |------|-------------|------|---------|:--------:|
 | <a name="input_access_roles"></a> [access\_roles](#input\_access\_roles) | Map of access roles to create (key is role name, use "default" for same as component). See iam-assume-role-policy module for details. | <pre>map(object({<br>    write_enabled           = bool<br>    allowed_roles           = map(list(string))<br>    denied_roles            = map(list(string))<br>    allowed_principal_arns  = list(string)<br>    denied_principal_arns   = list(string)<br>    allowed_permission_sets = map(list(string))<br>    denied_permission_sets  = map(list(string))<br>  }))</pre> | `{}` | no |
-| <a name="input_access_roles_enabled"></a> [access\_roles\_enabled](#input\_access\_roles\_enabled) | Enable creation of access roles. Set false for cold start (before account-map has been created). | `bool` | `true` | no |
+| <a name="input_access_roles_enabled"></a> [access\_roles\_enabled](#input\_access\_roles\_enabled) | Enable access roles to be assumed. Set `false` for cold start (before account-map has been created),<br>because the role to ARN mapping has not yet been created.<br>Note that the current caller and any `allowed_principal_arns` will always be allowed to assume the role. | `bool` | `true` | no |
 | <a name="input_additional_tag_map"></a> [additional\_tag\_map](#input\_additional\_tag\_map) | Additional key-value pairs to add to each map in `tags_as_list_of_maps`. Not added to `tags` or `id`.<br>This is for some rare cases where resources want additional configuration of tags<br>and therefore take a list of maps with tag key, value, and additional configuration. | `map(string)` | `{}` | no |
 | <a name="input_attributes"></a> [attributes](#input\_attributes) | ID element. Additional attributes (e.g. `workers` or `cluster`) to add to `id`,<br>in the order they appear in the list. New attributes are appended to the<br>end of the list. The elements of the list are joined by the `delimiter`<br>and treated as a single ID element. | `list(string)` | `[]` | no |
 | <a name="input_context"></a> [context](#input\_context) | Single object for setting entire context at once.<br>See description of individual variables for details.<br>Leave string and numeric variables as `null` to use default value.<br>Individual variable settings (non-null) override settings in context object,<br>except for attributes, tags, and additional\_tag\_map, which are merged. | `any` | <pre>{<br>  "additional_tag_map": {},<br>  "attributes": [],<br>  "delimiter": null,<br>  "descriptor_formats": {},<br>  "enabled": true,<br>  "environment": null,<br>  "id_length_limit": null,<br>  "label_key_case": null,<br>  "label_order": [],<br>  "label_value_case": null,<br>  "labels_as_tags": [<br>    "unset"<br>  ],<br>  "name": null,<br>  "namespace": null,<br>  "regex_replace_chars": null,<br>  "stage": null,<br>  "tags": {},<br>  "tenant": null<br>}</pre> | no |

modules/tfstate-backend/iam.tf

@@ -1,19 +1,28 @@
 locals {
-  access_roles = local.enabled && var.access_roles_enabled ? {
+  access_roles = local.enabled ? {
     for k, v in var.access_roles : (
       length(split(module.this.delimiter, k)) > 1 ? k : module.label[k].id
     ) => v
   } : {}
-  access_roles_enabled = module.this.enabled && length(keys(local.access_roles)) > 0
 
+  access_roles_enabled      = local.enabled && var.access_roles_enabled
+  cold_start_access_enabled = local.enabled && !var.access_roles_enabled
+
+  # Technically, `eks_role_arn` is incorrect, becuse it strips any path from the ARN,
+  # but since we do not expect there to be a path in the role ARN (as opposed to perhaps an attached IAM policy),
+  # it is OK. The advantage of using `eks_role_arn` is that it converts and Assumed Role ARN from STS, like
+  #    arn:aws:sts::123456789012:assumed-role/acme-core-gbl-root-admin/aws-go-sdk-1722029959251053170
+  # to the IAM Role ARN, like
+  #    arn:aws:iam::123456789012:role/acme-core-gbl-root-admin
   caller_arn = coalesce(data.awsutils_caller_identity.current.eks_role_arn, data.awsutils_caller_identity.current.arn)
 }
 
 data "awsutils_caller_identity" "current" {}
+data "aws_partition" "current" {}
 
 
 module "label" {
-  for_each = var.access_roles
+  for_each = local.enabled ? var.access_roles : {}
   source   = "cloudposse/label/null"
   version  = "0.25.0" # requires Terraform >= 0.13.0
 
@@ -28,7 +37,7 @@ module "label" {
 }
 
 module "assume_role" {
-  for_each = local.access_roles
+  for_each = local.access_roles_enabled ? local.access_roles : {}
   source   = "../account-map/modules/team-assume-role-policy"
 
   allowed_roles = each.value.allowed_roles
@@ -74,7 +83,7 @@ resource "aws_iam_role" "default" {
 
   name               = each.key
   description        = "${each.value.write_enabled ? "Access" : "Read-only access"} role for ${module.this.id}"
-  assume_role_policy = module.assume_role[each.key].policy_document
+  assume_role_policy = var.access_roles_enabled ? module.assume_role[each.key].policy_document : data.aws_iam_policy_document.cold_start_assume_role[each.key].json
   tags               = merge(module.this.tags, { Name = each.key })
 
   inline_policy {
@@ -83,3 +92,53 @@ resource "aws_iam_role" "default" {
   }
   managed_policy_arns = []
 }
+
+locals {
+  all_cold_start_access_principals = local.cold_start_access_enabled ? toset(concat([local.caller_arn],
+  flatten([for k, v in local.access_roles : v.allowed_principal_arns]))) : toset([])
+  cold_start_access_principal_arns = local.cold_start_access_enabled ? { for k, v in local.access_roles : k => distinct(concat(
+    [local.caller_arn], v.allowed_principal_arns
+  )) } : {}
+  cold_start_access_principals = local.cold_start_access_enabled ? {
+    for k, v in local.cold_start_access_principal_arns : k => formatlist("arn:%v:iam::%v:root", data.aws_partition.current.partition, distinct([
+      for arn in v : data.aws_arn.cold_start_access[arn].account
+    ]))
+  } : {}
+
+}
+
+data "aws_arn" "cold_start_access" {
+  for_each = local.all_cold_start_access_principals
+  arn      = each.value
+}
+
+# This is a basic policy that allows the caller and explicitly allowed principals to assume the role
+# during the period roles are being set up (cold start).
+data "aws_iam_policy_document" "cold_start_assume_role" {
+  for_each = local.cold_start_access_enabled ? local.access_roles : {}
+
+  statement {
+    sid = "ColdStartRoleAssumeRole"
+
+    effect = "Allow"
+    # These actions need to be kept in sync with the actions in the assume_role module
+    actions = [
+      "sts:AssumeRole",
+      "sts:SetSourceIdentity",
+      "sts:TagSession",
+    ]
+
+    condition {
+      test     = "ArnLike"
+      variable = "aws:PrincipalArn"
+      values   = local.cold_start_access_principal_arns[each.key]
+    }
+
+    principals {
+      type = "AWS"
+      # Principals is a required field, so we allow any principal in any of the accounts, restricted by the assumed Role ARN in the condition clauses.
+      # This allows us to allow non-existent (yet to be created) roles, which would not be allowed if directly specified in `principals`.
+      identifiers = local.cold_start_access_principals[each.key]
+    }
+  }
+}

modules/tfstate-backend/variables.tf

@@ -43,6 +43,10 @@ variable "access_roles" {
 
 variable "access_roles_enabled" {
   type        = bool
-  description = "Enable creation of access roles. Set false for cold start (before account-map has been created)."
+  description = <<-EOT
+    Enable access roles to be assumed. Set `false` for cold start (before account-map has been created),
+    because the role to ARN mapping has not yet been created.
+    Note that the current caller and any `allowed_principal_arns` will always be allowed to assume the role.
+    EOT
   default     = true
 }