-
Notifications
You must be signed in to change notification settings - Fork 0
feat(api): add POST /volumes/from-archive endpoint for multipart uploads #80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Split volume creation into two endpoints so Stainless can generate proper SDK methods for both: - POST /volumes (JSON) - create empty volume - POST /volumes/from-archive (multipart) - create volume from tar.gz This allows the Go SDK to generate a VolumeService.NewFromArchive() method with proper Content io.Reader field and MarshalMultipart() support, similar to how BuildService.New() works. Previously, having both content types on a single endpoint caused Stainless to only generate the JSON variant, requiring manual multipart request construction in client code.
Maps to the new POST /volumes/from-archive endpoint which will generate VolumeService.NewFromArchive() with multipart support.
✱ Stainless preview buildsThis PR will update the Edit this comment to update it. It will appear in the SDK's changelogs. ⚡ hypeman-go studio · conflict
✅ hypeman-typescript studio · code · diff
⚡ hypeman-cli studio · conflict
This comment is auto-generated by GitHub Actions and is automatically kept up to date as you push. |
- Regenerate oapi code from updated OpenAPI spec - Simplify CreateVolume handler to JSON-only (empty volumes) - Add CreateVolumeFromArchive handler for multipart uploads - Remove unused multipart import from volumes.go The new endpoint POST /volumes/from-archive handles creating volumes pre-populated with content from tar.gz archives.
- Keep multipart/form-data support on POST /volumes (marked as deprecated) - New integrations should use POST /volumes/from-archive instead - Update handler to support both JSON and multipart on /volumes - Regenerate oapi code This ensures existing clients using POST /volumes with multipart continue to work while new clients use the dedicated endpoint.
Buffer the content field to a temp file during parsing to decouple field order from validation. This fixes misleading "name is required" errors when clients send content before other form fields. The multipart/form-data spec doesn't guarantee field ordering, so servers should handle fields in any order. The previous implementation required name and size_gb to arrive before content. Changes: - Add parseVolumeMultipartForm helper that buffers content to temp file - Validate required fields after all fields are parsed - Clean up temp files via defer form.Close() - Apply fix to both CreateVolumeFromArchive and deprecated POST /volumes
Prevent temp file leaks by checking if ContentFile is already set before creating a new temp file. If a client sends multiple content fields, the request is rejected with a clear error message.
The Stainless config referenced onkernel/hypeman-go but the actual SDK repo is kernel/hypeman-go. This caused mixed import paths in the generated code (some imports used onkernel, others used kernel).
sjmiller609
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can do this without an intermediate file copy, and also I realized there is a pre-existing issue with allowing arbitrarily large volumes to be created but we probably want to limit that.
cmd/api/api/volumes.go
Outdated
| tempFile, err := os.CreateTemp("", "volume-archive-*.tar.gz") | ||
| if err != nil { | ||
| form.Close() | ||
| return nil, &formError{Code: "internal_error", Message: "failed to create temp file"} | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before this change, we have the whole archive being streamed directly into the volume. But this introduces an intermediate write to disk. So it instead works like stream into a local file, then copy from the file into the volume. That is slower in comparison to streaming from the HTTP request into the volume without an intermediate step.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with the original approach is that it assumed an order of the fields arriving but multi part HTTP uploads don't necessarily always come in the same field order. So I think part of this fix is to get all the fields before deciding what to do next. However, I think this approach is susceptible to arbitrarily large uploads so we need to think about that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's a better approach to put name and size_gb in the query parameters for the post request.
/volumes/from-archive:
post:
parameters:
- name: name
in: query
required: true
schema:
type: string
- name: size_gb
in: query
required: true
schema:
type: integer
requestBody:
required: true
content:
application/gzip:
schema:
type: string
format: binary
this allows the information for size_gb and name to exist when it's needed. we can still stream the data into the volume directly from HTTP request.
Also I think we should add a new environment variable MAX_VOLUME_SIZE used to control the max permitted size for any individual volume, and respect this when creating volumes, also respecting the existing MAX_TOTAL_VOLUME_STORAGE, which is the aggregated total volume storage
cmd/api/api/volumes.go
Outdated
| // parseVolumeMultipartForm parses a multipart form for volume creation. | ||
| // It buffers the content field to a temp file to handle any field order. | ||
| // Caller must call form.Close() to clean up temp files. | ||
| func parseVolumeMultipartForm(multipartReader *multipart.Reader) (*volumeMultipartForm, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think somewhat complicated logic should always live down in the services layer instead of in handler code files. handler code files should just be for mapping from API to domain types and error mapping, and things like that.
cmd/api/api/volumes.go
Outdated
| func (s *ApiService) createVolumeFromMultipart(ctx context.Context, multipartReader *multipart.Reader) (oapi.CreateVolumeResponseObject, error) { | ||
| // createVolumeFromMultipartDeprecated handles the deprecated multipart form on POST /volumes | ||
| // New integrations should use POST /volumes/from-archive instead | ||
| func (s *ApiService) createVolumeFromMultipartDeprecated(ctx context.Context, multipartReader *multipart.Reader) (oapi.CreateVolumeResponseObject, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of deprecating it, just delete it I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is technically a breaking change in staging and prod (for kernel org) if we simply delete this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but I guess that is fine
openapi.yaml
Outdated
| - Multipart form (DEPRECATED): Creates a volume from a tar.gz archive. | ||
| Use POST /volumes/from-archive instead for new integrations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should just delete it instead of deprecating it
BREAKING CHANGE: Remove deprecated multipart support from POST /volumes Changes: - POST /volumes now only accepts JSON body (empty volumes) - POST /volumes/from-archive uses query params (name, size_gb, id) and application/gzip body for direct streaming - Remove temp file buffering - archive streams directly to volume - Simplify handler code significantly This approach: - Avoids multipart field ordering issues entirely - Enables direct streaming (better performance) - Cleaner API design per reviewer feedback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
In Go's net/http, r.Body is never nil - empty requests use http.NoBody. The nil check was unreachable dead code. Empty/invalid archives will fail with a clear gzip error from the volume manager instead.
Summary
Split volume creation into two separate endpoints so Stainless can generate proper SDK methods for both use cases:
POST /volumes(JSON body) - creates an empty volume of specified sizePOST /volumes/from-archive(multipart/form-data) - creates a volume pre-populated from a tar.gz archiveProblem
The previous OpenAPI spec defined
POST /volumeswith bothapplication/jsonandmultipart/form-datacontent types. When Stainless generates SDKs from specs with multiple content types on the same endpoint, it picks one (typically JSON) and ignores the others. This resulted in:VolumeNewParamsonly having JSON fields (name,size_gb,id)Content io.Readerfield for the archive binaryMarshalMultipart()method generatedThis forced client code (like Kernel API) to manually construct multipart requests for "create volume from archive" functionality.
Solution
By splitting into two endpoints, Stainless will generate:
Go SDK:
client.Volumes.New()- JSON body, for empty volumesclient.Volumes.NewFromArchive()- multipart withContent io.ReaderfieldThis mirrors how
client.Builds.New()already works with itsSource io.Readerfield.Changes
openapi.yaml:
POST /volumesto JSON-onlyPOST /volumes/from-archiveendpoint with multipart-only schemastainless.yaml:
create_from_archive: post /volumes/from-archivemethod mappingServer-side changes needed
The Hypeman server will need to add a handler for
POST /volumes/from-archivethat performs the same logic as the current multipart handling inPOST /volumes. The existingPOST /volumescan be simplified to only handle JSON bodies.Test plan
VolumeService.NewFromArchive()withContent io.ReaderNote
Medium Risk
Changes the public API surface (request/route split) and regenerates large portions of OpenAPI-generated server/client code, which can introduce subtle compatibility or routing/parsing regressions despite limited underlying business logic changes.
Overview
Splits volume creation into two endpoints:
POST /volumesnow accepts only JSON to create an empty volume, and a newPOST /volumes/from-archivestreams a tar.gz request body (withname,size_gb, optionalidas query params) to create a pre-populated volume.Updates the API implementation and generated
lib/oapiserver/client bindings to remove multipart handling fromCreateVolume, addCreateVolumeFromArchiverequest/response types, routing, and client helpers/parsers, and adjusts unit tests to the new request shape.Refreshes
openapi.yamland embedded swagger and updatesstainless.yamlrepo/package settings plus a newcreate_from_archivemethod mapping for SDK generation.Written by Cursor Bugbot for commit 2466fb7. This will update automatically on new commits. Configure here.