Repository navigation
fix(deletes): read job ref from Location header (9 clients) - #7
Merged
wayneeseguin merged 14 commits intoMay 26, 2026
Merged
Conversation
CF v3 DELETE /v3/apps/{guid} is async: 202 Accepted with a Location
header pointing at /v3/jobs/{guid} and an empty body. The previous
signature discarded the header so callers couldn't poll for completion
- an anomaly versus OrganizationsClient / SpacesClient which already
return (*Job, error).
The async Location-header contract is defined in the CF v3 OpenAPI
spec under /v3/apps/{guid} delete. Impl parses the trailing GUID from
the header and returns a Job with GUID populated; consumers call
Jobs().Get (or Jobs().PollUntilComplete) for full state.
- Interface: AppCRUDClient.Delete now returns (*Job, error)
- Impl: parse Location header; guard missing/malformed values
- Test: simulate 202+Location; add missing-Location regression
- Batch + mock call sites updated for the new signature
- Example (examples/apps/main.go) prints the returned job guid
parse Location header
DELETE /v3/routes/{guid} and DELETE /v3/service_credential_bindings/{guid}
return 202 Accepted with empty body and a Location header pointing at
/v3/jobs/{jobGuid}. The previous impls json.Unmarshal'd the empty body
into capi.Job, which silently produced a Job with GUID="" — callers that
polled against /v3/jobs/ got 404s or errors instead of the actual job.
Same pattern as AppsClient.Delete (fix in 05c428d). ServiceCredentialBindings
is polymorphic: user-provided bindings return 204 No Content (sync, nil job);
managed bindings return 202 + Location (async). Tests cover both shapes
plus the missing-Location protocol-violation case.
Verified against:
- cloud_controller_ng main branch destroy actions (head :accepted,
'Location' => ...)
- CF v3 API docs 3.216/3.217 route + service_credential_binding deletes
CF v3 /v3/apps/{guid}/actions/{start|stop|restart|restage} are async:
CF responds 202 Accepted with a Location header pointing at
/v3/jobs/{jobGuid}. The prior implementation unmarshalled the empty
response body as an App (or Build, for restage) and discarded the
Location header, so callers got a zero-value resource and no way to
observe the action's completion.
Mirror the Delete fix: parse the Location header, extract the job
GUID, and return *Job. AppLifecycleClient interface, CLI callers, and
mocks updated in lockstep.
Restage switches from the client-side two-step workflow (list
packages + create build) back to the deprecated-but-still-supported
POST /actions/restage endpoint so all four lifecycle actions share
the same async-job shape. The two-step modern flow remains possible
via Packages().List + Builds().Create + Droplets().SetCurrent for
callers that need the build-state detail; restage() itself now lines
up with start/stop/restart for uniform callsite code.
POST /v3/processes/{guid}/actions/scale is async per the CF v3 API
spec: CF responds 202 Accepted with a Location header pointing at
/v3/jobs/{jobGuid}. Mirror the app-lifecycle actions (Start, Stop,
Restart, Restage) and return *Job so callers have a uniform async-
completion surface.
The prior implementation unmarshalled the response body as Process
and discarded the Location header, leaving callers with either a
stale Process or a zero value when CF sent no body.
AppLifecycleClient's Scale signature, tests, and the CLI + example
callers updated in lockstep. The CLI echoes the queued job GUID and
reports the user's requested scale targets — terminal state requires
polling Jobs().Get(job.GUID).
CF v3 transitioned lifecycle actions (start/stop/restart/restage) and
process scale from synchronous (200 + resource body) to asynchronous
(202 + Location → /v3/jobs/{jobGuid}) somewhere in the 3.18x–3.20x
range. The .3 tag assumed modern async only and errored with "no
Location header on async response" against older CF targets — we hit
this immediately running Stratos dev.61 against adepttech's 3.180.0.
AppsClient.postActionJob and ProcessesClient.Scale now treat missing
Location as synchronous completion and return (nil, nil), letting
callers branch on job == nil to synthesize a terminal-COMPLETE
response instead of polling a non-existent job. Matches the service-
binding delete pattern where user-provided bindings return 204 and
managed bindings return 202 + Location.
CF v3 has no /v3/apps/{guid}/actions/restage endpoint. It never did —
CF deliberately replaced the v2 atomic /v2/apps/{guid}/restage with the
builds resource, quoting v3.216 docs:
"The specialized /v2/apps/:guid/restage endpoint is replaced by the
builds resource. Builds allow finer-grained control and increased
flexibility when staging packages into droplets. The V3 API avoids
making assumptions about what users want to happen."
Our Apps().Restage() pointed at the non-existent v3 action URL
(inherited from fivetwenty-io/capi commit 255713a; see also:
https://github.com/cloudfoundry/cloud_controller_ng/wiki/How-to-restage-an-app-directly-using-V3-APIs-(with-examples)).
The method is unusable on every CF version. Removing it forces callers
to either use /v2/apps/{guid}/restage directly (still live per RFC-0032
until end of 2026) or compose primitives as cf-cli does.
CF CLI v8 restage reference sequence
------------------------------------
No-strategy (atomic downtime) path, as implemented in
command/v7/shared/app_stager.go + actor/v7action/:
1. GET /v3/apps?names=<n>&space_guids=<s> (resolve app)
2. GET /v3/packages?app_guids=<a>&states=READY
&order_by=-created_at&per_page=1 (newest READY pkg)
3. GET /v3/packages?guids=<p>&app_guids=<a> (verify)
4. POST /v3/builds body {"package":{"guid":"<p>"}}
5. GET /v3/builds/<build_guid> (poll until
state != STAGING; returns droplet_guid)
6. POST /v3/apps/<a>/actions/stop (if already running)
7. PATCH /v3/apps/<a>/relationships/current_droplet
body {"data":{"guid":"<droplet_guid>"}}
8. POST /v3/apps/<a>/actions/start
9. GET /v3/apps/<a>/processes
+ per-proc GET /v3/processes/<g>/stats (poll ready)
Strategy=rolling or canary replaces steps 6-8 with:
- POST /v3/deployments body {droplet, relationships.app, strategy,…}
- GET /v3/deployments/<d> (poll until DEPLOYED)
The primitives on fw-capi already cover every call above:
PackagesClient.List, BuildsClient.Create/Get, AppsClient.Stop/Start,
AppsClient.SetCurrentDroplet. Composition belongs to the caller or to
a future orchestration layer analogous to cf-cli's shared.AppStager.
Until that exists, Stratos routes restage through the v2 endpoint.
CF API responses carry meaningful per-link metadata in a `meta`
sub-object that the existing Link{Href,Method} pair drops. Add an
optional Meta map[string]interface{} so consumers can read it without
re-parsing the raw JSON.
Two concrete shapes Stratos and cf-cli both consume:
links.app_ssh.meta.host_key_fingerprint → SSH proxy host key
links.app_ssh.meta.oauth_client → UAA client_id ("ssh-proxy")
links.cloud_controller_v{2,3}.meta.version → API semver (e.g. 3.180.0)
The shape of meta varies by link, so a typed map keeps the API
extensible without locking us into a specific schema.
Backwards-compatible: omitempty means nil Meta produces wire-identical
output to pre-Meta versions. No existing call site references Meta.
Tests:
- app_ssh round-trips host_key_fingerprint + oauth_client
- cloud_controller_v3.meta.version is captured
- Marshalled output without Meta does not emit "meta" key
Add GET /v3/apps/:guid/revisions support to RevisionsClient, mirroring BuildsClient.ListForApp. Test uses existing httptest + client.New convention. Refs #4
Add GET /v3/apps/:guid/revisions/deployed support. Returns ListResponse[Revision] because mid-rolling-deployment more than one revision can be deployed simultaneously. Refs #5
The struct doubles as both response shape (CF returns the
server-assigned GUID on GET) and request shape on
POST /v3/routes/{guid}/destinations. CF v3's destinations-create
schema accepts only "app", "weight", "port" and "protocol" per
destination. Without omitempty on GUID, a zero-value marshal emits
{"guid":""} and CF answers 422 CF-UnprocessableEntity:
Destinations[0]: must have only "app" and optionally "weight",
"port" or "protocol".
Two-line fix on the json/yaml tag plus a doc comment recording the
constraint so future fields hold the same line.
v3 returns joined resources as a top-level `included` object on list responses when the request used `?include=...`. The Go type ListResponse[T] dropped the field, so callers couldn't read the joined resources from a single CAPI call and had to issue separate filter-by-guids batch lookups instead. Add `Included map[string][]json.RawMessage` (omitempty) to model v3's heterogeneous included block. Late-decode pattern — callers re-decode the bucket they want via json.Unmarshal into the concrete type. Tests cover round-trip, late-decode of one entry, and omitempty for handlers that don't use ?include=.
CF V3 DELETE endpoints return 202 Accepted with an empty body and the
async job reference in the Location header (`Location: /v3/jobs/{guid}`).
The fix-apps-delete work applied this for AppsClient.Delete. The same
9 sibling Delete methods still parsed the empty body as JSON, yielding
a zero-valued capi.Job and silently broken async-delete handling for
every downstream caller.
Apply the same Location-extraction pattern from apps.go to:
- OrganizationsClient.Delete
- SpacesClient.Delete
- DomainsClient.Delete
- UsersClient.Delete
- SecurityGroupsClient.Delete
- ServiceBrokersClient.Delete
- ServiceInstancesClient.Delete
- ServiceRouteBindingsClient.Delete
- BuildpacksClient.Delete
Service instances are special: purge=true is a SYNC delete (204 No
Content, no Location). Callers receive `nil, nil` on that path and
treat it as "delete completed, no job polling needed."
Test fixtures updated to mock the 202 + Location contract instead of
encoding a phantom Job into the body, and the shared RunJobDeleteTest
helper now writes the Location header only (the operationType param
is preserved for callsite documentation but no longer asserted on the
returned Job — Operation/State live in /v3/jobs/{guid}'s response body,
not in the Location-derived stub).
Contributor
|
Thank you for this |
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.
Summary
CF V3 DELETE endpoints return 202 Accepted with an empty body and the async job reference in the
Locationheader (Location: /v3/jobs/{guid}). PR #2 retrofitted Apps + Processes + Roles + Routes + ServiceCredentialBindings. The same bug pattern (parse empty body as JSON, return zero-valuedcapi.Job) still exists in 9 more sibling Delete methods.This PR completes the sweep — same pattern, no new mechanism, just consistency across the remaining clients.
Changes
Apply the same Location-extraction pattern from
apps.go:Deleteto:OrganizationsClient.DeleteSpacesClient.DeleteDomainsClient.DeleteUsersClient.DeleteSecurityGroupsClient.DeleteServiceBrokersClient.DeleteServiceInstancesClient.Delete(pluspurge=truesync path — returnsnil, nilfor 204 No Content)ServiceRouteBindingsClient.DeleteBuildpacksClient.DeleteTest fixtures updated to mock the 202 + Location contract instead of encoding a phantom Job into the body. Shared
RunJobDeleteTesthelper now writes the Location header only (operationType param preserved for callsite documentation but no longer asserted on the returned Job — Operation/State live in/v3/jobs/{guid}'s response body, not in the Location-derived stub).How this was surfaced
A downstream consumer (the Stratos UI via jetstream's CAPI proxy) hit
502: org delete: no job id returned from CFon the first delete attempt and404on retry. CF V3 had actually processed the first delete — the client just couldn't see the job ref because it was parsing the empty 202 body as JSON.Test plan
go build ./...go test ./internal/client/ -run Deletego test ./...(full suite green)