Skip to content

Feature/infra changes - #246

Open
MauroCominotti wants to merge 23 commits into
developfrom
feature/infra-changes
Open

Feature/infra changes#246
MauroCominotti wants to merge 23 commits into
developfrom
feature/infra-changes

Conversation

@MauroCominotti

Copy link
Copy Markdown
Collaborator

Fixes #<issue_number_goes_here>

It's a good idea to open an issue first for discussion.

  • Tests pass
  • Appropriate changes to documentation are included in the PR

MauroCominotti and others added 13 commits April 8, 2026 16:11
Gallery overhaul and media tagging system
chore: update workspace membership check in brand guideline service
docs: add comprehensive development guide
Implement admin dashboard with backend services and UI components, fix reference images on media detail page
…cc-creative-studio into feature/infra-changes
…Izumi Agent deployment into bootstrap script

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

## 📋 Review Summary

This Pull Request introduces significant, high-quality infrastructure changes, transitioning Creative Studio to a secure private network with a Private PostgreSQL instance, Cloud Armor WAF protection, and an automated Izumi agent deployment pipeline. The overall organization of Terraform modules and scripting is excellent and aligns with enterprise-grade deployment practices. However, several critical issues must be resolved before merging, including invalid probe endpoints, a non-existent database version, a broken SQL CLI command in the migration script, and a missing utility installation in the release workflow.

🔍 General Feedback

  • Excellent Security Design: Locking the database to a private VPC and enforcing Direct VPC egress for Cloud Run is a strong best practice.
  • Great Deployment Scripting: The addition of deploy.sh makes the bootstrapping and configuration process highly intuitive and seamless.
  • Dynamic Credentials: Great use of Secret Manager and ephemeral secrets in Terraform to keep sensitive passwords out of the state file.
  • Documentation Note: Clean up legacy file references in comment sections (such as platform.tf references) to keep the repository maintainable.

Comment thread .github/workflows/release.yml
Comment thread infrastructure/main.tf Outdated
Comment thread infrastructure/modules/database/variables.tf
Comment thread infrastructure/modules/gateway/main.tf Outdated
Comment thread infrastructure/modules/compute/main.tf
Comment thread infrastructure/migration/migrate_to_private_db.sh Outdated
Comment thread infrastructure/migration/migrate_to_private_db.sh
Comment thread bootstrap.sh Outdated

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

## 📋 Review Summary

This pull request introduces significant infrastructure enhancements to Google Cloud Creative Studio, including modularizing the Terraform setup under a new infrastructure directory, adding Private VPC and Private Services Access configurations, and incorporating automated data migration and Izumi Agent deployment steps. The architectural enhancements follow enterprise security best practices by isolating database traffic and utilizing modern Terraform features.

🔍 General Feedback

  • Directory Reorganization: Moving/renaming infra to infrastructure is a positive step towards clean modularity, but ensure that all shell scripts referencing these directories have been updated completely to avoid broken paths.
  • Modern Terraform Features: The adoption of Terraform 1.11+ write-only attributes (password_wo) and ephemeral blocks is a great security choice that prevents plaintext password exposure in State files.
  • Enterprise Network Isolation: Transitioning Cloud Run to Direct VPC Egress and Cloud SQL to Private IP only significantly improves the security profile of the application.

Comment thread .github/workflows/release.yml
Comment thread bootstrap.sh
Comment thread infrastructure/migration/migrate_to_private_db.sh Outdated
Comment thread deploy.sh Outdated
Comment thread bootstrap.sh

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

## 📋 Review Summary

This pull request introduces significant infrastructure enhancements, renaming the Terraform workspace from infra to infrastructure, modernizing the GCP resources with private network isolation (Cloud SQL Private IP and Direct VPC Egress for Cloud Run), and adding automated deployment pipelines. While the architectural improvements are excellent and security-focused, several critical regression blockers and syntax mismatches must be addressed before merging.

🔍 General Feedback

  • Pre-commit Build Regression (Critical): In infrastructure/pre-commit/Dockerfile.pre-commit (which was renamed with 100% similarity), the file still attempts to copy the entrypoint script using the old path COPY infra/pre-commit/entrypoint.sh /entrypoint.sh. Since that folder has been renamed, building the pre-commit environment will fail with a file-not-found error. Please update this to use the new path.
  • Bootstrapping Mismatches: With the flattening of the Terraform workspace structure directly under infrastructure/, the bootstrap.sh script continues to reference deleted environments templates and subdirectories. This completely breaks the setup workflow for new environments.
  • Provider & Version Sanity: The database version (POSTGRES_18) is non-existent in GCP Cloud SQL, and the CPU capacity format (1000m) is invalid for Cloud Run v2 (which rejects Kubernetes millicore syntax). Correcting these defaults prevents runtime provisioning failures.

Comment thread .github/workflows/release.yml Outdated
Comment thread deploy.sh Outdated
Comment thread infrastructure/hosting.tf
Comment thread infrastructure/modules/compute/variables.tf Outdated
Comment thread infrastructure/modules/database/variables.tf
Comment thread bootstrap.sh Outdated
…e connections, and update vertex AI agent configuration

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📋 Review Summary

This pull request transitions the Google Cloud Creative Studio deployment pattern to use a secure, private VPC network with Private Services Access for PostgreSQL database connections. This transition significantly enhances database resilience and aligns with enterprise-level Google Cloud architectural best practices. The updated infrastructure modules, backend database helper updates, and deployment scripts are exceptionally cohesive, robust, and well-designed.

🔍 General Feedback

  • State-of-the-Art Security: The integration of Terraform v1.11+ write-only attributes (password_wo in google_sql_user and secret_data_wo in google_secret_manager_secret_version) paired with ephemeral resources is an outstanding, highly secure practice that ensures sensitive plaintext passwords never touch the persistent .tfstate files.
  • VPC Boundary Security: Moving database connectivity to a private network using serverless NEGs, Direct VPC egress on Cloud Run, and Cloud Armor with OWASP rules is highly professional and enforces correct network isolation.
  • Excellent Migration Tooling: Providing automated schema migration checks and data transfer scripts (migrate_to_private_db.sh) prevents data loss when upgrading existing deployments.
  • Proper Test Coverage: Unit and integration tests are properly updated to mock the new IPTypes.PRIVATE connection configurations and Vertex AI reasoning engine initialization flows.

Comment thread infrastructure/locals.tf
Comment thread infrastructure/modules/database/main.tf
Comment thread backend/src/agents/agent_service.py

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

## 📋 Review Summary

This pull request introduces significant architectural upgrades to transition the Google Cloud Creative Studio database to a Private VPC connection and reorganize the Terraform infrastructure structure into modular components. While the overall goal and direction are excellent for enterprise security and maintainability, there are several critical configuration mismatches and pathing issues that would prevent successful deployment or execution.

🔍 General Feedback

  • Terraform Provider Mismatch (🔴 Critical): The infrastructure/main.tf defines provider constraints of ~> 6.50.0, but employs new Google provider features (password_wo and secret_data_wo) that require 7.0.0 or later, which will cause deployment validation failures.
  • Duplicate Environment Variables (🔴 Critical): In infrastructure/app.tf, AGENT_ENGINE_RESOURCE_NAME and AGENT_ENGINE_USER_AUTH_TOKEN_KEY are defined twice (both as plaintext env vars and as Secret Manager secrets), leading to a 400 Bad Request error from the Cloud Run v2 API.
  • Incorrect Pathing in Rename (🔴 Critical): The pre-commit Dockerfile (infrastructure/pre-commit/Dockerfile.pre-commit line 23) still attempts to copy infra/pre-commit/entrypoint.sh from the old path instead of the renamed infrastructure/pre-commit/entrypoint.sh path, which will fail during docker image builds.
  • AI Agent Service Account Permissions (🟠 High): The dedicated service account for AI Agents (agent_sa) is provisioned but lacks project-level IAM roles (such as roles/aiplatform.user), which will cause Vertex AI Gemini model invocations by the reasoning engine to fail with 403 Permission Denied errors.
  • Sensitive Data Lifecycle (🟠 High): The database migration script migrate_to_private_db.sh leaves the decrypted SQL database dump file in the GCS bucket indefinitely after successful import, presenting a major security risk for sensitive database content.

Comment thread infrastructure/main.tf Outdated
Comment thread infrastructure/app.tf
Comment thread infrastructure/modules/compute/main.tf
Comment thread infrastructure/migration/migrate_to_private_db.sh
Comment thread .github/workflows/release.yml
…gration cleanup, and remove unused agent configuration

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📋 Review Summary

This Pull Request introduces significant infrastructure updates to establish private, secure database connections for the Google Cloud Creative Studio Platform, migrating from public Cloud SQL access to Private Services Access (VPC Peering) and Serverless Direct VPC Egress for Cloud Run. While the architectural transition is highly positive for enterprise security, there are several critical deployment and integration bugs—specifically related to missing GCP service API enablements and randomized authentication variables—that must be resolved before this PR can be safely merged.

🔍 General Feedback

  • Positive Highlight on Data Migration: The database migration script (infrastructure/migration/migrate_to_private_db.sh) is exceptionally well designed. It employs robust retry loops, manages IAM policy bindings dynamically for export/import, and utilizes shell traps to guarantee clean-up of temporary Cloud Storage assets and permissions.
  • Improved Git Hygiene: Properly ignoring public/assets/config.json while introducing the .example template is an excellent standard practice that prevents developers from accidentally committing local config profiles.
  • Consistent Testing: Solid updates to the unit tests in backend/tests/test_database.py and backend/tests/agents/test_agent_service.py ensure that private IP routing and agent initialization are continuously verified going forward.

Comment thread bootstrap.sh
Comment thread deploy.sh Outdated
Comment thread infrastructure/variables.tf
Comment thread deploy.sh Outdated
Comment thread bootstrap.sh Outdated
Comment thread backend/alembic/env.py

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

## 📋 Review Summary

The Pull Request implements crucial architectural improvements by modernizing and flattening the infrastructure setup, introducing Private Services Access for PostgreSQL database connections, and updating the database lifecycle cleanup. The separation of environment-specific states using a GCS backend config pattern and the use of ephemeral resources in Terraform represent outstanding Cloud architecture practices. However, critical logic errors in bootstrap.sh (such as unassigned variables and an empty .tfvars file creation) currently block successful execution and need to be resolved before merging.

🔍 General Feedback

  • Great job on transitioning the database connection to Private Services Access for enhanced VPC security. This aligns perfectly with enterprise-grade GCP landing zone patterns.
  • Excellent usage of modern Terraform features (like ephemeral resources and write-only properties) to ensure database credentials never leak into state files.
  • Excellent addition of the database connector cleanup process in the FastAPI lifespan shutdown handler to prevent resource leaks during container recycling.
  • Please address the critical bugs identified in the bootstrap.sh script to ensure developers can successfully deploy the environment end-to-end.

Comment thread infrastructure/modules/artifact/main.tf
Comment thread infrastructure/hosting.tf
Comment thread backend/src/agents/agent_service.py
Comment thread bootstrap.sh
Comment thread bootstrap.sh Outdated

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

## 📋 Review Summary

This pull request introduces critical infrastructure and deployment enhancements, transitioning to secure private Cloud SQL connectivity via Direct VPC egress, and a robust offline test suite for the bootstrapper script. The overall architectural design is exceptional, utilizing dynamic secret mapping, VPC peering, and secure parameter state persistence to achieve production-grade isolation. However, several critical version constraints and invalid argument mismatches in the Terraform modules will cause execution to fail immediately on initialization and must be corrected before merge.

🔍 General Feedback

  • Dynamic Orchestration: The enhancements to bootstrap.sh to support automated, idempotent, profile-driven executions are extremely well implemented and handle resuming from late-stage steps cleanly.
  • VPC & Network Isolation: Transitioning the application components (FastAPI Backend, Cloud SQL, and migrations) to a private IP model represents a significant security improvement.
  • Code Quality & Verification: The addition of test_bootstrap_offline.sh is an excellent initiative to ensure the complex initialization logic is completely testable and verified offline.
  • Provider Consistency: Ensure that provider version requirements set in root and child files remain fully synchronized with the .terraform.lock.hcl and environment constraints.

Comment thread infrastructure/main.tf
Comment thread .github/workflows/release.yml
Comment thread .github/workflows/release.yml
Comment thread infrastructure/modules/database/main.tf
Comment thread infrastructure/modules/database/main.tf
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