Skip to content

Commit 179b748

Browse files
maelvlsmaelvls
committed
gzip: key pair and venafi connection modes now use gzip compression
For context, we have added gzip compression for the request bodies of data sent to the Venafi Control Plane API because we have hit 413 errors in NGINX and want to reduce the network load caused by the Agent for intermediate proxies and for our NGINX frontend (it buffers requests at the moment).
1 parent 3e33b61 commit 179b748

File tree

5 files changed

+254
-27
lines changed

5 files changed

+254
-27
lines changed

‎pkg/agent/config.go‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ import (
2323
"github.com/jetstack/preflight/pkg/version"
2424
)
2525

26-
// Config wraps the options for a run of the agent.
26+
// Config defines the YAML configuration file that you can pass using
27+
// `--config-file` or `-c`.
2728
type Config struct {
2829
// Deprecated: Schedule doesn't do anything. Use `period` instead.
2930
Schedule string `yaml:"schedule"`
@@ -159,6 +160,11 @@ type AgentCmdFlags struct {
159160

160161
// Prometheus (--enable-metrics) enables the Prometheus metrics server.
161162
Prometheus bool
163+
164+
// DisableCompression (--disable-compression) disables the GZIP compression
165+
// when uploading the data. Useful for debugging purposes, or when an
166+
// intermediate proxy doesn't like compressed data.
167+
DisableCompression bool
162168
}
163169

164170
func InitAgentCmdFlags(c *cobra.Command, cfg *AgentCmdFlags) {
@@ -285,6 +291,12 @@ func InitAgentCmdFlags(c *cobra.Command, cfg *AgentCmdFlags) {
285291
"Enables Prometheus metrics server on the agent (port: 8081).",
286292
)
287293

294+
c.PersistentFlags().BoolVar(
295+
&cfg.DisableCompression,
296+
"disable-compression",
297+
false,
298+
"Disables GZIP compression when uploading the data. Useful for debugging purposes or when an intermediate proxy doesn't like compressed data.",
299+
)
288300
}
289301

290302
type AuthMode string
@@ -326,6 +338,9 @@ type CombinedConfig struct {
326338
VenConnName string
327339
VenConnNS string
328340

341+
// VenafiCloudKeypair and VenafiCloudVenafiConnection modes only.
342+
DisableCompression bool
343+
329344
// Only used for testing purposes.
330345
OutputPath string
331346
InputPath string
@@ -558,6 +573,14 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
558573
}
559574
}
560575

576+
// Validation of --disable-compression.
577+
{
578+
if flags.DisableCompression && res.AuthMode != VenafiCloudKeypair && res.AuthMode != VenafiCloudVenafiConnection {
579+
errs = multierror.Append(errs, fmt.Errorf("--disable-compression can only be used with the %s and %s modes", VenafiCloudKeypair, VenafiCloudVenafiConnection))
580+
}
581+
res.DisableCompression = flags.DisableCompression
582+
}
583+
561584
if errs != nil {
562585
return CombinedConfig{}, nil, errs
563586
}
@@ -652,7 +675,7 @@ func validateCredsAndCreateClient(log *log.Logger, flagCredentialsPath, flagClie
652675
break // Don't continue with the client if kubeconfig wasn't loaded.
653676
}
654677

655-
preflightClient, err = client.NewVenConnClient(restCfg, metadata, cfg.InstallNS, cfg.VenConnName, cfg.VenConnNS, nil)
678+
preflightClient, err = client.NewVenConnClient(restCfg, metadata, cfg.InstallNS, cfg.VenConnName, cfg.VenConnNS, nil, cfg.DisableCompression)
656679
if err != nil {
657680
errs = multierror.Append(errs, err)
658681
}
@@ -710,7 +733,7 @@ func createCredentialClient(log *log.Logger, credentials client.Credentials, cfg
710733
log.Println("Loading upload_path from \"venafi-cloud\" configuration.")
711734
uploadPath = cfg.UploadPath
712735
}
713-
return client.NewVenafiCloudClient(agentMetadata, creds, cfg.Server, uploaderID, uploadPath)
736+
return client.NewVenafiCloudClient(agentMetadata, creds, cfg.Server, uploaderID, uploadPath, cfg.DisableCompression)
714737

715738
case *client.OAuthCredentials:
716739
return client.NewOAuthClient(agentMetadata, creds, cfg.Server)

‎pkg/agent/config_test.go‎

Lines changed: 142 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package agent
22

33
import (
44
"bytes"
5+
"compress/gzip"
56
"context"
67
"fmt"
78
"io"
@@ -373,6 +374,19 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
373374
assert.IsType(t, &client.OAuthClient{}, cl)
374375
})
375376

377+
t.Run("jetstack-secure-oauth-auth: can't use --disable-compression", func(t *testing.T) {
378+
path := withFile(t, `{"user_id":"fpp2624799349@affectionate-hertz6.platform.jetstack.io","user_secret":"foo","client_id": "k3TrDbfLhCgnpAbOiiT2kIE1AbovKzjo","client_secret": "f39w_3KT9Vp0VhzcPzvh-uVbudzqCFmHER3Huj0dvHgJwVrjxsoOQPIw_1SDiCfa","auth_server_domain":"auth.jetstack.io"}`)
379+
_, _, err := ValidateAndCombineConfig(discardLogs(),
380+
withConfig(testutil.Undent(`
381+
server: https://api.venafi.eu
382+
period: 1h
383+
organization_id: foo
384+
cluster_id: bar
385+
`)),
386+
withCmdLineFlags("--disable-compression", "--credentials-file", path))
387+
require.EqualError(t, err, "1 error occurred:\n\t* --disable-compression can only be used with the Venafi Cloud Key Pair Service Account and Venafi Cloud VenafiConnection modes\n\n")
388+
})
389+
376390
t.Run("jetstack-secure-oauth-auth: --credential-file used but file is missing", func(t *testing.T) {
377391
t.Setenv("POD_NAMESPACE", "venafi")
378392
got, _, err := ValidateAndCombineConfig(discardLogs(),
@@ -632,6 +646,81 @@ func Test_ValidateAndCombineConfig_VenafiCloudKeyPair(t *testing.T) {
632646
err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: "test cluster name"})
633647
require.NoError(t, err)
634648
})
649+
650+
t.Run("the request body is compressed", func(t *testing.T) {
651+
srv, cert, setVenafiCloudAssert := testutil.FakeVenafiCloud(t)
652+
setVenafiCloudAssert(func(t testing.TB, gotReq *http.Request) {
653+
if gotReq.URL.Path == "/v1/oauth/token/serviceaccount" {
654+
return
655+
}
656+
assert.Equal(t, "/v1/tlspk/upload/clusterdata/no", gotReq.URL.Path)
657+
658+
// Let's check that the body is compressed as expected.
659+
assert.Equal(t, "gzip", gotReq.Header.Get("Content-Encoding"))
660+
uncompressR, err := gzip.NewReader(gotReq.Body)
661+
require.NoError(t, err, "body might not be compressed")
662+
defer uncompressR.Close()
663+
uncompressed, err := io.ReadAll(uncompressR)
664+
require.NoError(t, err)
665+
assert.Contains(t, string(uncompressed), `{"agent_metadata":{"version":"development","cluster_id":"test cluster name"}`)
666+
})
667+
privKeyPath := withFile(t, fakePrivKeyPEM)
668+
got, cl, err := ValidateAndCombineConfig(discardLogs(),
669+
withConfig(testutil.Undent(`
670+
server: `+srv.URL+`
671+
period: 1h
672+
cluster_id: "test cluster name"
673+
venafi-cloud:
674+
uploader_id: no
675+
upload_path: /v1/tlspk/upload/clusterdata
676+
`)),
677+
withCmdLineFlags("--client-id", "5bc7d07c-45da-11ef-a878-523f1e1d7de1", "--private-key-path", privKeyPath),
678+
)
679+
testutil.TrustCA(t, cl, cert)
680+
assert.Equal(t, VenafiCloudKeypair, got.AuthMode)
681+
require.NoError(t, err)
682+
683+
err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: "test cluster name"})
684+
require.NoError(t, err)
685+
})
686+
687+
t.Run("--disable-compression works", func(t *testing.T) {
688+
srv, cert, setVenafiCloudAssert := testutil.FakeVenafiCloud(t)
689+
setVenafiCloudAssert(func(t testing.TB, gotReq *http.Request) {
690+
// Only care about /v1/tlspk/upload/clusterdata/:uploader_id?name=
691+
if gotReq.URL.Path == "/v1/oauth/token/serviceaccount" {
692+
return
693+
}
694+
695+
assert.Equal(t, "/v1/tlspk/upload/clusterdata/no", gotReq.URL.Path)
696+
697+
// Let's check that the body isn't compressed.
698+
assert.Equal(t, "", gotReq.Header.Get("Content-Encoding"))
699+
b := new(bytes.Buffer)
700+
_, err := b.ReadFrom(gotReq.Body)
701+
require.NoError(t, err)
702+
assert.Contains(t, b.String(), `{"agent_metadata":{"version":"development","cluster_id":"test cluster name"}`)
703+
})
704+
705+
privKeyPath := withFile(t, fakePrivKeyPEM)
706+
got, cl, err := ValidateAndCombineConfig(discardLogs(),
707+
withConfig(testutil.Undent(`
708+
server: `+srv.URL+`
709+
period: 1h
710+
cluster_id: "test cluster name"
711+
venafi-cloud:
712+
uploader_id: no
713+
upload_path: /v1/tlspk/upload/clusterdata
714+
`)),
715+
withCmdLineFlags("--disable-compression", "--client-id", "5bc7d07c-45da-11ef-a878-523f1e1d7de1", "--private-key-path", privKeyPath),
716+
)
717+
testutil.TrustCA(t, cl, cert)
718+
assert.Equal(t, VenafiCloudKeypair, got.AuthMode)
719+
require.NoError(t, err)
720+
721+
err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: "test cluster name"})
722+
require.NoError(t, err)
723+
})
635724
}
636725

