From 98d7437f455bc9686b74623b97e211b13e7f913e Mon Sep 17 00:00:00 2001 From: David Fernandez Date: Thu, 9 Apr 2026 15:55:48 -0300 Subject: [PATCH 1/5] fix: remove provider --- infrastructure/azure/acr/provider.tf | 7 ------- infrastructure/azure/aks/provider.tf | 6 ------ infrastructure/azure/dns/provider.tf | 7 ------- infrastructure/azure/private_dns/provider.tf | 6 ------ infrastructure/azure/resource_group/provider.tf | 7 ------- infrastructure/azure/vnet/provider.tf | 7 ------- 6 files changed, 40 deletions(-) diff --git a/infrastructure/azure/acr/provider.tf b/infrastructure/azure/acr/provider.tf index ef7f06eb..a2489744 100644 --- a/infrastructure/azure/acr/provider.tf +++ b/infrastructure/azure/acr/provider.tf @@ -8,10 +8,3 @@ terraform { } } -provider "azurerm" { - features {} - resource_provider_registrations = "none" - use_cli = true - subscription_id = var.subscription_id -} - diff --git a/infrastructure/azure/aks/provider.tf b/infrastructure/azure/aks/provider.tf index 7ecaa77c..a2489744 100644 --- a/infrastructure/azure/aks/provider.tf +++ b/infrastructure/azure/aks/provider.tf @@ -8,9 +8,3 @@ terraform { } } -provider "azurerm" { - features {} - resource_provider_registrations = "none" - use_cli = true - subscription_id = var.subscription_id -} diff --git a/infrastructure/azure/dns/provider.tf b/infrastructure/azure/dns/provider.tf index ef7f06eb..a2489744 100644 --- a/infrastructure/azure/dns/provider.tf +++ b/infrastructure/azure/dns/provider.tf @@ -8,10 +8,3 @@ terraform { } } -provider "azurerm" { - features {} - resource_provider_registrations = "none" - use_cli = true - subscription_id = var.subscription_id -} - diff --git a/infrastructure/azure/private_dns/provider.tf b/infrastructure/azure/private_dns/provider.tf index 7ecaa77c..a2489744 100644 --- a/infrastructure/azure/private_dns/provider.tf +++ b/infrastructure/azure/private_dns/provider.tf @@ -8,9 +8,3 @@ terraform { } } -provider "azurerm" { - features {} - resource_provider_registrations = "none" - use_cli = true - subscription_id = var.subscription_id -} diff --git a/infrastructure/azure/resource_group/provider.tf b/infrastructure/azure/resource_group/provider.tf index ef7f06eb..a2489744 100644 --- a/infrastructure/azure/resource_group/provider.tf +++ b/infrastructure/azure/resource_group/provider.tf @@ -8,10 +8,3 @@ terraform { } } -provider "azurerm" { - features {} - resource_provider_registrations = "none" - use_cli = true - subscription_id = var.subscription_id -} - diff --git a/infrastructure/azure/vnet/provider.tf b/infrastructure/azure/vnet/provider.tf index ef7f06eb..a2489744 100644 --- a/infrastructure/azure/vnet/provider.tf +++ b/infrastructure/azure/vnet/provider.tf @@ -8,10 +8,3 @@ terraform { } } -provider "azurerm" { - features {} - resource_provider_registrations = "none" - use_cli = true - subscription_id = var.subscription_id -} - From 0f67db6b5d864ca7b995109c82db9055a7a6d6c6 Mon Sep 17 00:00:00 2001 From: David Fernandez Date: Fri, 19 Jun 2026 10:31:17 -0300 Subject: [PATCH 2/5] docs(iam/agent): spec for splitting agent role into agent + permissions roles Co-Authored-By: Claude Opus 4.8 (1M context) --- .../2026-06-19-iam-agent-role-split-design.md | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-19-iam-agent-role-split-design.md diff --git a/docs/superpowers/specs/2026-06-19-iam-agent-role-split-design.md b/docs/superpowers/specs/2026-06-19-iam-agent-role-split-design.md new file mode 100644 index 00000000..68f0851b --- /dev/null +++ b/docs/superpowers/specs/2026-06-19-iam-agent-role-split-design.md @@ -0,0 +1,102 @@ +# Design: split del módulo `infrastructure/aws/iam/agent` en rol agente + rol de permisos + +Fecha: 2026-06-19 + +## Contexto + +Hoy el módulo `infrastructure/aws/iam/agent` crea un único rol IRSA +(`nullplatform_agent_role`) confiado por el OIDC provider del cluster, con todas +las políticas pegadas directamente: + +- `nullplatform_route53_policy` +- `nullplatform_eks_policy` +- `nullplatform_elb_policy` +- `nullplatform_avp_policy` +- `nullplatform_assume_role_policy` (condicional, solo si `assume_role_arns` no está vacío) +- `var.additional_policies` + +El token IRSA del service account, por lo tanto, tiene acceso directo a todos +esos permisos. + +## Objetivo + +Aplicar separación de privilegios (*role chaining*): + +- El rol IRSA (Rol A) queda solo con capacidad de `sts:AssumeRole`. +- Un nuevo rol de permisos (Rol B) concentra las políticas reales y confía en + el Rol A. + +Esto reduce el blast radius del token IRSA: el token por sí solo no puede tocar +Route53/EKS/ELB/AVP; primero tiene que asumir el Rol B. + +## Arquitectura resultante + +``` +Service Account (K8s) + │ OIDC / IRSA + ▼ +Rol A: nullplatform-{cluster}-agent-role ← rol agente (IRSA) + │ política única: sts:AssumeRole → [ Rol B, *assume_role_arns ] + │ + additional_policies (sin cambios) + ▼ (assume) +Rol B: nullplatform-{cluster}-agent-permissions-role ← rol de permisos (NUEVO) + trust: principal AWS = ARN de Rol A + políticas pegadas: route53, eks, elb, avp +``` + +## Decisiones de diseño + +1. **Qué políticas se mueven al Rol B:** solo las 4 gestionadas (route53, eks, + elb, avp). `additional_policies` se siguen pegando al Rol A. +2. **`assume_role_arns`:** el permiso se otorga en el Rol A. El Rol A puede + asumir tanto el Rol B como los `assume_role_arns` externos (se mantiene el + comportamiento actual de a quién apunta el agente). +3. **Outputs:** se mantiene `nullplatform_agent_role_arn` (Rol A) y se agrega + `nullplatform_agent_permissions_role_arn` (Rol B). No rompe consumidores. + +## Romper la dependencia circular + +El Rol A necesita el ARN del Rol B (en su policy de assume) y el Rol B necesita +el ARN del Rol A (en su trust policy). Para evitar el ciclo en el grafo de +Terraform, ambos ARNs se construyen como `locals` a partir de +`data.aws_caller_identity.current` + los nombres (que ya son `locals` +deterministas, con `use_name_prefix = false`), sin referenciar el recurso del +otro rol. + +## Cambios por archivo + +- **`data.tf`**: agregar `data.aws_caller_identity.current`. +- **`main.tf`**: + - `locals`: agregar `permissions_role_name` + (default `nullplatform-{cluster}-agent-permissions-role`), + `permissions_role_arn` y `agent_role_arn` (computados desde caller identity). + - Módulo `nullplatform_agent_role` (Rol A): el mapa `policies` pasa a ser solo + `nullplatform_assume_role_policy` + `var.additional_policies`. Se quitan + route53/eks/elb/avp. + - `aws_iam_policy.nullplatform_assume_role_policy`: deja de ser condicional + (siempre se crea). `Resource = concat([local.permissions_role_arn], var.assume_role_arns)`. + Se elimina el `count`. Se agrega `moved` block para migrar + `nullplatform_assume_role_policy[0]` → `nullplatform_assume_role_policy`. + - Nuevo `aws_iam_role.nullplatform_agent_permissions` (Rol B), con + `assume_role_policy` cuyo principal AWS = `local.agent_role_arn`. + - Nuevos 4 `aws_iam_role_policy_attachment` que pegan route53/eks/elb/avp al + Rol B. Los `aws_iam_policy` de esas 4 se mantienen iguales (mismo contenido y + nombres). +- **`variables.tf`**: agregar `permissions_role_name` (override, default `""`). +- **`outputs.tf`**: agregar `nullplatform_agent_permissions_role_arn`. +- **`tests/agent.tftest.hcl`**: la `assume_role_policy` ahora siempre existe (ya + no es `[0]`); agregar el nuevo Rol B y attachments en los mocks; actualizar + `assume_role_policy_not_created_by_default` (ahora sí se crea por defecto, + apuntando al rol de permisos). +- **`README.md`**: regenerar descripción/arquitectura/features/inputs/outputs + (bloques `BEGIN_TF_DOCS` y `BEGIN_AI_METADATA`). + +## Testing + +`tofu test` sobre el módulo con el provider mockeado. Se verifica: +- Nombres de las 4 políticas (sin cambios). +- JSON válido de todas las políticas. +- El rol de permisos existe y tiene los 4 attachments. +- La assume policy del rol agente referencia el ARN del rol de permisos por + defecto, y suma `assume_role_arns` cuando se proveen. +- El trust del rol de permisos referencia el ARN del rol agente. From cf19b16dd3d0f3b7c8d9b7fda055ed7eb5586c08 Mon Sep 17 00:00:00 2001 From: David Fernandez Date: Fri, 19 Jun 2026 11:17:53 -0300 Subject: [PATCH 3/5] feat(iam/agent)!: split agent role into agent + permissions roles Apply privilege separation to the agent IAM module. The IRSA agent role no longer carries the workload policies directly; it only holds an sts:AssumeRole policy and assumes a separate permissions role that holds the Route53/EKS/ELB/AVP policies and trusts only the agent role. - agent role: only sts:AssumeRole (permissions role + assume_role_arns) plus additional_policies - new permissions role (nullplatform-{cluster}-agent-permissions-role) with the four workload policies attached, trusting the agent role - ARNs derived from names + caller account id to avoid a circular dependency between the two roles - assume_role_policy is now always created (moved block migrates it from the previous conditional [0] address) - new permissions_role_name variable and nullplatform_agent_permissions_role_arn output BREAKING CHANGE: the IRSA token no longer has Route53/EKS/ELB/AVP permissions directly. The agent must assume the permissions role (exposed via the nullplatform_agent_permissions_role_arn output) to use them. Co-Authored-By: Claude Opus 4.8 (1M context) --- infrastructure/aws/iam/agent/README.md | 55 ++++++--- infrastructure/aws/iam/agent/data.tf | 3 + infrastructure/aws/iam/agent/main.tf | 110 +++++++++++------- infrastructure/aws/iam/agent/outputs.tf | 5 + .../aws/iam/agent/tests/agent.tftest.hcl | 93 ++++++++++++--- infrastructure/aws/iam/agent/variables.tf | 6 + 6 files changed, 195 insertions(+), 77 deletions(-) diff --git a/infrastructure/aws/iam/agent/README.md b/infrastructure/aws/iam/agent/README.md index 066e0881..a340ceb4 100644 --- a/infrastructure/aws/iam/agent/README.md +++ b/infrastructure/aws/iam/agent/README.md @@ -2,21 +2,22 @@ ## Description -Creates an IRSA-enabled IAM role with scoped policies for the nullplatform agent Kubernetes service account on EKS +Creates an IRSA-enabled IAM agent role for the nullplatform Kubernetes service account on EKS, using privilege separation: the agent role only carries an sts:AssumeRole policy and assumes a separate permissions role that holds the scoped workload policies ## Architecture -The module uses the terraform-aws-modules/iam//modules/iam-role-for-service-accounts submodule to create an aws_iam_role with an OIDC trust policy bound to a specific Kubernetes namespace and service account. Four aws_iam_policy resources are created for Route53, ELB, EKS, and Amazon Verified Permissions, and conditionally a fifth for sts:AssumeRole when assume_role_arns is non-empty. All policies are attached to the IAM role via the submodule's policies map, and the resulting role ARN is exposed as an output. +The module uses the terraform-aws-modules/iam//modules/iam-role-for-service-accounts submodule to create an aws_iam_role (the agent role) with an OIDC trust policy bound to a specific Kubernetes namespace and service account. The agent role only carries an sts:AssumeRole policy that allows it to assume a separate permissions role (and any additional assume_role_arns). The permissions role is a standalone aws_iam_role whose trust policy allows only the agent role to assume it, and the four aws_iam_policy resources (Route53, ELB, EKS, and Amazon Verified Permissions) are attached to it. ARNs are derived from role names and the caller account id to avoid a circular dependency between the two roles. Both role ARNs are exposed as outputs. ## Features -- Creates an IRSA IAM role scoped to a specific Kubernetes namespace and service account via OIDC provider trust -- Attaches a Route53 policy granting DNS record management permissions for hosted zones -- Attaches an ELB policy granting describe permissions for load balancers and target groups -- Attaches an EKS policy granting read access to clusters, node groups, and addons -- Attaches an Amazon Verified Permissions (AVP) policy granting full verifiedpermissions access -- Conditionally creates and attaches an sts:AssumeRole policy when assume_role_arns is provided -- Supports attaching additional custom IAM policies via the additional_policies map +- Creates an IRSA IAM agent role scoped to a specific Kubernetes namespace and service account via OIDC provider trust +- Keeps the agent role minimal: it only carries an sts:AssumeRole policy targeting the permissions role and any additional assume_role_arns +- Creates a separate permissions role that trusts only the agent role and holds the workload policies +- Attaches a Route53 policy granting DNS record management permissions for hosted zones to the permissions role +- Attaches an ELB policy granting describe permissions for load balancers and target groups to the permissions role +- Attaches an EKS policy granting read access to clusters, node groups, and addons to the permissions role +- Attaches an Amazon Verified Permissions (AVP) policy granting full verifiedpermissions access to the permissions role +- Supports attaching additional custom IAM policies to the agent role via the additional_policies map ## Basic Usage @@ -63,6 +64,13 @@ resource "example_resource" "this" { | [aws_iam_policy.nullplatform_eks_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource | | [aws_iam_policy.nullplatform_elb_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource | | [aws_iam_policy.nullplatform_route53_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource | +| [aws_iam_role.nullplatform_agent_permissions](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role) | resource | +| [aws_iam_role_policy_attachment.permissions_avp](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource | +| [aws_iam_role_policy_attachment.permissions_eks](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource | +| [aws_iam_role_policy_attachment.permissions_elb](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource | +| [aws_iam_role_policy_attachment.permissions_route53](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource | +| [aws_caller_identity.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/caller_identity) | data source | +| [aws_region.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/region) | data source | ## Inputs @@ -73,6 +81,7 @@ resource "example_resource" "this" { | [assume\_role\_arns](#input\_assume\_role\_arns) | List of IAM role ARNs the agent is allowed to assume via sts:AssumeRole | `list(string)` | `[]` | no | | [aws\_iam\_openid\_connect\_provider\_arn](#input\_aws\_iam\_openid\_connect\_provider\_arn) | ARN of the AWS IAM OIDC provider for EKS service account authentication | `string` | n/a | yes | | [cluster\_name](#input\_cluster\_name) | Name of the cluster where the policy runs | `string` | n/a | yes | +| [permissions\_role\_name](#input\_permissions\_role\_name) | Override for the permissions IAM role name. Defaults to nullplatform-{cluster\_name}-agent-permissions-role | `string` | `""` | no | | [policies\_name\_prefix](#input\_policies\_name\_prefix) | Override for IAM policy name prefix. Defaults to nullplatform\_{cluster\_name} | `string` | `""` | no | | [role\_name](#input\_role\_name) | Override for the IAM role name. Defaults to nullplatform-{cluster\_name}-agent-role | `string` | `""` | no | | [service\_account\_name](#input\_service\_account\_name) | Kubernetes service account name trusted by the IRSA role | `string` | `"nullplatform-agent"` | no | @@ -81,22 +90,24 @@ resource "example_resource" "this" { | Name | Description | |------|-------------| +| [nullplatform\_agent\_permissions\_role\_arn](#output\_nullplatform\_agent\_permissions\_role\_arn) | ARN of the permissions role assumed by the agent role | | [nullplatform\_agent\_role\_arn](#output\_nullplatform\_agent\_role\_arn) | ARN of the agent role | @@ -107,7 +143,8 @@ resource "example_resource" "this" { "Attaches an ELB policy granting describe permissions for load balancers and target groups to the permissions role", "Attaches an EKS policy granting read access to clusters, node groups, and addons to the permissions role", "Attaches an Amazon Verified Permissions (AVP) policy granting full verifiedpermissions access to the permissions role", - "Supports attaching additional custom IAM policies to the agent role via the additional_policies map" + "Supports attaching additional custom IAM policies to the agent role via the additional_policies map", + "Supports creating additional permissions roles via the permissions_roles map, each trusting the agent role and assumable by it" ], "inputs": [ { @@ -140,6 +177,11 @@ resource "example_resource" "this" { "description": "Override for the permissions IAM role name. Defaults to nullplatform-{cluster_name}-agent-permissions-role", "required": false }, + { + "name": "permissions_roles", + "description": "Additional permissions roles created by this module and assumable by the agent role. Map key is a logical name; name overrides the role name (defaults to nullplatform-{cluster_name}-{key}); policy_arns are the policy ARNs attached to the role.", + "required": false + }, { "name": "service_account_name", "description": "Kubernetes service account name trusted by the IRSA role", @@ -158,7 +200,8 @@ resource "example_resource" "this" { ], "outputs": [ "nullplatform_agent_role_arn", - "nullplatform_agent_permissions_role_arn" + "nullplatform_agent_permissions_role_arn", + "nullplatform_agent_extra_permissions_role_arns" ], "hash": "5142461751e55436dbc95fa82a376955" } diff --git a/infrastructure/aws/iam/agent/main.tf b/infrastructure/aws/iam/agent/main.tf index 1735ce3d..e9033bf5 100644 --- a/infrastructure/aws/iam/agent/main.tf +++ b/infrastructure/aws/iam/agent/main.tf @@ -8,6 +8,30 @@ locals { # (trust policy -> agent role). agent_role_arn = "arn:aws:iam::${data.aws_caller_identity.current.account_id}:role/${local.role_name}" permissions_role_arn = "arn:aws:iam::${data.aws_caller_identity.current.account_id}:role/${local.permissions_role_name}" + + # Resolve the name and (computed) ARN of each extra permissions role up front, + # so both the agent assume policy and the role trust policies reference the + # same deterministic ARN without depending on each other's resources. + extra_permissions_role_names = { + for key, cfg in var.permissions_roles : key => coalesce(cfg.name, "nullplatform-${var.cluster_name}-${key}") + } + extra_permissions_role_arns = [ + for name in values(local.extra_permissions_role_names) : "arn:aws:iam::${data.aws_caller_identity.current.account_id}:role/${name}" + ] + + # Flatten the extra permissions roles into one (role, policy_arn) pair per + # attachment, keyed by "role::arn" so the for_each key is stable regardless of + # list ordering. + extra_permissions_attachments = { + for pair in flatten([ + for role_key, cfg in var.permissions_roles : [ + for arn in cfg.policy_arns : { + role_key = role_key + arn = arn + } + ] + ]) : "${pair.role_key}::${pair.arn}" => pair + } } ################################################################################ @@ -79,6 +103,36 @@ resource "aws_iam_role_policy_attachment" "permissions_avp" { policy_arn = aws_iam_policy.nullplatform_avp_policy.arn } +################################################################################ +# Additional permissions roles assumed by the agent role +################################################################################ + +# Extra permissions roles created on demand via var.permissions_roles. Each one +# trusts only the agent role and gets the provided policy ARNs attached. The +# agent role's assume policy is extended with all of these role ARNs. +resource "aws_iam_role" "extra_permissions" { + for_each = var.permissions_roles + + name = local.extra_permissions_role_names[each.key] + description = "Additional permissions role assumed by the nullplatform agent role" + + assume_role_policy = jsonencode({ + Version = "2012-10-17" + Statement = [{ + Effect = "Allow" + Principal = { AWS = local.agent_role_arn } + Action = "sts:AssumeRole" + }] + }) +} + +resource "aws_iam_role_policy_attachment" "extra_permissions" { + for_each = local.extra_permissions_attachments + + role = aws_iam_role.extra_permissions[each.value.role_key].name + policy_arn = each.value.arn +} + ################################################################################ # Route 53 IAM policy ################################################################################ @@ -195,9 +249,13 @@ resource "aws_iam_policy" "nullplatform_assume_role_policy" { policy = jsonencode({ Version = "2012-10-17" Statement = [{ - Effect = "Allow" - Action = "sts:AssumeRole" - Resource = concat([local.permissions_role_arn], var.assume_role_arns) + Effect = "Allow" + Action = "sts:AssumeRole" + Resource = concat( + [local.permissions_role_arn], + local.extra_permissions_role_arns, + var.assume_role_arns + ) }] }) } diff --git a/infrastructure/aws/iam/agent/outputs.tf b/infrastructure/aws/iam/agent/outputs.tf index 9a99fd61..67437bb5 100644 --- a/infrastructure/aws/iam/agent/outputs.tf +++ b/infrastructure/aws/iam/agent/outputs.tf @@ -7,3 +7,8 @@ output "nullplatform_agent_permissions_role_arn" { description = "ARN of the permissions role assumed by the agent role" value = aws_iam_role.nullplatform_agent_permissions.arn } + +output "nullplatform_agent_extra_permissions_role_arns" { + description = "Map of logical name to ARN for each additional permissions role created via permissions_roles" + value = { for k, r in aws_iam_role.extra_permissions : k => r.arn } +} diff --git a/infrastructure/aws/iam/agent/tests/agent.tftest.hcl b/infrastructure/aws/iam/agent/tests/agent.tftest.hcl index 23615d63..72793197 100644 --- a/infrastructure/aws/iam/agent/tests/agent.tftest.hcl +++ b/infrastructure/aws/iam/agent/tests/agent.tftest.hcl @@ -202,3 +202,64 @@ run "assume_role_policy_includes_additional_arns" { error_message = "assume_role policy should include additional assume_role_arns" } } + +run "extra_permissions_roles_not_created_by_default" { + command = plan + + assert { + condition = length(aws_iam_role.extra_permissions) == 0 + error_message = "No extra permissions roles should be created when permissions_roles is empty" + } +} + +run "extra_permissions_roles_created_and_assumable" { + command = plan + + variables { + permissions_roles = { + data = { + policy_arns = ["arn:aws:iam::aws:policy/AmazonS3ReadOnlyAccess"] + } + ops = { + name = "custom-ops-role" + policy_arns = ["arn:aws:iam::123456789012:policy/ops-policy"] + } + } + } + + assert { + condition = aws_iam_role.extra_permissions["data"].name == "nullplatform-test-cluster-data" + error_message = "Extra role should default its name to nullplatform-{cluster}-{key}" + } + + assert { + condition = aws_iam_role.extra_permissions["ops"].name == "custom-ops-role" + error_message = "Extra role should honor the name override" + } + + assert { + condition = jsondecode(aws_iam_role.extra_permissions["data"].assume_role_policy).Statement[0].Principal.AWS == "arn:aws:iam::123456789012:role/nullplatform-test-cluster-agent-role" + error_message = "Extra role trust policy should allow the agent role to assume it" + } + + assert { + condition = length(aws_iam_role_policy_attachment.extra_permissions) == 2 + error_message = "Each extra role policy_arn should produce one attachment" + } + + assert { + condition = contains( + jsondecode(aws_iam_policy.nullplatform_assume_role_policy.policy).Statement[0].Resource, + "arn:aws:iam::123456789012:role/nullplatform-test-cluster-data" + ) + error_message = "agent assume_role policy should include the data extra role" + } + + assert { + condition = contains( + jsondecode(aws_iam_policy.nullplatform_assume_role_policy.policy).Statement[0].Resource, + "arn:aws:iam::123456789012:role/custom-ops-role" + ) + error_message = "agent assume_role policy should include the ops extra role" + } +} diff --git a/infrastructure/aws/iam/agent/variables.tf b/infrastructure/aws/iam/agent/variables.tf index 4a1e704f..2cb93037 100644 --- a/infrastructure/aws/iam/agent/variables.tf +++ b/infrastructure/aws/iam/agent/variables.tf @@ -48,6 +48,20 @@ variable "permissions_role_name" { default = "" } +variable "permissions_roles" { + description = "Additional permissions roles created by this module and assumable by the agent role. Map key is a logical name; name overrides the role name (defaults to nullplatform-{cluster_name}-{key}); policy_arns are the policy ARNs attached to the role." + type = map(object({ + name = optional(string) + policy_arns = optional(list(string), []) + })) + default = {} + + validation { + condition = alltrue([for cfg in values(var.permissions_roles) : alltrue([for arn in cfg.policy_arns : can(regex("^arn:aws:iam::(aws|[0-9]{12}):policy/.+", arn))])]) + error_message = "Each policy_arn must match arn:aws:iam:::policy/" + } +} + variable "policies_name_prefix" { description = "Override for IAM policy name prefix. Defaults to nullplatform_{cluster_name}" type = string From d5606245f103877ec2eb7a8a635c76cd22f8bb70 Mon Sep 17 00:00:00 2001 From: David Fernandez Date: Tue, 23 Jun 2026 18:37:32 -0300 Subject: [PATCH 5/5] refactor(iam/agent): move permissions role to k8s scope tofu module The default permissions role (nullplatform_agent_permissions) and its workload policies (Route53, ELB, EKS, AVP) are no longer created by this module. They are now provisioned per-cluster by the k8s scope's OpenTofu module (scopes: k8s/scope/tofu/iam/modules). This module keeps only the agent IRSA role and its sts:AssumeRole policy, which still authorizes assuming the permissions role by its conventional ARN (nullplatform-{cluster_name}-agent-permissions-role). The output now returns that computed ARN instead of a created resource. Removed: nullplatform_agent_permissions role, its 4 policy attachments, the 4 workload policies, and the now-unused aws_region data source. Tests and README updated accordingly. NOTE: existing consumers will destroy the permissions role on next apply; the scope module must create the replacement under the same conventional name. Co-Authored-By: Claude Opus 4.8 (1M context) --- infrastructure/aws/iam/agent/README.md | 32 ++-- infrastructure/aws/iam/agent/data.tf | 3 - infrastructure/aws/iam/agent/main.tf | 176 +----------------- infrastructure/aws/iam/agent/outputs.tf | 4 +- .../aws/iam/agent/tests/agent.tftest.hcl | 137 -------------- 5 files changed, 20 insertions(+), 332 deletions(-) diff --git a/infrastructure/aws/iam/agent/README.md b/infrastructure/aws/iam/agent/README.md index ee3206dc..a2009f66 100644 --- a/infrastructure/aws/iam/agent/README.md +++ b/infrastructure/aws/iam/agent/README.md @@ -2,21 +2,19 @@ ## Description -Creates an IRSA-enabled IAM agent role for the nullplatform Kubernetes service account on EKS, using privilege separation: the agent role only carries an sts:AssumeRole policy and assumes a separate permissions role that holds the scoped workload policies +Creates an IRSA-enabled IAM agent role for the nullplatform Kubernetes service account on EKS, using privilege separation: the agent role only carries an sts:AssumeRole policy and assumes a separate permissions role (provisioned outside this module) that holds the scoped workload policies ## Architecture -The module uses the terraform-aws-modules/iam//modules/iam-role-for-service-accounts submodule to create an aws_iam_role (the agent role) with an OIDC trust policy bound to a specific Kubernetes namespace and service account. The agent role only carries an sts:AssumeRole policy that allows it to assume a separate permissions role (and any additional assume_role_arns). The permissions role is a standalone aws_iam_role whose trust policy allows only the agent role to assume it, and the four aws_iam_policy resources (Route53, ELB, EKS, and Amazon Verified Permissions) are attached to it. ARNs are derived from role names and the caller account id to avoid a circular dependency between the two roles. Both role ARNs are exposed as outputs. +The module uses the terraform-aws-modules/iam//modules/iam-role-for-service-accounts submodule to create an aws_iam_role (the agent role) with an OIDC trust policy bound to a specific Kubernetes namespace and service account. The agent role only carries an sts:AssumeRole policy that allows it to assume a permissions role (and any additional assume_role_arns). + +The default permissions role and its workload policies (Route53, ELB, EKS, AVP) are **no longer created by this module**: they are provisioned per-cluster by the k8s scope's OpenTofu module (`k8s/scope/tofu/iam/modules` in the scopes repo). This module still authorizes assuming that role by its conventional ARN (`nullplatform-{cluster_name}-agent-permissions-role`), derived from the role name and the caller account id, and exposes that ARN as an output. The scope module must create the permissions role with that same conventional name so the wiring matches. ## Features - Creates an IRSA IAM agent role scoped to a specific Kubernetes namespace and service account via OIDC provider trust -- Keeps the agent role minimal: it only carries an sts:AssumeRole policy targeting the permissions role and any additional assume_role_arns -- Creates a separate permissions role that trusts only the agent role and holds the workload policies -- Attaches a Route53 policy granting DNS record management permissions for hosted zones to the permissions role -- Attaches an ELB policy granting describe permissions for load balancers and target groups to the permissions role -- Attaches an EKS policy granting read access to clusters, node groups, and addons to the permissions role -- Attaches an Amazon Verified Permissions (AVP) policy granting full verifiedpermissions access to the permissions role +- Keeps the agent role minimal: it only carries an sts:AssumeRole policy targeting the (externally-created) permissions role and any additional assume_role_arns +- Authorizes assuming the conventional permissions role ARN even though the role itself is created elsewhere (k8s scope tofu module) - Supports attaching additional custom IAM policies to the agent role via the additional_policies map - Supports creating additional permissions roles via the permissions_roles map, each trusting the agent role and assumable by it @@ -34,8 +32,10 @@ module "agent" { ## Multiple permissions roles -The default permissions role (Route53/EKS/ELB/AVP) is always created. To have the -agent assume additional, module-created roles with their own policies, use the +The agent is always allowed to assume the default permissions role by its +conventional ARN (`nullplatform-{cluster_name}-agent-permissions-role`), which is +created externally by the k8s scope tofu module. To have the agent assume +additional, module-created roles with their own policies, use the `permissions_roles` map. Each entry creates a role that trusts the agent role and gets the given policy ARNs attached; the agent's assume policy is extended with all of them. @@ -92,19 +92,9 @@ resource "example_resource" "this" { | Name | Type | |------|------| | [aws_iam_policy.nullplatform_assume_role_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource | -| [aws_iam_policy.nullplatform_avp_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource | -| [aws_iam_policy.nullplatform_eks_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource | -| [aws_iam_policy.nullplatform_elb_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource | -| [aws_iam_policy.nullplatform_route53_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource | | [aws_iam_role.extra_permissions](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role) | resource | -| [aws_iam_role.nullplatform_agent_permissions](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role) | resource | | [aws_iam_role_policy_attachment.extra_permissions](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource | -| [aws_iam_role_policy_attachment.permissions_avp](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource | -| [aws_iam_role_policy_attachment.permissions_eks](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource | -| [aws_iam_role_policy_attachment.permissions_elb](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource | -| [aws_iam_role_policy_attachment.permissions_route53](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource | | [aws_caller_identity.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/caller_identity) | data source | -| [aws_region.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/region) | data source | ## Inputs @@ -126,7 +116,7 @@ resource "example_resource" "this" { | Name | Description | |------|-------------| | [nullplatform\_agent\_extra\_permissions\_role\_arns](#output\_nullplatform\_agent\_extra\_permissions\_role\_arns) | Map of logical name to ARN for each additional permissions role created via permissions\_roles | -| [nullplatform\_agent\_permissions\_role\_arn](#output\_nullplatform\_agent\_permissions\_role\_arn) | ARN of the permissions role assumed by the agent role | +| [nullplatform\_agent\_permissions\_role\_arn](#output\_nullplatform\_agent\_permissions\_role\_arn) | Conventional ARN of the permissions role the agent role is allowed to assume. The role itself is created externally (k8s scope tofu module), not by this module. | | [nullplatform\_agent\_role\_arn](#output\_nullplatform\_agent\_role\_arn) | ARN of the agent role | diff --git a/infrastructure/aws/iam/agent/data.tf b/infrastructure/aws/iam/agent/data.tf index 3d8e9e84..b9215b20 100644 --- a/infrastructure/aws/iam/agent/data.tf +++ b/infrastructure/aws/iam/agent/data.tf @@ -1,5 +1,2 @@ -data "aws_region" "current" { -} - data "aws_caller_identity" "current" { } diff --git a/infrastructure/aws/iam/agent/main.tf b/infrastructure/aws/iam/agent/main.tf index e9033bf5..3758c3b6 100644 --- a/infrastructure/aws/iam/agent/main.tf +++ b/infrastructure/aws/iam/agent/main.tf @@ -62,46 +62,13 @@ module "nullplatform_agent_role" { ) } -################################################################################ -# IAM permissions role assumed by the agent role -################################################################################ - -# Holds the actual workload policies (Route53, EKS, ELB, AVP). Trusts only the -# agent role, so the IRSA token cannot use these permissions without first -# assuming this role. -resource "aws_iam_role" "nullplatform_agent_permissions" { - name = local.permissions_role_name - description = "Permissions role assumed by the nullplatform agent role" - - assume_role_policy = jsonencode({ - Version = "2012-10-17" - Statement = [{ - Effect = "Allow" - Principal = { AWS = local.agent_role_arn } - Action = "sts:AssumeRole" - }] - }) -} - -resource "aws_iam_role_policy_attachment" "permissions_route53" { - role = aws_iam_role.nullplatform_agent_permissions.name - policy_arn = aws_iam_policy.nullplatform_route53_policy.arn -} - -resource "aws_iam_role_policy_attachment" "permissions_eks" { - role = aws_iam_role.nullplatform_agent_permissions.name - policy_arn = aws_iam_policy.nullplatform_eks_policy.arn -} - -resource "aws_iam_role_policy_attachment" "permissions_elb" { - role = aws_iam_role.nullplatform_agent_permissions.name - policy_arn = aws_iam_policy.nullplatform_elb_policy.arn -} - -resource "aws_iam_role_policy_attachment" "permissions_avp" { - role = aws_iam_role.nullplatform_agent_permissions.name - policy_arn = aws_iam_policy.nullplatform_avp_policy.arn -} +# NOTE: The permissions role (nullplatform_agent_permissions) and its workload +# policies (Route53, EKS, ELB, AVP) are no longer created by this module. They +# are now provisioned per-cluster by the k8s scope's OpenTofu module +# (scopes: k8s/scope/tofu/iam/modules). This module keeps only the agent IRSA +# role and an assume policy that authorizes assuming that externally-created +# permissions role by its conventional ARN (see nullplatform_assume_role_policy +# and local.permissions_role_arn). ################################################################################ # Additional permissions roles assumed by the agent role @@ -133,112 +100,6 @@ resource "aws_iam_role_policy_attachment" "extra_permissions" { policy_arn = each.value.arn } -################################################################################ -# Route 53 IAM policy -################################################################################ - -# Grant permissions to manage Route 53 DNS records for service discovery -resource "aws_iam_policy" "nullplatform_route53_policy" { - name = "${local.policies_name_prefix}_route53_policy" - description = "Policy for managing Route 53 DNS records" - policy = jsonencode({ - "Version" : "2012-10-17", - "Statement" : [ - { - "Effect" : "Allow", - "Action" : [ - "route53:ChangeResourceRecordSets", - "route53:ListResourceRecordSets", - "route53:GetHostedZone", - "route53:ListHostedZones", - "route53:ListHostedZonesByName" - ], - "Resource" : [ - "arn:aws:route53:::hostedzone/*" - ], - - } - ] - }) -} - -################################################################################ -# Elastic Load Balancing (ELB) IAM policy -################################################################################ - -# Grant permissions to describe and monitor load balancers and target groups -resource "aws_iam_policy" "nullplatform_elb_policy" { - name = "${local.policies_name_prefix}_elb_policy" - description = "Policy for managing Elastic Load Balancing resources" - policy = jsonencode( - { - "Version" : "2012-10-17", - "Statement" : [ - { - "Effect" : "Allow", - "Action" : [ - "elasticloadbalancing:DescribeLoadBalancers", - "elasticloadbalancing:DescribeTargetGroups" - ], - "Resource" : "*", - "Condition" : { - "StringEquals" : { - "aws:RequestedRegion" : [ - data.aws_region.current.region - ] - } - } - }, - { - "Effect" : "Allow", - "Action" : [ - "elasticloadbalancing:DescribeTargetHealth", - "elasticloadbalancing:DescribeListeners", - "elasticloadbalancing:DescribeRules" - ], - "Resource" : [ - "arn:aws:elasticloadbalancing:*:*:loadbalancer/app/k8s-nullplatform-*", - "arn:aws:elasticloadbalancing:*:*:targetgroup/k8s-nullplatform-*" - ], - - } - ] - } - ) -} - -################################################################################ -# EKS IAM policy -################################################################################ - -# Grant permissions to describe and list EKS cluster resources -resource "aws_iam_policy" "nullplatform_eks_policy" { - name = "${local.policies_name_prefix}_eks_policy" - description = "Policy for managing EKS cluster resources" - policy = jsonencode({ - "Version" : "2012-10-17", - "Statement" : [ - { - "Effect" : "Allow", - "Action" : [ - "eks:DescribeCluster", - "eks:ListClusters", - "eks:DescribeNodegroup", - "eks:ListNodegroups", - "eks:DescribeAddon", - "eks:ListAddons" - ], - "Resource" : [ - "arn:aws:eks:*:*:cluster/*", - "arn:aws:eks:*:*:nodegroup/*", - "arn:aws:eks:*:*:addon/*" - ], - - } - ] - }) -} - ################################################################################ # STS AssumeRole IAM policy ################################################################################ @@ -266,26 +127,3 @@ moved { from = aws_iam_policy.nullplatform_assume_role_policy[0] to = aws_iam_policy.nullplatform_assume_role_policy } - -################################################################################ -# AVP policy -################################################################################ - -# Grant permissions to describe and list EKS cluster resources -resource "aws_iam_policy" "nullplatform_avp_policy" { - name = "${local.policies_name_prefix}_avp_policy" - description = "Policy for managing AVP resources" - policy = jsonencode({ - "Version" : "2012-10-17", - "Statement" : [ - { - "Effect" : "Allow", - "Action" : [ - "verifiedpermissions:*" - ], - "Resource" : "*", - - } - ] - }) -} diff --git a/infrastructure/aws/iam/agent/outputs.tf b/infrastructure/aws/iam/agent/outputs.tf index 67437bb5..b2859597 100644 --- a/infrastructure/aws/iam/agent/outputs.tf +++ b/infrastructure/aws/iam/agent/outputs.tf @@ -4,8 +4,8 @@ output "nullplatform_agent_role_arn" { } output "nullplatform_agent_permissions_role_arn" { - description = "ARN of the permissions role assumed by the agent role" - value = aws_iam_role.nullplatform_agent_permissions.arn + description = "Conventional ARN of the permissions role the agent role is allowed to assume. The role itself is created externally (k8s scope tofu module), not by this module." + value = local.permissions_role_arn } output "nullplatform_agent_extra_permissions_role_arns" { diff --git a/infrastructure/aws/iam/agent/tests/agent.tftest.hcl b/infrastructure/aws/iam/agent/tests/agent.tftest.hcl index 72793197..f78cbf70 100644 --- a/infrastructure/aws/iam/agent/tests/agent.tftest.hcl +++ b/infrastructure/aws/iam/agent/tests/agent.tftest.hcl @@ -1,10 +1,4 @@ mock_provider "aws" { - override_data { - target = data.aws_region.current - values = { - region = "us-east-1" - } - } override_data { target = data.aws_caller_identity.current values = { @@ -17,30 +11,6 @@ mock_provider "aws" { json = "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Principal\":{\"Federated\":\"test\"},\"Action\":\"sts:AssumeRoleWithWebIdentity\"}]}" } } - override_resource { - target = aws_iam_policy.nullplatform_route53_policy - values = { - arn = "arn:aws:iam::123456789012:policy/nullplatform_test-cluster_route53_policy" - } - } - override_resource { - target = aws_iam_policy.nullplatform_elb_policy - values = { - arn = "arn:aws:iam::123456789012:policy/nullplatform_test-cluster_elb_policy" - } - } - override_resource { - target = aws_iam_policy.nullplatform_eks_policy - values = { - arn = "arn:aws:iam::123456789012:policy/nullplatform_test-cluster_eks_policy" - } - } - override_resource { - target = aws_iam_policy.nullplatform_avp_policy - values = { - arn = "arn:aws:iam::123456789012:policy/nullplatform_test-cluster_avp_policy" - } - } override_resource { target = aws_iam_policy.nullplatform_assume_role_policy values = { @@ -55,113 +25,6 @@ variables { aws_iam_openid_connect_provider_arn = "arn:aws:iam::123456789012:oidc-provider/oidc.eks.us-east-1.amazonaws.com/id/EXAMPLE" } -run "route53_policy_naming" { - command = plan - - assert { - condition = aws_iam_policy.nullplatform_route53_policy.name == "nullplatform_test-cluster_route53_policy" - error_message = "Route53 policy name should follow naming convention" - } -} - -run "elb_policy_naming" { - command = plan - - assert { - condition = aws_iam_policy.nullplatform_elb_policy.name == "nullplatform_test-cluster_elb_policy" - error_message = "ELB policy name should follow naming convention" - } -} - -run "eks_policy_naming" { - command = plan - - assert { - condition = aws_iam_policy.nullplatform_eks_policy.name == "nullplatform_test-cluster_eks_policy" - error_message = "EKS policy name should follow naming convention" - } -} - -run "avp_policy_naming" { - command = plan - - assert { - condition = aws_iam_policy.nullplatform_avp_policy.name == "nullplatform_test-cluster_avp_policy" - error_message = "AVP policy name should follow naming convention" - } -} - -run "all_policies_valid_json" { - command = plan - - assert { - condition = can(jsondecode(aws_iam_policy.nullplatform_route53_policy.policy)) - error_message = "Route53 policy should be valid JSON" - } - - assert { - condition = can(jsondecode(aws_iam_policy.nullplatform_elb_policy.policy)) - error_message = "ELB policy should be valid JSON" - } - - assert { - condition = can(jsondecode(aws_iam_policy.nullplatform_eks_policy.policy)) - error_message = "EKS policy should be valid JSON" - } - - assert { - condition = can(jsondecode(aws_iam_policy.nullplatform_avp_policy.policy)) - error_message = "AVP policy should be valid JSON" - } -} - -run "permissions_role_naming" { - command = plan - - assert { - condition = aws_iam_role.nullplatform_agent_permissions.name == "nullplatform-test-cluster-agent-permissions-role" - error_message = "Permissions role name should follow naming convention" - } -} - -run "permissions_role_trusts_agent_role" { - command = plan - - assert { - condition = jsondecode(aws_iam_role.nullplatform_agent_permissions.assume_role_policy).Statement[0].Principal.AWS == "arn:aws:iam::123456789012:role/nullplatform-test-cluster-agent-role" - error_message = "Permissions role trust policy should allow the agent role to assume it" - } -} - -run "permissions_role_has_workload_policies_attached" { - command = plan - - assert { - condition = aws_iam_role_policy_attachment.permissions_route53.policy_arn == aws_iam_policy.nullplatform_route53_policy.arn - error_message = "Route53 policy should be attached to the permissions role" - } - - assert { - condition = aws_iam_role_policy_attachment.permissions_eks.policy_arn == aws_iam_policy.nullplatform_eks_policy.arn - error_message = "EKS policy should be attached to the permissions role" - } - - assert { - condition = aws_iam_role_policy_attachment.permissions_elb.policy_arn == aws_iam_policy.nullplatform_elb_policy.arn - error_message = "ELB policy should be attached to the permissions role" - } - - assert { - condition = aws_iam_role_policy_attachment.permissions_avp.policy_arn == aws_iam_policy.nullplatform_avp_policy.arn - error_message = "AVP policy should be attached to the permissions role" - } - - assert { - condition = aws_iam_role_policy_attachment.permissions_route53.role == aws_iam_role.nullplatform_agent_permissions.name - error_message = "Attachments should target the permissions role" - } -} - run "assume_role_policy_always_created_and_targets_permissions_role" { command = plan