Skip to content

Commit 899a5ac

Browse files
committed
Make gzipped API requests opt-in
1 parent aa564cf commit 899a5ac

File tree

4 files changed

+136
-13
lines changed

4 files changed

+136
-13
lines changed

api/client.go

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ type Config struct {
5050
// If true timings for each request will be logged
5151
TraceHTTP bool
5252

53+
// If true, request bodies will be gzip compressed
54+
GzipAPIRequests bool
55+
5356
// The http client used, leave nil for the default
5457
HTTPClient *http.Client
5558

@@ -227,14 +230,20 @@ func (c *Client) newRequest(
227230
buf := new(bytes.Buffer)
228231

229232
if body != nil {
230-
gzipWriter := gzip.NewWriter(buf)
231-
err := json.NewEncoder(gzipWriter).Encode(body)
232-
if err != nil {
233-
return nil, err
234-
}
233+
if c.conf.GzipAPIRequests {
234+
gzipWriter := gzip.NewWriter(buf)
235+
err := json.NewEncoder(gzipWriter).Encode(body)
236+
if err != nil {
237+
return nil, err
238+
}
235239

236-
if err := gzipWriter.Close(); err != nil {
237-
return nil, fmt.Errorf("closing gzip writer: %w", err)
240+
if err := gzipWriter.Close(); err != nil {
241+
return nil, fmt.Errorf("closing gzip writer: %w", err)
242+
}
243+
} else {
244+
if err := json.NewEncoder(buf).Encode(body); err != nil {
245+
return nil, err
246+
}
238247
}
239248
}
240249

@@ -267,7 +276,9 @@ func (c *Client) newRequest(
267276

268277
if body != nil {
269278
req.Header.Add("Content-Type", "application/json")
270-
req.Header.Add("Content-Encoding", "gzip")
279+
if c.conf.GzipAPIRequests {
280+
req.Header.Add("Content-Encoding", "gzip")
281+
}
271282
}
272283

273284
return req, nil

api/client_test.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
package api_test
22

33
import (
4+
"bytes"
5+
"compress/gzip"
46
"context"
57
"crypto/tls"
68
"fmt"
9+
"io"
710
"net/http"
811
"net/http/httptest"
912
"strings"
@@ -78,6 +81,100 @@ func TestRegisteringAndConnectingClient(t *testing.T) {
7881
}
7982
}
8083

84+
func TestCompressionBehavior(t *testing.T) {
85+
tests := []struct {
86+
name string
87+
gzipAPIRequests bool
88+
expectCompressed bool
89+
}{
90+
{
91+
name: "compression disabled by default",
92+
gzipAPIRequests: false,
93+
expectCompressed: false,
94+
},
95+
{
96+
name: "compression enabled when requested",
97+
gzipAPIRequests: true,
98+
expectCompressed: true,
99+
},
100+
}
101+
102+
for _, tt := range tests {
103+
t.Run(tt.name, func(t *testing.T) {
104+
var requestBody []byte
105+
var isCompressed bool
106+
107+
server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
108+
// Read the request body
109+
body, err := io.ReadAll(req.Body)
110+
if err != nil {
111+
http.Error(rw, err.Error(), http.StatusInternalServerError)
112+
return
113+
}
114+
requestBody = body
115+
116+
// Check if the request is compressed
117+
isCompressed = req.Header.Get("Content-Encoding") == "gzip"
118+
119+
// If compressed, try to decompress to verify it's valid gzip
120+
if isCompressed {
121+
gzReader, err := gzip.NewReader(bytes.NewReader(body))
122+
if err != nil {
123+
http.Error(rw, "Invalid gzip: "+err.Error(), http.StatusBadRequest)
124+
return
125+
}
126+
defer gzReader.Close()
127+
128+
_, err = io.ReadAll(gzReader)
129+
if err != nil {
130+
http.Error(rw, "Failed to decompress: "+err.Error(), http.StatusBadRequest)
131+
return
132+
}
133+
}
134+
135+
rw.WriteHeader(http.StatusOK)
136+
fmt.Fprint(rw, `{}`)
137+
}))
138+
defer server.Close()
139+
140+
ctx := context.Background()
141+
client := api.NewClient(logger.Discard, api.Config{
142+
Endpoint: server.URL,
143+
Token: "test-token",
144+
GzipAPIRequests: tt.gzipAPIRequests,
145+
})
146+
147+
// Make a request that will have a body (pipeline upload)
148+
testPipeline := map[string]interface{}{
149+
"steps": []interface{}{
150+
map[string]interface{}{
151+
"command": "echo hello",
152+
},
153+
},
154+
}
155+
156+
_, err := client.UploadPipeline(ctx, "test-job-id", &api.PipelineChange{
157+
UUID: "test-uuid",
158+
Pipeline: testPipeline,
159+
})
160+
161+
if err != nil {
162+
t.Fatalf("UploadPipeline failed: %v", err)
163+
}
164+
165+
// Verify compression behavior matches expectation
166+
if isCompressed != tt.expectCompressed {
167+
t.Errorf("Expected compressed=%v, got compressed=%v", tt.expectCompressed, isCompressed)
168+
}
169+
170+
// Verify we received a non-empty body
171+
if len(requestBody) == 0 {
172+
t.Error("Expected non-empty request body")
173+
}
174+
})
175+
}
176+
}
177+
81178
func authToken(req *http.Request) string {
82179
return strings.TrimPrefix(req.Header.Get("Authorization"), "Token ")
83180
}

clicommand/agent_start.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,12 @@ type AgentStartConfig struct {
185185
NoMultipartArtifactUpload bool `cli:"no-multipart-artifact-upload"`
186186

187187
// API config
188-
DebugHTTP bool `cli:"debug-http"`
189-
TraceHTTP bool `cli:"trace-http"`
190-
Token string `cli:"token" validate:"required"`
191-
Endpoint string `cli:"endpoint" validate:"required"`
192-
NoHTTP2 bool `cli:"no-http2"`
188+
DebugHTTP bool `cli:"debug-http"`
189+
TraceHTTP bool `cli:"trace-http"`
190+
GzipAPIRequests bool `cli:"gzip-api-requests"`
191+
Token string `cli:"token" validate:"required"`
192+
Endpoint string `cli:"endpoint" validate:"required"`
193+
NoHTTP2 bool `cli:"no-http2"`
193194

194195
// Deprecated
195196
NoSSHFingerprintVerification bool `cli:"no-automatic-ssh-fingerprint-verification" deprecated-and-renamed-to:"NoSSHKeyscan"`
@@ -742,6 +743,7 @@ var AgentStartCommand = cli.Command{
742743
NoHTTP2Flag,
743744
DebugHTTPFlag,
744745
TraceHTTPFlag,
746+
GzipAPIRequestsFlag,
745747

746748
// Other shared flags
747749
RedactedVars,

clicommand/global.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,12 @@ var (
128128
EnvVar: "BUILDKITE_AGENT_EXPERIMENT",
129129
}
130130

131+
GzipAPIRequestsFlag = cli.BoolFlag{
132+
Name: "gzip-api-requests",
133+
Usage: "Enable gzip compression for API request bodies",
134+
EnvVar: "BUILDKITE_GZIP_API_REQUESTS",
135+
}
136+
131137
RedactedVars = cli.StringSliceFlag{
132138
Name: "redacted-vars",
133139
Usage: "Pattern of environment variable names containing sensitive values",
@@ -172,6 +178,7 @@ type APIConfig struct {
172178
TraceHTTP bool `cli:"trace-http"`
173179
Endpoint string `cli:"endpoint" validate:"required"`
174180
NoHTTP2 bool `cli:"no-http2"`
181+
GzipAPIRequests bool `cli:"gzip-api-requests"`
175182
}
176183

177184
func globalFlags() []cli.Flag {
@@ -191,6 +198,7 @@ func apiFlags() []cli.Flag {
191198
NoHTTP2Flag,
192199
DebugHTTPFlag,
193200
TraceHTTPFlag,
201+
GzipAPIRequestsFlag,
194202
}
195203
}
196204

@@ -355,6 +363,11 @@ func loadAPIClientConfig(cfg any, tokenField string) api.Config {
355363
conf.DisableHTTP2 = noHTTP2.(bool)
356364
}
357365

366+
gzipAPIRequests, err := reflections.GetField(cfg, "GzipAPIRequests")
367+
if err == nil {
368+
conf.GzipAPIRequests = gzipAPIRequests.(bool)
369+
}
370+
358371
return conf
359372
}
360373

0 commit comments

Comments
 (0)