diff --git a/.env.example b/.env.example index 026f186..f26e197 100644 --- a/.env.example +++ b/.env.example @@ -13,6 +13,8 @@ # Option A — MySQL (default): legacy vars or DATABASE_URL DATABASE_BACKEND=mysql DATABASE_HOST=127.0.0.1 +# DATABASE_PORT defaults to 3306 (MySQL) or 5432 (PostgreSQL) when unset. +# Set explicitly for non-standard ports (e.g. TiDB Cloud often uses 4000). # DATABASE_PORT=3306 DATABASE_USER=root DATABASE_PASSWORD=rootpass diff --git a/.github/dependabot.yml b/.github/dependabot.yml index e644f37..7295bc5 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,20 +1,5 @@ version: 2 updates: - - package-ecosystem: "pip" - directory: "/syncbot" - schedule: - interval: "weekly" - open-pull-requests-limit: 10 - groups: - minor-and-patch: - update-types: ["minor", "patch"] - - - package-ecosystem: "pip" - directory: "/infra/aws/db_setup" - schedule: - interval: "weekly" - open-pull-requests-limit: 10 - - package-ecosystem: "github-actions" directory: "/" schedule: @@ -26,3 +11,6 @@ updates: schedule: interval: "weekly" open-pull-requests-limit: 10 + ignore: + - dependency-name: "python" + update-types: ["version-update:semver-major"] diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9824f5a..d1c0fa7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,12 +16,12 @@ jobs: permissions: contents: write steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 with: ref: ${{ github.head_ref || github.ref_name }} repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }} fetch-depth: 0 - - uses: actions/setup-python@v5 + - uses: actions/setup-python@v6 with: python-version: "3.12" - name: Install Poetry and export plugin @@ -34,24 +34,26 @@ jobs: PR_HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} run: | poetry export -f requirements.txt --without-hashes -o syncbot/requirements.txt - if git diff --quiet syncbot/requirements.txt; then - echo "requirements.txt is already in sync." + echo "# Required for MySQL 8+ caching_sha2_password; pin for reproducible CI (sam build)." > infra/aws/db_setup/requirements.txt + grep -E "^(pymysql|psycopg2-binary|cryptography)==" syncbot/requirements.txt >> infra/aws/db_setup/requirements.txt + if git diff --quiet syncbot/requirements.txt infra/aws/db_setup/requirements.txt; then + echo "requirements.txt files are already in sync." elif [[ -n "${PR_HEAD_REPO}" && "${PR_HEAD_REPO}" != "${GITHUB_REPOSITORY}" ]]; then - echo "::error::syncbot/requirements.txt is out of sync with poetry.lock. From the repo root run: poetry export -f requirements.txt --without-hashes -o syncbot/requirements.txt" + echo "::error::Requirements files are out of sync with poetry.lock. Commit with pre-commit installed (sync-requirements hook) or follow docs/DEVELOPMENT.md." exit 1 else git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" - git add syncbot/requirements.txt - git commit -m "chore: sync requirements.txt with poetry.lock" + git add syncbot/requirements.txt infra/aws/db_setup/requirements.txt + git commit -m "chore: sync requirements.txt files with poetry.lock" git push - echo "::notice::requirements.txt was out of sync and has been auto-fixed." + echo "::notice::requirements.txt files were out of sync and have been auto-fixed." fi sam-lint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - uses: aws-actions/setup-sam@v2 with: use-installer: true @@ -63,8 +65,8 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 + - uses: actions/checkout@v6 + - uses: actions/setup-python@v6 with: python-version: "3.12" - name: Install dependencies diff --git a/.github/workflows/deploy-aws.yml b/.github/workflows/deploy-aws.yml index fab46d5..eb2e4c0 100644 --- a/.github/workflows/deploy-aws.yml +++ b/.github/workflows/deploy-aws.yml @@ -23,8 +23,8 @@ jobs: if: vars.DEPLOY_TARGET != 'gcp' runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 + - uses: actions/checkout@v6 + - uses: actions/setup-python@v6 with: python-version: '3.12' - uses: aws-actions/setup-sam@v2 @@ -36,7 +36,7 @@ jobs: sam validate -t infra/aws/template.yaml --lint sam validate -t infra/aws/template.bootstrap.yaml --lint - - uses: aws-actions/configure-aws-credentials@v4 + - uses: aws-actions/configure-aws-credentials@v6 with: role-to-assume: ${{ vars.AWS_ROLE_TO_ASSUME }} aws-region: ${{ vars.AWS_REGION }} @@ -50,7 +50,7 @@ jobs: pip-audit -r infra/aws/db_setup/requirements.txt - name: Publish artifact - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: build-artifact path: './.aws-sam/build' @@ -61,13 +61,13 @@ jobs: environment: test needs: sam-build steps: - - uses: aws-actions/configure-aws-credentials@v4 + - uses: aws-actions/configure-aws-credentials@v6 with: role-to-assume: ${{ vars.AWS_ROLE_TO_ASSUME }} aws-region: ${{ vars.AWS_REGION }} - name: Download artifact - uses: actions/download-artifact@v4 + uses: actions/download-artifact@v8 with: name: build-artifact path: './.aws-sam/build' @@ -103,6 +103,11 @@ jobs: ExistingDatabaseNetworkMode=${{ vars.EXISTING_DATABASE_NETWORK_MODE || 'public' }} \ ExistingDatabaseSubnetIdsCsv=${{ vars.EXISTING_DATABASE_SUBNET_IDS_CSV }} \ ExistingDatabaseLambdaSecurityGroupId=${{ vars.EXISTING_DATABASE_LAMBDA_SECURITY_GROUP_ID }} \ + ExistingDatabasePort=${{ vars.EXISTING_DATABASE_PORT }} \ + ExistingDatabaseCreateAppUser=${{ vars.EXISTING_DATABASE_CREATE_APP_USER || 'true' }} \ + ExistingDatabaseCreateSchema=${{ vars.EXISTING_DATABASE_CREATE_SCHEMA || 'true' }} \ + ExistingDatabaseUsernamePrefix=${{ vars.EXISTING_DATABASE_USERNAME_PREFIX }} \ + ExistingDatabaseAppUsername=${{ vars.EXISTING_DATABASE_APP_USERNAME }} \ DatabaseSchema=${{ vars.DATABASE_SCHEMA }} \ LogLevel=${{ vars.LOG_LEVEL || 'INFO' }} \ RequireAdmin=${{ vars.REQUIRE_ADMIN || 'true' }} \ @@ -117,19 +122,32 @@ jobs: SlackSigningSecret=${{ secrets.SLACK_SIGNING_SECRET }} \ $OVERRIDE_PARAM" + - name: Run migrations and warm up Lambda + run: | + FUNCTION_ARN=$(aws cloudformation describe-stacks \ + --stack-name ${{ vars.AWS_STACK_NAME }} \ + --query "Stacks[0].Outputs[?OutputKey=='SyncBotFunctionArn'].OutputValue" \ + --output text) + aws lambda invoke \ + --function-name "$FUNCTION_ARN" \ + --payload '{"action":"migrate"}' \ + --cli-binary-format raw-in-base64-out \ + /tmp/migrate-response.json + cat /tmp/migrate-response.json + sam-deploy-prod: if: github.ref == 'refs/heads/prod' runs-on: ubuntu-latest environment: prod needs: sam-build steps: - - uses: aws-actions/configure-aws-credentials@v4 + - uses: aws-actions/configure-aws-credentials@v6 with: role-to-assume: ${{ vars.AWS_ROLE_TO_ASSUME }} aws-region: ${{ vars.AWS_REGION }} - name: Download artifact - uses: actions/download-artifact@v4 + uses: actions/download-artifact@v8 with: name: build-artifact path: './.aws-sam/build' @@ -165,6 +183,11 @@ jobs: ExistingDatabaseNetworkMode=${{ vars.EXISTING_DATABASE_NETWORK_MODE || 'public' }} \ ExistingDatabaseSubnetIdsCsv=${{ vars.EXISTING_DATABASE_SUBNET_IDS_CSV }} \ ExistingDatabaseLambdaSecurityGroupId=${{ vars.EXISTING_DATABASE_LAMBDA_SECURITY_GROUP_ID }} \ + ExistingDatabasePort=${{ vars.EXISTING_DATABASE_PORT }} \ + ExistingDatabaseCreateAppUser=${{ vars.EXISTING_DATABASE_CREATE_APP_USER || 'true' }} \ + ExistingDatabaseCreateSchema=${{ vars.EXISTING_DATABASE_CREATE_SCHEMA || 'true' }} \ + ExistingDatabaseUsernamePrefix=${{ vars.EXISTING_DATABASE_USERNAME_PREFIX }} \ + ExistingDatabaseAppUsername=${{ vars.EXISTING_DATABASE_APP_USERNAME }} \ DatabaseSchema=${{ vars.DATABASE_SCHEMA }} \ LogLevel=${{ vars.LOG_LEVEL || 'INFO' }} \ RequireAdmin=${{ vars.REQUIRE_ADMIN || 'true' }} \ @@ -178,3 +201,16 @@ jobs: SlackClientSecret=${{ secrets.SLACK_CLIENT_SECRET }} \ SlackSigningSecret=${{ secrets.SLACK_SIGNING_SECRET }} \ $OVERRIDE_PARAM" + + - name: Run migrations and warm up Lambda + run: | + FUNCTION_ARN=$(aws cloudformation describe-stacks \ + --stack-name ${{ vars.AWS_STACK_NAME }} \ + --query "Stacks[0].Outputs[?OutputKey=='SyncBotFunctionArn'].OutputValue" \ + --output text) + aws lambda invoke \ + --function-name "$FUNCTION_ARN" \ + --payload '{"action":"migrate"}' \ + --cli-binary-format raw-in-base64-out \ + /tmp/migrate-response.json + cat /tmp/migrate-response.json diff --git a/.github/workflows/deploy-gcp.yml b/.github/workflows/deploy-gcp.yml index 0415265..172fc48 100644 --- a/.github/workflows/deploy-gcp.yml +++ b/.github/workflows/deploy-gcp.yml @@ -25,7 +25,7 @@ jobs: if: vars.DEPLOY_TARGET == 'gcp' runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 # Workload Identity Federation: authenticate without a key file # - uses: google-github-actions/auth@v2 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 3bf51ed..8e37bfd 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -27,8 +27,8 @@ repos: - repo: local hooks: - id: sync-requirements - name: Sync requirements.txt with poetry.lock - entry: bash -c 'poetry export -f requirements.txt --without-hashes -o syncbot/requirements.txt && git add syncbot/requirements.txt' + name: Sync requirements.txt files with poetry.lock + entry: bash -c 'poetry export -f requirements.txt --without-hashes -o syncbot/requirements.txt && echo "# Required for MySQL 8+ caching_sha2_password; pin for reproducible CI (sam build)." > infra/aws/db_setup/requirements.txt && grep -E "^(pymysql|psycopg2-binary|cryptography)==" syncbot/requirements.txt >> infra/aws/db_setup/requirements.txt && git add syncbot/requirements.txt infra/aws/db_setup/requirements.txt' language: system files: ^poetry\.lock$ pass_filenames: false diff --git a/CHANGELOG.md b/CHANGELOG.md index 0068f3f..2c27bd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,26 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Added + +- External DB deploy parameters: `ExistingDatabasePort`, `ExistingDatabaseCreateAppUser`, `ExistingDatabaseCreateSchema`, `ExistingDatabaseUsernamePrefix`, `ExistingDatabaseAppUsername` (AWS) / GCP equivalents — support TiDB Cloud and other managed DB providers with cluster-prefixed usernames and 32-char limits + +### Changed + +- Shortened default DB usernames: `sbadmin_{stage}` (was `syncbot_admin_{stage}`), `sbapp_{stage}` (was `syncbot_user_{stage}`). Existing RDS instances keep their original master username. +- Bumped GitHub Actions: `actions/checkout` v6, `actions/setup-python` v6, `actions/upload-artifact` v7, `actions/download-artifact` v8, `aws-actions/configure-aws-credentials` v6 +- Dependabot: ignore semver-major updates for the Docker `python` image (keeps base image on Python 3.12.x line) +- AWS Lambda: Alembic migrations now run via a post-deploy invoke instead of on every cold start, fixing Slack ack timeouts after deployment; Cloud Run and local dev unchanged +- AWS Lambda memory increased from 128 MB to 256 MB for faster cold starts +- EventBridge keep-warm invokes now return a clean JSON response instead of falling through to Slack Bolt +- AWS bootstrap deploy policy: added `lambda:InvokeFunction` -- **re-run the deploy script (Bootstrap task) or `aws cloudformation deploy` the bootstrap stack to pick up this permission** + +### Fixed + +- Replaced deprecated `datetime.utcnow()` with `datetime.now(UTC)` in backup/migration export helpers + ## [1.0.1] - 2026-03-26 ### Changed diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 426d16e..2c81c75 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,6 +1,6 @@ # Contributing -Thanks for helping improve SyncBot. +Thanks for helping to improve SyncBot! ## Branching (upstream vs downstream) @@ -11,7 +11,27 @@ The **upstream** repository ([F3Nation-Community/syncbot](https://github.com/F3N | **`main`** | Tracks upstream. Use it to merge PRs and to **sync with the upstream repository** (`git pull upstream main`, etc.). | | **`test`** / **`prod`** | On your fork, use these for **deployments**: GitHub Actions deploy workflows run on **push** to `test` and `prod` (see [docs/DEPLOYMENT.md](docs/DEPLOYMENT.md)). | -Typical flow: develop on a feature branch → open a PR to **`main`** → merge → when ready to deploy, merge **`main`** into **`test`** or **`prod`** on your fork. +Typical flow: develop a fix or new feature on a branch in your repo → test and deploy to your infra → open a PR to **`upstream/main`**. + +### Branch Naming Conventions + +Format: `/` or `/-` + +Types: + +- feature/ New functionality +- bugfix/ Bug fixes for existing features +- hotfix/ Urgent production issues +- refactor/ Code improvements without behavior changes +- docs/ Documentation only changes +- chore/ Build process, dependency updates, etc. + +Rules: + +- Use lowercase +- Separate words with hyphens +- Keep descriptions under 50 characters +- Be specific: feature/user-auth not feature/auth ## Workflow diff --git a/deploy.sh b/deploy.sh index 34c59ad..830105e 100755 --- a/deploy.sh +++ b/deploy.sh @@ -669,7 +669,9 @@ main() { poetry update --quiet if poetry self show plugins 2>/dev/null | grep -q poetry-plugin-export; then poetry export -f requirements.txt --without-hashes -o "$REPO_ROOT/syncbot/requirements.txt" - echo "syncbot/requirements.txt updated from poetry.lock." + echo "# Required for MySQL 8+ caching_sha2_password; pin for reproducible CI (sam build)." > "$REPO_ROOT/infra/aws/db_setup/requirements.txt" + grep -E "^(pymysql|psycopg2-binary|cryptography)==" "$REPO_ROOT/syncbot/requirements.txt" >> "$REPO_ROOT/infra/aws/db_setup/requirements.txt" + echo "syncbot/requirements.txt and infra/aws/db_setup/requirements.txt updated from poetry.lock." else echo "Warning: poetry-plugin-export not installed. Run: poetry self add poetry-plugin-export" >&2 echo "Skipping requirements.txt sync." >&2 diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a395926..dba2cc0 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -119,6 +119,8 @@ flowchart TB All infrastructure is defined in `infra/aws/template.yaml` (AWS SAM). Dashed lines indicate resources that are conditionally created — when `Existing*` parameters are set, those resources are skipped. +**Lambda cold start vs Slack acks:** The main function uses **256 MB** memory (faster init than 128 MB). Alembic migrations run only when the function is invoked with `{"action":"migrate"}` (post-deploy in CI), not on every cold start, so the first Slack interaction after deploy can ack within Slack’s time limit. EventBridge keep-warm ScheduleV2 invokes are handled in `app.handler` with a trivial JSON response instead of the Slack Bolt adapter. + ## Security & Hardening | Layer | Protection | diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index cf02d2e..566e272 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -48,7 +48,7 @@ Runs from repo root (or via `./deploy.sh` → **aws**). It: 3. **Bootstrap probe** — Reads bootstrap stack outputs if the stack exists (for suggested stack names and later CI/CD). Full **bootstrap** create/sync runs only if you select it in **Deploy Tasks** (see below). 4. **App stack identity** — Prompts for stage (`test`/`prod`) and stack name; detects an existing CloudFormation stack for update. 5. **Deploy Tasks** — Multi-select menu (comma-separated, default all): **Bootstrap** (create/sync bootstrap stack; respects `SYNCBOT_SKIP_BOOTSTRAP_SYNC=1` for sync), **Build/Deploy** (full config + SAM), **CI/CD** (`gh` / GitHub Actions), **Slack API**, **Backup Secrets** (DR plaintext echo). Omitting **Build/Deploy** requires an existing stack for tasks that need live outputs. -6. **Configuration** (if Build/Deploy selected) — **Database source** (stack-managed RDS vs existing RDS host) and **engine** (MySQL vs PostgreSQL). **Slack app credentials** (signing secret, client secret, client ID). **Existing database host** mode: RDS endpoint, admin user/password, **public vs private** network mode, and for **private** mode: subnet IDs and Lambda security group (with optional auto-detect and **connectivity preflight**). **New RDS in stack** mode: summarizes auto-generated DB users and prompts for **DatabaseSchema**. Optional **token encryption** recovery override, **log level** (numbered list `1`–`5` with `Choose level [N]:`, default from prior stack or **INFO**), **deploy summary**, then **SAM build** (`--use-container`) and **sam deploy**. +6. **Configuration** (if Build/Deploy selected) — **Database source** (stack-managed RDS vs existing RDS host) and **engine** (MySQL vs PostgreSQL). **Slack app credentials** (signing secret, client secret, client ID). **Existing database host** mode: RDS endpoint, admin user/password, optional **ExistingDatabasePort** (blank = engine default; use for non-standard ports e.g. TiDB **4000**), optional **ExistingDatabaseUsernamePrefix** (e.g. TiDB Cloud cluster prefix `abc123`; a dot separator is added automatically; prepended to **ExistingDatabaseAdminUser** and the default app user `{prefix}.sbapp_{stage}` — use bare admin names like `root` when set), optional **ExistingDatabaseAppUsername** (full app username override when the default would exceed provider limits, e.g. MySQL 32 chars), whether to **create a dedicated app DB user** and whether to run **`CREATE DATABASE IF NOT EXISTS`**, **public vs private** network mode, and for **private** mode: subnet IDs and Lambda security group (with optional auto-detect and **connectivity preflight** using the effective DB port). **New RDS in stack** mode: summarizes auto-generated DB users and prompts for **DatabaseSchema**. Optional **token encryption** recovery override, **log level** (numbered list `1`–`5` with `Choose level [N]:`, default from prior stack or **INFO**), **deploy summary**, then **SAM build** (`--use-container`) and **sam deploy**. 7. **Post-deploy** — According to selected tasks: stack outputs, `slack-manifest_.json`, Slack API, **`gh`** setup, deploy receipt under `deploy-receipts/` (gitignored), and DR backup lines. ### GCP: `infra/gcp/scripts/deploy.sh` @@ -90,7 +90,7 @@ See [infra/gcp/README.md](../infra/gcp/README.md) for Terraform variables and ou ## Database backends -The app supports **MySQL** (default), **PostgreSQL**, and **SQLite**. Schema changes are applied at startup via Alembic (`alembic upgrade head`). +The app supports **MySQL** (default), **PostgreSQL**, and **SQLite**. Schema changes use Alembic (`alembic upgrade head`). **AWS Lambda:** Applied after each deploy via a workflow step that invokes the function with `{"action":"migrate"}` (not on every cold start). **Cloud Run / local:** Applied at process startup before serving HTTP. - **AWS:** Choose engine in the deploy script or pass `DatabaseEngine=mysql` / `postgresql` to `sam deploy`. - **Contract:** [INFRA_CONTRACT.md](INFRA_CONTRACT.md) — `DATABASE_BACKEND`, `DATABASE_URL` or host/user/password/schema. @@ -146,12 +146,23 @@ sam deploy \ ... ``` -Use **`sam deploy --guided`** the first time if you prefer prompts. For **existing RDS**, set `ExistingDatabaseHost`, `ExistingDatabaseAdminUser`, `ExistingDatabaseAdminPassword`, and for **private** DBs also `ExistingDatabaseNetworkMode=private`, `ExistingDatabaseSubnetIdsCsv`, `ExistingDatabaseLambdaSecurityGroupId`. Omit `ExistingDatabaseHost` to create a **new** RDS in the stack. +Use **`sam deploy --guided`** the first time if you prefer prompts. For **existing RDS**, set `ExistingDatabaseHost`, `ExistingDatabaseAdminUser`, `ExistingDatabaseAdminPassword`, and for **private** DBs also `ExistingDatabaseNetworkMode=private`, `ExistingDatabaseSubnetIdsCsv`, `ExistingDatabaseLambdaSecurityGroupId`. Optional: `ExistingDatabasePort` (empty = engine default), `ExistingDatabaseCreateAppUser` / `ExistingDatabaseCreateSchema` (`true`/`false`). Omit `ExistingDatabaseHost` to create a **new** RDS in the stack. **samconfig:** Predefined profiles in `samconfig.toml` (`test-new-rds`, `test-existing-rds`, etc.) — adjust placeholders before use. **Token key:** The stack can auto-generate `TOKEN_ENCRYPTION_KEY` in Secrets Manager. Back it up after first deploy. Optional: `TokenEncryptionKeyOverride`, `ExistingTokenEncryptionKeySecretArn` for recovery. +**Post-deploy migrate (Lambda only):** After `sam deploy`, run Alembic and warm the function (same as CI): + +```bash +FUNCTION_ARN=$(aws cloudformation describe-stacks --stack-name syncbot-test \ + --query "Stacks[0].Outputs[?OutputKey=='SyncBotFunctionArn'].OutputValue" --output text) +aws lambda invoke --function-name "$FUNCTION_ARN" --payload '{"action":"migrate"}' \ + --cli-binary-format raw-in-base64-out /tmp/migrate.json && cat /tmp/migrate.json +``` + +The GitHub deploy role and bootstrap policy must allow `lambda:InvokeFunction` on `syncbot-*` functions; re-deploy the **bootstrap** stack if your policy predates that permission. + ### 3. GitHub Actions (AWS) Workflow: `.github/workflows/deploy-aws.yml` (runs on push to `test`/`prod` when not using GCP). @@ -175,18 +186,18 @@ Configure **per-environment** (`test` / `prod`) variables and secrets so they ma | Var | `EXISTING_DATABASE_NETWORK_MODE` | `public` or `private` | | Var | `EXISTING_DATABASE_SUBNET_IDS_CSV` | **Private** mode: comma-separated subnet IDs (no spaces) | | Var | `EXISTING_DATABASE_LAMBDA_SECURITY_GROUP_ID` | **Private** mode: Lambda ENI security group | +| Var | `EXISTING_DATABASE_PORT` | Optional; non-standard TCP port (e.g. `4000`). Empty = engine default in SAM. | +| Var | `EXISTING_DATABASE_CREATE_APP_USER` | `true` / `false` (default `true`). Set `false` when the DB cannot create a dedicated app user. | +| Var | `EXISTING_DATABASE_CREATE_SCHEMA` | `true` / `false` (default `true`). Set `false` when the database/schema already exists. | +| Var | `EXISTING_DATABASE_USERNAME_PREFIX` | Optional. Provider-specific username prefix (e.g. TiDB Cloud `abc123`; dot separator added automatically). Prepended to admin and default app user `{prefix}.sbapp_{stage}` in the bootstrap Lambda; use bare `EXISTING_DATABASE_ADMIN_USER` (e.g. `root`). Empty for RDS/standard MySQL. | +| Var | `EXISTING_DATABASE_APP_USERNAME` | Optional. Full dedicated app DB username (bypasses prefix + default `sbapp_{stage}`). Use if the auto name exceeds provider limits. Empty = default. | | Secret | `SLACK_SIGNING_SECRET`, `SLACK_CLIENT_SECRET` | | | Secret | `EXISTING_DATABASE_ADMIN_PASSWORD` | When `EXISTING_DATABASE_HOST` is set | | Secret | `TOKEN_ENCRYPTION_KEY_OVERRIDE` | Optional DR only | The interactive deploy script can set these via `gh` when you opt in. Re-run that step after changing DB mode or engine so CI stays aligned. -**Dependency hygiene:** The workflow runs `pip-audit` on `syncbot/requirements.txt` and `infra/aws/db_setup/requirements.txt`. After changing `pyproject.toml`: - -```bash -poetry lock -poetry export --only main --format requirements.txt --without-hashes --output syncbot/requirements.txt -``` +**Dependency hygiene:** The AWS deploy workflow runs `pip-audit` on `syncbot/requirements.txt` and `infra/aws/db_setup/requirements.txt`. After changing `pyproject.toml`, run `poetry lock` and commit; the **pre-commit `sync-requirements` hook** (see [.pre-commit-config.yaml](../.pre-commit-config.yaml)) regenerates both requirements files when `poetry.lock` changes. If you do not use pre-commit, run the export commands documented in [DEVELOPMENT.md](DEVELOPMENT.md). ### 4. Ongoing local deploys (least privilege) @@ -232,9 +243,9 @@ Build and push an image to Artifact Registry, then `gcloud run deploy` or `terra ## Using an existing RDS host (AWS) -When **ExistingDatabaseHost** is set, the template **does not** create VPC/RDS; a custom resource creates the schema and `syncbot_user_` with a generated app password in Secrets Manager. +When **ExistingDatabaseHost** is set, the template **does not** create VPC/RDS; a custom resource can create the schema and optionally a dedicated app user (default `sbapp_`, or **ExistingDatabaseAppUsername** if set) with a generated app password in Secrets Manager—or skip user/schema steps and copy the admin password into the app secret when **`ExistingDatabaseCreateAppUser=false`**. -- **Public:** Lambda is not in your VPC; RDS must be reachable on the Internet on port **3306** or **5432**. +- **Public:** Lambda is not in your VPC; the DB must be reachable on the Internet on the configured port (**`ExistingDatabasePort`**, or **3306** / **5432** by engine). - **Private:** Lambda uses `ExistingDatabaseSubnetIdsCsv` and `ExistingDatabaseLambdaSecurityGroupId`; DB security group must allow the Lambda SG; subnets need **NAT** egress for Slack API calls. See also [Sharing infrastructure across apps](#sharing-infrastructure-across-apps-aws) below. @@ -270,7 +281,10 @@ See also [Sharing infrastructure across apps](#sharing-infrastructure-across-app ## Database schema (Alembic) -Schema lives under `syncbot/db/alembic/`. On startup the app runs **`alembic upgrade head`**. +Schema lives under `syncbot/db/alembic/`. **`alembic upgrade head`** runs: + +- **AWS (GitHub Actions):** After `sam deploy`, the workflow invokes the Lambda with `{"action":"migrate"}` (migrations + warm instance). Manual `sam deploy` from the guided script should be followed by the same invoke (see script post-deploy or run `aws lambda invoke` with that payload using stack output `SyncBotFunctionArn`). +- **Cloud Run / `python app.py`:** At process startup before the server listens. --- diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index b2affa4..1d45cd8 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -62,13 +62,18 @@ syncbot/ ## Dependency management -After `poetry add` / `poetry update`, regenerate the pinned file used by the Docker image and **`pip-audit`** in CI so it matches `poetry.lock`: +After `poetry add` / `poetry update`, keep `poetry.lock` and the pinned requirements files aligned: + +- **Recommended:** Install [pre-commit](https://pre-commit.com) (`pip install pre-commit && pre-commit install`). When you commit a change to `poetry.lock`, the **`sync-requirements`** hook runs `poetry export` and refreshes **`syncbot/requirements.txt`** and **`infra/aws/db_setup/requirements.txt`** (the DbSetup Lambda subset) automatically. + +- **Without pre-commit:** Run the export yourself (Poetry 2.x needs the export plugin once: `poetry self add poetry-plugin-export`): ```bash -poetry self add poetry-plugin-export # Poetry 2.x; once per Poetry install poetry export -f requirements.txt --without-hashes -o syncbot/requirements.txt +echo "# Required for MySQL 8+ caching_sha2_password; pin for reproducible CI (sam build)." > infra/aws/db_setup/requirements.txt +grep -E "^(pymysql|psycopg2-binary|cryptography)==" syncbot/requirements.txt >> infra/aws/db_setup/requirements.txt ``` -The root **`./deploy.sh`** may run `poetry update` and regenerate `syncbot/requirements.txt` when Poetry is on your `PATH` (see [DEPLOYMENT.md](DEPLOYMENT.md)). +The root **`./deploy.sh`** dependency-sync menu may run `poetry update` and regenerate both requirements files when Poetry is on your `PATH` (see [DEPLOYMENT.md](DEPLOYMENT.md)). -CI runs `pip-audit` on `syncbot/requirements.txt` and `infra/aws/db_setup/requirements.txt` (see [.github/workflows/ci.yml](../.github/workflows/ci.yml)). +The AWS deploy workflow runs **`pip-audit`** on `syncbot/requirements.txt` and `infra/aws/db_setup/requirements.txt` (see [.github/workflows/deploy-aws.yml](../.github/workflows/deploy-aws.yml)). CI verifies both files match `poetry.lock` (see [.github/workflows/ci.yml](../.github/workflows/ci.yml)). diff --git a/docs/INFRA_CONTRACT.md b/docs/INFRA_CONTRACT.md index 216dabe..afa63a0 100644 --- a/docs/INFRA_CONTRACT.md +++ b/docs/INFRA_CONTRACT.md @@ -4,7 +4,7 @@ This document defines what any infrastructure provider (AWS, GCP, Azure, etc.) m **Deploy entrypoint:** From the repo root, `./deploy.sh` (macOS/Linux, or Git Bash/WSL bash) or `.\deploy.ps1` (Windows PowerShell — finds Git Bash or WSL, then bash) runs an interactive helper that delegates to `infra//scripts/deploy.sh`. After identity/auth prompts, each provider script shows a **Deploy Tasks** menu (comma-separated numbers, default all): bootstrap (AWS only), build/deploy, CI/CD (GitHub Actions), Slack API configuration, and DR backup secret output—so operators can run subsets (e.g. CI/CD only against an existing stack) without mid-flow surprises. That flow sets Cloud/Terraform resources and runtime env vars consistent with this document. Step-by-step and manual alternatives: [DEPLOYMENT.md](DEPLOYMENT.md). -**Schema:** The database schema is managed by **Alembic**. On startup the app runs **`alembic upgrade head`** so new and existing databases stay current with the latest migrations. +**Schema:** The database schema is managed by **Alembic** (`alembic upgrade head`). **AWS Lambda:** Migrations are **not** run on every cold start (that would exceed Slack’s interaction ack budget). The Lambda handler accepts a post-deploy invoke with payload `{"action":"migrate"}` to run migrations; the reference GitHub Actions deploy workflow invokes this after `sam deploy`. **Cloud Run / local / container:** Migrations still run at process startup before the HTTP server accepts traffic (no Slack ack on that path). ## Runtime Environment Variables @@ -18,12 +18,7 @@ The application reads configuration from environment variables. Providers must i - CI Python version - `pyproject.toml` Python constraint - `syncbot/requirements.txt` deployment pins -- When dependency constraints change in `pyproject.toml`, refresh both lock and deployment requirements: - -```bash -poetry lock -poetry export --only main --format requirements.txt --without-hashes --output syncbot/requirements.txt -``` +- When dependency constraints change in `pyproject.toml`, refresh the lockfile and deployment requirements. The **pre-commit `sync-requirements` hook** regenerates **`syncbot/requirements.txt`** and **`infra/aws/db_setup/requirements.txt`** from `poetry.lock` when you commit lockfile changes. Manually: `poetry lock`, then `poetry export -f requirements.txt --without-hashes -o syncbot/requirements.txt` and rebuild `infra/aws/db_setup/requirements.txt` as in [.pre-commit-config.yaml](../.pre-commit-config.yaml). ### Database (backend-agnostic) @@ -32,8 +27,8 @@ poetry export --only main --format requirements.txt --without-hashes --output sy | `DATABASE_BACKEND` | `mysql` (default), `postgresql`, or `sqlite`. | | `DATABASE_URL` | Full SQLAlchemy URL. When set, overrides host/user/password/schema. **Required for SQLite** (e.g. `sqlite:///path/to/syncbot.db`). For `mysql` / `postgresql`, optional if unset (legacy vars below are used). | | `DATABASE_HOST` | Database hostname (IP or FQDN). Required when backend is `mysql` or `postgresql` and `DATABASE_URL` is unset. | -| `DATABASE_PORT` | Optional. Defaults to **5432** for `postgresql`, **3306** for `mysql`. | -| `DATABASE_USER` | Username. Required when backend is `mysql` or `postgresql` and `DATABASE_URL` is unset. | +| `DATABASE_PORT` | Optional. Defaults to **5432** for `postgresql`, **3306** for `mysql`. Set explicitly for external providers that use a non-standard port (e.g. TiDB Cloud **4000**). | +| `DATABASE_USER` | Username. Required when backend is `mysql` or `postgresql` and `DATABASE_URL` is unset. Some providers (e.g. TiDB Cloud Serverless) require a cluster-specific prefix on every SQL user; AWS SAM exposes **`ExistingDatabaseUsernamePrefix`** so the bootstrap Lambda prepends it to **ExistingDatabaseAdminUser** and to the default app user `{prefix}.sbapp_{stage}` (a dot separator is added automatically; use bare admin names like `root` when set). **`ExistingDatabaseAppUsername`** (AWS) / **`existing_db_app_username`** (GCP) optionally sets the full app username and bypasses prefix + default. New stack-managed RDS uses master **`sbadmin_{stage}`** and app **`sbapp_{stage}`**. | | `DATABASE_PASSWORD` | Password. Required when backend is `mysql` or `postgresql` and `DATABASE_URL` is unset. | | `DATABASE_SCHEMA` | Database name (MySQL) or PostgreSQL database name (same convention as MySQL). Use alphanumeric and underscore only for PostgreSQL when the app must `CREATE DATABASE` at bootstrap. | | `DATABASE_TLS_ENABLED` | Optional TLS toggle (`true`/`false`). Defaults to enabled outside local dev. | @@ -56,7 +51,7 @@ poetry export --only main --format requirements.txt --without-hashes --output sy | `SLACK_USER_SCOPES` | Comma-separated OAuth **user** scopes. Must match `oauth_config.scopes.user` and `syncbot/slack_manifest_scopes.py` `USER_SCOPES`. If this env requests scopes that are not declared on the Slack app, install fails with `invalid_scope`. | | `TOKEN_ENCRYPTION_KEY` | **Required** in production; must be a strong, random value (e.g. 16+ characters). Providers may auto-generate it (e.g. AWS Secrets Manager). Back up the key after first deploy. In local dev you may set it manually or leave unset. | -**Reference wiring:** AWS SAM ([`infra/aws/template.yaml`](../infra/aws/template.yaml)) maps CloudFormation parameters to Lambda env: **`SlackOauthBotScopes`** / **`SlackOauthUserScopes`** → **`SLACK_BOT_SCOPES`** / **`SLACK_USER_SCOPES`** (defaults match `BOT_SCOPES` / `USER_SCOPES`); **`LogLevel`** → **`LOG_LEVEL`**; **`RequireAdmin`** → **`REQUIRE_ADMIN`**; **`SoftDeleteRetentionDays`** → **`SOFT_DELETE_RETENTION_DAYS`**; **`SyncbotFederationEnabled`**, **`SyncbotInstanceId`**, **`SyncbotPublicUrl`** (optional override) → federation env vars; **`PrimaryWorkspace`** → **`PRIMARY_WORKSPACE`**; **`EnableDbReset`** → **`ENABLE_DB_RESET`** (boolean `true` when enabled); optional **`DatabaseTlsEnabled`** / **`DatabaseSslCaPath`** → **`DATABASE_TLS_ENABLED`** / **`DATABASE_SSL_CA_PATH`** (omit when empty so app defaults apply). **`SYNCBOT_PUBLIC_URL`** defaults to the API Gateway stage base URL unless **`SyncbotPublicUrl`** is set; stack output **`SyncBotPublicBaseUrl`** documents that base. GCP Terraform uses **`secret_slack_bot_scopes`** (Secret Manager → `SLACK_BOT_SCOPES`) and variables **`slack_user_scopes`**, **`log_level`**, **`require_admin`**, **`database_backend`**, **`database_port`**, **`soft_delete_retention_days`**, **`syncbot_federation_enabled`**, **`syncbot_instance_id`**, **`syncbot_public_url_override`**, **`primary_workspace`**, **`enable_db_reset`**, **`database_tls_enabled`**, **`database_ssl_ca_path`** for the corresponding runtime env on Cloud Run (see [infra/gcp/README.md](../infra/gcp/README.md)); **`syncbot_public_url_override`** is empty by default—set it to your service’s public HTTPS base (e.g. after first deploy) if you need **`SYNCBOT_PUBLIC_URL`** for federation. +**Reference wiring:** AWS SAM ([`infra/aws/template.yaml`](../infra/aws/template.yaml)) maps CloudFormation parameters to Lambda env: **`SlackOauthBotScopes`** / **`SlackOauthUserScopes`** → **`SLACK_BOT_SCOPES`** / **`SLACK_USER_SCOPES`** (defaults match `BOT_SCOPES` / `USER_SCOPES`); **`LogLevel`** → **`LOG_LEVEL`**; **`RequireAdmin`** → **`REQUIRE_ADMIN`**; **`SoftDeleteRetentionDays`** → **`SOFT_DELETE_RETENTION_DAYS`**; **`SyncbotFederationEnabled`**, **`SyncbotInstanceId`**, **`SyncbotPublicUrl`** (optional override) → federation env vars; **`PrimaryWorkspace`** → **`PRIMARY_WORKSPACE`**; **`EnableDbReset`** → **`ENABLE_DB_RESET`** (boolean `true` when enabled); optional **`DatabaseTlsEnabled`** / **`DatabaseSslCaPath`** → **`DATABASE_TLS_ENABLED`** / **`DATABASE_SSL_CA_PATH`** (omit when empty so app defaults apply). When using an **existing** DB host: optional **`ExistingDatabasePort`** → **`DATABASE_PORT`** (empty uses engine default); **`ExistingDatabaseCreateAppUser`** / **`ExistingDatabaseCreateSchema`** control the DB setup custom resource (dedicated app user and `CREATE DATABASE`), not direct Lambda env names—see [DEPLOYMENT.md](DEPLOYMENT.md). **`SYNCBOT_PUBLIC_URL`** defaults to the API Gateway stage base URL unless **`SyncbotPublicUrl`** is set; stack output **`SyncBotPublicBaseUrl`** documents that base. GCP Terraform uses **`secret_slack_bot_scopes`** (Secret Manager → `SLACK_BOT_SCOPES`) and variables **`slack_user_scopes`**, **`log_level`**, **`require_admin`**, **`database_backend`**, **`database_port`**, **`soft_delete_retention_days`**, **`syncbot_federation_enabled`**, **`syncbot_instance_id`**, **`syncbot_public_url_override`**, **`primary_workspace`**, **`enable_db_reset`**, **`database_tls_enabled`**, **`database_ssl_ca_path`** for the corresponding runtime env on Cloud Run (see [infra/gcp/README.md](../infra/gcp/README.md)); when **`use_existing_database`** is true, **`existing_db_create_app_user`** / **`existing_db_create_schema`** are recorded as Cloud Run service labels for operator documentation. **`syncbot_public_url_override`** is empty by default—set it to your service’s public HTTPS base (e.g. after first deploy) if you need **`SYNCBOT_PUBLIC_URL`** for federation. ### Optional @@ -93,7 +88,7 @@ The provider must deliver: **PostgreSQL / MySQL:** In non–local environments the app uses TLS by default; allow outbound TCP to the DB host (typically **5432** for PostgreSQL, **3306** for MySQL). **SQLite:** No network; the app uses a local file. Single-writer; ensure backups and file durability for production use. 4. **Keep-warm / scheduled ping (optional but recommended)** - To avoid cold-start latency, the app supports a periodic HTTP GET to a configurable path. The provider should support a scheduled job (e.g. CloudWatch Events, Cloud Scheduler) that hits the service on an interval (e.g. 5 minutes). + To avoid cold-start latency, the app supports a periodic HTTP GET to a configurable path. The provider should support a scheduled job (e.g. CloudWatch Events, Cloud Scheduler) that hits the service on an interval (e.g. 5 minutes). **AWS (SAM):** EventBridge Scheduler invokes the Lambda directly on a schedule; the Lambda handler returns a small JSON success for `source` `aws.scheduler` / `aws.events` without treating the payload as a Slack request. 5. **Stateless execution** The app is stateless; state lives in the configured database (PostgreSQL, MySQL, or SQLite). Horizontal scaling is supported with PostgreSQL/MySQL as long as all instances share the same DB and env; SQLite is single-writer. diff --git a/infra/aws/db_setup/handler.py b/infra/aws/db_setup/handler.py index f72a328..1eff719 100644 --- a/infra/aws/db_setup/handler.py +++ b/infra/aws/db_setup/handler.py @@ -1,16 +1,16 @@ """ Custom CloudFormation resource: create database and app user for SyncBot. -Supports MySQL (port 3306) and PostgreSQL (port 5432). It can use: -- explicit admin password (existing-host mode), or -- admin password fetched from an admin secret ARN (new-RDS mode). +Supports MySQL and PostgreSQL with configurable port, optional schema creation, +and optional dedicated app user (external DBs may disallow CREATE USER). """ +import base64 import json import re -import base64 -import time import socket +import ssl +import time import boto3 import psycopg2 @@ -66,9 +66,7 @@ def handler(event, context): try: send(event, context, "FAILED", reason=f"Unhandled error: {e}") except Exception as send_err: - raise RuntimeError( - f"Unhandled error in handler: {e}; failed to notify CloudFormation: {send_err}" - ) from e + raise RuntimeError(f"Unhandled error in handler: {e}; failed to notify CloudFormation: {send_err}") from e raise @@ -78,6 +76,44 @@ def _safe_ident(name: str) -> str: return name +def _safe_username(name: str) -> str: + """Validate a database username. Allows dots for provider prefixes (e.g. TiDB Cloud).""" + if not re.match(r"^[A-Za-z0-9_][A-Za-z0-9_.]*$", name): + raise ValueError(f"Invalid username: {name}") + return name + + +def _default_port(database_engine: str) -> int: + return 3306 if database_engine == "mysql" else 5432 + + +def _parse_port(props: dict, database_engine: str) -> int: + raw = (props.get("Port") or "").strip() + if not raw: + return _default_port(database_engine) + try: + p = int(raw) + except ValueError as exc: + raise ValueError(f"Invalid Port: {raw!r}") from exc + if p < 1 or p > 65535: + raise ValueError(f"Invalid Port out of range: {p}") + return p + + +def _parse_bool_prop(props: dict, key: str, default: bool = True) -> bool: + v = (props.get(key) or "").strip().lower() + if v == "false": + return False + if v == "true": + return True + return default + + +def put_secret_string(secret_arn: str, secret_string: str) -> None: + client = boto3.client("secretsmanager") + client.put_secret_value(SecretId=secret_arn, SecretString=secret_string) + + def _handler_impl(event, context): request_type = event.get("RequestType", "Create") props = event.get("ResourceProperties", {}) @@ -89,6 +125,9 @@ def _handler_impl(event, context): stage = (props.get("Stage") or "test").strip() secret_arn = (props.get("SecretArn") or "").strip() database_engine = (props.get("DatabaseEngine") or "mysql").strip().lower() + port = _parse_port(props, database_engine) + create_app_user = _parse_bool_prop(props, "CreateAppUser", default=True) + create_schema = _parse_bool_prop(props, "CreateSchema", default=True) if request_type == "Delete": # Must return the same PhysicalResourceId as Create; never use a placeholder. @@ -113,12 +152,23 @@ def _handler_impl(event, context): ) return - app_username = f"syncbot_user_{stage}".replace("-", "_") - try: - app_password = get_secret_value(secret_arn) - except Exception as e: - send(event, context, "FAILED", reason=f"GetSecretValue failed: {e}") - return + username_prefix = (props.get("UsernamePrefix") or "").strip() + if username_prefix and not username_prefix.endswith("."): + username_prefix += "." + if username_prefix: + admin_user = f"{username_prefix}{admin_user}" + app_username_override = (props.get("AppUsername") or "").strip() + if app_username_override: + app_username = app_username_override + else: + app_username = f"{username_prefix}sbapp_{stage}".replace("-", "_") + app_password = "" + if create_app_user: + try: + app_password = get_secret_value(secret_arn) + except Exception as e: + send(event, context, "FAILED", reason=f"GetSecretValue failed: {e}") + return if not admin_password: try: # RDS-managed master-user secrets store JSON; extract the password field. @@ -127,33 +177,52 @@ def _handler_impl(event, context): send(event, context, "FAILED", reason=f"Get admin secret failed: {e}") return + result_username = app_username if create_app_user else admin_user + physical_resource_id = result_username + try: # Fail fast on obvious network connectivity issues before opening DB client sessions. - _assert_tcp_reachable(host, 3306 if database_engine == "mysql" else 5432) - if database_engine == "mysql": - setup_database_mysql( - host=host, - admin_user=admin_user, - admin_password=admin_password, - schema=schema, - app_username=app_username, - app_password=app_password, - ) - else: - setup_database_postgresql( - host=host, - admin_user=admin_user, - admin_password=admin_password, - schema=schema, - app_username=app_username, - app_password=app_password, - ) + _assert_tcp_reachable(host, port) + if create_schema or create_app_user: + if database_engine == "mysql": + setup_database_mysql( + host=host, + admin_user=admin_user, + admin_password=admin_password, + schema=schema, + app_username=app_username, + app_password=app_password, + port=port, + create_schema=create_schema, + create_app_user=create_app_user, + ) + else: + setup_database_postgresql( + host=host, + admin_user=admin_user, + admin_password=admin_password, + schema=schema, + app_username=app_username, + app_password=app_password, + port=port, + create_schema=create_schema, + create_app_user=create_app_user, + ) + if not create_app_user: + put_secret_string(secret_arn, admin_password) except Exception as e: send(event, context, "FAILED", reason=f"Database setup failed: {e}") return - send(event, context, "SUCCESS", {"Username": app_username}, reason="OK", physical_resource_id=app_username) - return {"Username": app_username} + send( + event, + context, + "SUCCESS", + {"Username": result_username}, + reason="OK", + physical_resource_id=physical_resource_id, + ) + return {"Username": result_username} def _assert_tcp_reachable(host: str, port: int) -> None: @@ -169,9 +238,7 @@ def _assert_tcp_reachable(host: str, port: int) -> None: time.sleep(DB_CONNECT_RETRY_SECONDS) finally: sock.close() - raise RuntimeError( - f"Cannot reach {host}:{port} over TCP after {DB_CONNECT_ATTEMPTS} attempts: {last_exc}" - ) + raise RuntimeError(f"Cannot reach {host}:{port} over TCP after {DB_CONNECT_ATTEMPTS} attempts: {last_exc}") def get_secret_value(secret_arn: str, json_key: str | None = None) -> str: @@ -207,9 +274,13 @@ def setup_database_mysql( schema: str, app_username: str, app_password: str, + port: int, + create_schema: bool, + create_app_user: bool, ) -> None: safe_schema = _safe_ident(schema) - _safe_ident(app_username) + if create_app_user: + _safe_username(app_username) conn = None last_exc = None for _attempt in range(1, DB_CONNECT_ATTEMPTS + 1): @@ -218,28 +289,29 @@ def setup_database_mysql( host=host, user=admin_user, password=admin_password, - port=3306, + port=port, charset="utf8mb4", cursorclass=DictCursor, connect_timeout=DB_CONNECT_TIMEOUT_SECONDS, + ssl=ssl.create_default_context(), ) break except Exception as exc: last_exc = exc time.sleep(DB_CONNECT_RETRY_SECONDS) if conn is None: - raise RuntimeError( - f"MySQL connect failed after {DB_CONNECT_ATTEMPTS} attempts: {last_exc}" - ) + raise RuntimeError(f"MySQL connect failed after {DB_CONNECT_ATTEMPTS} attempts: {last_exc}") try: with conn.cursor() as cur: - cur.execute(f"CREATE DATABASE IF NOT EXISTS `{safe_schema}`") - cur.execute( - "CREATE USER IF NOT EXISTS %s@'%%' IDENTIFIED BY %s", - (app_username, app_password), - ) - cur.execute(f"GRANT ALL PRIVILEGES ON `{safe_schema}`.* TO %s@'%%'", (app_username,)) - cur.execute("FLUSH PRIVILEGES") + if create_schema: + cur.execute(f"CREATE DATABASE IF NOT EXISTS `{safe_schema}`") + if create_app_user: + cur.execute( + "CREATE USER IF NOT EXISTS %s@'%%' IDENTIFIED BY %s", + (app_username, app_password), + ) + cur.execute(f"GRANT ALL PRIVILEGES ON `{safe_schema}`.* TO %s@'%%'", (app_username,)) + cur.execute("FLUSH PRIVILEGES") conn.commit() finally: conn.close() @@ -253,16 +325,22 @@ def setup_database_postgresql( schema: str, app_username: str, app_password: str, + port: int, + create_schema: bool, + create_app_user: bool, ) -> None: max_db_connect_attempts = POSTGRES_DB_CONNECT_ATTEMPTS db_connect_retry_seconds = POSTGRES_DB_CONNECT_RETRY_SECONDS _safe_ident(schema) - _safe_ident(app_username) + if create_app_user: + _safe_username(app_username) + _safe_username(admin_user) + conn = psycopg2.connect( host=host, user=admin_user, password=admin_password, - port=5432, + port=port, dbname="postgres", connect_timeout=DB_CONNECT_TIMEOUT_SECONDS, sslmode="require", @@ -270,29 +348,42 @@ def setup_database_postgresql( conn.autocommit = True try: with conn.cursor() as cur: - cur.execute("SELECT 1 FROM pg_roles WHERE rolname = %s", (app_username,)) - if cur.fetchone() is None: - q = psql.SQL("CREATE ROLE {name} WITH LOGIN PASSWORD %s").format( - name=psql.Identifier(app_username), - ) - cur.execute(q, (app_password,)) - else: - q = psql.SQL("ALTER ROLE {name} WITH LOGIN PASSWORD %s").format( - name=psql.Identifier(app_username), - ) - cur.execute(q, (app_password,)) - - cur.execute("SELECT 1 FROM pg_database WHERE datname = %s", (schema,)) - if cur.fetchone() is None: - cur.execute( - psql.SQL("CREATE DATABASE {db} OWNER {owner}").format( - db=psql.Identifier(schema), - owner=psql.Identifier(app_username), + if create_app_user: + cur.execute("SELECT 1 FROM pg_roles WHERE rolname = %s", (app_username,)) + if cur.fetchone() is None: + q = psql.SQL("CREATE ROLE {name} WITH LOGIN PASSWORD %s").format( + name=psql.Identifier(app_username), ) - ) + cur.execute(q, (app_password,)) + else: + q = psql.SQL("ALTER ROLE {name} WITH LOGIN PASSWORD %s").format( + name=psql.Identifier(app_username), + ) + cur.execute(q, (app_password,)) + + if create_schema: + cur.execute("SELECT 1 FROM pg_database WHERE datname = %s", (schema,)) + if cur.fetchone() is None: + if create_app_user: + cur.execute( + psql.SQL("CREATE DATABASE {db} OWNER {owner}").format( + db=psql.Identifier(schema), + owner=psql.Identifier(app_username), + ) + ) + else: + cur.execute( + psql.SQL("CREATE DATABASE {db} OWNER {owner}").format( + db=psql.Identifier(schema), + owner=psql.Identifier(admin_user), + ) + ) finally: conn.close() + if not create_app_user: + return + # Ensure runtime role can connect and run migrations in the target DB. # After CREATE DATABASE, RDS can take a short time before accepting connections. last_exc = None @@ -302,7 +393,7 @@ def setup_database_postgresql( host=host, user=admin_user, password=admin_password, - port=5432, + port=port, dbname=schema, connect_timeout=DB_CONNECT_TIMEOUT_SECONDS, sslmode="require", @@ -328,6 +419,6 @@ def setup_database_postgresql( last_exc = exc time.sleep(db_connect_retry_seconds) raise RuntimeError( - f"Failed connecting to newly created database '{schema}' after " + f"Failed connecting to database '{schema}' after " f"{max_db_connect_attempts} attempts: {last_exc}" ) diff --git a/infra/aws/db_setup/requirements.txt b/infra/aws/db_setup/requirements.txt index 4e51763..9deb9fe 100644 --- a/infra/aws/db_setup/requirements.txt +++ b/infra/aws/db_setup/requirements.txt @@ -1,4 +1,4 @@ -pymysql==1.1.2 -psycopg2-binary==2.9.11 -# Required for MySQL 8+ caching_sha2_password; pin for reproducible CI (pip-audit / sam build). -cryptography==46.0.6 +# Required for MySQL 8+ caching_sha2_password; pin for reproducible CI (sam build). +cryptography==46.0.6 ; python_version >= "3.12" and python_version < "4.0" +psycopg2-binary==2.9.11 ; python_version >= "3.12" and python_version < "4.0" +pymysql==1.1.2 ; python_version >= "3.12" and python_version < "4.0" diff --git a/infra/aws/scripts/deploy.sh b/infra/aws/scripts/deploy.sh index 0380d76..1ff50bd 100755 --- a/infra/aws/scripts/deploy.sh +++ b/infra/aws/scripts/deploy.sh @@ -97,6 +97,92 @@ required_from_env_or_prompt() { fi } +# When local env overrides differ from the CloudFormation stack (e.g. GitHub-deployed TiDB vs .env RDS), +# prompt the operator instead of silently preferring env. +resolve_with_conflict_check() { + local label="$1" + local env_value="$2" + local stack_value="$3" + local prompt_default_value="$4" + local mode="${5:-plain}" # plain|secret|bool + + if [[ -z "$env_value" ]]; then + if [[ "$mode" == "secret" ]]; then + prompt_secret_required "$label" + elif [[ "$mode" == "bool" ]]; then + local yn_def="${prompt_default_value:-y}" + if prompt_yes_no "$label" "$yn_def"; then + echo "true" + else + echo "false" + fi + else + prompt_default "$label" "$prompt_default_value" + fi + return 0 + fi + + if [[ "$mode" == "bool" ]]; then + if [[ "$env_value" != "true" && "$env_value" != "false" ]]; then + echo "Error: environment value for $label must be true or false." >&2 + exit 1 + fi + if [[ -z "$stack_value" || "$env_value" == "$stack_value" ]]; then + echo "Using $label from environment variable." >&2 + echo "$env_value" + return 0 + fi + echo "" >&2 + echo "CONFLICT: $label differs between local env and deployed stack:" >&2 + echo " Local env: $env_value" >&2 + echo " AWS stack: $stack_value" >&2 + local choice + read -r -p "Use (l)ocal env / (a)ws stack / (e)nter new value? [l/a/e]: " choice >&2 + case "$choice" in + a | A) echo "$stack_value" ;; + e | E) + if prompt_yes_no "$label" "${prompt_default_value:-y}"; then + echo "true" + else + echo "false" + fi + ;; + *) echo "$env_value" ;; + esac + return 0 + fi + + if [[ -z "$stack_value" || "$env_value" == "$stack_value" ]]; then + echo "Using $label from environment variable." >&2 + echo "$env_value" + return 0 + fi + + local display_env="$env_value" + local display_stack="$stack_value" + if [[ "$mode" == "secret" ]]; then + display_env="(hidden)" + display_stack="(hidden)" + fi + echo "" >&2 + echo "CONFLICT: $label differs between local env and deployed stack:" >&2 + echo " Local env: $display_env" >&2 + echo " AWS stack: $display_stack" >&2 + local choice + read -r -p "Use (l)ocal env / (a)ws stack / (e)nter new value? [l/a/e]: " choice >&2 + case "$choice" in + a | A) echo "$stack_value" ;; + e | E) + if [[ "$mode" == "secret" ]]; then + prompt_secret_required "$label" + else + prompt_required "$label" + fi + ;; + *) echo "$env_value" ;; + esac +} + prompt_yes_no() { local prompt="$1" local default="${2:-y}" @@ -331,6 +417,11 @@ configure_github_actions_aws() { # $12 Comma-separated subnet IDs for Lambda in private mode # $13 Lambda ENI security group id in private mode # $14 Database engine: mysql | postgresql + # $15 Existing DB port (empty = engine default in SAM) + # $16 ExistingDatabaseCreateAppUser: true | false + # $17 ExistingDatabaseCreateSchema: true | false + # $18 ExistingDatabaseUsernamePrefix (e.g. TiDB cluster prefix; empty for RDS) + # $19 ExistingDatabaseAppUsername (optional full app DB user; empty = default) local bootstrap_outputs="$1" local bootstrap_stack_name="$2" local aws_region="$3" @@ -347,6 +438,13 @@ configure_github_actions_aws() { local existing_db_lambda_sg_id="${13:-}" local database_engine="${14:-}" [[ -z "$database_engine" ]] && database_engine="mysql" + local existing_db_port="${15:-}" + local existing_db_create_app_user="${16:-true}" + local existing_db_create_schema="${17:-true}" + local existing_db_username_prefix="${18:-}" + local existing_db_app_username="${19:-}" + [[ -z "$existing_db_create_app_user" ]] && existing_db_create_app_user="true" + [[ -z "$existing_db_create_schema" ]] && existing_db_create_schema="true" local role bucket boot_region role="$(output_value "$bootstrap_outputs" "GitHubDeployRoleArn")" bucket="$(output_value "$bootstrap_outputs" "DeploymentBucketName")" @@ -403,6 +501,11 @@ configure_github_actions_aws() { gh variable set EXISTING_DATABASE_SUBNET_IDS_CSV --env "$env_name" --body "" -R "$repo" gh variable set EXISTING_DATABASE_LAMBDA_SECURITY_GROUP_ID --env "$env_name" --body "" -R "$repo" fi + gh variable set EXISTING_DATABASE_PORT --env "$env_name" --body "$existing_db_port" -R "$repo" + gh variable set EXISTING_DATABASE_CREATE_APP_USER --env "$env_name" --body "$existing_db_create_app_user" -R "$repo" + gh variable set EXISTING_DATABASE_CREATE_SCHEMA --env "$env_name" --body "$existing_db_create_schema" -R "$repo" + gh variable set EXISTING_DATABASE_USERNAME_PREFIX --env "$env_name" --body "$existing_db_username_prefix" -R "$repo" + gh variable set EXISTING_DATABASE_APP_USERNAME --env "$env_name" --body "$existing_db_app_username" -R "$repo" else # Clear existing-host vars for new-RDS mode to avoid stale CI config. gh variable set EXISTING_DATABASE_HOST --env "$env_name" --body "" -R "$repo" @@ -410,6 +513,11 @@ configure_github_actions_aws() { gh variable set EXISTING_DATABASE_NETWORK_MODE --env "$env_name" --body "public" -R "$repo" gh variable set EXISTING_DATABASE_SUBNET_IDS_CSV --env "$env_name" --body "" -R "$repo" gh variable set EXISTING_DATABASE_LAMBDA_SECURITY_GROUP_ID --env "$env_name" --body "" -R "$repo" + gh variable set EXISTING_DATABASE_PORT --env "$env_name" --body "" -R "$repo" + gh variable set EXISTING_DATABASE_CREATE_APP_USER --env "$env_name" --body "true" -R "$repo" + gh variable set EXISTING_DATABASE_CREATE_SCHEMA --env "$env_name" --body "true" -R "$repo" + gh variable set EXISTING_DATABASE_USERNAME_PREFIX --env "$env_name" --body "" -R "$repo" + gh variable set EXISTING_DATABASE_APP_USERNAME --env "$env_name" --body "" -R "$repo" fi echo "Environment variables updated for '$env_name'." fi @@ -849,11 +957,13 @@ validate_private_existing_db_connectivity() { local db_vpc="$5" local db_sgs_csv="$6" local db_host="$7" + local db_port_override="${8:-}" local db_port subnet_list subnet_vpcs first_vpc line subnet_id subnet_vpc db_sg_id lambda_sg_vpc db_sg_list route_target rt_id ingress_ok local -a no_nat_subnets db_port="3306" [[ "$engine" == "postgresql" ]] && db_port="5432" + [[ -n "$db_port_override" ]] && db_port="$db_port_override" IFS=',' read -r -a subnet_list <<< "$subnet_csv" if [[ "${#subnet_list[@]}" -lt 1 ]]; then @@ -1187,6 +1297,11 @@ PREV_EXISTING_DATABASE_ADMIN_USER="" PREV_EXISTING_DATABASE_NETWORK_MODE="" PREV_EXISTING_DATABASE_SUBNET_IDS_CSV="" PREV_EXISTING_DATABASE_LAMBDA_SG_ID="" +PREV_EXISTING_DATABASE_PORT="" +PREV_EXISTING_DATABASE_CREATE_APP_USER="" +PREV_EXISTING_DATABASE_CREATE_SCHEMA="" +PREV_EXISTING_DATABASE_USERNAME_PREFIX="" +PREV_EXISTING_DATABASE_APP_USERNAME="" PREV_DATABASE_ENGINE="" PREV_DATABASE_SCHEMA="" PREV_LOG_LEVEL="" @@ -1215,6 +1330,11 @@ if [[ -n "$EXISTING_STACK_STATUS" && "$EXISTING_STACK_STATUS" != "None" ]]; then PREV_EXISTING_DATABASE_NETWORK_MODE="$(stack_param_value "$EXISTING_STACK_PARAMS" "ExistingDatabaseNetworkMode")" PREV_EXISTING_DATABASE_SUBNET_IDS_CSV="$(stack_param_value "$EXISTING_STACK_PARAMS" "ExistingDatabaseSubnetIdsCsv")" PREV_EXISTING_DATABASE_LAMBDA_SG_ID="$(stack_param_value "$EXISTING_STACK_PARAMS" "ExistingDatabaseLambdaSecurityGroupId")" + PREV_EXISTING_DATABASE_PORT="$(stack_param_value "$EXISTING_STACK_PARAMS" "ExistingDatabasePort")" + PREV_EXISTING_DATABASE_CREATE_APP_USER="$(stack_param_value "$EXISTING_STACK_PARAMS" "ExistingDatabaseCreateAppUser")" + PREV_EXISTING_DATABASE_CREATE_SCHEMA="$(stack_param_value "$EXISTING_STACK_PARAMS" "ExistingDatabaseCreateSchema")" + PREV_EXISTING_DATABASE_USERNAME_PREFIX="$(stack_param_value "$EXISTING_STACK_PARAMS" "ExistingDatabaseUsernamePrefix")" + PREV_EXISTING_DATABASE_APP_USERNAME="$(stack_param_value "$EXISTING_STACK_PARAMS" "ExistingDatabaseAppUsername")" PREV_DATABASE_ENGINE="$(stack_param_value "$EXISTING_STACK_PARAMS" "DatabaseEngine")" PREV_DATABASE_SCHEMA="$(stack_param_value "$EXISTING_STACK_PARAMS" "DatabaseSchema")" PREV_LOG_LEVEL="$(stack_param_value "$EXISTING_STACK_PARAMS" "LogLevel")" @@ -1361,6 +1481,11 @@ SLACK_CLIENT_ID="$(required_from_env_or_prompt "SLACK_CLIENT_ID" "SlackClientID" ENV_EXISTING_DATABASE_HOST="${EXISTING_DATABASE_HOST:-}" ENV_EXISTING_DATABASE_ADMIN_USER="${EXISTING_DATABASE_ADMIN_USER:-}" ENV_EXISTING_DATABASE_ADMIN_PASSWORD="${EXISTING_DATABASE_ADMIN_PASSWORD:-}" +ENV_EXISTING_DATABASE_PORT="${EXISTING_DATABASE_PORT:-}" +ENV_EXISTING_DATABASE_CREATE_APP_USER="${EXISTING_DATABASE_CREATE_APP_USER:-}" +ENV_EXISTING_DATABASE_CREATE_SCHEMA="${EXISTING_DATABASE_CREATE_SCHEMA:-}" +ENV_EXISTING_DATABASE_USERNAME_PREFIX="${EXISTING_DATABASE_USERNAME_PREFIX:-}" +ENV_EXISTING_DATABASE_APP_USERNAME="${EXISTING_DATABASE_APP_USERNAME:-}" EXISTING_DB_ADMIN_PASSWORD_SOURCE="prompt" EXISTING_DATABASE_HOST="" EXISTING_DATABASE_ADMIN_USER="" @@ -1368,6 +1493,12 @@ EXISTING_DATABASE_ADMIN_PASSWORD="" EXISTING_DATABASE_NETWORK_MODE="public" EXISTING_DATABASE_SUBNET_IDS_CSV="" EXISTING_DATABASE_LAMBDA_SG_ID="" +EXISTING_DATABASE_PORT="" +EXISTING_DATABASE_CREATE_APP_USER="true" +EXISTING_DATABASE_CREATE_SCHEMA="true" +EXISTING_DATABASE_USERNAME_PREFIX="" +EXISTING_DATABASE_APP_USERNAME="" +EXISTING_DB_EFFECTIVE_PORT="" DATABASE_SCHEMA="" DATABASE_SCHEMA_DEFAULT="syncbot_${STAGE}" if [[ "$IS_STACK_UPDATE" == "true" && -n "$PREV_DATABASE_SCHEMA" ]]; then @@ -1382,12 +1513,11 @@ if [[ "$DB_MODE" == "2" ]]; then EXISTING_DATABASE_ADMIN_USER_DEFAULT="admin" [[ -n "$PREV_EXISTING_DATABASE_ADMIN_USER" ]] && EXISTING_DATABASE_ADMIN_USER_DEFAULT="$PREV_EXISTING_DATABASE_ADMIN_USER" - if [[ -n "$ENV_EXISTING_DATABASE_HOST" ]]; then - echo "Using ExistingDatabaseHost from environment variable EXISTING_DATABASE_HOST." - EXISTING_DATABASE_HOST="$ENV_EXISTING_DATABASE_HOST" - else - EXISTING_DATABASE_HOST="$(prompt_default "ExistingDatabaseHost (RDS endpoint hostname)" "$EXISTING_DATABASE_HOST_DEFAULT")" - fi + EXISTING_DATABASE_HOST="$(resolve_with_conflict_check \ + "ExistingDatabaseHost (RDS endpoint hostname)" \ + "$ENV_EXISTING_DATABASE_HOST" \ + "$PREV_EXISTING_DATABASE_HOST" \ + "$EXISTING_DATABASE_HOST_DEFAULT")" DETECTED_ADMIN_USER="" DETECTED_ADMIN_SECRET_ARN="" @@ -1403,12 +1533,11 @@ if [[ "$DB_MODE" == "2" ]]; then if [[ -z "$EXISTING_DATABASE_ADMIN_USER_DEFAULT" || "$EXISTING_DATABASE_ADMIN_USER_DEFAULT" == "admin" ]]; then [[ -n "$DETECTED_ADMIN_USER" ]] && EXISTING_DATABASE_ADMIN_USER_DEFAULT="$DETECTED_ADMIN_USER" fi - if [[ -n "$ENV_EXISTING_DATABASE_ADMIN_USER" ]]; then - echo "Using ExistingDatabaseAdminUser from environment variable EXISTING_DATABASE_ADMIN_USER." - EXISTING_DATABASE_ADMIN_USER="$ENV_EXISTING_DATABASE_ADMIN_USER" - else - EXISTING_DATABASE_ADMIN_USER="$(prompt_default "ExistingDatabaseAdminUser" "$EXISTING_DATABASE_ADMIN_USER_DEFAULT")" - fi + EXISTING_DATABASE_ADMIN_USER="$(resolve_with_conflict_check \ + "ExistingDatabaseAdminUser" \ + "$ENV_EXISTING_DATABASE_ADMIN_USER" \ + "$PREV_EXISTING_DATABASE_ADMIN_USER" \ + "$EXISTING_DATABASE_ADMIN_USER_DEFAULT")" if [[ -n "$ENV_EXISTING_DATABASE_ADMIN_PASSWORD" ]]; then echo "Using ExistingDatabaseAdminPassword from environment variable EXISTING_DATABASE_ADMIN_PASSWORD." @@ -1431,6 +1560,60 @@ if [[ "$DB_MODE" == "2" ]]; then DATABASE_SCHEMA="$(prompt_default "DatabaseSchema" "$DATABASE_SCHEMA_DEFAULT")" + echo + echo "=== Existing database port and setup ===" + echo "Leave port blank to use the engine default (3306 MySQL, 5432 PostgreSQL)." + DEFAULT_EXISTING_DB_PORT="" + [[ -n "$PREV_EXISTING_DATABASE_PORT" ]] && DEFAULT_EXISTING_DB_PORT="$PREV_EXISTING_DATABASE_PORT" + EXISTING_DATABASE_PORT="$(resolve_with_conflict_check \ + "ExistingDatabasePort (optional)" \ + "$ENV_EXISTING_DATABASE_PORT" \ + "$PREV_EXISTING_DATABASE_PORT" \ + "$DEFAULT_EXISTING_DB_PORT")" + if [[ "$DATABASE_ENGINE" == "mysql" && "$EXISTING_DATABASE_PORT" == "3306" ]]; then + EXISTING_DATABASE_PORT="" + fi + if [[ "$DATABASE_ENGINE" == "postgresql" && "$EXISTING_DATABASE_PORT" == "5432" ]]; then + EXISTING_DATABASE_PORT="" + fi + EXISTING_DB_EFFECTIVE_PORT="3306" + [[ "$DATABASE_ENGINE" == "postgresql" ]] && EXISTING_DB_EFFECTIVE_PORT="5432" + [[ -n "$EXISTING_DATABASE_PORT" ]] && EXISTING_DB_EFFECTIVE_PORT="$EXISTING_DATABASE_PORT" + + CREATE_APP_DEFAULT="y" + [[ "${PREV_EXISTING_DATABASE_CREATE_APP_USER:-}" == "false" ]] && CREATE_APP_DEFAULT="n" + EXISTING_DATABASE_CREATE_APP_USER="$(resolve_with_conflict_check \ + "Create dedicated app DB user (CREATE USER / grants)?" \ + "$ENV_EXISTING_DATABASE_CREATE_APP_USER" \ + "${PREV_EXISTING_DATABASE_CREATE_APP_USER:-}" \ + "$CREATE_APP_DEFAULT" \ + bool)" + + CREATE_SCHEMA_DEFAULT="y" + [[ "${PREV_EXISTING_DATABASE_CREATE_SCHEMA:-}" == "false" ]] && CREATE_SCHEMA_DEFAULT="n" + EXISTING_DATABASE_CREATE_SCHEMA="$(resolve_with_conflict_check \ + "Run CREATE DATABASE IF NOT EXISTS for DatabaseSchema?" \ + "$ENV_EXISTING_DATABASE_CREATE_SCHEMA" \ + "${PREV_EXISTING_DATABASE_CREATE_SCHEMA:-}" \ + "$CREATE_SCHEMA_DEFAULT" \ + bool)" + + EXISTING_DATABASE_USERNAME_PREFIX_DEFAULT="" + [[ -n "$PREV_EXISTING_DATABASE_USERNAME_PREFIX" ]] && EXISTING_DATABASE_USERNAME_PREFIX_DEFAULT="$PREV_EXISTING_DATABASE_USERNAME_PREFIX" + EXISTING_DATABASE_USERNAME_PREFIX="$(resolve_with_conflict_check \ + "DB username prefix (e.g. abc123 for TiDB Cloud; blank for RDS/standard)" \ + "$ENV_EXISTING_DATABASE_USERNAME_PREFIX" \ + "$PREV_EXISTING_DATABASE_USERNAME_PREFIX" \ + "$EXISTING_DATABASE_USERNAME_PREFIX_DEFAULT")" + + EXISTING_DATABASE_APP_USERNAME_DEFAULT="" + [[ -n "$PREV_EXISTING_DATABASE_APP_USERNAME" ]] && EXISTING_DATABASE_APP_USERNAME_DEFAULT="$PREV_EXISTING_DATABASE_APP_USERNAME" + EXISTING_DATABASE_APP_USERNAME="$(resolve_with_conflict_check \ + "ExistingDatabaseAppUsername (optional; full app user, bypasses prefix+sbapp_{stage}; blank for default)" \ + "$ENV_EXISTING_DATABASE_APP_USERNAME" \ + "$PREV_EXISTING_DATABASE_APP_USERNAME" \ + "$EXISTING_DATABASE_APP_USERNAME_DEFAULT")" + if [[ -z "$EXISTING_DATABASE_HOST" || "$EXISTING_DATABASE_HOST" == REPLACE_ME* ]]; then echo "Error: valid ExistingDatabaseHost is required for existing DB mode." >&2 exit 1 @@ -1514,7 +1697,8 @@ if [[ "$DB_MODE" == "2" ]]; then "$EXISTING_DATABASE_LAMBDA_SG_ID" \ "$DETECTED_VPC" \ "$DETECTED_SGS" \ - "$EXISTING_DATABASE_HOST"; then + "$EXISTING_DATABASE_HOST" \ + "$EXISTING_DB_EFFECTIVE_PORT"; then echo "Fix network settings and rerun deploy." >&2 exit 1 fi @@ -1523,8 +1707,8 @@ else echo echo "=== New RDS Database ===" echo "New RDS mode uses:" - echo " - admin user: syncbot_admin_${STAGE} (password auto-generated)" - echo " - app user: syncbot_user_${STAGE} (password auto-generated)" + echo " - admin user: sbadmin_${STAGE} (password auto-generated)" + echo " - app user: sbapp_${STAGE} (password auto-generated)" DATABASE_SCHEMA="$(prompt_default "DatabaseSchema" "$DATABASE_SCHEMA_DEFAULT")" fi @@ -1605,12 +1789,38 @@ if [[ "$DB_MODE" == "2" ]]; then echo "DB subnets: $EXISTING_DATABASE_SUBNET_IDS_CSV" echo "Lambda SG: $EXISTING_DATABASE_LAMBDA_SG_ID" fi + echo "DB port: ${EXISTING_DB_EFFECTIVE_PORT:-engine default}" + echo "DB create user: $EXISTING_DATABASE_CREATE_APP_USER" + echo "DB create schema: $EXISTING_DATABASE_CREATE_SCHEMA" + echo "DB admin user (parameter): $EXISTING_DATABASE_ADMIN_USER" + if [[ -n "$EXISTING_DATABASE_APP_USERNAME" ]]; then + echo "DB app username override: $EXISTING_DATABASE_APP_USERNAME" + fi + if [[ -n "$EXISTING_DATABASE_USERNAME_PREFIX" ]]; then + _dbpfx="$EXISTING_DATABASE_USERNAME_PREFIX" + [[ "$_dbpfx" != *. ]] && _dbpfx="${_dbpfx}." + echo "DB username prefix: $EXISTING_DATABASE_USERNAME_PREFIX" + echo " effective admin (bootstrap): ${_dbpfx}${EXISTING_DATABASE_ADMIN_USER}" + if [[ -n "$EXISTING_DATABASE_APP_USERNAME" ]]; then + echo " effective app user (if created): $EXISTING_DATABASE_APP_USERNAME (override)" + else + echo " effective app user (if created): ${_dbpfx}sbapp_${STAGE//-/_}" + fi + else + echo "DB username prefix: (none)" + echo " admin (bootstrap): $EXISTING_DATABASE_ADMIN_USER" + if [[ -n "$EXISTING_DATABASE_APP_USERNAME" ]]; then + echo " app user (if created): $EXISTING_DATABASE_APP_USERNAME (override)" + else + echo " app user (if created): sbapp_${STAGE//-/_}" + fi + fi echo "DB schema: $DATABASE_SCHEMA" else echo "DB mode: create new RDS" echo "DB engine: $DATABASE_ENGINE" - echo "DB admin user: syncbot_admin_${STAGE} (auto password)" - echo "DB app user: syncbot_user_${STAGE} (auto password)" + echo "DB admin user: sbadmin_${STAGE} (auto password)" + echo "DB app user: sbapp_${STAGE} (auto password)" echo "DB schema: $DATABASE_SCHEMA" fi if [[ -n "$TOKEN_OVERRIDE" ]]; then @@ -1680,6 +1890,13 @@ if [[ "$DB_MODE" == "2" ]]; then "ExistingDatabaseLambdaSecurityGroupId=$EXISTING_DATABASE_LAMBDA_SG_ID" ) fi + [[ -n "$EXISTING_DATABASE_PORT" ]] && PARAMS+=("ExistingDatabasePort=$EXISTING_DATABASE_PORT") + PARAMS+=( + "ExistingDatabaseCreateAppUser=$EXISTING_DATABASE_CREATE_APP_USER" + "ExistingDatabaseCreateSchema=$EXISTING_DATABASE_CREATE_SCHEMA" + "ExistingDatabaseUsernamePrefix=$EXISTING_DATABASE_USERNAME_PREFIX" + "ExistingDatabaseAppUsername=$EXISTING_DATABASE_APP_USERNAME" + ) else # Clear existing-host parameters on updates to avoid stale previous values. # SAM rejects Key= (empty value) in shorthand; use ParameterKey=K,ParameterValue= instead. @@ -1690,6 +1907,11 @@ else "ExistingDatabaseNetworkMode=public" "ParameterKey=ExistingDatabaseSubnetIdsCsv,ParameterValue=" "ParameterKey=ExistingDatabaseLambdaSecurityGroupId,ParameterValue=" + "ParameterKey=ExistingDatabasePort,ParameterValue=" + "ExistingDatabaseCreateAppUser=true" + "ExistingDatabaseCreateSchema=true" + "ParameterKey=ExistingDatabaseUsernamePrefix,ParameterValue=" + "ParameterKey=ExistingDatabaseAppUsername,ParameterValue=" ) fi @@ -1716,6 +1938,21 @@ sam deploy \ APP_OUTPUTS="$(app_describe_outputs "$STACK_NAME" "$REGION")" + FUNCTION_ARN="$(output_value "$APP_OUTPUTS" "SyncBotFunctionArn")" + if [[ -n "$FUNCTION_ARN" ]]; then + echo "=== Lambda migrate + warm-up ===" + TMP_MIGRATE="$(mktemp)" + aws lambda invoke \ + --function-name "$FUNCTION_ARN" \ + --payload '{"action":"migrate"}' \ + --cli-binary-format raw-in-base64-out \ + "$TMP_MIGRATE" \ + --region "$REGION" + cat "$TMP_MIGRATE" + echo + rm -f "$TMP_MIGRATE" + fi + else echo echo "Skipping Build/Deploy (task 2 not selected)." @@ -1734,6 +1971,13 @@ else EXISTING_DATABASE_NETWORK_MODE="${PREV_EXISTING_DATABASE_NETWORK_MODE:-public}" EXISTING_DATABASE_SUBNET_IDS_CSV="${PREV_EXISTING_DATABASE_SUBNET_IDS_CSV:-}" EXISTING_DATABASE_LAMBDA_SG_ID="${PREV_EXISTING_DATABASE_LAMBDA_SG_ID:-}" + EXISTING_DATABASE_PORT="${PREV_EXISTING_DATABASE_PORT:-}" + EXISTING_DATABASE_CREATE_APP_USER="${PREV_EXISTING_DATABASE_CREATE_APP_USER:-true}" + EXISTING_DATABASE_CREATE_SCHEMA="${PREV_EXISTING_DATABASE_CREATE_SCHEMA:-true}" + EXISTING_DATABASE_USERNAME_PREFIX="${PREV_EXISTING_DATABASE_USERNAME_PREFIX:-}" + EXISTING_DATABASE_APP_USERNAME="${PREV_EXISTING_DATABASE_APP_USERNAME:-}" + [[ -z "$EXISTING_DATABASE_CREATE_APP_USER" ]] && EXISTING_DATABASE_CREATE_APP_USER="true" + [[ -z "$EXISTING_DATABASE_CREATE_SCHEMA" ]] && EXISTING_DATABASE_CREATE_SCHEMA="true" SLACK_SIGNING_SECRET="${SLACK_SIGNING_SECRET:-}" SLACK_CLIENT_SECRET="${SLACK_CLIENT_SECRET:-}" SLACK_CLIENT_ID="${SLACK_CLIENT_ID:-}" @@ -1780,7 +2024,12 @@ if [[ "$TASK_CICD" == "true" ]]; then "$EXISTING_DATABASE_NETWORK_MODE" \ "$EXISTING_DATABASE_SUBNET_IDS_CSV" \ "$EXISTING_DATABASE_LAMBDA_SG_ID" \ - "$DATABASE_ENGINE" + "$DATABASE_ENGINE" \ + "${EXISTING_DATABASE_PORT:-}" \ + "${EXISTING_DATABASE_CREATE_APP_USER:-true}" \ + "${EXISTING_DATABASE_CREATE_SCHEMA:-true}" \ + "${EXISTING_DATABASE_USERNAME_PREFIX:-}" \ + "${EXISTING_DATABASE_APP_USERNAME:-}" fi if [[ "$TASK_BUILD_DEPLOY" == "true" || "$TASK_BACKUP_SECRETS" == "true" ]]; then diff --git a/infra/aws/template.bootstrap.yaml b/infra/aws/template.bootstrap.yaml index 5c19c82..b8110b4 100644 --- a/infra/aws/template.bootstrap.yaml +++ b/infra/aws/template.bootstrap.yaml @@ -164,6 +164,7 @@ Resources: - lambda:AddPermission - lambda:RemovePermission - lambda:PublishVersion + - lambda:InvokeFunction Resource: !Sub "arn:aws:lambda:${AWS::Region}:${AWS::AccountId}:function:syncbot-*" - Sid: ApiGateway Effect: Allow diff --git a/infra/aws/template.yaml b/infra/aws/template.yaml index dc57999..b223272 100644 --- a/infra/aws/template.yaml +++ b/infra/aws/template.yaml @@ -90,6 +90,7 @@ Parameters: Description: > Database admin user that can create databases and users (e.g. RDS master). Used only when ExistingDatabaseHost is set; the deploy creates a dedicated app user and schema. + When ExistingDatabaseUsernamePrefix is set, use the bare name (e.g. root); the prefix is prepended in the bootstrap Lambda. Type: String Default: "" @@ -126,6 +127,51 @@ Parameters: Type: String Default: "" + ExistingDatabasePort: + Description: > + TCP port for ExistingDatabaseHost (DATABASE_PORT). Leave empty to use engine default (3306 MySQL, 5432 PostgreSQL). + Set for external providers with a non-standard port (e.g. TiDB Cloud 4000). Ignored when creating new RDS in stack. + Type: String + Default: "" + + ExistingDatabaseCreateAppUser: + Description: > + When ExistingDatabaseHost is set: "true" (default) creates a dedicated app user and grants schema access. + Set "false" for managed DBs that disallow CREATE USER; admin password is copied to the app DB secret and DATABASE_USER is the admin user. + Type: String + Default: "true" + AllowedValues: + - "true" + - "false" + + ExistingDatabaseCreateSchema: + Description: > + When ExistingDatabaseHost is set: "true" (default) runs CREATE DATABASE IF NOT EXISTS for DatabaseSchema. + Set "false" when the database already exists. Ignored when creating new RDS in stack. + Type: String + Default: "true" + AllowedValues: + - "true" + - "false" + + ExistingDatabaseUsernamePrefix: + Description: > + Username prefix required by some managed DB providers (e.g. TiDB Cloud cluster prefix "abc123"). + A dot separator is added automatically if missing. Prepended to ExistingDatabaseAdminUser and to the + default app user ({prefix}.sbapp_{Stage}) unless ExistingDatabaseAppUsername is set. + Use the bare admin name (e.g. root) when this is set. + Leave empty for standard databases (RDS, self-hosted). Ignored when creating new RDS in stack. + Type: String + Default: "" + + ExistingDatabaseAppUsername: + Description: > + When ExistingDatabaseHost is set and CreateAppUser is true: optional full app DB username (bypasses + prefix + default sbapp_{Stage}). Use when the auto-generated name exceeds provider limits (e.g. MySQL 32 chars) + or you need a fixed name. Leave empty for default {prefix}.sbapp_{Stage}. Ignored when creating new RDS in stack. + Type: String + Default: "" + DatabaseSchema: Description: > Database/schema name for MySQL or PostgreSQL. Each app sharing an RDS instance @@ -286,6 +332,9 @@ Conditions: UseExistingDatabasePrivateVpc: !And - !Condition UseExistingDatabase - !Equals [!Ref ExistingDatabaseNetworkMode, "private"] + UseExistingDatabaseWithCustomPort: !And + - !Condition UseExistingDatabase + - !Not [!Equals [!Ref ExistingDatabasePort, ""]] HasTokenEncryptionKeyOverride: !Not [!Equals [!Ref TokenEncryptionKeyOverride, ""]] HasExistingTokenEncryptionKeySecretArn: !Not [!Equals [!Ref ExistingTokenEncryptionKeySecretArn, ""]] HasAppDbPasswordOverride: !Not [!Equals [!Ref AppDbPasswordOverride, ""]] @@ -462,7 +511,7 @@ Resources: Engine: mysql # Minor version must match cfn-lint / RDS allowed list (major-only "8.0" fails E3691) EngineVersion: "8.0.40" - MasterUsername: !Sub "syncbot_admin_${Stage}" + MasterUsername: !Sub "sbadmin_${Stage}" ManageMasterUserPassword: true DBName: !Ref DatabaseSchema AllocatedStorage: 20 @@ -495,7 +544,7 @@ Resources: DBInstanceClass: !Ref DatabaseInstanceClass Engine: postgres EngineVersion: "16.6" - MasterUsername: !Sub "syncbot_admin_${Stage}" + MasterUsername: !Sub "sbadmin_${Stage}" ManageMasterUserPassword: true DBName: !Ref DatabaseSchema AllocatedStorage: 20 @@ -573,7 +622,9 @@ Resources: - Version: "2012-10-17" Statement: - Effect: Allow - Action: secretsmanager:GetSecretValue + Action: + - secretsmanager:GetSecretValue + - secretsmanager:PutSecretValue Resource: - !If - HasAppDbPasswordOverride @@ -607,7 +658,7 @@ Resources: AdminUser: !If - UseExistingDatabase - !Ref ExistingDatabaseAdminUser - - !Sub "syncbot_admin_${Stage}" + - !Sub "sbadmin_${Stage}" AdminPassword: !If - UseExistingDatabase - !Ref ExistingDatabaseAdminPassword @@ -626,6 +677,29 @@ Resources: - !Ref AppDbCredentialsSecretProvided - !Ref AppDbCredentialsSecretGenerated DatabaseEngine: !Ref DatabaseEngine + Port: !If + - UseExistingDatabaseWithCustomPort + - !Ref ExistingDatabasePort + - !If + - IsMysqlEngine + - "3306" + - "5432" + CreateAppUser: !If + - UseExistingDatabase + - !Ref ExistingDatabaseCreateAppUser + - "true" + CreateSchema: !If + - UseExistingDatabase + - !Ref ExistingDatabaseCreateSchema + - "true" + UsernamePrefix: !If + - UseExistingDatabase + - !Ref ExistingDatabaseUsernamePrefix + - "" + AppUsername: !If + - UseExistingDatabase + - !Ref ExistingDatabaseAppUsername + - "" # ============================================================ # Lambda Function @@ -645,7 +719,7 @@ Resources: Architectures: - x86_64 Timeout: 30 - MemorySize: 128 + MemorySize: 256 Policies: - AWSLambdaVPCAccessExecutionRole VpcConfig: !If @@ -688,9 +762,12 @@ Resources: SLACK_CLIENT_ID: !Ref SlackClientID DATABASE_BACKEND: !Ref DatabaseEngine DATABASE_PORT: !If - - IsMysqlEngine - - "3306" - - "5432" + - UseExistingDatabaseWithCustomPort + - !Ref ExistingDatabasePort + - !If + - IsMysqlEngine + - "3306" + - "5432" DATABASE_HOST: !If - UseExistingDatabase - !Ref ExistingDatabaseHost diff --git a/infra/gcp/README.md b/infra/gcp/README.md index 0d37b14..2fea415 100644 --- a/infra/gcp/README.md +++ b/infra/gcp/README.md @@ -48,6 +48,8 @@ Minimal Terraform scaffold to run SyncBot on Google Cloud. Satisfies the [infras | `stage` | Stage name, e.g. `test` or `prod` | | `use_existing_database` | If `true`, use `existing_db_*` vars instead of creating Cloud SQL | | `existing_db_host`, `existing_db_schema`, `existing_db_user` | Existing MySQL connection (when `use_existing_database = true`) | +| `existing_db_username_prefix` | Optional (e.g. TiDB Cloud `abc123`). A dot separator is added automatically. When set, `DATABASE_USER` is `{prefix}.sbapp_{stage}` unless `existing_db_app_username` is set; `existing_db_user` is ignored | +| `existing_db_app_username` | Optional full `DATABASE_USER` (bypasses prefix + `sbapp_{stage}` and `existing_db_user`) | | `cloud_run_image` | Container image URL for Cloud Run (set after first build) | | `secret_slack_bot_scopes` | Secret Manager secret ID for **bot** OAuth scopes (runtime `SLACK_BOT_SCOPES`; default `syncbot-slack-scopes`). The **secret value** must match `oauth_config.scopes.bot` / `BOT_SCOPES` (same requirement as AWS SAM `SlackOauthBotScopes`). | | `slack_user_scopes` | Plain-text **user** OAuth scopes for Cloud Run (`SLACK_USER_SCOPES`). Default matches repo standard (same comma-separated string as AWS SAM `SlackOauthUserScopes`); must match manifest `oauth_config.scopes.user` and `USER_SCOPES` in `slack_manifest_scopes.py`. | diff --git a/infra/gcp/main.tf b/infra/gcp/main.tf index 8a34047..fb2c46a 100644 --- a/infra/gcp/main.tf +++ b/infra/gcp/main.tf @@ -44,7 +44,17 @@ locals { length(google_sql_database_instance.main) > 0 ? google_sql_database_instance.main[0].public_ip_address : "" ) db_schema = var.use_existing_database ? var.existing_db_schema : "syncbot" - db_user = var.use_existing_database ? var.existing_db_user : "syncbot_app" + stage_sbapp_user = "sbapp_${replace(var.stage, "-", "_")}" + normalized_prefix = ( + trimspace(var.existing_db_username_prefix) != "" + ? (endswith(trimspace(var.existing_db_username_prefix), ".") ? trimspace(var.existing_db_username_prefix) : "${trimspace(var.existing_db_username_prefix)}.") + : "" + ) + db_user = var.use_existing_database ? ( + trimspace(var.existing_db_app_username) != "" ? trimspace(var.existing_db_app_username) : ( + local.normalized_prefix != "" ? "${local.normalized_prefix}${local.stage_sbapp_user}" : var.existing_db_user + ) + ) : "syncbot_app" # Non-secret Cloud Run env (see docs/INFRA_CONTRACT.md) syncbot_public_url_effective = trimspace(var.syncbot_public_url_override) != "" ? trimspace(var.syncbot_public_url_override) : "" @@ -263,6 +273,14 @@ resource "google_cloud_run_v2_service" "syncbot" { location = var.region ingress = "INGRESS_TRAFFIC_ALL" + labels = merge( + {}, + var.use_existing_database ? { + syncbot_existing_db_create_app_user = var.existing_db_create_app_user ? "true" : "false" + syncbot_existing_db_create_schema = var.existing_db_create_schema ? "true" : "false" + } : {}, + ) + template { service_account = google_service_account.cloud_run.email diff --git a/infra/gcp/scripts/deploy.sh b/infra/gcp/scripts/deploy.sh index 90af4ec..8f28b9b 100755 --- a/infra/gcp/scripts/deploy.sh +++ b/infra/gcp/scripts/deploy.sh @@ -569,6 +569,9 @@ fi if [[ "$TASK_BUILD_DEPLOY" == "true" ]]; then echo echo "=== Configuration ===" +DB_PORT="3306" +EXISTING_DB_CREATE_APP_USER="true" +EXISTING_DB_CREATE_SCHEMA="true" echo "=== Database Source ===" # USE_EXISTING=true: point Terraform at an external DB only (use_existing_database); skip creating Cloud SQL. # USE_EXISTING_DEFAULT: y/n default for the prompt when redeploying without a managed instance for this stage. @@ -600,17 +603,50 @@ if [[ -n "$EXISTING_SERVICE_URL" ]]; then DETECTED_EXISTING_USER="$(cloud_run_env_value "$PROJECT_ID" "$REGION" "$SERVICE_NAME" "DATABASE_USER")" fi if [[ "$USE_EXISTING" == "true" ]]; then + EXISTING_DB_APP_USERNAME="" EXISTING_HOST="$(prompt_line "Existing DB host" "$DETECTED_EXISTING_HOST")" EXISTING_SCHEMA="$(prompt_line "Database schema name" "${DETECTED_EXISTING_SCHEMA:-syncbot}")" - EXISTING_USER="$(prompt_line "Database user" "$DETECTED_EXISTING_USER")" + EXISTING_DB_USERNAME_PREFIX="$(prompt_line "DB username prefix (optional; e.g. TiDB Cloud abc123; blank = enter full DB user next)" "")" + if [[ -n "$EXISTING_DB_USERNAME_PREFIX" ]]; then + EXISTING_USER="" + else + EXISTING_USER="$(prompt_line "Database user" "$DETECTED_EXISTING_USER")" + fi + EXISTING_DB_APP_USERNAME="$(prompt_line "Override DATABASE_USER (optional; full username e.g. TiDB-prefixed; blank = prefix+sbapp_{stage} or Database user above)" "")" if [[ -z "$EXISTING_HOST" ]]; then echo "Error: Existing DB host is required when using existing database mode." >&2 exit 1 fi - if [[ -z "$EXISTING_USER" ]]; then - echo "Error: Database user is required when using existing database mode." >&2 + if [[ -z "$EXISTING_USER" && -z "$EXISTING_DB_USERNAME_PREFIX" && -z "$EXISTING_DB_APP_USERNAME" ]]; then + echo "Error: Database user, DB username prefix, or DATABASE_USER override is required when using existing database mode." >&2 + exit 1 + fi + + echo + echo "=== Existing database port and setup (operator / external DB) ===" + DEFAULT_DB_PORT="3306" + if [[ -n "$EXISTING_SERVICE_URL" ]]; then + DETECTED_DB_PORT_EARLY="$(cloud_run_env_value "$PROJECT_ID" "$REGION" "$SERVICE_NAME" "DATABASE_PORT")" + [[ -n "$DETECTED_DB_PORT_EARLY" ]] && DEFAULT_DB_PORT="$DETECTED_DB_PORT_EARLY" + fi + DB_PORT="$(prompt_line "Database TCP port (DATABASE_PORT)" "$DEFAULT_DB_PORT")" + if [[ -z "$DB_PORT" ]]; then + echo "Error: Database TCP port is required when using existing database mode." >&2 exit 1 fi + + CREATE_APP_DEF="y" + CREATE_SCHEMA_DEF="y" + if prompt_yn "Create dedicated app DB user on the server (CREATE USER / grants)?" "$CREATE_APP_DEF"; then + EXISTING_DB_CREATE_APP_USER="true" + else + EXISTING_DB_CREATE_APP_USER="false" + fi + if prompt_yn "Run CREATE DATABASE IF NOT EXISTS for DatabaseSchema (you or a hook)?" "$CREATE_SCHEMA_DEF"; then + EXISTING_DB_CREATE_SCHEMA="true" + else + EXISTING_DB_CREATE_SCHEMA="false" + fi fi DETECTED_CLOUD_IMAGE="" @@ -652,7 +688,6 @@ ENABLE_DB_RESET_VAR="" DB_TLS_VAR="" DB_SSL_CA_VAR="" DB_BACKEND="mysql" -DB_PORT="3306" if [[ -n "$EXISTING_SERVICE_URL" ]]; then DETECTED_RA="$(cloud_run_env_value "$PROJECT_ID" "$REGION" "$SERVICE_NAME" "REQUIRE_ADMIN")" [[ -n "$DETECTED_RA" ]] && REQUIRE_ADMIN_DEFAULT="$DETECTED_RA" @@ -679,8 +714,10 @@ if [[ -n "$EXISTING_SERVICE_URL" ]]; then DB_SSL_CA_VAR="${DETECTED_DB_SSL_CA:-}" DETECTED_DB_BACKEND="$(cloud_run_env_value "$PROJECT_ID" "$REGION" "$SERVICE_NAME" "DATABASE_BACKEND")" [[ -n "$DETECTED_DB_BACKEND" ]] && DB_BACKEND="$DETECTED_DB_BACKEND" - DETECTED_DB_PORT="$(cloud_run_env_value "$PROJECT_ID" "$REGION" "$SERVICE_NAME" "DATABASE_PORT")" - [[ -n "$DETECTED_DB_PORT" ]] && DB_PORT="$DETECTED_DB_PORT" + if [[ "$USE_EXISTING" != "true" ]]; then + DETECTED_DB_PORT="$(cloud_run_env_value "$PROJECT_ID" "$REGION" "$SERVICE_NAME" "DATABASE_PORT")" + [[ -n "$DETECTED_DB_PORT" ]] && DB_PORT="$DETECTED_DB_PORT" + fi fi echo @@ -726,8 +763,14 @@ if [[ "$USE_EXISTING" == "true" ]]; then VARS+=("-var=existing_db_host=$EXISTING_HOST") VARS+=("-var=existing_db_schema=$EXISTING_SCHEMA") VARS+=("-var=existing_db_user=$EXISTING_USER") + VARS+=("-var=existing_db_username_prefix=$EXISTING_DB_USERNAME_PREFIX") + VARS+=("-var=existing_db_app_username=$EXISTING_DB_APP_USERNAME") + VARS+=("-var=existing_db_create_app_user=$EXISTING_DB_CREATE_APP_USER") + VARS+=("-var=existing_db_create_schema=$EXISTING_DB_CREATE_SCHEMA") else VARS+=("-var=use_existing_database=false") + VARS+=("-var=existing_db_username_prefix=") + VARS+=("-var=existing_db_app_username=") fi VARS+=("-var=cloud_run_image=$CLOUD_IMAGE") diff --git a/infra/gcp/tests/test_terraform_validate.py b/infra/gcp/tests/test_terraform_validate.py index 515f01a..8a02516 100644 --- a/infra/gcp/tests/test_terraform_validate.py +++ b/infra/gcp/tests/test_terraform_validate.py @@ -39,8 +39,7 @@ def test_terraform_validates() -> None: ) if init.returncode != 0: pytest.skip( - "terraform init failed (terraform missing or no network for providers?):\n" - f"{init.stdout}\n{init.stderr}" + f"terraform init failed (terraform missing or no network for providers?):\n{init.stdout}\n{init.stderr}" ) validate = subprocess.run( [tf, "validate"], diff --git a/infra/gcp/variables.tf b/infra/gcp/variables.tf index 9237f90..5d87d7d 100644 --- a/infra/gcp/variables.tf +++ b/infra/gcp/variables.tf @@ -45,7 +45,31 @@ variable "existing_db_schema" { variable "existing_db_user" { type = string default = "" - description = "Existing MySQL user (when use_existing_database = true)" + description = "Existing MySQL user (when use_existing_database = true). Ignored when existing_db_app_username or existing_db_username_prefix is set." +} + +variable "existing_db_username_prefix" { + type = string + default = "" + description = "Optional prefix for DATABASE_USER (e.g. TiDB Cloud cluster prefix \"abc123\"). A dot separator is added automatically. When non-empty, DATABASE_USER is {prefix}.sbapp_{stage} unless existing_db_app_username is set; existing_db_user is ignored." +} + +variable "existing_db_app_username" { + type = string + default = "" + description = "Optional full DATABASE_USER override when use_existing_database (bypasses prefix + sbapp_{stage} and existing_db_user)." +} + +variable "existing_db_create_app_user" { + type = bool + default = true + description = "When use_existing_database: operator note — whether a dedicated app DB user exists (no Cloud SQL user resource; app uses existing_db_user / secret)." +} + +variable "existing_db_create_schema" { + type = bool + default = true + description = "When use_existing_database: operator note — whether DatabaseSchema was created manually (Terraform does not create schema for existing host)." } # --------------------------------------------------------------------------- diff --git a/poetry.lock b/poetry.lock index 7ea3e3b..54d08ad 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 2.1.3 and should not be changed by hand. +# This file is automatically @generated by Poetry 2.3.2 and should not be changed by hand. [[package]] name = "alembic" @@ -22,18 +22,18 @@ tz = ["tzdata"] [[package]] name = "boto3" -version = "1.42.76" +version = "1.42.78" description = "The AWS SDK for Python" optional = false python-versions = ">=3.9" groups = ["dev"] files = [ - {file = "boto3-1.42.76-py3-none-any.whl", hash = "sha256:63c6779c814847016b89ae1b72ed968f8a63d80e589ba337511aa6fc1b59585e"}, - {file = "boto3-1.42.76.tar.gz", hash = "sha256:aa2b1973eee8973a9475d24bb579b1dee7176595338d4e4f7880b5c6189b8814"}, + {file = "boto3-1.42.78-py3-none-any.whl", hash = "sha256:480a34a077484a5ca60124dfd150ba3ea6517fc89963a679e45b30c6db614d26"}, + {file = "boto3-1.42.78.tar.gz", hash = "sha256:cef2ebdb9be5c0e96822f8d3941ac4b816c90a5737a7ffb901d664c808964b63"}, ] [package.dependencies] -botocore = ">=1.42.76,<1.43.0" +botocore = ">=1.42.78,<1.43.0" jmespath = ">=0.7.1,<2.0.0" s3transfer = ">=0.16.0,<0.17.0" @@ -42,14 +42,14 @@ crt = ["botocore[crt] (>=1.21.0,<2.0a0)"] [[package]] name = "botocore" -version = "1.42.76" +version = "1.42.78" description = "Low-level, data-driven core of boto 3." optional = false python-versions = ">=3.9" groups = ["dev"] files = [ - {file = "botocore-1.42.76-py3-none-any.whl", hash = "sha256:151e714ae3c32f68ea0b4dc60751401e03f84a87c6cf864ea0ee64aa10eb4607"}, - {file = "botocore-1.42.76.tar.gz", hash = "sha256:c553fa0ae29e36a5c407f74da78b78404b81b74b15fb62bf640a3cd9385f0874"}, + {file = "botocore-1.42.78-py3-none-any.whl", hash = "sha256:038ab63c7f898e8b5db58cb6a45e4da56c31dd984e7e995839a3540c735564ea"}, + {file = "botocore-1.42.78.tar.gz", hash = "sha256:61cbd49728e23f68cfd945406ab40044d49abed143362f7ffa4a4f4bd4311791"}, ] [package.dependencies] @@ -857,10 +857,10 @@ files = [ ] [package.dependencies] -botocore = ">=1.37.4,<2.0a.0" +botocore = ">=1.37.4,<2.0a0" [package.extras] -crt = ["botocore[crt] (>=1.37.4,<2.0a.0)"] +crt = ["botocore[crt] (>=1.37.4,<2.0a0)"] [[package]] name = "six" diff --git a/syncbot/app.py b/syncbot/app.py index b0d234a..c7f0169 100644 --- a/syncbot/app.py +++ b/syncbot/app.py @@ -56,10 +56,17 @@ ) from routing import MAIN_MAPPER, VIEW_ACK_MAPPER, VIEW_MAPPER -_SENSITIVE_KEYS = frozenset({ - "token", "bot_token", "access_token", "shared_secret", - "public_key", "private_key", "private_key_encrypted", -}) +_SENSITIVE_KEYS = frozenset( + { + "token", + "bot_token", + "access_token", + "shared_secret", + "public_key", + "private_key", + "private_key_encrypted", + } +) def _redact_sensitive(obj, _depth=0): @@ -67,10 +74,7 @@ def _redact_sensitive(obj, _depth=0): if _depth > 10: return obj if isinstance(obj, dict): - return { - k: "[REDACTED]" if k in _SENSITIVE_KEYS else _redact_sensitive(v, _depth + 1) - for k, v in obj.items() - } + return {k: "[REDACTED]" if k in _SENSITIVE_KEYS else _redact_sensitive(v, _depth + 1) for k, v in obj.items()} if isinstance(obj, list): return [_redact_sensitive(v, _depth + 1) for v in obj] return obj @@ -80,7 +84,10 @@ def _redact_sensitive(obj, _depth=0): configure_logging() validate_config() -initialize_database() +# On Lambda, defer Alembic to a post-deploy invoke (see handler migrate branch) so cold +# starts stay under Slack's 3s ack budget. Cloud Run / local still run migrations here. +if not os.environ.get("AWS_LAMBDA_FUNCTION_NAME"): + initialize_database() app = App( process_before_response=not LOCAL_DEVELOPMENT, @@ -109,7 +116,25 @@ def handler(event: dict, context: dict) -> dict: Receives an API Gateway proxy event. Federation API paths (``/api/federation/*``) are handled directly; everything else is delegated to the Slack Bolt request handler. + + Also handles post-deploy ``{"action": "migrate"}`` (Alembic) and EventBridge + keep-warm invokes before Slack routing. """ + if event.get("action") == "migrate": + initialize_database() + return { + "statusCode": 200, + "headers": {"Content-Type": "application/json"}, + "body": json.dumps({"status": "ok", "action": "migrate"}), + } + + if event.get("source") in ("aws.scheduler", "aws.events"): + return { + "statusCode": 200, + "headers": {"Content-Type": "application/json"}, + "body": json.dumps({"status": "ok", "action": "warmup"}), + } + path = event.get("path", "") or event.get("rawPath", "") if path.startswith("/api/federation"): return _lambda_federation_handler(event) @@ -224,11 +249,7 @@ def main_response(body: dict, logger, client, ack, context: dict) -> None: ) raise else: - if not ( - request_type == "view_submission" - and request_id in VIEW_ACK_MAPPER - and request_id not in VIEW_MAPPER - ): + if not (request_type == "view_submission" and request_id in VIEW_ACK_MAPPER and request_id not in VIEW_MAPPER): _logger.error( "no_handler", extra={ @@ -386,9 +407,7 @@ def _handle_federation(self, method: str) -> None: content_len = 0 body_str = self.rfile.read(content_len).decode() if content_len else "" headers = {k: v for k, v in self.headers.items()} - status, resp = dispatch_federation_request( - method, self._path_no_query(), body_str, headers - ) + status, resp = dispatch_federation_request(method, self._path_no_query(), body_str, headers) self._send_raw( status, {"Content-Type": ["application/json"]}, diff --git a/syncbot/constants.py b/syncbot/constants.py index 5c670a0..31cfaea 100644 --- a/syncbot/constants.py +++ b/syncbot/constants.py @@ -101,6 +101,7 @@ def _has_real_bot_token() -> bool: # handles any requests. Fails fast in production; warns in local dev. # --------------------------------------------------------------------------- + def get_database_backend() -> str: """Return ``postgresql``, ``mysql``, or ``sqlite``. diff --git a/syncbot/db/__init__.py b/syncbot/db/__init__.py index 63fb4df..90f9b67 100644 --- a/syncbot/db/__init__.py +++ b/syncbot/db/__init__.py @@ -48,9 +48,7 @@ class DatabaseField: # Repo root locally; Lambda deployment root (/var/task) in AWS — used for relative SQLite paths. _syncbot_dir = Path(__file__).resolve().parent.parent -_PROJECT_ROOT = ( - _syncbot_dir if os.environ.get("AWS_LAMBDA_FUNCTION_NAME") else _syncbot_dir.parent -) +_PROJECT_ROOT = _syncbot_dir if os.environ.get("AWS_LAMBDA_FUNCTION_NAME") else _syncbot_dir.parent def _mysql_port() -> str: @@ -202,6 +200,7 @@ def _ensure_database_exists() -> None: def _alembic_config(): """Build Alembic config with script_location set to syncbot/db/alembic.""" from alembic.config import Config # pyright: ignore[reportMissingImports] + config = Config() config.set_main_option("script_location", str(_ALEMBIC_SCRIPT_LOCATION)) return config @@ -253,22 +252,14 @@ def _drop_all_tables_dialect_aware(engine) -> None: if engine.dialect.name == "postgresql": with engine.begin() as conn: result = conn.execute( - text( - "SELECT tablename FROM pg_tables " - "WHERE schemaname = 'public' ORDER BY tablename" - ) + text("SELECT tablename FROM pg_tables WHERE schemaname = 'public' ORDER BY tablename") ) for (table_name,) in result: conn.execute(text(f'DROP TABLE IF EXISTS "{table_name}" CASCADE')) return with engine.begin() as conn: conn.execute(text("SET FOREIGN_KEY_CHECKS = 0")) - result = conn.execute( - text( - "SELECT TABLE_NAME FROM information_schema.TABLES " - "WHERE TABLE_SCHEMA = DATABASE()" - ) - ) + result = conn.execute(text("SELECT TABLE_NAME FROM information_schema.TABLES WHERE TABLE_SCHEMA = DATABASE()")) for (table_name,) in result: conn.execute(text(f"DROP TABLE IF EXISTS `{table_name}`")) conn.execute(text("SET FOREIGN_KEY_CHECKS = 1")) @@ -283,9 +274,7 @@ def drop_and_init_db() -> None: """ global GLOBAL_ENGINE, GLOBAL_SESSION, GLOBAL_SCHEMA - _logger.critical( - "DB RESET: emptying schema and reinitializing via Alembic. All data will be lost." - ) + _logger.critical("DB RESET: emptying schema and reinitializing via Alembic. All data will be lost.") db_url, connect_args = _get_database_url_and_args() engine = create_engine( @@ -316,9 +305,7 @@ def get_engine(echo: bool = False, schema: str = None): backend = constants.get_database_backend() target_schema = ( - (schema or os.environ.get(constants.DATABASE_SCHEMA, "syncbot")) - if backend in ("mysql", "postgresql") - else "" + (schema or os.environ.get(constants.DATABASE_SCHEMA, "syncbot")) if backend in ("mysql", "postgresql") else "" ) cache_key = target_schema or backend @@ -565,4 +552,3 @@ def delete_records(cls: T, filters, schema=None): raise finally: close_session(session) - diff --git a/syncbot/db/alembic/versions/001_baseline.py b/syncbot/db/alembic/versions/001_baseline.py index eeec36e..69f853f 100644 --- a/syncbot/db/alembic/versions/001_baseline.py +++ b/syncbot/db/alembic/versions/001_baseline.py @@ -5,6 +5,7 @@ Create Date: Baseline from ORM models + OAuth tables """ + from collections.abc import Sequence import sqlalchemy as sa diff --git a/syncbot/federation/api.py b/syncbot/federation/api.py index 91fd017..d63b232 100644 --- a/syncbot/federation/api.py +++ b/syncbot/federation/api.py @@ -45,6 +45,7 @@ def _find_post_records(post_id: str, sync_channel_id: int) -> list[schemas.PostM [schemas.PostMeta.post_id == pid, schemas.PostMeta.sync_channel_id == sync_channel_id], ) + _PAIRING_CODE_RE = re.compile(r"^FED-[0-9A-Fa-f]{8}$") _FIELD_MAX_LENGTHS = { @@ -83,7 +84,9 @@ def _validate_fields(body: dict, required: list[str], extras: list[str] | None = return None -def _pick_user_mapping_for_federated_target(source_user_id: str, target_workspace_id: int) -> schemas.UserMapping | None: +def _pick_user_mapping_for_federated_target( + source_user_id: str, target_workspace_id: int +) -> schemas.UserMapping | None: maps = DbManager.find_records( schemas.UserMapping, [ diff --git a/syncbot/federation/core.py b/syncbot/federation/core.py index 99800f4..f275e2e 100644 --- a/syncbot/federation/core.py +++ b/syncbot/federation/core.py @@ -98,12 +98,8 @@ def get_or_create_instance_keypair(): return private_key, existing[0].public_key private_key = Ed25519PrivateKey.generate() - public_pem = private_key.public_key().public_bytes( - Encoding.PEM, PublicFormat.SubjectPublicKeyInfo - ).decode() - private_pem = private_key.private_bytes( - Encoding.PEM, PrivateFormat.PKCS8, NoEncryption() - ).decode() + public_pem = private_key.public_key().public_bytes(Encoding.PEM, PublicFormat.SubjectPublicKeyInfo).decode() + private_pem = private_key.private_bytes(Encoding.PEM, PrivateFormat.PKCS8, NoEncryption()).decode() record = schemas.InstanceKey( public_key=public_pem, @@ -219,6 +215,7 @@ def validate_webhook_url(url: str) -> bool: return False import socket + try: addr_infos = socket.getaddrinfo(hostname, None) for info in addr_infos: diff --git a/syncbot/handlers/_common.py b/syncbot/handlers/_common.py index bd43ce5..669d736 100644 --- a/syncbot/handlers/_common.py +++ b/syncbot/handlers/_common.py @@ -100,9 +100,7 @@ def _get_selected_conversation_or_option(body: dict, action_id: str) -> str | No """Return selected conversation ID, falling back to selected option value.""" for aid, action_data in _iter_view_state_actions(body): if aid == action_id: - return action_data.get("selected_conversation") or helpers.safe_get( - action_data, "selected_option", "value" - ) + return action_data.get("selected_conversation") or helpers.safe_get(action_data, "selected_option", "value") return None diff --git a/syncbot/handlers/channel_sync.py b/syncbot/handlers/channel_sync.py index d9fe8b4..d12f4a8 100644 --- a/syncbot/handlers/channel_sync.py +++ b/syncbot/handlers/channel_sync.py @@ -708,9 +708,7 @@ def handle_subscribe_channel( channel_options = _get_publishable_channel_options(client, workspace_record.id) if not channel_options: channel_options = [ - orm.SelectorOption( - name="— No Channels available to Sync in this Workspace. —", value="__none__" - ), + orm.SelectorOption(name="— No Channels available to Sync in this Workspace. —", value="__none__"), ] blocks.append( orm.InputBlock( diff --git a/syncbot/handlers/export_import.py b/syncbot/handlers/export_import.py index 45148c0..2b186e3 100644 --- a/syncbot/handlers/export_import.py +++ b/syncbot/handlers/export_import.py @@ -772,7 +772,13 @@ def handle_data_migration_submit_work( if not constants.FEDERATION_ENABLED: return err, data, group_id, team_id_to_workspace_id, workspace_record = _data_migration_prepare(body, client, context) - if err is not None or data is None or group_id is None or team_id_to_workspace_id is None or workspace_record is None: + if ( + err is not None + or data is None + or group_id is None + or team_id_to_workspace_id is None + or workspace_record is None + ): return source = data.get("source_instance") diff --git a/syncbot/handlers/federation_cmds.py b/syncbot/handlers/federation_cmds.py index e0dd65d..8c50f0e 100644 --- a/syncbot/handlers/federation_cmds.py +++ b/syncbot/handlers/federation_cmds.py @@ -345,6 +345,7 @@ def handle_remove_federation_connection( return from datetime import UTC, datetime + now = datetime.now(UTC) DbManager.update_records( schemas.WorkspaceGroupMember, diff --git a/syncbot/handlers/group_manage.py b/syncbot/handlers/group_manage.py index 1f279f4..6a8af46 100644 --- a/syncbot/handlers/group_manage.py +++ b/syncbot/handlers/group_manage.py @@ -48,7 +48,7 @@ def handle_leave_group( blocks=[ orm.SectionBlock( label=( - f":warning: *Are you sure you want to leave the group \"{group.name}\"?*\n\n" + f':warning: *Are you sure you want to leave the group "{group.name}"?*\n\n' "This will:\n" "\u2022 Stop all channel syncs you have in this group\n" "\u2022 Remove your synced message history from this group\n" @@ -162,6 +162,7 @@ def handle_leave_group_confirm( ) from datetime import UTC, datetime + now = datetime.now(UTC) for member in members: DbManager.update_records( diff --git a/syncbot/handlers/messages.py b/syncbot/handlers/messages.py index 00aca22..bfb8784 100644 --- a/syncbot/handlers/messages.py +++ b/syncbot/handlers/messages.py @@ -23,6 +23,7 @@ def _find_source_workspace_id(records: list[tuple], channel_id: str, ws_index: i return workspace.id return None + _logger = logging.getLogger(__name__) diff --git a/syncbot/handlers/sync.py b/syncbot/handlers/sync.py index 4f8878a..2ae0982 100644 --- a/syncbot/handlers/sync.py +++ b/syncbot/handlers/sync.py @@ -119,12 +119,8 @@ def handle_refresh_home( cooldown_sec = getattr(constants, "REFRESH_COOLDOWN_SECONDS", 60) if action == "cooldown" and cached_blocks is not None and remaining is not None: - refresh_idx = helpers.index_of_block_with_action( - cached_blocks, actions.CONFIG_REFRESH_HOME - ) - blocks_with_message = helpers.inject_cooldown_message( - cached_blocks, refresh_idx, remaining - ) + refresh_idx = helpers.index_of_block_with_action(cached_blocks, actions.CONFIG_REFRESH_HOME) + blocks_with_message = helpers.inject_cooldown_message(cached_blocks, refresh_idx, remaining) client.views_publish(user_id=user_id, view={"type": "home", "blocks": blocks_with_message}) return if action == "cached" and cached_blocks is not None: diff --git a/syncbot/helpers/export_import.py b/syncbot/helpers/export_import.py index 43b5352..e4a696f 100644 --- a/syncbot/helpers/export_import.py +++ b/syncbot/helpers/export_import.py @@ -9,7 +9,7 @@ import json import logging import os -from datetime import datetime +from datetime import UTC, datetime from decimal import Decimal from typing import Any @@ -23,12 +23,14 @@ BACKUP_VERSION = 1 MIGRATION_VERSION = 1 _RAW_BACKUP_TABLES = ("slack_bots", "slack_installations", "slack_oauth_states") -_DATETIME_COLUMNS = frozenset({ - "bot_token_expires_at", - "user_token_expires_at", - "installed_at", - "expire_at", -}) +_DATETIME_COLUMNS = frozenset( + { + "bot_token_expires_at", + "user_token_expires_at", + "installed_at", + "expire_at", + } +) def _dump_raw_table(table_name: str) -> list[dict]: @@ -122,11 +124,12 @@ def _records_to_list(records: list, cls: type) -> list[dict]: # Full-instance backup # --------------------------------------------------------------------------- + def build_full_backup() -> dict: """Build full-instance backup payload (all tables, version, exported_at, encryption_key_hash, hmac).""" payload = { "version": BACKUP_VERSION, - "exported_at": datetime.utcnow().isoformat() + "Z", + "exported_at": datetime.now(UTC).isoformat() + "Z", "encryption_key_hash": _compute_encryption_key_hash(), } tables = [ @@ -243,9 +246,11 @@ def restore_full_backup( # Cache invalidation after restore/import # --------------------------------------------------------------------------- + def invalidate_home_tab_caches_for_team(team_id: str) -> None: """Clear home_tab_hash and home_tab_blocks for a team so next Refresh does full rebuild.""" from helpers._cache import _cache_delete_prefix + _cache_delete_prefix(f"home_tab_hash:{team_id}") _cache_delete_prefix(f"home_tab_blocks:{team_id}") @@ -259,6 +264,7 @@ def invalidate_home_tab_caches_for_all_teams(team_ids: list[str]) -> None: def invalidate_sync_list_cache_for_channel(channel_id: str) -> None: """Clear get_sync_list cache for a channel.""" from helpers._cache import _cache_delete + _cache_delete(f"sync_list:{channel_id}") @@ -266,6 +272,7 @@ def invalidate_sync_list_cache_for_channel(channel_id: str) -> None: # Data migration export (workspace-scoped) # --------------------------------------------------------------------------- + def build_migration_export(workspace_id: int, include_source_instance: bool = True) -> dict: """Build workspace-scoped migration JSON. Optionally sign with Ed25519 and include source_instance.""" workspace = DbManager.get_record(schemas.Workspace, workspace_id) @@ -317,27 +324,33 @@ def build_migration_export(workspace_id: int, include_source_instance: bool = Tr tw = DbManager.get_record(schemas.Workspace, sync.target_workspace_id) if tw: tgt_team = tw.team_id - syncs_data.append({ - "title": sync.title, - "sync_mode": sync.sync_mode or "group", - "publisher_team_id": pub_team, - "target_team_id": tgt_team, - "is_publisher": sync.publisher_workspace_id == workspace_id, - }) + syncs_data.append( + { + "title": sync.title, + "sync_mode": sync.sync_mode or "group", + "publisher_team_id": pub_team, + "target_team_id": tgt_team, + "is_publisher": sync.publisher_workspace_id == workspace_id, + } + ) for sync_channel in sync_channels_w: if sync_channel.sync_id != sync_id: continue - sync_channels_data.append({ - "sync_title": sync.title, - "channel_id": sync_channel.channel_id, - "status": sync_channel.status or "active", - }) + sync_channels_data.append( + { + "sync_title": sync.title, + "channel_id": sync_channel.channel_id, + "status": sync_channel.status or "active", + } + ) key = f"{sync.title}:{sync_channel.channel_id}" post_metas = DbManager.find_records( schemas.PostMeta, [schemas.PostMeta.sync_channel_id == sync_channel.id], ) - post_meta_by_key[key] = [{"post_id": post_meta.post_id, "ts": float(post_meta.ts)} for post_meta in post_metas] + post_meta_by_key[key] = [ + {"post_id": post_meta.post_id, "ts": float(post_meta.ts)} for post_meta in post_metas + ] # user_directory for W ud_records = DbManager.find_records( @@ -349,37 +362,42 @@ def build_migration_export(workspace_id: int, include_source_instance: bool = Tr ) user_directory_data = [] for u in ud_records: - user_directory_data.append({ - "slack_user_id": u.slack_user_id, - "email": u.email, - "real_name": u.real_name, - "display_name": u.display_name, - "normalized_name": u.normalized_name, - "updated_at": u.updated_at.isoformat() if u.updated_at else None, - }) + user_directory_data.append( + { + "slack_user_id": u.slack_user_id, + "email": u.email, + "real_name": u.real_name, + "display_name": u.display_name, + "normalized_name": u.normalized_name, + "updated_at": u.updated_at.isoformat() if u.updated_at else None, + } + ) # user_mappings involving W (export with team_id for other side) um_records = DbManager.find_records( schemas.UserMapping, [ - (schemas.UserMapping.source_workspace_id == workspace_id) | (schemas.UserMapping.target_workspace_id == workspace_id), + (schemas.UserMapping.source_workspace_id == workspace_id) + | (schemas.UserMapping.target_workspace_id == workspace_id), ], ) user_mappings_data = [] for um in um_records: src_ws = DbManager.get_record(schemas.Workspace, um.source_workspace_id) if um.source_workspace_id else None tgt_ws = DbManager.get_record(schemas.Workspace, um.target_workspace_id) if um.target_workspace_id else None - user_mappings_data.append({ - "source_team_id": src_ws.team_id if src_ws else None, - "target_team_id": tgt_ws.team_id if tgt_ws else None, - "source_user_id": um.source_user_id, - "target_user_id": um.target_user_id, - "match_method": um.match_method, - }) + user_mappings_data.append( + { + "source_team_id": src_ws.team_id if src_ws else None, + "target_team_id": tgt_ws.team_id if tgt_ws else None, + "source_user_id": um.source_user_id, + "target_user_id": um.target_user_id, + "match_method": um.match_method, + } + ) payload = { "version": MIGRATION_VERSION, - "exported_at": datetime.utcnow().isoformat() + "Z", + "exported_at": datetime.now(UTC).isoformat() + "Z", "workspace": {"team_id": team_id, "workspace_name": workspace_name}, "groups": groups_data, "syncs": syncs_data, @@ -391,11 +409,14 @@ def build_migration_export(workspace_id: int, include_source_instance: bool = Tr if include_source_instance: from federation import core as federation + try: url = federation.get_public_url() instance_id = federation.get_instance_id() _, public_key_pem = federation.get_or_create_instance_keypair() - code = federation.generate_federation_code(webhook_url=url, instance_id=instance_id, public_key=public_key_pem) + code = federation.generate_federation_code( + webhook_url=url, instance_id=instance_id, public_key=public_key_pem + ) payload["source_instance"] = { "webhook_url": url, "instance_id": instance_id, @@ -408,7 +429,8 @@ def build_migration_export(workspace_id: int, include_source_instance: bool = Tr # Sign with Ed25519 (exclude signature from signed bytes; include signed_at) try: from federation import core as federation - payload["signed_at"] = datetime.utcnow().isoformat() + "Z" + + payload["signed_at"] = datetime.now(UTC).isoformat() + "Z" to_sign = {k: v for k, v in payload.items() if k != "signature"} raw = canonical_json_dumps(to_sign).decode("utf-8") payload["signature"] = federation.sign_body(raw) @@ -430,6 +452,7 @@ def verify_migration_signature(data: dict) -> bool: to_verify = {k: v for k, v in data.items() if k != "signature"} raw = canonical_json_dumps(to_verify).decode("utf-8") from federation import core as federation + return federation.verify_body(raw, sig, public_key) @@ -445,8 +468,6 @@ def import_migration_data( - Replace mode: soft-delete W's SyncChannels in this group and their PostMeta, then create from export. - team_id_to_workspace_id: map export team_id -> B's workspace id (for publisher/target and user_mappings). """ - from datetime import UTC - syncs_export = data.get("syncs", []) sync_channels_export = data.get("sync_channels", []) post_meta_export = data.get("post_meta", {}) @@ -496,7 +517,11 @@ def import_migration_data( tgt_team = s.get("target_team_id") is_publisher = s.get("is_publisher") pub_ws_id = (workspace_id if is_publisher else team_id_to_workspace_id.get(pub_team)) if pub_team else None - tgt_ws_id = (workspace_id if tgt_team == export_team_id else team_id_to_workspace_id.get(tgt_team)) if tgt_team else None + tgt_ws_id = ( + (workspace_id if tgt_team == export_team_id else team_id_to_workspace_id.get(tgt_team)) + if tgt_team + else None + ) new_sync = schemas.Sync( title=title, group_id=group_id, @@ -525,11 +550,13 @@ def import_migration_data( DbManager.create_record(new_sync_channel) key = f"{sync_title}:{channel_id}" for post_meta in post_meta_export.get(key, []): - DbManager.create_record(schemas.PostMeta( - post_id=post_meta["post_id"], - sync_channel_id=new_sync_channel.id, - ts=Decimal(str(post_meta["ts"])), - )) + DbManager.create_record( + schemas.PostMeta( + post_id=post_meta["post_id"], + sync_channel_id=new_sync_channel.id, + ts=Decimal(str(post_meta["ts"])), + ) + ) # user_directory for W (replace: remove existing for this workspace then insert) DbManager.delete_records( @@ -537,15 +564,19 @@ def import_migration_data( [schemas.UserDirectory.workspace_id == workspace_id], ) for u in user_directory_export: - DbManager.create_record(schemas.UserDirectory( - workspace_id=workspace_id, - slack_user_id=u["slack_user_id"], - email=u.get("email"), - real_name=u.get("real_name"), - display_name=u.get("display_name"), - normalized_name=u.get("normalized_name"), - updated_at=datetime.fromisoformat(u["updated_at"].replace("Z", "+00:00")) if u.get("updated_at") else datetime.now(UTC), - )) + DbManager.create_record( + schemas.UserDirectory( + workspace_id=workspace_id, + slack_user_id=u["slack_user_id"], + email=u.get("email"), + real_name=u.get("real_name"), + display_name=u.get("display_name"), + normalized_name=u.get("normalized_name"), + updated_at=datetime.fromisoformat(u["updated_at"].replace("Z", "+00:00")) + if u.get("updated_at") + else datetime.now(UTC), + ) + ) # user_mappings where both source and target workspace exist on B for um in user_mappings_export: @@ -565,12 +596,14 @@ def import_migration_data( ) if existing: continue - DbManager.create_record(schemas.UserMapping( - source_workspace_id=src_ws_id, - source_user_id=um["source_user_id"], - target_workspace_id=tgt_ws_id, - target_user_id=um.get("target_user_id"), - match_method=um.get("match_method", "none"), - matched_at=datetime.now(UTC), - group_id=group_id, - )) + DbManager.create_record( + schemas.UserMapping( + source_workspace_id=src_ws_id, + source_user_id=um["source_user_id"], + target_workspace_id=tgt_ws_id, + target_user_id=um.get("target_user_id"), + match_method=um.get("match_method", "none"), + matched_at=datetime.now(UTC), + group_id=group_id, + ) + ) diff --git a/syncbot/helpers/files.py b/syncbot/helpers/files.py index 87c8931..7518342 100644 --- a/syncbot/helpers/files.py +++ b/syncbot/helpers/files.py @@ -87,9 +87,7 @@ def download_public_file(url: str, logger: Logger) -> dict | None: return None -def download_slack_files( - files: list[dict], client: WebClient, logger: Logger -) -> list[dict]: +def download_slack_files(files: list[dict], client: WebClient, logger: Logger) -> list[dict]: """Download files from Slack to /tmp for direct re-upload.""" downloaded: list[dict] = [] auth_headers = {"Authorization": f"Bearer {client.token}"} @@ -106,11 +104,13 @@ def download_slack_files( _download_to_file(url, file_path, headers=auth_headers) - downloaded.append({ - "path": file_path, - "name": file_name, - "mimetype": f.get("mimetype", "application/octet-stream"), - }) + downloaded.append( + { + "path": file_path, + "name": file_name, + "mimetype": f.get("mimetype", "application/octet-stream"), + } + ) except Exception as e: logger.error(f"download_slack_files: failed for {f.get('id')}: {e}") return downloaded @@ -130,10 +130,12 @@ def upload_files_to_slack( slack_client = WebClient(bot_token) file_uploads = [] for f in files: - file_uploads.append({ - "file": f["path"], - "filename": f["name"], - }) + file_uploads.append( + { + "file": f["path"], + "filename": f["name"], + } + ) kwargs: dict = {"channel": channel_id} if initial_comment: @@ -158,7 +160,9 @@ def upload_files_to_slack( def _extract_file_message_ts( - client: WebClient, upload_response, channel_id: str, + client: WebClient, + upload_response, + channel_id: str, thread_ts: str | None = None, ) -> str | None: """Extract the message ts created by a file upload.""" @@ -189,8 +193,9 @@ def _extract_file_message_ts( channel_shares = shares.get(share_type, {}).get(channel_id, []) if channel_shares: ts = channel_shares[0].get("ts") - _logger.info("_extract_file_message_ts: success", - extra={"file_id": file_id, "ts": ts, "attempt": attempt}) + _logger.info( + "_extract_file_message_ts: success", extra={"file_id": file_id, "ts": ts, "attempt": attempt} + ) return ts except (KeyError, TypeError, IndexError): pass diff --git a/syncbot/helpers/oauth.py b/syncbot/helpers/oauth.py index 9ecedc3..70ca03f 100644 --- a/syncbot/helpers/oauth.py +++ b/syncbot/helpers/oauth.py @@ -50,11 +50,7 @@ def get_oauth_flow(): ) bot_scopes = [s.strip() for s in scopes_raw.split(",") if s.strip()] - user_scopes = ( - [s.strip() for s in user_scopes_raw.split(",") if s.strip()] - if user_scopes_raw - else list(USER_SCOPES) - ) + user_scopes = [s.strip() for s in user_scopes_raw.split(",") if s.strip()] if user_scopes_raw else list(USER_SCOPES) return OAuthFlow( settings=OAuthSettings( diff --git a/syncbot/helpers/refresh.py b/syncbot/helpers/refresh.py index f500c69..60b46a5 100644 --- a/syncbot/helpers/refresh.py +++ b/syncbot/helpers/refresh.py @@ -16,8 +16,7 @@ def cooldown_message_block(remaining_seconds: int) -> dict: """Return a Block Kit context block dict for the refresh cooldown message.""" text = ( - f"No new data. Wait {remaining_seconds} second{'s' if remaining_seconds != 1 else ''} " - "before refreshing again." + f"No new data. Wait {remaining_seconds} second{'s' if remaining_seconds != 1 else ''} before refreshing again." ) return { "type": "context", diff --git a/syncbot/helpers/user_matching.py b/syncbot/helpers/user_matching.py index e6661a6..da3cace 100644 --- a/syncbot/helpers/user_matching.py +++ b/syncbot/helpers/user_matching.py @@ -516,9 +516,7 @@ def find_synced_channel_in_target(source_channel_id: str, target_workspace_id: i return target_rows[0].channel_id -_ARCHIVE_LINK_PATTERN = re.compile( - r"]+)>" -) +_ARCHIVE_LINK_PATTERN = re.compile(r"]+)>") def _rewrite_slack_archive_links_to_native_channels(msg_text: str, target_workspace_id: int) -> str: diff --git a/syncbot/helpers/workspace.py b/syncbot/helpers/workspace.py index f64a234..13b7dcd 100644 --- a/syncbot/helpers/workspace.py +++ b/syncbot/helpers/workspace.py @@ -263,7 +263,8 @@ def _restore_workspace( ) syncs_in_group = DbManager.find_records( - schemas.Sync, [schemas.Sync.group_id == group_id], + schemas.Sync, + [schemas.Sync.group_id == group_id], ) other_channel_ids = [] for sync in syncs_in_group: diff --git a/syncbot/logger.py b/syncbot/logger.py index 7cd116e..c4bb215 100644 --- a/syncbot/logger.py +++ b/syncbot/logger.py @@ -130,10 +130,10 @@ class DevFormatter(logging.Formatter): _RESERVED = frozenset(logging.LogRecord("", 0, "", 0, "", (), None).__dict__.keys()) _COLORS = { - "DEBUG": "\033[90m", # grey - "INFO": "\033[32m", # green - "WARNING": "\033[33m", # yellow - "ERROR": "\033[31m", # red + "DEBUG": "\033[90m", # grey + "INFO": "\033[32m", # green + "WARNING": "\033[33m", # yellow + "ERROR": "\033[31m", # red "CRITICAL": "\033[1;31m", # bold red } _RESET = "\033[0m" diff --git a/syncbot/slack/deferred_ack_views.py b/syncbot/slack/deferred_ack_views.py index 3ffb144..d647d33 100644 --- a/syncbot/slack/deferred_ack_views.py +++ b/syncbot/slack/deferred_ack_views.py @@ -10,9 +10,11 @@ CONFIG_PUBLISH_MODE_SUBMIT, ) -DEFERRED_ACK_VIEW_CALLBACK_IDS = frozenset({ - CONFIG_PUBLISH_MODE_SUBMIT, - CONFIG_PUBLISH_CHANNEL_SUBMIT, - CONFIG_BACKUP_RESTORE_SUBMIT, - CONFIG_DATA_MIGRATION_SUBMIT, -}) +DEFERRED_ACK_VIEW_CALLBACK_IDS = frozenset( + { + CONFIG_PUBLISH_MODE_SUBMIT, + CONFIG_PUBLISH_CHANNEL_SUBMIT, + CONFIG_BACKUP_RESTORE_SUBMIT, + CONFIG_DATA_MIGRATION_SUBMIT, + } +) diff --git a/tests/test_app_main_response.py b/tests/test_app_main_response.py index 359379b..e39c4df 100644 --- a/tests/test_app_main_response.py +++ b/tests/test_app_main_response.py @@ -1,5 +1,6 @@ """Unit tests for syncbot.app.view_ack and main_response (ack + lazy work).""" +import json import os from unittest.mock import MagicMock, patch @@ -118,3 +119,36 @@ def handler(b, c, log, ctx): app_module.main_response(_body_view_submit(cid), MagicMock(), MagicMock(), ack, context) ack.assert_not_called() + + +class TestLambdaHandler: + """AWS Lambda :func:`~app.handler` branches (migrate, warmup, Slack).""" + + def test_handler_migrate_event_calls_initialize_database(self): + with patch.object(app_module, "initialize_database") as mock_init: + result = app_module.handler({"action": "migrate"}, {}) + mock_init.assert_called_once() + assert result["statusCode"] == 200 + assert json.loads(result["body"]) == {"status": "ok", "action": "migrate"} + + def test_handler_warmup_scheduler_returns_ok(self): + with patch.object(app_module, "SlackRequestHandler") as mock_srh: + result = app_module.handler({"source": "aws.scheduler"}, {}) + mock_srh.assert_not_called() + assert result["statusCode"] == 200 + assert json.loads(result["body"]) == {"status": "ok", "action": "warmup"} + + def test_handler_warmup_events_returns_ok(self): + with patch.object(app_module, "SlackRequestHandler") as mock_srh: + result = app_module.handler({"source": "aws.events"}, {}) + mock_srh.assert_not_called() + assert result["statusCode"] == 200 + assert json.loads(result["body"])["action"] == "warmup" + + def test_handler_slack_event_delegates_to_bolt(self): + mock_handle = MagicMock(return_value={"statusCode": 200, "body": "{}"}) + with patch.object(app_module, "SlackRequestHandler") as mock_srh_class: + mock_srh_class.return_value.handle = mock_handle + app_module.handler({"httpMethod": "POST", "path": "/slack/events", "body": "{}"}, {}) + mock_srh_class.assert_called_once_with(app=app_module.app) + mock_handle.assert_called_once() diff --git a/tests/test_db_setup.py b/tests/test_db_setup.py index 9bf794f..8c94a77 100644 --- a/tests/test_db_setup.py +++ b/tests/test_db_setup.py @@ -92,3 +92,154 @@ def test_handler_calls_postgresql_setup(cfn_create_event): mock_pg.assert_called_once() mock_mysql.assert_not_called() assert mock_send.call_args[0][2] == "SUCCESS" + + +def test_safe_username_accepts_dotted_prefix(): + handler = _fresh_handler() + handler._safe_username("42bvZAUSurKwhxc.sbapp_test") + + +def test_safe_ident_rejects_dots(): + handler = _fresh_handler() + with pytest.raises(ValueError, match="Invalid identifier"): + handler._safe_ident("bad.schema") + + +def test_handler_username_prefix_with_dot(cfn_create_event): + cfn_create_event["ResourceProperties"]["UsernamePrefix"] = "pre." + handler = _fresh_handler() + with ( + patch.object(handler, "send"), + patch.object(handler, "get_secret_value", return_value="apppw"), + patch.object(handler, "_assert_tcp_reachable"), + patch.object(handler, "setup_database_mysql") as mock_mysql, + patch.object(handler, "setup_database_postgresql"), + ): + handler._handler_impl(cfn_create_event, MagicMock()) + assert mock_mysql.call_args[1]["app_username"] == "pre.sbapp_test" + assert mock_mysql.call_args[1]["admin_user"] == "pre.admin" + + +def test_handler_username_prefix_without_dot(cfn_create_event): + cfn_create_event["ResourceProperties"]["UsernamePrefix"] = "pre" + handler = _fresh_handler() + with ( + patch.object(handler, "send"), + patch.object(handler, "get_secret_value", return_value="apppw"), + patch.object(handler, "_assert_tcp_reachable"), + patch.object(handler, "setup_database_mysql") as mock_mysql, + patch.object(handler, "setup_database_postgresql"), + ): + handler._handler_impl(cfn_create_event, MagicMock()) + assert mock_mysql.call_args[1]["app_username"] == "pre.sbapp_test" + assert mock_mysql.call_args[1]["admin_user"] == "pre.admin" + + +def test_handler_username_prefix_applied_to_bare_root_admin(cfn_create_event): + cfn_create_event["ResourceProperties"]["AdminUser"] = "root" + cfn_create_event["ResourceProperties"]["UsernamePrefix"] = "cluster" + handler = _fresh_handler() + with ( + patch.object(handler, "send"), + patch.object(handler, "get_secret_value", return_value="apppw"), + patch.object(handler, "_assert_tcp_reachable"), + patch.object(handler, "setup_database_mysql") as mock_mysql, + patch.object(handler, "setup_database_postgresql"), + ): + handler._handler_impl(cfn_create_event, MagicMock()) + assert mock_mysql.call_args[1]["admin_user"] == "cluster.root" + assert mock_mysql.call_args[1]["app_username"] == "cluster.sbapp_test" + + +def test_handler_app_username_override_bypasses_prefix_and_default(cfn_create_event): + cfn_create_event["ResourceProperties"]["UsernamePrefix"] = "pre" + cfn_create_event["ResourceProperties"]["AppUsername"] = "custom.app_user" + handler = _fresh_handler() + with ( + patch.object(handler, "send"), + patch.object(handler, "get_secret_value", return_value="apppw"), + patch.object(handler, "_assert_tcp_reachable"), + patch.object(handler, "setup_database_mysql") as mock_mysql, + patch.object(handler, "setup_database_postgresql"), + ): + handler._handler_impl(cfn_create_event, MagicMock()) + assert mock_mysql.call_args[1]["app_username"] == "custom.app_user" + assert mock_mysql.call_args[1]["admin_user"] == "pre.admin" + + +def test_handler_custom_port_passed_to_tcp_and_mysql(cfn_create_event): + cfn_create_event["ResourceProperties"]["Port"] = "4000" + handler = _fresh_handler() + with ( + patch.object(handler, "send"), + patch.object(handler, "get_secret_value", return_value="apppw"), + patch.object(handler, "_assert_tcp_reachable") as mock_tcp, + patch.object(handler, "setup_database_mysql") as mock_mysql, + patch.object(handler, "setup_database_postgresql"), + ): + handler._handler_impl(cfn_create_event, MagicMock()) + mock_tcp.assert_called_once_with("db.example.com", 4000) + assert mock_mysql.call_args[1]["port"] == 4000 + + +def test_handler_mysql_create_schema_false(cfn_create_event): + cfn_create_event["ResourceProperties"]["CreateSchema"] = "false" + handler = _fresh_handler() + with ( + patch.object(handler, "send"), + patch.object(handler, "get_secret_value", return_value="apppw"), + patch.object(handler, "_assert_tcp_reachable"), + patch.object(handler, "setup_database_mysql") as mock_mysql, + patch.object(handler, "setup_database_postgresql"), + ): + handler._handler_impl(cfn_create_event, MagicMock()) + assert mock_mysql.call_args[1]["create_schema"] is False + + +def test_handler_put_secret_when_no_app_user(cfn_create_event): + cfn_create_event["ResourceProperties"]["CreateAppUser"] = "false" + handler = _fresh_handler() + with ( + patch.object(handler, "send") as mock_send, + patch.object(handler, "get_secret_value") as mock_get, + patch.object(handler, "_assert_tcp_reachable"), + patch.object(handler, "setup_database_mysql") as mock_mysql, + patch.object(handler, "setup_database_postgresql") as mock_pg, + patch.object(handler, "put_secret_string") as mock_put, + ): + handler._handler_impl(cfn_create_event, MagicMock()) + mock_get.assert_not_called() + mock_mysql.assert_called_once() + assert mock_mysql.call_args[1]["create_app_user"] is False + mock_pg.assert_not_called() + mock_put.assert_called_once_with( + cfn_create_event["ResourceProperties"]["SecretArn"], + "adminpw", + ) + assert mock_send.call_args[0][2] == "SUCCESS" + assert mock_send.call_args[0][3] == {"Username": "admin"} + + +def test_handler_skip_both_no_db_client(cfn_create_event): + cfn_create_event["ResourceProperties"]["CreateAppUser"] = "false" + cfn_create_event["ResourceProperties"]["CreateSchema"] = "false" + handler = _fresh_handler() + with ( + patch.object(handler, "send") as mock_send, + patch.object(handler, "get_secret_value") as mock_get, + patch.object(handler, "_assert_tcp_reachable") as mock_tcp, + patch.object(handler, "setup_database_mysql") as mock_mysql, + patch.object(handler, "setup_database_postgresql") as mock_pg, + patch.object(handler, "put_secret_string") as mock_put, + ): + handler._handler_impl(cfn_create_event, MagicMock()) + mock_get.assert_not_called() + mock_mysql.assert_not_called() + mock_pg.assert_not_called() + mock_tcp.assert_called_once_with("db.example.com", 3306) + mock_put.assert_called_once_with( + cfn_create_event["ResourceProperties"]["SecretArn"], + "adminpw", + ) + assert mock_send.call_args[0][2] == "SUCCESS" + assert mock_send.call_args[0][3] == {"Username": "admin"} diff --git a/tests/test_federation_reactions.py b/tests/test_federation_reactions.py index ea6bfb4..fe8044a 100644 --- a/tests/test_federation_reactions.py +++ b/tests/test_federation_reactions.py @@ -60,7 +60,9 @@ def test_invalid_name_reaction_falls_back_to_thread_text(self): patch.object(federation_api, "_find_post_records", return_value=[post_meta]), patch.object(federation_api.helpers, "decrypt_bot_token", return_value="xoxb-test"), patch.object(federation_api, "WebClient", return_value=ws_client), - patch.object(federation_api.helpers, "post_message", return_value={"ts": "200.000001"}) as post_message_mock, + patch.object( + federation_api.helpers, "post_message", return_value={"ts": "200.000001"} + ) as post_message_mock, ): status, resp = federation_api.handle_message_react(body, fed_ws) @@ -201,7 +203,9 @@ def test_missing_user_fields_use_defaults(self): patch.object(federation_api, "_find_post_records", return_value=[post_meta]), patch.object(federation_api.helpers, "decrypt_bot_token", return_value="xoxb-test"), patch.object(federation_api, "WebClient", return_value=ws_client), - patch.object(federation_api.helpers, "post_message", return_value={"ts": "200.000001"}) as post_message_mock, + patch.object( + federation_api.helpers, "post_message", return_value={"ts": "200.000001"} + ) as post_message_mock, ): status, resp = federation_api.handle_message_react(body, fed_ws) diff --git a/tests/test_groups_handlers.py b/tests/test_groups_handlers.py index e02e289..da25291 100644 --- a/tests/test_groups_handlers.py +++ b/tests/test_groups_handlers.py @@ -35,11 +35,7 @@ def test_invalid_group_code_log_is_sanitized(self): ): handle_join_group_submit(body, client, logger, context={}) - matched = [ - call - for call in warn_log.call_args_list - if call.args and call.args[0] == "group_code_invalid" - ] + matched = [call for call in warn_log.call_args_list if call.args and call.args[0] == "group_code_invalid"] assert matched, "Expected group_code_invalid warning" extra = matched[0].kwargs["extra"] assert "code" not in extra diff --git a/tests/test_helpers.py b/tests/test_helpers.py index b81b937..adce72e 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -325,9 +325,7 @@ def conv_info(channel): client.conversations_info.side_effect = conv_info client.team_info.return_value = {"team": {"domain": "acme"}} ws = self._make_workspace(team_id="T123", name="Acme") - result = helpers.resolve_channel_references( - "see <#CABC111> and <#CABC222>", client, ws - ) + result = helpers.resolve_channel_references("see <#CABC111> and <#CABC222>", client, ws) assert "archives/CABC111" in result assert "archives/CABC222" in result assert "#alpha" in result @@ -344,9 +342,7 @@ def test_native_channel_when_synced_to_target(self, mock_find): mock_find.return_value = "C_LOCAL_TARGET" client = self._make_client(channel_name="general", domain="acme") ws = self._make_workspace(team_id="T123", name="Acme") - result = helpers.resolve_channel_references( - "see <#CSOURCE123>", client, ws, target_workspace_id=42 - ) + result = helpers.resolve_channel_references("see <#CSOURCE123>", client, ws, target_workspace_id=42) assert result == "see <#C_LOCAL_TARGET>" mock_find.assert_called_with("CSOURCE123", 42) assert "slack.com" not in result diff --git a/tests/test_message_event_dedup.py b/tests/test_message_event_dedup.py index 94d5894..45baf2d 100644 --- a/tests/test_message_event_dedup.py +++ b/tests/test_message_event_dedup.py @@ -86,7 +86,10 @@ def test_file_share_subtype_still_calls_new_post(self): with ( patch("handlers.messages._is_own_bot_message", return_value=False), patch("handlers.messages._handle_new_post") as mock_new, - patch("handlers.messages._build_file_context", return_value=([], [], [{"path": "/tmp/x", "name": "x.jpg", "mimetype": "image/jpeg"}])), + patch( + "handlers.messages._build_file_context", + return_value=([], [], [{"path": "/tmp/x", "name": "x.jpg", "mimetype": "image/jpeg"}]), + ), ): respond_to_message_event(body, client, logger, context) diff --git a/tests/test_split_message_reactions.py b/tests/test_split_message_reactions.py index 44a21ff..ea903c4 100644 --- a/tests/test_split_message_reactions.py +++ b/tests/test_split_message_reactions.py @@ -40,7 +40,9 @@ def capture_post_meta(rows): created.extend(rows) with ( - patch("handlers.messages.helpers.get_sync_list", return_value=[(sc_source, ws_source), (sc_target, ws_target)]), + patch( + "handlers.messages.helpers.get_sync_list", return_value=[(sc_source, ws_source), (sc_target, ws_target)] + ), patch("handlers.messages.helpers.get_user_info", return_value=("N", "http://i")), patch("handlers.messages.helpers.get_mapped_target_user_id", return_value=None), patch("handlers.messages.helpers.get_federated_workspace_for_sync", return_value=None),