637726
// Slower test cases due to envtest. That's why they are separated from the
@@ -711,8 +800,12 @@ func Test_ValidateAndCombineConfig_VenafiConnection(t *testing.T) {
711800
})
712801

713802
cfg, cl, err := ValidateAndCombineConfig(discardLogs(),
714-
Config{Server: "http://this-url-should-be-ignored", Period: 1 * time.Hour, ClusterID: "test cluster name"},
715-
AgentCmdFlags{VenConnName: "venafi-components", InstallNS: "venafi"})
803+
withConfig(testutil.Undent(`
804+
server: http://this-url-should-be-ignored
805+
period: 1h
806+
cluster_id: test cluster name
807+
`)),
808+
withCmdLineFlags("--venafi-connection", "venafi-components", "--install-namespace", "venafi"))
716809
require.NoError(t, err)
717810

718811
testutil.VenConnStartWatching(t, cl)
@@ -724,6 +817,53 @@ func Test_ValidateAndCombineConfig_VenafiConnection(t *testing.T) {
724817
err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: cfg.ClusterID})
725818
require.NoError(t, err)
726819
})
820+
821+
t.Run("the request is compressed by default", func(t *testing.T) {
822+
setVenafiCloudAssert(func(t testing.TB, gotReq *http.Request) {
823+
// Let's check that the body is compressed as expected.
824+
assert.Equal(t, "gzip", gotReq.Header.Get("Content-Encoding"))
825+
uncompressR, err := gzip.NewReader(gotReq.Body)
826+
require.NoError(t, err, "body might not be compressed")
827+
defer uncompressR.Close()
828+
uncompressed, err := io.ReadAll(uncompressR)
829+
require.NoError(t, err)
830+
assert.Contains(t, string(uncompressed), `{"agent_metadata":{"version":"development","cluster_id":"test cluster name"}`)
831+
})
832+
cfg, cl, err := ValidateAndCombineConfig(discardLogs(),
833+
withConfig(testutil.Undent(`
834+
period: 1h
835+
cluster_id: test cluster name
836+
`)),
837+
withCmdLineFlags("--venafi-connection", "venafi-components", "--install-namespace", "venafi"))
838+
require.NoError(t, err)
839+
testutil.VenConnStartWatching(t, cl)
840+
testutil.TrustCA(t, cl, cert)
841+
err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: cfg.ClusterID})
842+
require.NoError(t, err)
843+
})
844+
845+
t.Run("--disable-compression works", func(t *testing.T) {
846+
setVenafiCloudAssert(func(t testing.TB, gotReq *http.Request) {
847+
// Let's check that the body isn't compressed.
848+
assert.Equal(t, "", gotReq.Header.Get("Content-Encoding"))
849+
b := new(bytes.Buffer)
850+
_, err := b.ReadFrom(gotReq.Body)
851+
require.NoError(t, err)
852+
assert.Contains(t, b.String(), `{"agent_metadata":{"version":"development","cluster_id":"test cluster name"}`)
853+
})
854+
cfg, cl, err := ValidateAndCombineConfig(discardLogs(),
855+
withConfig(testutil.Undent(`
856+
server: `+srv.URL+`
857+
period: 1h
858+
cluster_id: test cluster name
859+
`)),
860+
withCmdLineFlags("--disable-compression", "--venafi-connection", "venafi-components", "--install-namespace", "venafi"))
861+
require.NoError(t, err)
862+
testutil.VenConnStartWatching(t, cl)
863+
testutil.TrustCA(t, cl, cert)
864+
err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: cfg.ClusterID})
865+
require.NoError(t, err)
866+
})
727867
}
728868

