From 34dde5a3329641eb0a6326294cbb678145ab0b2a Mon Sep 17 00:00:00 2001 From: Joshua Burns Date: Thu, 21 Sep 2023 11:20:55 -0700 Subject: [PATCH 1/7] attempt at making more dry --- terraform/_outputs.tf | 6 ++-- terraform/_terraform.tf | 2 +- terraform/_varibales.tf | 8 ++++- terraform/backend.tf | 8 ++--- terraform/ecr.tf | 2 +- terraform/ecs.tf | 32 +++++++++++--------- terraform/oidc/_variables.tf | 6 ++++ terraform/oidc/main.tf | 55 ++++++++++++++++++++++++++++++++++- terraform/oidc/terragrunt.hcl | 4 +++ terraform/shared.tfvars | 3 ++ terraform/terragrunt.hcl | 20 ++++++++----- terraform/vpc.tf | 8 ++--- 12 files changed, 118 insertions(+), 36 deletions(-) create mode 100644 terraform/shared.tfvars diff --git a/terraform/_outputs.tf b/terraform/_outputs.tf index 99a5ac5..857c9e5 100644 --- a/terraform/_outputs.tf +++ b/terraform/_outputs.tf @@ -1,5 +1,5 @@ output "ecr_repository_url" { - value = aws_ecr_repository.knowledgeshare_ui_ecr.repository_url + value = aws_ecr_repository.knowledgeshare_ui_ecr.repository_url description = "URL of the ECR repository" } @@ -14,11 +14,11 @@ output "ecs_cluster_arn" { } output "ecs_task_arn" { - value = aws_ecs_task_definition.knowledgeshare_ui_task.arn + value = data.aws_ecs_task_definition.current_task.arn description = "ARN of the ECS task definition" } output "ecs_service_arn" { - value = aws_ecs_service.knowledgeshare_ui_service.id + value = aws_ecs_service.knowledgeshare_ui_service.id description = "ARN of the ECS Service" } diff --git a/terraform/_terraform.tf b/terraform/_terraform.tf index 64bee2f..a8fa043 100644 --- a/terraform/_terraform.tf +++ b/terraform/_terraform.tf @@ -8,5 +8,5 @@ terraform { } provider "aws" { - region = "us-west-2" + region = var.aws_region } diff --git a/terraform/_varibales.tf b/terraform/_varibales.tf index 09fe119..f8092bd 100644 --- a/terraform/_varibales.tf +++ b/terraform/_varibales.tf @@ -1,5 +1,11 @@ variable "name" { type = string description = "The Repository Name" - default = "keyless-workflow-demo" + default = "keyless-workflow-demo" } + +variable "tfstate_bucket" {} + +variable "tfstate_dynamodb_table" {} + +variable "aws_region" {} diff --git a/terraform/backend.tf b/terraform/backend.tf index bdb1641..e93172e 100644 --- a/terraform/backend.tf +++ b/terraform/backend.tf @@ -1,10 +1,10 @@ # Generated by Terragrunt. Sig: nIlQXj57tbuaRZEa terraform { backend "s3" { - bucket = "keyless-workflow-demo" - dynamodb_table = "tflocks" + bucket = "keyless-workflow-demo-tfstate" + dynamodb_table = "keyless-demo-tflock" encrypt = true - key = "keyless-workflow-demo/terraform.tfstate" - region = "us-west-2" + key = "./terraform.tfstate" + region = "us-east-2" } } diff --git a/terraform/ecr.tf b/terraform/ecr.tf index 59bf9cd..0b901d0 100644 --- a/terraform/ecr.tf +++ b/terraform/ecr.tf @@ -1,6 +1,6 @@ resource "aws_ecr_repository" "knowledgeshare_ui_ecr" { name = var.name - force_delete = true + force_delete = true image_tag_mutability = "MUTABLE" image_scanning_configuration { diff --git a/terraform/ecs.tf b/terraform/ecs.tf index 671b486..5146c03 100644 --- a/terraform/ecs.tf +++ b/terraform/ecs.tf @@ -1,7 +1,7 @@ resource "aws_ecs_cluster" "knowledgeshare_ui_ecs_cluster" { name = "knowledgeshare-demo-clster" setting { - name = "containerInsights" + name = "containerInsights" value = "enabled" } } @@ -30,36 +30,40 @@ resource "aws_iam_role_policy_attachment" "ecs_execution_role_attachment" { } resource "aws_ecs_task_definition" "knowledgeshare_ui_task" { - family = "keyless-workflow-demo-td" + family = "keyless-workflow-demo-td" network_mode = "awsvpc" # FARGATE requires awsvpc from what I can tell requires_compatibilities = ["FARGATE"] - cpu = "1024" # Choose based on your requirements - memory = "2048" # Choose based on your requirements + cpu = "1024" # Choose based on your requirements + memory = "2048" # Choose based on your requirements execution_role_arn = aws_iam_role.ecs_execution_role.arn # FARGATE also requires this role to fetch images from ECR container_definitions = jsonencode([{ - name = "knowledgeshare-ui" # TODO make this a variable that can be made into output so we can fetch programatically - image = "${aws_ecr_repository.knowledgeshare_ui_ecr.repository_url}:latest" + name = "knowledgeshare-ui" # TODO make this a variable that can be made into output so we can fetch programatically + image = "${aws_ecr_repository.knowledgeshare_ui_ecr.repository_url}:latest" essential = true portMappings = [ { containerPort = 3000 - hostPort = 3000 + hostPort = 3000 } ] }]) } +data "aws_ecs_task_definition" "current_task" { + task_definition = aws_ecs_task_definition.knowledgeshare_ui_task.family +} + resource "aws_ecs_service" "knowledgeshare_ui_service" { - name = "knowledgeshare_ui" - launch_type = "FARGATE" - cluster = aws_ecs_cluster.knowledgeshare_ui_ecs_cluster.id - task_definition = aws_ecs_task_definition.knowledgeshare_ui_task.arn - desired_count = 1 + name = "knowledgeshare_ui" + launch_type = "FARGATE" + cluster = aws_ecs_cluster.knowledgeshare_ui_ecs_cluster.id + task_definition = data.aws_ecs_task_definition.current_task.arn + desired_count = 1 force_new_deployment = true network_configuration { - subnets = [aws_subnet.public_subnet_a.id, aws_subnet.public_subnet_b.id] - security_groups = [aws_security_group.keyless_workflow_demo_sg.id] + subnets = [aws_subnet.public_subnet_a.id, aws_subnet.public_subnet_b.id] + security_groups = [aws_security_group.keyless_workflow_demo_sg.id] assign_public_ip = true } # iam_role = aws_iam_role.foo.arn diff --git a/terraform/oidc/_variables.tf b/terraform/oidc/_variables.tf index 18190e0..d7735b0 100644 --- a/terraform/oidc/_variables.tf +++ b/terraform/oidc/_variables.tf @@ -14,3 +14,9 @@ variable "ecr_repository_arn" { description = "ARN of the ECR repository" type = string } + +variable "tfstate_bucket" {} + +variable "tfstate_dynamodb_table" {} + +variable "aws_region" {} diff --git a/terraform/oidc/main.tf b/terraform/oidc/main.tf index d7c2d8b..639cb19 100644 --- a/terraform/oidc/main.tf +++ b/terraform/oidc/main.tf @@ -52,6 +52,59 @@ resource "aws_iam_policy" "ecs_ecr_policy" { }) } +# TODO see if you can trim the permissions down such that they match +# { +# "Version": "2012-10-17", +# "Statement": [ +# { +# "Sid": "AllowListS3ActionsOnTerraformBucket", +# "Effect": "Allow", +# "Action": "s3:ListBucket", +# "Resource": "arn:aws:s3:::admin-terraform-state.liatrio.com" +# }, +# { +# "Sid": "AllowGetS3ActionsOnTerraformBucketPath", +# "Effect": "Allow", +# "Action": "s3:GetObject", +# "Resource": "arn:aws:s3:::admin-terraform-state.liatrio.com/*" +# } +# ] +# } +resource "aws_iam_policy" "terraform_read" { + name = "terraform_read" + description = "Policy that gives permissions to access the backend s3 bucket the tf state file is stored" + + policy = jsonencode({ + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "S3BucketPermissions", + "Effect": "Allow", + "Action": [ + "s3:GetObject", + "s3:PutObject", + "s3:DeleteObject", + "s3:ListBucket" + ], + "Resource": [ + "arn:aws:s3:::${var.tfstate_bucket}", + "arn:aws:s3:::${var.tfstate_bucket}/*" + ] + }, + { + "Sid": "DynamoDBLockTable", + "Effect": "Allow", + "Action": [ + "dynamodb:GetItem", + "dynamodb:PutItem", + "dynamodb:DescribeTable", + "dynamodb:DeleteItem" + ], + "Resource": "arn:aws:dynamodb:${var.aws_region}:*:table/${var.tfstate_dynamodb_table}" + } + ] + }) +} data "aws_iam_policy_document" "gha_trust_policy" { statement { @@ -77,5 +130,5 @@ data "aws_iam_policy_document" "gha_trust_policy" { resource "aws_iam_role" "gha_role" { name = "gha_role" assume_role_policy = data.aws_iam_policy_document.gha_trust_policy.json - managed_policy_arns = [aws_iam_policy.ecs_ecr_policy.arn] + managed_policy_arns = [aws_iam_policy.ecs_ecr_policy.arn, aws_iam_policy.terraform_read.arn] } diff --git a/terraform/oidc/terragrunt.hcl b/terraform/oidc/terragrunt.hcl index aa9557e..b1a1f23 100644 --- a/terraform/oidc/terragrunt.hcl +++ b/terraform/oidc/terragrunt.hcl @@ -9,3 +9,7 @@ inputs = { ecs_task_arn = dependency.parent_outputs.outputs.ecs_task_arn ecr_repository_arn = dependency.parent_outputs.outputs.ecr_repository_arn } + +include { + path = find_in_parent_folders() +} diff --git a/terraform/shared.tfvars b/terraform/shared.tfvars new file mode 100644 index 0000000..b1955c2 --- /dev/null +++ b/terraform/shared.tfvars @@ -0,0 +1,3 @@ +tfstate_bucket = "keyless-workflow-demo-tfstate" +tfstate_dynamodb_table = "keyless-demo-tflock" +aws_region = "us-east-2" diff --git a/terraform/terragrunt.hcl b/terraform/terragrunt.hcl index 7d4a10c..d03886c 100644 --- a/terraform/terragrunt.hcl +++ b/terraform/terragrunt.hcl @@ -5,11 +5,11 @@ remote_state { if_exists = "overwrite_terragrunt" } config = { - bucket = "keyless-workflow-demo" - key = "keyless-workflow-demo/terraform.tfstate" + bucket = "keyless-workflow-demo-tfstate" + key = "${path_relative_to_include()}/terraform.tfstate" - region = "us-west-2" - dynamodb_table = "tflocks" + region = "us-east-2" + dynamodb_table = "keyless-demo-tflock" disable_bucket_update = true # Permissions thing @@ -19,6 +19,12 @@ remote_state { } } -# terraform { -# source = ".//tf" -# } +terraform { + extra_arguments "shared_vars" { + commands = get_terraform_commands_that_need_vars() + optional_var_files = [ + "${get_parent_terragrunt_dir()}/shared.tfvars", + "${find_in_parent_folders("shared.tfvars", "ignore")}" + ] + } +} diff --git a/terraform/vpc.tf b/terraform/vpc.tf index 52e78c6..e05c786 100644 --- a/terraform/vpc.tf +++ b/terraform/vpc.tf @@ -10,8 +10,8 @@ resource "aws_vpc" "keyless_workflow_demo_vpc" { resource "aws_subnet" "public_subnet_a" { vpc_id = aws_vpc.keyless_workflow_demo_vpc.id cidr_block = "10.0.1.0/24" - availability_zone = "us-west-2a" # Adjust as needed - map_public_ip_on_launch = true # This will give instances in this subnet a public IP by default + availability_zone = "${var.aws_region}a" # Adjust as needed + map_public_ip_on_launch = true # This will give instances in this subnet a public IP by default tags = { Name = "keyless_workflow_demo_subnet" } @@ -20,8 +20,8 @@ resource "aws_subnet" "public_subnet_a" { resource "aws_subnet" "public_subnet_b" { vpc_id = aws_vpc.keyless_workflow_demo_vpc.id cidr_block = "10.0.2.0/24" - availability_zone = "us-west-2b" # Adjust as needed - map_public_ip_on_launch = true # This will give instances in this subnet a public IP by default + availability_zone = "${var.aws_region}b" # Adjust as needed + map_public_ip_on_launch = true # This will give instances in this subnet a public IP by default tags = { Name = "keyless_workflow_demo_subnet" } From 8d216b0b0c31f0544742b44b8d2fb036fcc2fd5c Mon Sep 17 00:00:00 2001 From: Joshua Burns Date: Thu, 21 Sep 2023 13:12:32 -0700 Subject: [PATCH 2/7] updated workflow to use oidc role --- .github/workflows/build-infra.yaml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build-infra.yaml b/.github/workflows/build-infra.yaml index 7b95a50..12db972 100644 --- a/.github/workflows/build-infra.yaml +++ b/.github/workflows/build-infra.yaml @@ -23,9 +23,10 @@ jobs: - name: Configure AWS credentials uses: aws-actions/configure-aws-credentials@v3 with: - aws-access-key-id: ${{ secrets.PERSONAL_ACCESS_KEY }} - aws-secret-access-key: ${{ secrets.PERSONAL_SECRET_ACCESS_KEY }} - role-to-assume: ${{ secrets.ROLE_TO_ASSUME }} + # aws-access-key-id: ${{ secrets.PERSONAL_ACCESS_KEY }} + # aws-secret-access-key: ${{ secrets.PERSONAL_SECRET_ACCESS_KEY }} + # role-to-assume: ${{ secrets.ROLE_TO_ASSUME }} + role-to-assume: ${{ vars.OIDC_ROLE }} aws-region: ${{ vars.AWS_REGION }} role-skip-session-tagging: true From 6f16f0fcf40c1ab4b87af8bc46cdfec54841cd79 Mon Sep 17 00:00:00 2001 From: Joshua Burns Date: Thu, 21 Sep 2023 13:17:37 -0700 Subject: [PATCH 3/7] added permissions to the workflow so we can modify JWT used for OIDC --- .github/workflows/build-infra.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/build-infra.yaml b/.github/workflows/build-infra.yaml index 12db972..03afdbe 100644 --- a/.github/workflows/build-infra.yaml +++ b/.github/workflows/build-infra.yaml @@ -6,6 +6,11 @@ on: - main workflow_dispatch: {} +permissions: + id-token: write # Needed to modify JWT token for OIDC + contents: read # Needed for actions/checkout + + jobs: run: name: run From 10e95a51361c187b595016b55983006c369a4bff Mon Sep 17 00:00:00 2001 From: Joshua Burns Date: Thu, 21 Sep 2023 13:22:33 -0700 Subject: [PATCH 4/7] added environment to workflow --- .github/workflows/build-infra.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build-infra.yaml b/.github/workflows/build-infra.yaml index 03afdbe..917bd92 100644 --- a/.github/workflows/build-infra.yaml +++ b/.github/workflows/build-infra.yaml @@ -15,6 +15,7 @@ jobs: run: name: run runs-on: ubuntu-latest + environment: production steps: - uses: actions/checkout@v3 From b896e9fddd9cc58b11bcb23cd1591db91ae6ad3d Mon Sep 17 00:00:00 2001 From: Joshua Burns Date: Thu, 21 Sep 2023 15:05:11 -0700 Subject: [PATCH 5/7] update SIDs to be more descriptive, and added missing permissions to policy discovered when testing deploy via OIDC --- terraform/oidc/_outputs.tf | 4 +++ terraform/oidc/_variables.tf | 2 ++ terraform/oidc/backend.tf | 10 ++++++ terraform/oidc/main.tf | 61 ++++++++++++++++++----------------- terraform/oidc/terragrunt.hcl | 1 + 5 files changed, 49 insertions(+), 29 deletions(-) create mode 100644 terraform/oidc/_outputs.tf create mode 100644 terraform/oidc/backend.tf diff --git a/terraform/oidc/_outputs.tf b/terraform/oidc/_outputs.tf new file mode 100644 index 0000000..51f044e --- /dev/null +++ b/terraform/oidc/_outputs.tf @@ -0,0 +1,4 @@ +output "gha_role_arn" { + description = "ARN of the GitHub Action OIDC role" + value = aws_iam_role.gha_role.arn +} diff --git a/terraform/oidc/_variables.tf b/terraform/oidc/_variables.tf index d7735b0..07910da 100644 --- a/terraform/oidc/_variables.tf +++ b/terraform/oidc/_variables.tf @@ -20,3 +20,5 @@ variable "tfstate_bucket" {} variable "tfstate_dynamodb_table" {} variable "aws_region" {} + +variable "ecs_service_arn" {} diff --git a/terraform/oidc/backend.tf b/terraform/oidc/backend.tf new file mode 100644 index 0000000..7381873 --- /dev/null +++ b/terraform/oidc/backend.tf @@ -0,0 +1,10 @@ +# Generated by Terragrunt. Sig: nIlQXj57tbuaRZEa +terraform { + backend "s3" { + bucket = "keyless-workflow-demo-tfstate" + dynamodb_table = "keyless-demo-tflock" + encrypt = true + key = "oidc/terraform.tfstate" + region = "us-east-2" + } +} diff --git a/terraform/oidc/main.tf b/terraform/oidc/main.tf index 639cb19..28ac374 100644 --- a/terraform/oidc/main.tf +++ b/terraform/oidc/main.tf @@ -1,7 +1,3 @@ -data "tls_certificate" "github_thumbprint" { - url = "https://token.actions.githubusercontent.com/.well-known/openid-configuration" -} - ## Uncomment this block of code if you are testing this in a personal aws account ## This is a central resource that in my org is not managed via terraform and thus ## including this resouce causes issues. @@ -17,6 +13,10 @@ data "tls_certificate" "github_thumbprint" { # ] # } +data "tls_certificate" "github_thumbprint" { + url = "https://token.actions.githubusercontent.com/.well-known/openid-configuration" +} + data "aws_iam_openid_connect_provider" "github" { url = "https://token.actions.githubusercontent.com" } @@ -35,8 +35,32 @@ resource "aws_iam_policy" "ecs_ecr_policy" { ], Resource = [ var.ecs_cluster_arn, # ARN of the ECS cluster - replace(var.ecs_task_arn, "/:\\d+$/", ":*") # ARN of the ECS task definition - # Add ARNs of other ECS resources if needed + var.ecs_service_arn, # ARN of the ECS service + replace(var.ecs_task_arn, "/:\\d+$/", ":*") # ARN that matches all task-definition revisions for the task definition created in this repo + ] + }, + { + Effect = "Allow", + Action = [ + "ecs:DescribeTaskDefinition", + "ecs:RegisterTaskDefinition" + ], + Resource = [ "*" ] + }, + { + Effect = "Allow", + Action = [ + "iam:PassRole" + ], + Resource = [ "*" ] + }, + { + Effect = "Allow", + Action = [ + "ecr:GetAuthorizationToken", + ], + Resource = [ + "*" ] }, { @@ -52,24 +76,6 @@ resource "aws_iam_policy" "ecs_ecr_policy" { }) } -# TODO see if you can trim the permissions down such that they match -# { -# "Version": "2012-10-17", -# "Statement": [ -# { -# "Sid": "AllowListS3ActionsOnTerraformBucket", -# "Effect": "Allow", -# "Action": "s3:ListBucket", -# "Resource": "arn:aws:s3:::admin-terraform-state.liatrio.com" -# }, -# { -# "Sid": "AllowGetS3ActionsOnTerraformBucketPath", -# "Effect": "Allow", -# "Action": "s3:GetObject", -# "Resource": "arn:aws:s3:::admin-terraform-state.liatrio.com/*" -# } -# ] -# } resource "aws_iam_policy" "terraform_read" { name = "terraform_read" description = "Policy that gives permissions to access the backend s3 bucket the tf state file is stored" @@ -78,12 +84,10 @@ resource "aws_iam_policy" "terraform_read" { "Version": "2012-10-17", "Statement": [ { - "Sid": "S3BucketPermissions", + "Sid": "TerraformStateS3ReadPermissions", "Effect": "Allow", "Action": [ "s3:GetObject", - "s3:PutObject", - "s3:DeleteObject", "s3:ListBucket" ], "Resource": [ @@ -92,13 +96,12 @@ resource "aws_iam_policy" "terraform_read" { ] }, { - "Sid": "DynamoDBLockTable", + "Sid": "TerraformStateDynamoDBReadPermissions", "Effect": "Allow", "Action": [ "dynamodb:GetItem", "dynamodb:PutItem", "dynamodb:DescribeTable", - "dynamodb:DeleteItem" ], "Resource": "arn:aws:dynamodb:${var.aws_region}:*:table/${var.tfstate_dynamodb_table}" } diff --git a/terraform/oidc/terragrunt.hcl b/terraform/oidc/terragrunt.hcl index b1a1f23..e054f8a 100644 --- a/terraform/oidc/terragrunt.hcl +++ b/terraform/oidc/terragrunt.hcl @@ -8,6 +8,7 @@ inputs = { ecs_cluster_arn = dependency.parent_outputs.outputs.ecs_cluster_arn ecs_task_arn = dependency.parent_outputs.outputs.ecs_task_arn ecr_repository_arn = dependency.parent_outputs.outputs.ecr_repository_arn + ecs_service_arn = dependency.parent_outputs.outputs.ecs_service_arn } include { From 43a97198ace3d36a9ac840075a91d98ecfcb12c2 Mon Sep 17 00:00:00 2001 From: Joshua Burns Date: Thu, 21 Sep 2023 15:06:54 -0700 Subject: [PATCH 6/7] updated workflow to not use oidc --- .github/workflows/build-infra.yaml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build-infra.yaml b/.github/workflows/build-infra.yaml index 917bd92..4af8a79 100644 --- a/.github/workflows/build-infra.yaml +++ b/.github/workflows/build-infra.yaml @@ -6,9 +6,9 @@ on: - main workflow_dispatch: {} -permissions: - id-token: write # Needed to modify JWT token for OIDC - contents: read # Needed for actions/checkout +# permissions: +# id-token: write # Needed to modify JWT token for OIDC +# contents: read # Needed for actions/checkout jobs: @@ -29,10 +29,10 @@ jobs: - name: Configure AWS credentials uses: aws-actions/configure-aws-credentials@v3 with: - # aws-access-key-id: ${{ secrets.PERSONAL_ACCESS_KEY }} - # aws-secret-access-key: ${{ secrets.PERSONAL_SECRET_ACCESS_KEY }} - # role-to-assume: ${{ secrets.ROLE_TO_ASSUME }} - role-to-assume: ${{ vars.OIDC_ROLE }} + aws-access-key-id: ${{ secrets.PERSONAL_ACCESS_KEY }} + aws-secret-access-key: ${{ secrets.PERSONAL_SECRET_ACCESS_KEY }} + role-to-assume: ${{ secrets.ROLE_TO_ASSUME }} + # role-to-assume: ${{ vars.OIDC_ROLE }} aws-region: ${{ vars.AWS_REGION }} role-skip-session-tagging: true From 9eac371b4db7c0624a79ad38be5f07e1be3ab9f8 Mon Sep 17 00:00:00 2001 From: Joshua Burns Date: Thu, 21 Sep 2023 15:19:15 -0700 Subject: [PATCH 7/7] added comment describing terragrunt block --- terraform/terragrunt.hcl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/terraform/terragrunt.hcl b/terraform/terragrunt.hcl index d03886c..2b67074 100644 --- a/terraform/terragrunt.hcl +++ b/terraform/terragrunt.hcl @@ -19,6 +19,8 @@ remote_state { } } +# This block will read all variables in shared.tfvars and append them +# to all terrform commands that accept inputs terraform { extra_arguments "shared_vars" { commands = get_terraform_commands_that_need_vars()