feat: new edge function upload handling#650
Conversation
✅ Deploy Preview for open-api ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR adds support for deploying Edge Functions. The swagger spec gains a new Estimated code review effort: 3 (Moderate) | ~30 minutes Changes
Sequence Diagram(s)sequenceDiagram
participant DoDeploy
participant bundleEdgeFunctions
participant uploadFiles
participant Client
participant Server
DoDeploy->>bundleEdgeFunctions: parse manifest.json, hash bundles
bundleEdgeFunctions-->>DoDeploy: edgeFunctions bundle set
DoDeploy->>uploadFiles: upload edgeFunctionUpload for RequiredEdgeFunctions
uploadFiles->>Client: UploadDeployEdgeFunction(deployID, codeSha, fileBody)
Client->>Server: PUT /deploys/{deploy_id}/edge_functions/{code_sha}
Server-->>Client: 200 OK or error
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
||
| // bundleEdgeFunctions reads the edge-bundler manifest from edgeFunctionsDir and turns each bundle it | ||
| // lists into an uploadable FileBundle. The deploy declares these as its edge_functions map | ||
| // ({format => code_sha}); the server replies with the subset (required_edge_functions) not already |
There was a problem hiding this comment.
Note - in here we send everything in edge-bundler manifest - including eszip bundles. Bitballoon intentionally only cares about tar ( https://github.com/netlify/bitballoon/blob/ad4d323eb9f9fabf68c5518e4ce701ca9faa964e/app/models/deploy.rb#L1114-L1119 )
Goal of this was to make clients to not to make such decisions and instead have our backed alone decide about what goes through which upload channel.
This way it's much easier for us to control things. It's less relevant for porcelain client which we can rollout with buildbot anytime we want. But the same will be replicated for CLI where impact of this decision is bigger, because we can't fully control over which version is actually used and this should allow us to just eventually turn off tar uploads going through static files uploads channel
pieh
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
swagger.yml (1)
1668-1694: 🧹 Nitpick | 🔵 TrivialConfirm downstream service ownership for this contract.
Per the linked
netlify/blueprintsdocs, edge-function deploy/upload appears to be owned by the Functions Origin service (Nimble/Kraft cloud), not just netlify-server. Given the PR description flags "placeholder assumptions that may change" pending Bitballoon/netlify-server work, worth confirming this exact path/schema shape (/deploys/{deploy_id}/edge_functions/{code_sha},required_edge_functions/edge_functions) is the agreed contract with whichever service ultimately implements it, to avoid a breaking spec change later.Also applies to: 4773-4779, 4879-4883
🤖 Prompt for 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. In `@swagger.yml` around lines 1668 - 1694, The edge-function upload contract in the uploadDeployEdgeFunction path should be verified against the downstream owner before it is finalized, since the current request/response shape may not match the service that will actually implement it. Confirm that the /deploys/{deploy_id}/edge_functions/{code_sha} route, the file_body binary body, and any related required_edge_functions/edge_functions schema expectations are the agreed API contract with the Functions Origin service; if not, adjust the Swagger definition and any referenced schemas together to match the authoritative implementation.Source: Linked repositories
🤖 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.
Nitpick comments:
In `@swagger.yml`:
- Around line 1668-1694: The edge-function upload contract in the
uploadDeployEdgeFunction path should be verified against the downstream owner
before it is finalized, since the current request/response shape may not match
the service that will actually implement it. Confirm that the
/deploys/{deploy_id}/edge_functions/{code_sha} route, the file_body binary body,
and any related required_edge_functions/edge_functions schema expectations are
the agreed API contract with the Functions Origin service; if not, adjust the
Swagger definition and any referenced schemas together to match the
authoritative implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f2bb8278-b9ec-4a55-a3ae-4882f0fca438
📒 Files selected for processing (9)
go/models/deploy.gogo/models/deploy_files.gogo/plumbing/operations/operations_client.gogo/plumbing/operations/upload_deploy_edge_function_parameters.gogo/plumbing/operations/upload_deploy_edge_function_responses.gogo/porcelain/deploy.gogo/porcelain/deploy_test.gogo/porcelain/edge_functions_manifest.goswagger.yml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
netlify/blueprints(manual)
| /deploys/{deploy_id}/edge_functions/{code_sha}: | ||
| put: | ||
| operationId: uploadDeployEdgeFunction | ||
| tags: [edgeFunction] | ||
| consumes: | ||
| - application/octet-stream | ||
| parameters: | ||
| - name: deploy_id | ||
| type: string | ||
| in: path | ||
| required: true | ||
| - name: code_sha | ||
| type: string | ||
| in: path | ||
| required: true | ||
| - name: file_body | ||
| in: body | ||
| schema: | ||
| type: string | ||
| format: binary | ||
| required: true | ||
| - $ref: '#/parameters/retryCount' | ||
| responses: | ||
| '200': | ||
| description: OK | ||
| default: | ||
| $ref: '#/responses/error' |
There was a problem hiding this comment.
I think it's worth doing some extra check here, but here's claude's summary of "correctness" of this:
The endpoint definition is correct — it will interoperate properly with the server. I traced every part of the swagger spec against the handler. Here's the validation:
┌─────────────────────────────────────────────────────┬───────────────────────────────────────────────────────────────────────────────────────────────┬───────────────────────┐
│ Swagger (swagger.yml:1668-1694) │ Server (request_edge_function_upload.go / server.go) │ Match │
├─────────────────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────┼───────────────────────┤
│ Path /deploys/{deploy_id}/edge_functions/{code_sha} │ Regex ^/api/v1/deploys/(?P<deploy_id>[a-f0-9]+)/edge_functions/(?P<code_sha>[a-f0-9]+)$ (:19) │ ✅ │
├─────────────────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────┼───────────────────────┤
│ Method put │ Test uses http.MethodPut → 200 (:703-705) │ ✅ (works) │
├─────────────────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────┼───────────────────────┤
│ consumes: application/octet-stream │ Body streamed raw to UploadEdgeFunctionBundle (:82) │ ✅ │
├─────────────────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────┼───────────────────────┤
│ file_body binary, required │ Reads req body │ ✅ │
├─────────────────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────┼───────────────────────┤
│ retryCount param │ not read by handler │ ⚠️ ignored (harmless) │
├─────────────────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────┼───────────────────────┤
│ Response 200, no schema (empty body) │ Returns nil → default 200, empty body (:93); test asserts StatusOK │ ✅ │
└─────────────────────────────────────────────────────┴───────────────────────────────────────────────────────────────────────────────────────────────┴───────────────────────┘
Two minor, non-blocking discrepancies worth knowing
1. Method isn't enforced server-side. The dispatch at server.go:657 matches on URL path only (isFOEdgeFunctionUploadPath(req.URL.Path)), with no method check — unlike the swagger which declares put. So the server would actually accept any verb on that path. This isn't a bug for you: the generated client always issues PUT, so declaring put in swagger is the correct, tighter contract. Just don't expect a GET/POST to that path to be rejected with a 405.
2. retryCount is sent but ignored. Your client sends X-Nf-Retry-Count (params.WithXNfRetryCount), and the swagger declares the retryCount param — but handleFOEdgeFunctionUpload never reads it. So retries work (the client re-PUTs), but the retry-count header has no server-side effect on this endpoint (no metrics/dedup keyed on it). Contrast with the function-upload path, which does thread it through. Not a correctness issue — worth a mental note if you ever expect the server to behave differently on retries.
I did use - $ref: '#/parameters/retryCount' to mimic function uploads - netlify-server similarly doesn't do anything with it - so it's a bit useless, but also seems harmless to have it ... at least it will be consistent with already existing functions definition
🤖 I have created a release *beep* *boop* --- ## [2.57.0](v2.56.1...v2.57.0) (2026-07-07) ### Features * new edge function upload handling ([#650](#650)) ([223e2de](223e2de)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: token-generator-app[bot] <82042599+token-generator-app[bot]@users.noreply.github.com> Co-authored-by: token-generator-app[bot] <token-generator-app[bot]@users.noreply.github.com>
2 participants
Relies on https://github.com/netlify/bitballoon/pull/22198 (merged) + https://github.com/netlify/netlify-server/pull/4267 (also merged)
Part of https://linear.app/netlify/issue/RUN-2937/implement-end-to-end-env-vars-plumbing-for-functions