Skip to content

Commit 6a5e097

Browse files
maelvlsmaelvls
authored
Merge pull request #628 from jetstack/VC-37264-disable-compression
VC-37264: Disable compression to give us time to work on a fix
2 parents eb3c30a + 4a8bccd commit 6a5e097

File tree

5 files changed

+49
-241
lines changed

5 files changed

+49
-241
lines changed

‎pkg/agent/config.go

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -167,11 +167,6 @@ type AgentCmdFlags struct {
167167

168168
// Prometheus (--enable-metrics) enables the Prometheus metrics server.
169169
Prometheus bool
170-
171-
// DisableCompression (--disable-compression) disables the GZIP compression
172-
// when uploading the data. Useful for debugging purposes, or when an
173-
// intermediate proxy doesn't like compressed data.
174-
DisableCompression bool
175170
}
176171

177172
func InitAgentCmdFlags(c *cobra.Command, cfg *AgentCmdFlags) {
@@ -298,12 +293,15 @@ func InitAgentCmdFlags(c *cobra.Command, cfg *AgentCmdFlags) {
298293
"Enables Prometheus metrics server on the agent (port: 8081).",
299294
)
300295

296+
var dummy bool
301297
c.PersistentFlags().BoolVar(
302-
&cfg.DisableCompression,
298+
&dummy,
303299
"disable-compression",
304300
false,
305-
"Disables GZIP compression when uploading the data. Useful for debugging purposes or when an intermediate proxy doesn't like compressed data.",
301+
"Deprecated. No longer has an effect.",
306302
)
303+
c.PersistentFlags().MarkDeprecated("disable-compression", "no longer has an effect")
304+
307305
}
308306

309307
type AuthMode string
@@ -346,7 +344,6 @@ type CombinedConfig struct {
346344
VenConnNS string
347345

348346
// VenafiCloudKeypair and VenafiCloudVenafiConnection modes only.
349-
DisableCompression bool
350347
ExcludeAnnotationKeysRegex []*regexp.Regexp
351348
ExcludeLabelKeysRegex []*regexp.Regexp
352349

@@ -586,14 +583,6 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
586583
}
587584
}
588585

589-
// Validation of --disable-compression.
590-
{
591-
if flags.DisableCompression && res.AuthMode != VenafiCloudKeypair && res.AuthMode != VenafiCloudVenafiConnection {
592-
errs = multierror.Append(errs, fmt.Errorf("--disable-compression can only be used with the %s and %s modes", VenafiCloudKeypair, VenafiCloudVenafiConnection))
593-
}
594-
res.DisableCompression = flags.DisableCompression
595-
}
596-
597586
// Validation of the config fields exclude_annotation_keys_regex and
598587
// exclude_label_keys_regex.
599588
{
@@ -709,7 +698,7 @@ func validateCredsAndCreateClient(log logr.Logger, flagCredentialsPath, flagClie
709698
break // Don't continue with the client if kubeconfig wasn't loaded.
710699
}
711700

712-
preflightClient, err = client.NewVenConnClient(restCfg, metadata, cfg.InstallNS, cfg.VenConnName, cfg.VenConnNS, nil, cfg.DisableCompression)
701+
preflightClient, err = client.NewVenConnClient(restCfg, metadata, cfg.InstallNS, cfg.VenConnName, cfg.VenConnNS, nil)
713702
if err != nil {
714703
errs = multierror.Append(errs, err)
715704
}
@@ -767,7 +756,7 @@ func createCredentialClient(log logr.Logger, credentials client.Credentials, cfg
767756
log.Info("Loading upload_path from \"venafi-cloud\" configuration.")
768757
uploadPath = cfg.UploadPath
769758
}
770-
return client.NewVenafiCloudClient(agentMetadata, creds, cfg.Server, uploaderID, uploadPath, cfg.DisableCompression)
759+
return client.NewVenafiCloudClient(agentMetadata, creds, cfg.Server, uploaderID, uploadPath)
771760

772761
case *client.OAuthCredentials:
773762
return client.NewOAuthClient(agentMetadata, creds, cfg.Server)

‎pkg/agent/config_test.go

Lines changed: 20 additions & 140 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
11
package agent
22

33
import (
4-
"bytes"
5-
"compress/gzip"
64
"context"
75
"fmt"
8-
"io"
96
"net/http"
107
"os"
118
"testing"
@@ -165,6 +162,26 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
165162
assert.Equal(t, true, got.StrictMode)
166163
})
167164

165+
t.Run("--disable-compression is deprecated and doesn't do anything", func(t *testing.T) {
166+
path := withFile(t, `{"user_id":"[email protected]","user_secret":"foo","client_id": "k3TrDbfLhCgnpAbOiiT2kIE1AbovKzjo","client_secret": "f39w_3KT9Vp0VhzcPzvh-uVbudzqCFmHER3Huj0dvHgJwVrjxsoOQPIw_1SDiCfa","auth_server_domain":"auth.jetstack.io"}`)
167+
log, b := recordLogs(t)
168+
_, _, err := ValidateAndCombineConfig(log,
169+
withConfig(testutil.Undent(`
170+
server: https://api.venafi.eu
171+
period: 1h
172+
organization_id: foo
173+
cluster_id: bar
174+
`)),
175+
withCmdLineFlags("--disable-compression", "--credentials-file", path, "--install-namespace", "venafi"))
176+
require.NoError(t, err)
177+
178+
// The log line printed by pflag is not captured by the log recorder.
179+
assert.Equal(t, testutil.Undent(`
180+
INFO Using the Jetstack Secure OAuth auth mode since --credentials-file was specified without --venafi-cloud.
181+
INFO Using period from config period="1h0m0s"
182+
`), b.String())
183+
})
184+
168185
t.Run("error when no auth method specified", func(t *testing.T) {
169186
_, cl, err := ValidateAndCombineConfig(discardLogs(),
170187
withConfig(testutil.Undent(`
@@ -375,19 +392,6 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
375392
assert.IsType(t, &client.OAuthClient{}, cl)
376393
})
377394

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

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

872752
func Test_ParseConfig(t *testing.T) {

‎pkg/client/client_venafi_cloud.go

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

33
import (
44
"bytes"
5-
"compress/gzip"
65
"crypto"
76
"crypto/ecdsa"
87
"crypto/ed25519"
@@ -51,8 +50,6 @@ type (
5150
jwtSigningAlg jwt.SigningMethod
5251
lock sync.RWMutex
5352

54-
disableCompression bool
55-
5653
// Made public for testing purposes.
5754
Client *http.Client
5855
}
@@ -87,7 +84,7 @@ const (
8784

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

112109
return &VenafiCloudClient{
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,
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,
123119
}, nil
124120
}
125121

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

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)
263+
req, err := http.NewRequest(http.MethodPost, fullURL(c.baseURL, path), body)
284264
if err != nil {
285265
return nil, err
286266
}
287267

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-
}
300268
req.Header.Set("Accept", "application/json")
301269
req.Header.Set("Content-Type", "application/json")
302270

0 commit comments

Comments
 (0)