From 98afe3b70434d1f64881d7c1b97e5e9a1cbedece Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Thu, 28 Nov 2024 10:10:03 +0000 Subject: [PATCH] [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 --- .../charts/venafi-kubernetes-agent/README.md | 14 +++++++-- .../values.schema.json | 2 +- .../venafi-kubernetes-agent/values.yaml | 11 ++++++- hack/e2e/values.venafi-kubernetes-agent.yaml | 2 +- pkg/agent/config_test.go | 18 ++++++++++-- pkg/agent/run.go | 14 ++------- pkg/client/client.go | 7 +++-- pkg/client/client_api_token.go | 19 +++++++----- pkg/client/client_oauth.go | 29 +++++++++++-------- pkg/client/client_venafi_cloud.go | 27 ++++++++++------- pkg/client/client_venconn.go | 15 +++++----- pkg/client/client_venconn_test.go | 28 +++++++++++------- pkg/logs/logs.go | 2 +- pkg/logs/logs_test.go | 2 +- pkg/testutil/envtest.go | 4 +-- 15 files changed, 120 insertions(+), 74 deletions(-) diff --git a/deploy/charts/venafi-kubernetes-agent/README.md b/deploy/charts/venafi-kubernetes-agent/README.md index af9098f9..1d7a7a5b 100644 --- a/deploy/charts/venafi-kubernetes-agent/README.md +++ b/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 diff --git a/deploy/charts/venafi-kubernetes-agent/values.schema.json b/deploy/charts/venafi-kubernetes-agent/values.schema.json index dea7c43c..36de4b36 100644 --- a/deploy/charts/venafi-kubernetes-agent/values.schema.json +++ b/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" }, diff --git a/deploy/charts/venafi-kubernetes-agent/values.yaml b/deploy/charts/venafi-kubernetes-agent/values.yaml index bff466bb..88ceee4f 100644 --- a/deploy/charts/venafi-kubernetes-agent/values.yaml +++ b/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 diff --git a/hack/e2e/values.venafi-kubernetes-agent.yaml b/hack/e2e/values.venafi-kubernetes-agent.yaml index 18d5a073..630d76ea 100644 --- a/hack/e2e/values.venafi-kubernetes-agent.yaml +++ b/hack/e2e/values.venafi-kubernetes-agent.yaml @@ -10,4 +10,4 @@ authentication: extraArgs: - --logging-format=json -- --log-level=2 +- --log-level=6 diff --git a/pkg/agent/config_test.go b/pkg/agent/config_test.go index 79cb7f9b..b2e23ab9 100644 --- a/pkg/agent/config_test.go +++ b/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) }) } diff --git a/pkg/agent/run.go b/pkg/agent/run.go index 54832dbe..f7c65da7 100644 --- a/pkg/agent/run.go +++ b/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) } diff --git a/pkg/client/client.go b/pkg/client/client.go index fef5be65..fea102bb 100644 --- a/pkg/client/client.go +++ b/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. diff --git a/pkg/client/client_api_token.go b/pkg/client/client_api_token.go index 2ee04cb1..33588d34 100644 --- a/pkg/client/client_api_token.go +++ b/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 } diff --git a/pkg/client/client_oauth.go b/pkg/client/client_oauth.go index 89aca601..c410d0c5 100644 --- a/pkg/client/client_oauth.go +++ b/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) } diff --git a/pkg/client/client_venafi_cloud.go b/pkg/client/client_venafi_cloud.go index 7e317faf..459099d6 100644 --- a/pkg/client/client_venafi_cloud.go +++ b/pkg/client/client_venafi_cloud.go @@ -2,6 +2,7 @@ package client import ( "bytes" + "context" "crypto" "crypto/ecdsa" "crypto/ed25519" @@ -26,6 +27,7 @@ import ( "github.com/google/uuid" "github.com/hashicorp/go-multierror" "github.com/microcosm-cc/bluemonday" + "k8s.io/client-go/transport" "github.com/jetstack/preflight/api" ) @@ -111,7 +113,10 @@ func NewVenafiCloudClient(agentMetadata *api.AgentMetadata, credentials *VenafiS credentials: credentials, baseURL: baseURL, accessToken: &venafiCloudAccessToken{}, - Client: &http.Client{Timeout: time.Minute}, + Client: &http.Client{ + Timeout: time.Minute, + Transport: transport.DebugWrappers(http.DefaultTransport), + }, uploaderID: uploaderID, uploadPath: uploadPath, privateKey: privateKey, @@ -168,7 +173,7 @@ func (c *VenafiSvcAccountCredentials) IsClientSet() (ok bool, why string) { // PostDataReadingsWithOptions uploads the slice of api.DataReading to the Venafi Cloud backend to be processed. // The Options are then passed as URL params in the request -func (c *VenafiCloudClient) PostDataReadingsWithOptions(readings []*api.DataReading, opts Options) error { +func (c *VenafiCloudClient) PostDataReadingsWithOptions(ctx context.Context, readings []*api.DataReading, opts Options) error { payload := api.DataReadingsPost{ AgentMetadata: c.agentMetadata, DataGatherTime: time.Now().UTC(), @@ -199,7 +204,7 @@ func (c *VenafiCloudClient) PostDataReadingsWithOptions(readings []*api.DataRead } venafiCloudUploadURL.RawQuery = query.Encode() - res, err := c.Post(venafiCloudUploadURL.String(), bytes.NewBuffer(data)) + res, err := c.Post(ctx, venafiCloudUploadURL.String(), bytes.NewBuffer(data)) if err != nil { return err } @@ -219,7 +224,7 @@ func (c *VenafiCloudClient) PostDataReadingsWithOptions(readings []*api.DataRead // PostDataReadings uploads the slice of api.DataReading to the Venafi Cloud backend to be processed for later // viewing in the user-interface. -func (c *VenafiCloudClient) PostDataReadings(_ string, _ string, readings []*api.DataReading) error { +func (c *VenafiCloudClient) PostDataReadings(ctx context.Context, _ string, _ string, readings []*api.DataReading) error { // orgID and clusterID are ignored in Venafi Cloud auth payload := api.DataReadingsPost{ @@ -235,7 +240,7 @@ func (c *VenafiCloudClient) PostDataReadings(_ string, _ string, readings []*api if !strings.HasSuffix(c.uploadPath, "/") { c.uploadPath = fmt.Sprintf("%s/", c.uploadPath) } - res, err := c.Post(filepath.Join(c.uploadPath, c.uploaderID), bytes.NewBuffer(data)) + res, err := c.Post(ctx, filepath.Join(c.uploadPath, c.uploaderID), bytes.NewBuffer(data)) if err != nil { return err } @@ -254,8 +259,8 @@ func (c *VenafiCloudClient) PostDataReadings(_ string, _ string, readings []*api } // Post performs an HTTP POST request. -func (c *VenafiCloudClient) Post(path string, body io.Reader) (*http.Response, error) { - token, err := c.getValidAccessToken() +func (c *VenafiCloudClient) Post(ctx context.Context, path string, body io.Reader) (*http.Response, error) { + token, err := c.getValidAccessToken(ctx) if err != nil { return nil, err } @@ -278,9 +283,9 @@ func (c *VenafiCloudClient) Post(path string, body io.Reader) (*http.Response, e // 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 *VenafiCloudClient) getValidAccessToken() (*venafiCloudAccessToken, error) { +func (c *VenafiCloudClient) getValidAccessToken(ctx context.Context) (*venafiCloudAccessToken, error) { if c.accessToken == nil || time.Now().Add(time.Minute).After(c.accessToken.expirationTime) { - err := c.updateAccessToken() + err := c.updateAccessToken(ctx) if err != nil { return nil, err } @@ -289,7 +294,7 @@ func (c *VenafiCloudClient) getValidAccessToken() (*venafiCloudAccessToken, erro return c.accessToken, nil } -func (c *VenafiCloudClient) updateAccessToken() error { +func (c *VenafiCloudClient) updateAccessToken(ctx context.Context) error { jwtToken, err := c.generateAndSignJwtToken() if err != nil { return err @@ -302,7 +307,7 @@ func (c *VenafiCloudClient) updateAccessToken() error { tokenURL := fullURL(c.baseURL, accessTokenEndpoint) encoded := values.Encode() - request, err := http.NewRequest(http.MethodPost, tokenURL, strings.NewReader(encoded)) + request, err := http.NewRequestWithContext(ctx, http.MethodPost, tokenURL, strings.NewReader(encoded)) if err != nil { return err } diff --git a/pkg/client/client_venconn.go b/pkg/client/client_venconn.go index d8553624..a0ee8646 100644 --- a/pkg/client/client_venconn.go +++ b/pkg/client/client_venconn.go @@ -19,6 +19,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/rest" + "k8s.io/client-go/transport" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "github.com/jetstack/preflight/api" @@ -99,11 +100,11 @@ func NewVenConnClient(restcfg *rest.Config, agentMetadata *api.AgentMetadata, in } vcpClient := &http.Client{} + tr := http.DefaultTransport.(*http.Transport).Clone() if trustedCAs != nil { - tr := http.DefaultTransport.(*http.Transport).Clone() tr.TLSClientConfig.RootCAs = trustedCAs - vcpClient.Transport = tr } + vcpClient.Transport = transport.DebugWrappers(tr) return &VenConnClient{ agentMetadata: agentMetadata, @@ -123,12 +124,12 @@ func (c *VenConnClient) Start(ctx context.Context) error { // `opts.ClusterName` and `opts.ClusterDescription` are the only values used // from the Options struct. OrgID and ClusterID are not used in Venafi Cloud. -func (c *VenConnClient) PostDataReadingsWithOptions(readings []*api.DataReading, opts Options) error { +func (c *VenConnClient) PostDataReadingsWithOptions(ctx context.Context, readings []*api.DataReading, opts Options) error { if opts.ClusterName == "" { return fmt.Errorf("programmer mistake: the cluster name (aka `cluster_id` in the config file) cannot be left empty") } - _, token, err := c.connHandler.Get(context.Background(), c.installNS, auth.Scope{}, types.NamespacedName{Name: c.venConnName, Namespace: c.venConnNS}) + _, token, err := c.connHandler.Get(ctx, c.installNS, auth.Scope{}, types.NamespacedName{Name: c.venConnName, Namespace: c.venConnNS}) if err != nil { return fmt.Errorf("while loading the VenafiConnection %s/%s: %w", c.venConnNS, c.venConnName, err) } @@ -161,7 +162,7 @@ func (c *VenConnClient) PostDataReadingsWithOptions(readings []*api.DataReading, // 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"), encodedBody) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, fullURL(token.BaseURL, "/v1/tlspk/upload/clusterdata/no"), encodedBody) if err != nil { return err } @@ -206,13 +207,13 @@ func (c *VenConnClient) PostDataReadingsWithOptions(readings []*api.DataReading, // Cloud needs a `clusterName` and `clusterDescription`, but this function can // only pass `orgID` and `clusterID` which are both useless in Venafi Cloud. Use // PostDataReadingsWithOptions instead. -func (c *VenConnClient) PostDataReadings(_orgID, _clusterID string, readings []*api.DataReading) error { +func (c *VenConnClient) PostDataReadings(_ context.Context, _orgID, _clusterID string, readings []*api.DataReading) error { return fmt.Errorf("programmer mistake: PostDataReadings is not implemented for Venafi Cloud") } // Post isn't implemented for Venafi Cloud because /v1/tlspk/upload/clusterdata // requires using the query parameters `name` and `description` which can't be // set using Post. Use PostDataReadingsWithOptions instead. -func (c *VenConnClient) Post(path string, body io.Reader) (*http.Response, error) { +func (c *VenConnClient) Post(_ context.Context, path string, body io.Reader) (*http.Response, error) { return nil, fmt.Errorf("programmer mistake: Post is not implemented for Venafi Cloud") } diff --git a/pkg/client/client_venconn_test.go b/pkg/client/client_venconn_test.go index f765f1ad..91e50edd 100644 --- a/pkg/client/client_venconn_test.go +++ b/pkg/client/client_venconn_test.go @@ -13,6 +13,8 @@ import ( "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/rest" + "k8s.io/klog/v2" + "k8s.io/klog/v2/ktesting" ctrlruntime "sigs.k8s.io/controller-runtime/pkg/client" "github.com/jetstack/preflight/api" @@ -34,13 +36,17 @@ import ( // // [1] https://github.com/kubernetes-sigs/controller-runtime/issues/2341 func TestVenConnClient_PostDataReadingsWithOptions(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) _, restconf, kclient := testutil.WithEnvtest(t) for _, obj := range testutil.Parse(testutil.VenConnRBAC) { - require.NoError(t, kclient.Create(context.Background(), obj)) + require.NoError(t, kclient.Create(ctx, obj)) } t.Parallel() - t.Run("valid accessToken", run_TestVenConnClient_PostDataReadingsWithOptions(restconf, kclient, testcase{ + t.Run("valid accessToken", run_TestVenConnClient_PostDataReadingsWithOptions(ctx, restconf, kclient, testcase{ given: testutil.Undent(` apiVersion: jetstack.io/v1alpha1 kind: VenafiConnection @@ -93,7 +99,7 @@ func TestVenConnClient_PostDataReadingsWithOptions(t *testing.T) { `), expectReadyCondMsg: "ea744d098c2c1c6044e4c4e9d3bf7c2a68ef30553db00f1714886cedf73230f1", })) - t.Run("error when the apiKey field is used", run_TestVenConnClient_PostDataReadingsWithOptions(restconf, kclient, testcase{ + t.Run("error when the apiKey field is used", run_TestVenConnClient_PostDataReadingsWithOptions(ctx, restconf, kclient, testcase{ // Why isn't it possible to use the 'apiKey' field? Although the // Kubernetes Discovery endpoint works with an API key, we have decided // to not support it because it isn't recommended. @@ -152,7 +158,7 @@ func TestVenConnClient_PostDataReadingsWithOptions(t *testing.T) { expectReadyCondMsg: "b099d634ccec56556da28028743475dab67f79d079b668bedc3ef544f7eed2f3", expectErr: "VenafiConnection error-when-the-apikey-field-is-used/venafi-components: the agent cannot be used with an API key", })) - t.Run("error when the tpp field is used", run_TestVenConnClient_PostDataReadingsWithOptions(restconf, kclient, testcase{ + t.Run("error when the tpp field is used", run_TestVenConnClient_PostDataReadingsWithOptions(ctx, restconf, kclient, testcase{ // IMPORTANT: The user may think they can use 'tpp', spend time // debugging and making the venafi connection work, and then find out // that it doesn't work. The reason is because as of now, we don't first @@ -220,10 +226,11 @@ type testcase struct { // All tests share the same envtest (i.e., the same apiserver and etcd process), // so each test needs to be contained in its own Kubernetes namespace. -func run_TestVenConnClient_PostDataReadingsWithOptions(restcfg *rest.Config, kclient ctrlruntime.WithWatch, test testcase) func(t *testing.T) { +func run_TestVenConnClient_PostDataReadingsWithOptions(ctx context.Context, restcfg *rest.Config, kclient ctrlruntime.WithWatch, test testcase) func(t *testing.T) { return func(t *testing.T) { t.Helper() - + log := ktesting.NewLogger(t, ktesting.NewConfig(ktesting.Verbosity(10))) + ctx := klog.NewContext(ctx, log) fakeVenafiCloud, certCloud, fakeVenafiAssert := testutil.FakeVenafiCloud(t) fakeTPP, certTPP := testutil.FakeTPP(t) fakeVenafiAssert(func(t testing.TB, r *http.Request) { @@ -249,7 +256,7 @@ func run_TestVenConnClient_PostDataReadingsWithOptions(restcfg *rest.Config, kcl ) require.NoError(t, err) - testutil.VenConnStartWatching(t, cl) + testutil.VenConnStartWatching(ctx, t, cl) test.given = strings.ReplaceAll(test.given, "FAKE_VENAFI_CLOUD_URL", fakeVenafiCloud.URL) test.given = strings.ReplaceAll(test.given, "FAKE_TPP_URL", fakeTPP.URL) @@ -263,10 +270,9 @@ func run_TestVenConnClient_PostDataReadingsWithOptions(restcfg *rest.Config, kcl name: `+testNameToNamespace(t)))...) givenObjs = append(givenObjs, testutil.Parse(test.given)...) for _, obj := range givenObjs { - require.NoError(t, kclient.Create(context.Background(), obj)) + require.NoError(t, kclient.Create(ctx, obj)) } - - err = cl.PostDataReadingsWithOptions([]*api.DataReading{}, client.Options{ClusterName: "test cluster name"}) + err = cl.PostDataReadingsWithOptions(ctx, []*api.DataReading{}, client.Options{ClusterName: "test cluster name"}) if test.expectErr != "" { assert.EqualError(t, err, test.expectErr) } else { @@ -274,7 +280,7 @@ func run_TestVenConnClient_PostDataReadingsWithOptions(restcfg *rest.Config, kcl } got := v1alpha1.VenafiConnection{} - err = kclient.Get(context.Background(), types.NamespacedName{Name: "venafi-components", Namespace: testNameToNamespace(t)}, &got) + err = kclient.Get(ctx, types.NamespacedName{Name: "venafi-components", Namespace: testNameToNamespace(t)}, &got) require.NoError(t, err) require.Len(t, got.Status.Conditions, 1) assert.Equal(t, test.expectReadyCondMsg, got.Status.Conditions[0].Message) diff --git a/pkg/logs/logs.go b/pkg/logs/logs.go index f2e6dd8f..5ebe6cf7 100644 --- a/pkg/logs/logs.go +++ b/pkg/logs/logs.go @@ -104,7 +104,7 @@ func AddFlags(fs *pflag.FlagSet) { if f.Name == "v" { f.Name = "log-level" f.Shorthand = "v" - f.Usage = fmt.Sprintf("%s. 0=Info, 1=Debug, 2=Trace. Use 3-10 for even greater detail. (default: 0)", f.Usage) + f.Usage = fmt.Sprintf("%s. 0=Info, 1=Debug, 2=Trace. Use 6-9 for increasingly verbose HTTP request logging. (default: 0)", f.Usage) } }) fs.AddFlagSet(&tfs) diff --git a/pkg/logs/logs_test.go b/pkg/logs/logs_test.go index f48a1b4c..c903fc47 100644 --- a/pkg/logs/logs_test.go +++ b/pkg/logs/logs_test.go @@ -83,7 +83,7 @@ func TestLogs(t *testing.T) { name: "help", flags: "-h", expectStdout: ` - -v, --log-level Level number for the log level verbosity. 0=Info, 1=Debug, 2=Trace. Use 3-10 for even greater detail. (default: 0) + -v, --log-level Level number for the log level verbosity. 0=Info, 1=Debug, 2=Trace. Use 6-9 for increasingly verbose HTTP request logging. (default: 0) --logging-format string Sets the log format. Permitted formats: "json", "text". (default "text") --vmodule pattern=N,... comma-separated list of pattern=N settings for file-filtered logging (only works for text log format) `, diff --git a/pkg/testutil/envtest.go b/pkg/testutil/envtest.go index cd93b4a6..e8d5f4e6 100644 --- a/pkg/testutil/envtest.go +++ b/pkg/testutil/envtest.go @@ -106,7 +106,7 @@ func WithKubeconfig(t testing.TB, restCfg *rest.Config) string { // Tests calling to VenConnClient.PostDataReadingsWithOptions must call this // function to start the VenafiConnection watcher. If you don't call this, the // test will stall. -func VenConnStartWatching(t *testing.T, cl client.Client) { +func VenConnStartWatching(ctx context.Context, t *testing.T, cl client.Client) { t.Helper() require.IsType(t, &client.VenConnClient{}, cl) @@ -116,7 +116,7 @@ func VenConnStartWatching(t *testing.T, cl client.Client) { // the message "timeout waiting for process kube-apiserver to stop". See: // https://github.com/jetstack/venafi-connection-lib/pull/158#issuecomment-1949002322 // https://github.com/kubernetes-sigs/controller-runtime/issues/1571#issuecomment-945535598 - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(ctx) go func() { err := cl.(*client.VenConnClient).Start(ctx) require.NoError(t, err)