diff --git a/pkg/agent/config.go b/pkg/agent/config.go index 6f8186cc..4954deb6 100644 --- a/pkg/agent/config.go +++ b/pkg/agent/config.go @@ -27,7 +27,8 @@ const ( inClusterNamespacePath = "/var/run/secrets/kubernetes.io/serviceaccount/namespace" ) -// 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"` @@ -164,6 +165,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) { @@ -291,6 +297,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 @@ -332,6 +344,9 @@ type CombinedConfig struct { VenConnNS string InstallNS string + // VenafiCloudKeypair and VenafiCloudVenafiConnection modes only. + DisableCompression bool + // Only used for testing purposes. OutputPath string InputPath string @@ -564,6 +579,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 } @@ -658,7 +681,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) } @@ -716,7 +739,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 index c726a598..a934ff1d 100644 --- 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" @@ -341,6 +342,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)) + 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) { got, _, err := ValidateAndCombineConfig(discardLogs(), withConfig(testutil.Undent(` @@ -604,6 +618,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 @@ -683,8 +772,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) @@ -696,6 +789,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 index 7e317faf..ae1920d8 100644 --- 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") diff --git a/pkg/client/client_venconn.go b/pkg/client/client_venconn.go index f614ecd0..32f61e0e 100644 --- a/pkg/client/client_venconn.go +++ b/pkg/client/client_venconn.go @@ -2,6 +2,7 @@ package client import ( "bytes" + "compress/gzip" "context" "crypto/x509" "encoding/base64" @@ -34,6 +35,8 @@ type VenConnClient struct { venConnName string // Name of the VenafiConnection resource to use. venConnNS string // Namespace of the VenafiConnection resource to use. + disableCompression bool + // Used to make HTTP requests to Venafi Cloud. This field is public for // testing purposes so that we can configure trusted CAs; there should be a // way to do that without messing with the client directly (e.g., a flag to @@ -53,7 +56,7 @@ type VenConnClient struct { // empty. `venConnName` and `venConnNS` must not be empty either. The passed // `restcfg` is not mutated. `trustedCAs` is only used for connecting to Venafi // Cloud and Vault and can be left nil. -func NewVenConnClient(restcfg *rest.Config, agentMetadata *api.AgentMetadata, installNS, venConnName, venConnNS string, trustedCAs *x509.CertPool) (*VenConnClient, error) { +func NewVenConnClient(restcfg *rest.Config, agentMetadata *api.AgentMetadata, installNS, venConnName, venConnNS string, trustedCAs *x509.CertPool, disableCompression bool) (*VenConnClient, error) { // TODO(mael): The rest of the codebase uses the standard "log" package, // venafi-connection-lib uses "go-logr/logr", and client-go uses "klog". We // should standardize on one of them, probably "slog". @@ -113,12 +116,13 @@ func NewVenConnClient(restcfg *rest.Config, agentMetadata *api.AgentMetadata, in } return &VenConnClient{ - agentMetadata: agentMetadata, - connHandler: handler, - installNS: installNS, - venConnName: venConnName, - venConnNS: venConnNS, - Client: vcpClient, + agentMetadata: agentMetadata, + connHandler: handler, + installNS: installNS, + venConnName: venConnName, + venConnNS: venConnNS, + Client: vcpClient, + disableCompression: disableCompression, }, nil } @@ -157,19 +161,45 @@ func (c *VenConnClient) PostDataReadingsWithOptions(readings []*api.DataReading, DataGatherTime: time.Now().UTC(), DataReadings: readings, } - data, err := json.Marshal(payload) - if err != nil { - return err + + encodedBody := &bytes.Buffer{} + if c.disableCompression { + err = json.NewEncoder(encodedBody).Encode(payload) + if err != nil { + return err + } + } else { + gz := gzip.NewWriter(encodedBody) + err = json.NewEncoder(gz).Encode(payload) + if err != nil { + return err + } + err := gz.Close() + if err != nil { + return err + } } // The path parameter "no" is a dummy parameter to make the Venafi Cloud // backend happy. This parameter, named `uploaderID` in the backend, is not // actually used by the backend. - req, err := http.NewRequest(http.MethodPost, fullURL(token.BaseURL, "/v1/tlspk/upload/clusterdata/no"), bytes.NewBuffer(data)) + req, err := http.NewRequest(http.MethodPost, fullURL(token.BaseURL, "/v1/tlspk/upload/clusterdata/no"), encodedBody) if err != nil { return 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("Content-Type", "application/json") req.Header.Set("User-Agent", fmt.Sprintf("venafi-kubernetes-agent/%s", version.PreflightVersion)) diff --git a/pkg/client/client_venconn_test.go b/pkg/client/client_venconn_test.go index f765f1ad..c696c8ad 100644 --- a/pkg/client/client_venconn_test.go +++ b/pkg/client/client_venconn_test.go @@ -233,6 +233,7 @@ func run_TestVenConnClient_PostDataReadingsWithOptions(restcfg *rest.Config, kcl // Let's make sure we didn't forget to add the arbitrary "/no" // (uploader_id) path segment to /v1/tlspk/upload/clusterdata. assert.Equal(t, "/v1/tlspk/upload/clusterdata/no", r.URL.Path) + assert.Equal(t, "gzip", r.Header.Get("Content-Encoding")) }) certPool := x509.NewCertPool() @@ -246,6 +247,7 @@ func run_TestVenConnClient_PostDataReadingsWithOptions(restcfg *rest.Config, kcl "venafi-components", // Name of the VenafiConnection. testNameToNamespace(t), // Namespace of the VenafiConnection. certPool, + false, // disableCompression ) require.NoError(t, err)