Merge branch 'develop' into mmd/update-default-pricing

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/allocationfilterutil/queryfilters.go

@@ -10,6 +10,51 @@ import (
 	"github.com/opencost/opencost/pkg/util/mapper"
 )
 
+const (
+	ParamFilterClusters        = "filterClusters"
+	ParamFilterNodes           = "filterNodes"
+	ParamFilterNamespaces      = "filterNamespaces"
+	ParamFilterControllerKinds = "filterControllerKinds"
+	ParamFilterControllers     = "filterControllers"
+	ParamFilterPods            = "filterPods"
+	ParamFilterContainers      = "filterContainers"
+
+	ParamFilterDepartments  = "filterDepartments"
+	ParamFilterEnvironments = "filterEnvironments"
+	ParamFilterOwners       = "filterOwners"
+	ParamFilterProducts     = "filterProducts"
+	ParamFilterTeams        = "filterTeams"
+
+	ParamFilterAnnotations = "filterAnnotations"
+	ParamFilterLabels      = "filterLabels"
+	ParamFilterServices    = "filterServices"
+)
+
+// AllHTTPParamKeys returns all HTTP GET parameters used for v1 filters. It is
+// intended to help validate HTTP queries in handlers to help avoid e.g.
+// spelling errors.
+func AllHTTPParamKeys() []string {
+	return []string{
+		ParamFilterClusters,
+		ParamFilterNodes,
+		ParamFilterNamespaces,
+		ParamFilterControllerKinds,
+		ParamFilterControllers,
+		ParamFilterPods,
+		ParamFilterContainers,
+
+		ParamFilterDepartments,
+		ParamFilterEnvironments,
+		ParamFilterOwners,
+		ParamFilterProducts,
+		ParamFilterTeams,
+
+		ParamFilterAnnotations,
+		ParamFilterLabels,
+		ParamFilterServices,
+	}
+}
+
 // ============================================================================
 // This file contains:
 // Parsing (HTTP query params -> AllocationFilter) for V1 of filters
