Skip to content

Commit 3e899c8

Browse files
SashkoMarchukclaude
andcommitted
Fix CodeRabbit review comments, apply underscore naming convention, add GH Actions pipeline
Address all 6 CodeRabbit issues: KMS ARN validation, secrets bootstrap docs, parameterized secret paths, TF version upper bound, multipart upload cleanup, and DynamoDB PITR. Switch resource naming to underscore convention per Nomad standards (S3 keeps hyphens). Add Terraform CI/CD workflow for fmt/validate/plan/apply. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 2b18960 commit 3e899c8

13 files changed

Lines changed: 151 additions & 54 deletions

File tree

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
name: "Terraform — MN Vectorization"
2+
3+
on:
4+
pull_request:
5+
paths:
6+
- "mn-vectorization/infra/**"
7+
push:
8+
branches:
9+
- main
10+
paths:
11+
- "mn-vectorization/infra/**"
12+
13+
permissions:
14+
contents: read
15+
pull-requests: write
16+
17+
env:
18+
TF_WORKING_DIR: mn-vectorization/infra
19+
AWS_REGION: us-east-1
20+
21+
jobs:
22+
terraform:
23+
name: Terraform
24+
runs-on: ubuntu-latest
25+
defaults:
26+
run:
27+
working-directory: ${{ env.TF_WORKING_DIR }}
28+
29+
steps:
30+
- name: Checkout
31+
uses: actions/checkout@v4
32+
33+
- name: Setup Terraform
34+
uses: hashicorp/setup-terraform@v3
35+
with:
36+
terraform_version: "~1.5"
37+
38+
- name: Configure AWS Credentials
39+
uses: aws-actions/configure-aws-credentials@v4
40+
with:
41+
aws-access-key-id: ${{ secrets.MN_VECTORIZATION_AWS_ACCESS_KEY_ID }}
42+
aws-secret-access-key: ${{ secrets.MN_VECTORIZATION_AWS_SECRET_ACCESS_KEY }}
43+
aws-region: ${{ env.AWS_REGION }}
44+
45+
- name: Terraform Format Check
46+
run: terraform fmt -check -recursive
47+
48+
- name: Terraform Init
49+
run: |
50+
terraform init \
51+
-backend-config="bucket=sf-terraform-state" \
52+
-backend-config="key=mn-vectorization/dev/terraform.tfstate" \
53+
-backend-config="region=${{ env.AWS_REGION }}"
54+
55+
- name: Terraform Validate
56+
run: terraform validate
57+
58+
- name: Terraform Plan
59+
if: github.event_name == 'pull_request'
60+
run: terraform plan -var-file=environments/dev.tfvars -no-color -input=false
61+
continue-on-error: true
62+
63+
- name: Terraform Apply
64+
if: github.ref == 'refs/heads/main' && github.event_name == 'push'
65+
run: terraform apply -var-file=environments/dev.tfvars -auto-approve -input=false

