Merge branch 'develop' into atm/rm-cliff

Dockerfile

@@ -1,4 +1,4 @@
-FROM golang:latest as build-env
+FROM golang:1.24-alpine3.20 as build-env
 
 RUN mkdir /app
 WORKDIR /app

core/pkg/clusters/clusterinfo.go

@@ -31,6 +31,7 @@ type ClusterInfo struct {
 	Project     string `json:"project"`
 	Region      string `json:"region"`
 	Provisioner string `json:"provisioner"`
+	Version     string `json:"version"`
 }
 
 // Clone creates a copy of ClusterInfo and returns it
@@ -48,6 +49,7 @@ func (ci *ClusterInfo) Clone() *ClusterInfo {
 		Project:     ci.Project,
 		Region:      ci.Region,
 		Provisioner: ci.Provisioner,
+		Version:     ci.Version,
 	}
 }
 

core/pkg/clusters/util.go

@@ -25,6 +25,7 @@ func MapToClusterInfo(info map[string]string) (*ClusterInfo, error) {
 	var project string
 	var region string
 	var provisioner string
+	var version string
 
 	if cp, ok := info[ClusterInfoProfileKey]; ok {
 		clusterProfile = cp
@@ -50,6 +51,10 @@ func MapToClusterInfo(info map[string]string) (*ClusterInfo, error) {
 		provisioner = pvsr
 	}
 
+	if ver, ok := info[ClusterInfoVersionKey]; ok {
+		version = ver
+	}
+
 	return &ClusterInfo{
 		ID:          id,
 		Name:        name,
@@ -59,5 +64,6 @@ func MapToClusterInfo(info map[string]string) (*ClusterInfo, error) {
 		Project:     project,
 		Region:      region,
 		Provisioner: provisioner,
+		Version:     version,
 	}, nil
 }

core/pkg/clusters/util_test.go

@@ -0,0 +1,93 @@
+package clusters
+
+import "testing"
+
+const (
+	testClusterInfoIDKey      = "testClusterID"
+	testClusterInfoNameKey    = "testClusterName"
+	testClusterProfileKey     = "testProfile"
+	testClusterProviderKey    = "testProvider"
+	testClusterAccountKey     = "testAccount"
+	testClusterProjectKey     = "testProject"
+	testClusterRegionKey      = "testRegion"
+	testClusterProvisionerKey = "testProvisioner"
+	testClusterVersionKey     = "testVersion"
+)
+
+func TestMapToClusterInfo(t *testing.T) {
+	mapWOVersion := map[string]string{
+		ClusterInfoIdKey:          testClusterInfoIDKey,
+		ClusterInfoNameKey:        testClusterInfoNameKey,
+		ClusterInfoProfileKey:     testClusterProfileKey,
+		ClusterInfoProviderKey:    testClusterProviderKey,
+		ClusterInfoAccountKey:     testClusterAccountKey,
+		ClusterInfoProjectKey:     testClusterProjectKey,
+		ClusterInfoRegionKey:      testClusterRegionKey,
+		ClusterInfoProvisionerKey: testClusterProvisionerKey,
+	}
+	expectedCIwoVersion := ClusterInfo{
+		ID:          testClusterInfoIDKey,
+		Name:        testClusterInfoNameKey,
+		Profile:     testClusterProfileKey,
+		Provider:    testClusterProviderKey,
+		Account:     testClusterAccountKey,
+		Project:     testClusterProjectKey,
+		Region:      testClusterRegionKey,
+		Provisioner: testClusterProvisionerKey,
+	}
+	mapWVersion := map[string]string{
+		ClusterInfoIdKey:          testClusterInfoIDKey,
+		ClusterInfoNameKey:        testClusterInfoNameKey,
+		ClusterInfoProfileKey:     testClusterProfileKey,
+		ClusterInfoProviderKey:    testClusterProviderKey,
+		ClusterInfoAccountKey:     testClusterAccountKey,
+		ClusterInfoProjectKey:     testClusterProjectKey,
+		ClusterInfoRegionKey:      testClusterRegionKey,
+		ClusterInfoProvisionerKey: testClusterProvisionerKey,
+		ClusterInfoVersionKey:     testClusterVersionKey,
+	}
+	expectedCIwVersion := ClusterInfo{
+		ID:          testClusterInfoIDKey,
+		Name:        testClusterInfoNameKey,
+		Profile:     testClusterProfileKey,
+		Provider:    testClusterProviderKey,
+		Account:     testClusterAccountKey,
+		Project:     testClusterProjectKey,
+		Region:      testClusterRegionKey,
+		Provisioner: testClusterProvisionerKey,
+		Version:     testClusterVersionKey,
+	}
+	tests := []struct {
+		name     string
+		input    map[string]string
+		expected ClusterInfo
+		wantErr  bool
+	}{
+		{
+			name:     "when version is not in the cluster info map",
+			input:    mapWOVersion,
+			expected: expectedCIwoVersion,
+			wantErr:  false,
+		},
+		{
+			name:     "when version is in the cluster info map",
+			input:    mapWVersion,
+			expected: expectedCIwVersion,
+			wantErr:  false,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			returnCI, err := MapToClusterInfo(tc.input)
+			if (err != nil) != tc.wantErr {
+				t.Errorf("MapToClusterInfo() error = %v, wantErr %v", err, tc.wantErr)
+				return
+			}
+			if *returnCI != tc.expected {
+				t.Errorf("MapToClusterInfo() expected = %v, got %v", tc.expected, returnCI)
+				return
+			}
+		})
+	}
+}

core/pkg/nodestats/nodes_test.go

@@ -32,6 +32,7 @@ func TestNodeSummaryLive(t *testing.T) {
 	transport := &http.Transport{
 		TLSClientConfig: &tls.Config{
 			InsecureSkipVerify: true,
+			MinVersion:         tls.VersionTLS12,
 		},
 	}
 

core/pkg/opencost/allocationfilter_test.go

@@ -736,6 +736,89 @@ func Test_AllocationFilterAnd_Matches(t *testing.T) {
 	}
 }
 
+// Test_AllocationFilterAnd_OrderIndependence tests that AND filters work correctly
+// regardless of filter order and that all allocation properties are available for
+// matching before aggregation. This ensures filters like "namespace+cluster" produce
+// the same result as "cluster+namespace" and that filtering on properties like cluster
+// works even when results will later be aggregated by namespace.
+// Addresses: https://github.com/opencost/opencost/issues/3371
+func Test_AllocationFilterAnd_OrderIndependence(t *testing.T) {
+	cases := []struct {
+		name         string
+		a            *Allocation
+		filterString string
+		expected     bool
+	}{
+		{
+			name: "namespace matches, cluster does not -> should be false",
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Namespace: "ns1",
+					Cluster:   "cluster-one",
+				},
+			},
+			filterString: `namespace:"ns1"+cluster:"non-existent-uuid"`,
+			expected:     false,
+		},
+		{
+			name: "cluster first, namespace second (reversed order) -> should be false",
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Namespace: "ns1",
+					Cluster:   "cluster-one",
+				},
+			},
+			filterString: `cluster:"non-existent-uuid"+namespace:"ns1"`,
+			expected:     false,
+		},
+		{
+			name: "both namespace and cluster match -> should be true",
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Namespace: "ns1",
+					Cluster:   "cluster-one",
+				},
+			},
+			filterString: `namespace:"ns1"+cluster:"cluster-one"`,
+			expected:     true,
+		},
+		{
+			name: "namespace only filter -> should match",
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Namespace: "ns1",
+					Cluster:   "cluster-one",
+				},
+			},
+			filterString: `namespace:"ns1"`,
+			expected:     true,
+		},
+	}
+
+	parser := afilter.NewAllocationFilterParser()
+	compiler := NewAllocationMatchCompiler(nil)
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			filterNode, err := parser.Parse(c.filterString)
+			if err != nil {
+				t.Fatalf("Failed to parse filter '%s': %s", c.filterString, err)
+			}
+
+			matcher, err := compiler.Compile(filterNode)
+			if err != nil {
+				t.Fatalf("Failed to compile filter '%s': %s", c.filterString, err)
+			}
+
+			result := matcher.Matches(c.a)
+			if result != c.expected {
+				t.Errorf("Filter '%s' expected %t, got %t. AST: %s",
+					c.filterString, c.expected, result, ast.ToPreOrderShortString(filterNode))
+			}
+		})
+	}
+}
+
 func Test_AllocationFilterOr_Matches(t *testing.T) {
 	cases := []struct {
 		name   string

core/pkg/opencost/totals.go

@@ -209,6 +209,7 @@ type AssetTotals struct {
 	End                             time.Time `json:"end"`
 	Cluster                         string    `json:"cluster"`
 	Node                            string    `json:"node"`
+	ProviderID                      string    `json:"providerID,omitempty"`
 	Count                           int       `json:"count"`
 	AttachedVolumeCost              float64   `json:"attachedVolumeCost"`
 	AttachedVolumeCostAdjustment    float64   `json:"attachedVolumeCostAdjustment"`
@@ -245,6 +246,7 @@ func (art *AssetTotals) Clone() *AssetTotals {
 		End:                             art.End,
 		Cluster:                         art.Cluster,
 		Node:                            art.Node,
+		ProviderID:                      art.ProviderID,
 		Count:                           art.Count,
 		AttachedVolumeCost:              art.AttachedVolumeCost,
 		AttachedVolumeCostAdjustment:    art.AttachedVolumeCostAdjustment,
@@ -378,12 +380,18 @@ func ComputeAssetTotals(as *AssetSet, byAsset bool) map[string]*AssetTotals {
 		adjustedGPUCost := node.GPUCost * adjustmentRate
 		gpuCostAdjustment := adjustedGPUCost - node.GPUCost
 
+		var providerID string
+		if byAsset && node.Properties.ProviderID != "" {
+			providerID = node.Properties.ProviderID
+		}
+
 		if _, ok := arts[key]; !ok {
 			arts[key] = &AssetTotals{
-				Start:   node.Start,
-				End:     node.End,
-				Cluster: node.Properties.Cluster,
-				Node:    node.Properties.Name,
+				Start:      node.Start,
+				End:        node.End,
+				Cluster:    node.Properties.Cluster,
+				Node:       node.Properties.Name,
+				ProviderID: providerID,
 			}
 		}
 

core/pkg/opencost/totals_providerid_test.go

@@ -0,0 +1,229 @@
+package opencost
+
+import (
+	"testing"
+	"time"
+)
+
+// TestComputeAssetTotals_ProviderID_ByNode verifies that when computing
+// AssetTotals by node (byAsset=true), the ProviderID field is correctly
+// populated from the node's ProviderID.
+func TestComputeAssetTotals_ProviderID_ByNode(t *testing.T) {
+	// Create test window
+	start := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
+	end := time.Date(2024, 1, 1, 1, 0, 0, 0, time.UTC)
+	window := NewClosedWindow(start, end)
+
+	// Create AssetSet with nodes that have ProviderIDs
+	as := NewAssetSet(start, end)
+
+	// Node 1: AWS EC2 instance
+	node1 := NewNode("node1", "cluster1", "aws:///us-east-1a/i-0abc123def456789", start, end, window)
+	node1.CPUCost = 10.0
+	node1.RAMCost = 5.0
+
+	// Node 2: GCP instance
+	node2 := NewNode("node2", "cluster1", "gce://my-project/us-central1-a/instance-456", start, end, window)
+	node2.CPUCost = 8.0
+	node2.RAMCost = 4.0
+
+	// Node 3: Azure VM
+	node3 := NewNode("node3", "cluster1", "azure:///subscriptions/sub-id/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/vm-789", start, end, window)
+	node3.CPUCost = 12.0
+	node3.RAMCost = 6.0
+
+	as.Insert(node1, nil)
+	as.Insert(node2, nil)
+	as.Insert(node3, nil)
+
+	// Compute AssetTotals by node (byAsset=true)
+	totals := ComputeAssetTotals(as, true)
+
+	// Verify that each node's totals includes the correct ProviderID
+	expectedProviderIDs := map[string]string{
+		"cluster1/node1": "aws:///us-east-1a/i-0abc123def456789",
+		"cluster1/node2": "gce://my-project/us-central1-a/instance-456",
+		"cluster1/node3": "azure:///subscriptions/sub-id/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/vm-789",
+	}
+
+	for key, expectedProviderID := range expectedProviderIDs {
+		total, ok := totals[key]
+		if !ok {
+			t.Errorf("Expected to find totals for key %s", key)
+			continue
+		}
+
+		if total.ProviderID != expectedProviderID {
+			t.Errorf("For key %s: expected ProviderID %q, got %q", key, expectedProviderID, total.ProviderID)
+		}
+
+		// Verify Node field is also set correctly
+		expectedNodeName := key[len("cluster1/"):]
+		if total.Node != expectedNodeName {
+			t.Errorf("For key %s: expected Node %q, got %q", key, expectedNodeName, total.Node)
+		}
+	}
+}
+
+// TestComputeAssetTotals_ProviderID_ByCluster verifies that when computing
+// AssetTotals by cluster (byAsset=false), the ProviderID field is empty
+// because there's no single instance ID that represents all nodes.
+func TestComputeAssetTotals_ProviderID_ByCluster(t *testing.T) {
+	// Create test window
+	start := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
+	end := time.Date(2024, 1, 1, 1, 0, 0, 0, time.UTC)
+	window := NewClosedWindow(start, end)
+
+	// Create AssetSet with nodes that have ProviderIDs
+	as := NewAssetSet(start, end)
+
+	node1 := NewNode("node1", "cluster1", "aws:///us-east-1a/i-0abc123def456789", start, end, window)
+	node1.CPUCost = 10.0
+	node1.RAMCost = 5.0
+
+	node2 := NewNode("node2", "cluster1", "gce://my-project/us-central1-a/instance-456", start, end, window)
+	node2.CPUCost = 8.0
+	node2.RAMCost = 4.0
+
+	as.Insert(node1, nil)
+	as.Insert(node2, nil)
+
+	// Compute AssetTotals by cluster (byAsset=false)
+	totals := ComputeAssetTotals(as, false)
+
+	// Verify that cluster-level totals have an empty ProviderID
+	total, ok := totals["cluster1"]
+	if !ok {
+		t.Fatal("Expected to find totals for cluster1")
+	}
+
+	if total.ProviderID != "" {
+		t.Errorf("Expected empty ProviderID for cluster-level totals, got %q", total.ProviderID)
+	}
+
+	// Verify Node field is also empty at cluster level
+	if total.Node != "" {
+		t.Errorf("Expected empty Node for cluster-level totals, got %q", total.Node)
+	}
+
+	// Verify costs are aggregated correctly
+	expectedCPUCost := 18.0 // 10.0 + 8.0
+	expectedRAMCost := 9.0  // 5.0 + 4.0
+
+	if total.CPUCost != expectedCPUCost {
+		t.Errorf("Expected CPUCost %f, got %f", expectedCPUCost, total.CPUCost)
+	}
+
+	if total.RAMCost != expectedRAMCost {
+		t.Errorf("Expected RAMCost %f, got %f", expectedRAMCost, total.RAMCost)
+	}
+}
+
+// TestComputeAssetTotals_ProviderID_EmptyProviderID verifies that nodes
+// without a ProviderID still work correctly and result in an empty string
+// in the AssetTotals.
+func TestComputeAssetTotals_ProviderID_EmptyProviderID(t *testing.T) {
+	// Create test window
+	start := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
+	end := time.Date(2024, 1, 1, 1, 0, 0, 0, time.UTC)
+	window := NewClosedWindow(start, end)
+
+	// Create AssetSet with a node that has no ProviderID
+	as := NewAssetSet(start, end)
+
+	// Node without ProviderID (e.g., local/bare-metal cluster)
+	node := NewNode("node1", "cluster1", "", start, end, window)
+	node.CPUCost = 10.0
+	node.RAMCost = 5.0
+
+	as.Insert(node, nil)
+
+	// Compute AssetTotals by node (byAsset=true)
+	totals := ComputeAssetTotals(as, true)
+
+	// Verify that the node's totals has an empty ProviderID
+	total, ok := totals["cluster1/node1"]
+	if !ok {
+		t.Fatal("Expected to find totals for cluster1/node1")
+	}
+
+	if total.ProviderID != "" {
+		t.Errorf("Expected empty ProviderID for node without ProviderID, got %q", total.ProviderID)
+	}
+
+	// Verify other fields are still populated correctly
+	if total.Node != "node1" {
+		t.Errorf("Expected Node %q, got %q", "node1", total.Node)
+	}
+
+	if total.CPUCost != 10.0 {
+		t.Errorf("Expected CPUCost %f, got %f", 10.0, total.CPUCost)
+	}
+}
+
+// TestComputeAssetTotals_ProviderID_MultipleNodesAggregation tests that
+// when multiple nodes with different ProviderIDs are aggregated at the
+// cluster level, the ProviderID remains empty.
+func TestComputeAssetTotals_ProviderID_MultipleNodesAggregation(t *testing.T) {
+	// Create test window
+	start := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
+	end := time.Date(2024, 1, 1, 1, 0, 0, 0, time.UTC)
+	window := NewClosedWindow(start, end)
+
+	// Create AssetSet with multiple nodes across different clusters
+	as := NewAssetSet(start, end)
+
+	// Cluster 1 nodes
+	node1 := NewNode("node1", "cluster1", "aws:///us-east-1a/i-0abc123", start, end, window)
+	node1.CPUCost = 10.0
+	node1.RAMCost = 5.0
+
+	node2 := NewNode("node2", "cluster1", "aws:///us-east-1b/i-0def456", start, end, window)
+	node2.CPUCost = 8.0
+	node2.RAMCost = 4.0
+
+	// Cluster 2 node
+	node3 := NewNode("node3", "cluster2", "gce://project/zone/instance", start, end, window)
+	node3.CPUCost = 12.0
+	node3.RAMCost = 6.0
+
+	as.Insert(node1, nil)
+	as.Insert(node2, nil)
+	as.Insert(node3, nil)
+
+	// Compute AssetTotals by node (byAsset=true)
+	nodeTotal := ComputeAssetTotals(as, true)
+
+	// Verify each node has its own ProviderID
+	if nodeTotal["cluster1/node1"].ProviderID != "aws:///us-east-1a/i-0abc123" {
+		t.Errorf("Node1 ProviderID mismatch")
+	}
+	if nodeTotal["cluster1/node2"].ProviderID != "aws:///us-east-1b/i-0def456" {
+		t.Errorf("Node2 ProviderID mismatch")
+	}
+	if nodeTotal["cluster2/node3"].ProviderID != "gce://project/zone/instance" {
+		t.Errorf("Node3 ProviderID mismatch")
+	}
+
+	// Compute AssetTotals by cluster (byAsset=false)
+	clusterTotals := ComputeAssetTotals(as, false)
+
+	// Verify cluster-level totals have empty ProviderID
+	for clusterKey, total := range clusterTotals {
+		if total.ProviderID != "" {
+			t.Errorf("Cluster %s should have empty ProviderID, got %q", clusterKey, total.ProviderID)
+		}
+	}
+
+	// Verify cluster1 aggregates both nodes correctly
+	cluster1Total := clusterTotals["cluster1"]
+	expectedCPU := 18.0 // 10.0 + 8.0
+	expectedRAM := 9.0  // 5.0 + 4.0
+
+	if cluster1Total.CPUCost != expectedCPU {
+		t.Errorf("Cluster1 CPUCost: expected %f, got %f", expectedCPU, cluster1Total.CPUCost)
+	}
+	if cluster1Total.RAMCost != expectedRAM {
+		t.Errorf("Cluster1 RAMCost: expected %f, got %f", expectedRAM, cluster1Total.RAMCost)
+	}
+}

core/pkg/opencost/window.go

@@ -383,7 +383,7 @@ func (w Window) Contains(t time.Time) bool {
 		return false
 	}
 
-	if w.end != nil && !t.Before(*w.end) {
+	if w.end != nil && t.After(*w.end) {
 		return false
 	}
 

core/pkg/opencost/window_test.go

@@ -256,23 +256,23 @@ func TestParseWindowUTC(t *testing.T) {
 	if err != nil {
 		t.Fatalf(`unexpected error parsing "lastweek": %s`, err)
 	}
-	
+
 	// Verify lastweek window spans exactly 7 days
 	if lastweek.Duration().Hours() != 7*24 {
 		t.Fatalf(`expect: window "lastweek" to have duration 7 days; actual: %f hours`, lastweek.Duration().Hours())
 	}
-	
+
 	// Verify lastweek starts on a Sunday (weekday 0)
 	if lastweek.Start().Weekday() != time.Sunday {
 		t.Fatalf(`expect: window "lastweek" to start on Sunday; actual: %s starts on %s`, lastweek, lastweek.Start().Weekday())
 	}
-	
+
 	// Verify lastweek ends on a Saturday (should be 7 days after start)
 	expectedEnd := lastweek.Start().Add(7 * 24 * time.Hour)
 	if !lastweek.End().Equal(expectedEnd) {
 		t.Fatalf(`expect: window "lastweek" to end 7 days after start; actual: start %s, end %s`, lastweek.Start(), lastweek.End())
 	}
-	
+
 	// Verify lastweek ends before now
 	if !lastweek.End().Before(time.Now().UTC()) {
 		t.Fatalf(`expect: window "lastweek" to end before now; actual: %s ends after %s`, lastweek, time.Now().UTC())
@@ -844,7 +844,7 @@ func TestWindow_Contains(t *testing.T) {
 		{
 			window:   NewClosedWindow(t1, t2),
 			time:     t2,
-			expected: false, // Time at end of window (exclusive)
+			expected: true, // Time at end of window (inclusive)
 		},
 		{
 			window:   NewWindow(nil, &t2),
@@ -973,8 +973,8 @@ func TestWindow_ExpandEnd(t *testing.T) {
 	t3 := t1.Add(2 * time.Hour)
 
 	cases := []struct {
-		window Window
-		newEnd time.Time
+		window   Window
+		newEnd   time.Time
 		expected Window
 	}{
 		{

core/pkg/storage/http.go

@@ -75,7 +75,10 @@ func (config HTTPConfig) GetHTTPTransport() (http.RoundTripper, error) {
 
 // NewTLSConfig creates a new tls.Config from the given TLSConfig.
 func (cfg TLSConfig) ToConfig() (*tls.Config, error) {
-	tlsConfig := &tls.Config{InsecureSkipVerify: cfg.InsecureSkipVerify}
+	tlsConfig := &tls.Config{
+		InsecureSkipVerify: cfg.InsecureSkipVerify,
+		MinVersion:         tls.VersionTLS12,
+	}
 
 	// If a CA cert is provided then let's read it in.
 	if len(cfg.CAFile) > 0 {

core/pkg/storage/http_test.go

@@ -262,8 +262,10 @@ func TestTLSConfig_ToConfig(t *testing.T) {
 		validateFunc func(t *testing.T, tlsConfig *tls.Config)
 	}{
 		"default configuration": {
-			config:    &TLSConfig{},
-			want:      &tls.Config{},
+			config: &TLSConfig{},
+			want: &tls.Config{
+				MinVersion: tls.VersionTLS12,
+			},
 			wantError: false,
 		},
 		"with insecure skip verify": {
@@ -272,6 +274,7 @@ func TestTLSConfig_ToConfig(t *testing.T) {
 			},
 			want: &tls.Config{
 				InsecureSkipVerify: true,
+				MinVersion:         tls.VersionTLS12,
 			},
 			wantError: false,
 		},
@@ -293,6 +296,7 @@ func TestTLSConfig_ToConfig(t *testing.T) {
 			},
 			want: &tls.Config{
 				ServerName: "example.com",
+				MinVersion: tls.VersionTLS12,
 			},
 			wantError: false,
 		},
@@ -333,6 +337,7 @@ func TestTLSConfig_ToConfig(t *testing.T) {
 					CertFile: path.Join(tmpDir, caFileName),
 					KeyFile:  path.Join(tmpDir, keyFileName),
 				}.getClientCertificate,
+				MinVersion: tls.VersionTLS12,
 			},
 			wantError: false,
 		},

modules/prometheus-source/pkg/prom/prom.go

@@ -395,6 +395,7 @@ func NewPrometheusClient(address string, config *PrometheusClientConfig) (promet
 		TLSClientConfig: &tls.Config{
 			InsecureSkipVerify: config.TLSInsecureSkipVerify,
 			RootCAs:            config.RootCAs,
+			MinVersion:         tls.VersionTLS12,
 		},
 	})
 	pc := prometheus.Config{

pkg/cloud/aws/provider.go

@@ -1445,8 +1445,9 @@ func (aws *AWS) NodePricing(k models.Key) (*models.Node, models.PricingMetadata,
 		}
 		return aws.createNode(terms, usageType, k)
 	} else { // Fall back to base pricing if we can't find the key. Base pricing is handled at the costmodel level.
+		// we seem to have an issue where this error gets thrown during app start.
+		// somehow the ValidPricingKeys map is being accessed before all the pricing data has been downloaded
 		return nil, meta, fmt.Errorf("Invalid Pricing Key \"%s\"", key)
-
 	}
 }
 

pkg/cloud/provider/cloud_test.go

@@ -893,3 +893,49 @@ func TestNodePriceFromCSVByClass(t *testing.T) {
 	}
 
 }
+
+func TestPVPricing_CaseInsensitive(t *testing.T) {
+	confMan := config.NewConfigFileManager(storage.NewFileStorage("./"))
+	wantPrice := "0.1337"
+
+	c := &provider.CSVProvider{
+		CSVLocation: "../../../configs/pricing_schema_pv.csv",
+		PVMapField:  "metadata.name",
+		CustomProvider: &provider.CustomProvider{
+			Config: provider.NewProviderConfig(confMan, "../../../configs/default.json"),
+		},
+	}
+	c.DownloadPricingData()
+
+	t.Run("UppercaseInput", func(t *testing.T) {
+		pv := &clustercache.PersistentVolume{}
+		pv.Name = "PVC-08e1f205-d7a9-4430-90fc-7b3965a18c4D"
+
+		key := c.GetPVKey(pv, make(map[string]string), "")
+		resPV, err := c.PVPricing(key)
+		if err != nil {
+			t.Errorf("Error in PVPricing: %s", err.Error())
+		} else {
+			gotPrice := resPV.Cost
+			if gotPrice != wantPrice {
+				t.Errorf("Wanted price '%s' got price '%s'", wantPrice, gotPrice)
+			}
+		}
+	})
+
+	t.Run("LowercaseInput", func(t *testing.T) {
+		pv := &clustercache.PersistentVolume{}
+		pv.Name = "pvc-08e1f205-d7a9-4430-90fc-7b3965a18c4d"
+
+		key := c.GetPVKey(pv, make(map[string]string), "")
+		resPV, err := c.PVPricing(key)
+		if err != nil {
+			t.Errorf("Error in PVPricing: %s", err.Error())
+		} else {
+			gotPrice := resPV.Cost
+			if gotPrice != wantPrice {
+				t.Errorf("Wanted price '%s' got price '%s'", wantPrice, gotPrice)
+			}
+		}
+	})
+}

pkg/cloud/provider/csvprovider.go

@@ -423,7 +423,7 @@ func (c *CSVProvider) GetPVKey(pv *clustercache.PersistentVolume, parameters map
 	id := PVValueFromMapField(c.PVMapField, pv)
 	return &csvPVKey{
 		Labels:                 pv.Labels,
-		ProviderID:             id,
+		ProviderID:             strings.ToLower(id),
 		StorageClassName:       pv.Spec.StorageClassName,
 		StorageClassParameters: parameters,
 		Name:                   pv.Name,

pkg/costmodel/aggregation.go

@@ -227,7 +227,11 @@ func (a *Accesses) ComputeAllocationHandler(w http.ResponseWriter, r *http.Reque
 	// Get allocation filter if provided
 	allocationFilter := qp.Get("filter", "")
 
-	asr, err := a.Model.QueryAllocation(window, step, aggregateBy, includeIdle, idleByNode, includeProportionalAssetResourceCosts, includeAggregatedMetadata, sharedLoadBalancer, accumulateBy, shareIdle)
+	// Query allocations with filtering, aggregation, and accumulation.
+	// Filtering is done BEFORE aggregation inside QueryAllocation to ensure
+	// filters can match on all allocation properties (like cluster, node, etc.)
+	// before they are potentially lost or merged during aggregation.
+	asr, err := a.Model.QueryAllocation(window, step, aggregateBy, includeIdle, idleByNode, includeProportionalAssetResourceCosts, includeAggregatedMetadata, sharedLoadBalancer, accumulateBy, shareIdle, allocationFilter)
 	if err != nil {
 		if strings.Contains(strings.ToLower(err.Error()), "bad request") {
 			proto.WriteError(w, proto.BadRequest(err.Error()))
@@ -238,34 +242,5 @@ func (a *Accesses) ComputeAllocationHandler(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
-	// Apply allocation filter if provided
-	if allocationFilter != "" {
-		parser := allocation.NewAllocationFilterParser()
-		filterNode, err := parser.Parse(allocationFilter)
-		if err != nil {
-			proto.WriteError(w, proto.BadRequest(fmt.Sprintf("Invalid filter: %s", err)))
-			return
-		}
-		compiler := opencost.NewAllocationMatchCompiler(nil)
-		matcher, err := compiler.Compile(filterNode)
-		if err != nil {
-			proto.WriteError(w, proto.BadRequest(fmt.Sprintf("Failed to compile filter: %s", err)))
-			return
-		}
-		filteredASR := opencost.NewAllocationSetRange()
-		for _, as := range asr.Slice() {
-			filteredAS := opencost.NewAllocationSet(as.Start(), as.End())
-			for _, alloc := range as.Allocations {
-				if matcher.Matches(alloc) {
-					filteredAS.Set(alloc)
-				}
-			}
-			if filteredAS.Length() > 0 {
-				filteredASR.Append(filteredAS)
-			}
-		}
-		asr = filteredASR
-	}
-
 	WriteData(w, asr, nil)
 }

pkg/costmodel/costmodel.go

@@ -13,6 +13,7 @@ import (
 	"github.com/opencost/opencost/core/pkg/clustercache"
 	"github.com/opencost/opencost/core/pkg/clusters"
 	coreenv "github.com/opencost/opencost/core/pkg/env"
+	"github.com/opencost/opencost/core/pkg/filter/allocation"
 	"github.com/opencost/opencost/core/pkg/log"
 	"github.com/opencost/opencost/core/pkg/opencost"
 	"github.com/opencost/opencost/core/pkg/source"
@@ -886,7 +887,8 @@ func (cm *CostModel) GetNodeCost() (map[string]*costAnalyzerCloud.Node, error) {
 
 		cnode, _, err := cp.NodePricing(cp.GetKey(nodeLabels, n))
 		if err != nil {
-			log.Infof("Error getting node pricing. Error: %s", err.Error())
+			log.Infof("Could not get node pricing for node %s. Falling back to default pricing", name)
+			log.Debugf("Error getting node pricing: %s", err.Error())
 			if cnode != nil {
 				nodes[name] = cnode
 				continue
@@ -1553,7 +1555,7 @@ func measureTime(start time.Time, threshold time.Duration, name string) {
 	}
 }
 
-func (cm *CostModel) QueryAllocation(window opencost.Window, step time.Duration, aggregate []string, includeIdle, idleByNode, includeProportionalAssetResourceCosts, includeAggregatedMetadata, sharedLoadBalancer bool, accumulateBy opencost.AccumulateOption, shareIdle bool) (*opencost.AllocationSetRange, error) {
+func (cm *CostModel) QueryAllocation(window opencost.Window, step time.Duration, aggregate []string, includeIdle, idleByNode, includeProportionalAssetResourceCosts, includeAggregatedMetadata, sharedLoadBalancer bool, accumulateBy opencost.AccumulateOption, shareIdle bool, filterString string) (*opencost.AllocationSetRange, error) {
 	// Validate window is legal
 	if window.IsOpen() || window.IsNegative() {
 		return nil, fmt.Errorf("illegal window: %s", window)
@@ -1607,7 +1609,7 @@ func (cm *CostModel) QueryAllocation(window opencost.Window, step time.Duration,
 				}
 			}
 
-			idleSet, err := computeIdleAllocations(allocSet, assetSet, true)
+			idleSet, err := computeIdleAllocations(allocSet, assetSet, idleByNode)
 			if err != nil {
 				return nil, fmt.Errorf("error computing idle allocations for %s: %w", opencost.NewClosedWindow(stepStart, stepEnd), err)
 			}
@@ -1623,6 +1625,33 @@ func (cm *CostModel) QueryAllocation(window opencost.Window, step time.Duration,
 		stepEnd = stepStart.Add(step)
 	}
 
+	// Apply allocation filter BEFORE aggregation if provided
+	if filterString != "" {
+		parser := allocation.NewAllocationFilterParser()
+		filterNode, err := parser.Parse(filterString)
+		if err != nil {
+			return nil, fmt.Errorf("invalid filter: %w", err)
+		}
+		compiler := opencost.NewAllocationMatchCompiler(nil)
+		matcher, err := compiler.Compile(filterNode)
+		if err != nil {
+			return nil, fmt.Errorf("failed to compile filter: %w", err)
+		}
+		filteredASR := opencost.NewAllocationSetRange()
+		for _, as := range asr.Slice() {
+			filteredAS := opencost.NewAllocationSet(as.Start(), as.End())
+			for _, alloc := range as.Allocations {
+				if matcher.Matches(alloc) {
+					filteredAS.Set(alloc)
+				}
+			}
+			if filteredAS.Length() > 0 {
+				filteredASR.Append(filteredAS)
+			}
+		}
+		asr = filteredASR
+	}
+
 	// Set aggregation options and aggregate
 	var shareIdleOpt string
 	if shareIdle {
@@ -1795,7 +1824,7 @@ func computeIdleAllocations(allocSet *opencost.AllocationSet, assetSet *opencost
 			Properties: &opencost.AllocationProperties{
 				Cluster:    assetTotal.Cluster,
 				Node:       assetTotal.Node,
-				ProviderID: assetTotal.Node,
+				ProviderID: assetTotal.ProviderID,
 			},
 			Start:   assetTotal.Start,
 			End:     assetTotal.End,

pkg/costmodel/idle_providerid_test.go

@@ -0,0 +1,346 @@
+package costmodel
+
+import (
+	"testing"
+	"time"
+
+	"github.com/opencost/opencost/core/pkg/opencost"
+)
+
+// TestComputeIdleAllocations_ProviderID_ByNode verifies that when computing
+// idle allocations by node, each idle allocation correctly contains the
+// cloud provider instance ID from AssetTotals.ProviderID, not the node name.
+func TestComputeIdleAllocations_ProviderID_ByNode(t *testing.T) {
+	// Create test window
+	start := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
+	end := time.Date(2024, 1, 1, 1, 0, 0, 0, time.UTC)
+	window := opencost.NewClosedWindow(start, end)
+
+	// Create AssetSet with nodes that have ProviderIDs
+	assetSet := opencost.NewAssetSet(start, end)
+
+	node1 := opencost.NewNode("ip-10-2-1-100.ec2.internal", "cluster1", "aws:///us-east-1a/i-0abc123def456789", start, end, window)
+	node1.CPUCost = 10.0
+	node1.RAMCost = 5.0
+	node1.GPUCost = 0.0
+
+	node2 := opencost.NewNode("gke-cluster-default-pool-node-456", "cluster1", "gce://my-project/us-central1-a/instance-456", start, end, window)
+	node2.CPUCost = 8.0
+	node2.RAMCost = 4.0
+	node2.GPUCost = 0.0
+
+	assetSet.Insert(node1, nil)
+	assetSet.Insert(node2, nil)
+
+	// Create AllocationSet with some utilization (not 100% to ensure idle exists)
+	allocSet := opencost.NewAllocationSet(start, end)
+
+	// Create allocations that use 50% of each node's resources
+	alloc1 := &opencost.Allocation{
+		Name:    "namespace1/pod1/container1",
+		Start:   start,
+		End:     end,
+		Window:  window.Clone(),
+		CPUCost: 5.0, // 50% of node1's CPU
+		RAMCost: 2.5, // 50% of node1's RAM
+		GPUCost: 0.0,
+		Properties: &opencost.AllocationProperties{
+			Cluster: "cluster1",
+			Node:    "ip-10-2-1-100.ec2.internal",
+		},
+	}
+
+	alloc2 := &opencost.Allocation{
+		Name:    "namespace2/pod2/container2",
+		Start:   start,
+		End:     end,
+		Window:  window.Clone(),
+		CPUCost: 4.0, // 50% of node2's CPU
+		RAMCost: 2.0, // 50% of node2's RAM
+		GPUCost: 0.0,
+		Properties: &opencost.AllocationProperties{
+			Cluster: "cluster1",
+			Node:    "gke-cluster-default-pool-node-456",
+		},
+	}
+
+	allocSet.Insert(alloc1)
+	allocSet.Insert(alloc2)
+
+	// Compute idle allocations by node (idleByNode=true)
+	idleSet, err := computeIdleAllocations(allocSet, assetSet, true)
+	if err != nil {
+		t.Fatalf("Error computing idle allocations: %v", err)
+	}
+
+	// Expected idle allocations with ProviderIDs
+	expectedIdles := map[string]struct {
+		providerID string
+		nodeName   string
+	}{
+		"cluster1/ip-10-2-1-100.ec2.internal/__idle__": {
+			providerID: "aws:///us-east-1a/i-0abc123def456789",
+			nodeName:   "ip-10-2-1-100.ec2.internal",
+		},
+		"cluster1/gke-cluster-default-pool-node-456/__idle__": {
+			providerID: "gce://my-project/us-central1-a/instance-456",
+			nodeName:   "gke-cluster-default-pool-node-456",
+		},
+	}
+
+	// Verify each idle allocation has the correct ProviderID
+	foundCount := 0
+	for _, alloc := range idleSet.Allocations {
+		if !alloc.IsIdle() {
+			continue
+		}
+
+		expected, ok := expectedIdles[alloc.Name]
+		if !ok {
+			t.Errorf("Unexpected idle allocation: %s", alloc.Name)
+			continue
+		}
+
+		foundCount++
+
+		// Verify ProviderID is the cloud instance ID, not the node name
+		if alloc.Properties.ProviderID != expected.providerID {
+			t.Errorf("Allocation %s: expected ProviderID %q, got %q",
+				alloc.Name, expected.providerID, alloc.Properties.ProviderID)
+		}
+
+		// Verify ProviderID is NOT the node name (the bug we're fixing)
+		if alloc.Properties.ProviderID == expected.nodeName {
+			t.Errorf("Allocation %s: ProviderID should not be node name %q",
+				alloc.Name, expected.nodeName)
+		}
+
+		// Verify Node field still contains the node name
+		if alloc.Properties.Node != expected.nodeName {
+			t.Errorf("Allocation %s: expected Node %q, got %q",
+				alloc.Name, expected.nodeName, alloc.Properties.Node)
+		}
+
+		// Verify costs are non-zero (idle exists)
+		if alloc.CPUCost <= 0 && alloc.RAMCost <= 0 {
+			t.Errorf("Allocation %s: expected non-zero idle costs", alloc.Name)
+		}
+	}
+
+	if foundCount != len(expectedIdles) {
+		t.Errorf("Expected %d idle allocations, found %d", len(expectedIdles), foundCount)
+	}
+}
+
+// TestComputeIdleAllocations_ProviderID_ByCluster verifies that when computing
+// idle allocations by cluster, the idle allocation has an empty ProviderID
+// because there's no single instance ID for the entire cluster.
+func TestComputeIdleAllocations_ProviderID_ByCluster(t *testing.T) {
+	// Create test window
+	start := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
+	end := time.Date(2024, 1, 1, 1, 0, 0, 0, time.UTC)
+	window := opencost.NewClosedWindow(start, end)
+
+	// Create AssetSet with nodes that have ProviderIDs
+	assetSet := opencost.NewAssetSet(start, end)
+
+	node1 := opencost.NewNode("node1", "cluster1", "aws:///us-east-1a/i-0abc123", start, end, window)
+	node1.CPUCost = 10.0
+	node1.RAMCost = 5.0
+	node1.GPUCost = 0.0
+
+	node2 := opencost.NewNode("node2", "cluster1", "aws:///us-east-1b/i-0def456", start, end, window)
+	node2.CPUCost = 8.0
+	node2.RAMCost = 4.0
+	node2.GPUCost = 0.0
+
+	assetSet.Insert(node1, nil)
+	assetSet.Insert(node2, nil)
+
+	// Create AllocationSet with some utilization
+	allocSet := opencost.NewAllocationSet(start, end)
+
+	alloc := &opencost.Allocation{
+		Name:    "namespace1/pod1/container1",
+		Start:   start,
+		End:     end,
+		Window:  window.Clone(),
+		CPUCost: 9.0, // Uses 50% of total cluster CPU (18.0 total)
+		RAMCost: 4.5, // Uses 50% of total cluster RAM (9.0 total)
+		GPUCost: 0.0,
+		Properties: &opencost.AllocationProperties{
+			Cluster: "cluster1",
+			Node:    "node1",
+		},
+	}
+
+	allocSet.Insert(alloc)
+
+	// Compute idle allocations by cluster (idleByNode=false)
+	idleSet, err := computeIdleAllocations(allocSet, assetSet, false)
+	if err != nil {
+		t.Fatalf("Error computing idle allocations: %v", err)
+	}
+
+	// Find the cluster-level idle allocation
+	var clusterIdle *opencost.Allocation
+	for _, alloc := range idleSet.Allocations {
+		if alloc.IsIdle() && alloc.Name == "cluster1/__idle__" {
+			clusterIdle = alloc
+			break
+		}
+	}
+
+	if clusterIdle == nil {
+		t.Fatal("Expected to find cluster-level idle allocation")
+	}
+
+	// Verify ProviderID is empty for cluster-level idle
+	if clusterIdle.Properties.ProviderID != "" {
+		t.Errorf("Cluster-level idle allocation should have empty ProviderID, got %q",
+			clusterIdle.Properties.ProviderID)
+	}
+
+	// Verify Node is also empty for cluster-level idle
+	if clusterIdle.Properties.Node != "" {
+		t.Errorf("Cluster-level idle allocation should have empty Node, got %q",
+			clusterIdle.Properties.Node)
+	}
+
+	// Verify costs are non-zero
+	if clusterIdle.CPUCost <= 0 && clusterIdle.RAMCost <= 0 {
+		t.Error("Expected non-zero idle costs for cluster-level idle")
+	}
+}
+
+// TestComputeIdleAllocations_ProviderID_NoProviderID verifies that nodes
+// without a ProviderID (e.g., bare-metal, local clusters) result in idle
+// allocations with an empty ProviderID field.
+func TestComputeIdleAllocations_ProviderID_NoProviderID(t *testing.T) {
+	// Create test window
+	start := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
+	end := time.Date(2024, 1, 1, 1, 0, 0, 0, time.UTC)
+	window := opencost.NewClosedWindow(start, end)
+
+	// Create AssetSet with a node that has no ProviderID
+	assetSet := opencost.NewAssetSet(start, end)
+
+	node := opencost.NewNode("bare-metal-node-1", "cluster1", "", start, end, window)
+	node.CPUCost = 10.0
+	node.RAMCost = 5.0
+	node.GPUCost = 0.0
+
+	assetSet.Insert(node, nil)
+
+	// Create AllocationSet with partial utilization
+	allocSet := opencost.NewAllocationSet(start, end)
+
+	alloc := &opencost.Allocation{
+		Name:    "namespace1/pod1/container1",
+		Start:   start,
+		End:     end,
+		Window:  window.Clone(),
+		CPUCost: 5.0,
+		RAMCost: 2.5,
+		GPUCost: 0.0,
+		Properties: &opencost.AllocationProperties{
+			Cluster: "cluster1",
+			Node:    "bare-metal-node-1",
+		},
+	}
+
+	allocSet.Insert(alloc)
+
+	// Compute idle allocations by node
+	idleSet, err := computeIdleAllocations(allocSet, assetSet, true)
+	if err != nil {
+		t.Fatalf("Error computing idle allocations: %v", err)
+	}
+
+	// Find the idle allocation
+	var idle *opencost.Allocation
+	for _, alloc := range idleSet.Allocations {
+		if alloc.IsIdle() {
+			idle = alloc
+			break
+		}
+	}
+
+	if idle == nil {
+		t.Fatal("Expected to find idle allocation")
+	}
+
+	// Verify ProviderID is empty
+	if idle.Properties.ProviderID != "" {
+		t.Errorf("Node without ProviderID should result in empty ProviderID in idle allocation, got %q",
+			idle.Properties.ProviderID)
+	}
+
+	// Verify Node field is still populated
+	if idle.Properties.Node != "bare-metal-node-1" {
+		t.Errorf("Expected Node %q, got %q", "bare-metal-node-1", idle.Properties.Node)
+	}
+}
+
+// TestComputeIdleAllocations_ProviderID_AzureFormat tests that Azure VM
+// ProviderIDs are correctly propagated to idle allocations.
+func TestComputeIdleAllocations_ProviderID_AzureFormat(t *testing.T) {
+	// Create test window
+	start := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
+	end := time.Date(2024, 1, 1, 1, 0, 0, 0, time.UTC)
+	window := opencost.NewClosedWindow(start, end)
+
+	// Create AssetSet with Azure VM
+	assetSet := opencost.NewAssetSet(start, end)
+
+	azureProviderID := "azure:///subscriptions/12345678-1234-1234-1234-123456789abc/resourceGroups/my-rg/providers/Microsoft.Compute/virtualMachines/aks-nodepool1-12345678-vmss000000"
+	node := opencost.NewNode("aks-nodepool1-12345678-vmss000000", "cluster1", azureProviderID, start, end, window)
+	node.CPUCost = 10.0
+	node.RAMCost = 5.0
+	node.GPUCost = 0.0
+
+	assetSet.Insert(node, nil)
+
+	// Create AllocationSet
+	allocSet := opencost.NewAllocationSet(start, end)
+
+	alloc := &opencost.Allocation{
+		Name:    "namespace1/pod1/container1",
+		Start:   start,
+		End:     end,
+		Window:  window.Clone(),
+		CPUCost: 5.0,
+		RAMCost: 2.5,
+		GPUCost: 0.0,
+		Properties: &opencost.AllocationProperties{
+			Cluster: "cluster1",
+			Node:    "aks-nodepool1-12345678-vmss000000",
+		},
+	}
+
+	allocSet.Insert(alloc)
+
+	// Compute idle allocations
+	idleSet, err := computeIdleAllocations(allocSet, assetSet, true)
+	if err != nil {
+		t.Fatalf("Error computing idle allocations: %v", err)
+	}
+
+	// Find the idle allocation
+	var idle *opencost.Allocation
+	for _, alloc := range idleSet.Allocations {
+		if alloc.IsIdle() {
+			idle = alloc
+			break
+		}
+	}
+
+	if idle == nil {
+		t.Fatal("Expected to find idle allocation")
+	}
+
+	// Verify Azure ProviderID is correctly set
+	if idle.Properties.ProviderID != azureProviderID {
+		t.Errorf("Expected ProviderID %q, got %q", azureProviderID, idle.Properties.ProviderID)
+	}
+}

pkg/costmodel/nodeclientconfig.go

@@ -37,6 +37,7 @@ func NewNodeClientConfigFromEnv() (*nodes.NodeClientConfig, error) {
 		transport = &http.Transport{
 			TLSClientConfig: &tls.Config{
 				InsecureSkipVerify: true,
+				MinVersion:         tls.VersionTLS12,
 			},
 		}
 	} else {
@@ -60,12 +61,14 @@ func NewNodeClientConfigFromEnv() (*nodes.NodeClientConfig, error) {
 			tlsConfig = &tls.Config{
 				Certificates: []tls.Certificate{cert},
 				RootCAs:      caCertPool,
+				MinVersion:   tls.VersionTLS12,
 			}
 
 			transport = &http.Transport{TLSClientConfig: tlsConfig}
 		} else {
 			tlsConfig := &tls.Config{
-				RootCAs: caCertPool,
+				RootCAs:    caCertPool,
+				MinVersion: tls.VersionTLS12,
 			}
 			transport = &http.Transport{TLSClientConfig: tlsConfig}
 		}