From da5e8657bbbf19d53bbb6170009af694d0672122 Mon Sep 17 00:00:00 2001 From: rocket Date: Wed, 14 Aug 2024 15:40:32 -0700 Subject: [PATCH] Use existing identity provider in temporary environments (#717) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⚠️ Release notes: Admins that are updating their project to include this feature at the same time as the identity-provider feature and who set enable_identity_provider = true should be warned that the `ci-infra-service` github action will fail because it will attempt to connect to an existing identity provider when one doesn't yet exist for that environment. Mitigation strategies: - Ignore failing test and run `terraform plan` on the environment outside of CI to verify the update is working correctly - Temporarily manually create a user pool resource in the AWS Console with the expected identity_provider_name so that CI can connect to that. Delete the resource before merging --- .../env-config/identity-provider.tf | 2 +- infra/app/app-config/env-config/main.tf | 8 ++- infra/app/service/main.tf | 56 +++++++++++++++---- .../access-control.tf | 15 ----- .../resources/access-control.tf | 15 +++++ .../{ => resources}/main.tf | 2 +- .../{ => resources}/outputs.tf | 3 +- .../{ => resources}/variables.tf | 2 +- infra/modules/identity-provider/data/main.tf | 6 ++ .../modules/identity-provider/data/outputs.tf | 4 ++ .../identity-provider/data/variables.tf | 4 ++ .../identity-provider/{ => resources}/main.tf | 0 .../{ => resources}/outputs.tf | 0 .../{ => resources}/variables.tf | 0 template-only-bin/destroy-app-service | 2 +- 15 files changed, 85 insertions(+), 34 deletions(-) delete mode 100644 infra/modules/identity-provider-client/access-control.tf create mode 100644 infra/modules/identity-provider-client/resources/access-control.tf rename infra/modules/identity-provider-client/{ => resources}/main.tf (97%) rename infra/modules/identity-provider-client/{ => resources}/outputs.tf (70%) rename infra/modules/identity-provider-client/{ => resources}/variables.tf (94%) create mode 100644 infra/modules/identity-provider/data/main.tf create mode 100644 infra/modules/identity-provider/data/outputs.tf create mode 100644 infra/modules/identity-provider/data/variables.tf rename infra/modules/identity-provider/{ => resources}/main.tf (100%) rename infra/modules/identity-provider/{ => resources}/outputs.tf (100%) rename infra/modules/identity-provider/{ => resources}/variables.tf (100%) diff --git a/infra/app/app-config/env-config/identity-provider.tf b/infra/app/app-config/env-config/identity-provider.tf index e3d8ba3f..0151d4cc 100644 --- a/infra/app/app-config/env-config/identity-provider.tf +++ b/infra/app/app-config/env-config/identity-provider.tf @@ -15,7 +15,7 @@ locals { logout_url_path = "" identity_provider_config = var.enable_identity_provider ? { - identity_provider_name = "${local.prefix}${var.app_name}-${var.environment}" + identity_provider_name = "${var.app_name}-${var.environment}" password_policy = { password_minimum_length = 12 diff --git a/infra/app/app-config/env-config/main.tf b/infra/app/app-config/env-config/main.tf index 3eebe7a2..a7263269 100644 --- a/infra/app/app-config/env-config/main.tf +++ b/infra/app/app-config/env-config/main.tf @@ -1,7 +1,9 @@ locals { - # The prefix key/value pair is used for Terraform Workspaces, which is useful for projects with multiple infrastructure developers. - # By default, Terraform creates a workspace named “default.” If a non-default workspace is not created this prefix will equal “default”, - # if you choose not to use workspaces set this value to "dev" + # The prefix is used to create uniquely named resources per terraform workspace, which + # are needed in CI/CD for preview environments and tests. + # + # To isolate changes during infrastructure development by using manually created + # terraform workspaces, see: /docs/infra/develop-and-test-infrastructure-in-isolation-using-workspaces.md prefix = terraform.workspace == "default" ? "" : "${terraform.workspace}-" bucket_name = "${local.prefix}${var.project_name}-${var.app_name}-${var.environment}" diff --git a/infra/app/service/main.tf b/infra/app/service/main.tf index 38114280..240ddf04 100644 --- a/infra/app/service/main.tf +++ b/infra/app/service/main.tf @@ -22,12 +22,22 @@ data "aws_subnets" "private" { } locals { + # The prefix is used to create uniquely named resources per terraform workspace, which + # are needed in CI/CD for preview environments and tests. + # + # To isolate changes during infrastructure development by using manually created + # terraform workspaces, see: /docs/infra/develop-and-test-infrastructure-in-isolation-using-workspaces.md + prefix = terraform.workspace == "default" ? "" : "${terraform.workspace}-" + # Add environment specific tags tags = merge(module.project_config.default_tags, { environment = var.environment_name description = "Application resources created in ${var.environment_name} environment" }) + # All non-default terraform workspaces are considered temporary. + # Temporary environments do not have deletion protection enabled. + # Examples: pull request preview environments are temporary. is_temporary = terraform.workspace != "default" environment_config = module.app_config.environment_configs[var.environment_name] @@ -39,6 +49,17 @@ locals { notifications_config = local.environment_config.notifications_config network_config = module.project_config.network_configs[local.environment_config.network_name] + + # Identity provider locals. + # If this is a temporary environment, re-use an existing Cognito user pool. + # Otherwise, create a new one. + identity_provider_user_pool_id = module.app_config.enable_identity_provider ? ( + local.is_temporary ? module.existing_identity_provider[0].user_pool_id : module.identity_provider[0].user_pool_id + ) : null + identity_provider_environment_variables = module.app_config.enable_identity_provider ? { + COGNITO_USER_POOL_ID = local.identity_provider_user_pool_id, + COGNITO_CLIENT_ID = module.identity_provider_client[0].client_id + } : {} } terraform { @@ -157,10 +178,7 @@ module "service" { FEATURE_FLAGS_PROJECT = module.feature_flags.evidently_project_name BUCKET_NAME = local.storage_config.bucket_name }, - module.app_config.enable_identity_provider ? { - COGNITO_USER_POOL_ID = module.identity_provider[0].user_pool_id - COGNITO_CLIENT_ID = module.identity_provider_client[0].client_id - } : {}, + local.identity_provider_environment_variables, local.service_config.extra_environment_variables ) @@ -211,9 +229,12 @@ module "storage" { is_temporary = local.is_temporary } +# If the app has `enable_identity_provider` set to true AND this is not a temporary +# environment, then create a new identity provider. module "identity_provider" { - count = module.app_config.enable_identity_provider ? 1 : 0 - source = "../../modules/identity-provider" + count = module.app_config.enable_identity_provider && !local.is_temporary ? 1 : 0 + source = "../../modules/identity-provider/resources" + is_temporary = local.is_temporary name = local.identity_provider_config.identity_provider_name @@ -227,12 +248,25 @@ module "identity_provider" { reply_to_email = local.notifications_config == null ? null : local.notifications_config.reply_to_email } +# If the app has `enable_identity_provider` set to true AND this *is* a temporary +# environment, then use an existing identity provider. +module "existing_identity_provider" { + count = module.app_config.enable_identity_provider && local.is_temporary ? 1 : 0 + source = "../../modules/identity-provider/data" + + name = local.identity_provider_config.identity_provider_name +} + +# If the app has `enable_identity_provider` set to true, create a new identity provider +# client for the service. A new client is created for all environments, including +# temporary environments. module "identity_provider_client" { count = module.app_config.enable_identity_provider ? 1 : 0 - source = "../../modules/identity-provider-client" + source = "../../modules/identity-provider-client/resources" + + callback_urls = local.identity_provider_config.client.callback_urls + logout_urls = local.identity_provider_config.client.logout_urls + name = "${local.prefix}${local.identity_provider_config.identity_provider_name}" - name = local.identity_provider_config.identity_provider_name - cognito_user_pool_id = module.identity_provider[0].user_pool_id - callback_urls = local.identity_provider_config.client.callback_urls - logout_urls = local.identity_provider_config.client.logout_urls + user_pool_id = local.identity_provider_user_pool_id } diff --git a/infra/modules/identity-provider-client/access-control.tf b/infra/modules/identity-provider-client/access-control.tf deleted file mode 100644 index db1421df..00000000 --- a/infra/modules/identity-provider-client/access-control.tf +++ /dev/null @@ -1,15 +0,0 @@ -data "aws_caller_identity" "current" {} -data "aws_region" "current" {} - -resource "aws_iam_policy" "cognito_access" { - name = "${var.name}-cognito-access" - policy = data.aws_iam_policy_document.cognito_access.json -} - -data "aws_iam_policy_document" "cognito_access" { - statement { - actions = ["cognito-idp:*"] - effect = "Allow" - resources = ["arn:aws:cognito-idp:${data.aws_region.current.name}:${data.aws_caller_identity.current.id}:userpool/${var.cognito_user_pool_id}"] - } -} diff --git a/infra/modules/identity-provider-client/resources/access-control.tf b/infra/modules/identity-provider-client/resources/access-control.tf new file mode 100644 index 00000000..3e4bb9e3 --- /dev/null +++ b/infra/modules/identity-provider-client/resources/access-control.tf @@ -0,0 +1,15 @@ +data "aws_caller_identity" "current" {} +data "aws_region" "current" {} + +resource "aws_iam_policy" "identity_access" { + name = "${var.name}-identity-access" + policy = data.aws_iam_policy_document.identity_access.json +} + +data "aws_iam_policy_document" "identity_access" { + statement { + actions = ["cognito-idp:*"] + effect = "Allow" + resources = ["arn:aws:cognito-idp:${data.aws_region.current.name}:${data.aws_caller_identity.current.id}:userpool/${var.user_pool_id}"] + } +} diff --git a/infra/modules/identity-provider-client/main.tf b/infra/modules/identity-provider-client/resources/main.tf similarity index 97% rename from infra/modules/identity-provider-client/main.tf rename to infra/modules/identity-provider-client/resources/main.tf index d8110d52..7c65292d 100644 --- a/infra/modules/identity-provider-client/main.tf +++ b/infra/modules/identity-provider-client/resources/main.tf @@ -1,6 +1,6 @@ resource "aws_cognito_user_pool_client" "client" { name = var.name - user_pool_id = var.cognito_user_pool_id + user_pool_id = var.user_pool_id callback_urls = var.callback_urls logout_urls = var.logout_urls diff --git a/infra/modules/identity-provider-client/outputs.tf b/infra/modules/identity-provider-client/resources/outputs.tf similarity index 70% rename from infra/modules/identity-provider-client/outputs.tf rename to infra/modules/identity-provider-client/resources/outputs.tf index 2bf81ddf..3f339506 100644 --- a/infra/modules/identity-provider-client/outputs.tf +++ b/infra/modules/identity-provider-client/resources/outputs.tf @@ -1,5 +1,6 @@ output "access_policy_arn" { - value = aws_iam_policy.cognito_access.arn + description = "The arn for the IAM access policy granting access to the user pool" + value = aws_iam_policy.identity_access.arn } output "client_id" { diff --git a/infra/modules/identity-provider-client/variables.tf b/infra/modules/identity-provider-client/resources/variables.tf similarity index 94% rename from infra/modules/identity-provider-client/variables.tf rename to infra/modules/identity-provider-client/resources/variables.tf index f75f5296..5ac6e811 100644 --- a/infra/modules/identity-provider-client/variables.tf +++ b/infra/modules/identity-provider-client/resources/variables.tf @@ -4,7 +4,7 @@ variable "callback_urls" { default = [] } -variable "cognito_user_pool_id" { +variable "user_pool_id" { type = string description = "The ID of the user pool that the client will be associated with" } diff --git a/infra/modules/identity-provider/data/main.tf b/infra/modules/identity-provider/data/main.tf new file mode 100644 index 00000000..f3f2ed71 --- /dev/null +++ b/infra/modules/identity-provider/data/main.tf @@ -0,0 +1,6 @@ +############################################################################################ +## A module for retrieving an existing Cognito User Pool +############################################################################################ +data "aws_cognito_user_pools" "existing_user_pools" { + name = var.name +} diff --git a/infra/modules/identity-provider/data/outputs.tf b/infra/modules/identity-provider/data/outputs.tf new file mode 100644 index 00000000..51dd598f --- /dev/null +++ b/infra/modules/identity-provider/data/outputs.tf @@ -0,0 +1,4 @@ +output "user_pool_id" { + description = "The ID of the user pool." + value = tolist(data.aws_cognito_user_pools.existing_user_pools.ids)[0] +} diff --git a/infra/modules/identity-provider/data/variables.tf b/infra/modules/identity-provider/data/variables.tf new file mode 100644 index 00000000..83c24d80 --- /dev/null +++ b/infra/modules/identity-provider/data/variables.tf @@ -0,0 +1,4 @@ +variable "name" { + type = string + description = "The name of an existing cognito user pool" +} diff --git a/infra/modules/identity-provider/main.tf b/infra/modules/identity-provider/resources/main.tf similarity index 100% rename from infra/modules/identity-provider/main.tf rename to infra/modules/identity-provider/resources/main.tf diff --git a/infra/modules/identity-provider/outputs.tf b/infra/modules/identity-provider/resources/outputs.tf similarity index 100% rename from infra/modules/identity-provider/outputs.tf rename to infra/modules/identity-provider/resources/outputs.tf diff --git a/infra/modules/identity-provider/variables.tf b/infra/modules/identity-provider/resources/variables.tf similarity index 100% rename from infra/modules/identity-provider/variables.tf rename to infra/modules/identity-provider/resources/variables.tf diff --git a/template-only-bin/destroy-app-service b/template-only-bin/destroy-app-service index 6f327653..f6bdac20 100755 --- a/template-only-bin/destroy-app-service +++ b/template-only-bin/destroy-app-service @@ -18,7 +18,7 @@ backend_config_file="${environment}.s3.tfbackend" sed -i.bak 's/force_destroy = var.is_temporary/force_destroy = true/g' infra/modules/service/access-logs.tf sed -i.bak 's/force_destroy = var.is_temporary/force_destroy = true/g' infra/modules/storage/main.tf sed -i.bak 's/enable_deletion_protection = !var.is_temporary/enable_deletion_protection = false/g' infra/modules/service/load-balancer.tf -sed -i.bak 's/deletion_protection = var.is_temporary ? "INACTIVE" : "ACTIVE"/deletion_protection = "INACTIVE"/g' infra/modules/identity-provider/main.tf +sed -i.bak 's/deletion_protection = var.is_temporary ? "INACTIVE" : "ACTIVE"/deletion_protection = "INACTIVE"/g' infra/modules/identity-provider/resources/main.tf cd "infra/${app_name}/service"