Skip to content

Commit fa29555

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 64a9255 commit fa29555

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
@@ -27,7 +27,8 @@ const (
2727
inClusterNamespacePath = "/var/run/secrets/kubernetes.io/serviceaccount/namespace"
2828
)
2929

30-
// Config wraps the options for a run of the agent.
30+
// Config defines the YAML configuration file that you can pass using
31+
// `--config-file` or `-c`.
3132
type Config struct {
3233
// Deprecated: Schedule doesn't do anything. Use `period` instead.
3334
Schedule string `yaml:"schedule"`
@@ -164,6 +165,11 @@ type AgentCmdFlags struct {
164165

165166
// Prometheus (--enable-metrics) enables the Prometheus metrics server.
166167
Prometheus bool
168+
169+
// DisableCompression (--disable-compression) disables the GZIP compression
170+
// when uploading the data. Useful for debugging purposes, or when an
171+
// intermediate proxy doesn't like compressed data.
172+
DisableCompression bool
167173
}
168174

169175
func InitAgentCmdFlags(c *cobra.Command, cfg *AgentCmdFlags) {
@@ -291,6 +297,12 @@ func InitAgentCmdFlags(c *cobra.Command, cfg *AgentCmdFlags) {
291297
"Enables Prometheus metrics server on the agent (port: 8081).",
292298
)
293299

300+
c.PersistentFlags().BoolVar(
301+
&cfg.DisableCompression,
302+
"disable-compression",
303+
false,
304+
"Disables GZIP compression when uploading the data. Useful for debugging purposes or when an intermediate proxy doesn't like compressed data.",
305+
)
294306
}
295307

296308
type AuthMode string
@@ -332,6 +344,9 @@ type CombinedConfig struct {
332344
VenConnNS string
333345
InstallNS string
334346

347+
// VenafiCloudKeypair and VenafiCloudVenafiConnection modes only.
348+
DisableCompression bool
349+
335350
// Only used for testing purposes.
336351
OutputPath string
337352
InputPath string
@@ -564,6 +579,14 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
564579
}
565580
}
566581

582+
// Validation of --disable-compression.
583+
{
584+
if flags.DisableCompression && res.AuthMode != VenafiCloudKeypair && res.AuthMode != VenafiCloudVenafiConnection {
585+
errs = multierror.Append(errs, fmt.Errorf("--disable-compression can only be used with the %s and %s modes", VenafiCloudKeypair, VenafiCloudVenafiConnection))
586+
}
587+
res.DisableCompression = flags.DisableCompression
588+
}
589+
567590
if errs != nil {
568591
return CombinedConfig{}, nil, errs
569592
}
@@ -658,7 +681,7 @@ func validateCredsAndCreateClient(log *log.Logger, flagCredentialsPath, flagClie
658681
break // Don't continue with the client if kubeconfig wasn't loaded.
659682
}
660683

661-
preflightClient, err = client.NewVenConnClient(restCfg, metadata, cfg.InstallNS, cfg.VenConnName, cfg.VenConnNS, nil)
684+
preflightClient, err = client.NewVenConnClient(restCfg, metadata, cfg.InstallNS, cfg.VenConnName, cfg.VenConnNS, nil, cfg.DisableCompression)
662685
if err != nil {
663686
errs = multierror.Append(errs, err)
664687
}
@@ -716,7 +739,7 @@ func createCredentialClient(log *log.Logger, credentials client.Credentials, cfg
716739
log.Println("Loading upload_path from \"venafi-cloud\" configuration.")
717740
uploadPath = cfg.UploadPath
718741
}
719-
return client.NewVenafiCloudClient(agentMetadata, creds, cfg.Server, uploaderID, uploadPath)
742+
return client.NewVenafiCloudClient(agentMetadata, creds, cfg.Server, uploaderID, uploadPath, cfg.DisableCompression)
720743

721744
case *client.OAuthCredentials:
722745
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"
@@ -341,6 +342,19 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
341342
assert.IsType(t, &client.OAuthClient{}, cl)
342343
})
343344

