[VC-36032] Pass the context to Venafi clients and enable debug roundtripper (#627)

* Pass the context to the Venafi clients
* Add the client-go debug round tripper to the venafi clients
* Add a note about HTTP request logging to the logging flags
* Add a note about setting logging-format and log-level to the Helm chart values documentation.

Signed-off-by: Richard Wall <[email protected]>

Author: wallrj

deploy/charts/venafi-kubernetes-agent/README.md

@@ -265,8 +265,18 @@ Specify the command to run overriding default binary.
 > []
 > ```
 
-Specify additional arguments to pass to the agent binary.  
-Example: `["--strict", "--oneshot"]`
+Specify additional arguments to pass to the agent binary. For example, to enable JSON logging use `--logging-format`, or to increase the logging verbosity use `--log-level`.  
+The log levels are: 0=Info, 1=Debug, 2=Trace.  
+Use 6-9 for increasingly verbose HTTP request logging.  
+The default log level is 0.  
+  
+Example:
+
+```yaml
+extraArgs:
+- --logging-format=json
+- --log-level=6 # To enable HTTP request logging
+```
 #### **volumes** ~ `array`
 > Default value:
 > ```yaml

deploy/charts/venafi-kubernetes-agent/values.schema.json

@@ -308,7 +308,7 @@
     },
     "helm-values.extraArgs": {
       "default": [],
-      "description": "Specify additional arguments to pass to the agent binary.\nExample: `[\"--strict\", \"--oneshot\"]`",
+      "description": "Specify additional arguments to pass to the agent binary. For example, to enable JSON logging use `--logging-format`, or to increase the logging verbosity use `--log-level`.\nThe log levels are: 0=Info, 1=Debug, 2=Trace.\nUse 6-9 for increasingly verbose HTTP request logging.\nThe default log level is 0.\n\nExample:\nextraArgs:\n- --logging-format=json\n- --log-level=6 # To enable HTTP request logging",
       "items": {},
       "type": "array"
     },

deploy/charts/venafi-kubernetes-agent/values.yaml

@@ -146,7 +146,16 @@ affinity: {}
 command: []
 
 # Specify additional arguments to pass to the agent binary.
-# Example: `["--strict", "--oneshot"]`
+# For example, to enable JSON logging use `--logging-format`, or
+# to increase the logging verbosity use `--log-level`.
+# The log levels are: 0=Info, 1=Debug, 2=Trace.
+# Use 6-9 for increasingly verbose HTTP request logging.
+# The default log level is 0.
+#
+# Example:
+#  extraArgs:
+#  - --logging-format=json
+#  - --log-level=6 # To enable HTTP request logging
 extraArgs: []
 
 # Additional volumes to add to the Venafi Kubernetes Agent container. This is

hack/e2e/values.venafi-kubernetes-agent.yaml

@@ -10,4 +10,4 @@ authentication:
 
 extraArgs:
 - --logging-format=json
-- --log-level=2
+- --log-level=6

pkg/agent/config_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"k8s.io/klog/v2"
 	"k8s.io/klog/v2/ktesting"
 
 	"github.com/jetstack/preflight/pkg/client"
