Skip to content

Production Deploy - Cloudwatch Logging#74

Merged
NickSavino merged 23 commits into
mainfrom
fix/prod-conflicts
Mar 26, 2026
Merged

Production Deploy - Cloudwatch Logging#74
NickSavino merged 23 commits into
mainfrom
fix/prod-conflicts

Conversation

@NickSavino

Copy link
Copy Markdown
Contributor

No description provided.

NickSavino and others added 22 commits February 4, 2026 03:35
Syncing main to development
Feat/middleware-wiring-webhook-setup
* added user calibration endpoints and schema"
- Added user_calibration migration
- Added sqlc queries for calibration CRUD operations
- added /api/users/me/calibration endpoints: GET, POST, DELETE
- me endpoint now fetches user from clerk id using middleware, enhancing security
- removed GET methods for user (GET /{id}, GET /)

* Fixed line endings

* Fixed time formatting in handler

* Added NOT NULL constrain to updated_at, created_at in db table

switch go dev container port to correct mapping (8080)

fixed variable typo in middleware_auth.go

---------

Co-authored-by: NicolaSavino <nick.savino@arcurve.com>
Co-authored-by: NicolaSavino <nick.savino@arcurve.com>
* Implemented workout and calibration schema with included sqlc queries. Updated makefile and seeding files

* Regenerated SQLC to match new queries and models (workout type + workout session)

* Added workout session service

* Added Workout Handler Endpoint

* Addressed PR comments
- fixed syntax error in makefile
-  Fixed migration schema
- fixed seeding script
- added /service/helps/helpers.util file
- renamed Dtos to match go standards, (Id -> ID)

* Updated API endpoints

* updated dtos to match casing convention. updated workout_session logic

* Added functionality for workout sets and reps. (services, handlers, migrations, queries)

* Addressed PR comments

* Addressed PR comments

---------

Co-authored-by: NicolaSavino <nick.savino@arcurve.com>
* fix: added clerk webhook secret

* updated CRLF line endings to LF
* fix: added clerk webhook secret

* updated CRLF line endings to LF

* fix
…pment

