Require correct OIDC Audience value to assume role (#33)

README.md

@@ -167,15 +167,15 @@ Available targets:
 
 | Name | Version |
 |------|---------|
-| <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 0.13.0 |
-| <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 2.0 |
+| <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.0.0 |
+| <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 3.0 |
 | <a name="requirement_local"></a> [local](#requirement\_local) | >= 1.2 |
 
 ## Providers
 
 | Name | Version |
 |------|---------|
-| <a name="provider_aws"></a> [aws](#provider\_aws) | >= 2.0 |
+| <a name="provider_aws"></a> [aws](#provider\_aws) | >= 3.0 |
 
 ## Modules
 
@@ -218,10 +218,9 @@ Available targets:
 | <a name="input_namespace"></a> [namespace](#input\_namespace) | ID element. Usually an abbreviation of your organization name, e.g. 'eg' or 'cp', to help ensure generated IDs are globally unique | `string` | `null` | no |
 | <a name="input_permissions_boundary"></a> [permissions\_boundary](#input\_permissions\_boundary) | ARN of the policy that is used to set the permissions boundary for the role. | `string` | `null` | no |
 | <a name="input_regex_replace_chars"></a> [regex\_replace\_chars](#input\_regex\_replace\_chars) | Terraform regular expression (regex) string.<br>Characters matching the regex will be removed from the ID elements.<br>If not set, `"/[^a-zA-Z0-9-]/"` is used to remove all characters other than hyphens, letters and digits. | `string` | `null` | no |
-| <a name="input_service_account_list_qualifier"></a> [service\_account\_list\_qualifier](#input\_service\_account\_list\_qualifier) | Kubernetes ServiceAccount list qualifier, must be one of `ForAllValues` or `ForAnyValue` | `string` | `"ForAnyValue"` | no |
-| <a name="input_service_account_name"></a> [service\_account\_name](#input\_service\_account\_name) | Kubernetes ServiceAccount name | `string` | `null` | no |
-| <a name="input_service_account_namespace"></a> [service\_account\_namespace](#input\_service\_account\_namespace) | Kubernetes Namespace where service account is deployed | `string` | `null` | no |
-| <a name="input_service_account_namespace_name_list"></a> [service\_account\_namespace\_name\_list](#input\_service\_account\_namespace\_name\_list) | List of `namespace:name` for service account assume role IAM policy | `list(string)` | `null` | no |
+| <a name="input_service_account_name"></a> [service\_account\_name](#input\_service\_account\_name) | Kubernetes ServiceAccount name.<br>Leave empty or set to "*" to indicate all Service Accounts, or if using `service_account_namespace_name_list`. | `string` | `null` | no |
+| <a name="input_service_account_namespace"></a> [service\_account\_namespace](#input\_service\_account\_namespace) | Kubernetes Namespace where service account is deployed. Leave empty or set to "*" to indicate all Namespaces,<br>or if using `service_account_namespace_name_list`. | `string` | `null` | no |
+| <a name="input_service_account_namespace_name_list"></a> [service\_account\_namespace\_name\_list](#input\_service\_account\_namespace\_name\_list) | List of `namespace:name` for service account assume role IAM policy if you need more than one. May include wildcards. | `list(string)` | `[]` | no |
 | <a name="input_stage"></a> [stage](#input\_stage) | ID element. Usually used to indicate role, e.g. 'prod', 'staging', 'source', 'build', 'test', 'deploy', 'release' | `string` | `null` | no |
 | <a name="input_tags"></a> [tags](#input\_tags) | Additional tags (e.g. `{'BusinessUnit': 'XYZ'}`).<br>Neither the tag keys nor the tag values will be modified by this module. | `map(string)` | `{}` | no |
 | <a name="input_tenant"></a> [tenant](#input\_tenant) | ID element \_(Rarely used, not included by default)\_. A customer identifier, indicating who this instance of a resource is for | `string` | `null` | no |

docs/terraform.md

@@ -3,15 +3,15 @@
 
 | Name | Version |
 |------|---------|
-| <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 0.13.0 |
-| <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 2.0 |
+| <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.0.0 |
+| <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 3.0 |
 | <a name="requirement_local"></a> [local](#requirement\_local) | >= 1.2 |
 
 ## Providers
 
 | Name | Version |
 |------|---------|
-| <a name="provider_aws"></a> [aws](#provider\_aws) | >= 2.0 |
+| <a name="provider_aws"></a> [aws](#provider\_aws) | >= 3.0 |
 
 ## Modules
 
@@ -54,10 +54,9 @@
 | <a name="input_namespace"></a> [namespace](#input\_namespace) | ID element. Usually an abbreviation of your organization name, e.g. 'eg' or 'cp', to help ensure generated IDs are globally unique | `string` | `null` | no |
 | <a name="input_permissions_boundary"></a> [permissions\_boundary](#input\_permissions\_boundary) | ARN of the policy that is used to set the permissions boundary for the role. | `string` | `null` | no |
 | <a name="input_regex_replace_chars"></a> [regex\_replace\_chars](#input\_regex\_replace\_chars) | Terraform regular expression (regex) string.<br>Characters matching the regex will be removed from the ID elements.<br>If not set, `"/[^a-zA-Z0-9-]/"` is used to remove all characters other than hyphens, letters and digits. | `string` | `null` | no |
-| <a name="input_service_account_list_qualifier"></a> [service\_account\_list\_qualifier](#input\_service\_account\_list\_qualifier) | Kubernetes ServiceAccount list qualifier, must be one of `ForAllValues` or `ForAnyValue` | `string` | `"ForAnyValue"` | no |
-| <a name="input_service_account_name"></a> [service\_account\_name](#input\_service\_account\_name) | Kubernetes ServiceAccount name | `string` | `null` | no |
-| <a name="input_service_account_namespace"></a> [service\_account\_namespace](#input\_service\_account\_namespace) | Kubernetes Namespace where service account is deployed | `string` | `null` | no |
-| <a name="input_service_account_namespace_name_list"></a> [service\_account\_namespace\_name\_list](#input\_service\_account\_namespace\_name\_list) | List of `namespace:name` for service account assume role IAM policy | `list(string)` | `null` | no |
+| <a name="input_service_account_name"></a> [service\_account\_name](#input\_service\_account\_name) | Kubernetes ServiceAccount name.<br>Leave empty or set to "*" to indicate all Service Accounts, or if using `service_account_namespace_name_list`. | `string` | `null` | no |
+| <a name="input_service_account_namespace"></a> [service\_account\_namespace](#input\_service\_account\_namespace) | Kubernetes Namespace where service account is deployed. Leave empty or set to "*" to indicate all Namespaces,<br>or if using `service_account_namespace_name_list`. | `string` | `null` | no |
+| <a name="input_service_account_namespace_name_list"></a> [service\_account\_namespace\_name\_list](#input\_service\_account\_namespace\_name\_list) | List of `namespace:name` for service account assume role IAM policy if you need more than one. May include wildcards. | `list(string)` | `[]` | no |
 | <a name="input_stage"></a> [stage](#input\_stage) | ID element. Usually used to indicate role, e.g. 'prod', 'staging', 'source', 'build', 'test', 'deploy', 'release' | `string` | `null` | no |
 | <a name="input_tags"></a> [tags](#input\_tags) | Additional tags (e.g. `{'BusinessUnit': 'XYZ'}`).<br>Neither the tag keys nor the tag values will be modified by this module. | `map(string)` | `{}` | no |
 | <a name="input_tenant"></a> [tenant](#input\_tenant) | ID element \_(Rarely used, not included by default)\_. A customer identifier, indicating who this instance of a resource is for | `string` | `null` | no |

examples/complete/main.tf

@@ -2,7 +2,13 @@ provider "aws" {
   region = var.region
 }
 
-data "aws_caller_identity" "current" {}
+locals {
+  enabled = module.this.enabled
+}
+
+data "aws_caller_identity" "current" {
+  count = local.enabled ? 1 : 0
+}
 
 module "autoscaler_role" {
   source = "../.."
@@ -11,39 +17,37 @@ module "autoscaler_role" {
   service_account_name      = "autoscaler"
   service_account_namespace = "kube-system"
 
-  aws_account_number = data.aws_caller_identity.current.account_id
+  aws_account_number = one(data.aws_caller_identity.current[*].account_id)
   # Rather than create a whole cluster, just fake the OIDC URL
   # eks_cluster_oidc_issuer_url = module.eks_cluster.eks_cluster_identity_oidc_issuer
   eks_cluster_oidc_issuer_url = "https://oidc.eks.us-west-2.amazonaws.com/id/FEDCBA9876543210FEDCBA9876543210"
-  aws_iam_policy_document     = [data.aws_iam_policy_document.autoscaler.json]
+  aws_iam_policy_document     = [one(data.aws_iam_policy_document.autoscaler[*].json)]
 
   context = module.this.context
 }
 
-module "autoscaler_role_multiple_service_accounts" {
-  source     = "../.."
-  attributes = ["multiple", "sa"]
-
-  # Usually there is no need to add both service account methods of attachment.
-  # If you add both, they are joined via AND.
-  # See https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_multi-value-conditions.html#reference_policies_multiple-conditions-eval
+module "multiple_service_accounts_short" {
+  source = "../.."
 
+  # Test the rare case multiple service accounts are attached to the same role
   # Multiple Service Account attachments
   service_account_namespace_name_list = [
-    "kube-system:autoscaler",
-    "default:foo",
+    "app:app",
+    "pr-*:app",
   ]
-  service_account_list_qualifier = "ForAnyValue"
 
-  aws_account_number = data.aws_caller_identity.current.account_id
+  aws_account_number = one(data.aws_caller_identity.current[*].account_id)
   # Rather than create a whole cluster, just fake the OIDC URL
   # eks_cluster_oidc_issuer_url = module.eks_cluster.eks_cluster_identity_oidc_issuer
   eks_cluster_oidc_issuer_url = "https://oidc.eks.us-west-2.amazonaws.com/id/FEDCBA9876543210FEDCBA9876543210"
-  aws_iam_policy_document     = [data.aws_iam_policy_document.autoscaler.json]
+  aws_iam_policy_document     = [one(data.aws_iam_policy_document.autoscaler[*].json)]
 
   context = module.this.context
 }
 data "aws_iam_policy_document" "autoscaler" {
+  #bridgecrew:skip=BC_AWS_IAM_57:Skipping `Ensure IAM policies does not allow write access without constraint` because this is a test case
+  count = local.enabled ? 1 : 0
+
   statement {
     sid = "AllowToScaleEKSNodeGroupAutoScalingGroup"
 
@@ -63,9 +67,30 @@ data "aws_iam_policy_document" "autoscaler" {
 }
 
 data "aws_iam_policy" "autoscaler" {
+  count = local.enabled ? 1 : 0
+
   arn = module.autoscaler_role.service_account_policy_arn
 }
 
+module "multiple_service_accounts_long" {
+  source = "../.."
+
+  # Test the rare case multiple service accounts are attached to the same role
+  # Multiple Service Account attachments
+  service_account_namespace_name_list = [
+    "very-long-namespace:even-longer-service-account-name",
+    "app:app",
+  ]
+
+  aws_account_number = one(data.aws_caller_identity.current[*].account_id)
+  # Rather than create a whole cluster, just fake the OIDC URL
+  # eks_cluster_oidc_issuer_url = module.eks_cluster.eks_cluster_identity_oidc_issuer
+  eks_cluster_oidc_issuer_url = "https://oidc.eks.us-west-2.amazonaws.com/id/FEDCBA9876543210FEDCBA9876543210"
+  aws_iam_policy_document     = [one(data.aws_iam_policy_document.autoscaler[*].json)]
+
+  context = module.this.context
+}
+
 module "cert-manager_role" {
   source = "../.."
 
@@ -74,16 +99,19 @@ module "cert-manager_role" {
   service_account_name      = "cert-manager"
   service_account_namespace = "cert-manager"
 
-  aws_account_number = data.aws_caller_identity.current.account_id
+  aws_account_number = one(data.aws_caller_identity.current[*].account_id)
   # Rather than create a whole cluster, just fake the OIDC URL
   # eks_cluster_oidc_issuer_url = module.eks_cluster.eks_cluster_identity_oidc_issuer
   eks_cluster_oidc_issuer_url = "https://oidc.eks.us-west-2.amazonaws.com/id/FEDCBA9876543210FEDCBA9876543210"
-  aws_iam_policy_document     = [data.aws_iam_policy_document.cert-manager.json]
+  aws_iam_policy_document     = [one(data.aws_iam_policy_document.cert-manager[*].json)]
 
   context = module.this.context
 }
 
 data "aws_iam_policy_document" "cert-manager" {
+  #bridgecrew:skip=BC_AWS_IAM_57:Skipping `Ensure IAM policies does not allow write access without constraint` because this is a test case
+  count = local.enabled ? 1 : 0
+
   statement {
     sid = "GrantListHostedZonesListResourceRecordSets"
 
@@ -97,5 +125,6 @@ data "aws_iam_policy_document" "cert-manager" {
 }
 
 data "aws_iam_policy" "cert-manager" {
-  arn = module.cert-manager_role.service_account_policy_arn
+  count = local.enabled ? 1 : 0
+  arn   = module.cert-manager_role.service_account_policy_arn
 }

examples/complete/outputs.tf

@@ -1,15 +1,29 @@
 output "autoscaler_role" {
-  value = module.autoscaler_role.service_account_role_name
+  value       = module.autoscaler_role.service_account_role_name
+  description = "The name of the IAM role for the autoscaler service account"
 }
 
 output "autoscaler_policy" {
-  value = data.aws_iam_policy.autoscaler.policy
+  value       = one(data.aws_iam_policy.autoscaler[*].policy)
+  description = "The IAM policy for the autoscaler service account"
+}
+
+output "multiple_service_accounts_role_short" {
+  value       = module.multiple_service_accounts_short.service_account_role_name
+  description = "The name of the IAM role for the multiple_service_accounts_short test case"
+}
+
+output "multiple_service_accounts_role_long" {
+  value       = module.multiple_service_accounts_long.service_account_role_name
+  description = "The name of the IAM role for the multiple_service_accounts_long test case"
 }
 
 output "cert-manager_role" {
-  value = module.cert-manager_role.service_account_role_name
+  value       = module.cert-manager_role.service_account_role_name
+  description = "The name of the IAM role for the cert-manager service account"
 }
 
 output "cert-manager_policy" {
-  value = data.aws_iam_policy.cert-manager.policy
+  value       = one(data.aws_iam_policy.cert-manager[*].policy)
+  description = "The IAM policy for the cert-manager service account"
 }

examples/complete/versions.tf

@@ -1,10 +1,10 @@
 terraform {
-  required_version = ">= 0.13.0"
+  required_version = ">= 1.0.0"
 
   required_providers {
     aws = {
       source  = "hashicorp/aws"
-      version = ">= 2.0"
+      version = ">= 3.0"
     }
     local = {
       source  = "hashicorp/local"

main.tf

@@ -24,8 +24,16 @@ locals {
   # and the policy will use wildcards for both the namespace and the service account name in the test condition to allow all ServiceAccounts
   # in all Kubernetes namespaces to assume the IAM role (not recommended).
 
-  service_account_long_id = format("%v@%v", coalesce(var.service_account_name, "all"), coalesce(var.service_account_namespace, "all"))
-  service_account_id      = trimsuffix(local.service_account_long_id, format("@%v", var.service_account_name))
+
+  single_service_account = var.service_account_name == null && var.service_account_namespace == null && length(var.service_account_namespace_name_list) > 0 ? [] : [
+    format("%s:%s", coalesce(var.service_account_namespace, "*"), coalesce(var.service_account_name, "*"))
+  ]
+  service_account_namespace_name_list = concat(local.single_service_account, var.service_account_namespace_name_list)
+
+  role_name_service_account_name = replace(split(":", local.service_account_namespace_name_list[0])[1], "*", "all")
+  role_name_namespace            = replace(split(":", local.service_account_namespace_name_list[0])[0], "*", "all")
+  service_account_long_id        = format("%v@%v", local.role_name_service_account_name, local.role_name_namespace)
+  service_account_id             = trimsuffix(local.service_account_long_id, format("@%v", local.role_name_service_account_name))
 
   # Try to return the first element, if that doesn't work, try the tostring approach
   aws_iam_policy_document = try(var.aws_iam_policy_document[0], tostring(var.aws_iam_policy_document), "{}")
@@ -46,6 +54,7 @@ module "service_account_label" {
 
   # The standard module does not allow @ but we want it
   regex_replace_chars = "/[^-a-zA-Z0-9@_]/"
+  id_length_limit     = 64
 
   context = module.this.context
 }
@@ -74,24 +83,16 @@ data "aws_iam_policy_document" "service_account_assume_role" {
       identifiers = [format("arn:%s:iam::%s:oidc-provider/%s", var.aws_partition, local.aws_account_number, local.eks_cluster_oidc_issuer)]
     }
 
-    dynamic "condition" {
-      for_each = var.service_account_name != null && var.service_account_namespace != null ? ["true"] : []
-      content {
-        test = "StringLike"
-        values = [
-          format("system:serviceaccount:%s:%s", coalesce(var.service_account_namespace, "*"), coalesce(var.service_account_name, "*"))
-        ]
-        variable = format("%s:sub", local.eks_cluster_oidc_issuer)
-      }
-    }
+    condition {
+      test     = "StringLike"
+      values   = formatlist("system:serviceaccount:%s", local.service_account_namespace_name_list)
+      variable = format("%s:sub", local.eks_cluster_oidc_issuer)
 
-    dynamic "condition" {
-      for_each = var.service_account_namespace_name_list != null ? ["true"] : []
-      content {
-        test     = format("%s:StringLike", var.service_account_list_qualifier)
-        values   = formatlist("system:serviceaccount:%s", var.service_account_namespace_name_list)
-        variable = format("%s:sub", local.eks_cluster_oidc_issuer)
-      }
+    }
+    condition {
+      test     = "StringEquals"
+      values   = ["sts.amazonaws.com"]
+      variable = format("%s:aud", local.eks_cluster_oidc_issuer)
     }
   }
 }

test/Makefile

@@ -33,11 +33,11 @@ clean:
 all: module examples/complete
 
 ## Run basic sanity checks against the module itself
-module: export TESTS ?= installed lint get-modules module-pinning get-plugins provider-pinning validate terraform-docs input-descriptions output-descriptions
+module: export TESTS ?= installed lint module-pinning provider-pinning validate terraform-docs input-descriptions output-descriptions
 module: deps
 	$(call RUN_TESTS, ../)
 
 ## Run tests against example
-examples/complete: export TESTS ?= installed lint get-modules get-plugins validate
+examples/complete: export TESTS ?= installed lint validate
 examples/complete: deps
 	$(call RUN_TESTS, ../$@)

test/src/Makefile

@@ -1,9 +1,8 @@
-export TF_CLI_ARGS_init ?= -get-plugins=true
-export TERRAFORM_VERSION ?= $(shell curl -s https://checkpoint-api.hashicorp.com/v1/check/terraform | jq -r -M '.current_version' | cut -d. -f1-2)
+export TERRAFORM_VERSION ?= $(shell curl -s https://checkpoint-api.hashicorp.com/v1/check/terraform | jq -r -M '.current_version' | cut -d. -f1)
 
 .DEFAULT_GOAL : all
-.PHONY: all
 
+.PHONY: all
 ## Default target
 all: test
 
@@ -16,7 +15,7 @@ init:
 ## Run tests
 test: init
 	go mod download
-	go test -v -timeout 60m -run TestExamplesComplete
+	go test -v -timeout 60m
 
 ## Run tests in docker container
 docker/test: