Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/terraform-apply.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,5 @@ jobs:
with:
path: terraform
backend_config_file: terraform/prod.backend.tfvars
variables: iam_only = false
auto_approve: ${{ github.event_name == 'workflow_dispatch' }}
1 change: 1 addition & 0 deletions .github/workflows/terraform-plan.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,4 @@ jobs:
with:
path: terraform
backend_config_file: terraform/prod.backend.tfvars
variables: iam_only = false
9 changes: 9 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,15 @@ Remember to match these values to the ones in your backend state (and replace US
### **Terraform Setup and Execution Instructions**
Make the required changes and execute them to see the changes in your own AWS account

**Note:** By default this configuration manages IAM resources only (`iam_only = true`). The CloudTrail trails and log buckets in `cloudtrail.tf` belong to the Hack for LA AWS account and are skipped, so you don't need to set anything for a test run in your own account. Everything else is still created in your account, including:
* every IAM user this repo declares
* the `ops-leads` group, with `AdministratorAccess` attached
* a GitHub Actions OIDC provider. AWS allows only one per account, so `terraform apply` fails with `EntityAlreadyExists` if your account already has one.

Run `terraform destroy` when you are finished to remove them.

**Note:** Any plan or apply against the Hack for LA AWS account must pass `-var iam_only=false`. The GitHub Actions workflows already do this.

- Change into `terraform` directory with

```bash
Expand Down
8 changes: 2 additions & 6 deletions terraform/aws-custom-policies-legacy.tf
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
//
// ManageAccessKeys is attached to the ops-leads group by
// aws_iam_group_policy_attachment.manageAccessKeys in aws-groups.tf, which
// still refers to it by literal ARN. That attachment is unchanged here.
// refers to this resource's arn rather than a literal ARN, so it also resolves
// in a contributor's own AWS account.

resource "aws_iam_policy" "manage_access_keys" {
name = "ManageAccessKeys"
Expand All @@ -19,8 +20,3 @@ resource "aws_iam_policy" "manage_access_keys" {
description = "Policy for creating, listing, and updating Access Keys"
policy = file("${path.module}/aws-custom-policies/existing-policies/manage-access-keys-policy.json")
}

import {
to = aws_iam_policy.manage_access_keys
id = "arn:aws:iam::035866691871:policy/ManageAccessKeys"
}
5 changes: 0 additions & 5 deletions terraform/aws-groups-legacy.tf
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,3 @@
resource "aws_iam_group" "project_leads" {
name = "project-leads"
}

import {
to = aws_iam_group.project_leads
id = "project-leads"
}
4 changes: 2 additions & 2 deletions terraform/aws-groups.tf
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module "iam_read_only_group" {
}
}

//import ops-leads group
// ops-leads group
resource "aws_iam_group" "ops_leads_group" {
name = "ops-leads"
}
Expand All @@ -22,7 +22,7 @@ resource "aws_iam_group_policy_attachment" "admin"{

resource "aws_iam_group_policy_attachment" "manageAccessKeys"{
group = aws_iam_group.ops_leads_group.name
policy_arn = "arn:aws:iam::035866691871:policy/ManageAccessKeys"
policy_arn = aws_iam_policy.manage_access_keys.arn
}


10 changes: 0 additions & 10 deletions terraform/aws-users-legacy.tf
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,3 @@ resource "aws_iam_user" "fangyiliu" {
resource "aws_iam_user" "jack_pashayan" {
name = "jack.pashayan"
}

import {
to = aws_iam_user.fangyiliu
id = "fangyiliu"
}

import {
to = aws_iam_user.jack_pashayan
id = "jack.pashayan"
}
63 changes: 54 additions & 9 deletions terraform/cloudtrail.tf
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@
// indefinitely), and the disagreement over log file validation, which is reproduced below
// rather than resolved.
//
// Everything here is skipped unless var.iam_only is false (see variables.tf). The bucket
// names, bucket policies and the backend bucket selector all belong to the Hack for LA
// account, so none of this can be created in a contributor's own AWS account. The CI
// workflows set iam_only = false. The trails and buckets carry prevent_destroy so that a
// run against the Hack for LA account which forgets to do the same fails at plan time
// instead of planning to destroy the account's audit logging.
//
// A bucket is not one resource. Since AWS provider v4 the policy, public access block,
// encryption and ownership controls are each their own resource with their own import --
// the same trap terraform/import.tf in hackforla/incubator documents.
Expand All @@ -26,7 +33,13 @@
// ---------------------------------------------------------------------------

resource "aws_s3_bucket" "tf_backend_logs" {
count = var.iam_only ? 0 : 1

bucket = "aws-cloudtrail-logs-035866691871-4b8654bf"

lifecycle {
prevent_destroy = true
}
}

// Reproduced verbatim from the live bucket. These two statements are what let CloudTrail
Expand All @@ -38,7 +51,9 @@ resource "aws_s3_bucket" "tf_backend_logs" {
// equivalent to AWS, and both are left exactly as stored. Normalising them would be a
// change to a live policy for no behavioural gain.
resource "aws_s3_bucket_policy" "tf_backend_logs" {
bucket = aws_s3_bucket.tf_backend_logs.id
count = var.iam_only ? 0 : 1

bucket = aws_s3_bucket.tf_backend_logs[0].id

policy = jsonencode({
Version = "2012-10-17"
Expand Down Expand Up @@ -73,7 +88,9 @@ resource "aws_s3_bucket_policy" "tf_backend_logs" {
}

resource "aws_s3_bucket_public_access_block" "tf_backend_logs" {
bucket = aws_s3_bucket.tf_backend_logs.id
count = var.iam_only ? 0 : 1

bucket = aws_s3_bucket.tf_backend_logs[0].id

block_public_acls = true
block_public_policy = true
Expand All @@ -84,7 +101,9 @@ resource "aws_s3_bucket_public_access_block" "tf_backend_logs" {
// blocked_encryption_types is deliberately not set. The provider reads it back as
// ["SSE-C"] on both buckets but it is computed, so leaving it out produces no diff.
resource "aws_s3_bucket_server_side_encryption_configuration" "tf_backend_logs" {
bucket = aws_s3_bucket.tf_backend_logs.id
count = var.iam_only ? 0 : 1

bucket = aws_s3_bucket.tf_backend_logs[0].id

rule {
apply_server_side_encryption_by_default {
Expand All @@ -98,7 +117,9 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "tf_backend_logs"
// equivalent resource is declared for it below -- declaring one would create it rather
// than import it, which is a live change.
resource "aws_s3_bucket_ownership_controls" "tf_backend_logs" {
bucket = aws_s3_bucket.tf_backend_logs.id
count = var.iam_only ? 0 : 1

bucket = aws_s3_bucket.tf_backend_logs[0].id

rule {
object_ownership = "BucketOwnerEnforced"
Expand All @@ -109,8 +130,10 @@ resource "aws_s3_bucket_ownership_controls" "tf_backend_logs" {
// the provider's default_tags would plan to REMOVE it, which is why this is the one
// resource in this file carrying a tags block.
resource "aws_cloudtrail" "tf_backend_logs" {
count = var.iam_only ? 0 : 1

name = "devops-security-tf-backend-logs"
s3_bucket_name = aws_s3_bucket.tf_backend_logs.id
s3_bucket_name = aws_s3_bucket.tf_backend_logs[0].id
include_global_service_events = true
is_multi_region_trail = true
enable_log_file_validation = true
Expand All @@ -133,20 +156,32 @@ resource "aws_cloudtrail" "tf_backend_logs" {
tags = {
project = "devops-security"
}

lifecycle {
prevent_destroy = true
}
}

// ---------------------------------------------------------------------------
// management-events -- the account's management events
// ---------------------------------------------------------------------------

resource "aws_s3_bucket" "management_events" {
count = var.iam_only ? 0 : 1

bucket = "aws-cloudtrail-logs-035866691871-6539ef03"

lifecycle {
prevent_destroy = true
}
}

// Verbatim from the live bucket, including the "AWS:SourceArn" spelling -- see the note on
// the policy above.
resource "aws_s3_bucket_policy" "management_events" {
bucket = aws_s3_bucket.management_events.id
count = var.iam_only ? 0 : 1

bucket = aws_s3_bucket.management_events[0].id

policy = jsonencode({
Version = "2012-10-17"
Expand Down Expand Up @@ -181,7 +216,9 @@ resource "aws_s3_bucket_policy" "management_events" {
}

resource "aws_s3_bucket_public_access_block" "management_events" {
bucket = aws_s3_bucket.management_events.id
count = var.iam_only ? 0 : 1

bucket = aws_s3_bucket.management_events[0].id

block_public_acls = true
block_public_policy = true
Expand All @@ -190,7 +227,9 @@ resource "aws_s3_bucket_public_access_block" "management_events" {
}

resource "aws_s3_bucket_server_side_encryption_configuration" "management_events" {
bucket = aws_s3_bucket.management_events.id
count = var.iam_only ? 0 : 1

bucket = aws_s3_bucket.management_events[0].id

rule {
apply_server_side_encryption_by_default {
Expand All @@ -203,8 +242,10 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "management_events
// Log file validation is off on this trail and on for the other. That inconsistency is
// reproduced rather than resolved -- turning it on is a live change and its own ticket.
resource "aws_cloudtrail" "management_events" {
count = var.iam_only ? 0 : 1

name = "management-events"
s3_bucket_name = aws_s3_bucket.management_events.id
s3_bucket_name = aws_s3_bucket.management_events[0].id
include_global_service_events = true
is_multi_region_trail = true
enable_log_file_validation = false
Expand All @@ -217,4 +258,8 @@ resource "aws_cloudtrail" "management_events" {
equals = ["Management"]
}
}

lifecycle {
prevent_destroy = true
}
}
76 changes: 0 additions & 76 deletions terraform/imports.tf

This file was deleted.

12 changes: 12 additions & 0 deletions terraform/variables.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Defaults to true so that a contributor running this against their own AWS account,
// which is how changes are tested (see CONTRIBUTING.md), gets IAM resources only. The
// resources in cloudtrail.tf are specific to the Hack for LA account and cannot be
// created anywhere else.
//
// Any plan or apply against the Hack for LA account MUST set this to false. Both CI
// workflows do. See hackforla/devops-security#196.
variable "iam_only" {
description = "Manage IAM resources only, skipping the Hack for LA account's CloudTrail trails and log buckets. Set to false for the Hack for LA account."
type = bool
default = true
}
Loading