345+
t.Run("jetstack-secure-oauth-auth: can't use --disable-compression", func(t *testing.T) {
346+
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"}`)
347+
_, _, err := ValidateAndCombineConfig(discardLogs(),
348+
withConfig(testutil.Undent(`
349+
server: https://api.venafi.eu
350+
period: 1h
351+
organization_id: foo
352+
cluster_id: bar
353+
`)),
354+
withCmdLineFlags("--disable-compression", "--credentials-file", path))
355+
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")
356+
})
357+
344358
t.Run("jetstack-secure-oauth-auth: --credential-file used but file is missing", func(t *testing.T) {
345359
got, _, err := ValidateAndCombineConfig(discardLogs(),
346360
withConfig(testutil.Undent(`
@@ -604,6 +618,81 @@ func Test_ValidateAndCombineConfig_VenafiCloudKeyPair(t *testing.T) {
604618
err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: "test cluster name"})
605619
require.NoError(t, err)
606620
})
621+
622+
t.Run("the request body is compressed", func(t *testing.T) {
623+
srv, cert, setVenafiCloudAssert := testutil.FakeVenafiCloud(t)
624+
setVenafiCloudAssert(func(t testing.TB, gotReq *http.Request) {
625+
if gotReq.URL.Path == "/v1/oauth/token/serviceaccount" {
626+
return
627+
}
628+
assert.Equal(t, "/v1/tlspk/upload/clusterdata/no", gotReq.URL.Path)
629+
630+
// Let's check that the body is compressed as expected.
631+
assert.Equal(t, "gzip", gotReq.Header.Get("Content-Encoding"))
632+
uncompressR, err := gzip.NewReader(gotReq.Body)
633+
require.NoError(t, err, "body might not be compressed")
634+
defer uncompressR.Close()
635+
uncompressed, err := io.ReadAll(uncompressR)
636+
require.NoError(t, err)
637+
assert.Contains(t, string(uncompressed), `{"agent_metadata":{"version":"development","cluster_id":"test cluster name"}`)
638+
})
639+
privKeyPath := withFile(t, fakePrivKeyPEM)
640+
got, cl, err := ValidateAndCombineConfig(discardLogs(),
641+
withConfig(testutil.Undent(`
642+
server: `+srv.URL+`
643+
period: 1h
644+
cluster_id: "test cluster name"
645+
venafi-cloud:
646+
uploader_id: no
647+
upload_path: /v1/tlspk/upload/clusterdata
648+
`)),
649+
withCmdLineFlags("--client-id", "5bc7d07c-45da-11ef-a878-523f1e1d7de1", "--private-key-path", privKeyPath),
650+
)
651+
testutil.TrustCA(t, cl, cert)
652+
assert.Equal(t, VenafiCloudKeypair, got.AuthMode)
653+
require.NoError(t, err)
654+
655+
err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: "test cluster name"})
656+
require.NoError(t, err)
657+
})
658+
659+
t.Run("--disable-compression works", func(t *testing.T) {
660+
srv, cert, setVenafiCloudAssert := testutil.FakeVenafiCloud(t)
661+
setVenafiCloudAssert(func(t testing.TB, gotReq *http.Request) {
662+
// Only care about /v1/tlspk/upload/clusterdata/:uploader_id?name=
663+
if gotReq.URL.Path == "/v1/oauth/token/serviceaccount" {
664+
return
665+
}
666+
667+
assert.Equal(t, "/v1/tlspk/upload/clusterdata/no", gotReq.URL.Path)
668+
669+
// Let's check that the body isn't compressed.
670+
assert.Equal(t, "", gotReq.Header.Get("Content-Encoding"))
671+
b := new(bytes.Buffer)
672+
_, err := b.ReadFrom(gotReq.Body)
673+
require.NoError(t, err)
674+
assert.Contains(t, b.String(), `{"agent_metadata":{"version":"development","cluster_id":"test cluster name"}`)
675+
})
676+
677+
privKeyPath := withFile(t, fakePrivKeyPEM)
678+
got, cl, err := ValidateAndCombineConfig(discardLogs(),
679+
withConfig(testutil.Undent(`
680+
server: `+srv.URL+`
681+
period: 1h
682+
cluster_id: "test cluster name"
683+
venafi-cloud:
684+
uploader_id: no
685+
upload_path: /v1/tlspk/upload/clusterdata
686+
`)),
687+
withCmdLineFlags("--disable-compression", "--client-id", "5bc7d07c-45da-11ef-a878-523f1e1d7de1", "--private-key-path", privKeyPath),
688+
)
689+
testutil.TrustCA(t, cl, cert)
690+
assert.Equal(t, VenafiCloudKeypair, got.AuthMode)
691+
require.NoError(t, err)
692+
693+
err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: "test cluster name"})
694+
require.NoError(t, err)
695+
})
607696
}
608697

609698
// Slower test cases due to envtest. That's why they are separated from the
@@ -683,8 +772,12 @@ func Test_ValidateAndCombineConfig_VenafiConnection(t *testing.T) {
683772
})
684773

685774
cfg, cl, err := ValidateAndCombineConfig(discardLogs(),
686-
Config{Server: "http://this-url-should-be-ignored", Period: 1 * time.Hour, ClusterID: "test cluster name"},
687-
AgentCmdFlags{VenConnName: "venafi-components", InstallNS: "venafi"})
775+
withConfig(testutil.Undent(`
776+
server: http://this-url-should-be-ignored
777+
period: 1h
778+
cluster_id: test cluster name
779+
`)),
780+
withCmdLineFlags("--venafi-connection", "venafi-components", "--install-namespace", "venafi"))
688781
require.NoError(t, err)
689782

690783
testutil.VenConnStartWatching(t, cl)
@@ -696,6 +789,53 @@ func Test_ValidateAndCombineConfig_VenafiConnection(t *testing.T) {
696789
err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: cfg.ClusterID})
697790
require.NoError(t, err)
698791
})
792+
793+
t.Run("the request is compressed by default", func(t *testing.T) {
794+
setVenafiCloudAssert(func(t testing.TB, gotReq *http.Request) {
795+
// Let's check that the body is compressed as expected.
796+
assert.Equal(t, "gzip", gotReq.Header.Get("Content-Encoding"))
797+
uncompressR, err := gzip.NewReader(gotReq.Body)
798+
require.NoError(t, err, "body might not be compressed")
799+
defer uncompressR.Close()
800+
uncompressed, err := io.ReadAll(uncompressR)
801+
require.NoError(t, err)
802+
assert.Contains(t, string(uncompressed), `{"agent_metadata":{"version":"development","cluster_id":"test cluster name"}`)
803+
})
804+
cfg, cl, err := ValidateAndCombineConfig(discardLogs(),
805+
withConfig(testutil.Undent(`
806+
period: 1h
807+
cluster_id: test cluster name
808+
`)),
809+
withCmdLineFlags("--venafi-connection", "venafi-components", "--install-namespace", "venafi"))
810+
require.NoError(t, err)
811+
testutil.VenConnStartWatching(t, cl)
812+
testutil.TrustCA(t, cl, cert)
813+
err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: cfg.ClusterID})
814+
require.NoError(t, err)
815+
})
816+
817+
t.Run("--disable-compression works", func(t *testing.T) {
818+
setVenafiCloudAssert(func(t testing.TB, gotReq *http.Request) {
819+
// Let's check that the body isn't compressed.
820+
assert.Equal(t, "", gotReq.Header.Get("Content-Encoding"))
821+
b := new(bytes.Buffer)
822+
_, err := b.ReadFrom(gotReq.Body)
823+
require.NoError(t, err)
824+
assert.Contains(t, b.String(), `{"agent_metadata":{"version":"development","cluster_id":"test cluster name"}`)
825+
})
826+
cfg, cl, err := ValidateAndCombineConfig(discardLogs(),
827+
withConfig(testutil.Undent(`
828+
server: `+srv.URL+`
829+
period: 1h
830+
cluster_id: test cluster name
831+
`)),
832+
withCmdLineFlags("--disable-compression", "--venafi-connection", "venafi-components", "--install-namespace", "venafi"))
833+
require.NoError(t, err)
834+
testutil.VenConnStartWatching(t, cl)
835+
testutil.TrustCA(t, cl, cert)
836+
err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: cfg.ClusterID})
837+
require.NoError(t, err)
838+
})
699839
}
700840

701841
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)