729869
func Test_ParseConfig(t *testing.T) {

‎pkg/client/client_venafi_cloud.go‎

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package client
22

33
import (
44
"bytes"
5+
"compress/gzip"
56
"crypto"
67
"crypto/ecdsa"
78
"crypto/ed25519"
@@ -50,6 +51,8 @@ type (
5051
jwtSigningAlg jwt.SigningMethod
5152
lock sync.RWMutex
5253

54+
disableCompression bool
55+
5356
// Made public for testing purposes.
5457
Client *http.Client
5558
}
@@ -84,7 +87,7 @@ const (
8487

8588
// NewVenafiCloudClient returns a new instance of the VenafiCloudClient type that will perform HTTP requests using a bearer token
8689
// to authenticate to the backend API.
87-
func NewVenafiCloudClient(agentMetadata *api.AgentMetadata, credentials *VenafiSvcAccountCredentials, baseURL string, uploaderID string, uploadPath string) (*VenafiCloudClient, error) {
90+
func NewVenafiCloudClient(agentMetadata *api.AgentMetadata, credentials *VenafiSvcAccountCredentials, baseURL string, uploaderID string, uploadPath string, disableCompression bool) (*VenafiCloudClient, error) {
8891
if err := credentials.Validate(); err != nil {
8992
return nil, fmt.Errorf("cannot create VenafiCloudClient: %w", err)
9093
}
@@ -107,15 +110,16 @@ func NewVenafiCloudClient(agentMetadata *api.AgentMetadata, credentials *VenafiS
107110
}
108111

109112
return &VenafiCloudClient{
110-
agentMetadata: agentMetadata,
111-
credentials: credentials,
112-
baseURL: baseURL,
113-
accessToken: &venafiCloudAccessToken{},
114-
Client: &http.Client{Timeout: time.Minute},
115-
uploaderID: uploaderID,
116-
uploadPath: uploadPath,
117-
privateKey: privateKey,
118-
jwtSigningAlg: jwtSigningAlg,
113+
agentMetadata: agentMetadata,
114+
credentials: credentials,
115+
baseURL: baseURL,
116+
accessToken: &venafiCloudAccessToken{},
117+
Client: &http.Client{Timeout: time.Minute},
118+
uploaderID: uploaderID,
119+
uploadPath: uploadPath,
120+
privateKey: privateKey,
121+
jwtSigningAlg: jwtSigningAlg,
122+
disableCompression: disableCompression,
119123
}, nil
120124
}
121125

@@ -260,11 +264,39 @@ func (c *VenafiCloudClient) Post(path string, body io.Reader) (*http.Response, e
260264
return nil, err
261265
}
262266

263-
req, err := http.NewRequest(http.MethodPost, fullURL(c.baseURL, path), body)
267+
var encodedBody io.Reader
268+
if c.disableCompression {
269+
encodedBody = body
270+
} else {
271+
compressed := new(bytes.Buffer)
272+
gz := gzip.NewWriter(compressed)
273+
if _, err := io.Copy(gz, body); err != nil {
274+
return nil, err
275+
}
276+
err := gz.Close()
277+
if err != nil {
278+
return nil, err
279+
}
280+
encodedBody = compressed
281+
}
282+
283+
req, err := http.NewRequest(http.MethodPost, fullURL(c.baseURL, path), encodedBody)
264284
if err != nil {
265285
return nil, err
266286
}
267287

288+
// We have noticed that NGINX, which is Venafi Control Plane's API gateway,
289+
// has a limit on the request body size we can send (client_max_body_size).
290+
// On large clusters, the agent may exceed this limit, triggering the error
291+
// "413 Request Entity Too Large". Although this limit has been raised to
292+
// 1GB, NGINX still buffers the requests that the agent sends because
293+
// proxy_request_buffering isn't set to off. To reduce the strain on NGINX'
294+
// memory and disk, to avoid further 413s, and to avoid reaching the maximum
295+
// request body size of customer's proxies, we have decided to enable GZIP
296+
// compression. Ref: https://venafi.atlassian.net/browse/VC-36434.
297+
if !c.disableCompression {
298+
req.Header.Set("Content-Encoding", "gzip")
299+
}
268300
req.Header.Set("Accept", "application/json")
269301
req.Header.Set("Content-Type", "application/json")
270302

0 commit comments

Comments
 (0)