Skip to content

Commit

Permalink
Fix access to build repository from different AWS account (#737)
Browse files Browse the repository at this point in the history
## Changes

- Configure prod environment in separate prod AWS account
- Move build repository config to separate file under app-config module
- Consolidate image_repository_name config under
build_repostiory_config.name
- Add network_name, account_name, account_id, repository_arn, and
repository_url attributes to build repository config
- Replace ecr_repository data source in modules/service with
image_repository_arn and image_repository_url passed in through
variables

## Context

The FFS project discovered a bug in multi-account project setups. The
service module determined the build repository ARN and URL using a data
source, but a data source can only fetch resources in the same account.
In order to address this, we need to eliminate the data source and
construct the repository ARN and repository URL through different means.
The way we do it is by looking at which account the build repository is
in (through app-config) and getting the id of that account, which gives
us the necessary information to do this.
  • Loading branch information
lorenyu authored Aug 16, 2024
1 parent da5e865 commit c7d3f52
Show file tree
Hide file tree
Showing 11 changed files with 44 additions and 26 deletions.
2 changes: 1 addition & 1 deletion bin/is-image-published
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ image_tag=$(git rev-parse "${git_ref}")
# Need to init module when running in CD since GitHub actions does a fresh checkout of repo
terraform -chdir="infra/${app_name}/app-config" init > /dev/null
terraform -chdir="infra/${app_name}/app-config" apply -auto-approve > /dev/null
image_repository_name=$(terraform -chdir="infra/${app_name}/app-config" output -raw image_repository_name)
image_repository_name="$(terraform -chdir="infra/${app_name}/app-config" output -json build_repository_config | jq -r ".name")"
region=$(./bin/current-region)

result=""
Expand Down
2 changes: 1 addition & 1 deletion bin/publish-release
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ echo " image_tag=${image_tag}"
# Need to init module when running in CD since GitHub actions does a fresh checkout of repo
terraform -chdir="infra/${app_name}/app-config" init > /dev/null
terraform -chdir="infra/${app_name}/app-config" apply -auto-approve > /dev/null
image_repository_name=$(terraform -chdir="infra/${app_name}/app-config" output -raw image_repository_name)
image_repository_name="$(terraform -chdir="infra/${app_name}/app-config" output -json build_repository_config | jq -r ".name")"

region=$(./bin/current-region)
read -r image_registry_id image_repository_url <<< "$(aws ecr describe-repositories --repository-names "${image_repository_name}" --query "repositories[0].[registryId,repositoryUri]" --output text)"
Expand Down
20 changes: 20 additions & 0 deletions infra/app/app-config/build-repository.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
data "external" "account_ids_by_name" {
program = ["${path.module}/../../../bin/account-ids-by-name"]
}

locals {
image_repository_name = "${local.project_name}-${local.app_name}"
image_repository_region = module.project_config.default_region
image_repository_account_name = module.project_config.network_configs[local.shared_network_name].account_name
image_repository_account_id = data.external.account_ids_by_name.result[local.image_repository_account_name]

build_repository_config = {
name = local.image_repository_name
region = local.image_repository_region
network_name = local.shared_network_name
account_name = local.image_repository_account_name
account_id = local.image_repository_account_id
repository_arn = "arn:aws:ecr:${local.image_repository_region}:${local.image_repository_account_id}:repository/${local.image_repository_name}"
repository_url = "${local.image_repository_account_id}.dkr.ecr.${local.image_repository_region}.amazonaws.com/${local.image_repository_name}"
}
}
9 changes: 2 additions & 7 deletions infra/app/app-config/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ locals {
# the folder under /infra that corresponds to the application
app_name = regex("/infra/([^/]+)/app-config$", abspath(path.module))[0]

environments = ["dev", "staging", "prod"]
project_name = module.project_config.project_name
image_repository_name = "${local.project_name}-${local.app_name}"
environments = ["dev", "staging", "prod"]
project_name = module.project_config.project_name

# Whether or not the application has a database
# If enabled:
Expand Down Expand Up @@ -42,10 +41,6 @@ locals {
prod = module.prod_config
}

build_repository_config = {
region = module.project_config.default_region
}

# The name of the network that contains the resources shared across all
# application environments, such as the build repository.
# The list of networks can be found in /infra/networks
Expand Down
4 changes: 0 additions & 4 deletions infra/app/app-config/outputs.tf
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ output "enable_identity_provider" {
value = local.enable_identity_provider
}

output "image_repository_name" {
value = local.image_repository_name
}

output "shared_network_name" {
value = local.shared_network_name
}
8 changes: 5 additions & 3 deletions infra/app/build-repository/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ locals {
description = "Backend resources required for storing built release candidate artifacts to be used for deploying to environments."
})

build_repository_config = module.app_config.build_repository_config

# Get list of AWS account ids for the application environments that
# will need access to the build repository
network_names = toset([for environment_config in values(module.app_config.environment_configs) : environment_config.network_name])
Expand All @@ -34,7 +36,7 @@ terraform {
}

provider "aws" {
region = module.app_config.build_repository_config.region
region = local.build_repository_config.region
default_tags {
tags = local.tags
}
Expand All @@ -49,12 +51,12 @@ module "app_config" {
}

data "external" "account_ids_by_name" {
program = ["../../../bin/account-ids-by-name"]
program = ["${path.module}/../../../bin/account-ids-by-name"]
}

module "container_image_repository" {
source = "../../modules/container-image-repository"
name = module.app_config.image_repository_name
name = local.build_repository_config.name
push_access_role_arn = data.aws_iam_role.github_actions.arn
app_account_ids = local.app_account_ids
}
7 changes: 5 additions & 2 deletions infra/app/service/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ locals {
# Examples: pull request preview environments are temporary.
is_temporary = terraform.workspace != "default"

build_repository_config = module.app_config.build_repository_config
environment_config = module.app_config.environment_configs[var.environment_name]
service_config = local.environment_config.service_config
database_config = local.environment_config.database_config
Expand Down Expand Up @@ -140,8 +141,10 @@ module "service" {
source = "../../modules/service"
service_name = local.service_config.service_name

image_repository_name = module.app_config.image_repository_name
image_tag = local.image_tag
image_repository_arn = local.build_repository_config.repository_arn
image_repository_url = local.build_repository_config.repository_url

image_tag = local.image_tag

vpc_id = data.aws_vpc.network.id
public_subnet_ids = data.aws_subnets.public.ids
Expand Down
2 changes: 1 addition & 1 deletion infra/modules/service/access-control.tf
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ data "aws_iam_policy_document" "task_executor" {
"ecr:BatchGetImage",
"ecr:GetDownloadUrlForLayer",
]
resources = [data.aws_ecr_repository.app.arn]
resources = [var.image_repository_arn]
}

dynamic "statement" {
Expand Down
5 changes: 1 addition & 4 deletions infra/modules/service/main.tf
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
data "aws_caller_identity" "current" {}
data "aws_region" "current" {}
data "aws_ecr_repository" "app" {
name = var.image_repository_name
}

locals {
alb_name = var.service_name
Expand All @@ -11,7 +8,7 @@ locals {
log_group_name = "service/${var.service_name}"
log_stream_prefix = var.service_name
task_executor_role_name = "${var.service_name}-task-executor"
image_url = "${data.aws_ecr_repository.app.repository_url}:${var.image_tag}"
image_url = "${var.image_repository_url}:${var.image_tag}"

base_environment_variables = [
{ name : "PORT", value : tostring(var.container_port) },
Expand Down
7 changes: 6 additions & 1 deletion infra/modules/service/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,12 @@ variable "hosted_zone_id" {
default = null
}

variable "image_repository_name" {
variable "image_repository_arn" {
type = string
description = "The name of the container image repository"
}

variable "image_repository_url" {
type = string
description = "The name of the container image repository"
}
Expand Down
4 changes: 2 additions & 2 deletions template-only-test/template_infra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func SetUpAccount(t *testing.T) {
fmt.Println("::group::Setting up account")
shell.RunCommand(t, shell.Command{
Command: "make",
Args: []string{"infra-set-up-account", "ACCOUNT_NAME=prod"},
Args: []string{"infra-set-up-account", "ACCOUNT_NAME=dev"},
WorkingDir: "../",
})
fmt.Println("::endgroup::")
Expand Down Expand Up @@ -144,7 +144,7 @@ func ValidateGithubActionsAuth(t *testing.T, accountId string, projectName strin
// Check that GitHub Actions can authenticate with AWS
err := shell.RunCommandE(t, shell.Command{
Command: "make",
Args: []string{"infra-check-github-actions-auth", "ACCOUNT_NAME=prod"},
Args: []string{"infra-check-github-actions-auth", "ACCOUNT_NAME=dev"},
WorkingDir: "../",
})
assert.NoError(t, err, "GitHub actions failed to authenticate")
Expand Down

0 comments on commit c7d3f52

Please sign in to comment.