Repository navigation
Sync changes from dev to prod - #572
Merged
Merged
Conversation
The Express Mode default rollout is CANARY (5%, 3 min canary bake, 3 min final bake). The admin container serves no public traffic, so the bakes watched nothing and cost about 6 of the ~11 minutes per rollout. app-deploy.yml now sets the service's deployment configuration to ROLLING with no bake before rolling the image, keeping the circuit breaker and the RollbackAlarm (both enable + rollback) and the live max/min percents. It reads the strategy back and fails if ECS did not accept it. Neither Terraform nor update-express-gateway-service can set this, so the step uses update-service, and the deploy role gains ecs:UpdateService on its own admin service.
…ervice - Move the rollout-strategy step to right after AWS credentials, before the image build and task-definition registration, so a rejection or AccessDenied leaves nothing half-deployed and fails sooner. - Treat the configuration as done only when the strategy is ROLLING, the bake is 0, and no canaryConfiguration remains, both for the skip and the read-back. - After the roll, check the strategy is still ROLLING. It survives today only because Express Mode's infrastructure role is denied UpdateService. - Deny UpdateService calls from the deploy role that pass a task definition or enable ECS Exec. - Update the deploy role description and docs to match.
ECS may fill ecs:task-definition server-side even on a deploymentConfiguration-only UpdateService, in which case the Deny would block the rollout-strategy step itself. Keep the ECS Exec Deny.
## Line breakdown | Category | Lines added | % | |---|---|---| | Config/build | 128 | 94.1% | | Docs | 8 | 5.9% | | **Total** | **136** | **100%** | This PR switches the admin ECS service from Express Mode's default CANARY rollout to ROLLING with no bake, which removes about 6 minutes of baking from each ~11-minute rollout. ## What this does - **Switches the strategy on every deploy.** Right after AWS credentials are configured, before the image build and before any task-definition revisions are registered, `app-deploy.yml` sets the admin service to ROLLING with no bake, replacing CANARY (5% canary, 3-minute canary bake, 3-minute final bake). - **Skips the update when there is nothing to change.** If the service is already ROLLING with `bakeTimeInMinutes` 0 (or absent) and no `canaryConfiguration` left over, the step logs "nothing to change" and makes no update call. - **Keeps the rest of the live configuration.** `maximumPercent`, `minimumHealthyPercent`, and the rollback alarm names are copied from the live service (falling back to the `<cluster>/<service>/RollbackAlarm` name Express derives), with the circuit breaker and alarm both set to enable+rollback. - **Fails the deploy if ECS didn't take it.** The step reads the configuration back and stops the deploy before the build unless it is ROLLING, has `bakeTimeInMinutes` 0 (or absent), and has no `canaryConfiguration` left over, since it isn't documented whether UpdateService replaces the deployment configuration or merges into it. - **Checks the strategy again after the roll.** Once the roll converges, a new step fails the deploy if the strategy is no longer ROLLING or a canary block has come back, because Express Mode's infrastructure role tries an UpdateService on the service right after each `update-express-gateway-service` call, and the only reason the setting survives a roll is that this call is denied today (seen in CloudTrail). - **Grants the deploy role `ecs:UpdateService`.** The grant is scoped to the admin service ARN of the role's own environment. - **Denies UpdateService calls that turn on ECS Exec.** An explicit Deny matches requests where `ecs:enable-execute-command` is true, and `aws iam simulate-custom-policy` allows a deploymentConfiguration-only call while denying one with exec on. - **Updates the pipelines doc.** The sequence diagram shows the set step before the build and the check after the roll, and a short paragraph records that the setting depends on Express's infrastructure role being unable to modify the service. - **Describes the deploy role as "image roll + rollout strategy".** The role's description, `docs/infrastructure/README.md`, and `docs/infrastructure/identity.md` no longer say "image roll only". ## Decisions to review 1. **The deploy role holds `ecs:UpdateService` on the admin service on every run.** A Deny blocks turning on ECS Exec, but a leaked deploy token could still point the service at another task definition or change the desired count or network config. The alternative is an operator applying the switch once by hand, keeping the role image-roll only. 2. **The strategy is set in CI on every deploy, not declared in Terraform.** The `aws_ecs_express_gateway_service` resource in hashicorp/aws 6.55.0 and `update-express-gateway-service` expose no deployment configuration, so the setting is invisible to `terraform plan`, and removing the step does not restore CANARY. The post-roll check catches Express resetting it. ## Before merging - **This has not been tried against AWS.** Whether ECS accepts `update-service --deployment-configuration` on an Express Mode gateway service is unverified, and nothing was changed in AWS while making this PR. It is expected to work because the service reports deploymentController type `ECS`. If ECS rejects it, the step fails loudly at the start of the deploy, before the build, rather than silently. - **Production gets this too.** `app-deploy.yml` is shared by `deploy-development.yml` and `deploy-production.yml`, so production switches on its next deploy, not just development. The next dev deploy is the live test. - **Apply the bootstrap root before merging.** The IAM change is in the bootstrap root, which a human applies by hand; CI never applies it. If this merges first, every deploy fails with AccessDenied at the new step, before any build. - **Rolling back is one `update-service` call.** The before-state, identical in both environments, was: - strategy CANARY, `canaryPercent` 5.0, `canaryBakeTimeInMinutes` 3, `bakeTimeInMinutes` 3 - `maximumPercent` 200, `minimumHealthyPercent` 100 - circuit breaker enable+rollback - alarm `mmgis-<env>/mmgis-<env>-admin/RollbackAlarm` enable+rollback ## Follow-ups - Move the setting into Terraform if the provider gains an argument for it. - Re-add a Deny on UpdateService calls carrying `ecs:task-definition` once a dev deploy proves a configuration-only UpdateService still passes with it in place (ECS may fill that key server-side, which would deny this PR's own step).
…of ROLLING (#566) ## Line breakdown | Category | Lines added | % | |---|---|---| | Config/build | 36 | 92% | | Docs | 3 | 8% | | **Total** | **39** | **100%** | This PR fixes the admin-service deploy that broke after #553: ECS refuses to switch an Express gateway service to ROLLING, so the deploy now asks for CANARY at 100% with both bake times at 0, which rolls out the whole new task set at once and still drops the ~6 minutes of baking #553 was after. Follows #553. ## What this does - **The pre-build strategy step asks for CANARY instead of ROLLING:** it sends `canaryPercent` 100, `canaryBakeTimeInMinutes` 0 and `bakeTimeInMinutes` 0, and still copies max/min percent and alarm names from the live service with the circuit breaker and `RollbackAlarm` on enable+rollback. - **The step skips the update when the setting is already in place**, logging "Already zero-bake CANARY; nothing to change". - **The read-back check fails the deploy before the build unless ECS took every part** (strategy, the 100% canary and both zero bakes), so a partial update does not count as done. - **The post-roll "Confirm rollout strategy survived the roll" check uses the same rule** and names the renamed step "Set rollout strategy to zero-bake CANARY". - **The workflow comment says why the setting survives a roll:** the managed policy `AmazonECSInfrastructureRoleforExpressGatewayServices` has no `ecs:UpdateService`, so Express's own update attempt after each roll is denied. - **The pipelines doc and the IAM comment describe zero-bake CANARY**, and the doc notes that ECS rejects ROLLING on Express services; the `iam_deploy.tf` change is comment only, so the bootstrap root does not need re-applying. - **Production picks this up on its next deploy**, because `app-deploy.yml` is shared. - **The setting was proven on a scratch ECS Express Mode service with MMGIS's defaults** (private repo `CarsonDavis/ecs-express-rollout-lab`, `FINDINGS.md`): ECS stored CANARY 100/0/0 as sent, an image roll took 199 s versus 674 s under the default, the setting survived every roll, and ROLLING was the only strategy rejected. - **The first development deploy after merge is the live test and needs no infrastructure step**, because the `ecs:UpdateService` grant on both deploy roles is already applied. ## Decisions to review 1. **Zero bake time narrows alarm-based rollback.** With both bake times at 0, the `RollbackAlarm` the step keeps enabled has no bake window in which to trigger a rollback, so only the deployment itself and the circuit breaker (tasks failing to start or pass health checks) catch a bad image; the alternative is a short final bake, for example 1 minute, at the cost of that minute on every deploy.
## Summary PNG and PDF exports from ShareExport and MapControl no longer carry the hardcoded `mmgis-` prefix. Both tools build filenames through the shared `buildExportFilename` in `src/essence/Tools/_shared/adapters/shareActions.ts`. | | Before | After | |---|---|---| | Full view state | `mmgis-MSL_2026-07-01T12-00-00Z_4.5892_137.4416.png` | `MSL_2026-07-01T12-00-00Z_4.5892_137.4416.png` | | No view state | `mmgis-map.png` / `mmgis-map.pdf` | `map.png` / `map.pdf` | The legacy bottom-bar screenshot (`BottomBar.js`) is out of scope and unchanged. ## Test plan - [x] `npx vitest run tests/unit/ShareExport` (37/37 pass) - [x] Download a PNG and a PDF from ShareExport and MapControl and check the filenames
The exported legend's "Time cursor" and "Exported" lines now end in the zone core formatted them in, e.g. "2026-10-12T14:30:00 UTC". Core reports that zone through a time:getTimeZone bus request, kept beside its formatter, so the export never assumes UTC itself. The default mission time format drops its literal Z so the zone isn't named twice.
The default is %Y-%m-%dT%H:%M, with no seconds and no literal Z, since the export legend names the zone after the time.
## Summary
Two fixes from the Air4US review of the station timeseries chart.
**Title.** The card said "Washington" for a site the popup headed
"Washington, Georgia · Site 1". The chart title came from one property
(`titleProp: county_name`), while the popup title is a `{prop}`
template. The timeseries block now takes the same kind of template
(`title`), filled by the shared `fillTemplate` the popup uses, so the
two can be written identically. `titleProp` stays as the fallback when
the template fills to nothing.
**Tooltip clock.** "Aug 24, 2025, 00:00" never moved along the line. The
series are daily summaries keyed on `date_local`, so every point is UTC
midnight and the chart printed a clock for data that has none. The
tooltip title and the zoom window's end labels now print the date only.
Axis ticks are unchanged and still show hours inside a one-day span.
## What's in the diff
- `FetchTimeseries/lib/timeseries.ts`: `title?: string` on
`TimeseriesConfig`; `featureTitle()` tries the filled template first.
- `configure/src/metaconfigs/layer-vector-config.json`,
`layer-vectortile-config.json`: a "Title" field ahead of "Title
Property" in the Timeseries row.
- `SeriesChart/lib/chartData.ts`: `formatTooltipTime` is date-only;
`makeWindowLabelFormat` and its two-day clock branch are gone, the
window labels use the same date format.
- READMEs for both plugins.
- Tests: `fetchTimeseries.spec.js` (template ahead of titleProp, empty
fill falls through, dot path), `seriesChartData.spec.js` (date-only
title and window labels).
## Testing
- `npm run test:unit` green (211 files, 3578 tests) and `tsc --noEmit`
clean.
- Manual, on the Air4US mission with
`Air4US_v182_timeseries-title.config.json`: click a PM2.5 site in
Washington County, GA. Expect the title "Washington, Georgia · Site 1",
a tooltip titled "Feb 28, 2026", and window labels without a clock.
## Not in this PR
- Units: the reviewers asked for symbols (µg/m³, ppm) and whether "(LC)"
belongs. The response carries `units_of_measure` and no symbol field, so
the chart keeps showing the unit as the data provides it.
- PM10 sites: the v182 PM10 layer has no timeseries URL and no popup
action, and the daily PM10 table on eic-staging is loaded as text (its
date filter returns 500) and holds only parameter 86101 (PM10-2.5).
Needs a data-side fix before a config fix; parked.
- The config edit itself (five `timeseries.title` additions, structural
diff clean) goes to the mission, not this repo.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
## Problem On an exported image, the header's **Time cursor** and **Exported** timestamps don't say which time zone they're in. Core formats both in UTC, but a mission's time format doesn't have to print the zone, so a time like `2026-10-12 14:30` reads as local time. ## Fix - **Core** (`TimeControl.js`) reports the zone it formats in through a new `time:getTimeZone` bus request. The zone is the `DISPLAY_TIME_ZONE` constant (`'UTC'`), kept next to the formatter so the two can't disagree. - **Adapter** (`mmgisAPI.ts`): `mmgisGetTimeZone()`, which returns null if core doesn't provide the request. - **Export** (`getExportLegendModel.ts`) adds that zone after both timestamps, e.g. `2026-10-12T14:30:00 UTC`. If core can't name a zone, the times print without one; the export never assumes UTC itself. - The default mission time format becomes `%Y-%m-%dT%H:%M:%S` (no literal `Z`), so the default doesn't print the zone twice. The help text on the Configure Time tab and the docs give the new default. Layer URL time formats keep their `Z`, since they build server requests, not display text. To support a user's local time zone later, only core changes: format in that zone and return its name from `time:getTimeZone`. The export needs no changes. Docs: `time:getTimeZone` is added to `Event-Bus-API.md`, and `DATES.md` describes the header format. ## Testing - `npx vitest run src/essence`: 745 passed. The new tests cover a UTC suffix, a non-UTC zone reported by core, and times printed without a zone when core doesn't provide the request or the request throws. - Not yet checked with a real export in the app. ## Note A mission whose own format still ends in `Z` will show both `Z` and `UTC`. The fix for those is to change that mission's Time Format setting.
Terraform plan preview —
|
Terraform plan preview —
|
BhattaraiSijan
approved these changes
Oct 15, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sync changes from dev to prod