Merge pull request #594 from jetstack/VC-36510-compress-requests

VC-36510: Key Pair and Venafi Connection modes now use gzip compression

Author: tfadeyi

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)

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":"fpp2624799349@affectionate-hertz6.platform.jetstack.io","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, "--install-namespace", "venafi"))
+		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,83 @@ 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, "--install-namespace", "venafi"),
+		)
+		require.NoError(t, err)
+		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, "--install-namespace", "venafi"),
+		)
+		require.NoError(t, err)
+		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 +802,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 +819,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) {

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