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).
jetstack · Oct 18, 2024 · 179b748 · 179b748
1 parent 3e33b61
commit 179b748
Show file tree

Hide file tree

Showing 5 changed files with 254 additions and 27 deletions.
diff --git a/pkg/agent/config.go b/pkg/agent/config.go
@@ -23,7 +23,8 @@ import (
 	"github.com/jetstack/preflight/pkg/version"
 )
 
-// Config wraps the options for a run of the agent.
+// Config defines the YAML configuration file that you can pass using
+// `--config-file` or `-c`.
 type Config struct {
 	// Deprecated: Schedule doesn't do anything. Use `period` instead.
 	Schedule string        `yaml:"schedule"`
@@ -159,6 +160,11 @@ type AgentCmdFlags struct {
 
 	// Prometheus (--enable-metrics) enables the Prometheus metrics server.
 	Prometheus bool
+
+	// DisableCompression (--disable-compression) disables the GZIP compression
+	// when uploading the data. Useful for debugging purposes, or when an
+	// intermediate proxy doesn't like compressed data.
+	DisableCompression bool
 }
 
 func InitAgentCmdFlags(c *cobra.Command, cfg *AgentCmdFlags) {
@@ -285,6 +291,12 @@ func InitAgentCmdFlags(c *cobra.Command, cfg *AgentCmdFlags) {
 		"Enables Prometheus metrics server on the agent (port: 8081).",
 	)
 
+	c.PersistentFlags().BoolVar(
+		&cfg.DisableCompression,
+		"disable-compression",
+		false,
+		"Disables GZIP compression when uploading the data. Useful for debugging purposes or when an intermediate proxy doesn't like compressed data.",
+	)
 }
 
 type AuthMode string
@@ -326,6 +338,9 @@ type CombinedConfig struct {
 	VenConnName string
 	VenConnNS   string
 
+	// VenafiCloudKeypair and VenafiCloudVenafiConnection modes only.
+	DisableCompression bool
+
 	// Only used for testing purposes.
 	OutputPath string
 	InputPath  string
@@ -558,6 +573,14 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
 		}
 	}
 
+	// Validation of --disable-compression.
+	{
+		if flags.DisableCompression && res.AuthMode != VenafiCloudKeypair && res.AuthMode != VenafiCloudVenafiConnection {
+			errs = multierror.Append(errs, fmt.Errorf("--disable-compression can only be used with the %s and %s modes", VenafiCloudKeypair, VenafiCloudVenafiConnection))
+		}
+		res.DisableCompression = flags.DisableCompression
+	}
+
 	if errs != nil {
 		return CombinedConfig{}, nil, errs
 	}
@@ -652,7 +675,7 @@ func validateCredsAndCreateClient(log *log.Logger, flagCredentialsPath, flagClie
 			break // Don't continue with the client if kubeconfig wasn't loaded.
 		}
 
-		preflightClient, err = client.NewVenConnClient(restCfg, metadata, cfg.InstallNS, cfg.VenConnName, cfg.VenConnNS, nil)
+		preflightClient, err = client.NewVenConnClient(restCfg, metadata, cfg.InstallNS, cfg.VenConnName, cfg.VenConnNS, nil, cfg.DisableCompression)
 		if err != nil {
 			errs = multierror.Append(errs, err)
 		}
@@ -710,7 +733,7 @@ func createCredentialClient(log *log.Logger, credentials client.Credentials, cfg
 			log.Println("Loading upload_path from \"venafi-cloud\" configuration.")
 			uploadPath = cfg.UploadPath
 		}
-		return client.NewVenafiCloudClient(agentMetadata, creds, cfg.Server, uploaderID, uploadPath)
+		return client.NewVenafiCloudClient(agentMetadata, creds, cfg.Server, uploaderID, uploadPath, cfg.DisableCompression)
 
 	case *client.OAuthCredentials:
 		return client.NewOAuthClient(agentMetadata, creds, cfg.Server)

diff --git a/pkg/agent/config_test.go b/pkg/agent/config_test.go
@@ -2,6 +2,7 @@ package agent
 
 import (
 	"bytes"
+	"compress/gzip"
 	"context"
 	"fmt"
 	"io"
@@ -373,6 +374,19 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
 		assert.IsType(t, &client.OAuthClient{}, cl)
 	})
 
+	t.Run("jetstack-secure-oauth-auth: can't use --disable-compression", func(t *testing.T) {
+		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"}`)
+		_, _, err := ValidateAndCombineConfig(discardLogs(),
+			withConfig(testutil.Undent(`
+				server: https://api.venafi.eu
+				period: 1h
+				organization_id: foo
+				cluster_id: bar
+				`)),
+			withCmdLineFlags("--disable-compression", "--credentials-file", path))
+		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")
+	})
+
 	t.Run("jetstack-secure-oauth-auth: --credential-file used but file is missing", func(t *testing.T) {
 		t.Setenv("POD_NAMESPACE", "venafi")
 		got, _, err := ValidateAndCombineConfig(discardLogs(),
@@ -632,6 +646,81 @@ func Test_ValidateAndCombineConfig_VenafiCloudKeyPair(t *testing.T) {
 		err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: "test cluster name"})
 		require.NoError(t, err)
 	})
+
+	t.Run("the request body is compressed", func(t *testing.T) {
+		srv, cert, setVenafiCloudAssert := testutil.FakeVenafiCloud(t)
+		setVenafiCloudAssert(func(t testing.TB, gotReq *http.Request) {
+			if gotReq.URL.Path == "/v1/oauth/token/serviceaccount" {
+				return
+			}
+			assert.Equal(t, "/v1/tlspk/upload/clusterdata/no", gotReq.URL.Path)
+
+			// Let's check that the body is compressed as expected.
+			assert.Equal(t, "gzip", gotReq.Header.Get("Content-Encoding"))
+			uncompressR, err := gzip.NewReader(gotReq.Body)
+			require.NoError(t, err, "body might not be compressed")
+			defer uncompressR.Close()
+			uncompressed, err := io.ReadAll(uncompressR)
+			require.NoError(t, err)
+			assert.Contains(t, string(uncompressed), `{"agent_metadata":{"version":"development","cluster_id":"test cluster name"}`)
+		})
+		privKeyPath := withFile(t, fakePrivKeyPEM)
+		got, cl, err := ValidateAndCombineConfig(discardLogs(),
+			withConfig(testutil.Undent(`
+				server: `+srv.URL+`
+				period: 1h
+				cluster_id: "test cluster name"
+				venafi-cloud:
+				  uploader_id: no
+				  upload_path: /v1/tlspk/upload/clusterdata
+			`)),
+			withCmdLineFlags("--client-id", "5bc7d07c-45da-11ef-a878-523f1e1d7de1", "--private-key-path", privKeyPath),
+		)
+		testutil.TrustCA(t, cl, cert)
+		assert.Equal(t, VenafiCloudKeypair, got.AuthMode)
+		require.NoError(t, err)
+
+		err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: "test cluster name"})
+		require.NoError(t, err)
+	})
+
+	t.Run("--disable-compression works", func(t *testing.T) {
+		srv, cert, setVenafiCloudAssert := testutil.FakeVenafiCloud(t)
+		setVenafiCloudAssert(func(t testing.TB, gotReq *http.Request) {
+			// Only care about /v1/tlspk/upload/clusterdata/:uploader_id?name=
+			if gotReq.URL.Path == "/v1/oauth/token/serviceaccount" {
+				return
+			}
+
+			assert.Equal(t, "/v1/tlspk/upload/clusterdata/no", gotReq.URL.Path)
+
+			// Let's check that the body isn't compressed.
+			assert.Equal(t, "", gotReq.Header.Get("Content-Encoding"))
+			b := new(bytes.Buffer)
+			_, err := b.ReadFrom(gotReq.Body)
+			require.NoError(t, err)
+			assert.Contains(t, b.String(), `{"agent_metadata":{"version":"development","cluster_id":"test cluster name"}`)
+		})
+
+		privKeyPath := withFile(t, fakePrivKeyPEM)
+		got, cl, err := ValidateAndCombineConfig(discardLogs(),
+			withConfig(testutil.Undent(`
+				server: `+srv.URL+`
+				period: 1h
+				cluster_id: "test cluster name"
+				venafi-cloud:
+				  uploader_id: no
+				  upload_path: /v1/tlspk/upload/clusterdata
+			`)),
+			withCmdLineFlags("--disable-compression", "--client-id", "5bc7d07c-45da-11ef-a878-523f1e1d7de1", "--private-key-path", privKeyPath),
+		)
+		testutil.TrustCA(t, cl, cert)
+		assert.Equal(t, VenafiCloudKeypair, got.AuthMode)
+		require.NoError(t, err)
+
+		err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: "test cluster name"})
+		require.NoError(t, err)
+	})
 }
 
 // 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) {
 		})
 
 		cfg, cl, err := ValidateAndCombineConfig(discardLogs(),
-			Config{Server: "http://this-url-should-be-ignored", Period: 1 * time.Hour, ClusterID: "test cluster name"},
-			AgentCmdFlags{VenConnName: "venafi-components", InstallNS: "venafi"})
+			withConfig(testutil.Undent(`
+				server: http://this-url-should-be-ignored
+				period: 1h
+				cluster_id: test cluster name
+			`)),
+			withCmdLineFlags("--venafi-connection", "venafi-components", "--install-namespace", "venafi"))
 		require.NoError(t, err)
 
 		testutil.VenConnStartWatching(t, cl)
