Skip to content

Commit

Permalink
Use existing identity provider in temporary environments (#717)
Browse files Browse the repository at this point in the history
⚠️ 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
  • Loading branch information
rocketnova authored Aug 14, 2024
1 parent 8e42ad7 commit da5e865
Show file tree
Hide file tree
Showing 15 changed files with 85 additions and 34 deletions.
2 changes: 1 addition & 1 deletion infra/app/app-config/env-config/identity-provider.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions infra/app/app-config/env-config/main.tf
Original file line number Diff line number Diff line change
@@ -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}"
Expand Down
56 changes: 45 additions & 11 deletions infra/app/service/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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 {
Expand Down Expand Up @@ -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
)

Expand Down Expand Up @@ -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
Expand All @@ -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
}
15 changes: 0 additions & 15 deletions infra/modules/identity-provider-client/access-control.tf

This file was deleted.

15 changes: 15 additions & 0 deletions infra/modules/identity-provider-client/resources/access-control.tf
Original file line number Diff line number Diff line change
@@ -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}"]
}
}
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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" {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
6 changes: 6 additions & 0 deletions infra/modules/identity-provider/data/main.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
############################################################################################
## A module for retrieving an existing Cognito User Pool
############################################################################################
data "aws_cognito_user_pools" "existing_user_pools" {
name = var.name
}
4 changes: 4 additions & 0 deletions infra/modules/identity-provider/data/outputs.tf
Original file line number Diff line number Diff line change
@@ -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]
}
4 changes: 4 additions & 0 deletions infra/modules/identity-provider/data/variables.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
variable "name" {
type = string
description = "The name of an existing cognito user pool"
}
File renamed without changes.
File renamed without changes.
2 changes: 1 addition & 1 deletion template-only-bin/destroy-app-service
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down

0 comments on commit da5e865

Please sign in to comment.