Skip to content

Increaed EC2 Tier and added elastic ip#76

Merged
NickSavino merged 2 commits into
developmentfrom
infra/upgrade-ec2
Mar 29, 2026
Merged

Increaed EC2 Tier and added elastic ip#76
NickSavino merged 2 commits into
developmentfrom
infra/upgrade-ec2

Conversation

@NickSavino

Copy link
Copy Markdown
Contributor

No description provided.

@qodo-code-review

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

Review Summary by Qodo

Upgrade EC2 tier and add Elastic IP allocation

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Upgraded EC2 instance type to t3.xlarge for improved performance
• Added Elastic IP allocation and association to EC2 instance
• Removed hardcoded AMI lookup, now accepts AMI ID as variable
• Refactored instance type to configurable variable parameter
Diagram
flowchart LR
  var["Variables: instance_type, ami_id"]
  compute["Compute Module"]
  ec2["EC2 Instance t3.xlarge"]
  eip["Elastic IP"]
  assoc["EIP Association"]
  output["Public IP Output"]
  
  var -- "instance_type, ami_id" --> compute
  compute -- "creates" --> ec2
  compute -- "creates" --> eip
  compute -- "associates" --> assoc
  eip -- "provides" --> output
Loading

Grey Divider

File Changes

1. infra/env/prod/main.tf ⚙️ Configuration changes +3/-3

Pass instance type to compute module

• Added instance_type variable parameter to compute module
• Aligned variable assignments with consistent formatting
• Removed blank line between variable assignments

infra/env/prod/main.tf


2. infra/env/prod/variables.tf ⚙️ Configuration changes +5/-0

Add instance type variable with defaults

• Added new instance_type variable with default value t3.xlarge
• Removed default value from ami_id variable to make it required

infra/env/prod/variables.tf


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

Add Elastic IP and remove AMI lookup

• Removed hardcoded AMI data source lookup for Amazon Linux 2023
• Added aws_eip resource for Elastic IP allocation
• Added aws_eip_association resource to attach EIP to EC2 instance
• Instance now uses var.instance_type and var.ami_id parameters

infra/modules/compute/main.tf


View more (1)
4. infra/modules/compute/outputs.tf ✨ Enhancement +10/-10

Output Elastic IP instead of instance IP

• Updated public_ip output to return Elastic IP address instead of instance public IP
• Reformatted file with consistent line endings

infra/modules/compute/outputs.tf


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Mar 29, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Ephemeral public IP still enabled 🐞 Bug ⛨ Security
Description
The compute module now allocates/associates an Elastic IP and outputs it as public_ip, but the
instance is still configured to auto-assign a public IP at launch, creating a window where the
instance has a different public IP than the intended EIP. This can break IP allowlists/operational
workflows and briefly expose an unexpected public address during instance create/replace.
Code

infra/modules/compute/main.tf[R102-113]

+resource "aws_eip" "this" {
+  domain = "vpc"
+
+  tags = merge(var.tags, {
+    Name = "${var.name_prefix}-eip"
+  })
+}
+
+resource "aws_eip_association" "this" {
+  instance_id   = aws_instance.this.id
+  allocation_id = aws_eip.this.id
}
Evidence
aws_instance.this is configured to auto-assign a public IP, while the PR adds an EIP and
associates it to the instance and changes the exported public IP to the EIP. With the subnet also
configured to map public IPs on launch, the instance will get an ephemeral public IP first and then
be switched to the EIP after association, creating a temporary second/incorrect public address.

infra/modules/compute/main.tf[86-100]
infra/modules/compute/main.tf[102-113]
infra/modules/compute/outputs.tf[5-7]
infra/modules/network/main.tf[57-68]

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 EC2 instance is still configured to receive an auto-assigned public IP (`associate_public_ip_address = true`) even though the module now allocates and associates an Elastic IP and exports that EIP as the instance public IP.

### Issue Context
This creates a launch/replace window where the instance has a different public IP than the intended EIP, which can break IP allowlists and briefly expose an unexpected address.

### Fix Focus Areas
- infra/modules/compute/main.tf[86-95]
- infra/modules/compute/main.tf[102-113]
- (Optional) infra/modules/network/main.tf[57-68]

### Suggested change
Set `associate_public_ip_address = false` on `aws_instance.this` (and, if desired, ensure public subnet behavior aligns with EIP-only exposure).

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



Advisory comments

2. Missing EOF newlines 🐞 Bug ⚙ Maintainability
Description
Terraform files touched in this PR are missing a trailing newline at end-of-file, which commonly
causes formatter/linter failures and noisy diffs.
Code

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

variable "ami_id" {
  type = string
+}
+
+variable "instance_type" {
+  type    = string
+  default = "t3.xlarge"
}
Evidence
The PR diff explicitly marks \ No newline at end of file for multiple updated Terraform files,
indicating they were saved without a final newline.

infra/env/prod/variables.tf[22-29]
infra/modules/compute/main.tf[110-113]
infra/modules/compute/outputs.tf[9-11]

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

### Issue description
Files modified in this PR are missing a trailing newline at EOF.

### Issue Context
Many formatters/linters and some CI checks expect a final newline; missing it causes avoidable diffs and occasional tooling warnings.

### Fix Focus Areas
- infra/env/prod/variables.tf[22-29]
- infra/modules/compute/main.tf[110-113]
- infra/modules/compute/outputs.tf[9-11]

### Suggested change
Add a final newline to each affected file (running `terraform fmt` typically fixes this automatically).

ⓘ 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

Comment thread infra/modules/compute/main.tf
@NickSavino
NickSavino merged commit 3039626 into development Mar 29, 2026
NickSavino added a commit that referenced this pull request Mar 29, 2026
* 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

* prod fix

* Seed Script: Wipe Data (#69)

* 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>

* fix: (#70)

- updated workout session history endpoint
- added reset_workflow script along with github action
- added ingestion functionality for webhook

* Feat: Setup Cloudwatch Logs for EC2 (#73)

* feat: setup AWS Cloudwatch resource and updated deploy script to attach to logging

* Increaed EC2 Tier and added elastic ip (#76)

* Increaed EC2 Tier and added elastic ip

* added newline to EOF

* Logging Middleware (#75)

* Added Logging Middleware to increase observability

* small rename

* Refactored user handler logic to be executed in service

* Updated logging.go to adress bot comments

* Updated logging middleware to forward optional http.ResponseWriter interfaces

---------

Co-authored-by: NicolaSavino <nick.savino@arcurve.com>
Co-authored-by: Aarsh Shah <86862845+AarshShah9@users.noreply.github.com>
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.

2 participants