Merge pull request #1681 from opencost/mmd/extend-queryparams-validation

Add invalid parameter check support to QueryParams

pkg/env/env.go

@@ -14,6 +14,11 @@ import (
 // envMap contains Getter and Setter implementations for environment variables
 type envMap struct{}
 
+func (em *envMap) Has(key string) bool {
+	_, ok := os.LookupEnv(key)
+	return ok
+}
+
 // Get returns the value for the provided environment variable
 func (em *envMap) Get(key string) string {
 	return os.Getenv(key)

pkg/util/httputil/httputil.go

@@ -17,27 +17,75 @@ import (
 //  QueryParams
 //--------------------------------------------------------------------------
 
-type QueryParams = mapper.PrimitiveMap
+// valuesPrimitiveMap implements mapper.PrimitiveMap so we can build extra
+// functionality into the QueryParams interface.
+type valuesPrimitiveMap struct {
+	url.Values
+}
 
-// queryParamsMap is mapper.Map adapter for url.Values
-type queryParamsMap struct {
-	values url.Values
+func (values valuesPrimitiveMap) Has(key string) bool {
+	return values.Values.Has(key)
+}
+func (values valuesPrimitiveMap) Get(key string) string {
+	return values.Values.Get(key)
+}
+func (values valuesPrimitiveMap) Set(key, value string) error {
+	values.Values.Set(key, value)
+	return nil
 }
 
-// mapper.Getter implementation
-func (qpm *queryParamsMap) Get(key string) string {
-	return qpm.values.Get(key)
+// QueryParams provides basic map access to URL values as well as providing
+// helpful additional functionality for validation.
+type QueryParams interface {
+	mapper.PrimitiveMap
+
+	// InvalidKeys returns the set of param keys which are not present in the
+	// possible valid set. It is a set subtraction: present - valid = invalid
+	//
+	// Example usage to catch a typo:
+	// qp.InvalidKeys([]string{"window", "aggregate", "filterClusters"}) ->
+	//   "filterClsuters"
+	//
+	// If qp contains no keys, then this should always return an empty slice/nil
+	InvalidKeys(possibleValidKeys []string) (invalidKeys []string)
 }
 
-// mapper.Setter implementation
-func (qpm *queryParamsMap) Set(key, value string) error {
-	qpm.values.Set(key, value)
-	return nil
+// queryParamsMap implements the QueryParams interface on top of
+// valuesPrimitiveMap.
+type queryParamsMap struct {
+	values url.Values
+	mapper.PrimitiveMap
 }
 
 // NewQueryParams creates a primitive map using the request query parameters
 func NewQueryParams(values url.Values) QueryParams {
-	return mapper.NewMapper(&queryParamsMap{values})
+	vpm := valuesPrimitiveMap{values}
+
+	return &queryParamsMap{
+		values:       values,
+		PrimitiveMap: mapper.NewMapper(vpm),
+	}
+}
+
+// InvalidKeys performs a set difference: Params keys - possible valid keys.
+//
+// For now, dealing with cache busting parameters should be the handler's
+// responsibility.
+func (qpm *queryParamsMap) InvalidKeys(possibleValidKeys []string) []string {
+	validMap := map[string]struct{}{}
+	for _, validKey := range possibleValidKeys {
+		validMap[validKey] = struct{}{}
+	}
+
+	var invalidKeys []string
+
+	for key := range qpm.values {
+		if _, ok := validMap[key]; !ok {
+			invalidKeys = append(invalidKeys, key)
+		}
+	}
+
+	return invalidKeys
 }
 
 //--------------------------------------------------------------------------

pkg/util/httputil/httputil_test.go

@@ -2,9 +2,27 @@ package httputil
 
 import (
 	"net/http"
+	"net/url"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
 )
 
+func TestInvalidKeys(t *testing.T) {
+	vals := url.Values{}
+	vals.Set("window", "7d")
+	vals.Set("aggregate", "namespace")
+	vals.Set("filterClsuters", "cluster-two") // Intentional typo
+
+	qp := NewQueryParams(vals)
+
+	result := qp.InvalidKeys([]string{"window", "aggregate", "filterClusters", "filterNamespaces"})
+	expected := []string{"filterClsuters"}
+	if diff := cmp.Diff(result, expected); len(diff) > 0 {
+		t.Errorf("Expected: %+v. Got: %+v", expected, result)
+	}
+}
+
 func TestHeaderString(t *testing.T) {
 	h := make(http.Header)
 	h.Add("foo", "abc")

pkg/util/mapper/mapper.go

@@ -12,9 +12,11 @@ import (
 //  Contracts
 //--------------------------------------------------------------------------
 
-// Getter is an interface that retrieves a string value for a string key
+// Getter is an interface that retrieves a string value for a string key and
+// can check for the existence of a string key.
 type Getter interface {
 	Get(key string) string
+	Has(key string) bool
 }
 
 // Setter is an interface that sets the value of a string key to a string value.
@@ -32,6 +34,9 @@ type Map interface {
 // PrimitiveMapReader is an implementation contract for an object capable
 // of reading primitive values from a util.Map
 type PrimitiveMapReader interface {
+	// Has checks if the map contains the given key.
+	Has(key string) bool
+
 	// Get parses an string from the map key parameter. If the value
 	// is empty, the defaultValue parameter is returned.
 	Get(key string, defaultValue string) string
@@ -161,6 +166,12 @@ type GoMap struct {
 	m map[string]string
 }
 
+// Has implements mapper.Haser
+func (gm *GoMap) Has(key string) bool {
+	_, ok := gm.m[key]
+	return ok
+}
+
 // Get implements mapper.Getter
 func (gm *GoMap) Get(key string) string {
 	return gm.m[key]
@@ -236,6 +247,10 @@ func NewCompositionMapper(getter Getter, setter Setter) PrimitiveMap {
 	}
 }
 
+func (rom *readOnlyMapper) Has(key string) bool {
+	return rom.getter.Has(key)
+}
+
 // Get parses an string from the read-only mapper key parameter. If the value
 // is empty, the defaultValue parameter is returned.
 func (rom *readOnlyMapper) Get(key string, defaultValue string) string {