Feat: Setup Cloudwatch Logs for EC2

[NickSavino]

No description provided.

[qodo-code-review]

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Review Summary by Qodo

Setup CloudWatch Logs integration for EC2 container monitoring

✨ Enhancement

Walkthroughs

Description

• Create new CloudWatch Logs module for EC2 container logging
• Configure EC2 IAM role with CloudWatch Logs permissions
• Update deployment script to attach Docker logs to CloudWatch
• Add AMI ID as configurable variable for compute module
• Clean up Terraform lock file and apply formatting

Diagram

flowchart LR
  A["EC2 Instance"] -->|"logs via awslogs driver"| B["CloudWatch Log Group"]
  C["IAM Role"] -->|"permissions"| B
  D["Deploy Script"] -->|"configures log driver"| A
  E["Logs Module"] -->|"creates & manages"| B

Loading

File Changes

1. infra/modules/logs/main.tf ✨ Enhancement +31/-0

Create CloudWatch Logs module with IAM permissions

infra/modules/logs/main.tf

---

2. infra/modules/logs/outputs.tf ✨ Enhancement +7/-0

Export log group name and ARN outputs

infra/modules/logs/outputs.tf

---

3. infra/modules/logs/variables.tf ✨ Enhancement +18/-0

Define logs module input variables

infra/modules/logs/variables.tf

---

View more (10)

4. scripts/ci/deploy_via_ssm.sh ✨ Enhancement +4/-1

Add CloudWatch Logs driver to Docker container

scripts/ci/deploy_via_ssm.sh

---

5. infra/env/prod/main.tf ✨ Enhancement +16/-1

Integrate logs module and pass AMI ID variable

infra/env/prod/main.tf

---

6. infra/env/prod/variables.tf ✨ Enhancement +9/-0

Add log retention and AMI ID variables

infra/env/prod/variables.tf

---

7. infra/modules/compute/main.tf ✨ Enhancement +1/-1

Use configurable AMI ID instead of data source

infra/modules/compute/main.tf

---

8. infra/modules/compute/variables.tf ✨ Enhancement +5/-1

Add AMI ID as required input variable

infra/modules/compute/variables.tf

---

9. infra/env/prod/.terraform.lock.hcl Dependencies +0/-33

Remove unused local and tls provider dependencies

infra/env/prod/.terraform.lock.hcl

---

10. infra/modules/github_oidc/main.tf Formatting +182/-182

Apply Terraform formatting to file

infra/modules/github_oidc/main.tf

---

11. infra/modules/network/main.tf Formatting +206/-206

Apply Terraform formatting to file

infra/modules/network/main.tf

---

12. infra/modules/network/variables.tf Formatting +42/-42

Apply Terraform formatting to file

infra/modules/network/variables.tf

---

13. infra/modules/rds/main.tf Formatting +77/-77

Apply Terraform formatting to file

infra/modules/rds/main.tf

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

1. Missing LOG_GROUP_NAME env ☑ 🐞 Bug ⛯ Reliability

Description

scripts/ci/deploy_via_ssm.sh now hard-requires LOG_GROUP_NAME, but .github/workflows/deploy.yml
never sets it, so the deploy job will exit before sending the SSM command.

Code

scripts/ci/deploy_via_ssm.sh[10]

+: "${LOG_GROUP_NAME:?}"

Evidence

The deploy script now fails fast if LOG_GROUP_NAME is unset and uses it in the docker awslogs
configuration, but the deploy workflow only exports
AWS_REGION/ECR_REPO/EC2_INSTANCE_ID/CONTAINER_NAME and then runs the script without providing
LOG_GROUP_NAME.

scripts/ci/deploy_via_ssm.sh[5-10]
scripts/ci/deploy_via_ssm.sh[17-36]
.github/workflows/deploy.yml[7-12]
.github/workflows/deploy.yml[77-81]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`scripts/ci/deploy_via_ssm.sh` now requires `LOG_GROUP_NAME`, but the deploy workflow doesn’t set it, causing deployments to fail immediately.
### Issue Context
The script enforces the variable via `: "${LOG_GROUP_NAME:?}"` and configures Docker’s `awslogs` driver with `--log-opt awslogs-group=${LOG_GROUP_NAME}`.
### Fix Focus Areas
- .github/workflows/deploy.yml[7-12]
- .github/workflows/deploy.yml[77-81]
### Suggested fix
Add `LOG_GROUP_NAME` to the workflow `env:` (e.g., from a GitHub Secret like `secrets.LOG_GROUP_NAME`), or add a step to fetch the log group name (e.g., from Terraform outputs/state) and export it before running `deploy_via_ssm.sh`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Required ami_id breaks plans 🐞 Bug ⛯ Reliability

