feat: implement readiness and liveness probes for bucket migration#93
Conversation
…enhance migration locking mechanism, and improve environment variable handling in control-plane mode
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesThe application now starts HTTP serving before completing distributed bucket migrations, exposes migration-aware liveness and readiness behavior, concurrently moves S3/GCS objects, updates Helm environment and probe rendering, and validates missing legacy key configuration earlier. Migration-aware startup
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant HTTPServer
participant EnsureMigrations
participant RealRouter
HTTPServer->>HTTPServer: Serve /hc and temporary 503 responses
HTTPServer->>EnsureMigrations: EnsureMigrations()
EnsureMigrations-->>HTTPServer: Migrations complete
HTTPServer->>RealRouter: Activate router
RealRouter-->>HTTPServer: Serve /ready and application routes
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/api/main.go`:
- Around line 48-53: Update the http.Server initialization in the main server
setup to configure nonzero ReadHeaderTimeout, ReadTimeout, and IdleTimeout
values, preventing slow or idle clients from holding connections indefinitely.
Do not add WriteTimeout, or choose a sufficiently generous value that will not
interrupt long-lived manifest or asset responses.
In `@helm/helm_template_test.go`:
- Around line 75-82: Replace every exec.Command invocation in
helm_template_test.go, including the calls in the test sections around lines 75,
140, 166, and 195, with exec.CommandContext using an appropriate test-scoped
context so the noctx linter passes. Preserve each command’s arguments and
existing execution behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 52e65025-887f-4586-846f-dcae40800c88
📒 Files selected for processing (9)
cmd/api/main.gohelm/helm_template_test.gohelm/templates/deployment.yamlhelm/values.yamlinternal/bucket/v2_migration.gointernal/bucketmigration/runner.gointernal/database/postgres/migrations/20260613110419_migrate_infra_to_db.gointernal/database/postgres/migrations/seal_legacy_keys_test.gointernal/router/router.go
* feat: control plane mode (#65) * feat: use generic BUCKET_KEY_PREFIX instead of S3_KEY_PREFIX with backward compat * security: harden path inputs for bucket (defense in depth) * feat: init headers with expo-app-id * feat: add expo-app-id header + depreciation notes on eoas commands * feat: add apps prefix on handler * wip(): rewrite tests & add appId prefixes on bucket methods * wip(): docs + dashboard * fix: findigns * fix: 404 handling on missing app * fix: findings * fix: remove dead ctx * fix: docs * fix: gh copilot findings * feat: add more auth tests * feat: add metrics per appId * fix: prometheus metrics + grafana dashboard & helm chart * fix: harden app id validation, migration collision, boot-time key checks * fix: harden app id validation, migration collision, boot-time key checks * fix: prevent race condition * test: add race condition test * fix: tests * feat: control plane implementation * feat: new UI for control plane mode * feat(control-plane): remove EXPO_APPS_JSON, fix regressions, validate dashboard input Multi-app is now served exclusively by the control plane (DB mode). The stateless path stays single-app through the flat v1 env vars. Remove EXPO_APPS_JSON - config: drop the JSON app source; stateless mode reads only EXPO_APP_ID, EXPO_ACCESS_TOKEN, KEYS_STORAGE_TYPE and the mode-specific key vars - drop the now-dead multi-app guard in the v1->v2 bucket migration - helm: delete the apps-secret template and the multiApp flag - remove the unused ControlPlaneMasterKeyConfig struct Fix control-plane regressions - republish: restore the source-update guards (exists, normal type, valid, platform match) that the refactor dropped. Implemented through the injected update repo so they hold on both backends -- the global update.* helpers only read the bucket, so a DB-mode rollback would otherwise slip through - add IsUpdateValid to the update repository (database: valid by construction, bucket: .check sentinel) - app deletion failed on the RESTRICT foreign keys from channels and runtime_versions; cascade those from apps and defer the sibling checks (migration 20260614000000) - guard a nil dereference in the assets metadata path Validate dashboard input (internal/validation) - segment-safe resource names (branch, channel, release channel) and bounded display labels, enforced in the services and surfaced as 400 by the handlers - covers create, update, read, delete and id inputs Helm: add control-plane mode (controlPlane flag, DB_URL, master key, pool tuning) and gate the flat-env vars behind controlPlane=false. Docs: rewrite the multi-app page around the control plane; drop EXPO_APPS_JSON from the environment reference and the v1->v2 migration guide. Repair the integration suite, which no longer compiled after the control-plane refactor (handlers became structs, internal/auth moved into services, types relocated). 85/85 green. Move the shared migration engine into its own file so the injection point is discoverable from any migration. * feat: add App Selector even in mono app mode * feat: update channel-branch mapping to use IDs and names - Refactor `updateChannelBranchMapping` in `ApiClient` to accept release channel ID and name. - Modify `SelectBranch` component to handle branch ID and name for channel mapping. - Update `Channels` page to include branch selection when creating a new channel. - Adjust database queries to ensure branch and channel mappings are scoped by app ID. - Change environment variable names in documentation and configuration to reflect new master key structure. - Enhance error handling and validation for channel and branch IDs in the backend. - Update tests to validate the new mapping behavior and ensure cache invalidation works correctly. * migration: migrate app to database mode if using a legacy key store (environment | local path) * feat: rename bucket migrations package * chore: untrack tmp/main build artifact Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat: enhance migration and update processes, add versioning support, and improve error handling * feat: refactor update retrieval and rollback logic, remove unused parameters and functions * feat: rename LoadApps and ReadApps functions to LoadAppsFromFlatEnv and ReadAppsFromFlatEnv for clarity and consistency * Refactor providers and implement JWT functionality - Moved AWS and GCP provider functions to dedicated packages for better organization and clarity. - Updated references in the codebase to use the new provider packages. - Introduced a new JWT utility in the crypto package for generating and decoding JWT tokens. - Refactored the auth service to utilize the new JWT functions for token generation and validation. - Adjusted caching mechanisms to ensure proper cache invalidation and retrieval for channel mappings. - Enhanced error handling and logging for AWS and GCP interactions. * feat: add linting job to GitHub Actions workflow - Introduced a new `lint` job in the GitHub Actions workflow to check for dead code using `staticcheck` and `deadcode`. - Updated the Makefile to include a `lint` target that runs both tools and reports findings. refactor: replace AuthService with CliAuthService and DashboardAuthService - Refactored handlers to use `CliAuthService` for CLI-related authentication and `DashboardAuthService` for dashboard authentication. - Updated the router to utilize the new authentication services. - Removed the old `AuthService` and added new service files for `CliAuthService` and `DashboardAuthService`. chore: remove unused helpers and middleware - Deleted the `headers.go` and `cors_middleware.go` files as they were no longer needed. - Updated the `auth_middleware.go` to handle both CLI and dashboard authentication based on the request header. fix: update tests to reflect changes in authentication services - Modified test cases to use the new `DashboardSession` structure instead of `AuthResponse`. - Ensured that all references to the old authentication service are updated to the new structure. * feat: enhance AES-GCM sealing and unsealing with context binding, add database migration checks * feat: refactor update handling in BucketUpdateStore, streamline validation and metadata management * feat: introduce control plane for multi-app management - Added a new documentation page for the Control Plane, detailing its benefits and configuration. - Updated prerequisites documentation to clarify the use of `expo-app-id` and its importance in multi-app deployments. - Implemented a new `Doctor` command to probe the update server for manifest accessibility, supporting both v1 and v2 clients. - Enhanced the handling of legacy app ID fallback to ensure v1 clients can still receive updates when the `expo-app-id` header is absent. - Refactored asset and manifest handlers to utilize the new app ID resolution logic. - Updated tests to cover scenarios for missing and empty app ID headers, ensuring proper handling and fallback behavior. - Removed the skip flag for v1 to v2 bucket migration, enforcing a mandatory re-path during upgrades. * feat: share /updates volume for local bucket testing * feat: refactor CloudFront key handling and update related tests * fix: resolve bucket key prefix in generic CDN via ResolveKeyPrefix The generic CDN read S3_KEY_PREFIX directly, ignoring BUCKET_KEY_PREFIX — URLs built for operators using the new variable omitted the prefix and 404'd. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: count unique clients in update_error_users_total, not the runtime version The set key already contains the runtime, so adding the runtime as the member pinned the gauge to 1 whenever any error was reported. Add the clientId instead, matching the metric's documented meaning and TrackActiveUser's shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: gate /metrics on PROMETHEUS_ENABLED, ignore KEYS_STORAGE_TYPE for CloudFront key in DB mode, allow VITE_OTA_API_URL override in dashboard dev script - /metrics was registered unconditionally; PROMETHEUS_ENABLED gated nothing. - In control-plane mode a leftover KEYS_STORAGE_TYPE silently disabled the CDN by selecting an empty CloudFront key source; the variable has no meaning there and is now ignored. - The dashboard dev script hardcoded VITE_OTA_API_URL, shadowing any exported value; use a shell default instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * revert: keep KEYS_STORAGE_TYPE selecting the CloudFront key source in every mode The DB-mode override made control planes fall back through every source; the intended behavior is that a chosen mode always selects its matching source, with the fallthrough only when the variable is unset. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Update documentation links in README and command prompts - Changed the EOAS official documentation link in README.md to the new GitBook URL. - Updated the project ID prompt message in init.ts to reflect the new documentation link for getting started with stateless mode. - Modified the error message in wire.go to point to the updated documentation for the stateless (flat-env) config format. * feat: removing header in README.md * fix: linter * feat: enhance settings handler to include GCS bucket name, CDN type, and Expo account username (#86) - Added GCS_BUCKET_NAME to SettingsEnv struct. - Introduced CDN_TYPE to reflect the resolved CDN at boot. - Included EXPO_ACCOUNT_USERNAME for better user context in stateless mode. - Updated GetSettingsHandler to populate new fields based on configuration and context. - Modified test to reflect changes in expected settings response. * fix: change screenshot * feat: implements users on dashboard (#87) * feat: implements users on dashboard * fix: prevent race conditions while deleting admin user * fix: findings * fix: display password policy on USER_PASSWORD fail * feat: propagate ctx context to expo fetchName * feat: packages * feat: init enterprise + licensing (#88) * feat: add licenses activations & multi-replicas sync * fix: remove yarn packages from dashboard * docs: change contact email * feat(api-key-scopes): implement protected branches & ip protection (#89) * feat(api-key-scopes): implement per-key access restrictions with channel scoping and IP allowlist - Added new API endpoints for managing API key scopes, including retrieval and setting of channel IDs and allowed IPs. - Introduced a new Postgres store for persisting API key scopes and corresponding database migrations. - Updated existing services and handlers to incorporate the new API key scope functionality. - Enhanced error handling for various scenarios related to API key scopes. - Improved user interface messages for better clarity and user experience. * feat: Implement per-key access restrictions and branch protection - Added `CanAccessProtectedBranches` field to `ApiKey` model. - Introduced `Protected` field in `Branch` model to indicate branch protection status. - Updated SQL queries to support fetching and updating API key restrictions. - Modified CLI authentication to enforce access restrictions based on API key and branch protection. - Enhanced error handling for CLI authentication failures, distinguishing between unauthorized access and access denied due to restrictions. - Introduced `ClientIP` helper function to resolve client IP addresses for access control. - Updated middleware and handlers to utilize new authentication logic. - Refactored related tests to cover new functionality and ensure correctness. * feat: Add support for trusted proxy headers and depth configuration * feat: Implement CIDR allowlist normalization and IP restriction enforcement * feat: add resolved client IP in EOAS Error message to troubleshoot some issues in the future (ipv4 vs ipv6, bad TRUST_PROXY_DEPTH configuration etc... * feat: init sso (#90) * feat: init sso * fix: several finding & allow instant SSO Switch * fix: prevent unverified email address to connect through IDP * feat: implement CRUD for trust email * fix: release workflow * feat: add ee features * ci: run tests on plain runner, drop unpublished ci image * chore: upgrade Go version to 1.25 in Dockerfiles and go.mod * feat: add manual validation for SSO (#94) * feat: add manual validation for SSO * fix: test snapshot * fix: add GetMe in user service instead of using a DB_URL conditions... * fix: remove deadcode * fix: enabled field not stored * feat: implement readiness and liveness probes for bucket migration (#93) * feat: implement readiness and liveness probes for bucket migrations, enhance migration locking mechanism, and improve environment variable handling in control-plane mode * fix: add readheader & idle timeout on http server * fix: dockerfile-dev * fix: update license references from Expo Open OTA to Mercure Technologies * feat: implements progressive rollouts (#91) * feat: implements progressive rollouts * fix: remove deadcode * Update apps/dashboard/src/components/UpdateDetailsSheet/index.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat: enhance clipboard functionality and update rollout terminology * fix: findings --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat: add Postgres support for rollout and user store tests; enhance SQL queries and error messages * fix: user tests * fix: allow OOO migrations * fix: build tsc * feat: change readme * feat: multi app support README.md * feat: multi app support README.md * fix workflows with promote latest --------- Co-authored-by: Emanuele Coricciati <73798800+emacoricciati@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This pull request introduces several improvements and fixes to the migration process, Helm chart, and deployment readiness for the Expo Open OTA service. The main focus is on making bucket migrations safer and more observable, improving environment variable rendering for different deployment modes, and updating health probes to better coordinate with orchestrators. The most significant changes are grouped below.
Deployment and Migration Readiness Improvements:
/hc(liveness) during bucket migrations, returning 503 for all other endpoints until migrations complete. This prevents the orchestrator from killing pods during long migrations and only marks them as ready when safe to serve traffic./readyis not registered until migrations finish. (cmd/api/main.go)/hcis used for liveness (always 200), and/readyfor readiness (only 200 after migrations), ensuring correct orchestration behavior during upgrades. (helm/templates/deployment.yaml)Bucket Migration Parallelization and Observability:
BUCKET_MIGRATION_CONCURRENCY), significantly speeding up migrations. Marker files are moved only after all other objects to ensure safe retries. Progress is logged every 500 objects to provide visibility during long migrations. (internal/bucket/v2_migration.go) [1] [2] [3]Helm Chart Environment Variable Rendering:
helm/values.yaml,helm/templates/deployment.yaml) [1] [2] [3] [4]AWS_BASE_ENDPOINT,BUCKET_KEY_PREFIX) are now always rendered withenabled: trueto prevent accidental omission from deployments. (helm/values.yaml) [1] [2]Helm Chart Testing Enhancements:
helm/helm_template_test.go) [1] [2] [3]These changes together make migrations safer and more robust, improve deployment reliability, and ensure smooth upgrades with strong backward compatibility.
Summary by CodeRabbit
New Features
/hcremains available during startup, while/readybecomes available after migrations finish.Bug Fixes