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-36032] Pass the context to Venafi clients and enable debug roundtripper #627

Merged
merged 6 commits into from
Nov 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
14 changes: 12 additions & 2 deletions deploy/charts/venafi-kubernetes-agent/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#### **volumes** ~ `array`
> Default value:
> ```yaml
Expand Down
2 changes: 1 addition & 1 deletion deploy/charts/venafi-kubernetes-agent/values.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
11 changes: 10 additions & 1 deletion deploy/charts/venafi-kubernetes-agent/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: []
Copy link
Member Author

@wallrj wallrj Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


# Additional volumes to add to the Venafi Kubernetes Agent container. This is
Expand Down
2 changes: 1 addition & 1 deletion hack/e2e/values.venafi-kubernetes-agent.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ authentication:

extraArgs:
- --logging-format=json
- --log-level=2
- --log-level=6
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's how it looks in Google Logs Explorer

image

18 changes: 15 additions & 3 deletions pkg/agent/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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=
Expand Down Expand Up @@ -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)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing gets logged by the client in this test

$ go test ./pkg/agent/... -v -run "Test_ValidateAndCombineConfig_VenafiCloudKeyPair"
=== RUN   Test_ValidateAndCombineConfig_VenafiCloudKeyPair
=== RUN   Test_ValidateAndCombineConfig_VenafiCloudKeyPair/server,_uploader_id,_and_cluster_name_are_correctly_passed
    envtest.go:188: fake api.venafi.cloud received request: POST /v1/oauth/token/serviceaccount
    envtest.go:188: fake api.venafi.cloud received request: POST /v1/tlspk/upload/clusterdata/no
--- PASS: Test_ValidateAndCombineConfig_VenafiCloudKeyPair (0.01s)
    --- PASS: Test_ValidateAndCombineConfig_VenafiCloudKeyPair/server,_uploader_id,_and_cluster_name_are_correctly_passed (0.01s)
PASS
ok      github.com/jetstack/preflight/pkg/agent 0.044s

})
}
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ go test ./pkg/agent/... -v -run "Test_ValidateAndCombineConfig_VenafiConnection"
=== RUN   Test_ValidateAndCombineConfig_VenafiConnection
=== RUN   Test_ValidateAndCombineConfig_VenafiConnection/err_when_cluster_id_field_is_empty
=== RUN   Test_ValidateAndCombineConfig_VenafiConnection/the_server_field_is_ignored_when_VenafiConnection_is_used
=== NAME  Test_ValidateAndCombineConfig_VenafiConnection
    envtest.go:188: fake api.venafi.cloud received request: POST /v1/tlspk/upload/clusterdata/no
    envtest.go:51: Waiting for envtest to exit
--- PASS: Test_ValidateAndCombineConfig_VenafiConnection (7.01s)
    --- PASS: Test_ValidateAndCombineConfig_VenafiConnection/err_when_cluster_id_field_is_empty (0.00s)
    --- PASS: Test_ValidateAndCombineConfig_VenafiConnection/the_server_field_is_ignored_when_VenafiConnection_is_used (0.17s)
PASS
ok      github.com/jetstack/preflight/pkg/agent 7.041s

})
}
Expand Down
14 changes: 3 additions & 11 deletions pkg/agent/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
})
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/client/client.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package client

import (
"context"
"fmt"
"io"
"net/http"
Expand Down Expand Up @@ -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.
Expand Down
19 changes: 12 additions & 7 deletions pkg/client/client_api_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package client

import (
"bytes"
"context"
"encoding/json"
"fmt"
"io"
Expand All @@ -10,6 +11,7 @@ import (
"time"

"github.com/jetstack/preflight/api"
"k8s.io/client-go/transport"
)

type (
Expand All @@ -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(),
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down
29 changes: 17 additions & 12 deletions pkg/client/client_oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package client

import (
"bytes"
"context"
"encoding/json"
"fmt"
"io"
Expand All @@ -13,6 +14,7 @@ import (

"github.com/hashicorp/go-multierror"
"github.com/pkg/errors"
"k8s.io/client-go/transport"

"github.com/jetstack/preflight/api"
)
Expand Down Expand Up @@ -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(),
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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{}
Expand All @@ -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)
}
Expand Down
Loading