# Conflicts:
#	scripts/ci/deploy_via_ssm.sh
* Prod fix (#67)

* Small changes to dev tooling (docker/make/compose

* Server: minor variable rename

* Auth middleware + clerk webhook handler

* Added WithTransaction to store

* Removed debug logging message exposing userid

* Removed docker-compose changes not relevant to PR

* Renamed users.go to users.handler.go in handlers

* Added documentation explaining dev bypass

* Feat/calibration-endpoints (#57)

* added user calibration endpoints and schema"
- Added user_calibration migration
- Added sqlc queries for calibration CRUD operations
- added /api/users/me/calibration endpoints: GET, POST, DELETE
- me endpoint now fetches user from clerk id using middleware, enhancing security
- removed GET methods for user (GET /{id}, GET /)

* Fixed line endings

* Fixed time formatting in handler

* Added NOT NULL constrain to updated_at, created_at in db table

switch go dev container port to correct mapping (8080)

fixed variable typo in middleware_auth.go

---------

Co-authored-by: NicolaSavino <nick.savino@arcurve.com>

* Fixed syntax error on migration (#58)

Co-authored-by: NicolaSavino <nick.savino@arcurve.com>

* Feat/workouts api (#59)

* Implemented workout and calibration schema with included sqlc queries. Updated makefile and seeding files

* Regenerated SQLC to match new queries and models (workout type + workout session)

* Added workout session service

* Added Workout Handler Endpoint

* Addressed PR comments
- fixed syntax error in makefile
-  Fixed migration schema
- fixed seeding script
- added /service/helps/helpers.util file
- renamed Dtos to match go standards, (Id -> ID)

* Updated API endpoints

* updated dtos to match casing convention. updated workout_session logic

* Added functionality for workout sets and reps. (services, handlers, migrations, queries)

* Addressed PR comments

* Addressed PR comments

---------

Co-authored-by: NicolaSavino <nick.savino@arcurve.com>

* fix: added clerk webhook secret (#62)

* Fix/add clerkwebhook secret (#64)

* fix: added clerk webhook secret

* updated CRLF line endings to LF

* Fix/add clerkwebhook secret (#66)

* fix: added clerk webhook secret

* updated CRLF line endings to LF

* fix

---------

Co-authored-by: NicolaSavino <nick.savino@arcurve.com>

* fix: wipe data on seed

---------

Co-authored-by: Nicola Savino <77707655+NickSavino@users.noreply.github.com>
Co-authored-by: NicolaSavino <nick.savino@arcurve.com>
- updated workout session history endpoint
- added reset_workflow script along with github action
- added ingestion functionality for webhook
* feat: setup AWS Cloudwatch resource and updated deploy script to attach to logging
@qodo-code-review

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

Review Summary by Qodo

Production Deploy - CloudWatch Logging and User Deletion Webhook

✨ Enhancement 🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Add CloudWatch logging to production Docker deployment
• Implement user deletion webhook handler for Clerk integration
• Refactor workout session history query to include aggregated sets/reps
• Create new logs infrastructure module with IAM policies
• Add database reset workflow for production environment
• Fix line endings and formatting across infrastructure files
Diagram
flowchart LR
  A["Clerk Webhook"] -->|user.deleted event| B["HandleDeleteUserFromClerk"]
  B -->|DeleteUserByClerkID| C["Database"]
  D["Docker Container"] -->|awslogs driver| E["CloudWatch Logs"]
  F["Logs Module"] -->|IAM Policy| G["EC2 Role"]
  G -->|Permissions| E
  H["Reset Database Workflow"] -->|Manual Trigger| I["Database Migration"]
Loading

Grey Divider

File Changes

1. internal/core/service/user.service.go ✨ Enhancement +57/-48

Add user deletion handler for Clerk webhook

internal/core/service/user.service.go


2. internal/core/service/workout/workout_session.service.go ✨ Enhancement +38/-2

Add workout history DTO with aggregated metrics

internal/core/service/workout/workout_session.service.go


3. internal/db/users.sql.go ✨ Enhancement +9/-0

Add DeleteUserByClerkID database query

internal/db/users.sql.go


View more (22)
4. internal/db/workout_session.sql.go ✨ Enhancement +33/-33

Refactor history query with sets/reps aggregation

internal/db/workout_session.sql.go


5. internal/http/handlers/auth.handler.go ✨ Enhancement +7/-1

Handle user.deleted webhook event from Clerk

internal/http/handlers/auth.handler.go


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

Remove unused local and tls provider dependencies

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


7. scripts/ci/deploy_via_ssm.sh ✨ Enhancement +40/-37

Add CloudWatch logging configuration to Docker

scripts/ci/deploy_via_ssm.sh


8. scripts/ci/reset_database.sh ✨ Enhancement +28/-0

New script for production database reset via SSM

scripts/ci/reset_database.sh


9. .gitattributes ⚙️ Configuration changes +1/-0

Enforce Unix line endings for shell scripts

.gitattributes


10. .github/workflows/deploy.yml ⚙️ Configuration changes +84/-83

Add LOG_GROUP_NAME environment variable to workflow

.github/workflows/deploy.yml


11. .github/workflows/reset-database.yml ✨ Enhancement +37/-0

New manual workflow for production database reset

.github/workflows/reset-database.yml


12. db/queries/users.sql ✨ Enhancement +3/-0

Add DeleteUserByClerkID SQL query definition

db/queries/users.sql


13. db/queries/workout_session.sql ✨ Enhancement +16/-8

Refactor history query with aggregated metrics

db/queries/workout_session.sql


14. db/seeds/seed.sql ✨ Enhancement +6/-0

Add TRUNCATE CASCADE for clean seed data

db/seeds/seed.sql


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

Add logs module and ami_id variable to compute

infra/env/prod/main.tf


16. infra/env/prod/variables.tf ⚙️ Configuration changes +9/-0

Add api_log_retention and ami_id variables

infra/env/prod/variables.tf


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

Use ami_id variable instead of data source

infra/modules/compute/main.tf


18. infra/modules/compute/variables.tf ⚙️ Configuration changes +5/-1

Add ami_id variable for EC2 instance

infra/modules/compute/variables.tf


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

Fix line endings and formatting consistency

infra/modules/github_oidc/main.tf


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

New CloudWatch logs module with IAM policies

infra/modules/logs/main.tf


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

Export log group name and ARN outputs

infra/modules/logs/outputs.tf


22. infra/modules/logs/variables.tf ⚙️ Configuration changes +18/-0

Define logs module input variables

infra/modules/logs/variables.tf


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

Fix line endings and formatting consistency

infra/modules/network/main.tf


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

Fix line endings and formatting consistency

infra/modules/network/variables.tf


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

Fix line endings and formatting consistency

infra/modules/rds/main.tf


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Mar 26, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Prod DB reset workflow 🐞 Bug ⛨ Security
Description
The new workflow_dispatch job can run a destructive Goose reset against the production database
(via SSM-fetched /cgc-2026-prod/api/database_url), which can irreversibly delete prod data when
triggered. The only guardrails are a single text input and a main-branch check, which is weak for an
operation of this impact.
Code

.github/workflows/reset-database.yml[R12-37]

+  reset-database:
+    if: ${{ inputs.confirm_reset == 'RESET' && github.ref == 'refs/heads/main'}}
+    runs-on: ubuntu-latest
+
+    permissions:
+      id-token: write
+      contents: read
+    
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - name: Configure AWS credentials with OIDC
+        uses: aws-actions/configure-aws-credentials@v4
+        with:
+          role-to-assume: ${{ secrets.AWS_DEPLOY_ROLE_ARN }}
+          aws-region: ${{ secrets.AWS_REGION }}
+      
+      - name: Run database reset
+        env:
+          AWS_REGION: ${{ secrets.AWS_REGION }}
+          ECR_REPO: ${{ secrets.ECR_REPO }}
+          EC2_INSTANCE_ID: ${{ secrets.EC2_INSTANCE_ID }}
+          GITHUB_SHA: ${{ github.sha }}
+        run: |
+          bash ./scripts/ci/reset_database.sh
Evidence
The workflow is manually triggerable and executes a script that explicitly fetches the production DB
URL from SSM and runs goose ... reset, which drops/rolls back all migrations (destructive) before
re-applying them.

.github/workflows/reset-database.yml[1-37]
scripts/ci/reset_database.sh[12-28]

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

## Issue description
A manually-triggered GitHub Actions workflow can reset the **production** database (Goose `reset`) with minimal protection. This is a high-impact destructive operation that should require stronger, defense-in-depth controls.

## Issue Context
The workflow runs on `workflow_dispatch` and calls a script that pulls `/cgc-2026-prod/api/database_url` and runs `goose ... reset`.

## Fix Focus Areas
- .github/workflows/reset-database.yml[1-37]
- scripts/ci/reset_database.sh[12-23]

## Suggested changes
- Gate the job behind a protected GitHub **environment** (e.g., `environment: production`) with required reviewers/approvals.
- Add an allowlist check for `github.actor` (or require membership in a specific team) before permitting execution.
- Strengthen confirmation: require typing the full expected value (e.g., `RESET cgc-2026-prod`) and/or a second independent input.
- Consider removing `reset` entirely and providing a safer, non-destructive maintenance workflow (or restricting this workflow to non-prod environments).

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



Remediation recommended

2. Prod AMI ID now required 🐞 Bug ⛯ Reliability
Description
infra/env/prod now requires an explicit ami_id input (no default) and passes it to the compute
module, making prod infra configuration dependent on external variable injection. Without clearly
defined repo-side defaults or documented configuration, applying this environment becomes easy to
misconfigure (e.g., wrong AMI or missing value).
Code

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

+variable "ami_id" {
+  type = string
}
Evidence
The prod environment defines ami_id without a default and wires it into module.compute, while
the compute module also requires ami_id, so this environment cannot be applied without supplying
an AMI ID from outside the repo/configuration.

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

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 a manually provided `ami_id` with no repo-local default or documentation, increasing the chance of failed applies or wrong-AMI deployments.

## Issue Context
`infra/env/prod` passes `var.ami_id` into the compute module, and the compute module requires it.

## Fix Focus Areas
- infra/env/prod/variables.tf[17-24]
- infra/env/prod/main.tf[51-68]
- infra/modules/compute/variables.tf[39-41]

## Suggested changes
- Add documentation in-repo for how `ami_id` is set for prod (e.g., expected TF_VAR_ami_id, CI vars, or a standard tfvars mechanism).
- Optionally provide a safe default strategy (e.g., keep using a data source lookup in prod, or add validation constraints) to reduce misconfiguration risk.
- If the intent is pinning, add validation (regex for `ami-...`) and a comment explaining update process.

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



Advisory comments

3. Dead AMI data source 🐞 Bug ⚙ Maintainability
Description
The compute module now sets aws_instance.ami = var.ami_id but still declares `data "aws_ami"
"al2023"`, which is unused and adds confusion/maintenance overhead. This also causes an unnecessary
AMI lookup during planning/applying.
Code

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

+  ami                         = var.ami_id
Evidence
The module still defines an Amazon Linux 2023 AMI data source, but the instance no longer references
it after switching to var.ami_id.

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

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 switching the instance AMI to `var.ami_id`.

## Issue Context
Keeping unused data sources increases cognitive load and can trigger unnecessary provider API calls.

## Fix Focus Areas
- infra/modules/compute/main.tf[2-16]
- infra/modules/compute/main.tf[103-106]

## Suggested changes
- Remove `data "aws_ami" "al2023"` if the module will exclusively use `var.ami_id`.
- Alternatively, revert to using the data source (and remove `ami_id`) if dynamic AMI selection is preferred.

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


Grey Divider

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

Grey Divider

Qodo Logo

@NickSavino
NickSavino merged commit c285567 into main Mar 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants