Feature/infra changes - #246
Conversation
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
…-studio into feature/infra-changes
There was a problem hiding this comment.
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.shmakes 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.tfreferences) to keep the repository maintainable.
…te docker-compose configuration
There was a problem hiding this comment.
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
infratoinfrastructureis 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.
…nfrastructure paths and configuration scripts
…-studio into feature/infra-changes
There was a problem hiding this comment.
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 pathCOPY 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/, thebootstrap.shscript 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.
…e connections, and update vertex AI agent configuration
There was a problem hiding this comment.
📋 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_woingoogle_sql_userandsecret_data_woingoogle_secret_manager_secret_version) paired with ephemeral resources is an outstanding, highly secure practice that ensures sensitive plaintext passwords never touch the persistent.tfstatefiles. - 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.PRIVATEconnection configurations and Vertex AI reasoning engine initialization flows.
There was a problem hiding this comment.
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): Theinfrastructure/main.tfdefines provider constraints of~> 6.50.0, but employs new Google provider features (password_woandsecret_data_wo) that require7.0.0or later, which will cause deployment validation failures. - Duplicate Environment Variables (
🔴Critical): Ininfrastructure/app.tf,AGENT_ENGINE_RESOURCE_NAMEandAGENT_ENGINE_USER_AUTH_TOKEN_KEYare 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-commitline 23) still attempts to copyinfra/pre-commit/entrypoint.shfrom the old path instead of the renamedinfrastructure/pre-commit/entrypoint.shpath, 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 asroles/aiplatform.user), which will cause Vertex AI Gemini model invocations by the reasoning engine to fail with403 Permission Deniederrors. - Sensitive Data Lifecycle (
🟠High): The database migration scriptmigrate_to_private_db.shleaves the decrypted SQL database dump file in the GCS bucket indefinitely after successful import, presenting a major security risk for sensitive database content.
…gration cleanup, and remove unused agent configuration
There was a problem hiding this comment.
📋 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.jsonwhile introducing the.exampletemplate 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.pyandbackend/tests/agents/test_agent_service.pyensure that private IP routing and agent initialization are continuously verified going forward.
…ment configuration in bootstrap.sh
There was a problem hiding this comment.
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
ephemeralresources 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.shscript to ensure developers can successfully deploy the environment end-to-end.
…aving for project and repository configurations
…e support and GitHub Actions testing workflow
There was a problem hiding this comment.
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.shto 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.shis 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.hcland environment constraints.
Fixes #<issue_number_goes_here>