Skip to content

feat: redeploy the hosted PDP fleet on release - #331

Closed
dshoen619 wants to merge 2 commits into
mainfrom
feat/release-fleet-redeploy
Closed

feat: redeploy the hosted PDP fleet on release#331
dshoen619 wants to merge 2 commits into
mainfrom
feat/release-fleet-redeploy

Conversation

@dshoen619

@dshoen619 dshoen619 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

What

Adds a single new workflow, .github/workflows/redeploy-pdp-fleet.yml, that forces every hosted (managed) PDP ECS service to re-pull permitio/pdp-v2:latest after an official release.

release.yml is not modified. This PR adds one file and nothing else.

Why not the Terraform route

Every hosted customer in pdp-deployer is pinned to image_tag = "latest". The tag string never changes, so terraform plans an empty diff and nothing deploys. Refreshing the image today means flipping force_new_deployment = true in each customer's tfvars, applying, then remembering to flip it back — see permitio/pdp-deployer#30, which needs a follow-up revert PR to be safe.

That flip-back is what got missed on aen-gems-prod: it has sat at true on main since PER-14649, so every Digger apply touching it force-redeploys the service right now, intended or not.

update-service --force-new-deployment re-runs the existing task definition and Fargate pulls the image fresh. No new revision is registered and no terraform-tracked attribute changes, so this produces zero terraform drift — including on aen-gems-prod, the one customer with ignore_task_definition_changes = false.

How it is sequenced without touching release.yml

This is the one consequence of keeping release.yml untouched, and it is handled explicitly.

Because nothing in release.yml triggers this workflow, it listens to the same release: published event as the image build — which means it starts alongside the build, not after it. At that moment :latest is still the previous release.

The preflight job therefore waits until permitio/pdp-v2:latest and permitio/pdp-v2:<tag> resolve to the same manifest digest before any service is touched, with a 40-minute deadline covering the multi-arch build. If the build fails, this times out and fails loudly — "the fleet was NOT redeployed" — rather than rolling the fleet onto a stale image.

That gate does double duty: it also covers the overlapping-release case, where an older run would otherwise redeploy the fleet onto a :latest belonging to a newer release.

Rollout

All eleven services in one wave, max-parallel: 3, fail-fast: false so one customer cannot cancel the other ten.

Region Services
public-pdps-us-east-1 general-redoc, aenetworks-dev, aenetworks-prod, aen-cw-prod, aen-gems-prod, getwingwork, wingwork, inventa, iofinnet
public-pdps-us-east-2 allegion-test, allegion-tam

Per service: snapshot running task ARNs + image digests → update-service --force-new-deployment → poll to steady state → assert no pre-deploy task ARN survives → record a result the report job merges into a job-summary table.

Design notes

The steady-state loop replaces aws ecs wait services-stable, which is hardcoded to 40 polls × 15s = 10 minutes. These services need longer: task boot, plus healthy_threshold 2 × interval 30 on the ALB, plus the target groups' default 300s deregistration_delay while the old pair drains. It also treats a service as stable only once the old deployment has fully drained, and keys stall detection on failedTasks because rolloutState is only populated when the ECS deployment circuit breaker is enabled — which it is not on these services.

Task replacement is asserted by ARN set-intersection, not timestamps. ECS returns startedAt with a +00:00 offset that jq's fromdateiso8601 will not parse. Digest comparison is advisory only: a rebuild producing an identical image legitimately keeps the same digest.

workflow_dispatch is available with dry_run defaulting to true, so a manual run enumerates targets instead of deploying unless you opt in.

Prereleases and drafts never roll the fleet — they never push :latest either, so the gate matches the existing build condition.

Never cancel-in-progress — cancelling mid-roll abandons a service with its old tasks half drained.

Verification

