search

terraform-aws-modules / terraform-aws-iam

Terraform module to create AWS IAM resources 🇺🇦
https://registry.terraform.io/modules/terraform-aws-modules/iam/aws
Apache License 2.0
790 stars 1.01k forks source link

self_assume_role var.role_name is null #282

Closed FernandoMiguel closed 2 years ago

FernandoMiguel commented 2 years ago

Description

Please provide a clear and concise description of the issue you are encountering, and a reproduction of your configuration (see the examples/* directory for references that you can copy+paste and tailor to match your configs if you are unable to copy your exact configuration). The reproduction MUST be executable by running terraform init && terraform apply without any further changes.

If your request is for a new feature, please use the Feature request template.

⚠️ Note

Versions

Reproduction Code [Required]

module "argocd_iam_role_sa_cross_cluster" {
  source  = "terraform-aws-modules/iam/aws//modules/iam-role-for-service-accounts-eks"
  version = "~> v5.5"

  role_name_prefix       = substr("${var.addon_context.eks_cluster_id}-argocd-sts-", 0, 38)
  max_session_duration   = "3600"
  force_detach_policies  = true
  allow_self_assume_role = true

  oidc_providers = {
    argocd = {
      provider_arn = var.addon_context.eks_oidc_provider_arn
      namespace_service_accounts = [
        "${var.namespace}:argocd-application-controller",
        "${var.namespace}:argocd-server",
      ]
    }
  }
}

Expected behavior

for a role to be created

Actual behavior

│ Error: Invalid template interpolation value
│
│   on .terraform/modules/base_system.argocd.argocd_iam_role_sa_cross_cluster/modules/iam-role-for-service-accounts-eks/main.tf line 24, in data "aws_iam_policy_document" "this":
│   24:         identifiers = ["arn:${local.partition}:iam::${local.account_id}:role${var.role_path}${var.role_name}"]
│     ├────────────────
│     │ var.role_path is null
│
│ The expression result is null. Cannot include a null value in a string template.
╵
╷
│ Error: Invalid template interpolation value
│
│   on .terraform/modules/base_system.argocd.argocd_iam_role_sa_cross_cluster/modules/iam-role-for-service-accounts-eks/main.tf line 24, in data "aws_iam_policy_document" "this":
│   24:         identifiers = ["arn:${local.partition}:iam::${local.account_id}:role${var.role_path}${var.role_name}"]
│     ├────────────────
│     │ var.role_name is null
│
│ The expression result is null. Cannot include a null value in a string template.
╵

Additional context

related to https://github.com/terraform-aws-modules/terraform-aws-iam/pull/281

cc @bryantbiggs

FernandoMiguel commented 2 years ago

modified the module a bit

module "argocd_iam_role_sa_cross_cluster" {
  source  = "terraform-aws-modules/iam/aws//modules/iam-role-for-service-accounts-eks"
  version = "~> v5.5"

  # role_name_prefix       = substr("${var.addon_context.eks_cluster_id}-argocd-sts-", 0, 38)
  role_name              = substr(random_id.suffix.hex, 0, 64)
  max_session_duration   = "3600"
  force_detach_policies  = true
  allow_self_assume_role = true
  role_path              = "/"

  oidc_providers = {
    argocd = {
      provider_arn = var.addon_context.eks_oidc_provider_arn
      namespace_service_accounts = [
        "${var.namespace}:argocd-application-controller",
        "${var.namespace}:argocd-server",
      ]
    }
  }
}

used role_name instead of role_name_prefix added role_path (shouldn't "/" be the default in the variables.tf ? )

this module generated the following policy

"argocd_iam_role_sa_cross_cluster" = {
"data_aws_iam_policy_document" = [
    {
    "id" = "3350686316"
    "json" = <<-EOT
    {
        "Version": "2012-10-17",
        "Statement": [
        {
            "Sid": "ExplicitSelfRoleAssumption",
            "Effect": "Allow",
            "Action": "sts:AssumeRole",
            "Principal": {
            "AWS": "arn:aws:iam::XXXX:role/argocd-sts-c03686ae5f06138e"
            }
        },
        {
            "Sid": "",
            "Effect": "Allow",
            "Action": "sts:AssumeRoleWithWebIdentity",
            "Principal": {
            "Federated": "arn:aws:iam::XXXX:oidc-provider/oidc.eks.us-east-1.amazonaws.com/id/XXXX"
            },
            "Condition": {
            "StringEquals": {
                "oidc.eks.us-east-1.amazonaws.com/id/XXXX:aud": "sts.amazonaws.com",
                "oidc.eks.us-east-1.amazonaws.com/id/XXXX:sub": [
                "system:serviceaccount:argocd-system:argocd-application-controller",
                "system:serviceaccount:argocd-system:argocd-server"
                ]
            }
            }
        }
        ]
    }
    EOT
    "override_json" = tostring(null)
    "override_policy_documents" = tolist(null) /* of string */
    "policy_id" = tostring(null)
    "source_json" = tostring(null)
    "source_policy_documents" = tolist(null) /* of string */
    "statement" = tolist([
        {
        "actions" = toset([
            "sts:AssumeRole",
        ])
        "condition" = toset([])
        "effect" = "Allow"
        "not_actions" = toset([])
        "not_principals" = toset([])
        "not_resources" = toset([])
        "principals" = toset([
            {
            "identifiers" = toset([
                "arn:aws:iam::XXXX:role/argocd-sts-c03686ae5f06138e",
            ])
            "type" = "AWS"
            },
        ])
        "resources" = toset([])
        "sid" = "ExplicitSelfRoleAssumption"
        },
        {
        "actions" = toset([
            "sts:AssumeRoleWithWebIdentity",
        ])
        "condition" = toset([
            {
            "test" = "StringEquals"
            "values" = tolist([
                "sts.amazonaws.com",
            ])
            "variable" = "oidc.eks.us-east-1.amazonaws.com/id/XXXX:aud"
            },
            {
            "test" = "StringEquals"
            "values" = tolist([
                "system:serviceaccount:argocd-system:argocd-application-controller",
                "system:serviceaccount:argocd-system:argocd-server",
            ])
            "variable" = "oidc.eks.us-east-1.amazonaws.com/id/XXXX:sub"
            },
        ])
        "effect" = "Allow"
        "not_actions" = toset([])
        "not_principals" = toset([])
        "not_resources" = toset([])
        "principals" = toset([
            {
            "identifiers" = toset([
                "arn:aws:iam::XXXX:oidc-provider/oidc.eks.us-east-1.amazonaws.com/id/XXXX",
            ])
            "type" = "Federated"
            },
        ])
        "resources" = toset([])
        "sid" = ""
        },
    ])
    "version" = "2012-10-17"
    },
]
}

but the errored out with

│ Error: failed creating IAM Role (fernando-guiding-stud-argocd-sts-c03686ae5f06138e): MalformedPolicyDocument: Invalid principal in policy: "AWS":"arn:aws:iam::XXX:role/argocd-sts-c03686ae5f06138e"
│   status code: 400, request id: 40f427f6-4a21-4b6c-992a-f60a5cb90e91
│
│   with module.base_system.module.argocd[0].module.argocd_iam_role_sa_cross_cluster.aws_iam_role.this[0],
│   on .terraform/modules/base_system.argocd.argocd_iam_role_sa_cross_cluster/modules/iam-role-for-service-accounts-eks/main.tf line 59, in resource "aws_iam_role" "this":
│   59: resource "aws_iam_role" "this" {
FernandoMiguel commented 2 years ago

the policy does not take "AWS": "arn:aws:iam::XXXX:role/argocd-sts-c03686ae5f06138e" but is fine with "AWS": "arn:aws:iam::XXX:root"

FernandoMiguel commented 2 years ago

speaking with aws support, i'm told:

this is a known issue, because trust policies are evaluated upon creation using an automated engine the role was not created yet hence not mapped to a logical id the workaround is to create the role first then update the assume role policy

FernandoMiguel commented 2 years ago

guess we need to open a ticket with the aws provider to handle this logic change

FernandoMiguel commented 2 years ago

further discussion with aws support, this is a proposed solution.

        {
            "Sid": "ExplicitSelfRoleAssumption",
            "Effect": "Allow",
            "Principal": {
                "AWS": "*"
            },
            "Action": "sts:AssumeRole",
            "Condition": {
                "ArnLike": {
                    "aws:PrincipalArn": [
                        "arn:aws:iam::XXXX:role/argocd-sts-c03686ae5f06138e"
                    ]
                }
            }
        }

this one at least is valid. we are going to try to test this today in one of our EKS test clusters

FernandoMiguel commented 2 years ago

we confirmed that the conditional policy works for us.

antonbabenko commented 2 years ago

This issue has been resolved in version 5.5.2 :tada:

github-actions[bot] commented 2 years ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.