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..495c5b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,27 @@ 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 + +- Synced message author shows local display name and avatar for mapped users, including federated messages (no workspace suffix) +- 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/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 1bc266d..1148189 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -42,7 +42,7 @@ If a workspace uninstalls SyncBot, group memberships and syncs are paused (not d ## User Mapping -Users are automatically mapped across workspaces by email or display name. Admins can manually edit mappings via the User Mapping screen (scoped per group). Remote users are displayed as "Display Name (Workspace Name)" and sorted by normalized name. In synced messages, a mapped user is mentioned with a normal `@` tag in the receiving workspace; unmapped users appear as a code-style `[@Name (Workspace)]` label. Channel names that point at another synced channel in the same sync group are shown as native `#channel` links in each workspace. +Users are automatically mapped across workspaces by email or display name. Admins can manually edit mappings via the User Mapping screen (scoped per group). On that screen, remote users are listed as "Display Name (Workspace Name)" and sorted by normalized name. In synced messages, a mapped author appears with their **local** display name and profile photo (no workspace suffix in the author line); an unmapped author uses the remote display name and photo, with the source workspace in parentheses. The same applies to messages delivered over **External Connections** (cross-instance federation). In message text, a mapped user is mentioned with a normal `@` tag in the receiving workspace; unmapped users appear as a code-style `[@Name (Workspace)]` label. Channel names that point at another synced channel in the same sync group are shown as native `#channel` links in each workspace. ## Refresh Behavior 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..2c4c060 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, [ @@ -400,9 +403,22 @@ def handle_message(body: dict, fed_ws: schemas.FederatedWorkspace) -> tuple[int, user_name = user.get("display_name", "Remote User") user_avatar = user.get("avatar_url") workspace_name = user.get("workspace_name", "Remote") + remote_label_for_mentions = workspace_name - text = _resolve_mentions_for_federated(text, workspace.id, workspace_name) - ws_client = WebClient(token=helpers.decrypt_bot_token(workspace.bot_token)) + bot_token = helpers.decrypt_bot_token(workspace.bot_token) + ws_client = WebClient(token=bot_token) + + source_user_id = user.get("user_id") + if source_user_id: + mapping = _pick_user_mapping_for_federated_target(source_user_id, workspace.id) + if mapping and mapping.target_user_id: + local_name, local_icon = helpers.get_user_info(ws_client, mapping.target_user_id) + if local_name: + user_name = helpers.normalize_display_name(local_name) + user_avatar = local_icon or user_avatar + workspace_name = None + + text = _resolve_mentions_for_federated(text, workspace.id, remote_label_for_mentions) text = helpers.resolve_channel_references(text, ws_client, None, target_workspace_id=workspace.id) try: @@ -430,7 +446,7 @@ def handle_message(body: dict, fed_ws: schemas.FederatedWorkspace) -> tuple[int, ) res = helpers.post_message( - bot_token=helpers.decrypt_bot_token(workspace.bot_token), + bot_token=bot_token, channel_id=channel_id, msg_text=text, user_name=user_name, @@ -562,6 +578,17 @@ def handle_message_react(body: dict, fed_ws: schemas.FederatedWorkspace) -> tupl applied = 0 bot_token = helpers.decrypt_bot_token(workspace.bot_token) ws_client = WebClient(token=bot_token) + + source_user_id = body.get("user_id") + if source_user_id: + mapping = _pick_user_mapping_for_federated_target(source_user_id, workspace.id) + if mapping and mapping.target_user_id: + local_name, local_icon = helpers.get_user_info(ws_client, mapping.target_user_id) + if local_name: + user_name = helpers.normalize_display_name(local_name) + user_avatar_url = local_icon or user_avatar_url + workspace_name = None + for post_meta in post_records: try: if action == "add": diff --git a/syncbot/federation/core.py b/syncbot/federation/core.py index 99800f4..8b96ae3 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: @@ -604,18 +601,22 @@ def build_message_payload( thread_post_id: str | None = None, images: list[dict] | None = None, timestamp: str | None = None, + user_id: str | None = None, ) -> dict: """Build a standardised federation message payload.""" + user_obj: dict = { + "display_name": user_name, + "avatar_url": user_avatar_url, + "workspace_name": workspace_name, + } + if user_id: + user_obj["user_id"] = user_id return { "type": msg_type, "sync_id": sync_id, "post_id": post_id, "channel_id": channel_id, - "user": { - "display_name": user_name, - "avatar_url": user_avatar_url, - "workspace_name": workspace_name, - }, + "user": user_obj, "text": text, "thread_post_id": thread_post_id, "images": images or [], @@ -665,9 +666,10 @@ def build_reaction_payload( user_avatar_url: str | None = None, workspace_name: str | None = None, timestamp: str, + user_id: str | None = None, ) -> dict: """Build a federation reaction payload.""" - return { + payload: dict = { "type": "react", "post_id": post_id, "channel_id": channel_id, @@ -678,3 +680,6 @@ def build_reaction_payload( "workspace_name": workspace_name, "timestamp": timestamp, } + if user_id: + payload["user_id"] = user_id + return payload 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..017f1e0 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__) @@ -229,6 +230,7 @@ def _handle_new_post( text=fed_adapted_text, images=image_payloads, timestamp=helpers.safe_get(body, "event", "ts"), + user_id=user_id, ) result = federation.push_message(fed_ws, payload) ts = helpers.safe_get(result, "ts") if result else helpers.safe_get(body, "event", "ts") @@ -250,13 +252,15 @@ def _handle_new_post( adapted_text, client, source_ws, target_workspace_id=workspace.id ) - target_display_name, target_icon_url = helpers.get_display_name_and_icon_for_synced_message( - user_id or "", - source_workspace_id or 0, - user_name, - user_profile_url, - target_client, - workspace.id, + target_display_name, target_icon_url, author_is_mapped = ( + helpers.get_display_name_and_icon_for_synced_message( + user_id or "", + source_workspace_id or 0, + user_name, + user_profile_url, + target_client, + workspace.id, + ) ) name_for_target = target_display_name or user_name or "Someone" @@ -284,7 +288,7 @@ def _handle_new_post( msg_text=adapted_text, user_name=name_for_target, user_profile_url=target_icon_url or user_profile_url, - workspace_name=workspace_name, + workspace_name=None if author_is_mapped else workspace_name, blocks=photo_blocks, ) ts = helpers.safe_get(res, "ts") or helpers.safe_get(body, "event", "ts") @@ -388,6 +392,7 @@ def _handle_thread_reply( text=fed_adapted_text, thread_post_id=str(thread_post_id) if thread_post_id else None, timestamp=helpers.safe_get(body, "event", "ts"), + user_id=user_id, ) result = federation.push_message(fed_ws, payload) ts = helpers.safe_get(result, "ts") if result else helpers.safe_get(body, "event", "ts") @@ -410,13 +415,15 @@ def _handle_thread_reply( ) parent_ts = f"{post_meta.ts:.6f}" - target_display_name, target_icon_url = helpers.get_display_name_and_icon_for_synced_message( - user_id or "", - source_workspace_id or 0, - user_name, - user_profile_url, - target_client, - workspace.id, + target_display_name, target_icon_url, author_is_mapped = ( + helpers.get_display_name_and_icon_for_synced_message( + user_id or "", + source_workspace_id or 0, + user_name, + user_profile_url, + target_client, + workspace.id, + ) ) name_for_target = target_display_name or user_name or "Someone" @@ -446,7 +453,7 @@ def _handle_thread_reply( user_name=name_for_target, user_profile_url=target_icon_url or user_profile_url, thread_ts=parent_ts, - workspace_name=workspace_name, + workspace_name=None if author_is_mapped else workspace_name, blocks=photo_blocks, ) ts = helpers.safe_get(res, "ts") @@ -681,21 +688,25 @@ def _handle_reaction( user_avatar_url=user_profile_url, workspace_name=ws_name, timestamp=f"{post_meta.ts:.6f}", + user_id=user_id, ) federation.push_reaction(fed_ws, payload) else: target_client = WebClient(token=helpers.decrypt_bot_token(workspace.bot_token)) target_msg_ts = f"{post_meta.ts:.6f}" - target_display_name, target_icon_url = helpers.get_display_name_and_icon_for_synced_message( - user_id or "", - source_workspace_id or 0, - user_name, - user_profile_url, - target_client, - workspace.id, + target_display_name, target_icon_url, author_is_mapped = ( + helpers.get_display_name_and_icon_for_synced_message( + user_id or "", + source_workspace_id or 0, + user_name, + user_profile_url, + target_client, + workspace.id, + ) ) display_name = target_display_name or user_name or user_id or "Someone" + reaction_username_suffix = "" if author_is_mapped else posted_from permalink = None try: @@ -720,7 +731,7 @@ def _handle_reaction( resp = target_client.chat_postMessage( channel=sync_channel.channel_id, text=msg_text, - username=f"{display_name} {posted_from}", + username=f"{display_name} {reaction_username_suffix}".strip(), icon_url=target_icon_url or user_profile_url, thread_ts=target_msg_ts, unfurl_links=False, 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/slack_api.py b/syncbot/helpers/slack_api.py index 072e5df..84fd193 100644 --- a/syncbot/helpers/slack_api.py +++ b/syncbot/helpers/slack_api.py @@ -134,7 +134,7 @@ def post_message( ) -> dict: """Post or update a message in a Slack channel.""" slack_client = WebClient(bot_token) - posted_from = f"({workspace_name})" if workspace_name else "(via SyncBot)" + posted_from = f"({workspace_name})" if workspace_name else "" if blocks: if msg_text.strip(): msg_block = {"type": "section", "text": {"type": "mrkdwn", "text": msg_text}} @@ -152,10 +152,11 @@ def post_message( blocks=all_blocks, ) else: + username_str = f"{user_name} {posted_from}".strip() if user_name else None res = slack_client.chat_postMessage( channel=channel_id, text=fallback_text, - username=f"{user_name} {posted_from}", + username=username_str, icon_url=user_profile_url, thread_ts=thread_ts, blocks=all_blocks, diff --git a/syncbot/helpers/user_matching.py b/syncbot/helpers/user_matching.py index e6661a6..6af3340 100644 --- a/syncbot/helpers/user_matching.py +++ b/syncbot/helpers/user_matching.py @@ -343,22 +343,22 @@ def get_display_name_and_icon_for_synced_message( source_icon_url: str | None, target_client: WebClient, target_workspace_id: int, -) -> tuple[str | None, str | None]: - """Return (display_name, icon_url) to use when syncing a message into the target workspace. +) -> tuple[str | None, str | None, bool]: + """Return (display_name, icon_url, is_mapped) when syncing into the target workspace. If the source user is mapped to a user in the target workspace, returns that - local user's display name and profile image so the synced message appears - under the name familiar to users in the target workspace. Otherwise - returns the source display name and icon. Display names are normalized - (text in parens or after a dash at the end is dropped); the app then - appends the remote workspace name in parens when posting. + local user's display name and profile image (third element ``True``). Otherwise + returns the source display name and icon (``False``). Display names are + normalized (text in parens or after a dash at the end is dropped). Callers + omit the remote workspace suffix in the posted username when ``is_mapped`` + is true. """ mapped_id = get_mapped_target_user_id(source_user_id, source_workspace_id, target_workspace_id) if mapped_id: local_name, local_icon = get_user_info(target_client, mapped_id) if local_name: - return normalize_display_name(local_name), local_icon or source_icon_url - return normalize_display_name(source_display_name), source_icon_url + return normalize_display_name(local_name), local_icon or source_icon_url, True + return normalize_display_name(source_display_name), source_icon_url, False def resolve_mention_for_workspace( @@ -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..89ca2db 100644 --- a/tests/test_federation_reactions.py +++ b/tests/test_federation_reactions.py @@ -30,6 +30,65 @@ def test_build_reaction_payload_includes_user_fields(self): assert payload["user_avatar_url"] == "https://avatar.example/alice.png" assert payload["workspace_name"] == "Workspace A" assert payload["timestamp"] == "100.000001" + assert "user_id" not in payload + + def test_build_reaction_payload_includes_user_id_when_set(self): + payload = federation_core.build_reaction_payload( + post_id="post-1", + channel_id="C123", + reaction="thumbsup", + action="add", + user_name="Alice", + timestamp="1.0", + user_id="U_REMOTE", + ) + assert payload["user_id"] == "U_REMOTE" + + +class TestFederationMessageInbound: + def test_mapped_author_suppresses_workspace_suffix(self): + body = { + "channel_id": "C123", + "text": "hi", + "post_id": "", + "user": { + "display_name": "Alice Remote", + "avatar_url": "https://remote.example/a.png", + "workspace_name": "Partner WS", + "user_id": "U_REMOTE", + }, + } + fed_ws = SimpleNamespace(instance_id="remote-instance") + sync_channel = SimpleNamespace(id=101, channel_id="C123") + workspace = SimpleNamespace(id=55, bot_token="enc-token") + mapping = SimpleNamespace(target_user_id="ULOCAL") + + with ( + patch.object(federation_api, "_resolve_channel_for_federated", return_value=(sync_channel, workspace)), + patch.object(federation_api, "_pick_user_mapping_for_federated_target", return_value=mapping), + patch.object(federation_api.helpers, "decrypt_bot_token", return_value="xoxb-test"), + patch.object(federation_api, "WebClient", MagicMock()), + patch.object(federation_api.helpers, "get_user_info", return_value=("Local Nacho", "https://local.example/n.png")), + patch.object(federation_api, "_resolve_mentions_for_federated", side_effect=lambda t, *_: t), + patch.object(federation_api.helpers, "resolve_channel_references", side_effect=lambda t, *a, **k: t), + patch.object( + federation_api.helpers, "post_message", return_value={"ts": "99.000001"} + ) as post_message_mock, + ): + status, resp = federation_api.handle_message(body, fed_ws) + + assert status == 200 + assert resp["ok"] is True + post_message_mock.assert_called_once_with( + bot_token="xoxb-test", + channel_id="C123", + msg_text="hi", + user_name="Local Nacho", + user_profile_url="https://local.example/n.png", + workspace_name=None, + blocks=None, + thread_ts=None, + ) class TestFederationReactionFallback: @@ -60,7 +119,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 +262,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) @@ -216,3 +279,52 @@ def test_missing_user_fields_use_defaults(self): workspace_name="Remote", thread_ts="123.456", ) + + def test_invalid_name_fallback_mapped_user_suppresses_workspace_suffix(self): + body = { + "post_id": "post-1", + "channel_id": "C123", + "reaction": "missing_custom", + "action": "add", + "user_name": "Alice Remote", + "user_avatar_url": "https://remote.example/a.png", + "workspace_name": "Partner WS", + "user_id": "U_REMOTE", + } + fed_ws = SimpleNamespace(instance_id="remote-instance") + sync_channel = SimpleNamespace(id=101, channel_id="C123") + workspace = SimpleNamespace(id=55, bot_token="enc-token") + post_meta = SimpleNamespace(ts=123.456) + mapping = SimpleNamespace(target_user_id="ULOCAL") + + slack_response = MagicMock() + slack_response.get.return_value = "invalid_name" + slack_exc = SlackApiError(message="emoji not found", response=slack_response) + + ws_client = MagicMock() + ws_client.reactions_add.side_effect = slack_exc + + with ( + patch.object(federation_api, "_resolve_channel_for_federated", return_value=(sync_channel, workspace)), + patch.object(federation_api, "_find_post_records", return_value=[post_meta]), + patch.object(federation_api, "_pick_user_mapping_for_federated_target", return_value=mapping), + 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, "get_user_info", return_value=("Local Nacho", "https://local.example/n.png")), + 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) + + assert status == 200 + assert resp["applied"] == 1 + post_message_mock.assert_called_once_with( + bot_token="xoxb-test", + channel_id="C123", + msg_text="reacted with :missing_custom:", + user_name="Local Nacho", + user_profile_url="https://local.example/n.png", + workspace_name=None, + thread_ts="123.456", + ) 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..1d52386 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), @@ -50,7 +52,7 @@ def capture_post_meta(rows): patch("handlers.messages.helpers.get_workspace_by_id", return_value=None), patch( "handlers.messages.helpers.get_display_name_and_icon_for_synced_message", - return_value=("N", None), + return_value=("N", None, False), ), patch("handlers.messages.helpers.post_message", return_value={"ts": "200.000000"}), patch("handlers.messages.helpers.upload_files_to_slack", return_value=(None, "300.000000")), @@ -102,7 +104,7 @@ def test_thread_reply_text_plus_file_stores_file_ts_same_post_id(self): patch("handlers.messages.helpers.get_workspace_by_id", return_value=None), patch( "handlers.messages.helpers.get_display_name_and_icon_for_synced_message", - return_value=("N", None), + return_value=("N", None, False), ), patch("handlers.messages.helpers.post_message", return_value={"ts": "250.000000"}), patch("handlers.messages.helpers.upload_files_to_slack", return_value=(None, "350.000000")),