Use a more well defined spec to determine valid node names.

pkg/costmodel/containerkeys.go

@@ -2,8 +2,8 @@ package costmodel
 
 import (
 	"errors"
-	"net"
-	"strconv"
+	"fmt"
+	"regexp"
 	"strings"
 
 	"github.com/opencost/opencost/core/pkg/log"
@@ -25,13 +25,14 @@ var (
 	ErrNoNamespaceName error = errors.New("vector does not have string namespace")
 	ErrNoNodeName      error = errors.New("vector does not have string node")
 	ErrNoClusterID     error = errors.New("vector does not have string cluster id")
-	ErrIpPortNodeName  error = errors.New(`node name is an IP:PORT combo, not a node name. cAdvisor scrape configs require the following:
-	relabel_configs:
-	- action: labelmap
-	  regex: __meta_kubernetes_node_name
-	  replacement: node`)
 )
 
+const invalidNodeNameErrFmt = `invalid node name: %s was likely set from "instance" label. cAdvisor scrape configs require the following:
+relabel_configs:
+- action: labelmap
+  regex: __meta_kubernetes_node_name
+  replacement: node`
+
 //--------------------------------------------------------------------------
 //  KeyTuple
 //--------------------------------------------------------------------------
@@ -219,8 +220,8 @@ func NewContainerMetricFrom(result *source.ContainerMetricResult, defaultCluster
 	}
 
 	nodeName := result.Node
-	if isIpPortCombo(nodeName) {
-		return nil, ErrIpPortNodeName
+	if !isValidNodeName(nodeName) {
+		return nil, fmt.Errorf(invalidNodeNameErrFmt, nodeName)
 	}
 
 	if nodeName == "" {
@@ -243,24 +244,20 @@ func NewContainerMetricFrom(result *source.ContainerMetricResult, defaultCluster
 	}, nil
 }
 
-func isIpPortCombo(value string) bool {
-	// handle IPv6 values as well
-	idx := strings.LastIndex(value, ":")
-	if idx == -1 {
-		return false
-	}
-
-	ipStr, portStr := value[:idx], value[idx+1:]
-
-	ip := net.ParseIP(ipStr)
-	if ip == nil {
-		return false
-	}
-
-	port, err := strconv.Atoi(portStr)
-	if err != nil {
+/*
+- contain no more than 253 characters
+- contain only lowercase alphanumeric characters, '-' or '.'
+- start with an alphanumeric character
+- end with an alphanumeric character
+*/
+var nodeNameRegex = regexp.MustCompile(`^[a-z0-9][a-z0-9\-\.]+[a-z0-9]$`)
+
+// isValidNodeName determines if the nodeName provided is valid according to DNS subdomain
+// specifications: RFC 1123
+func isValidNodeName(nodeName string) bool {
+	if len(nodeName) > 253 {
 		return false
 	}
 
-	return port > 0 && port <= 65535
+	return nodeNameRegex.Match([]byte(nodeName))
 }

pkg/costmodel/costmodel_test.go

@@ -1,7 +1,9 @@
 package costmodel
 
 import (
+	"math/rand"
 	"testing"
+	"time"
 
 	"github.com/opencost/opencost/core/pkg/util"
 	"github.com/opencost/opencost/pkg/clustercache"
@@ -10,8 +12,29 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 )
 
-func TestIpPortCombo(t *testing.T) {
+func TestIsValidNodeName(t *testing.T) {
 	tests := []string{
+		"ip-10-1-2-3.ec2.internal",
+		"node-1",
+		"another.test.node",
+		"10-55.23-10",
+	}
+
+	for _, test := range tests {
+		if !isValidNodeName(test) {
+			t.Errorf("Expected %s to be a valid node name", test)
+		}
+	}
+
+	chars := "abcdefghijklmnopqrstuvwxyz"
+	longName := ""
+	r := rand.New(rand.NewSource(time.Now().UnixNano()))
+	for i := 0; i < 255; i++ {
+		longName += string(chars[r.Intn(len(chars))])
+	}
+
+	fails := []string{
+		longName,
 		"192.168.1.1:80",
 		"10.0.0.1:443",
 		"127.0.0.1:8080",
@@ -23,24 +46,16 @@ func TestIpPortCombo(t *testing.T) {
 		"fe80::1:22",
 		"10.1.2.3:10240",
 		":::80",
-	}
-
-	for _, test := range tests {
-		result := isIpPortCombo(test)
-		if !result {
-			t.Errorf("Expected %s to be a valid IP:Port combo", test)
-		}
-	}
-
-	fails := []string{
-		"foo:bar",
-		"ip-10-1-2-3.ec2.internal",
+		"node$-15",
+		"not:valid",
+		".hello-world",
+		"hello-world.",
+		"i--",
 	}
 
 	for _, fail := range fails {
-		result := isIpPortCombo(fail)
-		if result {
-			t.Errorf("Expected %s to be an invalid IP:Port combo", fail)
+		if isValidNodeName(fail) {
+			t.Errorf("Expected %s to be an invalid node name", fail)
 		}
 	}
 }