Skip to content

Commit 77da3d9

Browse files
fix: stream multipart upload in UploadArtifact to avoid unbounded memory allocation
Replace bytes.Buffer with io.Pipe() in UploadArtifact so multipart form data is streamed directly from disk to the HTTP request. This keeps memory usage constant regardless of artifact file size, preventing OOM kills on constrained CI/CD runners. Also replaces panic() calls with proper error returns in the upload path. Fixes #324 Signed-off-by: abhaygoudannavar <abhaysgoudnvr@gmail.com>
1 parent cfe2267 commit 77da3d9

2 files changed

Lines changed: 96 additions & 20 deletions

File tree

pkg/connectors/microcks_client.go

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -425,31 +425,41 @@ func (c *microcksClient) UploadArtifact(specificationFilePath string, mainArtifa
425425
}
426426
defer file.Close()
427427

428-
// Create a multipart request body, reading the file.
429-
body := &bytes.Buffer{}
430-
writer := multipart.NewWriter(body)
431-
part, err := writer.CreateFormFile("file", filepath.Base(specificationFilePath))
432-
if err != nil {
433-
return "", err
434-
}
435-
_, err = io.Copy(part, file)
436-
if err != nil {
437-
panic(err.Error())
438-
}
428+
// Use io.Pipe to stream the multipart form data directly to the HTTP
429+
// request without buffering the entire file in memory.
430+
pr, pw := io.Pipe()
431+
writer := multipart.NewWriter(pw)
432+
433+
// Write the multipart form data in a background goroutine so the pipe
434+
// reader can be consumed concurrently by the HTTP request.
435+
errCh := make(chan error, 1)
436+
go func() {
437+
defer pw.Close()
438+
439+
part, err := writer.CreateFormFile("file", filepath.Base(specificationFilePath))
440+
if err != nil {
441+
errCh <- err
442+
return
443+
}
444+
if _, err = io.Copy(part, file); err != nil {
445+
errCh <- err
446+
return
447+
}
439448

440-
// Add the mainArtifact flag to request.
441-
_ = writer.WriteField("mainArtifact", strconv.FormatBool(mainArtifact))
449+
// Add the mainArtifact flag to request.
450+
if err = writer.WriteField("mainArtifact", strconv.FormatBool(mainArtifact)); err != nil {
451+
errCh <- err
452+
return
453+
}
442454

443-
err = writer.Close()
444-
if err != nil {
445-
return "", err
446-
}
455+
errCh <- writer.Close()
456+
}()
447457

448458
// Ensure we have a correct URL.
449459
rel := &url.URL{Path: "artifact/upload"}
450460
u := c.APIURL.ResolveReference(rel)
451461

452-
req, err := http.NewRequest("POST", u.String(), body)
462+
req, err := http.NewRequest("POST", u.String(), pr)
453463
if err != nil {
454464
return "", err
455465
}
@@ -465,20 +475,25 @@ func (c *microcksClient) UploadArtifact(specificationFilePath string, mainArtifa
465475
}
466476
defer resp.Body.Close()
467477

478+
// Check for errors from the multipart writer goroutine.
479+
if pipeErr := <-errCh; pipeErr != nil {
480+
return "", fmt.Errorf("failed to write multipart form: %w", pipeErr)
481+
}
482+
468483
// Dump response if verbose required.
469484
config.DumpResponseIfRequired("Microcks for uploading artifact", resp, true)
470485

471486
respBody, err := io.ReadAll(resp.Body)
472487
if err != nil {
473-
panic(err.Error())
488+
return "", fmt.Errorf("failed to read upload response: %w", err)
474489
}
475490

476491
// Raise exception if not created.
477492
if resp.StatusCode != 201 {
478493
return "", errs.New(string(respBody))
479494
}
480495

481-
return string(respBody), err
496+
return string(respBody), nil
482497
}
483498

484499
func (c *microcksClient) DownloadArtifact(artifactURL string, mainArtifact bool, secret string) (string, error) {

pkg/connectors/microcks_client_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,73 @@
11
package connectors
22

33
import (
4+
"io"
45
"net/http"
56
"net/http/httptest"
7+
"os"
8+
"path/filepath"
69
"strings"
710
"testing"
811
)
912

13+
func TestUploadArtifactStreamsWithoutBuffering(t *testing.T) {
14+
const fileContent = `{"openapi":"3.0.0","info":{"title":"Test API","version":"1.0.0"}}`
15+
const expectedResponse = "artifact uploaded"
16+
17+
// Create a temporary file to simulate an API specification.
18+
tmpDir := t.TempDir()
19+
specPath := filepath.Join(tmpDir, "openapi.json")
20+
if err := os.WriteFile(specPath, []byte(fileContent), 0o600); err != nil {
21+
t.Fatalf("failed to create temp spec file: %v", err)
22+
}
23+
24+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
25+
if r.URL.Path != "/api/artifact/upload" {
26+
t.Fatalf("unexpected path: %s", r.URL.Path)
27+
}
28+
if r.Method != http.MethodPost {
29+
t.Fatalf("unexpected method: %s", r.Method)
30+
}
31+
32+
// Verify the multipart form contains the file.
33+
file, header, err := r.FormFile("file")
34+
if err != nil {
35+
t.Fatalf("failed to get form file: %v", err)
36+
}
37+
defer file.Close()
38+
39+
if header.Filename != "openapi.json" {
40+
t.Fatalf("unexpected filename: %s", header.Filename)
41+
}
42+
43+
body, err := io.ReadAll(file)
44+
if err != nil {
45+
t.Fatalf("failed to read uploaded file: %v", err)
46+
}
47+
if string(body) != fileContent {
48+
t.Fatalf("file content mismatch: got %q, want %q", string(body), fileContent)
49+
}
50+
51+
// Verify the mainArtifact field.
52+
if got := r.FormValue("mainArtifact"); got != "true" {
53+
t.Fatalf("unexpected mainArtifact value: %s", got)
54+
}
55+
56+
w.WriteHeader(http.StatusCreated)
57+
_, _ = w.Write([]byte(expectedResponse))
58+
}))
59+
defer server.Close()
60+
61+
client := NewMicrocksClient(server.URL)
62+
msg, err := client.UploadArtifact(specPath, true)
63+
if err != nil {
64+
t.Fatalf("UploadArtifact returned error: %v", err)
65+
}
66+
if strings.TrimSpace(msg) != expectedResponse {
67+
t.Fatalf("expected response %q, got %q", expectedResponse, msg)
68+
}
69+
}
70+
1071
func TestDownloadArtifactReturnsResponseBody(t *testing.T) {
1172
const expectedBody = "artifact downloaded"
1273

0 commit comments

Comments
 (0)