@@ -620,6 +621,12 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
 func Test_ValidateAndCombineConfig_VenafiCloudKeyPair(t *testing.T) {
 	t.Run("server, uploader_id, and cluster name are correctly passed", func(t *testing.T) {
 		t.Setenv("POD_NAMESPACE", "venafi")
+
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+		log := ktesting.NewLogger(t, ktesting.NewConfig(ktesting.Verbosity(10)))
+		ctx = klog.NewContext(ctx, log)
+
 		srv, cert, setVenafiCloudAssert := testutil.FakeVenafiCloud(t)
 		setVenafiCloudAssert(func(t testing.TB, gotReq *http.Request) {
 			// Only care about /v1/tlspk/upload/clusterdata/:uploader_id?name=
@@ -648,7 +655,7 @@ func Test_ValidateAndCombineConfig_VenafiCloudKeyPair(t *testing.T) {
 		testutil.TrustCA(t, cl, cert)
 		assert.Equal(t, VenafiCloudKeypair, got.AuthMode)
 
-		err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: "test cluster name"})
+		err = cl.PostDataReadingsWithOptions(ctx, nil, client.Options{ClusterName: "test cluster name"})
 		require.NoError(t, err)
 	})
 }
@@ -724,6 +731,11 @@ func Test_ValidateAndCombineConfig_VenafiConnection(t *testing.T) {
 	})
 
 	t.Run("the server field is ignored when VenafiConnection is used", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+		log := ktesting.NewLogger(t, ktesting.NewConfig(ktesting.Verbosity(10)))
+		ctx = klog.NewContext(ctx, log)
+
 		expected := srv.URL
 		setVenafiCloudAssert(func(t testing.TB, gotReq *http.Request) {
 			assert.Equal(t, expected, "https://"+gotReq.Host)
@@ -738,13 +750,13 @@ func Test_ValidateAndCombineConfig_VenafiConnection(t *testing.T) {
 			withCmdLineFlags("--venafi-connection", "venafi-components", "--install-namespace", "venafi"))
 		require.NoError(t, err)
 
-		testutil.VenConnStartWatching(t, cl)
+		testutil.VenConnStartWatching(ctx, t, cl)
 		testutil.TrustCA(t, cl, cert)
 
 		// TODO(mael): the client should keep track of the cluster ID, we
 		// shouldn't need to pass it as an option to
 		// PostDataReadingsWithOptions.
-		err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: cfg.ClusterID})
+		err = cl.PostDataReadingsWithOptions(ctx, nil, client.Options{ClusterName: cfg.ClusterID})
 		require.NoError(t, err)
 	})
 }

pkg/agent/run.go

