agent/caching: support proxying request query parameters (#6772)

* agent/caching: support proxying request query parameters

* update comment

* rejig other agent log messages to output method followed by path

command/agent/cache/api_proxy.go

@@ -41,8 +41,13 @@ func (ap *APIProxy) Send(ctx context.Context, req *SendRequest) (*SendResponse,
 	fwReq := client.NewRequest(req.Request.Method, req.Request.URL.Path)
 	fwReq.BodyBytes = req.RequestBody
 
+	query := req.Request.URL.Query()
+	if len(query) != 0 {
+		fwReq.Params = query
+	}
+
 	// Make the request to Vault and get the response
-	ap.logger.Info("forwarding request", "path", req.Request.URL.Path, "method", req.Request.Method)
+	ap.logger.Info("forwarding request", "method", req.Request.Method, "path", req.Request.URL.Path)
 
 	resp, err := client.RawRequestWithContext(ctx, fwReq)
 	if resp == nil && err != nil {

command/agent/cache/api_proxy_test.go

@@ -1,9 +1,10 @@
 package cache
 
 import (
+	"net/http"
 	"testing"
 
-	hclog "github.com/hashicorp/go-hclog"
+	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/vault/api"
 	"github.com/hashicorp/vault/helper/jsonutil"
 	"github.com/hashicorp/vault/helper/logging"
@@ -42,6 +43,53 @@ func TestAPIProxy(t *testing.T) {
 	}
 
 	if !result.Initialized || result.Sealed || result.Standby {
-		t.Fatalf("bad sys/health response")
+		t.Fatalf("bad sys/health response: %#v", result)
+	}
+}
+
+func TestAPIProxy_queryParams(t *testing.T) {
+	// Set up an agent that points to a standby node for this particular test
+	// since it needs to proxy a /sys/health?standbyok=true request to a standby
+	cleanup, client, _, _ := setupClusterAndAgentOnStandby(namespace.RootContext(nil), t, nil)
+	defer cleanup()
+
+	proxier, err := NewAPIProxy(&APIProxyConfig{
+		Client: client,
+		Logger: logging.NewVaultLogger(hclog.Trace),
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	r := client.NewRequest("GET", "/v1/sys/health")
+	req, err := r.ToHTTP()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Add a query parameter for testing
+	q := req.URL.Query()
+	q.Add("standbyok", "true")
+	req.URL.RawQuery = q.Encode()
+
+	resp, err := proxier.Send(namespace.RootContext(nil), &SendRequest{
+		Request: req,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var result api.HealthResponse
+	err = jsonutil.DecodeJSONFromReader(resp.Response.Body, &result)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if !result.Initialized || result.Sealed || !result.Standby {
+		t.Fatalf("bad sys/health response: %#v", result)
+	}
+
+	if resp.Response.StatusCode != http.StatusOK {
+		t.Fatalf("exptected standby to return 200, got: %v", resp.Response.StatusCode)
 	}
 }

command/agent/cache/cache_test.go

@@ -34,10 +34,22 @@ path "*" {
 `
 
 // setupClusterAndAgent is a helper func used to set up a test cluster and
-// caching agent. It returns a cleanup func that should be deferred immediately
-// along with two clients, one for direct cluster communication and another to
-// talk to the caching agent.
+// caching agent against the active node. It returns a cleanup func that should
+// be deferred immediately along with two clients, one for direct cluster
+// communication and another to talk to the caching agent.
 func setupClusterAndAgent(ctx context.Context, t *testing.T, coreConfig *vault.CoreConfig) (func(), *api.Client, *api.Client, *LeaseCache) {
+	return setupClusterAndAgentCommon(ctx, t, coreConfig, false)
+}
+
+// setupClusterAndAgentOnStandby is a helper func used to set up a test cluster
+// and caching agent against a standby node. It returns a cleanup func that
+// should be deferred immediately along with two clients, one for direct cluster
+// communication and another to talk to the caching agent.
+func setupClusterAndAgentOnStandby(ctx context.Context, t *testing.T, coreConfig *vault.CoreConfig) (func(), *api.Client, *api.Client, *LeaseCache) {
+	return setupClusterAndAgentCommon(ctx, t, coreConfig, true)
+}
+
+func setupClusterAndAgentCommon(ctx context.Context, t *testing.T, coreConfig *vault.CoreConfig, onStandby bool) (func(), *api.Client, *api.Client, *LeaseCache) {
 	t.Helper()
 
 	if ctx == nil {
@@ -70,21 +82,30 @@ func setupClusterAndAgent(ctx context.Context, t *testing.T, coreConfig *vault.C
 	cores := cluster.Cores
 	vault.TestWaitActive(t, cores[0].Core)
 
-	// clusterClient is the client that is used to talk directly to the cluster.
-	clusterClient := cores[0].Client
+	activeClient := cores[0].Client
+	standbyClient := cores[1].Client
+
+	// clienToUse is the client for the agent to point to.
+	clienToUse := activeClient
+	if onStandby {
+		clienToUse = standbyClient
+	}
 
 	// Add an admin policy
-	if err := clusterClient.Sys().PutPolicy("admin", policyAdmin); err != nil {
+	if err := activeClient.Sys().PutPolicy("admin", policyAdmin); err != nil {
 		t.Fatal(err)
 	}
 
 	// Set up the userpass auth backend and an admin user. Used for getting a token
 	// for the agent later down in this func.
-	clusterClient.Sys().EnableAuthWithOptions("userpass", &api.EnableAuthOptions{
+	err := activeClient.Sys().EnableAuthWithOptions("userpass", &api.EnableAuthOptions{
 		Type: "userpass",
 	})
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	_, err := clusterClient.Logical().Write("auth/userpass/users/foo", map[string]interface{}{
+	_, err = activeClient.Logical().Write("auth/userpass/users/foo", map[string]interface{}{
 		"password": "bar",
 		"policies": []string{"admin"},
 	})
@@ -94,7 +115,7 @@ func setupClusterAndAgent(ctx context.Context, t *testing.T, coreConfig *vault.C
 
 	// Set up env vars for agent consumption
 	origEnvVaultAddress := os.Getenv(api.EnvVaultAddress)
-	os.Setenv(api.EnvVaultAddress, clusterClient.Address())
+	os.Setenv(api.EnvVaultAddress, clienToUse.Address())
 
 	origEnvVaultCACert := os.Getenv(api.EnvVaultCACert)
 	os.Setenv(api.EnvVaultCACert, fmt.Sprintf("%s/ca_cert.pem", cluster.TempDir))
@@ -108,7 +129,7 @@ func setupClusterAndAgent(ctx context.Context, t *testing.T, coreConfig *vault.C
 
 	// Create the API proxier
 	apiProxy, err := NewAPIProxy(&APIProxyConfig{
-		Client: clusterClient,
+		Client: clienToUse,
 		Logger: cacheLogger.Named("apiproxy"),
 	})
 	if err != nil {
@@ -118,7 +139,7 @@ func setupClusterAndAgent(ctx context.Context, t *testing.T, coreConfig *vault.C
 	// Create the lease cache proxier and set its underlying proxier to
 	// the API proxier.
 	leaseCache, err := NewLeaseCache(&LeaseCacheConfig{
-		Client:      clusterClient,
+		Client:      clienToUse,
 		BaseContext: ctx,
 		Proxier:     apiProxy,
 		Logger:      cacheLogger.Named("leasecache"),
@@ -142,7 +163,7 @@ func setupClusterAndAgent(ctx context.Context, t *testing.T, coreConfig *vault.C
 	go server.Serve(listener)
 
 	// testClient is the client that is used to talk to the agent for proxying/caching behavior.
-	testClient, err := clusterClient.Clone()
+	testClient, err := activeClient.Clone()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -171,7 +192,7 @@ func setupClusterAndAgent(ctx context.Context, t *testing.T, coreConfig *vault.C
 		listener.Close()
 	}
 
-	return cleanup, clusterClient, testClient, leaseCache
+	return cleanup, clienToUse, testClient, leaseCache
 }
 
 func tokenRevocationValidation(t *testing.T, sampleSpace map[string]string, expected map[string]string, leaseCache *LeaseCache) {

command/agent/cache/handler.go

@@ -22,11 +22,11 @@ import (
 
 func Handler(ctx context.Context, logger hclog.Logger, proxier Proxier, inmemSink sink.Sink) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		logger.Info("received request", "path", r.URL.Path, "method", r.Method)
+		logger.Info("received request", "method", r.Method, "path", r.URL.Path)
 
 		token := r.Header.Get(consts.AuthHeaderName)
 		if token == "" && inmemSink != nil {
-			logger.Debug("using auto auth token", "path", r.URL.Path, "method", r.Method)
+			logger.Debug("using auto auth token", "method", r.Method, "path", r.URL.Path)
 			token = inmemSink.(sink.SinkReader).Token()
 		}
 
@@ -148,7 +148,7 @@ func processTokenLookupResponse(ctx context.Context, logger hclog.Logger, inmemS
 		return nil
 	}
 
-	logger.Info("stripping auto-auth token from the response", "path", req.Request.URL.Path, "method", req.Request.Method)
+	logger.Info("stripping auto-auth token from the response", "method", req.Request.Method, "path", req.Request.URL.Path)
 	secret, err := api.ParseSecret(bytes.NewReader(resp.ResponseBody))
 	if err != nil {
 		return fmt.Errorf("failed to parse token lookup response: %v", err)

command/agent/cache/lease_cache.go

@@ -202,11 +202,11 @@ func (c *LeaseCache) Send(ctx context.Context, req *SendRequest) (*SendResponse,
 	// in between this upgrade so we can simply return that. Otherwise, this request
 	// will be the one performing the cache write.
 	if sendResp != nil {
-		c.logger.Debug("returning cached response", "path", req.Request.URL.Path)
+		c.logger.Debug("returning cached response", "method", req.Request.Method, "path", req.Request.URL.Path)
 		return sendResp, nil
 	}
 
-	c.logger.Debug("forwarding request", "path", req.Request.URL.Path, "method", req.Request.Method)
+	c.logger.Debug("forwarding request", "method", req.Request.Method, "path", req.Request.URL.Path)
 
 	// Pass the request down and get a response
 	resp, err := c.proxier.Send(ctx, req)
@@ -254,7 +254,7 @@ func (c *LeaseCache) Send(ctx context.Context, req *SendRequest) (*SendResponse,
 
 	// Fast path for responses with no secrets
 	if secret == nil {
-		c.logger.Debug("pass-through response; no secret in response", "path", req.Request.URL.Path, "method", req.Request.Method)
+		c.logger.Debug("pass-through response; no secret in response", "method", req.Request.Method, "path", req.Request.URL.Path)
 		return resp, nil
 	}
 
@@ -265,22 +265,22 @@ func (c *LeaseCache) Send(ctx context.Context, req *SendRequest) (*SendResponse,
 		return nil, err
 	}
 	if !secret.Renewable && !tokenRenewable {
-		c.logger.Debug("pass-through response; secret not renewable", "path", req.Request.URL.Path, "method", req.Request.Method)
+		c.logger.Debug("pass-through response; secret not renewable", "method", req.Request.Method, "path", req.Request.URL.Path)
 		return resp, nil
 	}
 
 	var renewCtxInfo *cachememdb.ContextInfo
 	switch {
 	case secret.LeaseID != "":
-		c.logger.Debug("processing lease response", "path", req.Request.URL.Path, "method", req.Request.Method)
+		c.logger.Debug("processing lease response", "method", req.Request.Method, "path", req.Request.URL.Path)
 		entry, err := c.db.Get(cachememdb.IndexNameToken, req.Token)
 		if err != nil {
 			return nil, err
 		}
 		// If the lease belongs to a token that is not managed by the agent,
 		// return the response without caching it.
 		if entry == nil {
-			c.logger.Debug("pass-through lease response; token not managed by agent", "path", req.Request.URL.Path, "method", req.Request.Method)
+			c.logger.Debug("pass-through lease response; token not managed by agent", "method", req.Request.Method, "path", req.Request.URL.Path)
 			return resp, nil
 		}
 
@@ -291,7 +291,7 @@ func (c *LeaseCache) Send(ctx context.Context, req *SendRequest) (*SendResponse,
 		index.LeaseToken = req.Token
 
 	case secret.Auth != nil:
-		c.logger.Debug("processing auth response", "path", req.Request.URL.Path, "method", req.Request.Method)
+		c.logger.Debug("processing auth response", "method", req.Request.Method, "path", req.Request.URL.Path)
 
 		// Check if this token creation request resulted in a non-orphan token, and if so
 		// correctly set the parentCtx to the request's token context.
@@ -304,11 +304,11 @@ func (c *LeaseCache) Send(ctx context.Context, req *SendRequest) (*SendResponse,
 			// If parent token is not managed by the agent, child shouldn't be
 			// either.
 			if entry == nil {
-				c.logger.Debug("pass-through auth response; parent token not managed by agent", "path", req.Request.URL.Path, "method", req.Request.Method)
+				c.logger.Debug("pass-through auth response; parent token not managed by agent", "method", req.Request.Method, "path", req.Request.URL.Path)
 				return resp, nil
 			}
 
-			c.logger.Debug("setting parent context", "path", req.Request.URL.Path, "method", req.Request.Method)
+			c.logger.Debug("setting parent context", "method", req.Request.Method, "path", req.Request.URL.Path)
 			parentCtx = entry.RenewCtxInfo.Ctx
 
 			entry.TokenParent = req.Token
@@ -321,7 +321,7 @@ func (c *LeaseCache) Send(ctx context.Context, req *SendRequest) (*SendResponse,
 	default:
 		// We shouldn't be hitting this, but will err on the side of caution and
 		// simply proxy.
-		c.logger.Debug("pass-through response; secret without lease and token", "path", req.Request.URL.Path, "method", req.Request.Method)
+		c.logger.Debug("pass-through response; secret without lease and token", "method", req.Request.Method, "path", req.Request.URL.Path)
 		return resp, nil
 	}
 
@@ -353,7 +353,7 @@ func (c *LeaseCache) Send(ctx context.Context, req *SendRequest) (*SendResponse,
 	}
 
 	// Store the index in the cache
-	c.logger.Debug("storing response into the cache", "path", req.Request.URL.Path, "method", req.Request.Method)
+	c.logger.Debug("storing response into the cache", "method", req.Request.Method, "path", req.Request.URL.Path)
 	err = c.db.Set(index)
 	if err != nil {
 		c.logger.Error("failed to cache the proxied response", "error", err)
@@ -378,7 +378,7 @@ func (c *LeaseCache) createCtxInfo(ctx context.Context) *cachememdb.ContextInfo
 func (c *LeaseCache) startRenewing(ctx context.Context, index *cachememdb.Index, req *SendRequest, secret *api.Secret) {
 	defer func() {
 		id := ctx.Value(contextIndexID).(string)
-		c.logger.Debug("evicting index from cache", "id", id, "path", req.Request.URL.Path, "method", req.Request.Method)
+		c.logger.Debug("evicting index from cache", "id", id, "method", req.Request.Method, "path", req.Request.URL.Path)
 		err := c.db.Evict(cachememdb.IndexNameID, id)
 		if err != nil {
 			c.logger.Error("failed to evict index", "id", id, "error", err)
@@ -402,7 +402,7 @@ func (c *LeaseCache) startRenewing(ctx context.Context, index *cachememdb.Index,
 		return
 	}
 
-	c.logger.Debug("initiating renewal", "path", req.Request.URL.Path, "method", req.Request.Method)
+	c.logger.Debug("initiating renewal", "method", req.Request.Method, "path", req.Request.URL.Path)
 	go renewer.Renew()
 	defer renewer.Stop()