diff --git a/pkg/agent/config.go b/pkg/agent/config.go index e413a313..4d2be8cd 100644 --- a/pkg/agent/config.go +++ b/pkg/agent/config.go @@ -302,7 +302,7 @@ func InitAgentCmdFlags(c *cobra.Command, cfg *AgentCmdFlags) { &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.", + "Deprecated. No longer has an effect.", ) } @@ -346,7 +346,6 @@ type CombinedConfig struct { VenConnNS string // VenafiCloudKeypair and VenafiCloudVenafiConnection modes only. - DisableCompression bool ExcludeAnnotationKeysRegex []*regexp.Regexp ExcludeLabelKeysRegex []*regexp.Regexp @@ -588,10 +587,9 @@ func ValidateAndCombineConfig(log logr.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)) + if flags.DisableCompression { + log.Info("The flag --disable-compression has been deprecated an no longer has any effect.") } - res.DisableCompression = flags.DisableCompression } // Validation of the config fields exclude_annotation_keys_regex and @@ -709,7 +707,7 @@ func validateCredsAndCreateClient(log logr.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, cfg.DisableCompression) + preflightClient, err = client.NewVenConnClient(restCfg, metadata, cfg.InstallNS, cfg.VenConnName, cfg.VenConnNS, nil) if err != nil { errs = multierror.Append(errs, err) } @@ -767,7 +765,7 @@ func createCredentialClient(log logr.Logger, credentials client.Credentials, cfg log.Info("Loading upload_path from \"venafi-cloud\" configuration.") uploadPath = cfg.UploadPath } - return client.NewVenafiCloudClient(agentMetadata, creds, cfg.Server, uploaderID, uploadPath, cfg.DisableCompression) + return client.NewVenafiCloudClient(agentMetadata, creds, cfg.Server, uploaderID, uploadPath) 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 6a3bd07e..4025259a 100644 --- a/pkg/agent/config_test.go +++ b/pkg/agent/config_test.go @@ -1,11 +1,8 @@ package agent import ( - "bytes" - "compress/gzip" "context" "fmt" - "io" "net/http" "os" "testing" @@ -165,6 +162,25 @@ func Test_ValidateAndCombineConfig(t *testing.T) { assert.Equal(t, true, got.StrictMode) }) + t.Run("--disable-compression is deprecated and doesn't do anything", 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"}`) + log, b := recordLogs(t) + _, _, err := ValidateAndCombineConfig(log, + 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.NoError(t, err) + assert.Equal(t, testutil.Undent(` + INFO Using the Jetstack Secure OAuth auth mode since --credentials-file was specified without --venafi-cloud. + INFO Using period from config period="1h0m0s" + INFO The flag --disable-compression has been deprecated an no longer has any effect. + `), b.String()) + }) + t.Run("error when no auth method specified", func(t *testing.T) { _, cl, err := ValidateAndCombineConfig(discardLogs(), withConfig(testutil.Undent(` @@ -375,19 +391,6 @@ 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(), @@ -647,83 +650,6 @@ 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 @@ -820,53 +746,6 @@ 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 ae1920d8..7e317faf 100644 --- a/pkg/client/client_venafi_cloud.go +++ b/pkg/client/client_venafi_cloud.go @@ -2,7 +2,6 @@ package client import ( "bytes" - "compress/gzip" "crypto" "crypto/ecdsa" "crypto/ed25519" @@ -51,8 +50,6 @@ type ( jwtSigningAlg jwt.SigningMethod lock sync.RWMutex - disableCompression bool - // Made public for testing purposes. Client *http.Client } @@ -87,7 +84,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, disableCompression bool) (*VenafiCloudClient, error) { +func NewVenafiCloudClient(agentMetadata *api.AgentMetadata, credentials *VenafiSvcAccountCredentials, baseURL string, uploaderID string, uploadPath string) (*VenafiCloudClient, error) { if err := credentials.Validate(); err != nil { return nil, fmt.Errorf("cannot create VenafiCloudClient: %w", err) } @@ -110,16 +107,15 @@ 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, - disableCompression: disableCompression, + agentMetadata: agentMetadata, + credentials: credentials, + baseURL: baseURL, + accessToken: &venafiCloudAccessToken{}, + Client: &http.Client{Timeout: time.Minute}, + uploaderID: uploaderID, + uploadPath: uploadPath, + privateKey: privateKey, + jwtSigningAlg: jwtSigningAlg, }, nil } @@ -264,39 +260,11 @@ func (c *VenafiCloudClient) Post(path string, body io.Reader) (*http.Response, e return nil, err } - 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) + req, err := http.NewRequest(http.MethodPost, fullURL(c.baseURL, path), body) 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 c7daae42..d8553624 100644 --- a/pkg/client/client_venconn.go +++ b/pkg/client/client_venconn.go @@ -2,7 +2,6 @@ package client import ( "bytes" - "compress/gzip" "context" "crypto/x509" "encoding/base64" @@ -33,8 +32,6 @@ 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 @@ -54,7 +51,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, disableCompression bool) (*VenConnClient, error) { +func NewVenConnClient(restcfg *rest.Config, agentMetadata *api.AgentMetadata, installNS, venConnName, venConnNS string, trustedCAs *x509.CertPool) (*VenConnClient, error) { if installNS == "" { return nil, errors.New("programmer mistake: installNS must be provided") } @@ -109,13 +106,12 @@ func NewVenConnClient(restcfg *rest.Config, agentMetadata *api.AgentMetadata, in } return &VenConnClient{ - agentMetadata: agentMetadata, - connHandler: handler, - installNS: installNS, - venConnName: venConnName, - venConnNS: venConnNS, - Client: vcpClient, - disableCompression: disableCompression, + agentMetadata: agentMetadata, + connHandler: handler, + installNS: installNS, + venConnName: venConnName, + venConnNS: venConnNS, + Client: vcpClient, }, nil } @@ -156,21 +152,10 @@ func (c *VenConnClient) PostDataReadingsWithOptions(readings []*api.DataReading, } 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 - } + + err = json.NewEncoder(encodedBody).Encode(payload) + if err != nil { + return err } // The path parameter "no" is a dummy parameter to make the Venafi Cloud @@ -181,18 +166,6 @@ func (c *VenConnClient) PostDataReadingsWithOptions(readings []*api.DataReading, 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 c696c8ad..df75a8c5 100644 --- a/pkg/client/client_venconn_test.go +++ b/pkg/client/client_venconn_test.go @@ -247,7 +247,6 @@ 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)