mn-vectorization/infra/cloudwatch.tf

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ resource "aws_cloudwatch_log_group" "main" {
1616
# Alarm 1: Indexing failures (custom metric from Temporal worker)
1717
resource "aws_cloudwatch_metric_alarm" "indexing_failures" {
1818
count = var.is_alarm_enabled ? 1 : 0
19-
alarm_name = "${local.name_prefix}-indexing-failures"
19+
alarm_name = "${local.name_prefix}_indexing_failures_alarm"
2020
comparison_operator = "GreaterThanThreshold"
2121
evaluation_periods = 1
2222
metric_name = "IndexingFailures"
@@ -29,13 +29,13 @@ resource "aws_cloudwatch_metric_alarm" "indexing_failures" {
2929

3030
alarm_actions = var.alarm_sns_topic_arn != "" ? [var.alarm_sns_topic_arn] : []
3131

32-
tags = { Name = "${local.name_prefix}-indexing-failures" }
32+
tags = { Name = "${local.name_prefix}_indexing_failures_alarm" }
3333
}
3434

3535
# Alarm 2: Query latency p99 (custom metric from MCP server)
3636
resource "aws_cloudwatch_metric_alarm" "query_latency_p99" {
3737
count = var.is_alarm_enabled ? 1 : 0
38-
alarm_name = "${local.name_prefix}-query-latency-p99"
38+
alarm_name = "${local.name_prefix}_query_latency_p99_alarm"
3939
comparison_operator = "GreaterThanThreshold"
4040
evaluation_periods = 2
4141
metric_name = "QueryLatencyP99"
@@ -48,13 +48,13 @@ resource "aws_cloudwatch_metric_alarm" "query_latency_p99" {
4848

4949
alarm_actions = var.alarm_sns_topic_arn != "" ? [var.alarm_sns_topic_arn] : []
5050

51-
tags = { Name = "${local.name_prefix}-query-latency-p99" }
51+
tags = { Name = "${local.name_prefix}_query_latency_p99_alarm" }
5252
}
5353

5454
# Alarm 3: DynamoDB throttling (per table)
5555
resource "aws_cloudwatch_metric_alarm" "dynamodb_throttling" {
5656
for_each = var.is_alarm_enabled ? local.dynamodb_tables : {}
57-
alarm_name = "${local.name_prefix}-${each.key}-throttling"
57+
alarm_name = "${local.name_prefix}_${each.key}_throttling_alarm"
5858
comparison_operator = "GreaterThanThreshold"
5959
evaluation_periods = 1
6060
metric_name = "ThrottledRequests"
@@ -71,5 +71,5 @@ resource "aws_cloudwatch_metric_alarm" "dynamodb_throttling" {
7171

7272
alarm_actions = var.alarm_sns_topic_arn != "" ? [var.alarm_sns_topic_arn] : []
7373

74-
tags = { Name = "${local.name_prefix}-${each.key}-throttling" }
74+
tags = { Name = "${local.name_prefix}_${each.key}_throttling_alarm" }
7575
}

mn-vectorization/infra/dynamodb.tf

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
resource "aws_dynamodb_table" "main" {
77
for_each = local.dynamodb_tables
8-
name = "${local.name_prefix}-${each.key}"
8+
name = "${local.name_prefix}_${each.key}_ddb"
99
billing_mode = "PAY_PER_REQUEST"
1010
hash_key = each.value.hash_key
1111

@@ -22,5 +22,9 @@ resource "aws_dynamodb_table" "main" {
2222
}
2323
}
2424

25-
tags = { Name = "${local.name_prefix}-${each.key}" }
25+
point_in_time_recovery {
26+
enabled = var.environment == "prod"
27+
}
28+
29+
tags = { Name = "${local.name_prefix}_${each.key}_ddb" }
2630
}

mn-vectorization/infra/environments/dev.tfvars

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
environment = "dev"
2-
aws_region = "us-east-1"
3-
billing_tag = "mn-vectorization"
1+
environment = "dev"
2+
aws_region = "us-east-1"
3+
billing_tag = "mn-vectorization"
44

55
# Existing infrastructure — replace with actual IDs
66
vpc_id = "vpc-385f9a56"

mn-vectorization/infra/environments/prod.tfvars

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
environment = "prod"
2-
aws_region = "us-east-1"
3-
billing_tag = "mn-vectorization"
1+
environment = "prod"
2+
aws_region = "us-east-1"
3+
billing_tag = "mn-vectorization"
44

55
# Existing infrastructure — replace with actual IDs
66
vpc_id = "vpc-XXXXXXXXXXXXXXXXX"

mn-vectorization/infra/environments/staging.tfvars

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
environment = "staging"
2-
aws_region = "us-east-1"
3-
billing_tag = "mn-vectorization"
1+
environment = "staging"
2+
aws_region = "us-east-1"
3+
billing_tag = "mn-vectorization"
44

55
# Existing infrastructure — replace with actual IDs
66
vpc_id = "vpc-XXXXXXXXXXXXXXXXX"

mn-vectorization/infra/iam.tf

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
# -----------------------------------------------------
44

55
resource "aws_iam_role" "worker" {
6-
name = "${local.name_prefix}-worker-role"
6+
name = "${local.name_prefix}_worker_role"
77

88
assume_role_policy = jsonencode({
99
Version = "2012-10-17"
@@ -14,11 +14,11 @@ resource "aws_iam_role" "worker" {
1414
}]
1515
})
1616

17-
tags = { Name = "${local.name_prefix}-worker-role" }
17+
tags = { Name = "${local.name_prefix}_worker_role" }
1818
}
1919

2020
resource "aws_iam_instance_profile" "worker" {
21-
name = "${local.name_prefix}-worker-profile"
21+
name = "${local.name_prefix}_worker_profile"
2222
role = aws_iam_role.worker.name
2323
}
2424

@@ -68,16 +68,16 @@ resource "aws_iam_role_policy" "dynamodb_access" {
6868
Resource = [
6969
for t in aws_dynamodb_table.main : t.arn
7070
]
71-
},
72-
{
73-
Effect = "Allow"
74-
Action = [
75-
"dynamodb:Query",
76-
"dynamodb:Scan"
77-
]
78-
Resource = [
79-
for t in aws_dynamodb_table.main : "${t.arn}/index/*"
80-
]
71+
},
72+
{
73+
Effect = "Allow"
74+
Action = [
75+
"dynamodb:Query",
76+
"dynamodb:Scan"
77+
]
78+
Resource = [
79+
for t in aws_dynamodb_table.main : "${t.arn}/index/*"
80+
]
8181
}]
8282
})
8383
}
@@ -100,11 +100,11 @@ resource "aws_iam_role_policy" "bedrock_access" {
100100
"arn:aws:bedrock:${data.aws_region.current.name}::foundation-model/anthropic.*",
101101
"arn:aws:bedrock:${data.aws_region.current.name}::foundation-model/cohere.*"
102102
]
103-
},
104-
{
105-
Effect = "Allow"
106-
Action = ["bedrock:ApplyGuardrail"]
107-
Resource = "arn:aws:bedrock:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:guardrail/*"
103+
},
104+
{
105+
Effect = "Allow"
106+
Action = ["bedrock:ApplyGuardrail"]
107+
Resource = "arn:aws:bedrock:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:guardrail/*"
108108
}]
109109
})
110110
}
@@ -171,4 +171,11 @@ resource "aws_iam_role_policy" "kms_access" {
171171
Resource = [var.kms_key_arn]
172172
}]
173173
})
174+
175+
lifecycle {
176+
precondition {
177+
condition = var.kms_key_arn != ""
178+
error_message = "kms_key_arn must be set when is_kms_enabled = true"
179+
}
180+
}
174181
}

