Merge pull request #1274 from opencost/mmd/remove-allocation-match-func

Replace FilterFunc core filters with AllocationFilter

pkg/costmodel/aggregation.go

@@ -2201,7 +2201,7 @@ func (a *Accesses) ComputeAllocationHandlerSummary(w http.ResponseWriter, r *htt
 
 	sasl := []*kubecost.SummaryAllocationSet{}
 	for _, as := range asr.Slice() {
-		sas := kubecost.NewSummaryAllocationSet(as, []kubecost.AllocationMatchFunc{}, []kubecost.AllocationMatchFunc{}, false, false)
+		sas := kubecost.NewSummaryAllocationSet(as, nil, []kubecost.AllocationMatchFunc{}, false, false)
 		sasl = append(sasl, sas)
 	}
 	sasr := kubecost.NewSummaryAllocationSetRange(sasl...)

pkg/kubecost/allocation.go

@@ -830,7 +830,7 @@ func NewAllocationSet(start, end time.Time, allocs ...*Allocation) *AllocationSe
 // simple flag for sharing idle resources.
 type AllocationAggregationOptions struct {
 	AllocationTotalsStore AllocationTotalsStore
-	FilterFuncs           []AllocationMatchFunc
+	Filter                AllocationFilter
 	IdleByNode            bool
 	LabelConfig           *LabelConfig
 	MergeUnallocated      bool
@@ -906,13 +906,19 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 		options.ShareIdle = ShareNone
 	}
 
+	// Pre-flatten the filter so we can just check == nil to see if there are
+	// filters.
+	if options.Filter != nil {
+		options.Filter = options.Filter.Flattened()
+	}
+
 	var allocatedTotalsMap map[string]map[string]float64
 
 	// If aggregateBy is nil, we don't aggregate anything. On the other hand,
 	// an empty slice implies that we should aggregate everything. See
 	// generateKey for why that makes sense.
 	shouldAggregate := aggregateBy != nil
-	shouldFilter := len(options.FilterFuncs) > 0
+	shouldFilter := options.Filter != nil
 	shouldShare := len(options.SharedHourlyCosts) > 0 || len(options.ShareFuncs) > 0
 	if !shouldAggregate && !shouldFilter && !shouldShare && options.ShareIdle == ShareNone {
 		// There is nothing for AggregateBy to do, so simply return nil
@@ -1063,7 +1069,7 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 	// Note that this can happen for any field, not just cluster, so we again
 	// need to track this on a per-cluster or per-node, per-allocation, per-resource basis.
 	var idleFiltrationCoefficients map[string]map[string]map[string]float64
-	if len(options.FilterFuncs) > 0 && options.ShareIdle == ShareNone {
+	if shouldFilter && options.ShareIdle == ShareNone {
 		idleFiltrationCoefficients, _, err = computeIdleCoeffs(options, as, shareSet)
 		if err != nil {
 			return fmt.Errorf("error computing idle filtration coefficients: %s", err)
@@ -1115,12 +1121,10 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 
 		skip := false
 
-		// (3) If any of the filter funcs fail, immediately skip the allocation.
-		for _, ff := range options.FilterFuncs {
-			if !ff(alloc) {
-				skip = true
-				break
-			}
+		// (3) If the allocation does not match the filter, immediately skip the
+		// allocation.
+		if options.Filter != nil {
+			skip = !options.Filter.Matches(alloc)
 		}
 		if skip {
 			// If we are tracking idle filtration coefficients, delete the
@@ -1305,11 +1309,8 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 	// aggregate if an exact match is found.
 	for _, alloc := range externalSet.allocations {
 		skip := false
-		for _, ff := range options.FilterFuncs {
-			if !ff(alloc) {
-				skip = true
-				break
-			}
+		if options.Filter != nil {
+			skip = !options.Filter.Matches(alloc)
 		}
 		if !skip {
 			key := alloc.generateKey(aggregateBy, options.LabelConfig)
@@ -1342,11 +1343,8 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 		for _, idleAlloc := range idleSet.allocations {
 			// if the idle does not apply to the non-filtered values, skip it
 			skip := false
-			for _, ff := range options.FilterFuncs {
-				if !ff(idleAlloc) {
-					skip = true
-					break
-				}
+			if options.Filter != nil {
+				skip = !options.Filter.Matches(idleAlloc)
 			}
 			if skip {
 				continue
@@ -1481,11 +1479,8 @@ func computeShareCoeffs(aggregateBy []string, options *AllocationAggregationOpti
 		// is removed. (Otherwise, all the shared cost will get redistributed
 		// over the unfiltered results, inflating their shared costs.)
 		filtered := false
-		for _, ff := range options.FilterFuncs {
-			if !ff(alloc) {
-				filtered = true
-				break
-			}
+		if options.Filter != nil {
+			filtered = !options.Filter.Matches(alloc)
 		}
 		if filtered {
 			name = "__filtered__"

pkg/kubecost/allocation_test.go

@@ -750,13 +750,6 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	}
 
 	// Filters
-	isCluster := func(matchCluster string) func(*Allocation) bool {
-		return func(a *Allocation) bool {
-			cluster := a.Properties.Cluster
-			return cluster == matchCluster
-		}
-	}
-
 	isNamespace := func(matchNamespace string) func(*Allocation) bool {
 		return func(a *Allocation) bool {
 			namespace := a.Properties.Namespace
@@ -1190,8 +1183,12 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 			start: start,
 			aggBy: []string{AllocationClusterProp},
 			aggOpts: &AllocationAggregationOptions{
-				FilterFuncs: []AllocationMatchFunc{isCluster("cluster1")},
-				ShareIdle:   ShareNone,
+				Filter: AllocationFilterCondition{
+					Field: FilterClusterID,
+					Op:    FilterEquals,
+					Value: "cluster1",
+				},
+				ShareIdle: ShareNone,
 			},
 			numResults: 1 + numIdle,
 			totalCost:  66.0,
@@ -1208,8 +1205,8 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 			start: start,
 			aggBy: []string{AllocationClusterProp},
 			aggOpts: &AllocationAggregationOptions{
-				FilterFuncs: []AllocationMatchFunc{isCluster("cluster1")},
-				ShareIdle:   ShareWeighted,
+				Filter:    AllocationFilterCondition{Field: FilterClusterID, Op: FilterEquals, Value: "cluster1"},
+				ShareIdle: ShareWeighted,
 			},
 			numResults: 1,
 			totalCost:  66.0,
@@ -1225,8 +1222,8 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 			start: start,
 			aggBy: []string{AllocationNamespaceProp},
 			aggOpts: &AllocationAggregationOptions{
-				FilterFuncs: []AllocationMatchFunc{isCluster("cluster1")},
-				ShareIdle:   ShareNone,
+				Filter:    AllocationFilterCondition{Field: FilterClusterID, Op: FilterEquals, Value: "cluster1"},
+				ShareIdle: ShareNone,
 			},
 			numResults: 2 + numIdle,
 			totalCost:  66.0,
@@ -1244,8 +1241,8 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 			start: start,
 			aggBy: []string{AllocationClusterProp},
 			aggOpts: &AllocationAggregationOptions{
-				FilterFuncs: []AllocationMatchFunc{isNamespace("namespace2")},
-				ShareIdle:   ShareNone,
+				Filter:    AllocationFilterCondition{Field: FilterNamespace, Op: FilterEquals, Value: "namespace2"},
+				ShareIdle: ShareNone,
 			},
 			numResults: numClusters + numIdle,
 			totalCost:  46.31,
@@ -1287,8 +1284,8 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 			start: start,
 			aggBy: []string{AllocationNamespaceProp},
 			aggOpts: &AllocationAggregationOptions{
-				FilterFuncs: []AllocationMatchFunc{isNamespace("namespace2")},
-				ShareIdle:   ShareWeighted,
+				Filter:    AllocationFilterCondition{Field: FilterNamespace, Op: FilterEquals, Value: "namespace2"},
+				ShareIdle: ShareWeighted,
 			},
 			numResults: 1,
 			totalCost:  46.31,
@@ -1312,7 +1309,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 			start: start,
 			aggBy: []string{AllocationNamespaceProp},
 			aggOpts: &AllocationAggregationOptions{
-				FilterFuncs:       []AllocationMatchFunc{isNamespace("namespace2")},
+				Filter:            AllocationFilterCondition{Field: FilterNamespace, Op: FilterEquals, Value: "namespace2"},
 				SharedHourlyCosts: map[string]float64{"total": sharedOverheadHourlyCost},
 				ShareSplit:        ShareWeighted,
 			},
@@ -1331,9 +1328,9 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 			start: start,
 			aggBy: []string{AllocationNamespaceProp},
 			aggOpts: &AllocationAggregationOptions{
-				FilterFuncs: []AllocationMatchFunc{isNamespace("namespace2")},
-				ShareFuncs:  []AllocationMatchFunc{isNamespace("namespace1")},
-				ShareSplit:  ShareWeighted,
+				Filter:     AllocationFilterCondition{Field: FilterNamespace, Op: FilterEquals, Value: "namespace2"},
+				ShareFuncs: []AllocationMatchFunc{isNamespace("namespace1")},
+				ShareSplit: ShareWeighted,
 			},
 			numResults: 1 + numIdle,
 			totalCost:  79.6667, // should be 74.7708, but I'm punting -- too difficult (NK)
@@ -1350,10 +1347,10 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 			start: start,
 			aggBy: []string{AllocationNamespaceProp},
 			aggOpts: &AllocationAggregationOptions{
-				FilterFuncs: []AllocationMatchFunc{isNamespace("namespace2")},
-				ShareFuncs:  []AllocationMatchFunc{isNamespace("namespace1")},
-				ShareSplit:  ShareWeighted,
-				ShareIdle:   ShareWeighted,
+				Filter:     AllocationFilterCondition{Field: FilterNamespace, Op: FilterEquals, Value: "namespace2"},
+				ShareFuncs: []AllocationMatchFunc{isNamespace("namespace1")},
+				ShareSplit: ShareWeighted,
+				ShareIdle:  ShareWeighted,
 			},
 			numResults: 1,
 			totalCost:  74.77083,
@@ -1456,10 +1453,10 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 			start: start,
 			aggBy: []string{AllocationNamespaceProp},
 			aggOpts: &AllocationAggregationOptions{
-				FilterFuncs: []AllocationMatchFunc{isNamespace("namespace2")},
-				ShareFuncs:  []AllocationMatchFunc{isNamespace("namespace1")},
-				ShareSplit:  ShareWeighted,
-				ShareIdle:   ShareWeighted,
+				Filter:     AllocationFilterCondition{Field: FilterNamespace, Op: FilterEquals, Value: "namespace2"},
+				ShareFuncs: []AllocationMatchFunc{isNamespace("namespace1")},
+				ShareSplit: ShareWeighted,
+				ShareIdle:  ShareWeighted,
 			},
 			numResults: 1,
 			totalCost:  74.77,
@@ -1502,7 +1499,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 			start: start,
 			aggBy: []string{AllocationNamespaceProp},
 			aggOpts: &AllocationAggregationOptions{
-				FilterFuncs:       []AllocationMatchFunc{isNamespace("namespace2")},
+				Filter:            AllocationFilterCondition{Field: FilterNamespace, Op: FilterEquals, Value: "namespace2"},
 				ShareSplit:        ShareWeighted,
 				ShareIdle:         ShareWeighted,
 				SharedHourlyCosts: map[string]float64{"total": sharedOverheadHourlyCost},
@@ -1568,9 +1565,9 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 			start: start,
 			aggBy: []string{AllocationNamespaceProp},
 			aggOpts: &AllocationAggregationOptions{
-				FilterFuncs: []AllocationMatchFunc{isNamespace("namespace2")},
-				ShareIdle:   ShareWeighted,
-				IdleByNode:  true,
+				Filter:     AllocationFilterCondition{Field: FilterNamespace, Op: FilterEquals, Value: "namespace2"},
+				ShareIdle:  ShareWeighted,
+				IdleByNode: true,
 			},
 			numResults: 1,
 			totalCost:  46.31,

pkg/kubecost/allocationfilter.go

@@ -417,3 +417,14 @@ func (or AllocationFilterOr) Matches(a *Allocation) bool {
 
 	return false
 }
+
+// AllocationFilterNone is a filter that matches no allocations. This is useful
+// for applications like authorization, where a user/group/role may be disallowed
+// from viewing Allocation data entirely.
+type AllocationFilterNone struct{}
+
+func (afn AllocationFilterNone) String() string { return "(none)" }
+
+func (afn AllocationFilterNone) Flattened() AllocationFilter { return afn }
+
+func (afn AllocationFilterNone) Matches(a *Allocation) bool { return false }

pkg/kubecost/allocationfilter_test.go

@@ -615,6 +615,121 @@ func Test_AllocationFilterCondition_Matches(t *testing.T) {
 	}
 }
 
+func Test_AllocationFilterNone_Matches(t *testing.T) {
+	cases := []struct {
+		name string
+		a    *Allocation
+	}{
+		{
+			name: "nil",
+			a:    nil,
+		},
+		{
+			name: "nil properties",
+			a: &Allocation{
+				Properties: nil,
+			},
+		},
+		{
+			name: "empty properties",
+			a: &Allocation{
+				Properties: &AllocationProperties{},
+			},
+		},
+		{
+			name: "ClusterID",
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Cluster: "cluster-one",
+				},
+			},
+		},
+		{
+			name: "Node",
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Node: "node123",
+				},
+			},
+		},
+		{
+			name: "Namespace",
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Namespace: "kube-system",
+				},
+			},
+		},
+		{
+			name: "ControllerKind",
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					ControllerKind: "deployment", // We generally store controller kinds as all lowercase
+				},
+			},
+		},
+		{
+			name: "ControllerName",
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Controller: "kc-cost-analyzer",
+				},
+			},
+		},
+		{
+			name: "Pod",
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Pod: "pod-123 UID-ABC",
+				},
+			},
+		},
+		{
+			name: "Container",
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Container: "cost-model",
+				},
+			},
+		},
+		{
+			name: `label`,
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Labels: map[string]string{
+						"app": "foo",
+					},
+				},
+			},
+		},
+		{
+			name: `annotation`,
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Annotations: map[string]string{
+						"prom_modified_name": "testing123",
+					},
+				},
+			},
+		},
+		{
+			name: `services`,
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Services: []string{"serv1", "serv2"},
+				},
+			},
+		},
+	}
+
+	for _, c := range cases {
+		result := AllocationFilterNone{}.Matches(c.a)
+
+		if result {
+			t.Errorf("%s: should have been rejected", c.name)
+		}
+	}
+}
 func Test_AllocationFilterAnd_Matches(t *testing.T) {
 	cases := []struct {
 		name   string
@@ -723,6 +838,21 @@ func Test_AllocationFilterAnd_Matches(t *testing.T) {
 			}},
 			expected: false,
 		},
