Skip to content

Conversation

@kryczkal
Copy link
Collaborator

This pull request introduces updates to environment configuration, email service logic, and infrastructure provisioning. The changes aim to enhance security, improve email functionality, and streamline deployment processes. Key updates include replacing placeholder values with secure configurations, adding support for dynamic base URLs in email links, and integrating SendGrid API keys into AWS Secrets Manager.

Environment Configuration Updates:

  • .env.template: Updated database and security configurations with more secure and descriptive values, including changes to DB_URL, DB_NAME, SECRET_KEY, and EMAIL_API_KEY. Added APP_BASE_URL for constructing verification links.
  • docker-compose.yml: Added APP_BASE_URL as an environment variable for services, enabling dynamic URL configuration. [1] [2]

Email Service Enhancements:

  • backend/event_ticketing_service/app/services/email.py: Introduced APP_BASE_URL for constructing dynamic links in ticket confirmation emails. Updated fallback behavior to use a generic domain if APP_BASE_URL is not set. Modified email templates to use dynamic links for "View My Tickets," "Terms of Service," "Privacy Policy," and "Contact Us." [1] [2] [3]

Infrastructure Changes:

  • terraform/main/main.tf: Added resources for storing and managing SendGrid API keys in AWS Secrets Manager. Updated ECS cluster module to include sendgrid_api_key_arn, email_from_address, and app_base_url variables for email configuration. [1] [2]
  • terraform/modules/ecs_cluster/main.tf: Updated ECS task definitions to pass EMAIL_FROM_EMAIL and APP_BASE_URL as environment variables and EMAIL_API_KEY as a secret. Extended IAM policy to grant access to the SendGrid API key secret. [1] [2] [3]

Variable Updates:

  • terraform/main/variables.tf and terraform/modules/ecs_cluster/variables.tf: Added new variables for sendgrid_api_key, email_from_address, and app_base_url to support email configuration and dynamic link generation. [1] [2]

@kryczkal kryczkal requested a review from Copilot June 16, 2025 19:47
@kryczkal kryczkal changed the base branch from main to dev June 16, 2025 19:47
@kryczkal kryczkal merged commit 02bb1be into dev Jun 16, 2025
2 checks passed
@kryczkal kryczkal deleted the kryczkal/final-deploy branch June 16, 2025 19:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances security and email functionality by integrating SendGrid key management in AWS, enabling dynamic URL construction in emails, and streamlining environment and deployment configurations.

  • Add sendgrid_api_key, email_from_address, and app_base_url variables to Terraform modules and main config
  • Provision SendGrid API key in AWS Secrets Manager and pass it to ECS task definitions
  • Update email service to use APP_BASE_URL for constructing links and refresh environment templates and Docker Compose

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
terraform/modules/ecs_cluster/variables.tf Added new variables for SendGrid key ARN, sender address, base URL
terraform/modules/ecs_cluster/main.tf Updated IAM policy and ECS task definitions to include email vars and secrets
terraform/main/variables.tf Introduced sendgrid_api_key and email_from_address variables
terraform/main/main.tf Provisioned SendGrid secret, updated module invocation
docker-compose.yml Added APP_BASE_URL environment variable to services
backend/user_auth_service/.env.template Removed old placeholder template (must re-add with new vars)
backend/event_ticketing_service/app/services/email.py Integrated APP_BASE_URL, updated link templates, removed param docs
backend/event_ticketing_service/.env.template Removed old placeholder template (should re-add with new vars)
.env.template Refreshed shared environment template with comments and placeholders
Comments suppressed due to low confidence (2)

backend/user_auth_service/.env.template:1

  • The entire .env.template for the auth service was removed. Reintroduce an updated template including new variables like EMAIL_API_KEY, EMAIL_FROM_EMAIL, and APP_BASE_URL to ensure consistent local setup.
DB_URL=localhost

backend/event_ticketing_service/app/services/email.py:55

  • [nitpick] The docstring had its Args: and return documentation removed. Consider restoring parameter and return value descriptions to keep the function self-documented.
"""

# Fallback to a generic domain if not set, but log an error
base_url = "http://resellio.com"
else:
base_url = APP_BASE_URL.replace('/api', '') # Ensure we have the root URL
Copy link

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Using a simple string replace may inadvertently strip valid parts of URLs. Consider using urllib.parse or a similar URL library to reliably construct the root URL.

Copilot uses AI. Check for mistakes.
Comment on lines +62 to +63
# Fallback to a generic domain if not set, but log an error
base_url = "http://resellio.com"
Copy link

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since APP_BASE_URL has a default non-empty value from os.getenv, this branch will never execute. If you intend to detect a missing environment variable, retrieve the raw environment lookup first, then apply a default only after the check.

Suggested change
# Fallback to a generic domain if not set, but log an error
base_url = "http://resellio.com"
# Apply default or fallback explicitly
base_url = "http://localhost:8080" # Default for development

Copilot uses AI. Check for mistakes.
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