sanitize asset labels

Signed-off-by: Cliff Colvin <ccolvin@kubecost.com>

pkg/costmodel/cluster_helpers.go

@@ -636,13 +636,11 @@ func buildLabelsMap(
 			Name:    node,
 		}
 
-		m[key] = make(map[string]string)
-
-		for name, value := range result.Metric {
-			if val, ok := value.(string); ok {
-				m[key][name] = val
-			}
-		}
+		// The QueryResult.GetLabels function needs to be called to sanitize the
+		// ingested label data. This removes the label_ prefix that prometheus
+		// adds to emitted labels. It also keeps from ingesting prometheus labels
+		// that aren't a part of the asset.
+		m[key] = result.GetLabels()
 	}
 	return m
 }

pkg/costmodel/cluster_helpers_test.go

@@ -2,6 +2,7 @@ package costmodel
 
 import (
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 
@@ -1108,3 +1109,74 @@ func TestAssetCustompricing(t *testing.T) {
 	}
 
 }
+
+func TestBuildLabelsMap(t *testing.T) {
+	const (
+		labelKey1   = "testlabelkey1"
+		labelValue1 = "testlabel1-value"
+		labelKey2   = "test-label-key-2"
+		labelValue2 = "testlabel2.value"
+		nonLabelKey = "instance_type"
+		labelPrefix = "label_"
+	)
+
+	startTimestamp := float64(windowStart.Unix())
+
+	nodePromResult := []*prom.QueryResult{
+		{
+			Metric: map[string]interface{}{
+				"cluster_id":             "cluster1",
+				"node":                   "node1",
+				"instance_type":          "type1",
+				"provider_id":            "provider1",
+				"label_testlabelkey1":    "testlabel1-value",
+				"label_test-label-key-2": "testlabel2.value",
+			},
+			Values: []*util.Vector{
+				{
+					Timestamp: startTimestamp,
+					Value:     0.5,
+				},
+			},
+		},
+		{
+			Metric: map[string]interface{}{
+				"cluster_id":             "cluster1",
+				"node":                   "node2",
+				"instance_type":          "type1",
+				"provider_id":            "provider1",
+				"label_testlabelkey1":    "testlabel1-value",
+				"label_test-label-key-2": "testlabel2.value",
+			},
+			Values: []*util.Vector{
+				{
+					Timestamp: startTimestamp,
+					Value:     0.5,
+				},
+			},
+		},
+	}
+
+	nodeLabelMap := buildLabelsMap(nodePromResult)
+	// Test that for all nodes and all label keys in the map there isn't a key with the label_ prefix.
+	for _, labelMap := range nodeLabelMap {
+		for key, value := range labelMap {
+			if strings.HasPrefix(key, labelPrefix) {
+				t.Errorf("Asset label maps aren't sanitized. Expected no '%v' prefix in %v", labelPrefix, key)
+			}
+			// Test that the label value isn't touched
+			if key == labelKey1 && value != labelValue1 {
+				t.Errorf("Label Value didn't match. Got %v, but Expected: %v", value, labelValue1)
+			}
+			// Test that the label value isn't touched
+			if key == labelKey2 && value != labelValue2 {
+				t.Errorf("Label Value didn't match. Got %v, but Expected: %v", value, labelValue2)
+			}
+		}
+		// Test that keys that don't have the label_ prefix aren't in the resultant label map.
+		_, ok := labelMap[nonLabelKey]
+		if ok {
+			t.Errorf("Non-label keys are included in label mapping for asset labels. Expected '%v' to not exist'.", nonLabelKey)
+		}
+	}
+}

pkg/kubecost/asset.go

@@ -4,7 +4,6 @@ import (
 	"encoding"
 	"fmt"
 	"math"
-	"regexp"
 	"strings"
 	"time"
 
@@ -12,6 +11,7 @@ import (
 	"github.com/opencost/opencost/pkg/filter21/ast"
 	"github.com/opencost/opencost/pkg/filter21/matcher"
 	"github.com/opencost/opencost/pkg/log"
+	"github.com/opencost/opencost/pkg/prom"
 	"github.com/opencost/opencost/pkg/util/json"
 	"github.com/opencost/opencost/pkg/util/timeutil"
 )
@@ -4138,8 +4138,8 @@ func GetNodePoolName(provider string, labels map[string]string) string {
 }
 
 func getPoolNameHelper(label string, labels map[string]string) string {
-	sanitizedLabel := regexp.MustCompile(`[^a-zA-Z0-9 ]+`).ReplaceAllString(label, "_")
-	if poolName, found := labels[fmt.Sprintf("label_%s", sanitizedLabel)]; found {
+	sanitizedLabel := prom.SanitizeLabelName(label)
+	if poolName, found := labels[sanitizedLabel]; found {
 		return poolName
 	} else {
 		log.Warnf("unable to derive node pool name from node labels")