mn-vectorization/infra/locals.tf

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
locals {
2-
name_prefix = "${var.project_name}-${var.environment}"
2+
name_prefix = "${var.project_name}_${var.environment}"
3+
name_prefix_s3 = "${replace(var.project_name, "_", "-")}-${var.environment}"
34

45
# DynamoDB tables — iterated via for_each
56
dynamodb_tables = {
@@ -16,19 +17,19 @@ locals {
1617
# Secrets Manager entries — iterated via for_each
1718
secrets = {
1819
anthropic_api_key = {
19-
name = "mn-vectorization/${var.environment}/anthropic-api-key"
20+
name = "${var.project_name}/${var.environment}/anthropic-api-key"
2021
value = var.anthropic_api_key
2122
}
2223
cohere_api_key = {
23-
name = "mn-vectorization/${var.environment}/cohere-api-key"
24+
name = "${var.project_name}/${var.environment}/cohere-api-key"
2425
value = var.cohere_api_key
2526
}
2627
qdrant_api_key = {
27-
name = "mn-vectorization/${var.environment}/qdrant-api-key"
28+
name = "${var.project_name}/${var.environment}/qdrant-api-key"
2829
value = var.qdrant_api_key
2930
}
3031
qdrant_url = {
31-
name = "mn-vectorization/${var.environment}/qdrant-url"
32+
name = "${var.project_name}/${var.environment}/qdrant-url"
3233
value = var.qdrant_url
3334
}
3435
}

mn-vectorization/infra/provider.tf

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
terraform {
2-
required_version = ">= 1.5"
2+
required_version = ">= 1.5, < 2.0"
33

44
required_providers {
55
aws = {

mn-vectorization/infra/s3.tf

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
# -----------------------------------------------------
66

77
resource "aws_s3_bucket" "artifacts" {
8-
bucket = "${local.name_prefix}-bk"
9-
tags = { Name = "${local.name_prefix}-bk" }
8+
bucket = "${local.name_prefix_s3}-bk"
9+
tags = { Name = "${local.name_prefix_s3}-bk" }
1010
}
1111

1212
resource "aws_s3_bucket_versioning" "artifacts" {
@@ -37,19 +37,28 @@ resource "aws_s3_bucket_public_access_block" "artifacts" {
3737
}
3838

3939
resource "aws_s3_bucket_lifecycle_configuration" "artifacts" {
40-
count = var.embeddings_expiry_days > 0 ? 1 : 0
4140
bucket = aws_s3_bucket.artifacts.id
4241

4342
rule {
44-
id = "expire-embeddings"
43+
id = "abort-incomplete-multipart"
4544
status = "Enabled"
46-
47-
filter {
48-
prefix = "embeddings/"
45+
filter {}
46+
abort_incomplete_multipart_upload {
47+
days_after_initiation = 7
4948
}
49+
}
5050

51-
expiration {
52-
days = var.embeddings_expiry_days
51+
dynamic "rule" {
52+
for_each = var.embeddings_expiry_days > 0 ? [1] : []
53+
content {
54+
id = "expire-embeddings"
55+
status = "Enabled"
56+
filter {
57+
prefix = "embeddings/"
58+
}
59+
expiration {
60+
days = var.embeddings_expiry_days
61+
}
5362
}
5463
}
5564
}

0 commit comments

Comments
 (0)