Kaynağa Gözat

Added special case for unallocated/unassigned aliased fields when filtering. Added additional test case for filtering into an unallocated aliased field. (#2115)

Signed-off-by: Ashleigh <ashleigh@kubecost.com>
Ashleigh 2 yıl önce
ebeveyn
işleme
dea90c0d60

+ 2 - 1
pkg/filter21/allocation/fields.go

@@ -25,7 +25,8 @@ const (
 // Filtering based on label aliases (team, department, etc.) should be a
 // responsibility of the query handler. By the time it reaches this
 // structured representation, we shouldn't have to be aware of what is
-// aliased to what.
+// aliased to what. The aliases correspond to either a label or annotation,
+// defined by the user.
 type AllocationAlias string
 
 const (

+ 39 - 2
pkg/kubecost/allocationfilter_test.go

@@ -440,11 +440,11 @@ func Test_AllocationFilterCondition_Matches(t *testing.T) {
 			expected: true,
 		},
 		{
-			name: `product != unallocated -> true`,
+			name: `department != unallocated -> true`,
 			a: &Allocation{
 				Properties: &AllocationProperties{
 					Annotations: AllocationAnnotations{
-						"keyproduct": "foo",
+						"keydepartment": "foo",
 					},
 				},
 			},
@@ -460,6 +460,43 @@ func Test_AllocationFilterCondition_Matches(t *testing.T) {
 			},
 			expected: true,
 		},
+		{
+			name: `product == unallocated -> true`,
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Annotations: AllocationAnnotations{
+						"keydepartment": "foo",
+					},
+				},
+			},
+			filter: &ast.EqualOp{
+				Left: ast.Identifier{
+					Field: ast.NewAliasField(afilter.AliasProduct),
+				},
+				Right: UnallocatedSuffix,
+			},
+			expected: true,
+		},
+		{
+			name: `product == "" -> true`,
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Labels: AllocationLabels{
+						"keydepartment": "foo",
+					},
+					Annotations: AllocationAnnotations{
+						"keyowner": "bar",
+					},
+				},
+			},
+			filter: &ast.EqualOp{
+				Left: ast.Identifier{
+					Field: ast.NewAliasField(afilter.AliasProduct),
+				},
+				Right: "",
+			},
+			expected: true,
+		},
 	}
 
 	for _, c := range cases {

+ 26 - 2
pkg/kubecost/allocationmatcher.go

@@ -244,7 +244,9 @@ func convertAliasFilterToLabelAnnotationFilter(aliasKey string, filterValue stri
 		return nil, fmt.Errorf("unsupported op type '%s' for alias conversion", op)
 	}
 
-	return ops.Or(
+	// This handles the case where a label EXISTS/IS PRESENT for (is extant)
+	// for an aliased field. That's the primary case.
+	extantCaseNode := ops.Or(
 		ops.And(
 			ops.Contains(afilter.FieldLabel, aliasKey),
 			labelOp,
@@ -256,5 +258,27 @@ func convertAliasFilterToLabelAnnotationFilter(aliasKey string, filterValue stri
 				annotationOp,
 			),
 		),
-	), nil
+	)
+	var node ast.FilterNode
+	// This handles the special case of unallocated aliased value. There's
+	// two forms of this; first is where the label/annotation exists, but
+	// has an empty string value. That's actually handled by the extant case,
+	// because the API passes through that empty string. The other is when
+	// the aliased label/annotation doesn't exist for an allocation. That's
+	// what this modification to the tree handles. This matters when you're
+	// trying to drill into/identify workloads "not allocated" within that
+	// specific aliased field.
+	if filterValue == "" || filterValue == UnallocatedSuffix {
+		node = ops.Or(
+			extantCaseNode,
+			ops.And(
+				ops.Not(ops.Contains(afilter.FieldLabel, aliasKey)),
+				ops.Not(ops.Contains(afilter.FieldAnnotation, aliasKey)),
+			),
+		)
+	} else {
+		node = extantCaseNode
+	}
+
+	return node, nil
 }