Description

infra/env/prod introduces a required ami_id (no default) and wires it into module.compute, so
tofu plan/apply will fail in non-interactive contexts unless the caller explicitly provides
ami_id.

Code

infra/env/prod/variables.tf[R22-23]

+variable "ami_id" {
+  type = string

Evidence

ami_id is declared without a default in the prod environment and is passed directly into the
compute module, where the EC2 instance AMI is set from var.ami_id; Terraform/OpenTofu will error
if a required variable is not set when running non-interactively.

infra/env/prod/variables.tf[17-24]
infra/env/prod/main.tf[51-68]
infra/modules/compute/variables.tf[34-41]
infra/modules/compute/main.tf[103-110]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The prod environment now requires `ami_id` with no default, which makes `tofu plan/apply` fail unless the operator/automation always supplies `-var ami_id=...` (or `TF_VAR_ami_id`).
### Issue Context
Previously, the module selected an AL2023 AMI via a data source. After the change, the module uses `var.ami_id`, but the variable is required at both env and module levels.
### Fix Focus Areas
- infra/env/prod/variables.tf[17-24]
- infra/env/prod/main.tf[51-68]
- infra/modules/compute/variables.tf[34-41]
- infra/modules/compute/main.tf[1-16]
- infra/modules/compute/main.tf[103-110]
### Suggested fix
Implement a safe default path so callers are not forced to pass `ami_id`:
- Option A (module-level): set `variable "ami_id" { type = string, default = null }` and set `aws_instance.this.ami = coalesce(var.ami_id, data.aws_ami.al2023.id)`.
- Option B (env-level): keep module variable required, but add a data source in `infra/env/prod` and pass `ami_id = coalesce(var.ami_id, data.aws_ami.al2023.id)`, while giving env `ami_id` a `default = null`.
Either approach removes the hard requirement while still allowing pinning when desired.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Unused AMI data source 🐞 Bug ⚙ Maintainability

Description

infra/modules/compute still declares data.aws_ami.al2023, but the instance now uses var.ami_id,
leaving dead code and an unnecessary AWS API lookup during plans.

Code

infra/modules/compute/main.tf[105]

+  ami                         = var.ami_id

Evidence

The module continues to declare an AMI lookup data source, but the EC2 instance AMI no longer
references it after switching to var.ami_id, so the data source is unused.

infra/modules/compute/main.tf[2-16]
infra/modules/compute/main.tf[103-106]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`data "aws_ami" "al2023"` is now unused after changing the instance AMI to `var.ami_id`, which adds noise and an unnecessary lookup.
### Issue Context
This became unused when `aws_instance.this.ami` stopped referencing the data source.
### Fix Focus Areas
- infra/modules/compute/main.tf[2-16]
- infra/modules/compute/main.tf[103-106]
### Suggested fix
Either:
- Remove the `data "aws_ami" "al2023"` block entirely, or
- Use it as a fallback by setting `ami = coalesce(var.ami_id, data.aws_ami.al2023.id)` (which also addresses the required-variable sharp edge).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[qodo-code-review]

2. Required ami_id breaks plans 🐞 Bug ⛯ Reliability

infra/env/prod introduces a required ami_id (no default) and wires it into module.compute, so
tofu plan/apply will fail in non-interactive contexts unless the caller explicitly provides
ami_id.

Agent Prompt

### Issue description
The prod environment now requires `ami_id` with no default, which makes `tofu plan/apply` fail unless the operator/automation always supplies `-var ami_id=...` (or `TF_VAR_ami_id`).

### Issue Context
Previously, the module selected an AL2023 AMI via a data source. After the change, the module uses `var.ami_id`, but the variable is required at both env and module levels.

### Fix Focus Areas
- infra/env/prod/variables.tf[17-24]
- infra/env/prod/main.tf[51-68]
- infra/modules/compute/variables.tf[34-41]
- infra/modules/compute/main.tf[1-16]
- infra/modules/compute/main.tf[103-110]

### Suggested fix
Implement a safe default path so callers are not forced to pass `ami_id`:
- Option A (module-level): set `variable "ami_id" { type = string, default = null }` and set `aws_instance.this.ami = coalesce(var.ami_id, data.aws_ami.al2023.id)`.
- Option B (env-level): keep module variable required, but add a data source in `infra/env/prod` and pass `ami_id = coalesce(var.ami_id, data.aws_ami.al2023.id)`, while giving env `ami_id` a `default = null`.

Either approach removes the hard requirement while still allowing pinning when desired.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools