aws: parse disk region dynamically from AvailabilityZone in findCostF… (#3886)

Signed-off-by: Rucha0901 <imt_2025071@iiitm.ac.in>
Signed-off-by: Warwick <warwick@automatic.systems>
Co-authored-by: Warwick <warwick@automatic.systems>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Alex Meijer <ameijer@users.noreply.github.com>

pkg/cloud/aws/provider.go

@@ -2184,7 +2184,7 @@ func (aws *AWS) GetOrphanedResources() ([]models.OrphanedResource, error) {
 				zone = *volume.AvailabilityZone
 			}
 			var region, url string
-			region = regionRx.FindString(zone)
+			region = parseRegionFromAZ(zone)
 			if region != "" {
 				url = "https://console.aws.amazon.com/ec2/home?region=" + region + "#Volumes:sort=desc:createTime"
 			} else {
@@ -2242,9 +2242,8 @@ func (aws *AWS) GetOrphanedResources() ([]models.OrphanedResource, error) {
 }
 
 func (aws *AWS) findCostForDisk(disk *ec2Types.Volume) (*float64, error) {
-	// todo: use AWS pricing from all regions
 	if disk.AvailabilityZone == nil {
-		return nil, fmt.Errorf("nil region")
+		return nil, fmt.Errorf("nil AvailabilityZone")
 	}
 	if disk.Size == nil {
 		return nil, fmt.Errorf("nil disk size")
@@ -2252,7 +2251,14 @@ func (aws *AWS) findCostForDisk(disk *ec2Types.Volume) (*float64, error) {
 
 	class := volTypes[string(disk.VolumeType)]
 
-	key := aws.ClusterRegion + "," + class
+	region := parseRegionFromAZ(*disk.AvailabilityZone)
+
+	key := region + "," + class
+	_, ok := aws.Pricing[key]
+	if region == "" || !ok {
+		// Fallback to ClusterRegion
+		key = aws.ClusterRegion + "," + class
+	}
 
 	pricing, ok := aws.Pricing[key]
 	if !ok {
@@ -2786,3 +2792,16 @@ func (aws *AWS) PricingSourceSummary() interface{} {
 	// encode the pricing source summary as a JSON string
 	return aws.Pricing
 }
+
+func parseRegionFromAZ(zone string) string {
+	region := ""
+	for _, r := range awsRegions {
+		if strings.HasPrefix(zone, r) && len(r) > len(region) {
+			region = r
+		}
+	}
+	if region == "" {
+		region = regionRx.FindString(zone)
+	}
+	return region
+}

pkg/cloud/aws/provider_test.go

@@ -8,9 +8,11 @@ import (
 	"net/url"
 	"os"
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 
+	ec2Types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
 	"github.com/opencost/opencost/core/pkg/clustercache"
 	"github.com/opencost/opencost/pkg/cloud/models"
 	"github.com/opencost/opencost/pkg/config"
@@ -1295,3 +1297,198 @@ func TestAWS_PricingSourceStatus_spotPriceHistory(t *testing.T) {
 		}
 	})
 }
+
+func TestAWS_findCostForDisk(t *testing.T) {
+	aws := &AWS{
+		ClusterRegion: "us-east-1",
+		Pricing: map[string]*AWSProductTerms{
+			"us-east-1,EBS:VolumeUsage.gp2": {
+				PV: &models.PV{
+					Cost: "0.10",
+				},
+			},
+			"us-west-2,EBS:VolumeUsage.gp2": {
+				PV: &models.PV{
+					Cost: "0.12",
+				},
+			},
+			"us-gov-west-1,EBS:VolumeUsage.gp2": {
+				PV: &models.PV{
+					Cost: "0.15",
+				},
+			},
+		},
+	}
+
+	size1 := int32(100)
+	expectedCost1 := 0.10 * 730.0 * 100.0
+	expectedCost2 := 0.12 * 730.0 * 100.0
+	expectedCostGov := 0.15 * 730.0 * 100.0
+
+	checkCost := func(t *testing.T, disk *ec2Types.Volume, expectedCost float64) {
+		cost, err := aws.findCostForDisk(disk)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if cost == nil {
+			t.Fatalf("expected cost %v, got nil", expectedCost)
+		}
+		diff := *cost - expectedCost
+		if diff < 0 {
+			diff = -diff
+		}
+		if diff > 1e-9 {
+			t.Fatalf("expected cost %f, got %f", expectedCost, *cost)
+		}
+	}
+
+	// Case 1: Disk has AvailabilityZone matching ClusterRegion (e.g. us-east-1a)
+	t.Run("AZ matching ClusterRegion", func(t *testing.T) {
+		zone1 := "us-east-1a"
+		disk1 := &ec2Types.Volume{
+			AvailabilityZone: &zone1,
+			VolumeType:       ec2Types.VolumeTypeGp2,
+			Size:             &size1,
+		}
+		checkCost(t, disk1, expectedCost1)
+	})
+
+	// Case 2: Disk has AvailabilityZone from a different region (e.g. us-west-2b)
+	t.Run("AZ from different region", func(t *testing.T) {
+		zone2 := "us-west-2b"
+		disk2 := &ec2Types.Volume{
+			AvailabilityZone: &zone2,
+			VolumeType:       ec2Types.VolumeTypeGp2,
+			Size:             &size1,
+		}
+		checkCost(t, disk2, expectedCost2)
+	})
+
+	// Case 3: GovCloud region prefix match (e.g. us-gov-west-1a)
+	t.Run("GovCloud region prefix match", func(t *testing.T) {
+		zoneGov := "us-gov-west-1a"
+		diskGov := &ec2Types.Volume{
+			AvailabilityZone: &zoneGov,
+			VolumeType:       ec2Types.VolumeTypeGp2,
+			Size:             &size1,
+		}
+		checkCost(t, diskGov, expectedCostGov)
+	})
+
+	// Case 4: Invalid/unknown region format, should fall back to ClusterRegion
+	t.Run("Invalid AZ fallback", func(t *testing.T) {
+		zoneUnknown := "unknown-zone"
+		diskUnknown := &ec2Types.Volume{
+			AvailabilityZone: &zoneUnknown,
+			VolumeType:       ec2Types.VolumeTypeGp2,
+			Size:             &size1,
+		}
+		checkCost(t, diskUnknown, expectedCost1)
+	})
+
+	// Case 5: Valid region but pricing not loaded, should fall back to ClusterRegion
+	t.Run("No pricing loaded fallback", func(t *testing.T) {
+		zoneNoPrice := "eu-west-1b"
+		diskNoPrice := &ec2Types.Volume{
+			AvailabilityZone: &zoneNoPrice,
+			VolumeType:       ec2Types.VolumeTypeGp2,
+			Size:             &size1,
+		}
+		checkCost(t, diskNoPrice, expectedCost1)
+	})
+
+	// Case 6: Region-specific pricing key is present but value is nil.
+	// It should return an error, NOT fall back.
+	t.Run("Region pricing key present but value is nil", func(t *testing.T) {
+		awsCustom := &AWS{
+			ClusterRegion: "us-east-1",
+			Pricing: map[string]*AWSProductTerms{
+				"us-east-1,EBS:VolumeUsage.gp2": {
+					PV: &models.PV{
+						Cost: "0.10",
+					},
+				},
+				"us-west-2,EBS:VolumeUsage.gp2": nil,
+			},
+		}
+		zone := "us-west-2b"
+		disk := &ec2Types.Volume{
+			AvailabilityZone: &zone,
+			VolumeType:       ec2Types.VolumeTypeGp2,
+			Size:             &size1,
+		}
+		_, err := awsCustom.findCostForDisk(disk)
+		if err == nil {
+			t.Fatal("expected error because us-west-2 pricing is nil, but got nil error")
+		}
+		expectedErr := "nil pricing data for key 'us-west-2,EBS:VolumeUsage.gp2'"
+		if err.Error() != expectedErr {
+			t.Fatalf("expected error message %q, got %q", expectedErr, err.Error())
+		}
+	})
+
+	// Case 7: Region-specific pricing key is present but pricing.PV is nil.
+	// It should return an error, NOT fall back.
+	t.Run("Region pricing key present but PV is nil", func(t *testing.T) {
+		awsCustom := &AWS{
+			ClusterRegion: "us-east-1",
+			Pricing: map[string]*AWSProductTerms{
+				"us-east-1,EBS:VolumeUsage.gp2": {
+					PV: &models.PV{
+						Cost: "0.10",
+					},
+				},
+				"us-west-2,EBS:VolumeUsage.gp2": {
+					PV: nil,
+				},
+			},
+		}
+		zone := "us-west-2b"
+		disk := &ec2Types.Volume{
+			AvailabilityZone: &zone,
+			VolumeType:       ec2Types.VolumeTypeGp2,
+			Size:             &size1,
+		}
+		_, err := awsCustom.findCostForDisk(disk)
+		if err == nil {
+			t.Fatal("expected error because us-west-2 pricing PV is nil, but got nil error")
+		}
+		expectedErr := "pricing for key 'us-west-2,EBS:VolumeUsage.gp2' has nil PV"
+		if err.Error() != expectedErr {
+			t.Fatalf("expected error message %q, got %q", expectedErr, err.Error())
+		}
+	})
+
+	// Case 8: Region-specific pricing key is present but pricing.PV.Cost is unparsable.
+	// It should return an error, NOT fall back.
+	t.Run("Region pricing key present but Cost is unparsable", func(t *testing.T) {
+		awsCustom := &AWS{
+			ClusterRegion: "us-east-1",
+			Pricing: map[string]*AWSProductTerms{
+				"us-east-1,EBS:VolumeUsage.gp2": {
+					PV: &models.PV{
+						Cost: "0.10",
+					},
+				},
+				"us-west-2,EBS:VolumeUsage.gp2": {
+					PV: &models.PV{
+						Cost: "not-a-float",
+					},
+				},
+			},
+		}
+		zone := "us-west-2b"
+		disk := &ec2Types.Volume{
+			AvailabilityZone: &zone,
+			VolumeType:       ec2Types.VolumeTypeGp2,
+			Size:             &size1,
+		}
+		_, err := awsCustom.findCostForDisk(disk)
+		if err == nil {
+			t.Fatal("expected error because us-west-2 pricing cost is unparsable, but got nil error")
+		}
+		if !strings.Contains(err.Error(), "parsing \"not-a-float\"") {
+			t.Fatalf("expected parsing float error, got %q", err.Error())
+		}
+	})
+}