Verbose cache (#208)

Per #144, made cache handler have a verbose/succinct output. Default is to run succinctly. If URL contains parameter `verbose=true`, it will run the original verbose method, including all request metadata. Otherwise, it will return the number of requests in its place. Signed-off-by: Samra Belachew <[email protected]>
envoyproxy · Jan 6, 2021 · b984214 · b984214
1 parent c591a20
commit b984214
Show file tree

Hide file tree

Showing 8 changed files with 79 additions and 24 deletions.
diff --git a/internal/app/admin/http/cache.go b/internal/app/admin/http/cache.go
@@ -190,10 +190,11 @@ func keyDumpHandler(o *orchestrator.Orchestrator) http.HandlerFunc {
 func cacheDumpHandler(o *orchestrator.Orchestrator) http.HandlerFunc {
 	return func(w http.ResponseWriter, req *http.Request) {
 		cacheKey := getParam(req.URL.Path, cacheURL)
+		isVerbose, _ := getBoolQueryValue(req.URL.Query(), "verbose")
 		c := orchestrator.Orchestrator.GetReadOnlyCache(*o)
 		keysToPrint, err := getRelevantKeys(o, cacheKey, w)
 		if err == nil {
-			printCacheEntries(keysToPrint, c, w)
+			printCacheEntries(keysToPrint, c, w, isVerbose)
 		}
 	}
 }
@@ -215,7 +216,8 @@ func clearCacheHandler(o *orchestrator.Orchestrator) http.HandlerFunc {
 type marshallableResource struct {
 	Key            string
 	Resp           *marshalledDiscoveryResponse
-	Requests       []types.Resource
+	Requests       []types.Resource `json:",omitempty"`
+	NumRequests    int              `json:",omitempty"`
 	ExpirationTime time.Time
 }
 
@@ -252,12 +254,12 @@ func getRelevantKeys(o *orchestrator.Orchestrator, key string, w http.ResponseWr
 	return relevantKeys, nil
 }
 
-func printCacheEntries(keys []string, cache cache.ReadOnlyCache, w http.ResponseWriter) {
+func printCacheEntries(keys []string, cache cache.ReadOnlyCache, w http.ResponseWriter, isVerbose bool) {
 	resp := marshallableCache{}
 	for _, key := range keys {
 		resource, err := cache.FetchReadOnly(key)
 		if err == nil {
-			resp.Cache = append(resp.Cache, resourceToPayload(key, resource)...)
+			resp.Cache = append(resp.Cache, resourceToPayload(key, resource, isVerbose)...)
 		}
 	}
 	resourceString, err := stringify.InterfaceToString(resp)
@@ -295,7 +297,7 @@ func hasWildcardSuffix(key string) bool {
 // In order to marshal a Resource from the cache to JSON to be printed,
 // the map of requests is converted to a slice of just the keys,
 // since the bool value is meaningless.
-func resourceToPayload(key string, resource cache.Resource) []marshallableResource {
+func resourceToPayload(key string, resource cache.Resource, isVerbose bool) []marshallableResource {
 	var marshallableResources []marshallableResource
 	var requests []types.Resource
 	resource.Requests.ForEach(func(request transport.Request) {
@@ -305,13 +307,17 @@ func resourceToPayload(key string, resource cache.Resource) []marshallableResour
 			requests = append(requests, request.GetRaw().V3)
 		}
 	})
-
-	marshallableResources = append(marshallableResources, marshallableResource{
+	marshallableResource := marshallableResource{
 		Key:            key,
 		Resp:           marshalDiscoveryResponse(resource.Resp),
-		Requests:       requests,
+		NumRequests:    len(requests),
 		ExpirationTime: resource.ExpirationTime,
-	})
+	}
+
+	if isVerbose {
+		marshallableResource.Requests = requests
+	}
+	marshallableResources = append(marshallableResources, marshallableResource)
 
 	return marshallableResources
 }

diff --git a/internal/app/admin/http/cache_test.go b/internal/app/admin/http/cache_test.go
@@ -388,7 +388,7 @@ func TestAdminServer_CacheDumpHandler_NotFound(t *testing.T) {
 	assert.Equal(t, "", rr.Body.String())
 }
 
-func testAdminServerCacheDumpHelper(t *testing.T, urls []string) {
+func testAdminServerCacheDumpHelper(t *testing.T, isVerbose bool, urls []string) {
 	for _, url := range urls {
 		t.Run(url, func(t *testing.T) {
 			ctx := context.Background()
@@ -477,19 +477,19 @@ func testAdminServerCacheDumpHelper(t *testing.T, urls []string) {
 			handler.ServeHTTP(rr, req)
 			assert.Equal(t, http.StatusOK, rr.Code)
 
-			verifyCacheOutput(t, rr, "testdata/entire_cachev2_cds.json", "testdata/entire_cachev2_lds.json")
+			verifyCacheOutput(t, rr, isVerbose, "testdata/entire_cachev2_cds.json", "testdata/entire_cachev2_lds.json")
 			cancelLDSWatch()
 			cancelCDSWatch()
 		})
 	}
 }
 
 func TestAdminServer_CacheDumpHandler_EntireCache(t *testing.T) {
-	testAdminServerCacheDumpHelper(t, []string{cacheURL, cacheURL + "/", cacheURL + "/*"})
+	testAdminServerCacheDumpHelper(t, false, []string{cacheURL, cacheURL + "/", cacheURL + "/*"})
 }
 
 func TestAdminServer_CacheDumpHandler_WildcardSuffix(t *testing.T) {
-	testAdminServerCacheDumpHelper(t, []string{cacheURL + "/t*", cacheURL + "/tes*", cacheURL + "/test*"})
+	testAdminServerCacheDumpHelper(t, false, []string{cacheURL + "/t*", cacheURL + "/tes*", cacheURL + "/test*"})
 }
 
 func TestAdminServer_CacheDumpHandler_WildcardSuffix_NotFound(t *testing.T) {
@@ -591,7 +591,7 @@ func TestAdminServer_CacheDumpHandler_WildcardSuffix_NotFound(t *testing.T) {
 }
 
 // V3 Cache Dump Handler tests
-func testAdminServerCacheDumpHandlerV3(t *testing.T, urls []string) {
+func testAdminServerCacheDumpHandlerV3(t *testing.T, isVerbose bool, urls []string) {
 	for _, url := range urls {
 		t.Run(url, func(t *testing.T) {
 			ctx := context.Background()
@@ -679,7 +679,7 @@ func testAdminServerCacheDumpHandlerV3(t *testing.T, urls []string) {
 
 			handler.ServeHTTP(rr, req)
 			assert.Equal(t, http.StatusOK, rr.Code)
-			verifyCacheOutput(t, rr, "testdata/entire_cachev3_cds.json", "testdata/entire_cachev3_lds.json")
+			verifyCacheOutput(t, rr, isVerbose, "testdata/entire_cachev3_cds.json", "testdata/entire_cachev3_lds.json")
 
 			cancelLDSWatch()
 			cancelCDSWatch()
@@ -688,11 +688,19 @@ func testAdminServerCacheDumpHandlerV3(t *testing.T, urls []string) {
 }
 
 func TestAdminServer_CacheDumpHandler_EntireCacheV3(t *testing.T) {
-	testAdminServerCacheDumpHandlerV3(t, []string{cacheURL, cacheURL + "/", cacheURL + "/*"})
+	testAdminServerCacheDumpHandlerV3(t, false, []string{cacheURL, cacheURL + "/", cacheURL + "/*"})
 }
 
 func TestAdminServer_CacheDumpHandler_WildcardSuffixV3(t *testing.T) {
-	testAdminServerCacheDumpHandlerV3(t, []string{cacheURL + "/t*", cacheURL + "/tes*", cacheURL + "/test*"})
+	testAdminServerCacheDumpHandlerV3(t, false, []string{cacheURL + "/t*", cacheURL + "/tes*", cacheURL + "/test*"})
+}
+
+func TestAdminServer_CacheDumpHandler_VerboseCache(t *testing.T) {
+	testAdminServerCacheDumpHelper(t, true, []string{cacheURL + "/test*?verbose=true"})
+}
+
+func TestAdminServer_CacheDumpHandler_SuccinctCache(t *testing.T) {
+	testAdminServerCacheDumpHelper(t, false, []string{cacheURL + "/*?verbose=false"})
 }
 
 // Clear Cache Handler tests
@@ -1028,7 +1036,7 @@ func TestAdminServer_ClearCacheHandler_WildcardSuffixV3(t *testing.T) {
 	testAdminServerClearCacheHelperV3(t, []string{clearURL + "/t*", clearURL + "/tes*", clearURL + "/test*"})
 }
 
-func verifyCacheOutput(t *testing.T, rr *httptest.ResponseRecorder, cdsFile string, ldsFile string) {
+func verifyCacheOutput(t *testing.T, rr *httptest.ResponseRecorder, isVerbose bool, cdsFile string, ldsFile string) {
 	var actualResponse map[string]interface{}
 	err := json.Unmarshal(rr.Body.Bytes(), &actualResponse)
 	assert.NoError(t, err)
@@ -1062,8 +1070,15 @@ func verifyCacheOutput(t *testing.T, rr *httptest.ResponseRecorder, cdsFile stri
 	assert.Equal(t, expectedCdsResponse["Key"], actualCdsResponse["Key"])
 	assert.Equal(t, expectedLdsResponse["Resp"], actualLdsResponse["Resp"])
 	assert.Equal(t, expectedCdsResponse["Resp"], actualCdsResponse["Resp"])
-	assert.Equal(t, len(actualLdsResponse["Requests"].([]interface{})), 1)
-	assert.Equal(t, len(actualCdsResponse["Requests"].([]interface{})), 1)
+	assert.Equal(t, actualLdsResponse["NumRequests"], float64(1))
+	assert.Equal(t, actualCdsResponse["NumRequests"], float64(1))
+	if isVerbose {
+		assert.Equal(t, len(actualLdsResponse["Requests"].([]interface{})), 1)
+		assert.Equal(t, len(actualCdsResponse["Requests"].([]interface{})), 1)
+	} else {
+		assert.NotContains(t, actualLdsResponse, "Requests")
+		assert.NotContains(t, actualCdsResponse, "Requests")
+	}
 	assert.NotNil(t, actualLdsResponse["ExpirationTime"])
 	assert.NotNil(t, actualCdsResponse["ExpirationTime"])
 }

diff --git a/internal/app/admin/http/handler.go b/internal/app/admin/http/handler.go
@@ -4,7 +4,9 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/pprof"
+	"net/url"
 	"path/filepath"
+	"strconv"
 	"strings"
 
 	"github.com/envoyproxy/xds-relay/internal/pkg/log"
@@ -165,6 +167,11 @@ func getParam(path string, prefix string) string {
 	return param
 }
 
+func getBoolQueryValue(values url.Values, key string) (bool, error) {
+	val := values.Get(key)
+	return strconv.ParseBool(val)
+}
+
 func logLevelHandler(l log.Logger) http.HandlerFunc {
 	return func(w http.ResponseWriter, req *http.Request) {
 		if req.Method == "POST" {

diff --git a/internal/app/admin/http/handler_test.go b/internal/app/admin/http/handler_test.go
@@ -3,6 +3,10 @@ package handler
 import (
 	"bytes"
 	"context"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
 
 	v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2"
 	gcp "github.com/envoyproxy/go-control-plane/pkg/cache/v2"
@@ -12,10 +16,6 @@ import (
 	"github.com/golang/protobuf/ptypes"
 	"github.com/golang/protobuf/ptypes/any"
 	"github.com/stretchr/testify/assert"
-
-	"net/http"
-	"net/http/httptest"
-	"testing"
 )
 
 func TestAdminServer_DefaultHandler(t *testing.T) {
@@ -100,6 +100,29 @@ func TestGetParam_Empty(t *testing.T) {
 	assert.Equal(t, "", cacheKey)
 }
 
+func TestGetBoolQuery(t *testing.T) {
+	isVerbose, err := getBoolQueryValue(url.Values{
+		"verbose": []string{"true"},
+	}, "verbose")
+	assert.True(t, isVerbose)
+	assert.NoError(t, err)
+}
+
+func TestGetBoolQuery_Empty(t *testing.T) {
+	queryValue, err := getBoolQueryValue(url.Values{}, "abc")
+	assert.False(t, queryValue)
+	assert.Error(t, err)
+}
+
+func TestGetBoolQuery_Malformed(t *testing.T) {
+	isVerbose, err := getBoolQueryValue(url.Values{
+		"verbose":   []string{"abc"},
+		"something": []string{"true"},
+	}, "verbose")
+	assert.False(t, isVerbose)
+	assert.Error(t, err)
+}
+
 func TestAdminServer_LogLevelHandler(t *testing.T) {
 	ctx := context.Background()
 	var buf bytes.Buffer

diff --git a/internal/app/admin/http/testdata/entire_cachev2_cds.json b/internal/app/admin/http/testdata/entire_cachev2_cds.json
@@ -22,6 +22,7 @@
         "Nonce": "",
         "ControlPlane": null
       },
+      "NumRequests": "1",
       "Requests": [
         {
           "version_info": "2",

diff --git a/internal/app/admin/http/testdata/entire_cachev2_lds.json b/internal/app/admin/http/testdata/entire_cachev2_lds.json
@@ -20,6 +20,7 @@
         "Nonce": "",
         "ControlPlane": null
       },
+      "NumRequests": "1",
       "Requests": [
         {
           "version_info": "1",

diff --git a/internal/app/admin/http/testdata/entire_cachev3_cds.json b/internal/app/admin/http/testdata/entire_cachev3_cds.json
@@ -22,6 +22,7 @@
         "Nonce": "",
         "ControlPlane": null
       },
+      "NumRequests": "1",
       "Requests": [
         {
           "version_info": "2",

diff --git a/internal/app/admin/http/testdata/entire_cachev3_lds.json b/internal/app/admin/http/testdata/entire_cachev3_lds.json
@@ -20,6 +20,7 @@
         "Nonce": "",
         "ControlPlane": null
       },
+      "NumRequests": "1",
       "Requests": [
         {
           "version_info": "1",