No actionlint/shellcheck in the local environment, so this was checked directly:

  • The matrix diffed against pdp-deployer/terraform/customers/envs/*.tfvars — 11 services, none missing, none extra, every cluster matching its region, aen-gems-prod correctly the one entry without a service_name_suffix.
  • Every run: block parsed with bash -n.
  • Every jq program executed against fixtures for the rolling, draining, steady and stalled states, plus full / partial / zero task replacement. The draining case matters: new tasks at desired count while the old deployment still drains must not read as stable, and a bare runningCount == desiredCount check would have passed it.
  • git diff origin/main -- .github/workflows/release.yml is empty.

⚠️ Blocking prerequisite — IAM

PDP_CICD_AWS_ROLE is defined outside this repo and has only ever touched one service in us-east-1. It needs ecs:UpdateService, ecs:DescribeServices, ecs:ListTasks and ecs:DescribeTasks across both public-pdps-* clusters. Until that lands, the two Allegion services fail every run. iam:PassRole is not required — force-new-deployment registers no task definition and passes no role.

Once it lands, verify with a manual dispatch of Redeploy PDP fleet with dry_run: true (the default) — it enumerates all 11 targets and confirms the role authenticates in both regions without touching anything.

Known gaps, not addressed here

  • All eleven roll together. With one wave, a bad release reaches every managed customer in roughly 10 minutes. max-parallel: 3 bounds concurrency but there is no canary gate.
  • pdp-deployer cleanup — deleting the force_new_deployment lines from the tfvars and adding a CI guard that diffs them against this matrix, so onboarding customer Opensource ws rpc #12 fails the build. Until that exists, the matrix here is verified-at-commit-time but not enforced. permitio/pdp-deployer#30 is left open and untouched.
  • No ECS deployment circuit breaker — a stalled rollout retries forever at doubled capacity. The workflow catches it and fails within ~2 minutes, but nothing stops ECS itself.
  • No rollback — with :latest, every task-definition revision resolves the same mutable tag. Recovery means re-tagging on Docker Hub, which also affects self-hosted customers. Immutable version tags are the real fix and the natural follow-up.

🤖 Generated with Claude Code

Every hosted customer in pdp-deployer is pinned to image_tag = "latest", so
the tag string never changes, terraform plans an empty diff, and refreshing
the image means flipping force_new_deployment = true in each customer's
tfvars, applying, and remembering to flip it back. The flip-back is what got
missed on aen-gems-prod, which has been force-redeploying on every apply
since PER-14649.

Add a standalone workflow that does it instead. `update-service
--force-new-deployment` re-runs the existing task definition, so no new
revision is registered and no terraform-tracked attribute changes: this
produces zero terraform drift, including on aen-gems-prod, the one customer
with ignore_task_definition_changes = false.

release.yml is deliberately untouched. The consequence is that this workflow
listens to the same `release: published` event as the image build rather than
running after it, so it cannot assume :latest has moved. The preflight job
waits until permitio/pdp-v2:latest and the release tag resolve to the same
manifest digest before anything is deployed, and fails loudly if the build
never lands. That check also covers the overlapping-release case, where an
older run would otherwise redeploy the fleet onto a newer release's :latest.

All eleven services roll in one wave at max-parallel 3, fail-fast disabled so
one customer cannot cancel the other ten.

Per service: snapshot the running task ARNs and image digests, force the new
deployment, poll to steady state, then assert none of the pre-deploy task
ARNs survive. Compared as a set rather than by startedAt, which ECS returns
with a +00:00 offset that jq's fromdateiso8601 will not parse.

The steady-state loop replaces `aws ecs wait services-stable`, which is
hardcoded to 40 polls x 15s = 10 minutes. These services need longer: task
boot, plus healthy_threshold 2 x interval 30 on the ALB, plus the target
groups' default 300s deregistration_delay while the old pair drains. It also
treats a service as stable only once the old deployment has fully drained,
and keys stall detection on failedTasks because rolloutState is only
populated when the ECS deployment circuit breaker is enabled, which it is not
on these services.

Prereleases never push :latest, so they never roll the fleet.

Requires PDP_CICD_AWS_ROLE to allow ecs:UpdateService in us-east-2 as well as
us-east-1; it has only ever touched one us-east-1 service. The two Allegion
services fail until that lands. Verify with a dry-run dispatch first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dshoen619
dshoen619 force-pushed the feat/release-fleet-redeploy branch from 72053cc to b5855fb Compare August 12, 2026 18:33
@dshoen619 dshoen619 changed the title feat: roll the hosted PDP fleet automatically on release feat: redeploy the hosted PDP fleet on release Aug 12, 2026
@dshoen619
dshoen619 requested a lite review from Copilot August 12, 2026 18:35
@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown

🔍 Vulnerabilities of permitio/pdp-v2:next

📦 Image Reference permitio/pdp-v2:next
digestsha256:85c25de71ae174d5f951646ad15cd57645a5b8b4cdd41413c1749f6918532e04
vulnerabilitiescritical: 0 high: 3 medium: 3 low: 1 unspecified: 1
platformlinux/amd64
size133 MB
packages248
📦 Base Image alpine:3.23
also known as
  • 3.23.5
digestsha256:1beb0dc0a51de7ff38e3b5274078a2e0b81113ba5c7535e1a03d5913a5edbda3
vulnerabilitiescritical: 0 high: 0 medium: 1 low: 0
critical: 0 high: 2 medium: 2 low: 1 starlette 0.50.0 (pypi)

pkg:pypi/starlette@0.50.0

high 7.5: CVE--2026--54283 Allocation of Resources Without Limits or Throttling

Affected range>=0.4.1
<1.3.1
Fixed version1.3.1
CVSS Score7.5
CVSS VectorCVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
EPSS Score0.397%
EPSS Percentile33rd percentile
Description

Summary

request.form() accepts max_fields and max_part_size to bound resource consumption while parsing form data. These limits are enforced for multipart/form-data, but silently ignored for application/x-www-form-urlencoded. An unauthenticated attacker can therefore send a urlencoded body with an arbitrarily large number of fields or an arbitrarily large field, even when the application configured limits it believed would apply.

Details

request.form() dispatches to a different parser depending on the Content-Type. For multipart/form-data the max_files, max_fields, and max_part_size limits are forwarded to the parser, but for application/x-www-form-urlencoded the parser is constructed without them. It has no max_fields or max_part_size parameter to receive them, and it appends every field with no count check and accumulates each field's name and value with no size check. The configured limits are therefore both unreachable and unenforced for url-encoded bodies.

Because the url-encoded parser does its work synchronously between stream reads, the two attack shapes have different effects:

  • Field count drives CPU and event-loop blocking. A body of ~1,000,000 fields (a sub-10MB payload such as f0=v&f1=v&...) blocks the worker's event loop for several seconds while parsing, during which the worker serves no other request.
  • Field size drives memory. A single large field value (e.g. a 50MB value) is buffered in full to build the FormData, forcing memory allocation proportional to the request body.

The equivalent multipart/form-data request is correctly rejected with 400 Too many fields / 400 Field exceeded maximum size.

Impact

This Denial of service (DoS) vulnerability affects all applications built with Starlette (or FastAPI) that call request.form() on application/x-www-form-urlencoded requests. A single request with a very large number of fields blocks the event loop for several seconds, and a single request with a very large field forces unbounded memory allocation; in either case, parallel requests can render the service unusable. A reverse proxy that enforces a request body size limit reduces but does not eliminate the exposure, since a sub-10MB body is already enough to block the event loop.

Mitigation

Upgrade to a patched version, which forwards max_fields and max_part_size to the url-encoded parser and enforces them while parsing, raising before the oversized field or excess fields are accumulated. The defaults match multipart/form-data (max_fields=1000, max_part_size=1MB) and can be customized via request.form(max_fields=..., max_part_size=...).

high 7.5: CVE--2026--48818 Server-Side Request Forgery (SSRF)

Affected range<1.1.0
Fixed version1.1.0
CVSS Score7.5
CVSS VectorCVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N
EPSS Score0.368%
EPSS Percentile30th percentile
Description

Summary

When serving static files on Windows, StaticFiles resolves the requested path with os.path.realpath. If a UNC path (such as \\attacker.com\share) reaches the resolver, realpath causes the process to open a connection to the remote host over SMB (port 445). This is a server-side request forgery (SSRF) that leaks the service account's NTLMv2 credentials to the attacker-controlled host, which can then be cracked offline or relayed to other hosts.

Details

StaticFiles.lookup_path() joins the requested path onto the served directory and calls os.path.realpath on the result before checking containment with os.path.commonpath. On Windows, a UNC path is absolute, so os.path.join discards the served directory and realpath resolves the bare UNC path, triggering the outbound SMB connection and NTLM authentication before the containment check rejects the path. The HTTP response is a benign 404, but the credential disclosure has already happened. POSIX systems are not affected.

This only affects the default configuration (follow_symlink=False), which uses os.path.realpath. The follow_symlink=True branch uses os.path.abspath, which performs no I/O.

Impact

Applications running on Windows that serve files with StaticFiles (directly, or via a framework built on Starlette such as FastAPI) in the default configuration are affected. StaticFiles is typically unauthenticated, so any client can trigger the SMB connection and leak the service account's NTLMv2 hash. A secondary impact is discovering internal hosts reachable over SMB by timing responses for valid versus invalid addresses.

Mitigation

Applications not running on Windows are not affected. On Windows, serving static files through a dedicated web server (such as nginx or IIS) instead of StaticFiles avoids the issue. Blocking outbound SMB (port 445) from the application host prevents the credential disclosure even if a UNC path is resolved.

medium 6.5: CVE--2026--48710 Improper Validation of Unsafe Equivalence in Input

Affected range<=1.0.0
Fixed version1.0.1
CVSS Score6.5
CVSS VectorCVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:L/A:N
EPSS Score1.839%
EPSS Percentile77th percentile
Description

Summary

In affected versions, the HTTP Host request header was not validated before being used to reconstruct request.url. Because the routing algorithm relies on the raw HTTP path while request.url is rebuilt from the Host header, a malformed header could make request.url.path differ from the path that was actually requested. Middleware and endpoints that apply security restrictions based on request.url (rather than the raw scope path) could therefore be bypassed.

Details

When a client requests http://example.com/foo, it sends:

GET /foo HTTP/1.1
Host: example.com

Affected versions reconstructed the URL by concatenating http://{host}{path} and re-parsing the result. The Host value is only valid as a uri-host [ ":" port ] per RFC 9112 §3.2, where uri-host follows the restricted host grammar of RFC 3986 §3.2.2. When it contains characters outside that grammar - notably /, ?, or # - those characters move the path/query/fragment boundaries during re-parsing, so the parsed request.url.path no longer matches the path the server actually received. For example:

GET /foo HTTP/1.1
Host: example.com/abc?bar=

reconstructs to http://example.com/abc?bar=/foo, whose parsed path is /abc - even though routing used the real path /foo. The router still dispatches to /foo and the endpoint executes, but any middleware or code that reads request.url.path sees /abc, so path-based authorization checks can be bypassed.

Impact

Any application running an affected version that relies on request.url (or request.url.path) for security-sensitive decisions is affected. The most common case is middleware that gates access to certain path prefixes based on request.url.path. Deployments fronted by a proxy or load balancer are mitigated only if that proxy rejects or normalizes the malformed Host header before forwarding and the application does not trust attacker-controlled host headers (e.g. X-Forwarded-Host) elsewhere.

Mitigation

Upgrade to a patched version, which validates the Host header against the grammar of RFC 9112 §3.2 / RFC 3986 §3.2.2 when constructing request.url and falls back to scope["server"] for malformed values.

medium 5.3: CVE--2026--48817 Use of Externally-Controlled Input to Select Classes or Code ('Unsafe Reflection')

Affected range<1.1.0
Fixed version1.1.0
CVSS Score5.3
CVSS VectorCVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:L/A:N
EPSS Score0.213%
EPSS Percentile12th percentile
Description

Summary

When dispatching a request, HTTPEndpoint selects the handler by lowercasing the HTTP method and looking it up as an attribute with getattr, without restricting the lookup to a known set of HTTP verbs.

When an HTTPEndpoint subclass is registered through Route(...) without an explicit methods= argument, the route does not constrain the method and every method reaches the endpoint. If a non-standard HTTP method whose lowercased name matches an attribute on the endpoint subclass reaches the endpoint, that attribute is invoked as if it were a request handler. An attacker can use this to reach methods that were never meant to be HTTP handlers, such as internal helpers, without the authorization checks applied by the intended public handler.

Details

HTTPEndpoint uses the client-supplied method name to resolve an instance attribute, without validating it against the set of HTTP verbs the endpoint supports. A method such as _DO_DELETE therefore resolves an attribute like _do_delete and invokes it. Non-standard methods are valid RFC 9110 token methods, so an endpoint must not treat the method name as a trusted attribute selector.

Impact

An application is affected when all of the following hold:

  • It defines an HTTPEndpoint subclass and registers it via Route(...) without an explicit methods= argument.
  • The subclass defines additional methods whose names match a non-standard HTTP-method token shape and that accept a single request argument and return a response.

This also affects frameworks built on Starlette, like FastAPI.

Mitigation

Register HTTPEndpoint subclasses with an explicit methods= argument on the Route, listing only the HTTP verbs the endpoint supports. The route then rejects any other method with 405 Method Not Allowed before it reaches the endpoint, so non-standard methods cannot resolve an attribute.

low 3.7: CVE--2026--54282 Improper Input Validation

Affected range<1.3.0
Fixed version1.3.0
CVSS Score3.7
CVSS VectorCVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:L/A:N
EPSS Score0.187%
EPSS Percentile9th percentile
Description

Summary

In affected versions, the HTTP request path is not validated before being used to reconstruct request.url. Because request.url is rebuilt by concatenating {scheme}://{host}{path} and re-parsing the result, a path that does not begin with / (for example @<!-- -->google.com) moves the authority boundary during re-parsing, so request.url.hostname and request.url.netloc become attacker-controlled. Code that reads request.url.hostname (rather than the Host header or scope) can therefore be misled into trusting an attacker-supplied host.

Details

When a client requests a path that does not start with /:

GET @<!-- -->google.com HTTP/1.1
Host: localhost

affected versions reconstruct the URL as http://localhost@<!-- -->google.com. Per RFC 3986 §3.2.1, the substring before @ in the authority is userinfo, so re-parsing yields username = "localhost" and hostname = "google.com", with an empty path:

request.url          == "http://localhost@<!-- -->google.com"
request.url.hostname == "google.com"
request.url.path     == ""

The root cause is that the path is concatenated directly after the host without a separating /, and without validating that it begins with one. Only the Host header was validated when constructing request.url; the path was not.

This requires an ASGI server that forwards a request-target lacking a leading / into scope["path"].

Impact

Any application running an affected version that uses request.url, request.url.netloc, or request.url.hostname for a security-sensitive decision (host-based authorization, redirect/callback base, SSRF target, cache key, audit log) may be affected, when no fronting proxy or load balancer rejects the malformed request-target first.

Note that this is less exploitable than GHSA-86qp-5c8j-p5mr: there, the poison is carried in the Host header, so the real path still routes to a valid endpoint while request.url.path lies. Here, the poison must be carried in the path itself, and that path (@<!-- -->google.com) does not match any registered route, so routing returns 404 and no endpoint handler runs. The exposure is limited to code that reads request.url before routing - notably middleware - or in 404/exception handlers.

Mitigation

Upgrade to a patched version, which prevents the request path from crossing into the URL authority. The request above instead yields http://localhost/@<!-- -->google.com with request.url.hostname == "localhost".

critical: 0 high: 1 medium: 0 low: 0 ddtrace 3.19.8 (pypi)

pkg:pypi/ddtrace@3.19.8

high 7.5: CVE--2026--50271 Uncontrolled Resource Consumption

Affected range<4.8.2
Fixed version4.8.2
CVSS Score7.5
CVSS VectorCVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
EPSS Score0.441%
EPSS Percentile36th percentile
Description

Impact

Datadog tracing libraries that implement W3C baggage propagation parse incoming baggage HTTP headers without enforcing item-count or byte-size limits on the extract path. The DD_TRACE_BAGGAGE_MAX_ITEMS (default 64) and DD_TRACE_BAGGAGE_MAX_BYTES (default 8192) limits were applied only to baggage injection, not extraction. A remote, unauthenticated attacker can send a request whose baggage header contains an arbitrarily large number of comma-separated key-value pairs (or a single very large value). The tracer allocates a hash-map entry for each pair on every request, causing unbounded CPU and memory consumption and enabling a remote Denial of Service against any HTTP service that has the baggage propagation style enabled.
The baggage propagation style is enabled by default in most affected tracers, so any internet-facing service that has been instrumented with an affected tracer version is exposed unless the propagation style has been explicitly narrowed.

Patches

This is resolved in version 4.8.2 and later of the dd-trace-py library

Workarounds

If users cannot upgrade immediately:

  1. Disable baggage extraction by removing baggage from DD_TRACE_PROPAGATION_STYLE (or DD_TRACE_PROPAGATION_STYLE_EXTRACT if set independently).
  2. Cap the maximum HTTP request header size at an upstream proxy or web server (for example, Apache LimitRequestFieldSize, Nginx large_client_header_buffers, Envoy max_request_headers_kb).

Resources

Related upstream advisories:
opentelemetry-go GHSA-mh2q-q3fh-2475
opentelemetry-dotnet GHSA-g94r-2vxg-569j

critical: 0 high: 0 medium: 1 low: 0 busybox 1.37.0-r30 (apk)

pkg:apk/alpine/busybox@1.37.0-r30?os_name=alpine&os_version=3.23

medium : CVE--2025--60876

Affected range<=1.37.0-r30
Fixed versionNot Fixed
EPSS Score0.291%
EPSS Percentile21st percentile
Description
critical: 0 high: 0 medium: 0 low: 0 unspecified: 1golang.org/x/crypto 0.53.0 (golang)

pkg:golang/golang.org/x/crypto@0.53.0

unspecified : GO--2026--5932

Affected range>=0
Fixed versionNot Fixed
Description

The golang.org/x/crypto/openpgp package is unsafe by design, has numerous known security issues, is not maintained, and should not be used.

If you are required to interoperate with OpenPGP systems and need a maintained package, consider github.com/ProtonMail/go-crypto/openpgp which is a maintained fork that aims to be a drop-in replacement for this package.

@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown

🔍 Vulnerabilities of permitio/pdp-v2:next

📦 Image Reference permitio/pdp-v2:next
digestsha256:85c25de71ae174d5f951646ad15cd57645a5b8b4cdd41413c1749f6918532e04
vulnerabilitiescritical: 0 high: 0 medium: 0 low: 0
platformlinux/amd64
size133 MB
packages248
📦 Base Image alpine:3.23
also known as
  • 3.23.5
digestsha256:1beb0dc0a51de7ff38e3b5274078a2e0b81113ba5c7535e1a03d5913a5edbda3
vulnerabilitiescritical: 0 high: 0 medium: 1 low: 0

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new GitHub Actions workflow to redeploy all hosted/managed PDP ECS services after a published release by forcing a new ECS deployment once permitio/pdp-v2:latest matches the released tag’s digest.

Changes:

  • Introduces a standalone release: published + workflow_dispatch workflow that gates rollout on Docker Hub :latest matching the release tag digest.
  • Rolls a fixed matrix of 11 ECS services (max-parallel 3, fail-fast false), verifying steady state and full task replacement.
  • Produces a per-service artifact and a consolidated job summary table with pass/fail status.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/redeploy-pdp-fleet.yml
Comment on lines +229 to +246
set -uo pipefail

# `aws ecs wait services-stable` is hardcoded to 40 polls x 15s = 10 minutes.
# These services need longer: task boot, plus healthy_threshold 2 x interval 30
# on the ALB, plus the target groups' default 300s deregistration_delay while
# the old pair drains. 15 minutes leaves real headroom.
DEADLINE=$(( $(date +%s) + 900 ))

while :; do
SVC=$(aws ecs describe-services --cluster "$CL" --services "$SV" --output json)

read -r RUNNING DESIRED PENDING FAILED DEPLOYMENTS <<<"$(jq -r --arg d "$DID" '
.services[0] as $s
| ($s.deployments[] | select(.id == $d)) as $p
| [ $p.runningCount, $p.desiredCount, $p.pendingCount,
($p.failedTasks // 0), ($s.deployments | length) ] | @tsv' <<<"$SVC")"

echo "$SV running=$RUNNING/$DESIRED pending=$PENDING failedTasks=$FAILED deployments=$DEPLOYMENTS"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed in 82a0c06. I reproduced it before changing anything: with an empty or unmatched response, read assigns empty strings to all five fields, [ "" -ge 3 ] fails as a non-numeric comparison without set -e to catch it, and the loop spins to the 15 minute deadline having printed nothing useful.

One correction to the diagnosis, in the workflow's favour: it does not produce a false "stable". [ "$RUNNING" = "$DESIRED" ] is true for two empty strings, but the [ "$DEPLOYMENTS" = "1" ] half of the condition fails, so it never exits 0 on garbage. The bug is a silent spin, not a wrong success.

The missing set -e is deliberate — a transient describe-services failure has to be retried rather than abort the roll — so the fix is to make every failure path explicit instead:

  • describe-services failing is retried with the AWS error printed, and gives up after 10 consecutive failures rather than burning the whole deadline.
  • The service vanishing from the cluster fails immediately.
  • The deployment id no longer being present fails immediately and prints the deployments actually on the service. That is your second point, and it has a real trigger: a concurrent update-service or a terraform apply supersedes our deployment. It is caught with jq -e, which exits non-zero when the filter yields no output.

Every branch is driven by a mocked aws — transient error, service missing, deployment superseded, steady state, stalled rollout.

Addresses both findings from the Copilot review.

Dry runs no longer list or describe tasks. The snapshot step exists only to
give the post-deploy assertion a baseline, so it is now gated behind
DRY_RUN != 'true'. In its place a dry run does a read-only describe-services
that proves the role authenticates in the region and that the service exists
under the name the matrix claims - which is the point of a dry run - without
needing ecs:ListTasks or ecs:DescribeTasks.

The zero-running-tasks hard failure is downgraded to a warning. A service at
desiredCount 0, or one whose tasks are already down, is precisely a case
where forcing a new deployment is the right move rather than a reason to
abort. A wrong service name was never caught by that count anyway - it
surfaces as ServiceNotFoundException from the call itself. The replacement
assertion handles an empty baseline correctly: no pre-deploy ARN can survive
when there were none.

The steady-state loop no longer degrades into a silent spin. It deliberately
runs without `set -e` so a transient describe-services failure is retried
rather than aborting the roll, but that left three paths where an unreadable
response produced empty fields and looped to the 15 minute deadline without
ever printing why. Each is now an explicit branch:

- describe-services failing is retried with the AWS error printed, and gives
  up after 10 consecutive failures instead of burning the full deadline.
- The service vanishing from the cluster mid-deployment fails immediately.
- The deployment id no longer being present - superseded by a concurrent
  update-service or terraform apply - fails immediately and prints the
  deployments that are actually on the service. This is caught with `jq -e`,
  which exits non-zero when the filter yields no output.

Verified by driving every branch with a mocked aws: transient error, service
missing, deployment superseded, steady state and stalled rollout.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dshoen619 dshoen619 closed this Aug 12, 2026
@dshoen619
dshoen619 deleted the feat/release-fleet-redeploy branch August 12, 2026 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants