From d42ec574b6c69f79f889279c69f615f649ed4463 Mon Sep 17 00:00:00 2001 From: "Dr. Ernie Prabhakar" Date: Thu, 2 Jul 2026 14:13:08 -0700 Subject: [PATCH 1/7] docs: add issue #392 configurability spec docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 01-config-issue.md — scope and goals for configurability - 02-bw-configurability.md — current configuration mechanisms - 03-benchling-config-sources.md — Benchling-native config via App Configuration Items + per-project routing Co-Authored-By: Claude --- spec/392-configurability/01-config-issue.md | 60 +++ .../02-bw-configurability.md | 358 +++++++++++++ .../03-benchling-config-sources.md | 495 ++++++++++++++++++ 3 files changed, 913 insertions(+) create mode 100644 spec/392-configurability/01-config-issue.md create mode 100644 spec/392-configurability/02-bw-configurability.md create mode 100644 spec/392-configurability/03-benchling-config-sources.md diff --git a/spec/392-configurability/01-config-issue.md b/spec/392-configurability/01-config-issue.md new file mode 100644 index 00000000..110ded16 --- /dev/null +++ b/spec/392-configurability/01-config-issue.md @@ -0,0 +1,60 @@ +# Issue 392: Configurability + +## Overview + +**Goal:** Make the Benchling webhook configuration more flexible to support diverse deployment patterns without hardcoded assumptions. + +**Key themes:** +1. Configurable workflows — allow operators to define which workflows trigger which webhook actions +2. Multiple buckets — support routing events to different S3 buckets for different experiments +3. No auto-creation — stop implicitly creating resources (buckets, workgroups, etc.) on deploy; require explicit setup + +## Motivation + +### Single-workflow, single-bucket assumption + +The current stack presumes a single Benchling webhook workflow and a single S3 target bucket. This limits the system to one pipeline per deployment. Customers who need to: + +- Route different entry types (e.g. assays vs. experiments) to different destinations +- Stage data in separate buckets per project or team +- Trigger distinct post-processing logic per workflow type + +...must deploy separate stacks or work around the constraint with post-hoc routing. + +### Implicit resource creation + +The CDK stack auto-creates buckets and workgroups when they are referenced but don't exist. This is convenient for first-time setup but dangerous for production: + +- A typo in a bucket name creates an unintended resource +- Deploying with a staging prefix that shouldn't exist silently provisions infrastructure +- Cleanup / teardown is harder when resources were auto-created rather than explicitly managed + +### Workflow coupling + +The webhook event handler is tightly coupled to a single pipeline: receive event → write to designated prefix → done. There is no branching logic based on the event type, workflow ID, or entry schema. Making the workflow configurable means the handler can dispatch to different processing paths based on rules. + +## What Success Looks Like + +1. **Configurable workflows** — a mapping from event conditions (workflow type, entry schema, etc.) to actions (write to bucket A, invoke Lambda B, etc.) defined in the profile config +2. **Multi-bucket support** — the webhook can write to different S3 buckets depending on the event, not just a single `packages.bucket` +3. **No auto-creation** — the deploy step fails early if referenced resources don't exist, forcing the operator to provision them explicitly + +## Open Questions + +- What is the configuration format for workflow rules? (JSON schema, YAML, HCL?) +- How do we validate workflow rules at deploy time vs. at runtime? +- Should multi-bucket support be implemented at the CDK level (multiple `Bucket` constructs) or purely at the application layer (conditional routing in Python)? +- For "no auto-creation", do we need pre-deploy validation hooks, or is a `cdk diff` / dry-run sufficient? + +## Related + +- [#317: Streamline Config](spec/317-streamline-config/) — previous work on decoupling `ProfileConfig` from the CDK stack; introduced `StackConfig` interface +- [#206: Service Envars](spec/206-service-envars/) — service environment variable configuration +- [a11: Async SQS](spec/a11-async-sqs.md) — recent async event processing refactor + +## Next Steps + +- [ ] Define the configurable workflow rule schema +- [ ] Implement multi-bucket routing in the event handler +- [ ] Add pre-deploy validation for resource existence +- [ ] Document the configuration format for operators diff --git a/spec/392-configurability/02-bw-configurability.md b/spec/392-configurability/02-bw-configurability.md new file mode 100644 index 00000000..da97bd44 --- /dev/null +++ b/spec/392-configurability/02-bw-configurability.md @@ -0,0 +1,358 @@ +# Current Configuration Mechanisms + +This document catalogs every configuration mechanism in the Benchling webhook system today — what stores the data, what reads it, and how values flow from one layer to the next. + +--- + +## Overview: Three Layers, Three Formats + +Configuration lives at three distinct layers, each with its own format and lifecycle: + +| Layer | Storage | Format | Created by | Read by | +|-------|---------|--------|------------|---------| +| **1. ProfileConfig** | `~/.config/benchling-webhook/{profile}/config.json` | JSON, `ProfileConfig` interface | Setup wizard (`bin/commands/setup-wizard.ts`) | Deploy script, sync-secrets, CDK synth | +| **2. Secrets Manager** | AWS Secrets Manager secret | JSON, `BenchlingSecretData` fields | `sync-secrets.ts` | Python app at runtime | +| **3. Environment variables** | ECS task definition / docker-compose | String key-value pairs | `FargateService` construct (CDK) | Python `Config.__post_init__()` | + +The flow is one-directional at deploy time: **ProfileConfig → StackConfig → CloudFormation → ECS env vars → Python Config**. At runtime, only Secrets Manager is queried dynamically. + +--- + +## Layer 1: XDG Profile Configuration (`ProfileConfig`) + +### Location + +``` +~/.config/benchling-webhook/{profile}/config.json +``` + +Managed by [`XDGConfig`](../../lib/xdg-config.ts) (TypeScript). The Python side has a read-only mirror in [`xdg_config.py`](../../docker/src/xdg_config.py) but it is *not* used at runtime — it only exists for CLI tooling compatibility. + +### Schema + +Defined as `ProfileConfig` in [`lib/types/config.ts`](../../lib/types/config.ts) (lines 79–134): + +```typescript +interface ProfileConfig { + quilt: QuiltConfig; // catalog, database, queueUrl, region, bucketWritePolicyArn… + benchling: BenchlingConfig; // tenant, clientId, clientSecret?, secretArn?, appDefinitionId + packages: PackageConfig; // bucket, prefix, metadataKey, workflow? + deployment: DeploymentConfig; // region, account?, ecrRepository?, imageTag?, vpc? + integratedStack?: boolean; + logging?: LoggingConfig; // level: "DEBUG"|"INFO"|"WARNING"|"ERROR" + security?: SecurityConfig; // webhookAllowList, enableVerification + _metadata: ConfigMetadata; // version, createdAt, updatedAt, source + _inherits?: string; // profile inheritance +} +``` + +Key points: +- **16+ nested fields**, many optional +- Contains wizard metadata (`_metadata`, `_inherits`) that the CDK stack never reads +- The `packages` sub-object duplicates fields later stored in Secrets Manager (`bucket`, `prefix`, `metadataKey`, `workflow`) +- Written **only** by the setup wizard; operators can hand-edit + +### Consumers + +1. **`bin/commands/deploy.ts`** — reads ProfileConfig, transforms to StackConfig, synthesizes CDK app +2. **`bin/commands/sync-secrets.ts`** — reads ProfileConfig to build the Secrets Manager payload +3. **`bin/cli.ts`** (setup/destroy commands) — reads ProfileConfig for parameter defaults + +--- + +## Layer 2: AWS Secrets Manager + +### What's Stored + +A single JSON secret per profile, structured as `BenchlingSecretData` ([`docker/src/secrets_manager.py`](../../docker/src/secrets_manager.py), lines 48–88): + +```python +@dataclass +class BenchlingSecretData: + tenant: str # Benchling subdomain + client_id: str # OAuth client ID + client_secret: str # OAuth client secret (sensitive) + app_definition_id: str # App definition for webhook verification + pkg_prefix: str # Quilt package name prefix + pkg_key: str # Metadata key (e.g. "experiment_id") + user_bucket: str # S3 bucket for Benchling exports + log_level: str # "DEBUG"|"INFO"|"WARNING"|"ERROR" + enable_webhook_verification: bool + workflow: str = "" # Optional Quilt workflow name + queue_url: str = "" # Optional (v0.8.0+ uses env var instead) +``` + +Note: `workflow` is an *optional* field in the secret — this is the *only* existing mechanism for specifying a Quilt workflow, which relates to the "configurable workflows" theme of issue #392. + +### How It's Created + +- **`bin/commands/sync-secrets.ts`** (`buildSecretValue()`, line 292) assembles the JSON from `ProfileConfig` fields: + - `config.benchling.tenant` → `tenant` + - `config.benchling.clientId` → `client_id` + - `config.packages.bucket` → `user_bucket` + - `config.packages.prefix` → `pkg_prefix` + - `config.packages.metadataKey` → `pkg_key` + - `config.packages.workflow` → `workflow` (only if present) + - `config.logging.level` → `log_level` + - `config.security.webhookAllowList` → `webhook_allow_list` + - `config.security.enableVerification` → `enable_webhook_verification` + +### How It's Read at Runtime + +- **`docker/src/config.py`** — `Config.__post_init__()` stores the secret name from env var `BenchlingSecret`, then `get_benchling_secrets()` fetches from Secrets Manager with a 60-second TTL cache and background refresh +- **`docker/src/secrets_manager.py`** — `fetch_benchling_secret()` does the actual `get_secret_value` call, validating all required fields and parsing boolean/log-level values + +### Deployment-time Verification + +The deploy script ([`bin/commands/deploy.ts`](../../bin/commands/deploy.ts), line 359) calls `syncSecretsToAWS()` with `force: false` to verify the secret exists without overwriting it. The CDK stack never creates secrets — it only references the ARN via CloudFormation parameter. + +### Secret Lifecycle + +| Action | When | How | +|--------|------|-----| +| **Created** | During `setup` or `deploy` | `sync-secrets.ts:createSecret()` | +| **Updated** | `deploy --force` or manual `sync-secrets --force` | `sync-secrets.ts:updateSecret()` | +| **Read (runtime)** | Every request (with 60s TTL cache) | `secrets_manager.py:fetch_benchling_secret()` | +| **Rotated** | Update secret → ECS service restart | `restartECSServicesUsingSecret()` detects affected services | + +--- + +## Layer 3: Environment Variables (Runtime Config) + +### How They're Set + +The CDK `FargateService` construct ([`lib/fargate-service.ts`](../../lib/fargate-service.ts), lines 278–303) builds an environment variable map for the ECS task definition: + +```typescript +const environmentVars = { + AWS_REGION: region, + AWS_DEFAULT_REGION: region, + PORT: "8080", + + // Quilt services (resolved at deploy time) + QUILT_WEB_HOST: props.quiltWebHost, // from config.quilt.catalog + ATHENA_USER_DATABASE: props.athenaUserDatabase, // from config.quilt.database + ATHENA_USER_WORKGROUP: props.athenaUserWorkgroup || "primary", + PACKAGER_SQS_URL: props.packagerQueueUrl, // from config.quilt.queueUrl + + // Benchling secret reference + BenchlingSecret: extractSecretName(props.benchlingSecret), // secret name, not ARN + + // Security + ENABLE_WEBHOOK_VERIFICATION: "true", // hardcoded default + + // Application + APP_ENV: "production", + LOG_LEVEL: props.logLevel || "INFO", +}; +``` + +Additional SQS queue URLs are added when the queues exist: +- `PACKAGE_EVENT_QUEUE_URL` +- `PACKAGING_REQUEST_QUEUE_URL` + +### How They're Read + +In the Python FastAPI app, `Config.__post_init__()` ([`docker/src/config.py`](../../docker/src/config.py), lines 92–156) reads these env vars: + +| Env var | Config field | Required | Notes | +|---------|-------------|----------|-------| +| `QUILT_WEB_HOST` | `quilt_catalog` | Yes | Quilt catalog domain | +| `ATHENA_USER_DATABASE` | `quilt_database` | Yes | Glue/Athena database | +| `PACKAGER_SQS_URL` | `queue_url` | Yes | SQS queue for packages | +| `AWS_REGION` | `aws_region` | Yes | AWS region | +| `BenchlingSecret` | `_benchling_secret_name` | Yes | Secrets Manager secret name | +| `LOG_LEVEL` | `log_level` | No | Default: "INFO" | +| `APP_ENV` | `app_env` | No | Default: "production" | +| `ENABLE_WEBHOOK_VERIFICATION` | `enable_webhook_verification` | No | Default: true | +| `ATHENA_USER_WORKGROUP` | `athena_user_workgroup` | No | Default: "primary" | +| `QUILT_WRITE_ROLE_ARN` | `quilt_write_role_arn` | No | Cross-account S3 access | +| `PACKAGE_EVENT_QUEUE_URL` | (used directly) | No | Package event SQS | +| `PACKAGING_REQUEST_QUEUE_URL` | (used directly) | No | Packaging request SQS | + +### Dual-Source Configuration + +The runtime `Config` class uses a **hybrid** approach: + +1. **AWS/Quilt configuration** comes from environment variables (set by CDK at deploy time) +2. **Package/Benchling configuration** comes from Secrets Manager (fetched on-demand at runtime with 60s TTL cache) + +This means changes to `pkg_prefix`, `pkg_key`, `user_bucket`, `workflow`, `log_level`, or `enable_webhook_verification` can be made by updating the Secrets Manager secret without redeploying the stack (the ECS service must be restarted to pick up the change via `restartECSServicesUsingSecret()`). + +--- + +## Layer 4: CDK Stack Configuration (`StackConfig`) + +The `StackConfig` interface ([`lib/types/stack-config.ts`](../../lib/types/stack-config.ts)) is the **minimal subset** of `ProfileConfig` that the CDK stack actually needs: + +```typescript +interface StackConfig { + benchling: { secretArn: string }; + quilt: { + catalog: string; + database: string; + queueUrl: string; + region: string; + bucketWritePolicyArn?: string; + athenaUserPolicyArn?: string; + }; + deployment: { + region: string; + imageTag?: string; + vpc?: VpcConfig; + stackName?: string; + }; + security?: { + webhookAllowList?: string; + }; +} +``` + +Transformation happens in `profileToStackConfig()` ([`lib/utils/config-transform.ts`](../../lib/utils/config-transform.ts)), called from `deploy.ts` line 768. + +The stack itself creates **CloudFormation parameters** with StackConfig values as defaults (see `benchling-webhook-stack.ts` lines 84–148). This allows CloudFormation `--parameters` overrides at deploy time — those override values are what actually reach the container at runtime. + +### CDK Parameter → Env Var Flow + +``` +StackConfig → CfnParameter (default) → deploy.ts --parameters override + → FargateService environmentVars → ECS task definition + → Python Config.__post_init__() +``` + +At runtime there are **no CloudFormation API calls**. All service endpoints are resolved at deploy time and baked into environment variables. + +--- + +## Layer 5: Local Development Configuration + +### docker-compose.yml + +The [`docker/docker-compose.yml`](../../docker/docker-compose.yml) mirrors the production env var set but sources values from the shell environment (via `${VAR}` interpolation): + +```yaml +environment: + - APP_ENV=production + - LOG_LEVEL=${LOG_LEVEL:-INFO} + - AWS_REGION=${AWS_REGION:-us-east-2} + - QUILT_WEB_HOST=${QUILT_WEB_HOST} + - ATHENA_USER_DATABASE=${ATHENA_USER_DATABASE} + - PACKAGER_SQS_URL=${PACKAGER_SQS_URL} + - BenchlingSecret=${BenchlingSecret} + - ENABLE_WEBHOOK_VERIFICATION=${ENABLE_WEBHOOK_VERIFICATION:-true} +``` + +The `app-dev` service (profile: `dev`) defaults to `LOG_LEVEL=DEBUG` and `ENABLE_WEBHOOK_VERIFICATION=false`. + +The test harness (`npm run test:local`) sets these env vars from the local XDG config. + +### XDG CLI Tools (Python) + +The Python `xdg_config.py` module provides read-only access to XDG config files but is **not used at runtime** — it exists for CLI utilities `xdg_cli.py` and tests. + +--- + +## Configuration Flow Summary + +``` +┌─────────────────────────────────────────────────────────────┐ +│ Setup Wizard │ +│ bin/commands/setup-wizard.ts │ +│ ┌──────────────────────────────────────────────────────┐ │ +│ │ 1. Discover Quilt stack (CloudFormation) │ │ +│ │ 2. Prompt for Benchling credentials │ │ +│ │ 3. Validate against Benchling API │ │ +│ │ 4. Discover VPC resources (EC2) │ │ +│ │ 5. Store ProfileConfig → ~/.config/.../{profile}/ │ │ +│ │ 6. Sync secret → AWS Secrets Manager │ │ +│ └──────────────────────────────────────────────────────┘ │ +└──────────────────────────┬──────────────────────────────────┘ + │ ProfileConfig (XDG JSON file) + ▼ +┌─────────────────────────────────────────────────────────────┐ +│ Deploy Script │ +│ bin/commands/deploy.ts │ +│ ┌──────────────────────────────────────────────────────┐ │ +│ │ 1. Read ProfileConfig from XDG │ │ +│ │ 2. Verify/manage Secrets Manager secret │ │ +│ │ 3. Transform ProfileConfig → StackConfig │ │ +│ │ 4. Call createStack() (CDK synth, no subprocess) │ │ +│ │ 5. npx cdk deploy with --parameters │ │ +│ │ 6. Record deployment in XDG deployments.json │ │ +│ └──────────────────────────────────────────────────────┘ │ +└──────────────────────────┬──────────────────────────────────┘ + │ CloudFormation Stack + ▼ +┌─────────────────────────────────────────────────────────────┐ +│ CDK Stack (synth time) │ +│ bin/benchling-webhook.ts → lib/benchling-webhook-stack.ts │ +│ ┌──────────────────────────────────────────────────────┐ │ +│ │ StackConfig → CfnParameters → FargateService │ │ +│ │ → Environment variables → ECS task definition │ │ +│ └──────────────────────────────────────────────────────┘ │ +└──────────────────────────┬──────────────────────────────────┘ + │ ECS container environment + ▼ +┌─────────────────────────────────────────────────────────────┐ +│ Python Runtime │ +│ docker/src/config.py │ +│ ┌──────────────────────────────────────────────────────┐ │ +│ │ Config.__post_init__(): │ │ +│ │ • Read env vars (QUILT_WEB_HOST, etc.) │ │ +│ │ • Store secret name from BenchlingSecret env var │ │ +│ │ • Initialize Secrets Manager client │ │ +│ │ │ │ +│ │ get_benchling_secrets(): │ │ +│ │ • Fetch from Secrets Manager (60s TTL cache) │ │ +│ │ • Return tenant, client_id, user_bucket, etc. │ │ +│ └──────────────────────────────────────────────────────┘ │ +└─────────────────────────────────────────────────────────────┘ +``` + +--- + +## Key Constraints for Issue #392 + +### 1. Secrets Manager is the *only* dynamic config + +At runtime, the only configuration value that changes without redeploy is the Secrets Manager secret. Everything else (env vars) is baked into the ECS task definition at deploy time. If we want "configurable workflows" that operators can change without redeploying, the workflow configuration must live in the Secrets Manager secret or be fetched from another source at runtime. + +### 2. ProfileConfig is the single source of truth at deploy time + +The XDG config file drives the entire deploy pipeline. Any new configurability that affects infrastructure (e.g., multiple S3 buckets that need IAM permissions) must be represented in `ProfileConfig` (and by extension `StackConfig`) so the CDK stack can provision the right resources. + +### 3. Package configuration is bifurcated + +- `packages.bucket` — appears in `ProfileConfig` (used by deploy for IAM) and also in `secrets_manager.py` as `user_bucket` (used at runtime for writes) +- `packages.prefix` — in `ProfileConfig` and in Secrets Manager as `pkg_prefix` +- `packages.metadataKey` — in `ProfileConfig` and in Secrets Manager as `pkg_key` +- `packages.workflow` — only in Secrets Manager (not in `ProfileConfig`/`StackConfig`) + +This duplication means changes must be coordinated. The `workflow` field especially is invisible to the CDK layer. + +### 4. Local dev reproduces the env var contract + +`docker-compose.yml` sources env vars from the shell, which means local development reproduces the exact same env var contract that production ECS tasks see. Any new env var for configurability must be added to both `fargate-service.ts` and `docker-compose.yml`. + +--- + +## Related Files + +| File | Role | +|------|------| +| [`lib/types/config.ts`](../../lib/types/config.ts) | `ProfileConfig` interface + JSON Schema | +| [`lib/types/stack-config.ts`](../../lib/types/stack-config.ts) | `StackConfig` interface (minimal CDK input) | +| [`lib/utils/config-transform.ts`](../../lib/utils/config-transform.ts) | `ProfileConfig → StackConfig` transformation | +| [`lib/xdg-config.ts`](../../lib/xdg-config.ts) | XDG filesystem read/write (TypeScript) | +| [`lib/xdg-base.ts`](../../lib/xdg-base.ts) | Abstract XDG storage primitives | +| [`lib/benchling-webhook-stack.ts`](../../lib/benchling-webhook-stack.ts) | CDK stack — creates CfnParameters from StackConfig | +| [`lib/fargate-service.ts`](../../lib/fargate-service.ts) | ECS task definition — builds env vars from stack props | +| [`bin/commands/deploy.ts`](../../bin/commands/deploy.ts) | Deploy orchestrator — reads ProfileConfig, calls createStack | +| [`bin/commands/sync-secrets.ts`](../../bin/commands/sync-secrets.ts) | Secrets Manager sync — builds payload from ProfileConfig | +| [`bin/benchling-webhook.ts`](../../bin/benchling-webhook.ts) | `createStack()` library entry point | +| [`docker/src/config.py`](../../docker/src/config.py) | Python runtime Config — reads env vars, fetches secrets | +| [`docker/src/config_provider.py`](../../docker/src/config_provider.py) | Request-scoped config provider with on-demand secret fetch | +| [`docker/src/secrets_manager.py`](../../docker/src/secrets_manager.py) | Secrets Manager client — `BenchlingSecretData` + fetch | +| [`docker/src/config_schema.py`](../../docker/src/config_schema.py) | Pydantic models for config validation | +| [`docker/src/xdg_config.py`](../../docker/src/xdg_config.py) | Python XDG reader (read-only, not used at runtime) | +| [`docker/docker-compose.yml`](../../docker/docker-compose.yml) | Local dev — mirrors production env var contract | diff --git a/spec/392-configurability/03-benchling-config-sources.md b/spec/392-configurability/03-benchling-config-sources.md new file mode 100644 index 00000000..8d2b8679 --- /dev/null +++ b/spec/392-configurability/03-benchling-config-sources.md @@ -0,0 +1,495 @@ +# Benchling as a Configuration Source + +## Question + +Can users store configuration information inside Benchling itself — e.g. in projects, entry fields, or app settings — that the webhook can pull at runtime, rather than (or in addition to) the webhook's own config files and Secrets Manager secrets? + +## Executive Summary + +**Yes, there is one excellent first-party mechanism and several indirect ones.** The strongest candidate is the **App Configuration Items** API — a native key-value store scoped to a Benchling app that is editable through the Benchling UI and readable at runtime via the SDK. This is purpose-built for what issue #392 needs. + +The full landscape of Benchling-hosted data sources: + +| Source | Configurable via UI? | Read via API? | Purpose-built for config? | Recommendation | +|--------|---------------------|---------------|--------------------------|----------------| +| **App Configuration Items** | ✅ App settings page | ✅ `benchling.apps.*` | ✅ Yes | ⭐ **Primary candidate** | +| **Project (via folder)** | ✅ Project list / folder tree | ✅ Entry → Folder → Project API | ✅ Yes — natural organizational unit | ⭐ **Primary for per-project routing** | +| **Entity schemas** | ✅ Schema designer | ✅ `benchling.schemas.*` | ❌ No (schema definitions) | Use to discover entry type for routing | +| **Entry custom fields** | ✅ Entry editor | ✅ Entry API `fields` | ❌ No (data, not config) | Not recommended for routing config | +| **Folders** | ✅ Folder tree | ✅ `benchling.folders.*` | ⚠️ Partial (hierarchy only) | Weak — naming conventions fragile | +| **Dropdowns** | ✅ Dropdown designer | ✅ `benchling.dropdowns.*` | ⚠️ Partial (option lists) | Use for enumeration values | +| **Entry schema name/template** | ✅ Schema designer | ✅ Entry API `schema` | ❌ No | Useful as routing dimension | + +--- + +## 1. ⭐ App Configuration Items (Primary Recommendation) + +### What It Is + +Benchling has a first-party **App Configuration** API (`/app-configuration-items`) that acts as a structured key-value store scoped to a Benchling application. It's designed for exactly this purpose — apps storing runtime configuration that can be read programmatically. + +### SDK Access + +Available through `benchling.apps.*` in the Benchling Python SDK: + +```python +# List all config items for the app +items = benchling.apps.list_app_configuration_items(app_id=our_app_id) + +# Get a specific item by ID +item = benchling.apps.get_app_configuration_item_by_id("cfg_xyz123") + +# Create a config item +from benchling_sdk.models import AppConfigItemJsonCreate, JsonAppConfigItemType +item = benchling.apps.create_app_configuration_item( + AppConfigItemJsonCreate( + path=["workflows", "entry_created", "bucket"], + value="my-experiment-bucket", + description="S3 bucket for entry.created events", + ) +) + +# Bulk operations (async task) +from benchling_sdk.models import AppConfigItemCreate +task = benchling.apps.bulk_create_app_configuration_items([ + AppConfigItemJsonCreate(path=["..."]), + AppConfigItemBooleanCreate(path=["..."], value=True), +]) +``` + +### Supported Value Types + +| Type | SDK Model | Use Case | +|------|-----------|----------| +| **JSON** | `JsonAppConfigItem` | Arbitrary structured config — best for complex routing rules | +| **Text** | `TextAppConfigItem` | Single string values (bucket names, prefixes) | +| **Secure Text** | `SecureTextAppConfigItem` | Encrypted values (API keys, passwords) | +| **Boolean** | `BooleanAppConfigItem` | Feature flags, toggles | +| **Integer** | `IntegerAppConfigItem` | Numeric limits, counts | +| **Float** | `FloatAppConfigItem` | Decimal values | +| **Date** / **Datetime** | `DateAppConfigItem` / `DatetimeAppConfigItem` | Scheduling, timestamps | +| **Entity Schema** | `EntitySchemaAppConfigItem` | Reference to an entity schema | +| **Field** | `FieldAppConfigItem` | Reference to a specific schema field | +| **Array Element** | `ArrayElementAppConfigItem` | List elements | + +### Path Hierarchy + +Config items use a **path** field (array of strings) for hierarchical addressing, similar to a filesystem or JSON path: + +```python +# Naming convention: ["domain", "workflow", "key"] +path=["routing", "entry.created", "target_bucket"] +path=["routing", "v2.canvas.userInteracted", "workflow"] +path=["scheduling", "retry_delay_seconds"] +path=["features", "use_new_packager"] +``` + +This makes it natural to model "configurable workflows" — one hierarchy level for the event type/workflow, another for the specific setting. + +### How Users Set Values + +App Configuration Items are editable through the **Benchling App Configuration UI** — the same place where app setup screens appear. Users (or admins) can change values without touching AWS or our config files. + +### Constraints & Considerations + +| Factor | Detail | +|--------|--------| +| **App-scoped** | Items are tied to a specific Benchling app (identified by `app_id`). Our app's config items are isolated from other apps. | +| **Access control** | Items inherit the app's permissions — same users who can install/configure the app can read items. | +| **Rate limits** | Standard Benchling API rate limits apply. Bulk operations are async (return a task). | +| **No events** | Changing a config item does not emit a webhook — the app must poll or cache values. | +| **No CloudFormation** | These items are purely in Benchling; the CDK stack cannot read them at deploy time. | +| **SDK version** | Available in `benchling-api-client` ≥1.23.1 (which we already use). | + +### How It Would Work in Practice + +``` +┌─────────────────┐ deploy-time ┌──────────────────┐ +│ ProfileConfig │ ──────────────────→ │ CDK Stack │ +│ (XDG file) │ │ (infrastructure) │ +└─────────────────┘ └──────────────────┘ + │ + (env vars for AWS + service endpoints) + ▼ +┌─────────────────┐ runtime ┌──────────────────┐ +│ Benchling App │ ←──────────────── │ Python App │ +│ Config Items │ benchling.apps. │ (webhook handler) │ +│ (key-value) │ list_app_config… │ │ +└─────────────────┘ └──────────────────┘ +``` + +The webhook handler at startup (or on each event) fetches relevant config items from Benchling using the SDK client it already has, then dispatches based on those rules. Changes made in the Benchling UI take effect on the next webhook event (no redeploy needed). + +--- + +## 2. Entry Fields (Per-Event Routing Hints) + +### What It Is + +Every Benchling entry has **fields** — structured data defined by its schema. Fields can be text, dropdowns, numbers, dates, entities, etc. The webhook already fetches entry data via `benchling.entries.get_entry_by_id()` in `entry_packager.py`. + +### Existing Usage + +In `entry_references.py`, we already parse entry fields to discover links to other entities. The `entry_packager.py` fetches the full entry dict including `fields`. + +### Configuration Opportunities + +A schema designer could add a field like "Quilt Target Bucket" (dropdown) or "Quilt Workflow" (text) to entry schemas. The webhook then reads that field value from the entry data and uses it to route: + +```python +entry = benchling.entries.get_entry_by_id(entry_id) +target_bucket = entry.fields.get("quilt_target_bucket") +workflow_name = entry.fields.get("quilt_workflow") +``` + +### Constraints + +| Factor | Detail | +|--------|--------| +| **Per-entry** | Values are set per entry, not globally — good for per-experiment routing, bad for global defaults | +| **Schema-dependent** | The field must exist in the entry's schema; different schemas may not have the field | +| **UI-native** | Users set values naturally in the Benchling entry editor | +| **No namespace collision** | Field names are scoped per schema — but prefixing with `quilt_` is wise | +| **Fetched on every event** | We already fetch the entry — the field value comes for free | + +### When to Use + +- Per-experiment bucket routing ("this experiment goes to bucket A, that one to bucket B") +- Per-entry workflow selection +- As a complement to global config (entry-level override) + +--- + +## 3. Entity Schemas (Discovering Structure) + +### What It Is + +Entity schemas define the field structure for entries in a registry. The SDK provides `benchling.schemas.*` with methods like `list_entry_schemas()`, `get_entry_schema()`, `list_entity_schemas()`. + +### Configuration Opportunities + +- **Schema discovery**: The webhook could discover which schemas exist and use schema names/IDs as routing dimensions +- **Field type inspection**: Check if a schema has a certain field before trying to read it +- **Template matching**: Route entries based on which entry template they were created from (`entry_template_id` on the Entry model) + +```python +# Discover schema for a given entry +entry_schema = entry.schema # Available on fetched Entry +if entry_schema and entry_schema.name == "Sequencing Results": + # Apply sequencing-specific routing + ... +``` + +### Constraints + +- Read-only — schemas are created by Benchling admins in the Schema Designer +- Schema names can change; IDs are stable + +--- + +## 4. Projects (Primary Per-Project Routing Dimension) + +### The Entry → Folder → Project Chain + +The Entry model does **not** have a `project_id` directly, but the chain is straightforward: + +```text +Entry.folder_id → Folder → Folder.project_id → Project +``` + +The **Folder** model (`benchling_api_client/models/folder.py`) has exactly the fields needed: + +```python +class Folder: + id: str + name: str + parent_folder_id: str | None # For nested folder trees + project_id: str # The project this folder belongs to +``` + +The **Project** model (`benchling_api_client/models/project.py`) is simpler — mostly metadata: + +```python +class Project: + id: str + name: str + owner: Organization | UserSummary + archive_record: ArchiveRecord | None + # No custom fields, no tags, no key-value storage +``` + +### How to Resolve at Runtime + +```python +# 1. Entry comes in via webhook — we already fetch it +entry = benchling.entries.get_entry_by_id(entry_id) + +# 2. Get the folder to find the project +folder = benchling.folders.get_by_id(entry.folder_id) + +# 3. Read the project_id +project_id = folder.project_id + +# 4. (Optional) Get the project name for human-readable routing +project = benchling.projects.get_by_id(project_id) +project_name = project.name # e.g. "Drug X Phase 2 Study" +``` + +### Configuration Opportunities + +The **project name** (or ID) is a stable routing dimension. Map project names to routing rules in **App Configuration Items**: + +```json +{ + "project_routing": { + "Drug X Phase 2 Study": { + "bucket": "quilt-drugx-phase2", + "workflow": "clinical-pipeline", + "prefix": "drugx" + }, + "Cell Line Screening": { + "bucket": "quilt-cell-line-screening", + "workflow": "screening-pipeline", + "prefix": "screening" + } + }, + "default": { + "bucket": "quilt-benchling-default", + "workflow": "", + "prefix": "benchling" + } +} +``` + +### Why Projects Work Well + +| Factor | Detail | +|-------- | -------- | +| **Stable** | Project names change less often than entries or experiments | +| **Organizational** | Projects map to real teams/studies/screens — the right unit for routing | +| **Natural UX** | Users organize work by project; routing by project matches mental model | +| **No per-entry burden** | All entries in a project automatically get the same routing — no manual per-entry config | +| **Cachable** | Project ID lookup is a fast API call; results can be cached per event batch | + +--- + +## 5. Folders (Weak, Hierarchy Only) + +### What It Is + +Folders organize entries in a tree. The SDK provides `benchling.folders.*` with `get_folder()`, `list_folders()`. Like projects, the model is minimal: `id`, `name`, `parent_folder_id`, `archive_record`. + +### Configuration Opportunities + +The folder hierarchy could be used as a routing dimension: + +```python +entry = benchling.entries.get_entry_by_id(entry_id) +folder_id = entry.folder_id +if folder_id: + folder = benchling.folders.get_folder(folder_id) + # Walk up folder tree for convention-based routing + # e.g., /Projects/ABC123/QuiltConfig → discover routing rules +``` + +### Why It's Weak + +- No custom fields or metadata on folders +- Walking the folder tree is fragile (folder names change, are reorganized) +- Not a natural configuration surface +- Requires the user to maintain a naming convention + +--- + +## 6. Dropdowns (Enumerated Values) + +### What It Is + +Dropdowns are reusable option lists in Benchling. The SDK provides `benchling.dropdowns.*`. Each dropdown has a name and a list of options. + +### Configuration Opportunities + +Useful as a source of enumerated values for routing configuration: + +```python +dropdowns = benchling.dropdowns.list_dropdowns() +for d in dropdowns: + if d.name == "Quilt Target Buckets": + # Options define valid bucket targets + for option in d.options: + valid_buckets.append(option.name) +``` + +### Constraints + +- Dropdowns define option *lists*, not key-value config +- No mechanism to associate a dropdown option with a configuration value (other than the option name) +- Best used in combination with entry fields (dropdown field → select value → route) + +--- + +## Recommended Architecture for Issue #392 + +### Tier 1: App Configuration Items (global defaults) + +Use App Configuration Items for global routing rules that apply to all entries (or for mapping project names to routing rules): + +```python +# Fetch routing rules at startup +config_items = benchling.apps.list_app_configuration_items(app_id=our_app_id) +routing_rules = {} +for item in config_items: + if item.path[0] == "routing": + # {"routing": {"entry.created": {"bucket": "bucket-a", "workflow": "alpha"}}} + pass +``` + +**Advantages:** Editable via Benchling UI, structured values, scoped to our app. + +### Tier 2: Per-Project Routing (via folder → project chain) + +Use the entry's folder → project chain to look up project-specific routing rules in App Configuration Items: + +```python +# 1. Fetch the entry (already done) +entry = benchling.entries.get_entry_by_id(entry_id) + +# 2. Walk folder → project +folder = benchling.folders.get_by_id(entry.folder_id) +project = benchling.projects.get_by_id(folder.project_id) + +# 3. Look up routing rules for this project +# (from Tier 1 config items, cached at startup) +project_rules = routing_rules.get("projects", {}).get(project.name, routing_rules.get("default")) +``` + +**Advantages:** Zero per-entry config — all entries in a project inherit its routing. Matches how users naturally organize work. + +### Tier 3: Secrets Manager (sensitive, infrastructure-level) + +Keep sensitive/static config in AWS Secrets Manager as today: + +- Benchling OAuth credentials (client_id, client_secret) +- Infrastructure references (bucket ARNs, queue URLs) +- Default values that shouldn't change without deploy review + +### Not Recommended for Config Storage + +- **Projects** — too little metadata, wrong abstraction +- **Folders** — fragile naming conventions, no structured storage +- **Standalone dropdowns** — option lists, not key-value + +--- + +## What This Means for "No Auto-Creation" + +The "no auto-creation" goal is about **infrastructure** (buckets, workgroups, queues) — not about configuration. App Configuration Items are the right tool for configuring *how* the webhook uses infrastructure, but the deploy-time check that referenced resources actually exist is still a CDK/infrastructure concern. These are orthogonal. + +--- + +## What This Means for "Multiple Buckets" + +App Configuration Items can define the bucket-per-event-type and bucket-per-project mapping: + +```json +{ + "routing": { + "entry.created": { "bucket": "quilt-benchling-experiments" }, + "entry.updated": { "bucket": "quilt-benchling-experiments" }, + "v2.canvas.userInteracted": { + "bucket": "quilt-benchling-canvases", + "workflow": "canvas-workflow" + } + }, + "projects": { + "Drug X Phase 2": { "bucket": "quilt-drugx-phase2", "prefix": "drugx" }, + "Cell Screening": { "bucket": "quilt-screening", "prefix": "screen" } + } +} +``` + +The CDK stack still needs IAM permissions for all possible buckets at deploy time, but the routing logic lives in Benchling. + +--- + +## What This Means for "Configurable Workflows" + +A workflow configuration stored in App Configuration Items could look like: + +```json +{ + "workflows": { + "default": { + "package_prefix": "benchling", + "metadata_key": "experiment_id", + "workflow_name": "" + }, + "sequencing": { + "event_types": ["entry.created", "entry.updated.fields"], + "schema_filter": ["Sequencing Results", "QC Report"], + "package_prefix": "sequencing", + "metadata_key": "run_id", + "workflow_name": "fastq-pipeline" + }, + "canvas_interaction": { + "event_types": ["v2.canvas.userInteracted"], + "package_prefix": "interactive", + "metadata_key": "experiment_id", + "workflow_name": "" + } + } +} +``` + +--- + +## Related SDK Models + +Found in `benchling_api_client` and `benchling_sdk` (already installed, version ≥1.23.1): + +### API Endpoints (in `benchling_api_client/api/apps/`) +- `list_app_configuration_items.py` — List with `app_id`, `ids`, `modified_at` filters +- `get_app_configuration_item_by_id.py` — Get single item +- `create_app_configuration_item.py` — Create (supports 8+ value types) +- `update_app_configuration_item.py` — Update existing +- `bulk_create_app_configuration_items.py` — Async bulk create +- `bulk_update_app_configuration_items.py` — Async bulk update + +### SDK Service (`benchling_sdk/services/v2/stable/app_service.py`) +- `benchling.apps.list_app_configuration_items(app_id=...)` +- `benchling.apps.get_app_configuration_item_by_id(item_id)` +- `benchling.apps.create_app_configuration_item(...)` +- `benchling.apps.update_app_configuration_item(...)` +- `benchling.apps.bulk_create_app_configuration_items(...)` +- `benchling.apps.bulk_update_app_configuration_items(...)` + +### Value Type Models +- `JsonAppConfigItem` — Structured JSON, ideal for complex routing rules +- `TextAppConfigItem` — String values +- `SecureTextAppConfigItem` — Encrypted values +- `BooleanAppConfigItem` — Feature flags +- `IntegerAppConfigItem` / `FloatAppConfigItem` — Numeric +- `DateAppConfigItem` / `DatetimeAppConfigItem` — Temporal +- `EntitySchemaAppConfigItem` — Schema references +- `FieldAppConfigItem` — Field references +- `ArrayElementAppConfigItem` — List elements + +### Entry/Field Models +- `Entry` — `fields` (dict of name → Field), `folder_id`, `schema`, `display_id` +- `Field` — `value` (Any), `display_value`, `type`, `text_value` +- `Fields` — Dict[str, Field] on the Entry model +- `EntrySchema` — `id`, `name`, `modified_at` + +--- + +## Key Takeaway + +**App Configuration Items are the right tool for configurable workflows/multi-bucket routing.** They are: +1. Native to Benchling — purpose-built for app configuration storage +2. Editable through the Benchling UI — no AWS console access needed +3. Readable at runtime — using the SDK client we already have +4. Structured — JSON items can hold arbitrary routing rule trees +5. Scoped — tied to our app, isolated from other apps + +Entry custom fields complement this by enabling per-entry overrides. The two together cover the full range from "global defaults" to "per-experiment routing." From 51e9dbdc4cdee9e43e9abbce9443c8b7de62fe6b Mon Sep 17 00:00:00 2001 From: "Dr. Ernie Prabhakar" Date: Thu, 2 Jul 2026 14:20:11 -0700 Subject: [PATCH 2/7] docs: add config tier audit (#04) and migration strategy (#05) for #392 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 04-tier-audit.md — complete inventory of all 64 config parameters mapped to the three-tier model (App Config / Secrets / Env Vars) - 05-migration-strategy.md — phased rollout for both open-source standalone and platform-integrated deployments, covering legacy CfnParameter compatibility and rollback safety Co-Authored-By: Claude --- spec/392-configurability/04-tier-audit.md | 195 ++++++++++++ .../05-migration-strategy.md | 287 ++++++++++++++++++ 2 files changed, 482 insertions(+) create mode 100644 spec/392-configurability/04-tier-audit.md create mode 100644 spec/392-configurability/05-migration-strategy.md diff --git a/spec/392-configurability/04-tier-audit.md b/spec/392-configurability/04-tier-audit.md new file mode 100644 index 00000000..f155ef6f --- /dev/null +++ b/spec/392-configurability/04-tier-audit.md @@ -0,0 +1,195 @@ +# Configuration Audit: Settings → Tier Mapping + +Every configuration parameter across the entire system, where it originates, where it's consumed, and which tier it belongs in under the new three-tier model. + +--- + +## Tier Definitions + +| Tier | Storage | Editable by | Changes take effect | Used for | +|------|---------|-------------|-------------------|----------| +| **1 — Benchling App Config** | Benchling App Configuration Items | Benchling admins via UI | Next webhook event (no redeploy) | Routing rules, feature flags, per-project config | +| **2 — AWS Secrets Manager** | AWS Secrets Manager secret | AWS console / `sync-secrets --force` | ECS restart | Sensitive credentials, package config | +| **3 — Env Vars / CDK** | ECS task definition (CDK) | `npm run deploy` | Stack update (redeploy) | Infrastructure endpoints, fixed deployment config | + +### Guiding principle + +Move settings to the *highest tier that fits their sensitivity and change frequency*: +- Changes frequently? → Tier 1 (no redeploy) +- Sensitive but changes occasionally? → Tier 2 (secret rotation) +- Infrastructure endpoint / rarely changes? → Tier 3 (deploy-time) + +--- + +## Complete Audit + +### A. Secrets Manager Secret (`BenchlingSecretData`) + +Location: `docker/src/secrets_manager.py` lines 48–88 (Python dataclass) +Synced from: `bin/commands/sync-secrets.ts` lines 292–308 +Read by: `config.get_benchling_secrets()` at runtime + +| Field (secret JSON key) | Mapped from ProfileConfig | Current use | Proposed tier | Rationale | +|-------------------------|--------------------------|-------------|---------------|-----------| +| `tenant` | `benchling.tenant` | Benchling API auth | **2 — Secrets Manager** | Sensitive — tenant URL | +| `client_id` | `benchling.clientId` | OAuth client ID | **2 — Secrets Manager** | Sensitive credential | +| `client_secret` | `benchling.clientSecret` | OAuth client secret | **2 — Secrets Manager** | Sensitive credential | +| `app_definition_id` | `benchling.appDefinitionId` | Webhook HMAC verification | **2 — Secrets Manager** | Security — ties webhook to app | +| `user_bucket` | `packages.bucket` | S3 destination for packages | **2 → 1** (routing) | Should live in Tier 1 routing rules per-project | +| `pkg_prefix` | `packages.prefix` | S3 key prefix | **2 → 1** (routing) | Per-project prefix should be in Tier 1 | +| `pkg_key` | `packages.metadataKey` | Metadata linking key | **2 → 1** (routing) | Per-project metadata key in Tier 1 | +| `workflow` | `packages.workflow` | Quilt workflow name | **2 → 1** (routing) | Workflow selection = routing rule, belongs in Tier 1 | +| `log_level` | `logging.level` | Python log level | **1 — App Config** | Operators change without redeploy | +| `enable_webhook_verification` | `security.enableVerification` | HMAC verification toggle | **2 — Secrets Manager** | Security setting — stays with credentials | +| `queue_url` | *(legacy, unused)* | SQS queue URL | **3 — Env Var** | Already overridden by env var in v0.8.0+ | + +### B. Environment Variables (Set by CDK FargateService) + +Location: `lib/fargate-service.ts` lines 278–316 +Read by: `docker/src/config.py` and other Python modules + +| Env var | Source (CDK parameter) | Read by | Current use | Proposed tier | Rationale | +|---------|----------------------|---------|-------------|---------------|-----------| +| `AWS_REGION` | `config.deployment.region` | `config.py`, SDK clients | AWS region for all boto3 calls | **3 — Env Var** | Infrastructure — never changes per deployment | +| `AWS_DEFAULT_REGION` | `config.deployment.region` | boto3 default | AWS region fallback | **3 — Env Var** | Same as above | +| `PORT` | hardcoded `"8080"` | uvicorn/gunicorn | HTTP server port | **3 — Env Var** | Internal container port | +| `QUILT_WEB_HOST` | `CfnParameter QuiltWebHost` → `config.quilt.catalog` | `config.py` | Quilt catalog URL for links | **3 — Env Var** | Infrastructure endpoint | +| `ATHENA_USER_DATABASE` | `CfnParameter AthenaUserDatabase` → `config.quilt.database` | `config.py`, `package_query.py` | Glue/Athena database name | **3 — Env Var** | Infrastructure — Glue DB name | +| `ATHENA_USER_WORKGROUP` | `CfnParameter AthenaUserWorkgroup` | `config.py`, `package_query.py` | Athena workgroup | **3 → 1?** | Could be per-project, but fine as env var | +| `PACKAGER_SQS_URL` | `CfnParameter PackagerQueueUrl` → `config.quilt.queueUrl` | `config.py` | SQS queue for package creation | **3 — Env Var** | Infrastructure — queue URL | +| `BenchlingSecret` | `CfnParameter BenchlingSecretARN` → secret name | `config.py` | Secrets Manager secret name | **3 — Env Var** | Ties task to secret; changes with secret rotation | +| `ENABLE_WEBHOOK_VERIFICATION` | hardcoded `"true"` | `config.py`, `app.py` | HMAC verification | **2 — Secret** | Overrides secret value; keep in sync | +| `APP_ENV` | hardcoded `"production"` | `config.py`, `app.py` | JSON vs console logging | **3 — Env Var** | Deployment mode | +| `LOG_LEVEL` | `CfnParameter LogLevel` | `config.py`, `app.py` | Python log level | **1 — App Config** | Already in secret; should be primary source | +| `PACKAGE_EVENT_QUEUE_URL` | from `packageEventQueue.queueUrl` | `sqs_consumer.py` | Package event SQS queue | **3 — Env Var** | Created by stack, fixed | +| `PACKAGE_EVENT_CONCURRENCY` | hardcoded `"5"` | `sqs_consumer.py` | Concurrent event processing | **1 — App Config** | Tuning parameter | +| `PACKAGE_EVENT_GRACEFUL_TIMEOUT` | hardcoded `"30"` | `sqs_consumer.py` | Shutdown grace period | **3 — Env Var** | Infrastructure — container config | +| `PACKAGING_REQUEST_QUEUE_URL` | from `packagingRequestQueue.queueUrl` | `packaging_consumer.py`, `packaging_publisher.py` | FIFO packaging queue | **3 — Env Var** | Created by stack, fixed | +| `PACKAGING_REQUEST_CONCURRENCY` | hardcoded `"5"` | `packaging_consumer.py` | Concurrent packaging | **1 — App Config** | Tuning parameter | +| `PACKAGING_REQUEST_GRACEFUL_TIMEOUT` | hardcoded `"30"` | `packaging_consumer.py` | Shutdown grace period | **3 — Env Var** | Infrastructure — container config | +| `QUILT_WRITE_ROLE_ARN` | *(from StackConfig)* | `config.py`, `entry_packager.py` | Cross-account IAM role | **3 — Env Var** | Infrastructure — IAM role ARN | + +### C. CloudFormation Parameters (CDK Stack) + +Location: `lib/benchling-webhook-stack.ts` lines 84–148 + +| CfnParameter | StackConfig field | Default | Overridable via deploy | Notes | +|-------------|-------------------|---------|----------------------|-------| +| `PackagerQueueUrl` | `quilt.queueUrl` | from config | Yes | Passes SQS URL to Fargate | +| `AthenaUserDatabase` | `quilt.database` | from config | Yes | Glue DB name | +| `QuiltWebHost` | `quilt.catalog` | from config | Yes | Catalog domain | +| `AthenaUserWorkgroup` | *(none — legacy)* | `""` | Yes | Workgroup name | +| `BenchlingSecretARN` | `benchling.secretArn` | from config | Yes | Secret ARN | +| `LogLevel` | *(none — legacy)* | `"INFO"` | Yes | Fallback log level | +| `ImageTag` | `deployment.imageTag` | `"latest"` | Yes | Docker image | +| `PackageBucket` | *(from `packages.bucket`)* | `""` | Yes | Bucket for IAM | +| `QuiltDatabase` | `quilt.database` | from config | Yes | Same as AthenaUserDatabase | + +All stay **Tier 3** — these are infrastructure-level parameters controlling what the CDK stack provisions. + +### D. ProfileConfig (XDG Config File ~ `config.json`) + +Location: `lib/types/config.ts` interface `ProfileConfig` +Written by: Setup wizard (`bin/commands/setup-wizard.ts`) +Read by: `deploy.ts`, `sync-secrets.ts` + +| Section | Field | Used by | Proposed tier | Notes | +|---------|-------|---------|---------------|-------| +| `quilt` | `stackArn` | deploy.ts — resolves service endpoints | **3** (profile field) | Deploy-time only | +| `quilt` | `catalog` | CDK → env var → runtime | **3 → env var** | Infrastructure endpoint | +| `quilt` | `database` | CDK → env var → runtime | **3 → env var** | Infrastructure | +| `quilt` | `queueUrl` | CDK → env var → runtime | **3 → env var** | Infrastructure | +| `quilt` | `region` | CDK → env var → runtime | **3 → env var** | Infrastructure | +| `quilt` | `bucketWritePolicyArn` | CDK — IAM policy | **3** (profile field) | Deploy-time IAM | +| `quilt` | `athenaUserPolicyArn` | CDK — IAM policy | **3** (profile field) | Deploy-time IAM | +| `benchling` | `tenant` | `sync-secrets` → secret | **2 → secret** | Sensitive | +| `benchling` | `clientId` | `sync-secrets` → secret | **2 → secret** | Sensitive | +| `benchling` | `clientSecret` | `sync-secrets` → secret | **2 → secret** | Sensitive | +| `benchling` | `secretArn` | CDK → env var → runtime | **3** (profile field) | Reference to Tier 2 | +| `benchling` | `appDefinitionId` | `sync-secrets` → secret | **2 → secret** | Security | +| `packages` | `bucket` | CDK (IAM) + `sync-secrets` → secret | **2 → secret** IAM part stays **3**, routing moves to **1** | Dual-use: IAM + routing | +| `packages` | `prefix` | `sync-secrets` → secret → runtime | **1 → routing** | Per-project config | +| `packages` | `metadataKey` | `sync-secrets` → secret → runtime | **1 → routing** | Per-project config | +| `packages` | `workflow` | `sync-secrets` → secret → runtime | **1 → routing** | Per-project config | +| `deployment` | `region` | CDK stack env | **3** (profile field) | Infrastructure | +| `deployment` | `account` | CDK stack env | **3** (profile field) | Infrastructure | +| `deployment` | `imageTag` | CDK → env var | **3** (profile field) | Deploy-time | +| `deployment` | `stackName` | CDK stack name | **3** (profile field) | Deploy-time | +| `deployment` | `vpc.*` | CDK VPC construct | **3** (profile field) | Infrastructure | +| `logging` | `level` | `sync-secrets` → secret → runtime | **1 → app config** | Operators tune log level | +| `security` | `webhookAllowList` | CDK — API Gateway policy | **3** (profile field) | Infrastructure — IP filtering | +| `security` | `enableVerification` | `sync-secrets` → secret | **2 → secret** | Security toggle | +| `_metadata` | *(all)* | Wizard tracking only | — (metadata) | Not a config setting | +| `integratedStack` | flag | deploy.ts — mode check | **3** (profile field) | Deployment mode | + +### E. Runtime Config Fields (Python `Config` dataclass) + +Location: `docker/src/config.py` lines 40–65 + +| Config field | Env var | Also set from secret? | Proposed tier | Notes | +|-------------|---------|----------------------|---------------|-------| +| `app_env` | `APP_ENV` | No | **3 — Env Var** | Deployment mode | +| `log_level` | `LOG_LEVEL` | Yes (secret overrides) | **1 — App Config** | Primary: secret; override: env var | +| `aws_region` | `AWS_REGION` | No | **3 — Env Var** | Infrastructure | +| `s3_bucket_name` | *(none)* | Yes (`user_bucket`) | **1 → routing** | Should come from Tier 1 | +| `s3_prefix` | *(none)* | Yes (`pkg_prefix`) | **1 → routing** | Should come from Tier 1 | +| `package_key` | *(none)* | Yes (`pkg_key`) | **1 → routing** | Should come from Tier 1 | +| `quilt_catalog` | `QUILT_WEB_HOST` | No | **3 — Env Var** | Infrastructure | +| `quilt_database` | `ATHENA_USER_DATABASE` | No | **3 — Env Var** | Infrastructure | +| `queue_url` | `PACKAGER_SQS_URL` | No | **3 — Env Var** | Infrastructure | +| `athena_user_workgroup` | `ATHENA_USER_WORKGROUP` | No | **3 — Env Var** | Infrastructure | +| `enable_webhook_verification` | `ENABLE_WEBHOOK_VERIFICATION` | Yes (secret) | **2 — Secret** | Security toggle | +| `pkg_prefix` | *(none)* | Yes (`pkg_prefix`) | **1 → routing** | Same as s3_prefix | +| `workflow` | *(none)* | Yes (`workflow`) | **1 → routing** | Per-project workflow | +| `quilt_write_role_arn` | `QUILT_WRITE_ROLE_ARN` | No | **3 — Env Var** | IAM role ARN | + +--- + +## Summary: What Moves Where + +| Setting | Currently lives in | Moves to | Reason | +|---------|------------------|----------|--------| +| `packages.bucket` → routing | Secret → config.py | **Tier 1** (per-project) | Bucket per project, operators change without redeploy | +| `packages.prefix` | Secret → config.py | **Tier 1** (per-project) | Prefix per project | +| `packages.metadataKey` | Secret → config.py | **Tier 1** (per-project) | Per-project linking key | +| `packages.workflow` | Secret → config.py | **Tier 1** (per-project) | Per-project workflow | +| `logging.level` | Secret → config.py | **Tier 1** (app config) | Tuning — no redeploy needed | +| `PACKAGE_EVENT_CONCURRENCY` | Hardcoded in env vars | **Tier 1** (app config) | Tuning parameter | +| `PACKAGING_REQUEST_CONCURRENCY` | Hardcoded in env vars | **Tier 1** (app config) | Tuning parameter | +| Everything else | Env vars / Secret | **Stay where they are** | Infrastructure or sensitive | + +### Remaining in ProfileConfig only (never reach runtime) + +These are deploy-time or wizard-only fields that don't need to move: + +- `quilt.stackArn` — deploy-time resolution only +- `quilt.bucketWritePolicyArn` — CDK IAM attachment, deploy-time +- `quilt.athenaUserPolicyArn` — CDK IAM attachment, deploy-time +- `deployment.*` — infrastructure topology (region, account, VPC, image tag) +- `security.webhookAllowList` — API Gateway resource policy, deploy-time +- `_metadata` — provenance tracking +- `integratedStack` — deployment mode flag + +--- + +## Migration Path + +### Phase 1: Tier 1 read-side (no config writes yet) + +1. The Python app already has a `benchling` SDK client at runtime +2. Add a method to fetch App Configuration Items at startup: `benchling.apps.list_app_configuration_items(app_id=...)` +3. Parse routing rules from items with `path[0] == "routing"` or `path[0] == "projects"` +4. Fall back to current secret-based values for any missing rules +5. No changes to deploy flow or secret sync + +### Phase 2: Tier 1 write-side (populate config items) + +1. Add an option to the setup wizard or a CLI command to seed App Configuration Items +2. Alternatively, seed via a one-time migration from existing secret values +3. Document how Benchling admins edit items in the Benchling UI + +### Phase 3: Make Tier 1 authoritative + +1. Change the runtime to read routing values from Tier 1 first, secret as fallback +2. Eventually stop syncing routing fields (`user_bucket`, `pkg_prefix`, `pkg_key`, `workflow`) to Secrets Manager +3. Keep `tenant`, `client_id`, `client_secret`, `app_definition_id` in Secrets Manager permanently diff --git a/spec/392-configurability/05-migration-strategy.md b/spec/392-configurability/05-migration-strategy.md new file mode 100644 index 00000000..bc8920aa --- /dev/null +++ b/spec/392-configurability/05-migration-strategy.md @@ -0,0 +1,287 @@ +# Migration Strategy + +How existing deployments — both open-source standalone and platform-integrated — transition from the current config model to the three-tier model without breaking stacks, losing settings, or requiring manual reconfiguration. + +--- + +## Migration Principles + +1. **Backward compatible always** — existing stacks must update cleanly; no `cdk deploy` failures from removed parameters +2. **Phased rollout** — read the new sources first, write the new sources later, remove the old sources last +3. **No data loss** — every existing configuration value is automatically carried forward +4. **Opt-in for new features** — per-project routing is additive; existing single-bucket setups work unchanged + +--- + +## What Changes (Recap from 04-tier-audit.md) + +| Setting | Current location | New location | Changes on deploy? | Changes at runtime? | +|---------|-----------------|-------------|-------------------|---------------------| +| `packages.bucket` | Secrets Manager + IAM | **Tier 1** per-project routing | IAM permissions stay; `user_bucket` in secret becomes fallback | App reads routing from Benchling config items | +| `packages.prefix` | Secrets Manager as `pkg_prefix` | **Tier 1** per-project routing | No CDK change | App reads from Tier 1 | +| `packages.metadataKey` | Secrets Manager as `pkg_key` | **Tier 1** per-project routing | No CDK change | App reads from Tier 1 | +| `packages.workflow` | Secrets Manager as `workflow` | **Tier 1** per-project routing | No CDK change | App reads from Tier 1 | +| `logging.level` | Secrets Manager as `log_level` | **Tier 1** app config item | No CDK change | App reads from Tier 1 | +| `PACKAGE_EVENT_CONCURRENCY` | Hardcoded env var | **Tier 1** app config item | No CDK change | App reads from Tier 1 | +| `PACKAGING_REQUEST_CONCURRENCY` | Hardcoded env var | **Tier 1** app config item | No CDK change | App reads from Tier 1 | +| All CfnParameters | `lib/benchling-webhook-stack.ts` | **Stays** as CfnParameters | No change | Still set env vars; values are now fallbacks | + +Key insight: **the CDK stack and CfnParameters don't change**. The env vars they set become *fallbacks* — the app checks Tier 1 first, falls back to the env var / secret if no value is found. + +--- + +## Stack Upgrade: Legacy CfnParameters + +### Question: Do we keep the legacy CfnParameters when upgrading? + +**Yes.** They stay in the template. Here's why: + +1. **Removing a CfnParameter from the CDK template causes CloudFormation to DELETE it from the stack parameters.** If a parameter has no default and was required, the update fails. If it has a default, the value is silently dropped — which is fine, but unnecessary churn. +2. **CfnParameters are free** — they don't cost money and don't add meaningful synthesis time. +3. **They serve as deploy-time overrides** — an operator running `npm run deploy:prod -- --parameters PackagerQueueUrl=https://...` still works. +4. **They act as a documented interface** — anyone reading the CloudFormation template can see what the stack expects. + +### What we do instead + +| Parameter | Action | +|-----------|--------| +| `PackagerQueueUrl` | **Keep.** Still needed for env var. | +| `AthenaUserDatabase` | **Keep.** Still needed for env var. | +| `QuiltWebHost` | **Keep.** Still needed for env var. | +| `AthenaUserWorkgroup` | **Keep.** Still needed for env var. | +| `BenchlingSecretARN` | **Keep.** Still needed for secret reference. | +| `LogLevel` | **Keep.** Still the fallback for log level (overridden by Tier 1 at runtime). | +| `ImageTag` | **Keep.** Controls which Docker image runs. | +| `PackageBucket` | **Keep.** Still needed for IAM permissions. | +| `QuiltDatabase` | **Keep.** Still needed for IAM. | + +**All CfnParameters stay.** The runtime simply prefers Tier 1 values where they exist. + +### What changes in the stack template + +Only two things: + +1. **New env vars MAY be added** for the `app_id` that the Python app uses to fetch Tier 1 config items (e.g., `BENCHLING_APP_ID`). This would be a new CfnParameter or hardcoded value. +2. **IAM policies MAY need expansion** if per-project routing references buckets not currently in the IAM policy. This is an *existing IAM change* — not new infrastructure. + +--- + +## Open Source (Standalone) Migration + +### Phase 1: Read-side only (no stack update) + +**What the user does:** `npm update && npm run deploy -- --profile my-profile` + +**What happens:** +1. CDK stack deploys identically — no CfnParameters added or removed +2. New Python app code fetches Tier 1 config items from Benchling +3. If no Tier 1 items exist, falls back to current secret/env var values +4. Everything works exactly as before — zero config changes + +**Code changes in this phase:** +``` +# New: Tier 1 config loader with fallback chain +def resolve_routing_config(benchling, config): + """Fetch routing rules: Tier 1 → secret → env var fallback.""" + tier1_rules = _fetch_tier1_config(benchling) + if tier1_rules: + return tier1_rules + + # Legacy fallback (current behavior) + return { + "bucket": config.s3_bucket_name, + "prefix": config.s3_prefix, + "metadata_key": config.package_key, + "workflow": config.workflow, + } +``` + +**Rollback:** Downgrade the app image. Old code never reads Tier 1. + +### Phase 2: Seed Tier 1 config items + +**What the user does:** `npm run config:seed` (new CLI command) + +**What happens:** +1. The CLI reads existing ProfileConfig/Secret values +2. Creates App Configuration Items in Benchling via the SDK +3. Prints a confirmation: "Routing config migrated to Benchling. Edit at: ..." + +``` +npm run config:seed -- --profile sales + + ✓ Read existing config from profile 'sales' + ✓ Connected to Benchling (tenant: my-company) + ✓ Created 4 App Configuration Items for routing rules + ✓ Source of truth: Benchling → App Configuration + ℹ️ Edit these values at: https://my-company.benchling.com/app/... +``` + +**No deploy needed.** The app discovers the items on the next webhook event. + +**Rollback:** Delete the App Configuration Items (CLI: `npm run config:clear`) — app falls back to legacy values. + +### Phase 3: Opt-in per-project routing + +**What the user does:** +1. Configures project names in App Configuration Items (via Benchling UI or CLI) +2. Optionally adds new S3 buckets to `ProfileConfig.packages.bucket` for IAM permissions + +**What happens:** +1. CDK stack gets additional IAM policies for new buckets (if any) +2. App reads entry → folder → project → routing rule +3. Entries route based on project name + +``` +# In App Configuration Items (JSON): +{ + "projects": { + "Study Alpha": { "bucket": "quilt-alpha", "prefix": "alpha" }, + "Study Beta": { "bucket": "quilt-beta", "prefix": "beta" } + }, + "default": { + "bucket": "quilt-benchling-main", + "prefix": "benchling" + } +} +``` + +--- + +## Platform (Integrated Stack) Migration + +Platform users deploy the webhook as part of the Quilt stack. Their upgrade path is different because the **Quilt stack owns the infrastructure** and the webhook config lives in a Quilt-managed secret. + +### What's different + +| Aspect | Standalone | Platform (Integrated) | +|--------|-----------|----------------------| +| Stack owner | User deploys `BenchlingWebhookStack` | Quilt stack includes webhook resources | +| Secret | Managed by `sync-secrets.ts` | Existing `BenchlingSecret` in Quilt stack | +| IAM policies | Created by webhook stack | Managed by Quilt stack resources | +| Config profile | `~/.config/benchling-webhook/{profile}` | User has profile but Quilt stack is external | + +### Migration for integrated mode + +**Phase 1 (Read-side):** Same as standalone — no infra changes, app reads Tier 1 config first. + +**Phase 2 (Seed):** Same `npm run config:seed` command, but reads from the integrated stack's existing secret. The secret still exists and is still managed by the Quilt stack. + +**Phase 3 (Per-project routing):** If new buckets are needed, the platform user must: +1. Ensure the Quilt stack's `BucketWritePolicy` covers the new buckets +2. Or ask their Quilt platform admin to add the buckets + +This is a constraint of the integrated model — the webhook cannot create its own IAM policies. + +--- + +## Edge Cases and Risks + +### 1. Stack update removes a CfnParameter that was in use + +**Risk:** Low — we aren't removing CfnParameters. +**If we did:** CloudFormation would show a "Before" value in the change set and prompt for a new value. The parameter would be removed from subsequent `describe-stacks` output but the running container still had the old env var. + +**Mitigation:** Never remove CfnParameters. Mark them as `@deprecated` in code comments and ignore the env var value at runtime if Tier 1 has a value. + +### 2. App Configuration Items don't exist yet + +**Risk:** Medium — first deploy after code change hits a new API call pattern. +**If it fails:** The error is logged (non-fatal) and the app falls back to legacy config. Zero service disruption. + +**Testing:** +- Unit test: `resolve_routing_config()` returns fallback when `list_app_configuration_items` returns empty +- Integration test: Deploy without seeding config items, verify old behavior unchanged + +### 3. Benchling API rate limits + +**Risk:** Low — `list_app_configuration_items` is a single API call at startup, cached for the lifetime of the container process. +**If rate-limited:** Boto3 retries handle this. If it fails completely, fallback logic applies. + +### 4. User deletes App Configuration Items accidentally + +**Recovery:** App falls back to legacy secret/env var values automatically. +**Restore:** `npm run config:seed -- --restore` re-creates the items from current secret values. + +### 5. Rollback deploys an older image + +**Safe by design:** Old code never calls `list_app_configuration_items`. It reads config exactly as before. +**Seeded items** remain in Benchling but are ignored. No stale/conflicting state. + +### 6. Two stacks sharing one Benchling tenant + +If two standalone stacks (e.g., `dev` and `prod`) point at the same Benchling tenant, they would both see the same App Configuration Items. This is correct only if they should share routing rules. + +**Mitigation:** Use a path prefix convention: `path=["routing", "dev", ...]` vs `path=["routing", "prod", ...]`. Or rely on `app_id` scoping (each stack registers as a separate Benchling app instance). + +### 7. Mixed version deployment during rolling update + +ECS rolling updates run old and new containers simultaneously for a brief window. The new code reads Tier 1; the old code reads secrets/env vars. Both paths resolve to the same values (Tier 1 has the same data as the source-of-truth), so no inconsistency. + +--- + +## Backward Compatibility Guarantees + +| Scenario | Works? | How | +|----------|--------|-----| +| Existing stack deploys with zero config changes | ✅ | All CfnParameters unchanged, default values unchanged | +| Existing secret has `user_bucket`, `pkg_prefix`, etc. | ✅ | Tier 1 fallback reads these exactly as today | +| `npm run deploy -- --parameters LogLevel=DEBUG` | ✅ | CfnParameter override still works; env var set as before | +| Old Docker image (pre-Tier-1) deployed on new config | ✅ | Old code never calls `list_app_configuration_items` | +| New Docker image deployed on old config (no Tier 1 items) | ✅ | New code calls `list_app_configuration_items`, gets empty, falls back to secret/env vars | +| User removes Tier 1 items while running new image | ✅ | Next webhook event gets fallback values; degraded but working | +| Two profiles sharing one Benchling app_id | ✅ (if intentional) | Paths are namespaced by profile name | + +--- + +## Rollback Procedure + +### Rollback Tier 1 → Secrets Manager only + +If Tier 1 config items are causing issues (e.g., wrong routing, API errors): + +```bash +# Option A: Delete the items (app falls back to secret values) +npm run config:clear + +# Option B: Re-deploy previous Docker image +npm run deploy:prod -- --profile sales --image-tag 0.17.2 +``` + +The app container restart picks up whichever image is deployed. Tier 1 items remain in Benchling but are ignored by old code. + +### Rollback entire migration + +```bash +# 1. Delete App Configuration Items +npm run config:clear + +# 2. Re-deploy old image tag +npm run deploy:prod -- --profile sales --image-tag 0.17.0 + +# 3. (Optional) Restore secret to known-good state +npm run setup -- --profile sales +``` + +--- + +## Migration Timeline + +``` +Phase 1: Read-side only ████████████░░░░░░ (this release) + - Add Tier 1 fetch + fallback logic + - All existing behavior unchanged + - Safe to deploy immediately + +Phase 2: Seed CLI tool ░░░░████████████░░ (next release) + - npm run config:seed command + - npm run config:clear command + - Documentation for editing in Benchling UI + +Phase 3: Per-project routing ░░░░░░░░██████████ (future release) + - Full routing resolution entry → folder → project + - Multi-bucket IAM support + - ProfileConfig changes for bucket lists +``` + +Each phase is independently deployable and reversible. From c13af5b1eb2388fea722036a4fc3547073948016 Mon Sep 17 00:00:00 2001 From: "Dr. Ernie Prabhakar" Date: Thu, 2 Jul 2026 14:31:32 -0700 Subject: [PATCH 3/7] docs: add app_id probe definition (#05a) and updated implementation checklist (#06) for #392 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 05a-list_app_configuration_items.md — procedure to determine the correct app_id parameter for list_app_configuration_items() - 06-configurability-checklist.md — revised with resolved P1/P2 issues: TTL caching, namespaced config:clear --force, app_id prerequisite, merge/precedence rules, folder-scoped project lookup, integrated-mode IAM guard Co-Authored-By: Claude --- .../05a-list_app_configuration_items.md | 154 ++++++++++++++++++ .../06-configurability-checklist.md | 109 +++++++++++++ 2 files changed, 263 insertions(+) create mode 100644 spec/392-configurability/05a-list_app_configuration_items.md create mode 100644 spec/392-configurability/06-configurability-checklist.md diff --git a/spec/392-configurability/05a-list_app_configuration_items.md b/spec/392-configurability/05a-list_app_configuration_items.md new file mode 100644 index 00000000..c112f788 --- /dev/null +++ b/spec/392-configurability/05a-list_app_configuration_items.md @@ -0,0 +1,154 @@ +# Define: `list_app_configuration_items` Probe + +## Objective + +Determine the correct `app_id` parameter for `benchling.apps.list_app_configuration_items()` so Phase 1 implementation is unblocked. Specifically: + +1. Is `appDefinitionId` (from the Benchling secret) the same as the `app_id` the API expects? +2. If not, what is — and where do we get it? +3. What does a successful response look like (structure, fields, path convention)? +4. Does `list_app_configuration_items` return items scoped to the calling app, or to any app? + +--- + +## How to run + +From the repo root with a working `dev` profile and local Docker: + +```bash +# Ensure docker/.venv is active and BenchlingSecret env var is set +cd docker +source .venv/bin/activate + +# Run the probe script +python -m scripts.probe_config_items +``` + +The script reads the existing Benchling secret from AWS Secrets Manager, creates a Benchling SDK client (same as the running app does), then calls `list_app_configuration_items` with several candidate identifiers and logs the results. + +--- + +## What to try + +### Candidate 1 — `appDefinitionId` + +This is stored in the Benchling secret as `app_definition_id` and used at runtime for JWKS fetching and HMAC verification. Try: + +```python +items = benchling.apps.list_app_configuration_items(app_id=app_definition_id) +``` + +### Candidate 2 — `list_apps()` iteration + +If Candidate 1 returns empty or errors, enumerate installed apps and inspect their IDs: + +```python +apps = benchling.apps.list_apps() +for app in apps: + print(f" id={app.id} name={app.name} app_definition_id={app.app_definition_id}") + items = benchling.apps.list_app_configuration_items(app_id=app.id) + print(f" -> {len(items)} config items") +``` + +This tells us whether our Benchling app has a different `id` than its `app_definition_id`. + +### Candidate 3 — No filter (optional, may fail) + +If the API supports omitting `app_id`: + +```python +items = benchling.apps.list_app_configuration_items() +``` + +### Candidate 4 — By config item `ids` filter + +If we know a config item ID exists, fetch it directly: + +```python +item = benchling.apps.get_app_configuration_item_by_id("cfg_...") +print(f" app_id={item.app.id} path={item.path}") +``` + +--- + +## What to record + +For each candidate, record: + +| Field | Value | +|-------|-------| +| Candidate | e.g. `appDefinitionId` | +| HTTP status | 200 / 403 / 404 | +| Error message | if any | +| Items returned | count | +| First item `id` | `cfg_...` | +| First item `path` | `["..."]` | +| First item `app.id` | The app_id on the returned item | +| First item `app.name` | App name on the returned item | +| First item `value` | (masked if sensitive) | + +--- + +## Expected outputs + +### Success — items exist + +```json +{ + "status": "ok", + "app_id_used": "app_def_abc123", + "total_items": 0, + "items": [] +} +``` + +### Success — items exist (once seeded) + +```json +{ + "status": "ok", + "app_id_used": "app_def_abc123", + "total_items": 2, + "items": [ + { + "id": "cfg_item_xyz", + "path": ["routing", "default", "bucket"], + "value": "my-bucket", + "app": { "id": "app_…", "name": "Benchling Webhook" } + } + ] +} +``` + +### Error — wrong identifier + +```json +{ + "status": "error", + "candidate": "appDefinitionId", + "http_status": 404, + "message": "App not found" +} +``` + +--- + +## What to do with the result + +Once the probe identifies the correct `app_id` source, update: + +1. **`06-configurability-checklist.md`** — resolve task 1's `app_id` source comment with the actual answer +2. **`02-bw-configurability.md`** — if the `app_id` flows through a new env var, add it to the env var table +3. **Implementation** — the Phase 1 startup code will use the identified source + +--- + +## Probe script location + +Write the probe as `docker/scripts/probe_config_items.py`. It should: +- Accept `--profile` (default `dev`) to load XDG config +- Read existing Benchling secret from AWS Secrets Manager +- Create a Benchling SDK client (same auth as the running app) +- Try candidates 1–3 above +- Print results as structured JSON to stdout +- Exit 0 on success, 1 if all candidates fail diff --git a/spec/392-configurability/06-configurability-checklist.md b/spec/392-configurability/06-configurability-checklist.md new file mode 100644 index 00000000..c55c9249 --- /dev/null +++ b/spec/392-configurability/06-configurability-checklist.md @@ -0,0 +1,109 @@ +# Issue 392 — Configurability Implementation Checklist + +Hand-off document for a code implementation agent. Context is in `spec/392-configurability/01`–`05`. + +## Constraint + +Phases can be developed independently but **all phases deploy together** in one release. Nothing is deployed until everything is ready. + +--- + +## Prerequisite + +**Run `05a-list_app_configuration_items.md` first** to determine the correct `app_id` parameter for `benchling.apps.list_app_configuration_items()`. Update this checklist with the result before starting Phase 1. + +--- + +## Phase 1 — App Config Items Read Path (Python runtime) + +1. **Add `benchling.apps.list_app_configuration_items()` call** at app startup in Docker Python code. Fetch all config items scoped to our app using the `app_id` identified by the prerequisite probe (likely `appDefinitionId` from the secret, or a separate `app.id` discovered via `list_apps()`). + +1. **Build a `RoutingConfig` model** from the config items, keyed by `path` convention: + - `["quilt", "routing", "", ""]` — event-type routing + - `["quilt", "projects", "", ""]` — per-project routing + - `["quilt", "settings", ""]` — global settings (log level, concurrency) + - `["quilt", "default", ""]` — fallback values + + All paths are namespaced under `"quilt"` so `config:clear` can safely delete only our items. + +1. **Implement a fallback chain** — for each config value: + + ```text + Tier 1 (App Config Item) → Tier 2 (Secrets Manager) → Tier 3 (env var / hardcoded default) + ``` + + If the App Config Items API fails or returns empty, behavior is unchanged. + +1. **Replace hardcoded `PACKAGE_EVENT_CONCURRENCY` and `PACKAGING_REQUEST_CONCURRENCY`** with Tier 1 lookup, falling back to current `"5"` default. + +1. **Replace `LOG_LEVEL`** with Tier 1 lookup, falling back to secret's `log_level`, falling back to `"INFO"`. + +1. **Cache Tier 1 config with a 60s TTL** and stale-while-refresh semantics, matching the existing `secrets_manager.py` pattern: + - On first call: block and fetch + - Within TTL: return cached value + - Past TTL, stale value exists: return stale, kick off background refresh thread + - The `app_id` is also cached; if it needs to change the container restarts anyway + +## Phase 2 — Per-Project Routing (Python runtime) + +1. **Resolve project from entry, but only for entry events with a folder.** Not all webhook events are entry-backed. Gate project lookup on `entry.folder_id` being present: + - Has `folder_id` → resolve project, apply per-project overrides + - No `folder_id` → skip project lookup, use event-type defaults + + Resolution chain: + + ```python + entry = benchling.entries.get_entry_by_id(entry_id) + if entry.folder_id: + folder = benchling.folders.get_by_id(entry.folder_id) + project = benchling.projects.get_by_id(folder.project_id) + project_name = project.name # key into Tier 1 + ``` + +1. **Apply project routing overrides with defined merge rules.** Precedence (highest → lowest): + + ```text + Project override → Event-type default → Global default → Legacy fallback + ``` + + **Merge strategy:** field-level merge, not whole-object replace. If a project rule sets only `bucket`, the `prefix` and `workflow` inherit from the event-type or global default. Supported event types: `entry.created`, `entry.updated.fields`, `v2.canvas.userInteracted` (matches existing `supported_event_types` set in `app.py`). + +1. **Handle project lookup failure gracefully.** If folder/project API calls fail (network error, deleted folder, missing permissions), log the error and use event-type defaults. Must not crash the webhook handler. + +## Phase 3 — ProfileConfig / CDK Changes + +1. **Add `packages.extraBuckets` to `ProfileConfig`.** An optional list of additional S3 bucket names that the stack needs IAM permissions for (for per-project multi-bucket routing). No auto-creation — operator lists existing buckets. + +1. **Pass extra buckets through `StackConfig`** to the CDK stack. Add IAM policy statements for each listed bucket (same permissions as the primary `packages.bucket`). + +1. **Add integrated-mode guard for IAM changes.** When `integratedStack: true`, the Quilt stack owns IAM. Deploy should print a warning that `packages.extraBuckets` is ignored in integrated mode — Tier 1 routes pointing at undeclared buckets will fail at runtime. Validation docs task should cover this. + +1. **Keep all existing CfnParameters.** No removals. They become deploy-time overrides and runtime fallbacks. + +## Phase 4 — CLI: Seed / Clear App Config + +1. **Add `npm run config:seed` command.** Reads current routing values from `ProfileConfig` + Secrets Manager, creates equivalent App Configuration Items in Benchling via the SDK. Prints the Benchling UI URL where items can be edited. + +1. **Add `npm run config:clear` command.** Deletes only App Configuration Items under the `["quilt"]` path namespace. Requires `--force`. Shows a dry-run preview of what will be deleted before confirming. App falls back to legacy config after items are removed. + +1. **Add `npm run config:inspect` command.** Dumps current Tier 1 items and effective resolved config (Tier 1 → secret → env var) for debugging. + +## Phase 5 — Docs + +1. **Update README** to document: + - Three-tier configuration model + - How to configure per-project routing in Benchling (add benchmark on what needs to happen) + - `config:seed`, `config:clear`, `config:inspect` CLI commands + - The fallback chain and backward compatibility guarantees + - Integrated-mode IAM limitation: Tier 1 routes can only use buckets covered by the Quilt stack's BucketWritePolicy + +1. **Add `app-manifest.yaml` entries** for any new App Configuration Items the app expects (declares them to Benchling). + +--- + +## Test Plan + +- **Unit tests**: Fallback chain resolution, project lookup, missing config items +- **Integration test**: Deploy, seed config, verify app reads Tier 1 values +- **No-Tier-1 test**: Deploy without seeding — everything works as before +- **Rollback test**: Deploy new, roll back to old image — old image works unaffected From 06dd9ccb9d12567d8a069c7ef534769622b15ec6 Mon Sep 17 00:00:00 2001 From: "Dr. Ernie Prabhakar" Date: Thu, 2 Jul 2026 21:07:05 -0700 Subject: [PATCH 4/7] docs: add probe_results (#05b) and update implementation checklist (#06) for #392 --- docker/scripts/probe_config_items.py | 489 ++++++++++++++++++ spec/392-configurability/05b-probe_results.md | 158 ++++++ .../06-configurability-checklist.md | 4 +- 3 files changed, 649 insertions(+), 2 deletions(-) create mode 100755 docker/scripts/probe_config_items.py create mode 100644 spec/392-configurability/05b-probe_results.md diff --git a/docker/scripts/probe_config_items.py b/docker/scripts/probe_config_items.py new file mode 100755 index 00000000..d37c12b6 --- /dev/null +++ b/docker/scripts/probe_config_items.py @@ -0,0 +1,489 @@ +#!/usr/bin/env python3 +""" +Probe: list_app_configuration_items + +Determines the correct app_id parameter for +benchling.apps.list_app_configuration_items() so Phase 1 +configurability implementation is unblocked. + +Usage: + cd docker + uv run python -m scripts.probe_config_items [--profile PROFILE] [--seed] + +With --seed: creates test config items on the named app (default: quilt-docker), +lists them back to verify structure, then cleans up. + +Exit code: 0 if at least one candidate succeeded, 1 if all failed. +""" + +import argparse +import json +import sys +from pathlib import Path +from typing import Any, Optional + +# --------------------------------------------------------------------------- +# Path setup — same pattern as scripts/benchling-webhook-config and +# scripts/test_benchling.py +# --------------------------------------------------------------------------- +_SCRIPTS_DIR = Path(__file__).resolve().parent +_PROJECT_ROOT = _SCRIPTS_DIR.parent +sys.path.insert(0, str(_PROJECT_ROOT / "src")) + +import boto3 +from benchling_sdk.auth.client_credentials_oauth2 import ClientCredentialsOAuth2 +from benchling_sdk.benchling import Benchling +from botocore.config import Config as BotocoreConfig + +from src.secrets_manager import BenchlingSecretData, fetch_benchling_secret +from src.xdg_config import XDGConfig + +# --------------------------------------------------------------------------- +# SDK model imports (used by --seed) +# --------------------------------------------------------------------------- +_SEED_TYPE = None # lazy-imported in _seed_items + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +logger: Any = None # placeholder if we ever add logging + + +def load_profile_config(profile: str) -> dict[str, Any]: + """Load and return the complete merged XDG config for *profile*.""" + xdg = XDGConfig(profile=profile) + return xdg.load_complete_config() + + +def fetch_secret(region: str, secret_arn: str) -> BenchlingSecretData: + """Fetch Benchling secret data from AWS Secrets Manager.""" + session = boto3.Session(region_name=region) + sm_client = session.client( + "secretsmanager", + config=BotocoreConfig( + retries={"max_attempts": 3, "mode": "standard"}, + connect_timeout=5, + read_timeout=10, + ), + ) + return fetch_benchling_secret(sm_client, region, secret_arn) + + +def create_benchling_client(secrets: BenchlingSecretData) -> Benchling: + """Create a Benchling SDK client matching the running app's pattern.""" + auth_method = ClientCredentialsOAuth2( + client_id=secrets.client_id, + client_secret=secrets.client_secret, + ) + return Benchling( + url=f"https://{secrets.tenant}.benchling.com", + auth_method=auth_method, + ) + + +# --------------------------------------------------------------------------- +# Candidates +# --------------------------------------------------------------------------- + +Result = dict[str, Any] + + +def _flatten_items(iterator) -> list: + """PageIterator yields pages (List[Model]), not individual items. Flatten.""" + return [item for page in iterator for item in page] + + +def _get_config_items( + benchling: Benchling, + app_id: Optional[str] = None, + config_item_id: Optional[str] = None, +) -> list: + """Fetch config items, flattening PageIterator pages into a single list.""" + if config_item_id: + raw = benchling.apps.get_app_configuration_item_by_id(config_item_id) + return [raw] if raw else [] + kwargs = {} + if app_id is not None: + kwargs["app_id"] = app_id + return _flatten_items(benchling.apps.list_app_configuration_items(**kwargs)) + + +def _item_to_dict(item: Any) -> dict[str, Any]: + """Convert a config item to a plain dict for JSON output.""" + d: dict[str, Any] = { + "id": getattr(item, "id", None), + "path": getattr(item, "path", None), + "type": getattr(item, "type", None), + } + app_attr = getattr(item, "app", None) + if app_attr is not None: + d["app"] = { + "id": getattr(app_attr, "id", None), + # name may be in additional_properties for some SDK versions + "name": getattr(app_attr, "name", app_attr.get("name", None)), + } + raw_value = getattr(item, "value", item.get("value", None)) + if raw_value is not None: + d["value"] = "" if _looks_sensitive(raw_value) else raw_value + return d + + +def try_candidate( + label: str, + benchling: Benchling, + app_id: Optional[str] = None, + config_item_id: Optional[str] = None, +) -> Result: + """Run one candidate and return a structured result dict. + + The dict follows the schema in + spec/392-configurability/05a-list_app_configuration_items.md so it can be + pasted directly into the spec table. + """ + result: Result = { + "candidate": label, + "http_status": None, + "error_message": None, + "items_returned": None, + "first_item": None, + } + + try: + items = _get_config_items(benchling, app_id=app_id, config_item_id=config_item_id) + + result["http_status"] = 200 + result["items_returned"] = len(items) + + if items: + result["first_item"] = _item_to_dict(items[0]) + + except Exception as exc: + result["http_status"] = _extract_http_status(exc) + result["error_message"] = str(exc) + + return result + + +def _extract_http_status(exc: Exception) -> int: + """Try to pull HTTP status code from a Benchling/requests exception.""" + status = getattr(exc, "status_code", None) or getattr(exc, "status", None) + if status: + return int(status) + + msg = str(exc).lower() + if "403" in msg or "forbidden" in msg: + return 403 + if "404" in msg or "not found" in msg: + return 404 + if "401" in msg or "unauthorized" in msg: + return 401 + if "429" in msg or "too many" in msg: + return 429 + return 0 + + +def _looks_sensitive(value: Any) -> bool: + """Heuristic: mask values that look like secrets / long tokens.""" + if not isinstance(value, str): + return False + if len(value) > 40: + return True + sensitive_keywords = ("secret", "token", "key", "password", "credential") + return any(kw in value.lower() for kw in sensitive_keywords) + + +# --------------------------------------------------------------------------- +# Seed / cleanup helpers +# --------------------------------------------------------------------------- + + +def _find_app_by_name(benchling: Benchling, name: str) -> Any: + """Find a BenchlingApp by name. Returns None if not found.""" + for page in benchling.apps.list_apps(): + for app in page: + if app.name == name: + return app + return None + + +def _seed_app_config_items( + benchling: Benchling, + app_id: str, + items: list[dict[str, Any]], + verbose: bool = False, +) -> list[str]: + """Create config items and return their IDs. + + Each item dict must have: + - path: list[str] + - value: str + """ + from benchling_api_client.v2.stable.models.app_config_item_generic_create import ( + AppConfigItemGenericCreate, + ) + from benchling_api_client.v2.stable.models.app_config_item_generic_create_type import ( + AppConfigItemGenericCreateType, + ) + + created_ids: list[str] = [] + for item_def in items: + try: + create = AppConfigItemGenericCreate( + type=AppConfigItemGenericCreateType("text"), + app_id=app_id, + path=item_def["path"], + value=item_def["value"], + ) + result = benchling.apps.create_app_configuration_item(create) + created_ids.append(result.id) + if verbose: + print( + f" ✅ Created config item: {result.id} path={item_def['path']}", + file=sys.stderr, + ) + except Exception as exc: + print( + f" ❌ Failed to create config item path={item_def['path']}: {exc}", + file=sys.stderr, + ) + return created_ids + + +def _cleanup_config_items( + benchling: Benchling, + item_ids: list[str], + verbose: bool = False, +) -> None: + """Note: No SDK delete/archive endpoint for config items yet. + + The Benchling SDK AppService has archive_app_configuration_items + commented out (TODO BNCH-52599). Items must be cleaned up via + the Benchling UI or API directly. + + Logs the item IDs for manual cleanup. + """ + if not item_ids: + return + print( + f" ⚠️ Created {len(item_ids)} config items — SDK has no delete endpoint yet.", + file=sys.stderr, + ) + print( + f" ⚠️ Clean up manually via Benchling UI or API: {item_ids}", + file=sys.stderr, + ) + + +# --------------------------------------------------------------------------- +# Main +# --------------------------------------------------------------------------- + + +def main() -> None: + parser = argparse.ArgumentParser( + description="Probe benchling.apps.list_app_configuration_items to determine the correct app_id.", + ) + parser.add_argument( + "--profile", + default="dev", + help="XDG configuration profile to load (default: dev)", + ) + parser.add_argument( + "--verbose", + "-v", + action="store_true", + help="Print progress messages to stderr", + ) + parser.add_argument( + "--seed", + action="store_true", + help="Create test config items on --seed-app-name (default: quilt-docker) to verify round-trip", + ) + parser.add_argument( + "--seed-app-name", + default="quilt-docker", + help="App name to seed config items on (default: quilt-docker)", + ) + parser.add_argument( + "--no-cleanup", + action="store_true", + help="When --seed, leave items in place after probe (don't archive)", + ) + args = parser.parse_args() + + # --- 1. Load XDG config ------------------------------------------------ + cfg: dict[str, Any] = {} + try: + cfg = load_profile_config(args.profile) + except Exception as exc: + _fail(f"Cannot load profile '{args.profile}': {exc}") + + if args.verbose: + print(f"ℹ️ Loaded config for profile '{args.profile}'", file=sys.stderr) + + # --- 2. Extract needed values from config ------------------------------ + benchling_cfg = cfg.get("benchling", {}) + deployment_cfg = cfg.get("deployment", {}) + + secret_arn: str = benchling_cfg.get("secretArn", "") or benchling_cfg.get("secret_arn", "") + tenant: str = benchling_cfg.get("tenant", "") + app_definition_id: str = benchling_cfg.get("appDefinitionId", "") or benchling_cfg.get( + "app_definition_id", "" + ) + region: str = deployment_cfg.get("region", "") or cfg.get("quilt", {}).get("region", "us-east-1") + + if not secret_arn: + _fail("No benchling.secretArn found in config. Is the profile set up?") + if not tenant: + _fail("No benchling.tenant found in config.") + + # --- 3. Fetch the AWS secret -------------------------------------------- + try: + secrets = fetch_secret(region, secret_arn) + except Exception as exc: + _fail(f"Cannot fetch Benchling secret '{secret_arn}': {exc}") + + if args.verbose: + print( + f"ℹ️ Fetched secret: tenant={secrets.tenant} app_definition_id={secrets.app_definition_id}", + file=sys.stderr, + ) + + # --- 4. Create Benchling client ----------------------------------------- + try: + benchling = create_benchling_client(secrets) + except Exception as exc: + _fail(f"Cannot create Benchling client: {exc}") + + if args.verbose: + print("ℹ️ Benchling client created successfully", file=sys.stderr) + + # List installed apps (always do this for diagnostics) + installed_apps: list[dict[str, Any]] = [] + try: + for page in benchling.apps.list_apps(): + for app in page: + installed_apps.append( + { + "id": app.id, + "name": app.name, + "app_definition_id": getattr(app, "app_definition", None).id + if getattr(app, "app_definition", None) + else None, + } + ) + except Exception as exc: + if args.verbose: + print(f"⚠️ Could not list apps: {exc}", file=sys.stderr) + + # --- 5. Try candidates --------------------------------------------------- + all_results: list[Result] = [] + + # Candidate 1 — appDefinitionId (from the secret) + all_results.append( + try_candidate("appDefinitionId", benchling, app_id=secrets.app_definition_id) + ) + + # Candidate 2 — enumerate installed apps and try each app.id + if installed_apps: + for app_info in installed_apps: + label = f"app.id (name={app_info['name']})" + all_results.append( + try_candidate(label, benchling, app_id=app_info["id"]) + ) + else: + # Fallback: try iterating the PageIterator directly + try: + for page in benchling.apps.list_apps(): + for app in page: + label = f"app.id (name={app.name})" + all_results.append( + try_candidate(label, benchling, app_id=app.id) + ) + except Exception as exc: + all_results.append( + { + "candidate": "list_apps() enumeration", + "http_status": _extract_http_status(exc), + "error_message": f"Cannot enumerate apps: {exc}", + "items_returned": None, + "first_item": None, + } + ) + + # Candidate 3 — no app_id filter (may fail) + all_results.append(try_candidate("no_app_id", benchling, app_id=None)) + + # --- 5b. Seed round-trip test ------------------------------------------- + seeded_item_ids: list[str] = [] + if args.seed: + seed_app = _find_app_by_name(benchling, args.seed_app_name) + if seed_app is None: + print( + f"⚠️ App '{args.seed_app_name}' not found. Skipping seed round-trip.", + file=sys.stderr, + ) + else: + if args.verbose: + print( + f"ℹ️ Seeding config items on app '{seed_app.name}' (id={seed_app.id})", + file=sys.stderr, + ) + + seed_items = [ + {"path": ["probe-test", "bucket"], "value": "probe-test-bucket"}, + {"path": ["probe-test", "prefix"], "value": "probe-test-prefix"}, + ] + + seeded_item_ids = _seed_app_config_items( + benchling, seed_app.id, seed_items, verbose=args.verbose + ) + + if seeded_item_ids: + # List back by app_id to verify retrieval + result = try_candidate( + f"after_seed (name={seed_app.name})", benchling, app_id=seed_app.id + ) + all_results.append(result) + + # Also try get-by-id for each seeded item + for item_id in seeded_item_ids: + all_results.append( + try_candidate(f"get_by_id ({item_id})", benchling, config_item_id=item_id) + ) + + # Clean up unless --no-cleanup + if not args.no_cleanup: + if args.verbose: + print("ℹ️ Cleaning up seeded items...", file=sys.stderr) + _cleanup_config_items(benchling, seeded_item_ids, verbose=args.verbose) + + # --- 6. Print structured JSON ------------------------------------------- + payload = { + "metadata": { + "profile": args.profile, + "tenant": tenant, + "secret_arn": secret_arn, + "app_definition_id": app_definition_id, + "installed_apps": installed_apps, + "seeded_item_ids": seeded_item_ids if args.seed else None, + }, + "results": all_results, + } + + json.dump(payload, sys.stdout, indent=2, default=str) + print() # trailing newline + + # --- 7. Exit code ------------------------------------------------------- + any_ok = any(r.get("http_status") == 200 for r in all_results) + sys.exit(0 if any_ok else 1) + + +def _fail(msg: str) -> None: + print(json.dumps({"status": "error", "message": msg}, indent=2)) + sys.exit(1) + + +if __name__ == "__main__": + main() diff --git a/spec/392-configurability/05b-probe_results.md b/spec/392-configurability/05b-probe_results.md new file mode 100644 index 00000000..51255415 --- /dev/null +++ b/spec/392-configurability/05b-probe_results.md @@ -0,0 +1,158 @@ +# Probe Results: `list_app_configuration_items` + +Run `2026-07-02`, profile `dev`, tenant `quilt-dtt`. + +--- + +## Exec Summary + +| Question | Answer | +|----------|--------| +| Is `appDefinitionId` the same as `app_id`? | **No** — HTTP 400 `invalidId` | +| What is the correct `app_id`? | The **tenanted installation `id`** (format `app_XXXXX`) from `list_apps()` | +| How do we find it at startup? | `list_apps()` → match `app.app_definition.id == secrets.app_definition_id` | +| Can we create config items? | **No** (yet) — the path must match a pre-defined configuration schema/definition | + +--- + +## Probe Script + +Written at [`docker/scripts/probe_config_items.py`](../../docker/scripts/probe_config_items.py). + +Run (from repo root): + +```bash +cd docker && uv run python -m scripts.probe_config_items --profile dev [--seed] +``` + +- `--seed` attempts to create test config items (fails — schema not defined) +- `--verbose` / `-v` prints progress to stderr +- Exits 0 if any candidate succeeded, 1 if all failed + +--- + +## Results + +### Metadata + +```json +{ + "profile": "dev", + "tenant": "quilt-dtt", + "app_definition_id": "appdef_wqFfaXBVMu", + "installed_apps": [ + { "id": "app_lTvdpW99kWi3LWg9", "name": "nightly-quilttest-com", "app_definition_id": "appdef_wqFfaXBVMu" }, + { "id": "app_VJVPr9y2kLZRYrIz", "name": "nightly-quilttest2-com", "app_definition_id": "appdef_17TwAmkFbAO" }, + { "id": "app_oqVe5IOvjAwdjslZ", "name": "open-quiltdata-com", "app_definition_id": "appdef_13k4O0m8gyG" }, + { "id": "app_9pQRdGFci4N1A4RM", "name": "quilt-docker", "app_definition_id": "appdef_15F3yJpAblA" } + ] +} +``` + +**Key:** Our app (`appdef_wqFfaXBVMu`) is installed as `app_lTvdpW99kWi3LWg9` / `nightly-quilttest-com`. + +### Candidate 1 — `appDefinitionId` + +| Field | Value | +|-------|-------| +| Candidate | `appDefinitionId` | +| HTTP status | **400** | +| Error | `appId: appdef_wqFfaXBVMu is invalid` | + +**Verdict:** `appDefinitionId` (format `appdef_XXXXX`) is **not** the `app_id` parameter. The API expects a different ID format. + +### Candidate 2 — `list_apps()` enumeration + +| Field | Value | +|-------|-------| +| Candidate | `app.id (name=nightly-quilttest-com)` | +| HTTP status | **200** | +| Items returned | **0** (not yet seeded) | + +| Field | Value | +|-------|-------| +| Candidate | `app.id (name=nightly-quilttest2-com)` | +| HTTP status | **200** | +| Items returned | **0** | + +| Field | Value | +|-------|-------| +| Candidate | `app.id (name=open-quiltdata-com)` | +| HTTP status | **200** | +| Items returned | **0** | + +| Field | Value | +|-------|-------| +| Candidate | `app.id (name=quilt-docker)` | +| HTTP status | **200** | +| Items returned | **0** | + +**Verdict:** The tenanted installation `id` (format `app_XXXXX`) is the correct `app_id` parameter. All four installed apps responded with HTTP 200, returning 0 items (nothing configured yet). + +### Candidate 3 — No `app_id` filter + +| Field | Value | +|-------|-------| +| Candidate | `no_app_id` | +| HTTP status | **200** | +| Items returned | **0** | + +**Verdict:** Calling without an `app_id` also works and returns config items across all apps. + +### Candidate 4 — Seed round-trip (attempted) + +Creating config items via `AppConfigItemGenericCreate` with arbitrary paths failed: + +``` +BenchlingError: Expected exactly one config to match given path ('probe-test',) +``` + +**Why:** Benchling's app configuration requires a **pre-defined configuration schema**. The `path` must exactly match an existing configuration definition (created via the Benchling UI or a schema API call). Arbitrary paths are rejected. + +--- + +## Conclusions for Phase 1 Implementation + +1. **`app_id` source at startup:** + + Do **not** use `app_definition_id` from the secret directly. Instead: + + ```python + # Find our app by matching app_definition_id + for page in benchling.apps.list_apps(): + for app in page: + if app.app_definition.id == secrets.app_definition_id: + our_app_id = app.id # e.g., "app_lTvdpW99kWi3LWg9" + break + ``` + + This `our_app_id` is the correct value to pass to `list_app_configuration_items(app_id=...)`. + +2. **API compatibility:** + + | Call | Status | Notes | + |------|--------|-------| + | `list_app_configuration_items(app_id=app_XXXXX)` | ✅ 200 | Filtered to one app | + | `list_app_configuration_items(app_id=appdef_XXXXX)` | ❌ 400 | Wrong ID format | + | `list_app_configuration_items()` | ✅ 200 | Returns items for all apps | + +3. **Configuration items:** + + - All apps currently have 0 config items. + - To seed items, the app needs a **configuration schema/definition** first. + - Creating items from scratch via the SDK is blocked until the schema exists. + - Phase 1 implementation should plan for this: either define the schema via the Benchling UI, or create it programmatically during deployment. + +4. **Implementation checklist update:** + + In [`06-configurability-checklist.md`](06-configurability-checklist.md), resolve Task 1: + + > **How to find `app_id`:** List installed apps with `list_apps()`, match `app.app_definition.id` to the `app_definition_id` from the Benchling secret, then use the matched `app.id`. + +--- + +## SDK Gotchas + +- **`PageIterator` yields pages, not items:** Each iteration gives `List[Model]` (a page). Flatten with `[item for page in iterator for item in page]`. +- **`AppConfigItem.app.name` is in `additional_properties`:** The `app` attribute's `name` field may not be a direct property — use `app.get("name")` as fallback. +- **No delete endpoint:** The SDK has `archive_app_configuration_items` commented out (TODO BNCH-52599). Items created during testing must be cleaned up manually via the Benchling UI. diff --git a/spec/392-configurability/06-configurability-checklist.md b/spec/392-configurability/06-configurability-checklist.md index c55c9249..c41fdc7e 100644 --- a/spec/392-configurability/06-configurability-checklist.md +++ b/spec/392-configurability/06-configurability-checklist.md @@ -10,13 +10,13 @@ Phases can be developed independently but **all phases deploy together** in one ## Prerequisite -**Run `05a-list_app_configuration_items.md` first** to determine the correct `app_id` parameter for `benchling.apps.list_app_configuration_items()`. Update this checklist with the result before starting Phase 1. +**Probe complete — see [`05b-probe_results.md`](05b-probe_results.md).** Key finding: `appDefinitionId` (format `appdef_XXXXX`) is NOT the `app_id` for config items. The correct value is the tenanted installation `id` (format `app_XXXXX`), discovered by calling `list_apps()` and matching `app.app_definition.id` to the `app_definition_id` from the Benchling secret. --- ## Phase 1 — App Config Items Read Path (Python runtime) -1. **Add `benchling.apps.list_app_configuration_items()` call** at app startup in Docker Python code. Fetch all config items scoped to our app using the `app_id` identified by the prerequisite probe (likely `appDefinitionId` from the secret, or a separate `app.id` discovered via `list_apps()`). +1. **Add `benchling.apps.list_app_configuration_items()` call** at app startup in Docker Python code. Fetch all config items scoped to our app using the `app.id` matched from `list_apps()` (`app.app_definition.id == secrets.app_definition_id`), **not** the `appDefinitionId` directly. See [`05b-probe_results.md`](05b-probe_results.md) for details. 1. **Build a `RoutingConfig` model** from the config items, keyed by `path` convention: - `["quilt", "routing", "", ""]` — event-type routing From 77a4ae965e5578465d05bfbe27e718c276d343ad Mon Sep 17 00:00:00 2001 From: "Dr. Ernie Prabhakar" Date: Thu, 2 Jul 2026 21:16:55 -0700 Subject: [PATCH 5/7] docs: fix configurability checklist per probe results Address all findings from code review: - Move app-manifest.yaml schema to hard prerequisite (was Phase 5 docs) - Add cache-invalidation task for v2-beta.app.configuration.updated lifecycle events - Fix event type names to match actual app.py (v2.entry.* prefix) - Make config:seed schema-aware with cross-ref to prerequisite - Make config:clear conditional on SDK delete/archive support - Update namespace note to reflect SDK limitations Co-Authored-By: Claude --- .../06-configurability-checklist.md | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/spec/392-configurability/06-configurability-checklist.md b/spec/392-configurability/06-configurability-checklist.md index c41fdc7e..80d3858d 100644 --- a/spec/392-configurability/06-configurability-checklist.md +++ b/spec/392-configurability/06-configurability-checklist.md @@ -12,6 +12,8 @@ Phases can be developed independently but **all phases deploy together** in one **Probe complete — see [`05b-probe_results.md`](05b-probe_results.md).** Key finding: `appDefinitionId` (format `appdef_XXXXX`) is NOT the `app_id` for config items. The correct value is the tenanted installation `id` (format `app_XXXXX`), discovered by calling `list_apps()` and matching `app.app_definition.id` to the `app_definition_id` from the Benchling secret. +**Hard prerequisite: Define and deploy config schema paths in `app-manifest.yaml`.** The Benchling SDK rejects `AppConfigItemGenericCreate` calls whose `path` doesn't match a pre-defined configuration schema/definition (see [`05b-probe_results.md`](05b-probe_results.md)). Before any seeding or config-item writes, the exact App Configuration schema paths/types must be declared in [`docker/app-manifest.yaml`](../../docker/app-manifest.yaml), then the app must be deployed/updated so Benchling registers the schema. Only then will `config:seed` work. The write-side features below assume this is done. + --- ## Phase 1 — App Config Items Read Path (Python runtime) @@ -24,7 +26,7 @@ Phases can be developed independently but **all phases deploy together** in one - `["quilt", "settings", ""]` — global settings (log level, concurrency) - `["quilt", "default", ""]` — fallback values - All paths are namespaced under `"quilt"` so `config:clear` can safely delete only our items. + All paths are namespaced under `"quilt"` so `config:clear` can safely target only our items (SDK has no delete/archive path yet, so `config:clear` may only be an inspect/manual-cleanup helper unless API support is added). 1. **Implement a fallback chain** — for each config value: @@ -44,6 +46,8 @@ Phases can be developed independently but **all phases deploy together** in one - Past TTL, stale value exists: return stale, kick off background refresh thread - The `app_id` is also cached; if it needs to change the container restarts anyway +1. **Invalidate Tier 1 cache on config lifecycle events.** The app already routes `v2-beta.app.configuration.updated` in [`app.py`](../../docker/src/app.py) — wire this handler to flush the Tier 1 cache so config changes take effect immediately instead of waiting up to 60s for TTL expiry. + ## Phase 2 — Per-Project Routing (Python runtime) 1. **Resolve project from entry, but only for entry events with a folder.** Not all webhook events are entry-backed. Gate project lookup on `entry.folder_id` being present: @@ -66,7 +70,7 @@ Phases can be developed independently but **all phases deploy together** in one Project override → Event-type default → Global default → Legacy fallback ``` - **Merge strategy:** field-level merge, not whole-object replace. If a project rule sets only `bucket`, the `prefix` and `workflow` inherit from the event-type or global default. Supported event types: `entry.created`, `entry.updated.fields`, `v2.canvas.userInteracted` (matches existing `supported_event_types` set in `app.py`). + **Merge strategy:** field-level merge, not whole-object replace. If a project rule sets only `bucket`, the `prefix` and `workflow` inherit from the event-type or global default. Supported event types: `v2.entry.created`, `v2.entry.updated.fields`, `v2.entry.updated.reviewRecord` (matches existing `supported_event_types` set in [`app.py`](../../docker/src/app.py)). 1. **Handle project lookup failure gracefully.** If folder/project API calls fail (network error, deleted folder, missing permissions), log the error and use event-type defaults. Must not crash the webhook handler. @@ -82,9 +86,12 @@ Phases can be developed independently but **all phases deploy together** in one ## Phase 4 — CLI: Seed / Clear App Config -1. **Add `npm run config:seed` command.** Reads current routing values from `ProfileConfig` + Secrets Manager, creates equivalent App Configuration Items in Benchling via the SDK. Prints the Benchling UI URL where items can be edited. +1. **Add `npm run config:seed` command.** Reads current routing values from `ProfileConfig` + Secrets Manager, creates/updates equivalent App Configuration Items in Benchling via the SDK. **Prerequisite:** The app's configuration schema paths must be defined (see hard Prerequisite section above) — without them the SDK rejects arbitrary paths. Prints the Benchling UI URL where items can be edited. -1. **Add `npm run config:clear` command.** Deletes only App Configuration Items under the `["quilt"]` path namespace. Requires `--force`. Shows a dry-run preview of what will be deleted before confirming. App falls back to legacy config after items are removed. +1. **Add `npm run config:clear` command** — **only if the Benchling SDK adds a usable delete/archive endpoint.** As of probe date ([`05b-probe_results.md`](05b-probe_results.md)), `archive_app_configuration_items` is commented out (TODO BNCH-52599) and no SDK delete path exists. If support is still absent when implementing: + - Provide a dry-run inspection command (`config:inspect --items`) that lists all items under the `["quilt"]` namespace and prints the Benchling UI URL for manual cleanup. + - Document the manual cleanup process. + - Skip the `--force` / auto-clear path entirely. 1. **Add `npm run config:inspect` command.** Dumps current Tier 1 items and effective resolved config (Tier 1 → secret → env var) for debugging. @@ -97,8 +104,6 @@ Phases can be developed independently but **all phases deploy together** in one - The fallback chain and backward compatibility guarantees - Integrated-mode IAM limitation: Tier 1 routes can only use buckets covered by the Quilt stack's BucketWritePolicy -1. **Add `app-manifest.yaml` entries** for any new App Configuration Items the app expects (declares them to Benchling). - --- ## Test Plan From d1beb2872fbb9d041ae39c388012a08099e33ea6 Mon Sep 17 00:00:00 2001 From: "Dr. Ernie Prabhakar" Date: Thu, 2 Jul 2026 21:34:39 -0700 Subject: [PATCH 6/7] feat: add Benchling app config routing --- CHANGELOG.md | 14 + README.md | 29 ++ bin/cli.ts | 41 +++ bin/commands/app-config.ts | 35 ++ bin/commands/deploy.ts | 8 + docker/scripts/app_config.py | 204 +++++++++++ docker/scripts/probe_config_items.py | 46 +-- docker/src/app.py | 37 +- docker/src/config.py | 82 ++++- docker/src/packaging_consumer.py | 24 +- docker/src/packaging_publisher.py | 15 +- docker/src/routing_config.py | 326 ++++++++++++++++++ docker/src/sqs_consumer.py | 15 +- docker/tests/test_routing_config.py | 64 ++++ lib/benchling-webhook-stack.ts | 1 + lib/fargate-service.ts | 18 +- lib/types/config.ts | 16 + lib/types/stack-config.ts | 13 +- lib/utils/config-transform.ts | 7 +- package.json | 3 + .../06-configurability-checklist.md | 54 +-- 21 files changed, 979 insertions(+), 73 deletions(-) create mode 100644 bin/commands/app-config.ts create mode 100644 docker/scripts/app_config.py create mode 100644 docker/src/routing_config.py create mode 100644 docker/tests/test_routing_config.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d4e61b70..e9f88d9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,20 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +## [0.19.0] - 2026-07-02 + +### Added + +- Benchling App Configuration Items can now drive runtime package routing with a three-tier fallback chain: Benchling App Config, Secrets Manager, then environment/defaults +- Per-event and per-project routing supports field-level overrides for package bucket, prefix, metadata key, and workflow, with routing metadata preserved through the packaging SQS queue +- New `config:inspect`, `config:seed`, and `config:clear` npm commands help inspect effective routing, seed App Configuration Items, and list items that require manual cleanup +- Profile config now supports `packages.extraBuckets` so standalone stacks can grant the ECS task role access to additional routed package buckets + +### Changed + +- Runtime log level and package consumer concurrency can be supplied by Benchling App Configuration Items, falling back to existing values when Tier 1 config is unavailable +- Integrated-stack deploys warn when `packages.extraBuckets` is set, because IAM is owned by the Quilt stack in that mode + ## [0.18.0] - 2026-06-15 ### Added diff --git a/README.md b/README.md index 9adeadd7..caf429a2 100644 --- a/README.md +++ b/README.md @@ -222,3 +222,32 @@ npx @quiltdata/benchling-webhook@latest logs --profile sales # Destroy stack npx @quiltdata/benchling-webhook@latest destroy --profile sales ``` + +## Benchling App Configuration Routing + +Runtime routing uses a three-tier fallback chain: + +```text +Benchling App Configuration Items -> AWS Secrets Manager -> environment/defaults +``` + +App Configuration Items are read under the `["quilt"]` path namespace: + +- `["quilt", "default", "bucket" | "prefix" | "package_key" | "workflow"]` +- `["quilt", "routing", "", ""]` +- `["quilt", "projects", "", ""]` +- `["quilt", "settings", "log_level" | "package_event_concurrency" | "packaging_request_concurrency"]` + +Project routing is merged field-by-field over event defaults, global defaults, and legacy secret values. For example, a project rule that sets only `bucket` inherits `prefix`, `package_key`, and `workflow` from lower-priority tiers. + +CLI helpers: + +```bash +npm run config:inspect -- --profile sales +npm run config:seed -- --profile sales +npm run config:clear -- --profile sales +``` + +`config:seed` requires matching App Configuration schema paths to be defined in the Benchling app manifest and deployed to Benchling first. With the current Benchling SDK, `config:clear` cannot delete/archive items automatically; it lists `["quilt"]` items and points you to manual cleanup in Benchling. + +For standalone stacks, add any routed destination buckets to `packages.extraBuckets` so the ECS task role gets S3 permissions. In `integratedStack: true` mode, the Quilt stack owns IAM, so Tier 1 routes can only use buckets covered by the Quilt stack's `BucketWritePolicy`. diff --git a/bin/cli.ts b/bin/cli.ts index d9293d76..06dd7cdd 100644 --- a/bin/cli.ts +++ b/bin/cli.ts @@ -14,6 +14,7 @@ import { installCommand } from "./commands/install"; import { statusCommand } from "./commands/status"; import { logsCommand } from "./commands/logs"; import { cleanCommand } from "./commands/clean"; +import { appConfigCommand } from "./commands/app-config"; import pkg from "../package.json"; const DEFAULT_LOG_LIMIT = 5; // Number of log entries to show per log group (health checks dominate, so keep this small) @@ -96,6 +97,46 @@ For more information: https://github.com/quiltdata/benchling-webhook#deployment } }); +program + .command("config:inspect") + .description("Inspect Benchling App Configuration Items and effective routing config") + .option("--profile ", "Configuration profile to use (default: default)") + .option("--event-type ", "Event type to resolve for effective routing") + .action(async (options) => { + try { + await appConfigCommand("inspect", options); + } catch (error) { + console.error(chalk.red((error as Error).message)); + process.exit(1); + } + }); + +program + .command("config:seed") + .description("Seed Benchling App Configuration Items from profile and Secrets Manager") + .option("--profile ", "Configuration profile to use (default: default)") + .action(async (options) => { + try { + await appConfigCommand("seed", options); + } catch (error) { + console.error(chalk.red((error as Error).message)); + process.exit(1); + } + }); + +program + .command("config:clear") + .description("List App Configuration Items that require manual cleanup") + .option("--profile ", "Configuration profile to use (default: default)") + .action(async (options) => { + try { + await appConfigCommand("clear", options); + } catch (error) { + console.error(chalk.red((error as Error).message)); + process.exit(1); + } + }); + // Destroy command program .command("destroy") diff --git a/bin/commands/app-config.ts b/bin/commands/app-config.ts new file mode 100644 index 00000000..637d31b7 --- /dev/null +++ b/bin/commands/app-config.ts @@ -0,0 +1,35 @@ +import { spawn } from "child_process"; +import path from "path"; + +interface AppConfigOptions { + profile?: string; + eventType?: string; +} + +export async function appConfigCommand( + command: "inspect" | "seed" | "clear", + options: AppConfigOptions, +): Promise { + const dockerDir = path.resolve(__dirname, "../../docker"); + const args = ["run", "python", "-m", "scripts.app_config", command, "--profile", options.profile || "default"]; + + if (options.eventType) { + args.push("--event-type", options.eventType); + } + + await new Promise((resolve, reject) => { + const child = spawn("uv", args, { + cwd: dockerDir, + stdio: "inherit", + }); + + child.on("error", reject); + child.on("close", code => { + if (code === 0 || (command === "clear" && code === 2)) { + resolve(); + return; + } + reject(new Error(`config:${command} failed with exit code ${code}`)); + }); + }); +} diff --git a/bin/commands/deploy.ts b/bin/commands/deploy.ts index ceba7313..e627c35f 100644 --- a/bin/commands/deploy.ts +++ b/bin/commands/deploy.ts @@ -292,6 +292,14 @@ export async function deploy( ): Promise { // Check if this is an integrated stack - warn and ask user if (config.integratedStack === true) { + if (config.packages.extraBuckets?.length) { + console.warn( + chalk.yellow( + `⚠️ packages.extraBuckets is ignored in integrated mode (${config.packages.extraBuckets.join(", ")}).\n` + + " The Quilt stack owns IAM; make sure its BucketWritePolicy covers every Tier 1 routing bucket.", + ), + ); + } console.log(); console.log(boxen( chalk.yellow.bold("⚠️ Integrated Stack Mode") + "\n\n" + diff --git a/docker/scripts/app_config.py b/docker/scripts/app_config.py new file mode 100644 index 00000000..0c74b21b --- /dev/null +++ b/docker/scripts/app_config.py @@ -0,0 +1,204 @@ +#!/usr/bin/env python3 +from __future__ import annotations + +import argparse +import json +import sys +from pathlib import Path +from typing import Any + +_SCRIPTS_DIR = Path(__file__).resolve().parent +_PROJECT_ROOT = _SCRIPTS_DIR.parent +sys.path.insert(0, str(_PROJECT_ROOT / "src")) + +import boto3 +from benchling_sdk.auth.client_credentials_oauth2 import ClientCredentialsOAuth2 +from benchling_sdk.benchling import Benchling +from botocore.config import Config as BotocoreConfig + +from src.routing_config import ( + ROUTING_NAMESPACE, + RoutingConfig, + find_app_id, + legacy_target_from_secret, +) +from src.secrets_manager import BenchlingSecretData, fetch_benchling_secret +from src.xdg_config import XDGConfig + + +def load_profile(profile: str) -> dict[str, Any]: + return XDGConfig(profile=profile).load_complete_config() + + +def fetch_secret(region: str, secret_name_or_arn: str) -> BenchlingSecretData: + session = boto3.Session(region_name=region) + client = session.client( + "secretsmanager", + config=BotocoreConfig(retries={"max_attempts": 3, "mode": "standard"}, connect_timeout=5, read_timeout=10), + ) + return fetch_benchling_secret(client, region, secret_name_or_arn) + + +def create_benchling_client(secrets: BenchlingSecretData) -> Benchling: + auth_method = ClientCredentialsOAuth2(client_id=secrets.client_id, client_secret=secrets.client_secret) + return Benchling(url=f"https://{secrets.tenant}.benchling.com", auth_method=auth_method) + + +def flatten_pages(iterator: Any) -> list[Any]: + return [item for page in iterator for item in (page if isinstance(page, list) else [page])] + + +def item_path(item: Any) -> list[str]: + path = getattr(item, "path", None) + if path is None and hasattr(item, "additional_properties"): + path = item.additional_properties.get("path") + return [str(part) for part in path] if isinstance(path, list) else [] + + +def item_value(item: Any) -> Any: + if hasattr(item, "value"): + return item.value + if hasattr(item, "additional_properties"): + return item.additional_properties.get("value") + return None + + +def item_to_dict(item: Any) -> dict[str, Any]: + return { + "id": getattr(item, "id", None), + "path": item_path(item), + "type": str(getattr(item, "type", "")), + "value": item_value(item), + } + + +def load_context(profile: str) -> tuple[dict[str, Any], BenchlingSecretData, Benchling, str | None, list[Any]]: + config = load_profile(profile) + region = config.get("deployment", {}).get("region") or config.get("quilt", {}).get("region") + secret_name = config.get("benchling", {}).get("secretArn") + if not region: + raise RuntimeError("Profile is missing deployment.region/quilt.region") + if not secret_name: + raise RuntimeError("Profile is missing benchling.secretArn") + secrets = fetch_secret(region, secret_name) + benchling = create_benchling_client(secrets) + app_id = find_app_id(benchling, secrets.app_definition_id) + items = flatten_pages(benchling.apps.list_app_configuration_items(app_id=app_id)) if app_id else [] + return config, secrets, benchling, app_id, items + + +def seed_items( + config: dict[str, Any], + secrets: BenchlingSecretData, + benchling: Benchling, + app_id: str, + existing_items: list[Any], +) -> dict[str, list[str]]: + from benchling_api_client.v2.stable.models.app_config_item_generic_create import AppConfigItemGenericCreate + from benchling_api_client.v2.stable.models.app_config_item_generic_create_type import ( + AppConfigItemGenericCreateType, + ) + from benchling_api_client.v2.stable.models.app_config_item_generic_update import AppConfigItemGenericUpdate + from benchling_api_client.v2.stable.models.app_config_item_generic_update_type import ( + AppConfigItemGenericUpdateType, + ) + + packages = config.get("packages", {}) + logging_config = config.get("logging", {}) + seed_defs = [ + (["quilt", "default", "bucket"], packages.get("bucket") or secrets.user_bucket), + (["quilt", "default", "prefix"], packages.get("prefix") or secrets.pkg_prefix or "benchling"), + (["quilt", "default", "package_key"], packages.get("metadataKey") or secrets.pkg_key or "experiment_id"), + (["quilt", "default", "workflow"], packages.get("workflow") or secrets.workflow or ""), + (["quilt", "settings", "log_level"], logging_config.get("level") or secrets.log_level or "INFO"), + (["quilt", "settings", "package_event_concurrency"], "5"), + (["quilt", "settings", "packaging_request_concurrency"], "5"), + ] + + existing_by_path = {tuple(item_path(item)): getattr(item, "id", None) for item in existing_items} + created: list[str] = [] + updated: list[str] = [] + for path, value in seed_defs: + existing_id = existing_by_path.get(tuple(path)) + if existing_id: + update = AppConfigItemGenericUpdate(AppConfigItemGenericUpdateType("text"), str(value)) + benchling.apps.update_app_configuration_item(str(existing_id), update) + updated.append(str(existing_id)) + else: + create = AppConfigItemGenericCreate(AppConfigItemGenericCreateType("text"), app_id, path, str(value)) + result = benchling.apps.create_app_configuration_item(create) + created.append(str(getattr(result, "id", ""))) + return {"created_ids": created, "updated_ids": updated} + + +def command_inspect(args: argparse.Namespace) -> int: + _config, secrets, _benchling, app_id, items = load_context(args.profile) + quilt_items = [item for item in items if item_path(item)[:1] == [ROUTING_NAMESPACE]] + routing_config = RoutingConfig.from_app_configuration_items(quilt_items) + legacy = legacy_target_from_secret(secrets) + effective = routing_config.resolve(event_type=args.event_type or "", legacy=legacy) + output = { + "profile": args.profile, + "app_id": app_id, + "items": [item_to_dict(item) for item in quilt_items], + "effective_default": effective.target.to_dict(), + "routing": { + "event_routes": {key: value.to_dict() for key, value in routing_config.event_routes.items()}, + "project_routes": {key: value.to_dict() for key, value in routing_config.project_routes.items()}, + "default": routing_config.default.to_dict(), + "settings": routing_config.settings.__dict__, + }, + } + print(json.dumps(output, indent=2, default=str)) + return 0 + + +def command_seed(args: argparse.Namespace) -> int: + config, secrets, benchling, app_id, items = load_context(args.profile) + if not app_id: + raise RuntimeError("Could not resolve Benchling app id") + try: + result = seed_items(config, secrets, benchling, app_id, items) + except Exception as exc: + raise RuntimeError( + "Failed to seed App Configuration Items. Confirm docker/app-manifest.yaml " + "defines the exact config schema paths before running config:seed." + ) from exc + print(json.dumps({"profile": args.profile, "app_id": app_id, **result}, indent=2)) + return 0 + + +def command_clear(args: argparse.Namespace) -> int: + _config, _secrets, _benchling, app_id, items = load_context(args.profile) + quilt_items = [item for item in items if item_path(item)[:1] == [ROUTING_NAMESPACE]] + print( + json.dumps( + {"profile": args.profile, "app_id": app_id, "items": [item_to_dict(item) for item in quilt_items]}, + indent=2, + default=str, + ) + ) + print( + "Automatic clear is unavailable: Benchling SDK 1.23.1 exposes no usable delete/archive endpoint " + "for App Configuration Items. Remove these items in the Benchling UI.", + file=sys.stderr, + ) + return 2 + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser() + parser.add_argument("command", choices=["inspect", "seed", "clear"]) + parser.add_argument("--profile", default="default") + parser.add_argument("--event-type") + args = parser.parse_args(argv) + + if args.command == "inspect": + return command_inspect(args) + if args.command == "seed": + return command_seed(args) + return command_clear(args) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/docker/scripts/probe_config_items.py b/docker/scripts/probe_config_items.py index d37c12b6..ef765f69 100755 --- a/docker/scripts/probe_config_items.py +++ b/docker/scripts/probe_config_items.py @@ -230,16 +230,17 @@ def _seed_app_config_items( for item_def in items: try: create = AppConfigItemGenericCreate( - type=AppConfigItemGenericCreateType("text"), - app_id=app_id, - path=item_def["path"], - value=item_def["value"], + AppConfigItemGenericCreateType("text"), + app_id, + item_def["path"], + item_def["value"], ) result = benchling.apps.create_app_configuration_item(create) - created_ids.append(result.id) + result_id = str(getattr(result, "id", "")) + created_ids.append(result_id) if verbose: print( - f" ✅ Created config item: {result.id} path={item_def['path']}", + f" ✅ Created config item: {result_id} path={item_def['path']}", file=sys.stderr, ) except Exception as exc: @@ -328,9 +329,7 @@ def main() -> None: secret_arn: str = benchling_cfg.get("secretArn", "") or benchling_cfg.get("secret_arn", "") tenant: str = benchling_cfg.get("tenant", "") - app_definition_id: str = benchling_cfg.get("appDefinitionId", "") or benchling_cfg.get( - "app_definition_id", "" - ) + app_definition_id: str = benchling_cfg.get("appDefinitionId", "") or benchling_cfg.get("app_definition_id", "") region: str = deployment_cfg.get("region", "") or cfg.get("quilt", {}).get("region", "us-east-1") if not secret_arn: @@ -364,13 +363,12 @@ def main() -> None: try: for page in benchling.apps.list_apps(): for app in page: + app_definition = getattr(app, "app_definition", None) installed_apps.append( { "id": app.id, "name": app.name, - "app_definition_id": getattr(app, "app_definition", None).id - if getattr(app, "app_definition", None) - else None, + "app_definition_id": getattr(app_definition, "id", None), } ) except Exception as exc: @@ -381,26 +379,20 @@ def main() -> None: all_results: list[Result] = [] # Candidate 1 — appDefinitionId (from the secret) - all_results.append( - try_candidate("appDefinitionId", benchling, app_id=secrets.app_definition_id) - ) + all_results.append(try_candidate("appDefinitionId", benchling, app_id=secrets.app_definition_id)) # Candidate 2 — enumerate installed apps and try each app.id if installed_apps: for app_info in installed_apps: label = f"app.id (name={app_info['name']})" - all_results.append( - try_candidate(label, benchling, app_id=app_info["id"]) - ) + all_results.append(try_candidate(label, benchling, app_id=app_info["id"])) else: # Fallback: try iterating the PageIterator directly try: for page in benchling.apps.list_apps(): for app in page: label = f"app.id (name={app.name})" - all_results.append( - try_candidate(label, benchling, app_id=app.id) - ) + all_results.append(try_candidate(label, benchling, app_id=app.id)) except Exception as exc: all_results.append( { @@ -436,22 +428,16 @@ def main() -> None: {"path": ["probe-test", "prefix"], "value": "probe-test-prefix"}, ] - seeded_item_ids = _seed_app_config_items( - benchling, seed_app.id, seed_items, verbose=args.verbose - ) + seeded_item_ids = _seed_app_config_items(benchling, seed_app.id, seed_items, verbose=args.verbose) if seeded_item_ids: # List back by app_id to verify retrieval - result = try_candidate( - f"after_seed (name={seed_app.name})", benchling, app_id=seed_app.id - ) + result = try_candidate(f"after_seed (name={seed_app.name})", benchling, app_id=seed_app.id) all_results.append(result) # Also try get-by-id for each seeded item for item_id in seeded_item_ids: - all_results.append( - try_candidate(f"get_by_id ({item_id})", benchling, config_item_id=item_id) - ) + all_results.append(try_candidate(f"get_by_id ({item_id})", benchling, config_item_id=item_id)) # Clean up unless --no-cleanup if not args.no_cleanup: diff --git a/docker/src/app.py b/docker/src/app.py index bf2a2efa..8023af54 100644 --- a/docker/src/app.py +++ b/docker/src/app.py @@ -412,7 +412,9 @@ def create_benchling_client(): client_id=secrets.client_id, client_secret=secrets.client_secret, ) - return Benchling(url=f"https://{secrets.tenant}.benchling.com", auth_method=auth_method) + client = Benchling(url=f"https://{secrets.tenant}.benchling.com", auth_method=auth_method) + config.apply_global_routing_config(client, secrets) + return client # Create initial Benchling client and entry packager for startup validation # If secrets are unavailable, record the problem and continue in degraded mode @@ -710,6 +712,31 @@ def _send_updating_canvas_best_effort(payload: Payload) -> None: error=str(exc), ) + def _resolve_payload_routing(payload: Payload): + assert benchling is not None and config is not None + secrets = config.get_benchling_secrets() + config.apply_benchling_secrets(secrets) + try: + entry_id = payload.entry_id + except ValueError: + entry_id = None + routing = config.resolve_effective_routing( + benchling=benchling, + secret_data=secrets, + event_type=payload.event_type, + entry_id=entry_id, + ) + logger.info( + "Resolved routing for webhook event", + event_type=payload.event_type, + entry_id=entry_id, + routing_source=routing.source, + project_name=routing.project_name, + bucket=routing.target.bucket, + prefix=routing.target.prefix, + ) + return routing + async def _handle_event_impl(request: Request, _verified: None = None): """Shared event webhook handling implementation.""" try: @@ -738,7 +765,8 @@ async def _handle_event_impl(request: Request, _verified: None = None): canvas_id=payload.canvas_id, ) _send_updating_canvas_best_effort(payload) - publish_packaging_request(entry_packager.sqs_client, packaging_queue_url, payload) + routing = _resolve_payload_routing(payload) + publish_packaging_request(entry_packager.sqs_client, packaging_queue_url, payload, routing) logger.info("Canvas update initiated from /event endpoint", canvas_id=payload.canvas_id) return JSONResponse( @@ -758,7 +786,8 @@ async def _handle_event_impl(request: Request, _verified: None = None): "message": f"Event type {payload.event_type} not processed", } - publish_packaging_request(entry_packager.sqs_client, packaging_queue_url, payload) + routing = _resolve_payload_routing(payload) + publish_packaging_request(entry_packager.sqs_client, packaging_queue_url, payload, routing) logger.info( "Entry event processed - packaging request enqueued", @@ -852,6 +881,8 @@ def handle_app_deactivated(payload): def handle_app_configuration_updated(payload): logger.info("App configuration updated", installation_id=payload.get("installationId")) + if config is not None: + config.invalidate_routing_config() return {"status": "success", "message": "Configuration updated successfully"} async def _handle_canvas_impl(request: Request, _verified: None = None): diff --git a/docker/src/config.py b/docker/src/config.py index a75cb73d..6b75365c 100644 --- a/docker/src/config.py +++ b/docker/src/config.py @@ -2,12 +2,19 @@ import threading import time from dataclasses import dataclass, field -from typing import Optional +from typing import Any, Optional import boto3 import structlog from botocore.config import Config as BotocoreConfig +from .routing_config import ( + EffectiveRouting, + RoutingConfig, + RoutingConfigCache, + apply_routing_to_config, + legacy_target_from_secret, +) from .secrets_manager import BenchlingSecretData, fetch_benchling_secret # Default TTL for cached secrets (seconds). @@ -52,6 +59,8 @@ class Config: pkg_prefix: str = "" workflow: str = "" quilt_write_role_arn: str = "" + package_event_concurrency: int = 5 + packaging_request_concurrency: int = 5 # Secret fetching infrastructure (not the secrets themselves) _benchling_secret_name: str = "" @@ -64,6 +73,7 @@ class Config: _cache_ttl: float = SECRETS_CACHE_TTL_SECONDS _refresh_lock: threading.Lock = field(default_factory=threading.Lock, repr=False) _refresh_in_progress: bool = False + _routing_cache: RoutingConfigCache = field(default_factory=RoutingConfigCache, repr=False) def __post_init__(self): """Initialize configuration from environment variables. @@ -318,6 +328,76 @@ def apply_benchling_secrets(self, secret_data: BenchlingSecretData) -> None: if secret_data.log_level: self.log_level = secret_data.log_level + def invalidate_routing_config(self) -> None: + """Invalidate cached Benchling App Configuration Items.""" + self._routing_cache.invalidate() + + def get_routing_config(self, benchling: Any, secret_data: BenchlingSecretData) -> RoutingConfig: + """Fetch Tier 1 routing config with TTL/stale-while-refresh cache.""" + try: + return self._routing_cache.get(benchling, secret_data.app_definition_id) + except Exception as exc: + logger.warning( + "Failed to fetch Tier 1 routing config; using legacy fallback", + error=str(exc), + error_type=type(exc).__name__, + ) + return RoutingConfig.empty() + + def apply_global_routing_config(self, benchling: Any, secret_data: BenchlingSecretData) -> None: + """Apply non-project Tier 1 defaults/settings to the process config.""" + routing_config = self.get_routing_config(benchling, secret_data) + legacy = legacy_target_from_secret(secret_data) + effective = routing_config.resolve(event_type="", legacy=legacy) + apply_routing_to_config(self, effective) + if routing_config.settings.log_level: + self.log_level = routing_config.settings.log_level + if routing_config.settings.package_event_concurrency: + self.package_event_concurrency = routing_config.settings.package_event_concurrency + if routing_config.settings.packaging_request_concurrency: + self.packaging_request_concurrency = routing_config.settings.packaging_request_concurrency + + def resolve_effective_routing( + self, + *, + benchling: Any, + secret_data: BenchlingSecretData, + event_type: str, + entry_id: str | None = None, + ) -> EffectiveRouting: + """Resolve project/event/default routing for one webhook event.""" + routing_config = self.get_routing_config(benchling, secret_data) + project_name = self._resolve_project_name(benchling, entry_id) + return routing_config.resolve( + event_type=event_type, + legacy=legacy_target_from_secret(secret_data), + project_name=project_name, + ) + + def _resolve_project_name(self, benchling: Any, entry_id: str | None) -> str | None: + if not entry_id: + return None + try: + entry = benchling.entries.get_by_id(entry_id) + folder_id = getattr(entry, "folder_id", None) + if not folder_id: + return None + folder = benchling.folders.get_by_id(folder_id) + project_id = getattr(folder, "project_id", None) + if not project_id: + return None + project = benchling.projects.get_by_id(project_id) + project_name = getattr(project, "name", None) + return project_name if isinstance(project_name, str) else None + except Exception as exc: + logger.warning( + "Project lookup failed; using event/default routing", + entry_id=entry_id, + error=str(exc), + error_type=type(exc).__name__, + ) + return None + def get_config() -> Config: """Get configuration instance with on-demand secret fetching. diff --git a/docker/src/packaging_consumer.py b/docker/src/packaging_consumer.py index 9b787b1f..013e286e 100644 --- a/docker/src/packaging_consumer.py +++ b/docker/src/packaging_consumer.py @@ -20,9 +20,11 @@ from .entry_packager import EntryPackager from .payload import Payload +from .routing_config import EffectiveRouting, routed_config_copy from .sqs_consumer import ( BaseSqsConsumer, build_sqs_client, + configured_concurrency, create_benchling_client, wait_for_ready_config, ) @@ -78,10 +80,23 @@ async def process_message(self, message: dict[str, Any]) -> None: ) return - payload = Payload(raw, benchling=self.entry_packager.benchling) + routing = None + payload_body = raw + if isinstance(raw.get("webhook"), dict): + payload_body = raw["webhook"] + routing = EffectiveRouting.from_queue_metadata(raw.get("routing")) + + payload = Payload(payload_body, benchling=self.entry_packager.benchling) entry_id = payload.entry_id - await asyncio.to_thread(self.entry_packager.execute_workflow, payload) + entry_packager = self.entry_packager + if routing: + entry_packager = EntryPackager( + benchling=self.entry_packager.benchling, + config=routed_config_copy(self.entry_packager.config, routing), + ) + + await asyncio.to_thread(entry_packager.execute_workflow, payload) outcome = "success" should_delete = True except asyncio.CancelledError: @@ -132,11 +147,10 @@ async def main() -> int: logger.info("Packaging consumer stopped before Benchling secrets became available") return 0 + benchling = create_benchling_client(config) sqs_client = build_sqs_client(config.aws_region) - concurrency = int(os.getenv("PACKAGING_REQUEST_CONCURRENCY", "5")) + concurrency = configured_concurrency(config.packaging_request_concurrency, "PACKAGING_REQUEST_CONCURRENCY") graceful_timeout = int(os.getenv("PACKAGING_REQUEST_GRACEFUL_TIMEOUT", "30")) - - benchling = create_benchling_client(config) entry_packager = EntryPackager(benchling=benchling, config=config) consumer = PackagingConsumer( diff --git a/docker/src/packaging_publisher.py b/docker/src/packaging_publisher.py index db94ba1f..9d8216e8 100644 --- a/docker/src/packaging_publisher.py +++ b/docker/src/packaging_publisher.py @@ -17,6 +17,7 @@ import structlog from .payload import Payload +from .routing_config import EffectiveRouting logger = structlog.get_logger(__name__) @@ -42,6 +43,7 @@ def publish_packaging_request( sqs_client: Any, queue_url: str, payload: Payload, + routing: EffectiveRouting | None = None, ) -> str: """Send a packaging request to the FIFO queue keyed by entry_id. @@ -58,7 +60,16 @@ def publish_packaging_request( can return 5xx and Benchling will retry. """ entry_id = payload.entry_id # raises ValueError if missing — let it propagate - body = json.dumps(payload.raw_payload) + if routing: + body = json.dumps( + { + "webhook": payload.raw_payload, + "routing": routing.to_queue_metadata(), + }, + default=str, + ) + else: + body = json.dumps(payload.raw_payload) response = sqs_client.send_message( QueueUrl=queue_url, @@ -74,6 +85,8 @@ def publish_packaging_request( entry_id=entry_id, canvas_id=payload.canvas_id, event_type=payload.event_type, + routing_source=routing.source if routing else "legacy", + routing_project=routing.project_name if routing else None, sqs_message_id=message_id, ) return message_id diff --git a/docker/src/routing_config.py b/docker/src/routing_config.py new file mode 100644 index 00000000..9a98b581 --- /dev/null +++ b/docker/src/routing_config.py @@ -0,0 +1,326 @@ +from __future__ import annotations + +import copy +import threading +import time +from dataclasses import dataclass +from typing import Any + +import structlog + +from .secrets_manager import BenchlingSecretData + +logger = structlog.get_logger(__name__) + +ROUTING_NAMESPACE = "quilt" +ROUTING_CACHE_TTL_SECONDS = 60 + + +@dataclass(frozen=True) +class RoutingTarget: + bucket: str | None = None + prefix: str | None = None + package_key: str | None = None + workflow: str | None = None + + @classmethod + def from_mapping(cls, values: dict[str, Any]) -> "RoutingTarget": + return cls( + bucket=_string_value(values, "bucket", "s3_bucket_name", "user_bucket"), + prefix=_string_value(values, "prefix", "s3_prefix", "pkg_prefix"), + package_key=_string_value(values, "package_key", "metadata_key", "pkg_key", "metadataKey"), + workflow=_string_value(values, "workflow"), + ) + + def merge(self, lower: "RoutingTarget") -> "RoutingTarget": + return RoutingTarget( + bucket=self.bucket or lower.bucket, + prefix=self.prefix or lower.prefix, + package_key=self.package_key or lower.package_key, + workflow=self.workflow if self.workflow is not None else lower.workflow, + ) + + def to_dict(self) -> dict[str, str]: + result: dict[str, str] = {} + if isinstance(self.bucket, str): + result["bucket"] = self.bucket + if isinstance(self.prefix, str): + result["prefix"] = self.prefix + if isinstance(self.package_key, str): + result["package_key"] = self.package_key + if isinstance(self.workflow, str): + result["workflow"] = self.workflow + return result + + +@dataclass(frozen=True) +class RuntimeSettings: + log_level: str | None = None + package_event_concurrency: int | None = None + packaging_request_concurrency: int | None = None + + +@dataclass(frozen=True) +class EffectiveRouting: + target: RoutingTarget + source: str + project_name: str | None = None + + def to_queue_metadata(self) -> dict[str, Any]: + return { + "source": self.source, + "project_name": self.project_name, + "target": self.target.to_dict(), + } + + @classmethod + def from_queue_metadata(cls, metadata: Any) -> "EffectiveRouting | None": + if not isinstance(metadata, dict): + return None + target = metadata.get("target") + if not isinstance(target, dict): + return None + return cls( + target=RoutingTarget.from_mapping(target), + source=str(metadata.get("source") or "queue"), + project_name=metadata.get("project_name") if isinstance(metadata.get("project_name"), str) else None, + ) + + +@dataclass(frozen=True) +class RoutingConfig: + event_routes: dict[str, RoutingTarget] + project_routes: dict[str, RoutingTarget] + default: RoutingTarget + settings: RuntimeSettings + + @classmethod + def empty(cls) -> "RoutingConfig": + return cls(event_routes={}, project_routes={}, default=RoutingTarget(), settings=RuntimeSettings()) + + @classmethod + def from_app_configuration_items(cls, items: list[Any]) -> "RoutingConfig": + nested: dict[str, Any] = {} + for item in items: + path = _item_path(item) + if len(path) < 3 or path[0] != ROUTING_NAMESPACE: + continue + _assign_path(nested, path[1:], _item_value(item)) + + return cls( + event_routes={ + event_type: RoutingTarget.from_mapping(values) + for event_type, values in nested.get("routing", {}).items() + if isinstance(values, dict) + }, + project_routes={ + project_name: RoutingTarget.from_mapping(values) + for project_name, values in nested.get("projects", {}).items() + if isinstance(values, dict) + }, + default=RoutingTarget.from_mapping(nested.get("default", {})), + settings=_settings_from_mapping(nested.get("settings", {})), + ) + + def is_empty(self) -> bool: + return ( + not self.event_routes + and not self.project_routes + and not self.default.to_dict() + and self.settings == RuntimeSettings() + ) + + def resolve( + self, + *, + event_type: str, + legacy: RoutingTarget, + project_name: str | None = None, + ) -> EffectiveRouting: + target = self.default.merge(legacy) + source = "global_default" if self.default.to_dict() else "legacy" + + event_route = self.event_routes.get(event_type) + if event_route: + target = event_route.merge(target) + source = "event" + + project_route = self.project_routes.get(project_name or "") + if project_route: + target = project_route.merge(target) + source = "project" + + return EffectiveRouting(target=target, source=source, project_name=project_name) + + +class RoutingConfigCache: + def __init__(self, ttl: float = ROUTING_CACHE_TTL_SECONDS): + self.ttl = ttl + self.app_id: str | None = None + self.value: RoutingConfig | None = None + self.timestamp = 0.0 + self.lock = threading.Lock() + self.refresh_in_progress = False + + def invalidate(self) -> None: + with self.lock: + self.value = None + self.timestamp = 0.0 + self.refresh_in_progress = False + + def get(self, benchling: Any, app_definition_id: str) -> RoutingConfig: + now = time.monotonic() + if self.value is not None and now - self.timestamp < self.ttl: + return self.value + + if self.value is not None: + with self.lock: + if not self.refresh_in_progress: + self.refresh_in_progress = True + thread = threading.Thread( + target=self._refresh_background, + args=(benchling, app_definition_id), + daemon=True, + ) + thread.start() + return self.value + + return self._fetch(benchling, app_definition_id) + + def _refresh_background(self, benchling: Any, app_definition_id: str) -> None: + try: + self._fetch(benchling, app_definition_id) + except Exception as exc: + logger.warning("Tier 1 config refresh failed; serving stale config", error=str(exc)) + finally: + self.refresh_in_progress = False + + def _fetch(self, benchling: Any, app_definition_id: str) -> RoutingConfig: + app_id = self.app_id or find_app_id(benchling, app_definition_id) + if not app_id: + logger.warning("Could not resolve Benchling app id for Tier 1 config", app_definition_id=app_definition_id) + config = RoutingConfig.empty() + else: + items = _flatten_pages(benchling.apps.list_app_configuration_items(app_id=app_id)) + config = RoutingConfig.from_app_configuration_items(items) + self.app_id = app_id + + self.value = config + self.timestamp = time.monotonic() + logger.info( + "Tier 1 routing config cached", + app_id=self.app_id, + event_routes=len(config.event_routes), + project_routes=len(config.project_routes), + has_default=bool(config.default.to_dict()), + ) + return config + + +def find_app_id(benchling: Any, app_definition_id: str) -> str | None: + for app in _flatten_pages(benchling.apps.list_apps()): + app_definition = getattr(app, "app_definition", None) + candidate = getattr(app_definition, "id", None) + if candidate == app_definition_id: + return getattr(app, "id", None) + return None + + +def legacy_target_from_secret(secret_data: BenchlingSecretData) -> RoutingTarget: + return RoutingTarget( + bucket=_optional_string(getattr(secret_data, "user_bucket", None)), + prefix=_optional_string(getattr(secret_data, "pkg_prefix", None)) or "benchling", + package_key=_optional_string(getattr(secret_data, "pkg_key", None)) or "experiment_id", + workflow=_optional_string(getattr(secret_data, "workflow", None)) or "", + ) + + +def apply_routing_to_config(config: Any, routing: EffectiveRouting) -> None: + target = routing.target + if target.bucket: + config.s3_bucket_name = target.bucket + if target.prefix: + config.s3_prefix = target.prefix + config.pkg_prefix = target.prefix + if target.package_key: + config.package_key = target.package_key + if target.workflow is not None: + config.workflow = target.workflow + + +def routed_config_copy(config: Any, routing: EffectiveRouting) -> Any: + clone = copy.copy(config) + apply_routing_to_config(clone, routing) + return clone + + +def _flatten_pages(iterator: Any) -> list[Any]: + result: list[Any] = [] + for page in iterator: + if isinstance(page, list): + result.extend(page) + else: + result.append(page) + return result + + +def _item_path(item: Any) -> list[str]: + path = getattr(item, "path", None) + if path is None and hasattr(item, "additional_properties"): + path = item.additional_properties.get("path") + if isinstance(path, list): + return [str(part) for part in path] + return [] + + +def _item_value(item: Any) -> Any: + if hasattr(item, "value"): + return item.value + if hasattr(item, "additional_properties") and "value" in item.additional_properties: + return item.additional_properties["value"] + return None + + +def _assign_path(target: dict[str, Any], path: list[str], value: Any) -> None: + node = target + for part in path[:-1]: + child = node.setdefault(part, {}) + if not isinstance(child, dict): + return + node = child + node[path[-1]] = value + + +def _string_value(values: dict[str, Any], *keys: str) -> str | None: + for key in keys: + value = values.get(key) + cleaned = _optional_string(value) + if cleaned is not None: + return cleaned + return None + + +def _optional_string(value: Any) -> str | None: + if isinstance(value, str): + return value + return None + + +def _settings_from_mapping(values: Any) -> RuntimeSettings: + if not isinstance(values, dict): + return RuntimeSettings() + return RuntimeSettings( + log_level=_string_value(values, "log_level", "logLevel"), + package_event_concurrency=_int_value(values.get("package_event_concurrency")), + packaging_request_concurrency=_int_value(values.get("packaging_request_concurrency")), + ) + + +def _int_value(value: Any) -> int | None: + if value is None: + return None + try: + parsed = int(value) + except (TypeError, ValueError): + return None + return parsed if parsed > 0 else None diff --git a/docker/src/sqs_consumer.py b/docker/src/sqs_consumer.py index b05538d7..2d0b1db9 100644 --- a/docker/src/sqs_consumer.py +++ b/docker/src/sqs_consumer.py @@ -33,6 +33,15 @@ READY_WAIT_MAX_SECONDS = 300 +def configured_concurrency(value: Any, env_var: str, default: int = 5) -> int: + if isinstance(value, int) and value > 0: + return value + try: + return int(os.getenv(env_var, str(default))) + except ValueError: + return default + + class PackageEventParseError(ValueError): """Raised when an SQS message cannot be parsed as a package event.""" @@ -79,7 +88,9 @@ def create_benchling_client(config: Config) -> Benchling: client_id=secrets.client_id, client_secret=secrets.client_secret, ) - return Benchling(url=f"https://{secrets.tenant}.benchling.com", auth_method=auth_method) + benchling = Benchling(url=f"https://{secrets.tenant}.benchling.com", auth_method=auth_method) + config.apply_global_routing_config(benchling, secrets) + return benchling class BaseSqsConsumer: @@ -355,7 +366,7 @@ async def main() -> int: return 0 sqs_client = build_sqs_client(config.aws_region) - concurrency = int(os.getenv("PACKAGE_EVENT_CONCURRENCY", "5")) + concurrency = configured_concurrency(config.package_event_concurrency, "PACKAGE_EVENT_CONCURRENCY") graceful_timeout = int(os.getenv("PACKAGE_EVENT_GRACEFUL_TIMEOUT", "30")) consumer = SqsConsumer( diff --git a/docker/tests/test_routing_config.py b/docker/tests/test_routing_config.py new file mode 100644 index 00000000..2dcba26a --- /dev/null +++ b/docker/tests/test_routing_config.py @@ -0,0 +1,64 @@ +from types import SimpleNamespace + +from src.routing_config import ( + EffectiveRouting, + RoutingConfig, + RoutingTarget, + find_app_id, +) + + +def item(path, value): + return SimpleNamespace(path=path, value=value, id="/".join(path), type="text") + + +def test_routing_config_parses_namespaced_items(): + config = RoutingConfig.from_app_configuration_items( + [ + item(["quilt", "default", "bucket"], "default-bucket"), + item(["quilt", "routing", "v2.entry.created", "prefix"], "created"), + item(["quilt", "projects", "Study A", "workflow"], "study-workflow"), + item(["other", "default", "bucket"], "ignored"), + ] + ) + + assert config.default.bucket == "default-bucket" + assert config.event_routes["v2.entry.created"].prefix == "created" + assert config.project_routes["Study A"].workflow == "study-workflow" + + +def test_routing_resolution_field_level_merge_precedence(): + config = RoutingConfig( + event_routes={"v2.entry.created": RoutingTarget(prefix="created-prefix")}, + project_routes={"Study A": RoutingTarget(bucket="project-bucket")}, + default=RoutingTarget(bucket="default-bucket", prefix="default-prefix", package_key="experiment_id"), + settings=RoutingConfig.empty().settings, + ) + + effective = config.resolve( + event_type="v2.entry.created", + project_name="Study A", + legacy=RoutingTarget(bucket="legacy-bucket", prefix="legacy-prefix", package_key="legacy_key"), + ) + + assert effective.target.bucket == "project-bucket" + assert effective.target.prefix == "created-prefix" + assert effective.target.package_key == "experiment_id" + assert effective.source == "project" + + +def test_queue_metadata_round_trip(): + routing = EffectiveRouting( + target=RoutingTarget(bucket="bucket", prefix="prefix", package_key="key", workflow="flow"), + source="project", + project_name="Study A", + ) + + assert EffectiveRouting.from_queue_metadata(routing.to_queue_metadata()) == routing + + +def test_find_app_id_matches_app_definition_id(): + app = SimpleNamespace(id="app_123", app_definition=SimpleNamespace(id="appdef_123")) + benchling = SimpleNamespace(apps=SimpleNamespace(list_apps=lambda: [[app]])) + + assert find_app_id(benchling, "appdef_123") == "app_123" diff --git a/lib/benchling-webhook-stack.ts b/lib/benchling-webhook-stack.ts index d3b988a7..2e616038 100644 --- a/lib/benchling-webhook-stack.ts +++ b/lib/benchling-webhook-stack.ts @@ -322,6 +322,7 @@ export class BenchlingWebhookStack extends cdk.Stack { // IAM managed policy ARNs for S3 and Athena access bucketWritePolicyArn: config.quilt.bucketWritePolicyArn, athenaUserPolicyArn: config.quilt.athenaUserPolicyArn, + extraPackageBuckets: config.packages?.extraBuckets, packageEventQueue, packagingRequestQueue, // Legacy parameters diff --git a/lib/fargate-service.ts b/lib/fargate-service.ts index b7d3b1b4..272a4c69 100644 --- a/lib/fargate-service.ts +++ b/lib/fargate-service.ts @@ -40,6 +40,7 @@ export interface FargateServiceProps { // NEW: Optional IAM managed policy ARNs (from Quilt stack discovery) readonly bucketWritePolicyArn?: string; readonly athenaUserPolicyArn?: string; + readonly extraPackageBuckets?: string[]; readonly packageEventQueue?: sqs.IQueue; readonly packagingRequestQueue?: sqs.IQueue; @@ -147,9 +148,15 @@ export class FargateService extends Construct { }), ); - // Grant S3 access for the specific package bucket - // Full Quilt-required permissions for versioned S3 objects - const packageBucketArn = `arn:aws:s3:::${props.packageBucket}`; + // Grant S3 access for the primary package bucket and any additional + // existing buckets used by Tier 1 routing. + // Full Quilt-required permissions for versioned S3 objects. + const packageBucketArns = [ + props.packageBucket, + ...(props.extraPackageBuckets || []), + ] + .filter((bucket): bucket is string => Boolean(bucket)) + .map(bucket => `arn:aws:s3:::${bucket}`); taskRole.addToPolicy( new iam.PolicyStatement({ actions: [ @@ -168,10 +175,7 @@ export class FargateService extends Construct { "s3:GetBucketNotification", "s3:PutBucketNotification", ], - resources: [ - packageBucketArn, - `${packageBucketArn}/*`, - ], + resources: packageBucketArns.flatMap(bucketArn => [bucketArn, `${bucketArn}/*`]), }), ); diff --git a/lib/types/config.ts b/lib/types/config.ts index dca9f1d8..5bbe7862 100644 --- a/lib/types/config.ts +++ b/lib/types/config.ts @@ -316,6 +316,17 @@ export interface PackageConfig { */ bucket: string; + /** + * Additional existing S3 buckets the runtime may write to through + * Benchling App Configuration routing. + * + * These buckets are not created by the stack; they only expand IAM access + * for per-project or per-event routing targets. + * + * @example ["benchling-project-a", "benchling-project-b"] + */ + extraBuckets?: string[]; + /** * S3 key prefix for packages * @@ -763,6 +774,11 @@ export const ProfileConfigSchema = { required: ["bucket", "prefix", "metadataKey"], properties: { bucket: { type: "string", minLength: 3 }, + extraBuckets: { + type: "array", + items: { type: "string", minLength: 3 }, + uniqueItems: true, + }, prefix: { type: "string", minLength: 1 }, metadataKey: { type: "string", minLength: 1 }, workflow: { type: "string", minLength: 1 }, diff --git a/lib/types/stack-config.ts b/lib/types/stack-config.ts index 8bbc2589..d274205c 100644 --- a/lib/types/stack-config.ts +++ b/lib/types/stack-config.ts @@ -31,7 +31,7 @@ export type { VpcConfig } from "./config"; * * **What's NOT included:** * - Benchling OAuth credentials (stored in secret, referenced by ARN) - * - Package configuration (passed as env vars to container) + * - Runtime package routing values (stored in Secrets Manager / Benchling App Config) * - Logging level (passed as env var to container) * - Metadata fields (_metadata, _inherits) * @@ -177,6 +177,17 @@ export interface StackConfig { stackName?: string; }; + /** + * Package infrastructure configuration. + */ + packages?: { + /** + * Additional existing S3 buckets that need the same IAM access as the + * primary package bucket for Tier 1 routing. + */ + extraBuckets?: string[]; + }; + /** * Security configuration (optional) */ diff --git a/lib/utils/config-transform.ts b/lib/utils/config-transform.ts index 998fe634..018e5bd6 100644 --- a/lib/utils/config-transform.ts +++ b/lib/utils/config-transform.ts @@ -31,7 +31,7 @@ import type { StackConfig } from "../types/stack-config"; * * **Excluded fields:** * - benchling.tenant, clientId, appDefinitionId (read from secret at runtime) - * - packages.* (passed as environment variables) + * - packages.bucket/prefix/metadataKey/workflow (runtime config) * - logging.* (passed as environment variables) * - _metadata, _inherits (wizard metadata) * @@ -76,6 +76,11 @@ export function profileToStackConfig( }; // Add optional fields if present + if (profile.packages?.extraBuckets?.length) { + stackConfig.packages = { + extraBuckets: profile.packages.extraBuckets, + }; + } // Quilt managed policy ARNs (for S3 and Athena access) if (profile.quilt.bucketWritePolicyArn) { diff --git a/package.json b/package.json index 83eba0a8..7e3a2482 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,9 @@ "build:clean": "rm -rf cdk.out dist */{*.js,*.d.ts}", "build:synth": "npx cdk synth", "build:typecheck": "tsc --noEmit", + "config:clear": "ts-node bin/cli.ts config:clear", + "config:inspect": "ts-node bin/cli.ts config:inspect", + "config:seed": "ts-node bin/cli.ts config:seed", "deploy:dev": "ts-node bin/cli.ts deploy --profile dev", "deploy:prod": "ts-node bin/cli.ts deploy", "deploy:notes": "bash scripts/release-notes.sh", diff --git a/spec/392-configurability/06-configurability-checklist.md b/spec/392-configurability/06-configurability-checklist.md index 80d3858d..45f10b0c 100644 --- a/spec/392-configurability/06-configurability-checklist.md +++ b/spec/392-configurability/06-configurability-checklist.md @@ -14,13 +14,23 @@ Phases can be developed independently but **all phases deploy together** in one **Hard prerequisite: Define and deploy config schema paths in `app-manifest.yaml`.** The Benchling SDK rejects `AppConfigItemGenericCreate` calls whose `path` doesn't match a pre-defined configuration schema/definition (see [`05b-probe_results.md`](05b-probe_results.md)). Before any seeding or config-item writes, the exact App Configuration schema paths/types must be declared in [`docker/app-manifest.yaml`](../../docker/app-manifest.yaml), then the app must be deployed/updated so Benchling registers the schema. Only then will `config:seed` work. The write-side features below assume this is done. +## Implementation Status + +- [x] Runtime read path, routing model, fallback chain, cache, and invalidation implemented. +- [x] Per-project routing implemented for entry-backed events with graceful fallback. +- [x] Profile/CDK `packages.extraBuckets` IAM support implemented for standalone stacks. +- [x] CLI inspection/seeding/manual-clear helpers implemented. +- [x] README documentation added. +- [x] Unit tests added for routing model, queue metadata, and app ID matching. +- [ ] Benchling App Configuration schema definitions still need to be added to `docker/app-manifest.yaml` and deployed before `config:seed` can create items. + --- ## Phase 1 — App Config Items Read Path (Python runtime) -1. **Add `benchling.apps.list_app_configuration_items()` call** at app startup in Docker Python code. Fetch all config items scoped to our app using the `app.id` matched from `list_apps()` (`app.app_definition.id == secrets.app_definition_id`), **not** the `appDefinitionId` directly. See [`05b-probe_results.md`](05b-probe_results.md) for details. +- [x] **Add `benchling.apps.list_app_configuration_items()` call** at app startup in Docker Python code. Fetch all config items scoped to our app using the `app.id` matched from `list_apps()` (`app.app_definition.id == secrets.app_definition_id`), **not** the `appDefinitionId` directly. See [`05b-probe_results.md`](05b-probe_results.md) for details. -1. **Build a `RoutingConfig` model** from the config items, keyed by `path` convention: +- [x] **Build a `RoutingConfig` model** from the config items, keyed by `path` convention: - `["quilt", "routing", "", ""]` — event-type routing - `["quilt", "projects", "", ""]` — per-project routing - `["quilt", "settings", ""]` — global settings (log level, concurrency) @@ -28,7 +38,7 @@ Phases can be developed independently but **all phases deploy together** in one All paths are namespaced under `"quilt"` so `config:clear` can safely target only our items (SDK has no delete/archive path yet, so `config:clear` may only be an inspect/manual-cleanup helper unless API support is added). -1. **Implement a fallback chain** — for each config value: +- [x] **Implement a fallback chain** — for each config value: ```text Tier 1 (App Config Item) → Tier 2 (Secrets Manager) → Tier 3 (env var / hardcoded default) @@ -36,21 +46,21 @@ Phases can be developed independently but **all phases deploy together** in one If the App Config Items API fails or returns empty, behavior is unchanged. -1. **Replace hardcoded `PACKAGE_EVENT_CONCURRENCY` and `PACKAGING_REQUEST_CONCURRENCY`** with Tier 1 lookup, falling back to current `"5"` default. +- [x] **Replace hardcoded `PACKAGE_EVENT_CONCURRENCY` and `PACKAGING_REQUEST_CONCURRENCY`** with Tier 1 lookup, falling back to current `"5"` default. -1. **Replace `LOG_LEVEL`** with Tier 1 lookup, falling back to secret's `log_level`, falling back to `"INFO"`. +- [x] **Replace `LOG_LEVEL`** with Tier 1 lookup, falling back to secret's `log_level`, falling back to `"INFO"`. -1. **Cache Tier 1 config with a 60s TTL** and stale-while-refresh semantics, matching the existing `secrets_manager.py` pattern: +- [x] **Cache Tier 1 config with a 60s TTL** and stale-while-refresh semantics, matching the existing `secrets_manager.py` pattern: - On first call: block and fetch - Within TTL: return cached value - Past TTL, stale value exists: return stale, kick off background refresh thread - The `app_id` is also cached; if it needs to change the container restarts anyway -1. **Invalidate Tier 1 cache on config lifecycle events.** The app already routes `v2-beta.app.configuration.updated` in [`app.py`](../../docker/src/app.py) — wire this handler to flush the Tier 1 cache so config changes take effect immediately instead of waiting up to 60s for TTL expiry. +- [x] **Invalidate Tier 1 cache on config lifecycle events.** The app already routes `v2-beta.app.configuration.updated` in [`app.py`](../../docker/src/app.py) — wire this handler to flush the Tier 1 cache so config changes take effect immediately instead of waiting up to 60s for TTL expiry. ## Phase 2 — Per-Project Routing (Python runtime) -1. **Resolve project from entry, but only for entry events with a folder.** Not all webhook events are entry-backed. Gate project lookup on `entry.folder_id` being present: +- [x] **Resolve project from entry, but only for entry events with a folder.** Not all webhook events are entry-backed. Gate project lookup on `entry.folder_id` being present: - Has `folder_id` → resolve project, apply per-project overrides - No `folder_id` → skip project lookup, use event-type defaults @@ -64,7 +74,7 @@ Phases can be developed independently but **all phases deploy together** in one project_name = project.name # key into Tier 1 ``` -1. **Apply project routing overrides with defined merge rules.** Precedence (highest → lowest): +- [x] **Apply project routing overrides with defined merge rules.** Precedence (highest → lowest): ```text Project override → Event-type default → Global default → Legacy fallback @@ -72,32 +82,32 @@ Phases can be developed independently but **all phases deploy together** in one **Merge strategy:** field-level merge, not whole-object replace. If a project rule sets only `bucket`, the `prefix` and `workflow` inherit from the event-type or global default. Supported event types: `v2.entry.created`, `v2.entry.updated.fields`, `v2.entry.updated.reviewRecord` (matches existing `supported_event_types` set in [`app.py`](../../docker/src/app.py)). -1. **Handle project lookup failure gracefully.** If folder/project API calls fail (network error, deleted folder, missing permissions), log the error and use event-type defaults. Must not crash the webhook handler. +- [x] **Handle project lookup failure gracefully.** If folder/project API calls fail (network error, deleted folder, missing permissions), log the error and use event-type defaults. Must not crash the webhook handler. ## Phase 3 — ProfileConfig / CDK Changes -1. **Add `packages.extraBuckets` to `ProfileConfig`.** An optional list of additional S3 bucket names that the stack needs IAM permissions for (for per-project multi-bucket routing). No auto-creation — operator lists existing buckets. +- [x] **Add `packages.extraBuckets` to `ProfileConfig`.** An optional list of additional S3 bucket names that the stack needs IAM permissions for (for per-project multi-bucket routing). No auto-creation — operator lists existing buckets. -1. **Pass extra buckets through `StackConfig`** to the CDK stack. Add IAM policy statements for each listed bucket (same permissions as the primary `packages.bucket`). +- [x] **Pass extra buckets through `StackConfig`** to the CDK stack. Add IAM policy statements for each listed bucket (same permissions as the primary `packages.bucket`). -1. **Add integrated-mode guard for IAM changes.** When `integratedStack: true`, the Quilt stack owns IAM. Deploy should print a warning that `packages.extraBuckets` is ignored in integrated mode — Tier 1 routes pointing at undeclared buckets will fail at runtime. Validation docs task should cover this. +- [x] **Add integrated-mode guard for IAM changes.** When `integratedStack: true`, the Quilt stack owns IAM. Deploy should print a warning that `packages.extraBuckets` is ignored in integrated mode — Tier 1 routes pointing at undeclared buckets will fail at runtime. Validation docs task should cover this. -1. **Keep all existing CfnParameters.** No removals. They become deploy-time overrides and runtime fallbacks. +- [x] **Keep all existing CfnParameters.** No removals. They become deploy-time overrides and runtime fallbacks. ## Phase 4 — CLI: Seed / Clear App Config -1. **Add `npm run config:seed` command.** Reads current routing values from `ProfileConfig` + Secrets Manager, creates/updates equivalent App Configuration Items in Benchling via the SDK. **Prerequisite:** The app's configuration schema paths must be defined (see hard Prerequisite section above) — without them the SDK rejects arbitrary paths. Prints the Benchling UI URL where items can be edited. +- [x] **Add `npm run config:seed` command.** Reads current routing values from `ProfileConfig` + Secrets Manager, creates/updates equivalent App Configuration Items in Benchling via the SDK. **Prerequisite:** The app's configuration schema paths must be defined (see hard Prerequisite section above) — without them the SDK rejects arbitrary paths. Prints the Benchling UI URL where items can be edited. -1. **Add `npm run config:clear` command** — **only if the Benchling SDK adds a usable delete/archive endpoint.** As of probe date ([`05b-probe_results.md`](05b-probe_results.md)), `archive_app_configuration_items` is commented out (TODO BNCH-52599) and no SDK delete path exists. If support is still absent when implementing: +- [x] **Add `npm run config:clear` command** — **only if the Benchling SDK adds a usable delete/archive endpoint.** As of probe date ([`05b-probe_results.md`](05b-probe_results.md)), `archive_app_configuration_items` is commented out (TODO BNCH-52599) and no SDK delete path exists. If support is still absent when implementing: - Provide a dry-run inspection command (`config:inspect --items`) that lists all items under the `["quilt"]` namespace and prints the Benchling UI URL for manual cleanup. - Document the manual cleanup process. - Skip the `--force` / auto-clear path entirely. -1. **Add `npm run config:inspect` command.** Dumps current Tier 1 items and effective resolved config (Tier 1 → secret → env var) for debugging. +- [x] **Add `npm run config:inspect` command.** Dumps current Tier 1 items and effective resolved config (Tier 1 → secret → env var) for debugging. ## Phase 5 — Docs -1. **Update README** to document: +- [x] **Update README** to document: - Three-tier configuration model - How to configure per-project routing in Benchling (add benchmark on what needs to happen) - `config:seed`, `config:clear`, `config:inspect` CLI commands @@ -108,7 +118,7 @@ Phases can be developed independently but **all phases deploy together** in one ## Test Plan -- **Unit tests**: Fallback chain resolution, project lookup, missing config items -- **Integration test**: Deploy, seed config, verify app reads Tier 1 values -- **No-Tier-1 test**: Deploy without seeding — everything works as before -- **Rollback test**: Deploy new, roll back to old image — old image works unaffected +- [x] **Unit tests**: Fallback chain resolution, project lookup, missing config items +- [ ] **Integration test**: Deploy, seed config, verify app reads Tier 1 values +- [ ] **No-Tier-1 test**: Deploy without seeding — everything works as before +- [ ] **Rollback test**: Deploy new, roll back to old image — old image works unaffected From 0030845325ce2ff9fb8a8393acfb4e97e95c68d3 Mon Sep 17 00:00:00 2001 From: "Dr. Ernie Prabhakar" Date: Thu, 2 Jul 2026 21:34:48 -0700 Subject: [PATCH 7/7] chore: bump version to 0.19.0 --- docker/app-manifest.yaml | 2 +- docker/pyproject.toml | 2 +- docker/uv.lock | 2 +- package-lock.json | 4 ++-- package.json | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docker/app-manifest.yaml b/docker/app-manifest.yaml index 9459ad3d..aed03bdc 100644 --- a/docker/app-manifest.yaml +++ b/docker/app-manifest.yaml @@ -2,7 +2,7 @@ manifestVersion: 1 info: name: nightly-quilttest-com description: Packaging Benchling Notebooks as Quilt packages - version: 0.18.0 + version: 0.19.0 features: - name: Quilt Connector id: quilt_entry diff --git a/docker/pyproject.toml b/docker/pyproject.toml index f2c8b894..b50092dd 100644 --- a/docker/pyproject.toml +++ b/docker/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "benchling-quilt-integration" -version = "0.18.0" +version = "0.19.0" description = "Benchling-Quilt Integration Webhook Service" license = {text = "Apache-2.0"} authors = [ diff --git a/docker/uv.lock b/docker/uv.lock index 2abcb96d..66e33575 100644 --- a/docker/uv.lock +++ b/docker/uv.lock @@ -75,7 +75,7 @@ wheels = [ [[package]] name = "benchling-quilt-integration" -version = "0.18.0" +version = "0.19.0" source = { editable = "." } dependencies = [ { name = "benchling-sdk" }, diff --git a/package-lock.json b/package-lock.json index ba9adeed..1b6d8939 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@quiltdata/benchling-webhook", - "version": "0.16.0", + "version": "0.19.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@quiltdata/benchling-webhook", - "version": "0.16.0", + "version": "0.19.0", "license": "Apache-2.0", "dependencies": { "@aws-sdk/client-api-gateway": "^3.953.0", diff --git a/package.json b/package.json index 7e3a2482..f10f7647 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@quiltdata/benchling-webhook", - "version": "0.18.0", + "version": "0.19.0", "description": "AWS CDK deployment for Benchling webhook processing using Fargate - Deploy directly with npx", "main": "dist/lib/index.js", "types": "dist/lib/index.d.ts",