@@ -724,6 +817,53 @@ func Test_ValidateAndCombineConfig_VenafiConnection(t *testing.T) {
 		err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: cfg.ClusterID})
 		require.NoError(t, err)
 	})
+
+	t.Run("the request is compressed by default", func(t *testing.T) {
+		setVenafiCloudAssert(func(t testing.TB, gotReq *http.Request) {
+			// Let's check that the body is compressed as expected.
+			assert.Equal(t, "gzip", gotReq.Header.Get("Content-Encoding"))
+			uncompressR, err := gzip.NewReader(gotReq.Body)
+			require.NoError(t, err, "body might not be compressed")
+			defer uncompressR.Close()
+			uncompressed, err := io.ReadAll(uncompressR)
+			require.NoError(t, err)
+			assert.Contains(t, string(uncompressed), `{"agent_metadata":{"version":"development","cluster_id":"test cluster name"}`)
+		})
+		cfg, cl, err := ValidateAndCombineConfig(discardLogs(),
+			withConfig(testutil.Undent(`
+				period: 1h
+				cluster_id: test cluster name
+			`)),
+			withCmdLineFlags("--venafi-connection", "venafi-components", "--install-namespace", "venafi"))
+		require.NoError(t, err)
+		testutil.VenConnStartWatching(t, cl)
+		testutil.TrustCA(t, cl, cert)
+		err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: cfg.ClusterID})
+		require.NoError(t, err)
+	})
+
+	t.Run("--disable-compression works", func(t *testing.T) {
+		setVenafiCloudAssert(func(t testing.TB, gotReq *http.Request) {
+			// Let's check that the body isn't compressed.
+			assert.Equal(t, "", gotReq.Header.Get("Content-Encoding"))
+			b := new(bytes.Buffer)
+			_, err := b.ReadFrom(gotReq.Body)
+			require.NoError(t, err)
+			assert.Contains(t, b.String(), `{"agent_metadata":{"version":"development","cluster_id":"test cluster name"}`)
+		})
+		cfg, cl, err := ValidateAndCombineConfig(discardLogs(),
+			withConfig(testutil.Undent(`
+				server: `+srv.URL+`
+				period: 1h
+				cluster_id: test cluster name
+			`)),
+			withCmdLineFlags("--disable-compression", "--venafi-connection", "venafi-components", "--install-namespace", "venafi"))
+		require.NoError(t, err)
+		testutil.VenConnStartWatching(t, cl)
+		testutil.TrustCA(t, cl, cert)
+		err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: cfg.ClusterID})
+		require.NoError(t, err)
+	})
 }
 
 func Test_ParseConfig(t *testing.T) {

diff --git a/pkg/client/client_venafi_cloud.go b/pkg/client/client_venafi_cloud.go
@@ -2,6 +2,7 @@ package client
 
 import (
 	"bytes"
+	"compress/gzip"
 	"crypto"
 	"crypto/ecdsa"
 	"crypto/ed25519"
@@ -50,6 +51,8 @@ type (
 		jwtSigningAlg jwt.SigningMethod
 		lock          sync.RWMutex
 
+		disableCompression bool
+
 		// Made public for testing purposes.
 		Client *http.Client
 	}
@@ -84,7 +87,7 @@ const (
 
 // NewVenafiCloudClient returns a new instance of the VenafiCloudClient type that will perform HTTP requests using a bearer token
 // to authenticate to the backend API.
-func NewVenafiCloudClient(agentMetadata *api.AgentMetadata, credentials *VenafiSvcAccountCredentials, baseURL string, uploaderID string, uploadPath string) (*VenafiCloudClient, error) {
+func NewVenafiCloudClient(agentMetadata *api.AgentMetadata, credentials *VenafiSvcAccountCredentials, baseURL string, uploaderID string, uploadPath string, disableCompression bool) (*VenafiCloudClient, error) {
 	if err := credentials.Validate(); err != nil {
 		return nil, fmt.Errorf("cannot create VenafiCloudClient: %w", err)
 	}
@@ -107,15 +110,16 @@ func NewVenafiCloudClient(agentMetadata *api.AgentMetadata, credentials *VenafiS
 	}
 
 	return &VenafiCloudClient{
-		agentMetadata: agentMetadata,
-		credentials:   credentials,
-		baseURL:       baseURL,
-		accessToken:   &venafiCloudAccessToken{},
-		Client:        &http.Client{Timeout: time.Minute},
-		uploaderID:    uploaderID,
-		uploadPath:    uploadPath,
-		privateKey:    privateKey,
-		jwtSigningAlg: jwtSigningAlg,
+		agentMetadata:      agentMetadata,
+		credentials:        credentials,
+		baseURL:            baseURL,
+		accessToken:        &venafiCloudAccessToken{},
+		Client:             &http.Client{Timeout: time.Minute},
+		uploaderID:         uploaderID,
+		uploadPath:         uploadPath,
+		privateKey:         privateKey,
+		jwtSigningAlg:      jwtSigningAlg,
+		disableCompression: disableCompression,
 	}, nil
 }
 
@@ -260,11 +264,39 @@ func (c *VenafiCloudClient) Post(path string, body io.Reader) (*http.Response, e
 		return nil, err
 	}
 
-	req, err := http.NewRequest(http.MethodPost, fullURL(c.baseURL, path), body)
+	var encodedBody io.Reader
+	if c.disableCompression {
+		encodedBody = body
+	} else {
+		compressed := new(bytes.Buffer)
+		gz := gzip.NewWriter(compressed)
+		if _, err := io.Copy(gz, body); err != nil {
+			return nil, err
+		}
+		err := gz.Close()
+		if err != nil {
+			return nil, err
+		}
+		encodedBody = compressed
+	}
+
+	req, err := http.NewRequest(http.MethodPost, fullURL(c.baseURL, path), encodedBody)
 	if err != nil {
 		return nil, err
 	}
 
+	// We have noticed that NGINX, which is Venafi Control Plane's API gateway,
+	// has a limit on the request body size we can send (client_max_body_size).
+	// On large clusters, the agent may exceed this limit, triggering the error
+	// "413 Request Entity Too Large". Although this limit has been raised to
+	// 1GB, NGINX still buffers the requests that the agent sends because
+	// proxy_request_buffering isn't set to off. To reduce the strain on NGINX'
+	// memory and disk, to avoid further 413s, and to avoid reaching the maximum
+	// request body size of customer's proxies, we have decided to enable GZIP
+	// compression. Ref: https://venafi.atlassian.net/browse/VC-36434.
+	if !c.disableCompression {
+		req.Header.Set("Content-Encoding", "gzip")
+	}
 	req.Header.Set("Accept", "application/json")
 	req.Header.Set("Content-Type", "application/json")