Skip to content

Commit a70e576

Browse files
committed
Add iam_only variable and remove import blocks
1 parent 772feeb commit a70e576

10 files changed

Lines changed: 81 additions & 108 deletions

‎.github/workflows/terraform-apply.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,5 @@ jobs:
3838
with:
3939
path: terraform
4040
backend_config_file: terraform/prod.backend.tfvars
41+
variables: iam_only = false
4142
auto_approve: ${{ github.event_name == 'workflow_dispatch' }}

‎.github/workflows/terraform-plan.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,4 @@ jobs:
3434
with:
3535
path: terraform
3636
backend_config_file: terraform/prod.backend.tfvars
37+
variables: iam_only = false

‎CONTRIBUTING.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,15 @@ Remember to match these values to the ones in your backend state (and replace US
260260
### **Terraform Setup and Execution Instructions**
261261
Make the required changes and execute them to see the changes in your own AWS account
262262
263+
**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:
264+
* every IAM user this repo declares
265+
* the `ops-leads` group, with `AdministratorAccess` attached
266+
* a GitHub Actions OIDC provider. AWS allows only one per account, so `terraform apply` fails with `EntityAlreadyExists` if your account already has one.
267+
268+
Run `terraform destroy` when you are finished to remove them.
269+
270+
**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.
271+
263272
- Change into `terraform` directory with
264273

265274
```bash

‎terraform/aws-custom-policies-legacy.tf‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@
99
//
1010
// ManageAccessKeys is attached to the ops-leads group by
1111
// aws_iam_group_policy_attachment.manageAccessKeys in aws-groups.tf, which
12-
// still refers to it by literal ARN. That attachment is unchanged here.
12+
// refers to this resource's arn rather than a literal ARN, so it also resolves
13+
// in a contributor's own AWS account.
1314

1415
resource "aws_iam_policy" "manage_access_keys" {
1516
name = "ManageAccessKeys"
@@ -19,8 +20,3 @@ resource "aws_iam_policy" "manage_access_keys" {
1920
description = "Policy for creating, listing, and updating Access Keys"
2021
policy = file("${path.module}/aws-custom-policies/existing-policies/manage-access-keys-policy.json")
2122
}
22-
23-
import {
24-
to = aws_iam_policy.manage_access_keys
25-
id = "arn:aws:iam::035866691871:policy/ManageAccessKeys"
26-
}

‎terraform/aws-groups-legacy.tf‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,3 @@
1313
resource "aws_iam_group" "project_leads" {
1414
name = "project-leads"
1515
}
16-
17-
import {
18-
to = aws_iam_group.project_leads
19-
id = "project-leads"
20-
}

‎terraform/aws-groups.tf‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ module "iam_read_only_group" {
1010
}
1111
}
1212

13-
//import ops-leads group
13+
// ops-leads group
1414
resource "aws_iam_group" "ops_leads_group" {
1515
name = "ops-leads"
1616
}
@@ -22,7 +22,7 @@ resource "aws_iam_group_policy_attachment" "admin"{
2222

2323
resource "aws_iam_group_policy_attachment" "manageAccessKeys"{
2424
group = aws_iam_group.ops_leads_group.name
25-
policy_arn = "arn:aws:iam::035866691871:policy/ManageAccessKeys"
25+
policy_arn = aws_iam_policy.manage_access_keys.arn
2626
}
2727

2828

‎terraform/aws-users-legacy.tf‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,3 @@ resource "aws_iam_user" "fangyiliu" {
2121
resource "aws_iam_user" "jack_pashayan" {
2222
name = "jack.pashayan"
2323
}
24-
25-
import {
26-
to = aws_iam_user.fangyiliu
27-
id = "fangyiliu"
28-
}
29-
30-
import {
31-
to = aws_iam_user.jack_pashayan
32-
id = "jack.pashayan"
33-
}

‎terraform/cloudtrail.tf‎

Lines changed: 54 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@
1717
// indefinitely), and the disagreement over log file validation, which is reproduced below
1818
// rather than resolved.
1919
//
20+
// Everything here is skipped unless var.iam_only is false (see variables.tf). The bucket
21+
// names, bucket policies and the backend bucket selector all belong to the Hack for LA
22+
// account, so none of this can be created in a contributor's own AWS account. The CI
23+
// workflows set iam_only = false. The trails and buckets carry prevent_destroy so that a
24+
// run against the Hack for LA account which forgets to do the same fails at plan time
25+
// instead of planning to destroy the account's audit logging.
26+
//
2027
// A bucket is not one resource. Since AWS provider v4 the policy, public access block,
2128
// encryption and ownership controls are each their own resource with their own import --
2229
// the same trap terraform/import.tf in hackforla/incubator documents.
@@ -26,7 +33,13 @@
2633
// ---------------------------------------------------------------------------
2734

2835
resource "aws_s3_bucket" "tf_backend_logs" {
36+
count = var.iam_only ? 0 : 1
37+
2938
bucket = "aws-cloudtrail-logs-035866691871-4b8654bf"
39+
40+
lifecycle {
41+
prevent_destroy = true
42+
}
3043
}
3144

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

4358
policy = jsonencode({
4459
Version = "2012-10-17"
@@ -73,7 +88,9 @@ resource "aws_s3_bucket_policy" "tf_backend_logs" {
7388
}
7489

7590
resource "aws_s3_bucket_public_access_block" "tf_backend_logs" {
76-
bucket = aws_s3_bucket.tf_backend_logs.id
91+
count = var.iam_only ? 0 : 1
92+
93+
bucket = aws_s3_bucket.tf_backend_logs[0].id
7794

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

89108
rule {
90109
apply_server_side_encryption_by_default {
@@ -98,7 +117,9 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "tf_backend_logs"
98117
// equivalent resource is declared for it below -- declaring one would create it rather
99118
// than import it, which is a live change.
100119
resource "aws_s3_bucket_ownership_controls" "tf_backend_logs" {
101-
bucket = aws_s3_bucket.tf_backend_logs.id
120+
count = var.iam_only ? 0 : 1
121+
122+
bucket = aws_s3_bucket.tf_backend_logs[0].id
102123

103124
rule {
104125
object_ownership = "BucketOwnerEnforced"
@@ -109,8 +130,10 @@ resource "aws_s3_bucket_ownership_controls" "tf_backend_logs" {
109130
// the provider's default_tags would plan to REMOVE it, which is why this is the one
110131
// resource in this file carrying a tags block.
111132
resource "aws_cloudtrail" "tf_backend_logs" {
133+
count = var.iam_only ? 0 : 1
134+
112135
name = "devops-security-tf-backend-logs"
113-
s3_bucket_name = aws_s3_bucket.tf_backend_logs.id
136+
s3_bucket_name = aws_s3_bucket.tf_backend_logs[0].id
114137
include_global_service_events = true
115138
is_multi_region_trail = true
116139
enable_log_file_validation = true
@@ -133,20 +156,32 @@ resource "aws_cloudtrail" "tf_backend_logs" {
133156
tags = {
134157
project = "devops-security"
135158
}
159+
160+
lifecycle {
161+
prevent_destroy = true
162+
}
136163
}
137164

138165
// ---------------------------------------------------------------------------
139166
// management-events -- the account's management events
140167
// ---------------------------------------------------------------------------
141168

142169
resource "aws_s3_bucket" "management_events" {
170+
count = var.iam_only ? 0 : 1
171+
143172
bucket = "aws-cloudtrail-logs-035866691871-6539ef03"
173+
174+
lifecycle {
175+
prevent_destroy = true
176+
}
144177
}
145178

146179
// Verbatim from the live bucket, including the "AWS:SourceArn" spelling -- see the note on
147180
// the policy above.
148181
resource "aws_s3_bucket_policy" "management_events" {
149-
bucket = aws_s3_bucket.management_events.id
182+
count = var.iam_only ? 0 : 1
183+
184+
bucket = aws_s3_bucket.management_events[0].id
150185

151186
policy = jsonencode({
152187
Version = "2012-10-17"
@@ -181,7 +216,9 @@ resource "aws_s3_bucket_policy" "management_events" {
181216
}
182217

183218
resource "aws_s3_bucket_public_access_block" "management_events" {
184-
bucket = aws_s3_bucket.management_events.id
219+
count = var.iam_only ? 0 : 1
220+
221+
bucket = aws_s3_bucket.management_events[0].id
185222

186223
block_public_acls = true
187224
block_public_policy = true
@@ -190,7 +227,9 @@ resource "aws_s3_bucket_public_access_block" "management_events" {
190227
}
191228

192229
resource "aws_s3_bucket_server_side_encryption_configuration" "management_events" {
193-
bucket = aws_s3_bucket.management_events.id
230+
count = var.iam_only ? 0 : 1
231+
232+
bucket = aws_s3_bucket.management_events[0].id
194233

195234
rule {
196235
apply_server_side_encryption_by_default {
@@ -203,8 +242,10 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "management_events
203242
// Log file validation is off on this trail and on for the other. That inconsistency is
204243
// reproduced rather than resolved -- turning it on is a live change and its own ticket.
205244
resource "aws_cloudtrail" "management_events" {
245+
count = var.iam_only ? 0 : 1
246+
206247
name = "management-events"
207-
s3_bucket_name = aws_s3_bucket.management_events.id
248+
s3_bucket_name = aws_s3_bucket.management_events[0].id
208249
include_global_service_events = true
209250
is_multi_region_trail = true
210251
enable_log_file_validation = false
@@ -217,4 +258,8 @@ resource "aws_cloudtrail" "management_events" {
217258
equals = ["Management"]
218259
}
219260
}
261+
262+
lifecycle {
263+
prevent_destroy = true
264+
}
220265
}

‎terraform/imports.tf‎

Lines changed: 0 additions & 76 deletions
This file was deleted.

‎terraform/variables.tf‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Defaults to true so that a contributor running this against their own AWS account,
2+
// which is how changes are tested (see CONTRIBUTING.md), gets IAM resources only. The
3+
// resources in cloudtrail.tf are specific to the Hack for LA account and cannot be
4+
// created anywhere else.
5+
//
6+
// Any plan or apply against the Hack for LA account MUST set this to false. Both CI
7+
// workflows do. See hackforla/devops-security#196.
8+
variable "iam_only" {
9+
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."
10+
type = bool
11+
default = true
12+
}

0 commit comments

Comments
 (0)