@@ -83,7 +128,7 @@ func AllocationFilterFromParamsV1(
 	// filter structs (they evaluate to true always) there could be overhead
 	// when calling Matches() repeatedly for no purpose.
 
-	if filterClusters := qp.GetList("filterClusters", ","); len(filterClusters) > 0 {
+	if filterClusters := qp.GetList(ParamFilterClusters, ","); len(filterClusters) > 0 {
 		clustersOr := kubecost.AllocationFilterOr{
 			Filters: []kubecost.AllocationFilter{},
 		}
@@ -116,15 +161,15 @@ func AllocationFilterFromParamsV1(
 		filter.Filters = append(filter.Filters, clustersOr)
 	}
 
-	if raw := qp.GetList("filterNodes", ","); len(raw) > 0 {
+	if raw := qp.GetList(ParamFilterNodes, ","); len(raw) > 0 {
 		filter.Filters = append(filter.Filters, filterV1SingleValueFromList(raw, kubecost.FilterNode))
 	}
 
-	if raw := qp.GetList("filterNamespaces", ","); len(raw) > 0 {
+	if raw := qp.GetList(ParamFilterNamespaces, ","); len(raw) > 0 {
 		filter.Filters = append(filter.Filters, filterV1SingleValueFromList(raw, kubecost.FilterNamespace))
 	}
 
-	if raw := qp.GetList("filterControllerKinds", ","); len(raw) > 0 {
+	if raw := qp.GetList(ParamFilterControllerKinds, ","); len(raw) > 0 {
 		filter.Filters = append(filter.Filters, filterV1SingleValueFromList(raw, kubecost.FilterControllerKind))
 	}
 
@@ -132,7 +177,7 @@ func AllocationFilterFromParamsV1(
 	// "deployment:kubecost-cost-analyzer"
 	//
 	// Thus, we have to make a custom OR filter for this condition.
-	if filterControllers := qp.GetList("filterControllers", ","); len(filterControllers) > 0 {
+	if filterControllers := qp.GetList(ParamFilterControllers, ","); len(filterControllers) > 0 {
 		controllersOr := kubecost.AllocationFilterOr{
 			Filters: []kubecost.AllocationFilter{},
 		}
@@ -183,44 +228,44 @@ func AllocationFilterFromParamsV1(
 		}
 	}
 
-	if raw := qp.GetList("filterPods", ","); len(raw) > 0 {
+	if raw := qp.GetList(ParamFilterPods, ","); len(raw) > 0 {
 		filter.Filters = append(filter.Filters, filterV1SingleValueFromList(raw, kubecost.FilterPod))
 	}
 
-	if raw := qp.GetList("filterContainers", ","); len(raw) > 0 {
+	if raw := qp.GetList(ParamFilterContainers, ","); len(raw) > 0 {
 		filter.Filters = append(filter.Filters, filterV1SingleValueFromList(raw, kubecost.FilterContainer))
 	}
 
 	// Label-mapped queries require a label config to be present.
 	if labelConfig != nil {
-		if raw := qp.GetList("filterDepartments", ","); len(raw) > 0 {
+		if raw := qp.GetList(ParamFilterDepartments, ","); len(raw) > 0 {
 			filter.Filters = append(filter.Filters, filterV1LabelAliasMappedFromList(raw, labelConfig.DepartmentLabel))
 		}
-		if raw := qp.GetList("filterEnvironments", ","); len(raw) > 0 {
+		if raw := qp.GetList(ParamFilterEnvironments, ","); len(raw) > 0 {
 			filter.Filters = append(filter.Filters, filterV1LabelAliasMappedFromList(raw, labelConfig.EnvironmentLabel))
 		}
-		if raw := qp.GetList("filterOwners", ","); len(raw) > 0 {
+		if raw := qp.GetList(ParamFilterOwners, ","); len(raw) > 0 {
 			filter.Filters = append(filter.Filters, filterV1LabelAliasMappedFromList(raw, labelConfig.OwnerLabel))
 		}
-		if raw := qp.GetList("filterProducts", ","); len(raw) > 0 {
+		if raw := qp.GetList(ParamFilterProducts, ","); len(raw) > 0 {
 			filter.Filters = append(filter.Filters, filterV1LabelAliasMappedFromList(raw, labelConfig.ProductLabel))
 		}
-		if raw := qp.GetList("filterTeams", ","); len(raw) > 0 {
+		if raw := qp.GetList(ParamFilterTeams, ","); len(raw) > 0 {
 			filter.Filters = append(filter.Filters, filterV1LabelAliasMappedFromList(raw, labelConfig.TeamLabel))
 		}
 	} else {
 		log.Debugf("No label config is available. Not creating filters for label-mapped 'fields'.")
 	}
 
-	if raw := qp.GetList("filterAnnotations", ","); len(raw) > 0 {
+	if raw := qp.GetList(ParamFilterAnnotations, ","); len(raw) > 0 {
 		filter.Filters = append(filter.Filters, filterV1DoubleValueFromList(raw, kubecost.FilterAnnotation))
 	}
 
-	if raw := qp.GetList("filterLabels", ","); len(raw) > 0 {
+	if raw := qp.GetList(ParamFilterLabels, ","); len(raw) > 0 {
 		filter.Filters = append(filter.Filters, filterV1DoubleValueFromList(raw, kubecost.FilterLabel))
 	}
 
-	if filterServices := qp.GetList("filterServices", ","); len(filterServices) > 0 {
+	if filterServices := qp.GetList(ParamFilterServices, ","); len(filterServices) > 0 {
 		// filterServices= is the only filter that uses the "contains" operator.
 		servicesFilter := kubecost.AllocationFilterOr{
 			Filters: []kubecost.AllocationFilter{},

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 {