Просмотр исходного кода

fix: cover all Allocation cost fields in currency conversion

The original PR missed several cost fields that appear in JSON API
responses, resulting in mixed-currency output where most fields get
converted to EUR/INR/etc. while the unhandled fields remain in USD.
Summed totals computed client-side are therefore nonsense.

Audited core/pkg/opencost/allocation.go and added:

  * NetworkNatGatewayEgressCost  (@bingen:field[version=25])
  * NetworkNatGatewayIngressCost (@bingen:field[version=25])
  * PVAllocation.Adjustment      (json:"adjustment")
  * SharedCostBreakdown.{TotalCost, CPUCost, GPUCost, RAMCost, PVCost,
    NetworkCost, LBCost, ExternalCost}

SharedCostBreakdowns is map[string]SharedCostBreakdown (value type,
not pointer) so the conversion mutates a local copy and re-assigns
only when at least one field changed.

PV and LB loops now nil-check before dereferencing, matching the
pointer-valued map definitions (PVAllocations, LbAllocations).

Added a comment noting the costFields list must stay in sync with
Allocation struct changes -- the next @bingen:field[version=N] cost
field will otherwise silently leak USD values.

Signed-off-by: Warwick Peatey <warwick@automatic.systems>
Assisted-by: Claude Code
Warwick Peatey 1 месяц назад
Родитель
Сommit
847500ad63
1 измененных файлов с 52 добавлено и 3 удалено
  1. 52 3
      pkg/costmodel/currency_helper.go

+ 52 - 3
pkg/costmodel/currency_helper.go

@@ -16,7 +16,9 @@ func ConvertAllocation(alloc *opencost.Allocation, converter currency.Converter,
 
 
 	targetCurrency = strings.ToUpper(strings.TrimSpace(targetCurrency))
 	targetCurrency = strings.ToUpper(strings.TrimSpace(targetCurrency))
 
 
-	// List of all float64 cost fields to convert
+	// List of all float64 cost fields to convert. Keep in sync with
+	// Allocation cost fields in core/pkg/opencost/allocation.go that appear
+	// in JSON API responses.
 	costFields := []*float64{
 	costFields := []*float64{
 		&alloc.CPUCost,
 		&alloc.CPUCost,
 		&alloc.CPUCostAdjustment,
 		&alloc.CPUCostAdjustment,
@@ -29,6 +31,8 @@ func ConvertAllocation(alloc *opencost.Allocation, converter currency.Converter,
 		&alloc.NetworkCrossRegionCost,
 		&alloc.NetworkCrossRegionCost,
 		&alloc.NetworkInternetCost,
 		&alloc.NetworkInternetCost,
 		&alloc.NetworkCostAdjustment,
 		&alloc.NetworkCostAdjustment,
+		&alloc.NetworkNatGatewayEgressCost,
+		&alloc.NetworkNatGatewayIngressCost,
 		&alloc.LoadBalancerCost,
 		&alloc.LoadBalancerCost,
 		&alloc.LoadBalancerCostAdjustment,
 		&alloc.LoadBalancerCostAdjustment,
 		&alloc.PVCostAdjustment,
 		&alloc.PVCostAdjustment,
@@ -51,20 +55,34 @@ func ConvertAllocation(alloc *opencost.Allocation, converter currency.Converter,
 		}
 		}
 	}
 	}
 
 
-	// Convert PV costs (nested structure)
+	// Convert PV costs (nested structure). Both Cost and Adjustment appear
+	// in JSON responses and must be converted.
 	for pvKey, pv := range alloc.PVs {
 	for pvKey, pv := range alloc.PVs {
+		if pv == nil {
+			continue
+		}
 		if pv.Cost != 0 {
 		if pv.Cost != 0 {
 			converted, err := converter.Convert(pv.Cost, "USD", targetCurrency)
 			converted, err := converter.Convert(pv.Cost, "USD", targetCurrency)
 			if err != nil {
 			if err != nil {
 				return fmt.Errorf("failed to convert PV cost: %w", err)
 				return fmt.Errorf("failed to convert PV cost: %w", err)
 			}
 			}
 			pv.Cost = converted
 			pv.Cost = converted
-			alloc.PVs[pvKey] = pv
 		}
 		}
+		if pv.Adjustment != 0 {
+			converted, err := converter.Convert(pv.Adjustment, "USD", targetCurrency)
+			if err != nil {
+				return fmt.Errorf("failed to convert PV adjustment: %w", err)
+			}
+			pv.Adjustment = converted
+		}
+		alloc.PVs[pvKey] = pv
 	}
 	}
 
 
 	// Convert LoadBalancer costs (nested structure)
 	// Convert LoadBalancer costs (nested structure)
 	for lbKey, lb := range alloc.LoadBalancers {
 	for lbKey, lb := range alloc.LoadBalancers {
+		if lb == nil {
+			continue
+		}
 		if lb.Cost != 0 {
 		if lb.Cost != 0 {
 			converted, err := converter.Convert(lb.Cost, "USD", targetCurrency)
 			converted, err := converter.Convert(lb.Cost, "USD", targetCurrency)
 			if err != nil {
 			if err != nil {
@@ -75,6 +93,37 @@ func ConvertAllocation(alloc *opencost.Allocation, converter currency.Converter,
 		}
 		}
 	}
 	}
 
 
+	// Convert SharedCostBreakdown entries. SharedCostBreakdowns is a
+	// map[string]SharedCostBreakdown (value, not pointer), so mutate a
+	// local copy and re-assign.
+	for key, scb := range alloc.SharedCostBreakdown {
+		scbFields := []*float64{
+			&scb.TotalCost,
+			&scb.CPUCost,
+			&scb.GPUCost,
+			&scb.RAMCost,
+			&scb.PVCost,
+			&scb.NetworkCost,
+			&scb.LBCost,
+			&scb.ExternalCost,
+		}
+		mutated := false
+		for _, costPtr := range scbFields {
+			if *costPtr == 0 {
+				continue
+			}
+			converted, err := converter.Convert(*costPtr, "USD", targetCurrency)
+			if err != nil {
+				return fmt.Errorf("failed to convert SharedCostBreakdown %q: %w", key, err)
+			}
+			*costPtr = converted
+			mutated = true
+		}
+		if mutated {
+			alloc.SharedCostBreakdown[key] = scb
+		}
+	}
+
 	return nil
 	return nil
 }
 }