@@ -231,8 +231,6 @@ func Run(cmd *cobra.Command, args []string) (returnErr error) {
 	// If any of the go routines exit (with nil or error) the main context will
 	// be cancelled, which will cause this blocking loop to exit
 	// instead of waiting for the time period.
-	// TODO(wallrj): Pass a context to gatherAndOutputData, so that we don't
-	// have to wait for it to finish before exiting the process.
 	for {
 		if err := gatherAndOutputData(klog.NewContext(ctx, log), eventf, config, preflightClient, dataGatherers); err != nil {
 			return err
@@ -397,9 +395,7 @@ func postData(ctx context.Context, config CombinedConfig, preflightClient client
 
 	if config.AuthMode == VenafiCloudKeypair || config.AuthMode == VenafiCloudVenafiConnection {
 		// orgID and clusterID are not required for Venafi Cloud auth
-		// TODO(wallrj): Pass the context to PostDataReadingsWithOptions, so
-		// that its network operations can be cancelled.
-		err := preflightClient.PostDataReadingsWithOptions(readings, client.Options{
+		err := preflightClient.PostDataReadingsWithOptions(ctx, readings, client.Options{
 			ClusterName:        config.ClusterID,
 			ClusterDescription: config.ClusterDescription,
 		})
@@ -427,9 +423,7 @@ func postData(ctx context.Context, config CombinedConfig, preflightClient client
 		if path == "" {
 			path = "/api/v1/datareadings"
 		}
-		// TODO(wallrj): Pass the context to Post, so that its network
-		// operations can be cancelled.
-		res, err := preflightClient.Post(path, bytes.NewBuffer(data))
+		res, err := preflightClient.Post(ctx, path, bytes.NewBuffer(data))
 
 		if err != nil {
 			return fmt.Errorf("failed to post data: %+v", err)
@@ -453,9 +447,7 @@ func postData(ctx context.Context, config CombinedConfig, preflightClient client
 		return fmt.Errorf("post to server failed: missing clusterID from agent configuration")
 	}
 
-	// TODO(wallrj): Pass the context to PostDataReadings, so
-	// that its network operations can be cancelled.
-	err := preflightClient.PostDataReadings(config.OrganizationID, config.ClusterID, readings)
+	err := preflightClient.PostDataReadings(ctx, config.OrganizationID, config.ClusterID, readings)
 	if err != nil {
 		return fmt.Errorf("post to server failed: %+v", err)
 	}

pkg/client/client.go

@@ -1,6 +1,7 @@
 package client
 
 import (
+	"context"
 	"fmt"
 	"io"
 	"net/http"
@@ -29,9 +30,9 @@ type (
 
 	// The Client interface describes types that perform requests against the Jetstack Secure backend.
 	Client interface {
-		PostDataReadings(orgID, clusterID string, readings []*api.DataReading) error
-		PostDataReadingsWithOptions(readings []*api.DataReading, options Options) error
-		Post(path string, body io.Reader) (*http.Response, error)
+		PostDataReadings(ctx context.Context, orgID, clusterID string, readings []*api.DataReading) error
+		PostDataReadingsWithOptions(ctx context.Context, readings []*api.DataReading, options Options) error
+		Post(ctx context.Context, path string, body io.Reader) (*http.Response, error)
 	}
 
 	// The Credentials interface describes methods for credential types to implement for verification.

pkg/client/client_api_token.go

@@ -2,6 +2,7 @@ package client
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -10,6 +11,7 @@ import (
 	"time"
 
 	"github.com/jetstack/preflight/api"
+	"k8s.io/client-go/transport"
 )
 
 type (
@@ -34,19 +36,22 @@ func NewAPITokenClient(agentMetadata *api.AgentMetadata, apiToken, baseURL strin
 		apiToken:      apiToken,
 		agentMetadata: agentMetadata,
 		baseURL:       baseURL,
-		client:        &http.Client{Timeout: time.Minute},
+		client: &http.Client{
+			Timeout:   time.Minute,
+			Transport: transport.DebugWrappers(http.DefaultTransport),
+		},
 	}, nil
 }
 
 // PostDataReadingsWithOptions uploads the slice of api.DataReading to the Jetstack Secure backend to be processed for later
 // viewing in the user-interface.
-func (c *APITokenClient) PostDataReadingsWithOptions(readings []*api.DataReading, opts Options) error {
-	return c.PostDataReadings(opts.OrgID, opts.ClusterID, readings)
+func (c *APITokenClient) PostDataReadingsWithOptions(ctx context.Context, readings []*api.DataReading, opts Options) error {
+	return c.PostDataReadings(ctx, opts.OrgID, opts.ClusterID, readings)
 }
 
 // PostDataReadings uploads the slice of api.DataReading to the Jetstack Secure backend to be processed for later
 // viewing in the user-interface.
-func (c *APITokenClient) PostDataReadings(orgID, clusterID string, readings []*api.DataReading) error {
+func (c *APITokenClient) PostDataReadings(ctx context.Context, orgID, clusterID string, readings []*api.DataReading) error {
 	payload := api.DataReadingsPost{
 		AgentMetadata:  c.agentMetadata,
 		DataGatherTime: time.Now().UTC(),
@@ -57,7 +62,7 @@ func (c *APITokenClient) PostDataReadings(orgID, clusterID string, readings []*a
 		return err
 	}
 
-	res, err := c.Post(filepath.Join("/api/v1/org", orgID, "datareadings", clusterID), bytes.NewBuffer(data))
+	res, err := c.Post(ctx, filepath.Join("/api/v1/org", orgID, "datareadings", clusterID), bytes.NewBuffer(data))
 	if err != nil {
 		return err
 	}
@@ -77,8 +82,8 @@ func (c *APITokenClient) PostDataReadings(orgID, clusterID string, readings []*a
 }
 
 // Post performs an HTTP POST request.
-func (c *APITokenClient) Post(path string, body io.Reader) (*http.Response, error) {
-	req, err := http.NewRequest(http.MethodPost, fullURL(c.baseURL, path), body)
+func (c *APITokenClient) Post(ctx context.Context, path string, body io.Reader) (*http.Response, error) {
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost, fullURL(c.baseURL, path), body)
 	if err != nil {
 		return nil, err
 	}

pkg/client/client_oauth.go

@@ -2,6 +2,7 @@ package client
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -13,6 +14,7 @@ import (
 
 	"github.com/hashicorp/go-multierror"
 	"github.com/pkg/errors"
+	"k8s.io/client-go/transport"
 
 	"github.com/jetstack/preflight/api"
 )
@@ -93,17 +95,20 @@ func NewOAuthClient(agentMetadata *api.AgentMetadata, credentials *OAuthCredenti
 		credentials:   credentials,
 		baseURL:       baseURL,
 		accessToken:   &accessToken{},
-		client:        &http.Client{Timeout: time.Minute},
+		client: &http.Client{
+			Timeout:   time.Minute,
+			Transport: transport.DebugWrappers(http.DefaultTransport),
+		},
 	}, nil
 }
 
-func (c *OAuthClient) PostDataReadingsWithOptions(readings []*api.DataReading, opts Options) error {
-	return c.PostDataReadings(opts.OrgID, opts.ClusterID, readings)
+func (c *OAuthClient) PostDataReadingsWithOptions(ctx context.Context, readings []*api.DataReading, opts Options) error {
+	return c.PostDataReadings(ctx, opts.OrgID, opts.ClusterID, readings)
 }
 
 // PostDataReadings uploads the slice of api.DataReading to the Jetstack Secure backend to be processed for later
 // viewing in the user-interface.
-func (c *OAuthClient) PostDataReadings(orgID, clusterID string, readings []*api.DataReading) error {
+func (c *OAuthClient) PostDataReadings(ctx context.Context, orgID, clusterID string, readings []*api.DataReading) error {
 	payload := api.DataReadingsPost{
 		AgentMetadata:  c.agentMetadata,
 		DataGatherTime: time.Now().UTC(),
@@ -114,7 +119,7 @@ func (c *OAuthClient) PostDataReadings(orgID, clusterID string, readings []*api.
 		return err
 	}
 
-	res, err := c.Post(filepath.Join("/api/v1/org", orgID, "datareadings", clusterID), bytes.NewBuffer(data))
+	res, err := c.Post(ctx, filepath.Join("/api/v1/org", orgID, "datareadings", clusterID), bytes.NewBuffer(data))
 	if err != nil {
 		return err
 	}
@@ -134,13 +139,13 @@ func (c *OAuthClient) PostDataReadings(orgID, clusterID string, readings []*api.
 }
 
 // Post performs an HTTP POST request.
-func (c *OAuthClient) Post(path string, body io.Reader) (*http.Response, error) {
-	token, err := c.getValidAccessToken()
+func (c *OAuthClient) Post(ctx context.Context, path string, body io.Reader) (*http.Response, error) {
+	token, err := c.getValidAccessToken(ctx)
 	if err != nil {
 		return nil, err
 	}
 
-	req, err := http.NewRequest(http.MethodPost, fullURL(c.baseURL, path), body)
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost, fullURL(c.baseURL, path), body)
 	if err != nil {
 		return nil, err
 	}
@@ -157,9 +162,9 @@ func (c *OAuthClient) Post(path string, body io.Reader) (*http.Response, error)
 // getValidAccessToken returns a valid access token. It will fetch a new access
 // token from the auth server in case the current access token does not exist
 // or it is expired.
-func (c *OAuthClient) getValidAccessToken() (*accessToken, error) {
+func (c *OAuthClient) getValidAccessToken(ctx context.Context) (*accessToken, error) {
 	if c.accessToken.needsRenew() {
-		err := c.renewAccessToken()
+		err := c.renewAccessToken(ctx)
 		if err != nil {
 			return nil, err
 		}
@@ -168,7 +173,7 @@ func (c *OAuthClient) getValidAccessToken() (*accessToken, error) {
 	return c.accessToken, nil
 }
 
-func (c *OAuthClient) renewAccessToken() error {
+func (c *OAuthClient) renewAccessToken(ctx context.Context) error {
 	tokenURL := fmt.Sprintf("https://%s/oauth/token", c.credentials.AuthServerDomain)
 	audience := "https://preflight.jetstack.io/api/v1"
 	payload := url.Values{}
@@ -178,7 +183,7 @@ func (c *OAuthClient) renewAccessToken() error {
 	payload.Set("audience", audience)
 	payload.Set("username", c.credentials.UserID)
 	payload.Set("password", c.credentials.UserSecret)
-	req, err := http.NewRequest("POST", tokenURL, strings.NewReader(payload.Encode()))
+	req, err := http.NewRequestWithContext(ctx, "POST", tokenURL, strings.NewReader(payload.Encode()))
 	if err != nil {
 		return errors.WithStack(err)
 	}