+		{
+			name: `(and none) matches nothing`,
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Namespace: "kube-system",
+					Labels: map[string]string{
+						"app": "bar",
+					},
+				},
+			},
+			filter: AllocationFilterAnd{[]AllocationFilter{
+				AllocationFilterNone{},
+			}},
+			expected: false,
+		},
 	}
 
 	for _, c := range cases {
@@ -983,6 +1113,11 @@ func Test_AllocationFilter_Flattened(t *testing.T) {
 				},
 			}},
 		},
+		{
+			name:     "AllocationFilterNone",
+			input:    AllocationFilterNone{},
+			expected: AllocationFilterNone{},
+		},
 	}
 
 	for _, c := range cases {

pkg/kubecost/query.go

@@ -39,7 +39,7 @@ type AllocationQueryOptions struct {
 	AggregateBy             []string
 	Compute                 bool
 	DisableAggregatedStores bool
-	FilterFuncs             []AllocationMatchFunc
+	Filter                  AllocationFilter
 	IdleByNode              bool
 	IncludeExternal         bool
 	IncludeIdle             bool

pkg/kubecost/summaryallocation.go

@@ -301,15 +301,21 @@ type SummaryAllocationSet struct {
 // required for unfortunate reasons to do with performance and legacy order-of-
 // operations details, as well as the fact that reconciliation has been
 // pushed down to the conversion step between Allocation and SummaryAllocation.
-func NewSummaryAllocationSet(as *AllocationSet, ffs, kfs []AllocationMatchFunc, reconcile, reconcileNetwork bool) *SummaryAllocationSet {
+func NewSummaryAllocationSet(as *AllocationSet, filter AllocationFilter, kfs []AllocationMatchFunc, reconcile, reconcileNetwork bool) *SummaryAllocationSet {
 	if as == nil {
 		return nil
 	}
 
+	// Pre-flatten the filter so we can just check == nil to see if there are
+	// filters.
+	if filter != nil {
+		filter = filter.Flattened()
+	}
+
 	// If we can know the exact size of the map, use it. If filters or sharing
 	// functions are present, we can't know the size, so we make a default map.
 	var sasMap map[string]*SummaryAllocation
-	if len(ffs) == 0 && len(kfs) == 0 {
+	if filter == nil && len(kfs) == 0 {
 		// No filters, so make the map of summary allocations exactly the size
 		// of the origin allocation set.
 		sasMap = make(map[string]*SummaryAllocation, len(as.allocations))
@@ -342,16 +348,8 @@ func NewSummaryAllocationSet(as *AllocationSet, ffs, kfs []AllocationMatchFunc,
 
 		// If the allocation does not pass any of the given filter functions,
 		// do not insert it into the set.
-		shouldFilter := false
-		for _, ff := range ffs {
-			if !ff(alloc) {
-				shouldFilter = true
-				break
-			}
-		}
-		if shouldFilter {
+		if filter != nil && !filter.Matches(alloc) {
 			continue
-
 		}
 
 		err := sas.Insert(NewSummaryAllocation(alloc, reconcile, reconcileNetwork))
@@ -475,6 +473,12 @@ func (sas *SummaryAllocationSet) AggregateBy(aggregateBy []string, options *Allo
 		options.LabelConfig = NewLabelConfig()
 	}
 
+	// Pre-flatten the filter so we can just check == nil to see if there are
+	// filters.
+	if options.Filter != nil {
+		options.Filter = options.Filter.Flattened()
+	}
+
 	// Check if we have any work to do; if not, then early return. If
 	// aggregateBy is nil, we don't aggregate anything. On the other hand,
 	// an empty slice implies that we should aggregate everything. (See
@@ -672,7 +676,7 @@ func (sas *SummaryAllocationSet) AggregateBy(aggregateBy []string, options *Allo
 	// recorded by idle-key (cluster or node, depending on the IdleByNode
 	// option). Instantiating this map is a signal to record the totals.
 	var allocTotalsAfterFilters map[string]*AllocationTotals
-	if len(resultSet.idleKeys) > 0 && len(options.FilterFuncs) > 0 {
+	if len(resultSet.idleKeys) > 0 && options.Filter != nil {
 		allocTotalsAfterFilters = make(map[string]*AllocationTotals, len(resultSet.idleKeys))
 	}
 
@@ -961,11 +965,9 @@ func (sas *SummaryAllocationSet) AggregateBy(aggregateBy []string, options *Allo
 		// be filtered or not.
 		// TODO:CLEANUP do something about external cost, this stinks
 		ea := &Allocation{Properties: sa.Properties}
-		for _, ff := range options.FilterFuncs {
-			if !ff(ea) {
-				skip = true
-				break
-			}
+
+		if options.Filter != nil {
+			skip = !options.Filter.Matches(ea)
 		}
 
 		if !skip {
@@ -987,11 +989,8 @@ func (sas *SummaryAllocationSet) AggregateBy(aggregateBy []string, options *Allo
 		// be filtered or not.
 		// TODO:CLEANUP do something about external cost, this stinks
 		ia := &Allocation{Properties: isa.Properties}
-		for _, ff := range options.FilterFuncs {
-			if !ff(ia) {
-				skip = true
-				break
-			}
+		if options.Filter != nil {
+			skip = !options.Filter.Matches(ia)
 		}
 		if skip {
 			continue