Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions pkg/agent/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down
146 changes: 144 additions & 2 deletions pkg/agent/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package agent

import (
"bytes"
"compress/gzip"
"context"
"fmt"
"io"
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
54 changes: 43 additions & 11 deletions pkg/client/client_venafi_cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package client

import (
"bytes"
"compress/gzip"
"crypto"
"crypto/ecdsa"
"crypto/ed25519"
Expand Down Expand Up @@ -50,6 +51,8 @@ type (
jwtSigningAlg jwt.SigningMethod
lock sync.RWMutex

disableCompression bool

// Made public for testing purposes.
Client *http.Client
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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
}

Expand Down Expand Up @@ -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")

Expand Down
Loading