Browse Source

Redo network pricing; add testing to core/pkg/pricing

Niko Kovacevic 4 days ago
parent
commit
8be00b6fe5

+ 14 - 2
core/pkg/pricing/memory.go

@@ -3,9 +3,11 @@ package pricing
 import (
 	"context"
 	"errors"
+	"sync"
 )
 
 type MemoryPricingStore struct {
+	mu      sync.RWMutex
 	pricing *PricingSet
 }
 
@@ -15,16 +17,26 @@ func NewMemoryPricingStore() *MemoryPricingStore {
 	}
 }
 
+// GetPricingSet returns a deep copy of the stored pricing set so that callers
+// cannot mutate the store's internal state through the returned pointer.
 func (mps *MemoryPricingStore) GetPricingSet(ctx context.Context) (*PricingSet, error) {
-	return mps.pricing, nil
+	mps.mu.RLock()
+	defer mps.mu.RUnlock()
+
+	return mps.pricing.Clone(), nil
 }
 
+// SetPricingSet stores a deep copy of the provided pricing set so that the
+// caller cannot mutate the store's internal state after setting it.
 func (mps *MemoryPricingStore) SetPricingSet(ctx context.Context, pricing *PricingSet) error {
 	if pricing == nil {
 		return errors.New("nil pricing")
 	}
 
-	mps.pricing = pricing
+	mps.mu.Lock()
+	defer mps.mu.Unlock()
+
+	mps.pricing = pricing.Clone()
 
 	return nil
 }

+ 159 - 0
core/pkg/pricing/memory_test.go

@@ -0,0 +1,159 @@
+package pricing
+
+import (
+	"sync"
+	"testing"
+
+	"github.com/opencost/opencost/core/pkg/unit"
+)
+
+// TestMemoryGetEmpty verifies that a fresh store returns a non-nil, empty set.
+func TestMemoryGetEmpty(t *testing.T) {
+	mps := NewMemoryPricingStore()
+
+	ps, err := mps.GetPricingSet(t.Context())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if ps == nil {
+		t.Fatal("expected non-nil pricing set")
+	}
+	if !ps.IsEmpty() {
+		t.Errorf("expected empty pricing set, got %+v", ps)
+	}
+}
+
+// TestMemoryRoundTrip verifies that a set survives a Set/Get round trip while
+// being returned as a distinct copy, not the stored pointer.
+func TestMemoryRoundTrip(t *testing.T) {
+	mps := NewMemoryPricingStore()
+	in := fullPricingSet()
+
+	if err := mps.SetPricingSet(t.Context(), in); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	out, err := mps.GetPricingSet(t.Context())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	csIn, err := in.Checksum()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	csOut, err := out.Checksum()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if csIn != csOut {
+		t.Errorf("round-trip changed contents: %q -> %q", csIn, csOut)
+	}
+
+	if out == in {
+		t.Error("expected Get to return a copy, not the set pointer")
+	}
+}
+
+// TestMemorySetIsolation verifies that mutating the value passed to Set after
+// the call does not affect what the store returns.
+func TestMemorySetIsolation(t *testing.T) {
+	mps := NewMemoryPricingStore()
+	in := fullPricingSet()
+
+	if err := mps.SetPricingSet(t.Context(), in); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	before, err := mps.GetPricingSet(t.Context())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	csBefore, err := before.Checksum()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Mutate the caller's copy after setting it.
+	in.NodePricing[0].Properties.Labels["team"] = "mutated"
+	in.NodePricing[0].Prices[ResourceNode] = Price{Unit: unit.Hour, Price: 99}
+	in.NodePricing = append(in.NodePricing, nodePricing("m5.xlarge", 0.192))
+
+	after, err := mps.GetPricingSet(t.Context())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	csAfter, err := after.Checksum()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if csBefore != csAfter {
+		t.Errorf("mutating the set argument changed store contents: %q -> %q", csBefore, csAfter)
+	}
+}
+
+// TestMemoryGetIsolation verifies that mutating a value returned by Get does not
+// affect what subsequent Get calls return.
+func TestMemoryGetIsolation(t *testing.T) {
+	mps := NewMemoryPricingStore()
+	if err := mps.SetPricingSet(t.Context(), fullPricingSet()); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	got, err := mps.GetPricingSet(t.Context())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	csBefore, err := got.Checksum()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Mutate the returned copy.
+	got.NodePricing[0].Properties.Labels["team"] = "mutated"
+	got.NodePricing[0].Prices[ResourceNode] = Price{Unit: unit.Hour, Price: 99}
+
+	again, err := mps.GetPricingSet(t.Context())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	csAfter, err := again.Checksum()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if csBefore != csAfter {
+		t.Errorf("mutating a returned set changed store contents: %q -> %q", csBefore, csAfter)
+	}
+}
+
+// TestMemorySetNil verifies that Set rejects a nil pricing set.
+func TestMemorySetNil(t *testing.T) {
+	mps := NewMemoryPricingStore()
+	if err := mps.SetPricingSet(t.Context(), nil); err == nil {
+		t.Error("expected error for nil pricing set")
+	}
+}
+
+// TestMemoryConcurrentAccess exercises the store under concurrent readers and
+// writers; run with -race to catch data races.
+func TestMemoryConcurrentAccess(t *testing.T) {
+	mps := NewMemoryPricingStore()
+
+	var wg sync.WaitGroup
+	for i := 0; i < 50; i++ {
+		wg.Add(2)
+		go func() {
+			defer wg.Done()
+			_ = mps.SetPricingSet(t.Context(), fullPricingSet())
+		}()
+		go func() {
+			defer wg.Done()
+			if _, err := mps.GetPricingSet(t.Context()); err != nil {
+				t.Errorf("unexpected error: %v", err)
+			}
+		}()
+	}
+	wg.Wait()
+}

+ 3 - 6
core/pkg/pricing/mock.go

@@ -85,15 +85,12 @@ func (mpm *MockPricingModule) GetNetworkPricing(ctx context.Context, props Netwo
 
 	// Search through the mock data for a matching network pricing entry
 	for _, np := range mpm.NetworkPricing {
-		if np.Properties.Provider == props.Provider &&
-			np.Properties.TrafficDirection == props.TrafficDirection &&
-			np.Properties.TrafficType == props.TrafficType &&
-			np.Properties.IsNatGateway == props.IsNatGateway {
+		if np.Properties.Provider == props.Provider {
 			return np, nil
 		}
 	}
-	return nil, fmt.Errorf("network pricing not found for provider=%s, trafficDirection=%s, trafficType=%s, isNatGateway=%t",
-		props.Provider, props.TrafficDirection, props.TrafficType, props.IsNatGateway)
+
+	return nil, fmt.Errorf("network pricing not found for provider=%s", props.Provider)
 }
 
 func (mpm *MockPricingModule) NewNetworkPricingReader(ctx context.Context) (reader.Reader[*NetworkPricing], error) {

+ 16 - 26
core/pkg/pricing/mock_test.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/opencost/opencost/core/pkg/model/kubemodel"
 	"github.com/opencost/opencost/core/pkg/model/shared"
 	"github.com/opencost/opencost/core/pkg/reader"
 )
@@ -180,46 +179,37 @@ func TestMockGetClusterPricing(t *testing.T) {
 	}
 }
 
-// TestMockGetNetworkPricing verifies network lookup, including that the NAT
-// gateway flag discriminates between otherwise-identical entries.
+// TestMockGetNetworkPricing verifies network lookup by provider and that the
+// per-egress-type resource prices load from YAML.
 func TestMockGetNetworkPricing(t *testing.T) {
 	mpm := newMock(t)
 
-	internet, err := mpm.GetNetworkPricing(t.Context(), NetworkPricingProperties{
-		Provider:         shared.ProviderAWS,
-		TrafficDirection: kubemodel.TrafficDirectionEgress,
-		TrafficType:      kubemodel.TrafficTypeInternet,
+	np, err := mpm.GetNetworkPricing(t.Context(), NetworkPricingProperties{
+		Provider: shared.ProviderAWS,
 	})
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if internet.Prices[ResourceNetworkTraffic].Price != 0.09 {
-		t.Errorf("expected internet egress price 0.09, got %v", internet.Prices[ResourceNetworkTraffic].Price)
-	}
 
-	nat, err := mpm.GetNetworkPricing(t.Context(), NetworkPricingProperties{
-		Provider:         shared.ProviderAWS,
-		TrafficDirection: kubemodel.TrafficDirectionEgress,
-		TrafficType:      kubemodel.TrafficTypeInternet,
-		IsNatGateway:     true,
-	})
-	if err != nil {
-		t.Fatalf("unexpected error (nat): %v", err)
+	internet, ok := np.Prices[ResourceInternetEgress]
+	if !ok || internet.Price != 0.09 {
+		t.Errorf("expected internet egress price 0.09, got %v (ok=%t)", internet.Price, ok)
 	}
-	if nat.Prices[ResourceNetworkTraffic].Price != 0.045 {
-		t.Errorf("expected NAT gateway price 0.045, got %v", nat.Prices[ResourceNetworkTraffic].Price)
+
+	nat, ok := np.Prices[ResourceNATGatewayEgress]
+	if !ok || nat.Price != 0.045 {
+		t.Errorf("expected NAT gateway egress price 0.045, got %v (ok=%t)", nat.Price, ok)
 	}
 
-	if internet.Prices[ResourceNetworkTraffic].Price == nat.Prices[ResourceNetworkTraffic].Price {
-		t.Errorf("expected NAT gateway flag to discriminate pricing")
+	if internet.Price == nat.Price {
+		t.Errorf("expected internet egress and NAT gateway egress prices to differ")
 	}
 
+	// Missing provider should error rather than return a zero value.
 	if _, err := mpm.GetNetworkPricing(t.Context(), NetworkPricingProperties{
-		Provider:         shared.ProviderAWS,
-		TrafficDirection: kubemodel.TrafficDirectionIngress,
-		TrafficType:      kubemodel.TrafficTypeInternet,
+		Provider: shared.ProviderOracle,
 	}); err == nil {
-		t.Errorf("expected error for unknown traffic direction, got nil")
+		t.Errorf("expected error for unknown provider, got nil")
 	}
 }
 

+ 4 - 14
core/pkg/pricing/network.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/opencost/opencost/core/pkg/model/kubemodel"
 	"github.com/opencost/opencost/core/pkg/model/shared"
 )
 
@@ -18,22 +17,13 @@ func (sp *NetworkPricing) String() string {
 }
 
 type NetworkPricingProperties struct {
-	Provider         shared.Provider            `json:"provider,omitempty" yaml:"provider,omitempty"`
-	TrafficDirection kubemodel.TrafficDirection `json:"trafficDirection,omitempty" yaml:"trafficDirection,omitempty"`
-	TrafficType      kubemodel.TrafficType      `json:"trafficType,omitempty" yaml:"trafficType,omitempty"`
-	IsNatGateway     bool                       `json:"isNatGateway,omitempty" yaml:"isNatGateway,omitempty"`
-	Start            *time.Time                 `json:"start,omitempty" yaml:"start,omitempty"`
-	End              *time.Time                 `json:"end,omitempty" yaml:"end,omitempty"`
+	Provider shared.Provider `json:"provider,omitempty" yaml:"provider,omitempty"`
+	Start    *time.Time      `json:"start,omitempty" yaml:"start,omitempty"`
+	End      *time.Time      `json:"end,omitempty" yaml:"end,omitempty"`
 }
 
 func (sp *NetworkPricingProperties) String() string {
-	return fmt.Sprintf("%s:%s:%s:nat=%t:%s",
-		sp.Provider,
-		sp.TrafficDirection,
-		sp.TrafficType,
-		sp.IsNatGateway,
-		sp.timeKey(),
-	)
+	return fmt.Sprintf("%s:%s", sp.Provider, sp.timeKey())
 }
 
 func (sp *NetworkPricingProperties) timeKey() string {

+ 140 - 0
core/pkg/pricing/pricingset.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"hash/fnv"
 	"slices"
+	"time"
 )
 
 type PricingSet struct {
@@ -77,6 +78,145 @@ func (ps *PricingSet) Checksum() (string, error) {
 	return hex.EncodeToString(hasher.Sum(nil)), nil
 }
 
+// Clone returns a deep copy of the PricingSet. Mutating the returned set
+// (including its nested slices, maps, and time pointers) does not affect the
+// original, and vice versa. A nil receiver returns an empty, non-nil set.
+func (ps *PricingSet) Clone() *PricingSet {
+	if ps == nil {
+		return &PricingSet{}
+	}
+
+	return &PricingSet{
+		ClusterPricing:          cloneSlice(ps.ClusterPricing, (*ClusterPricing).clone),
+		NetworkPricing:          cloneSlice(ps.NetworkPricing, (*NetworkPricing).clone),
+		NodePricing:             cloneSlice(ps.NodePricing, (*NodePricing).clone),
+		PersistentVolumePricing: cloneSlice(ps.PersistentVolumePricing, (*PersistentVolumePricing).clone),
+		ServicePricing:          cloneSlice(ps.ServicePricing, (*ServicePricing).clone),
+	}
+}
+
+// cloneSlice deep-copies a slice of pointers using the provided element clone
+// function, preserving a nil source slice as nil.
+func cloneSlice[T any](src []*T, clone func(*T) *T) []*T {
+	if src == nil {
+		return nil
+	}
+
+	dst := make([]*T, len(src))
+	for i, e := range src {
+		dst[i] = clone(e)
+	}
+
+	return dst
+}
+
+func (cp *ClusterPricing) clone() *ClusterPricing {
+	if cp == nil {
+		return nil
+	}
+
+	clone := *cp
+	clone.Properties.Start = cloneTime(cp.Properties.Start)
+	clone.Properties.End = cloneTime(cp.Properties.End)
+	clone.Prices = clonePrices(cp.Prices)
+
+	return &clone
+}
+
+func (np *NetworkPricing) clone() *NetworkPricing {
+	if np == nil {
+		return nil
+	}
+
+	clone := *np
+	clone.Properties.Start = cloneTime(np.Properties.Start)
+	clone.Properties.End = cloneTime(np.Properties.End)
+	clone.Prices = clonePrices(np.Prices)
+
+	return &clone
+}
+
+func (np *NodePricing) clone() *NodePricing {
+	if np == nil {
+		return nil
+	}
+
+	clone := *np
+	clone.Properties.Labels = cloneLabels(np.Properties.Labels)
+	clone.Properties.Start = cloneTime(np.Properties.Start)
+	clone.Properties.End = cloneTime(np.Properties.End)
+	clone.Prices = clonePrices(np.Prices)
+
+	return &clone
+}
+
+func (pvp *PersistentVolumePricing) clone() *PersistentVolumePricing {
+	if pvp == nil {
+		return nil
+	}
+
+	clone := *pvp
+	clone.Properties.Labels = cloneLabels(pvp.Properties.Labels)
+	clone.Properties.Start = cloneTime(pvp.Properties.Start)
+	clone.Properties.End = cloneTime(pvp.Properties.End)
+	clone.Prices = clonePrices(pvp.Prices)
+
+	return &clone
+}
+
+func (sp *ServicePricing) clone() *ServicePricing {
+	if sp == nil {
+		return nil
+	}
+
+	clone := *sp
+	clone.Properties.Start = cloneTime(sp.Properties.Start)
+	clone.Properties.End = cloneTime(sp.Properties.End)
+	clone.Prices = clonePrices(sp.Prices)
+
+	return &clone
+}
+
+// clonePrices returns a deep copy of a Prices map, preserving nil as nil. Price
+// values contain no reference fields, so a shallow value copy per entry is safe.
+func clonePrices(src Prices) Prices {
+	if src == nil {
+		return nil
+	}
+
+	dst := make(Prices, len(src))
+	for k, v := range src {
+		dst[k] = v
+	}
+
+	return dst
+}
+
+// cloneLabels returns a deep copy of a labels map, preserving nil as nil.
+func cloneLabels(src map[string]string) map[string]string {
+	if src == nil {
+		return nil
+	}
+
+	dst := make(map[string]string, len(src))
+	for k, v := range src {
+		dst[k] = v
+	}
+
+	return dst
+}
+
+// cloneTime returns a copy of the time pointer, preserving nil as nil.
+func cloneTime(src *time.Time) *time.Time {
+	if src == nil {
+		return nil
+	}
+
+	t := *src
+
+	return &t
+}
+
 // Sort sorts the pricing data to ensure deterministic serialization.
 func (ps *PricingSet) Sort() {
 	if ps == nil {

+ 117 - 0
core/pkg/pricing/pricingset_test.go

@@ -2,6 +2,7 @@ package pricing
 
 import (
 	"testing"
+	"time"
 
 	"github.com/opencost/opencost/core/pkg/model/shared"
 	"github.com/opencost/opencost/core/pkg/unit"
@@ -108,6 +109,122 @@ func TestIsEmptyAllKinds(t *testing.T) {
 	}
 }
 
+// fullPricingSet returns a PricingSet that exercises every kind plus every
+// reference-typed field (Labels maps, Prices maps, and *time.Time pointers) so
+// that Clone independence can be verified end to end.
+func fullPricingSet() *PricingSet {
+	start := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
+	end := time.Date(2026, 1, 2, 0, 0, 0, 0, time.UTC)
+
+	node := nodePricing("m5.large", 0.096)
+	node.Properties.Labels = map[string]string{"team": "platform"}
+	node.Properties.Start = &start
+	node.Properties.End = &end
+
+	pv := pvPricing(VolumeTypeGP3, 0.0001)
+	pv.Properties.Labels = map[string]string{"env": "prod"}
+	pv.Properties.Start = &start
+
+	return &PricingSet{
+		ClusterPricing: []*ClusterPricing{{
+			Properties: ClusterPricingProperties{Provider: shared.Provider("AWS"), Start: &start},
+			Prices:     Prices{ResourceCluster: {Unit: unit.Hour, Price: 1.0}},
+		}},
+		NetworkPricing: []*NetworkPricing{{
+			Properties: NetworkPricingProperties{Provider: shared.Provider("AWS"), End: &end},
+			Prices:     Prices{ResourceInternetEgress: {Unit: unit.GiB, Price: 0.09}},
+		}},
+		NodePricing:             []*NodePricing{node},
+		PersistentVolumePricing: []*PersistentVolumePricing{pv},
+		ServicePricing: []*ServicePricing{{
+			Properties: ServicePricingProperties{Provider: shared.Provider("AWS"), Region: "us-east-1", Start: &start},
+			Prices:     Prices{ResourceService: {Unit: unit.Hour, Price: 0.025}},
+		}},
+	}
+}
+
+// TestCloneEquality verifies that a clone is equal to the original by checksum.
+func TestCloneEquality(t *testing.T) {
+	orig := fullPricingSet()
+	clone := orig.Clone()
+
+	csOrig, err := orig.Checksum()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	csClone, err := clone.Checksum()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if csOrig != csClone {
+		t.Errorf("expected clone checksum %q to equal original %q", csClone, csOrig)
+	}
+}
+
+// TestCloneIndependence verifies that mutating a clone's nested slices, maps,
+// and time pointers does not affect the original.
+func TestCloneIndependence(t *testing.T) {
+	orig := fullPricingSet()
+
+	csBefore, err := orig.Checksum()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	clone := orig.Clone()
+
+	// Mutate every reference-typed field reachable from the clone.
+	clone.NodePricing[0].Properties.Labels["team"] = "mutated"
+	clone.NodePricing[0].Prices[ResourceNode] = Price{Unit: unit.Hour, Price: 99}
+	*clone.NodePricing[0].Properties.Start = time.Date(1999, 1, 1, 0, 0, 0, 0, time.UTC)
+	clone.NodePricing[0].Properties.End = nil
+	clone.PersistentVolumePricing[0].Properties.Labels["env"] = "mutated"
+	clone.PersistentVolumePricing[0].Prices[ResourceStorage] = Price{Unit: unit.GiBHour, Price: 99}
+	clone.ClusterPricing[0].Prices[ResourceCluster] = Price{Unit: unit.Hour, Price: 99}
+	clone.NetworkPricing[0].Prices[ResourceInternetEgress] = Price{Unit: unit.GiB, Price: 99}
+	clone.ServicePricing[0].Prices[ResourceService] = Price{Unit: unit.Hour, Price: 99}
+
+	// Replace whole slices to confirm the slice headers are independent too.
+	clone.NodePricing = append(clone.NodePricing, nodePricing("m5.xlarge", 0.192))
+
+	csAfter, err := orig.Checksum()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if csBefore != csAfter {
+		t.Errorf("mutating clone changed original: checksum %q -> %q", csBefore, csAfter)
+	}
+}
+
+// TestCloneNilReceiver verifies that Clone handles a nil receiver by returning
+// an empty, non-nil set rather than panicking.
+func TestCloneNilReceiver(t *testing.T) {
+	var ps *PricingSet
+	clone := ps.Clone()
+
+	if clone == nil {
+		t.Fatal("expected non-nil clone from nil receiver")
+	}
+	if !clone.IsEmpty() {
+		t.Errorf("expected empty clone from nil receiver")
+	}
+}
+
+// TestClonePreservesNilSlices verifies that Clone does not turn nil pricing
+// slices into empty ones, keeping serialization semantics stable.
+func TestClonePreservesNilSlices(t *testing.T) {
+	clone := (&PricingSet{}).Clone()
+
+	if clone.NodePricing != nil {
+		t.Errorf("expected nil NodePricing slice, got %v", clone.NodePricing)
+	}
+	if clone.ClusterPricing != nil {
+		t.Errorf("expected nil ClusterPricing slice, got %v", clone.ClusterPricing)
+	}
+}
+
 // TestMockGetPricingSetAllKinds verifies that the mock's GetPricingSet exposes
 // the same kinds as its readers, not just node + persistent volume.
 func TestMockGetPricingSetAllKinds(t *testing.T) {

+ 26 - 11
core/pkg/pricing/resource.go

@@ -8,15 +8,20 @@ import (
 type Resource string
 
 const (
-	ResourceNil            Resource = ""
-	ResourceNode           Resource = "node"
-	ResourceCPU            Resource = "cpu"
-	ResourceRAM            Resource = "ram"
-	ResourceGPU            Resource = "gpu"
-	ResourceStorage        Resource = "storage"
-	ResourceCluster        Resource = "cluster"
-	ResourceService        Resource = "service"
-	ResourceNetworkTraffic Resource = "networktraffic"
+	ResourceNil               Resource = ""
+	ResourceNode              Resource = "node"
+	ResourceCPU               Resource = "cpu"
+	ResourceRAM               Resource = "ram"
+	ResourceGPU               Resource = "gpu"
+	ResourceStorage           Resource = "storage"
+	ResourceCluster           Resource = "cluster"
+	ResourceService           Resource = "service"
+	ResourceLocalEgress       Resource = "local-egress"
+	ResourceCrossZoneEgress   Resource = "x-zone-egress"
+	ResourceCrossRegionEgress Resource = "x-region-egress"
+	ResourceInternetEgress    Resource = "internet-egress"
+	ResourceNATGatewayEgress  Resource = "nat-egress"
+	ResourceNATGatewayIngress Resource = "nat-ingress"
 )
 
 func ParseResource(str string) (Resource, error) {
@@ -35,8 +40,18 @@ func ParseResource(str string) (Resource, error) {
 		return ResourceCluster, nil
 	case string(ResourceService):
 		return ResourceService, nil
-	case string(ResourceNetworkTraffic):
-		return ResourceNetworkTraffic, nil
+	case string(ResourceLocalEgress):
+		return ResourceLocalEgress, nil
+	case string(ResourceCrossZoneEgress):
+		return ResourceCrossZoneEgress, nil
+	case string(ResourceCrossRegionEgress):
+		return ResourceCrossRegionEgress, nil
+	case string(ResourceInternetEgress):
+		return ResourceInternetEgress, nil
+	case string(ResourceNATGatewayEgress):
+		return ResourceNATGatewayEgress, nil
+	case string(ResourceNATGatewayIngress):
+		return ResourceNATGatewayIngress, nil
 	default:
 		return ResourceNil, fmt.Errorf("unknown resource %q", str)
 	}

+ 8 - 0
core/pkg/pricing/storage.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"sync"
 
 	"github.com/opencost/opencost/core/pkg/storage"
 )
@@ -12,6 +13,7 @@ import (
 type StoragePricingStore struct {
 	store storage.Storage
 	path  string
+	mu    sync.RWMutex
 }
 
 func NewStoragePricingStore(ctx context.Context, store storage.Storage, path string) (*StoragePricingStore, error) {
@@ -43,6 +45,9 @@ func NewStoragePricingStore(ctx context.Context, store storage.Storage, path str
 }
 
 func (sps *StoragePricingStore) GetPricingSet(ctx context.Context) (*PricingSet, error) {
+	sps.mu.RLock()
+	defer sps.mu.RUnlock()
+
 	data, err := sps.store.Read(sps.path)
 	if err != nil {
 		return nil, fmt.Errorf("reading path '%s': %w", sps.path, err)
@@ -58,6 +63,9 @@ func (sps *StoragePricingStore) GetPricingSet(ctx context.Context) (*PricingSet,
 }
 
 func (sps *StoragePricingStore) SetPricingSet(ctx context.Context, pricing *PricingSet) error {
+	sps.mu.Lock()
+	defer sps.mu.Unlock()
+
 	if pricing == nil {
 		return errors.New("nil pricing")
 	}

+ 139 - 0
core/pkg/pricing/storage_test.go

@@ -0,0 +1,139 @@
+package pricing
+
+import (
+	"sync"
+	"testing"
+
+	"github.com/opencost/opencost/core/pkg/storage"
+)
+
+// TestStorageNewValidation verifies the constructor's argument validation and
+// that a non-existent path is auto-initialized with an empty pricing set.
+func TestStorageNewValidation(t *testing.T) {
+	if _, err := NewStoragePricingStore(t.Context(), nil, "pricing.json"); err == nil {
+		t.Error("expected error for nil storage")
+	}
+
+	if _, err := NewStoragePricingStore(t.Context(), storage.NewMemoryStorage(), ""); err == nil {
+		t.Error("expected error for empty path")
+	}
+
+	store := storage.NewMemoryStorage()
+	sps, err := NewStoragePricingStore(t.Context(), store, "pricing.json")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	exists, err := store.Exists("pricing.json")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if !exists {
+		t.Error("expected constructor to initialize the pricing path")
+	}
+
+	ps, err := sps.GetPricingSet(t.Context())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if !ps.IsEmpty() {
+		t.Errorf("expected auto-initialized set to be empty, got %+v", ps)
+	}
+}
+
+// TestStorageRoundTrip verifies that a set survives a Set/Get round trip through
+// the backing storage.
+func TestStorageRoundTrip(t *testing.T) {
+	sps, err := NewStoragePricingStore(t.Context(), storage.NewMemoryStorage(), "pricing.json")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	in := fullPricingSet()
+	if err := sps.SetPricingSet(t.Context(), in); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	out, err := sps.GetPricingSet(t.Context())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	csIn, err := in.Checksum()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	csOut, err := out.Checksum()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if csIn != csOut {
+		t.Errorf("round-trip changed contents: %q -> %q", csIn, csOut)
+	}
+}
+
+// TestStorageSetNil verifies that Set rejects a nil pricing set.
+func TestStorageSetNil(t *testing.T) {
+	sps, err := NewStoragePricingStore(t.Context(), storage.NewMemoryStorage(), "pricing.json")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if err := sps.SetPricingSet(t.Context(), nil); err == nil {
+		t.Error("expected error for nil pricing set")
+	}
+}
+
+// TestStorageGetMissingPath verifies that Get surfaces a read error when the
+// backing path does not exist. The store is constructed directly to bypass the
+// constructor's auto-initialization.
+func TestStorageGetMissingPath(t *testing.T) {
+	sps := &StoragePricingStore{
+		store: storage.NewMemoryStorage(),
+		path:  "missing.json",
+	}
+
+	if _, err := sps.GetPricingSet(t.Context()); err == nil {
+		t.Error("expected error reading a missing path")
+	}
+}
+
+// TestStorageGetMalformedData verifies that Get surfaces a decode error when the
+// backing data is not valid JSON.
+func TestStorageGetMalformedData(t *testing.T) {
+	store := storage.NewMemoryStorage()
+	if err := store.Write("pricing.json", []byte("not json")); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	sps := &StoragePricingStore{store: store, path: "pricing.json"}
+
+	if _, err := sps.GetPricingSet(t.Context()); err == nil {
+		t.Error("expected error decoding malformed pricing data")
+	}
+}
+
+// TestStorageConcurrentAccess exercises the store under concurrent readers and
+// writers; run with -race to catch data races.
+func TestStorageConcurrentAccess(t *testing.T) {
+	sps, err := NewStoragePricingStore(t.Context(), storage.NewMemoryStorage(), "pricing.json")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	var wg sync.WaitGroup
+	for i := 0; i < 50; i++ {
+		wg.Add(2)
+		go func() {
+			defer wg.Done()
+			_ = sps.SetPricingSet(t.Context(), fullPricingSet())
+		}()
+		go func() {
+			defer wg.Done()
+			if _, err := sps.GetPricingSet(t.Context()); err != nil {
+				t.Errorf("unexpected error: %v", err)
+			}
+		}()
+	}
+	wg.Wait()
+}

+ 16 - 20
core/pkg/pricing/test/aws.yaml

@@ -166,28 +166,24 @@ clusterPricing:
 networkPricing:
   - properties:
       provider: AWS
-      trafficDirection: Egress
-      trafficType: Internet
     prices:
-      networktraffic:
-        unit: GB
-        price: 0.09
-  - properties:
-      provider: AWS
-      trafficDirection: Egress
-      trafficType: CrossRegion
-    prices:
-      networktraffic:
-        unit: GB
+      local-egress:
+        unit: GiB
+        price: 0.0
+      x-zone-egress:
+        unit: GiB
+        price: 0.01
+      x-region-egress:
+        unit: GiB
         price: 0.02
-  - properties:
-      provider: AWS
-      trafficDirection: Egress
-      trafficType: Internet
-      isNatGateway: true
-    prices:
-      networktraffic:
-        unit: GB
+      internet-egress:
+        unit: GiB
+        price: 0.09
+      nat-egress:
+        unit: GiB
+        price: 0.045
+      nat-ingress:
+        unit: GiB
         price: 0.045
 servicePricing:
   - properties:

+ 23 - 1
core/pkg/pricing/test/azure.yaml

@@ -155,4 +155,26 @@ persistentVolumePricing:
     prices:
       storage:
         unit: GiB-hr
-        price: 0.0000767
+        price: 0.0000767
+networkPricing:
+  - properties:
+      provider: Azure
+    prices:
+      local-egress:
+        unit: GiB
+        price: 0.0
+      x-zone-egress:
+        unit: GiB
+        price: 0.01
+      x-region-egress:
+        unit: GiB
+        price: 0.02
+      internet-egress:
+        unit: GiB
+        price: 0.087
+      nat-egress:
+        unit: GiB
+        price: 0.045
+      nat-ingress:
+        unit: GiB
+        price: 0.045

+ 21 - 0
core/pkg/pricing/test/default.yaml

@@ -13,3 +13,24 @@ persistentVolumePricing:
       storage:
         unit: GiB-hr
         price: 0.0000581
+networkPricing:
+  - properties: {}
+    prices:
+      local-egress:
+        unit: GiB
+        price: 0.0
+      x-zone-egress:
+        unit: GiB
+        price: 0.01
+      x-region-egress:
+        unit: GiB
+        price: 0.01
+      internet-egress:
+        unit: GiB
+        price: 0.143
+      nat-egress:
+        unit: GiB
+        price: 0.045
+      nat-ingress:
+        unit: GiB
+        price: 0.045

+ 23 - 1
core/pkg/pricing/test/gcp.yaml

@@ -216,4 +216,26 @@ persistentVolumePricing:
     prices:
       storage:
         unit: GiB-hr
-        price: 0.0000581
+        price: 0.0000581
+networkPricing:
+  - properties:
+      provider: GCP
+    prices:
+      local-egress:
+        unit: GiB
+        price: 0.0
+      x-zone-egress:
+        unit: GiB
+        price: 0.01
+      x-region-egress:
+        unit: GiB
+        price: 0.01
+      internet-egress:
+        unit: GiB
+        price: 0.12
+      nat-egress:
+        unit: GiB
+        price: 0.045
+      nat-ingress:
+        unit: GiB
+        price: 0.045

+ 21 - 57
modules/pricing/basic/default.go

@@ -1,18 +1,18 @@
 package basic
 
 import (
-	"github.com/opencost/opencost/core/pkg/model/kubemodel"
 	"github.com/opencost/opencost/core/pkg/pricing"
 	"github.com/opencost/opencost/core/pkg/unit"
 )
 
 const DefaultClusterPricePerHour float64 = 0.0
 
-const DefaultNetworkLocalPricePerGiB float64 = 0.0
-const DefaultNetworkCrossZonePricePerGiB float64 = 0.01
-const DefaultNetworkCrossRegionPricePerGiB float64 = 0.01
-const DefaultNetworkInternetPricePerGiB float64 = 0.143
-const DefaultNetworkNATPricePerGiB float64 = 0.045
+const DefaultNetworkLocalEgressPricePerGiB float64 = 0.0
+const DefaultNetworkCrossZoneEgressPricePerGiB float64 = 0.01
+const DefaultNetworkCrossRegionEgressPricePerGiB float64 = 0.01
+const DefaultNetworkInternetEgressPricePerGiB float64 = 0.143
+const DefaultNetworkNATGatewayEgressPricePerGiB float64 = 0.045
+const DefaultNetworkNATGatewayIngressPricePerGiB float64 = 0.045
 
 const DefaultNodePricePerVCPUHour float64 = 0.031611
 const DefaultNodePricePerRAMGiBHour float64 = 0.004237
@@ -50,67 +50,31 @@ func GetDefaultClusterPricing() []*pricing.ClusterPricing {
 func GetDefaultNetworkPricing() []*pricing.NetworkPricing {
 	return []*pricing.NetworkPricing{
 		{
-			Properties: pricing.NetworkPricingProperties{
-				TrafficDirection: kubemodel.TrafficDirectionEgress,
-				TrafficType:      kubemodel.TrafficTypeLocal,
-				IsNatGateway:     false,
-			},
+			Properties: pricing.NetworkPricingProperties{},
 			Prices: pricing.Prices{
-				pricing.ResourceNetworkTraffic: {
+				pricing.ResourceLocalEgress: {
 					Unit:  unit.GiB,
-					Price: DefaultNetworkLocalPricePerGiB,
+					Price: DefaultNetworkLocalEgressPricePerGiB,
 				},
-			},
-		},
-		{
-			Properties: pricing.NetworkPricingProperties{
-				TrafficDirection: kubemodel.TrafficDirectionEgress,
-				TrafficType:      kubemodel.TrafficTypeCrossZone,
-				IsNatGateway:     false,
-			},
-			Prices: pricing.Prices{
-				pricing.ResourceNetworkTraffic: {
+				pricing.ResourceCrossZoneEgress: {
 					Unit:  unit.GiB,
-					Price: DefaultNetworkCrossZonePricePerGiB,
+					Price: DefaultNetworkCrossZoneEgressPricePerGiB,
 				},
-			},
-		},
-		{
-			Properties: pricing.NetworkPricingProperties{
-				TrafficDirection: kubemodel.TrafficDirectionEgress,
-				TrafficType:      kubemodel.TrafficTypeCrossRegion,
-				IsNatGateway:     false,
-			},
-			Prices: pricing.Prices{
-				pricing.ResourceNetworkTraffic: {
+				pricing.ResourceCrossRegionEgress: {
 					Unit:  unit.GiB,
-					Price: DefaultNetworkCrossRegionPricePerGiB,
+					Price: DefaultNetworkCrossRegionEgressPricePerGiB,
 				},
-			},
-		},
-		{
-			Properties: pricing.NetworkPricingProperties{
-				TrafficDirection: kubemodel.TrafficDirectionEgress,
-				TrafficType:      kubemodel.TrafficTypeInternet,
-				IsNatGateway:     false,
-			},
-			Prices: pricing.Prices{
-				pricing.ResourceNetworkTraffic: {
+				pricing.ResourceInternetEgress: {
 					Unit:  unit.GiB,
-					Price: DefaultNetworkInternetPricePerGiB,
+					Price: DefaultNetworkInternetEgressPricePerGiB,
 				},
-			},
-		},
-		{
-			Properties: pricing.NetworkPricingProperties{
-				TrafficDirection: kubemodel.TrafficDirectionEgress,
-				TrafficType:      kubemodel.TrafficTypeInternet,
-				IsNatGateway:     true,
-			},
-			Prices: pricing.Prices{
-				pricing.ResourceNetworkTraffic: {
+				pricing.ResourceNATGatewayEgress: {
+					Unit:  unit.GiB,
+					Price: DefaultNetworkNATGatewayEgressPricePerGiB,
+				},
+				pricing.ResourceNATGatewayIngress: {
 					Unit:  unit.GiB,
-					Price: DefaultNetworkInternetPricePerGiB + DefaultNetworkNATPricePerGiB,
+					Price: DefaultNetworkNATGatewayIngressPricePerGiB,
 				},
 			},
 		},

+ 189 - 26
modules/pricing/basic/module.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"slices"
 	"sync"
 
 	"github.com/opencost/opencost/core/pkg/pricing"
@@ -15,6 +14,9 @@ import (
 // PricingModule must satisfy the pricing.PricingModule interface
 var _ pricing.PricingModule = (*PricingModule)(nil)
 
+const SourceKind = "basic"
+const SourceName = "basic"
+
 type PricingModule struct {
 	mu    sync.RWMutex
 	store pricing.PricingStore
@@ -82,22 +84,16 @@ func (pm *PricingModule) GetNetworkPricing(ctx context.Context, props pricing.Ne
 	pm.mu.RLock()
 	defer pm.mu.RUnlock()
 
-	nps, err := pm.getNetworkPricing(ctx)
+	np, err := pm.getNetworkPricing(ctx)
 	if err != nil {
 		return nil, err
 	}
 
-	// Search through the mock data for a matching network pricing entry
-	for _, np := range nps {
-		if np.Properties.Provider == props.Provider &&
-			np.Properties.TrafficDirection == props.TrafficDirection &&
-			np.Properties.TrafficType == props.TrafficType &&
-			np.Properties.IsNatGateway == props.IsNatGateway {
-			return np, nil
-		}
+	if np != nil {
+		return np, nil
 	}
-	return nil, fmt.Errorf("network pricing not found for provider=%s, trafficDirection=%s, trafficType=%s, isNatGateway=%t",
-		props.Provider, props.TrafficDirection, props.TrafficType, props.IsNatGateway)
+
+	return nil, errors.New("no network pricing")
 }
 
 func (pm *PricingModule) NewNetworkPricingReader(ctx context.Context) (reader.Reader[*pricing.NetworkPricing], error) {
@@ -109,7 +105,7 @@ func (pm *PricingModule) NewNetworkPricingReader(ctx context.Context) (reader.Re
 		return nil, fmt.Errorf("getting node pricing: %w", err)
 	}
 
-	return reader.NewSliceReader(slices.Clone(np)), nil
+	return reader.NewSliceReader([]*pricing.NetworkPricing{np}), nil
 }
 
 func (pm *PricingModule) GetNodePricing(ctx context.Context, props pricing.NodePricingProperties) (*pricing.NodePricing, error) {
@@ -204,11 +200,11 @@ func (pm *PricingModule) GetPricingSet(ctx context.Context) (*pricing.PricingSet
 }
 
 func (pm *PricingModule) SourceKind() string {
-	return "basic"
+	return SourceKind
 }
 
 func (pm *PricingModule) SourceName() string {
-	return "basic"
+	return SourceName
 }
 
 func (pm *PricingModule) Checksum(ctx context.Context) (string, error) {
@@ -227,6 +223,55 @@ func (pm *PricingModule) Checksum(ctx context.Context) (string, error) {
 
 // Public CRUD functions
 
+func (pm *PricingModule) SetClusterPricePerHour(ctx context.Context, price float64) error {
+	pm.mu.Lock()
+	defer pm.mu.Unlock()
+
+	return pm.setClusterPrice(ctx, pricing.ResourceCluster, unit.Hour, price)
+}
+
+func (pm *PricingModule) SetNetworkLocalEgressPricePerGiB(ctx context.Context, price float64) error {
+	pm.mu.Lock()
+	defer pm.mu.Unlock()
+
+	return pm.setNetworkPrice(ctx, pricing.ResourceLocalEgress, unit.GiB, price)
+}
+
+func (pm *PricingModule) SetNetworkCrossZoneEgressPricePerGiB(ctx context.Context, price float64) error {
+	pm.mu.Lock()
+	defer pm.mu.Unlock()
+
+	return pm.setNetworkPrice(ctx, pricing.ResourceCrossZoneEgress, unit.GiB, price)
+}
+
+func (pm *PricingModule) SetNetworkCrossRegionEgressPricePerGiB(ctx context.Context, price float64) error {
+	pm.mu.Lock()
+	defer pm.mu.Unlock()
+
+	return pm.setNetworkPrice(ctx, pricing.ResourceCrossRegionEgress, unit.GiB, price)
+}
+
+func (pm *PricingModule) SetNetworkInternetEgressPricePerGiB(ctx context.Context, price float64) error {
+	pm.mu.Lock()
+	defer pm.mu.Unlock()
+
+	return pm.setNetworkPrice(ctx, pricing.ResourceInternetEgress, unit.GiB, price)
+}
+
+func (pm *PricingModule) SetNetworkNATGatewayEgressPricePerGiB(ctx context.Context, price float64) error {
+	pm.mu.Lock()
+	defer pm.mu.Unlock()
+
+	return pm.setNetworkPrice(ctx, pricing.ResourceNATGatewayEgress, unit.GiB, price)
+}
+
+func (pm *PricingModule) SetNetworkNATGatewayIngressPricePerGiB(ctx context.Context, price float64) error {
+	pm.mu.Lock()
+	defer pm.mu.Unlock()
+
+	return pm.setNetworkPrice(ctx, pricing.ResourceNATGatewayIngress, unit.GiB, price)
+}
+
 func (pm *PricingModule) SetNodePricePerCPUCoreHour(ctx context.Context, price float64) error {
 	pm.mu.Lock()
 	defer pm.mu.Unlock()
@@ -255,15 +300,60 @@ func (pm *PricingModule) SetNodePricePerLocalDiskGiBHour(ctx context.Context, pr
 	return pm.setNodePrice(ctx, pricing.ResourceStorage, unit.GiBHour, price)
 }
 
-func (pm *PricingModule) SetVolumePricePerStorageGiBHour(ctx context.Context, price float64) error {
+func (pm *PricingModule) SetPersistentVolumePricePerStorageGiBHour(ctx context.Context, price float64) error {
 	pm.mu.Lock()
 	defer pm.mu.Unlock()
 
-	return pm.setVolumePrice(ctx, pricing.ResourceStorage, unit.GiBHour, price)
+	return pm.setPersistentVolumePrice(ctx, pricing.ResourceStorage, unit.GiBHour, price)
+}
+
+func (pm *PricingModule) SetServicePricePerHour(ctx context.Context, price float64) error {
+	pm.mu.Lock()
+	defer pm.mu.Unlock()
+
+	return pm.setServicePrice(ctx, pricing.ResourceService, unit.Hour, price)
 }
 
 // Private functions to set a price by resource and unit
 
+func (pm *PricingModule) setClusterPrice(ctx context.Context, resource pricing.Resource, unit unit.Unit, price float64) error {
+	cp, err := pm.getClusterPricing(ctx)
+	if err != nil {
+		return fmt.Errorf("getting cluster pricing: %w", err)
+	}
+
+	if cp.Prices == nil {
+		cp.Prices = pricing.Prices{}
+	}
+	cp.Prices[resource] = pricing.Price{Unit: unit, Price: price}
+
+	err = pm.setClusterPricing(ctx, cp)
+	if err != nil {
+		return fmt.Errorf("setting cluster pricing: %w", err)
+	}
+
+	return nil
+}
+
+func (pm *PricingModule) setNetworkPrice(ctx context.Context, resource pricing.Resource, unit unit.Unit, price float64) error {
+	np, err := pm.getNetworkPricing(ctx)
+	if err != nil {
+		return fmt.Errorf("getting network pricing: %w", err)
+	}
+
+	if np.Prices == nil {
+		np.Prices = pricing.Prices{}
+	}
+	np.Prices[resource] = pricing.Price{Unit: unit, Price: price}
+
+	err = pm.setNetworkPricing(ctx, np)
+	if err != nil {
+		return fmt.Errorf("setting network pricing: %w", err)
+	}
+
+	return nil
+}
+
 func (pm *PricingModule) setNodePrice(ctx context.Context, resource pricing.Resource, unit unit.Unit, price float64) error {
 	np, err := pm.getNodePricing(ctx)
 	if err != nil {
@@ -283,7 +373,7 @@ func (pm *PricingModule) setNodePrice(ctx context.Context, resource pricing.Reso
 	return nil
 }
 
-func (pm *PricingModule) setVolumePrice(ctx context.Context, resource pricing.Resource, unit unit.Unit, price float64) error {
+func (pm *PricingModule) setPersistentVolumePrice(ctx context.Context, resource pricing.Resource, unit unit.Unit, price float64) error {
 	vp, err := pm.getPersistentVolumePricing(ctx)
 	if err != nil {
 		return fmt.Errorf("getting volume pricing: %w", err)
@@ -302,6 +392,25 @@ func (pm *PricingModule) setVolumePrice(ctx context.Context, resource pricing.Re
 	return nil
 }
 
+func (pm *PricingModule) setServicePrice(ctx context.Context, resource pricing.Resource, unit unit.Unit, price float64) error {
+	sp, err := pm.getServicePricing(ctx)
+	if err != nil {
+		return fmt.Errorf("getting service pricing: %w", err)
+	}
+
+	if sp.Prices == nil {
+		sp.Prices = pricing.Prices{}
+	}
+	sp.Prices[resource] = pricing.Price{Unit: unit, Price: price}
+
+	err = pm.setServicePricing(ctx, sp)
+	if err != nil {
+		return fmt.Errorf("setting service pricing: %w", err)
+	}
+
+	return nil
+}
+
 // Private functions to get and set pricing
 
 func (pm *PricingModule) getClusterPricing(ctx context.Context) (*pricing.ClusterPricing, error) {
@@ -320,11 +429,29 @@ func (pm *PricingModule) getClusterPricing(ctx context.Context) (*pricing.Cluste
 }
 
 func (pm *PricingModule) setClusterPricing(ctx context.Context, cp *pricing.ClusterPricing) error {
-	// TODO
-	return errors.New("not implemented")
+	if cp == nil {
+		return errors.New("nil cluster pricing")
+	}
+
+	// Get the pricing set
+	ps, err := pm.store.GetPricingSet(ctx)
+	if err != nil {
+		return fmt.Errorf("getting pricing: %w", err)
+	}
+
+	// Only one default ClusterPricing is allowed in basic pricing.
+	ps.ClusterPricing = []*pricing.ClusterPricing{cp}
+
+	// Set the new pricing set
+	err = pm.store.SetPricingSet(ctx, ps)
+	if err != nil {
+		return fmt.Errorf("setting pricing: %w", err)
+	}
+
+	return nil
 }
 
-func (pm *PricingModule) getNetworkPricing(ctx context.Context) ([]*pricing.NetworkPricing, error) {
+func (pm *PricingModule) getNetworkPricing(ctx context.Context) (*pricing.NetworkPricing, error) {
 	ps, err := pm.store.GetPricingSet(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("getting pricing: %w", err)
@@ -334,12 +461,30 @@ func (pm *PricingModule) getNetworkPricing(ctx context.Context) ([]*pricing.Netw
 		return nil, errors.New("not found")
 	}
 
-	return ps.NetworkPricing, nil
+	return ps.NetworkPricing[0], nil
 }
 
 func (pm *PricingModule) setNetworkPricing(ctx context.Context, np *pricing.NetworkPricing) error {
-	// TODO
-	return errors.New("not implemented")
+	if np == nil {
+		return errors.New("nil network pricing")
+	}
+
+	// Get the pricing set
+	ps, err := pm.store.GetPricingSet(ctx)
+	if err != nil {
+		return fmt.Errorf("getting pricing: %w", err)
+	}
+
+	// Only one default NetworkPricing is allowed in basic pricing.
+	ps.NetworkPricing = []*pricing.NetworkPricing{np}
+
+	// Set the new pricing set
+	err = pm.store.SetPricingSet(ctx, ps)
+	if err != nil {
+		return fmt.Errorf("setting pricing: %w", err)
+	}
+
+	return nil
 }
 
 func (pm *PricingModule) getNodePricing(ctx context.Context) (*pricing.NodePricing, error) {
@@ -434,6 +579,24 @@ func (pm *PricingModule) getServicePricing(ctx context.Context) (*pricing.Servic
 }
 
 func (pm *PricingModule) setServicePricing(ctx context.Context, sp *pricing.ServicePricing) error {
-	// TODO
-	return errors.New("not implemented")
+	if sp == nil {
+		return errors.New("nil service pricing")
+	}
+
+	// Get the pricing set
+	ps, err := pm.store.GetPricingSet(ctx)
+	if err != nil {
+		return fmt.Errorf("getting pricing: %w", err)
+	}
+
+	// Only one default ServicePricing is allowed in basic pricing.
+	ps.ServicePricing = []*pricing.ServicePricing{sp}
+
+	// Set the new pricing set
+	err = pm.store.SetPricingSet(ctx, ps)
+	if err != nil {
+		return fmt.Errorf("setting pricing: %w", err)
+	}
+
+	return nil
 }

+ 320 - 8
modules/pricing/basic/module_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/opencost/opencost/core/pkg/pricing"
 	"github.com/opencost/opencost/core/pkg/reader"
 	"github.com/opencost/opencost/core/pkg/storage"
+	"github.com/opencost/opencost/core/pkg/unit"
 	"github.com/stretchr/testify/require"
 )
 
@@ -55,6 +56,34 @@ func testPricingModuleWithStore(store pricing.PricingStore) func(t *testing.T) {
 			testDefaultPricing(t, ctx, pm)
 		})
 
+		t.Run("Metadata", func(t *testing.T) {
+			testMetadata(t, pm)
+		})
+
+		t.Run("GetPricingSet", func(t *testing.T) {
+			testGetPricingSet(t, ctx, pm)
+		})
+
+		t.Run("PublicGetters", func(t *testing.T) {
+			testPublicGetters(t, ctx, pm)
+		})
+
+		t.Run("SetClusterPricePerHour", func(t *testing.T) {
+			testSetClusterPricePerHour(t, ctx, pm)
+		})
+
+		t.Run("SetNetworkPrices", func(t *testing.T) {
+			testSetNetworkPrices(t, ctx, pm)
+		})
+
+		t.Run("SetServicePricePerHour", func(t *testing.T) {
+			testSetServicePricePerHour(t, ctx, pm)
+		})
+
+		t.Run("Checksum", func(t *testing.T) {
+			testChecksum(t, ctx, pm)
+		})
+
 		t.Run("SetNodePricePerCPUCoreHour", func(t *testing.T) {
 			testSetNodePricePerCPUCoreHour(t, ctx, pm)
 		})
@@ -83,20 +112,43 @@ func testPricingModuleWithStore(store pricing.PricingStore) func(t *testing.T) {
 			testNewVolumePricingReader(t, ctx, pm)
 		})
 
+		t.Run("NewClusterPricingReader", func(t *testing.T) {
+			testNewClusterPricingReader(t, ctx, pm)
+		})
+
+		t.Run("NewNetworkPricingReader", func(t *testing.T) {
+			testNewNetworkPricingReader(t, ctx, pm)
+		})
+
+		t.Run("NewServicePricingReader", func(t *testing.T) {
+			testNewServicePricingReader(t, ctx, pm)
+		})
+
 		t.Run("ModulePersistence", func(t *testing.T) {
 			// Create a new PricingModule with the same store
 			pm2, err := NewBasicPricingModule(store)
 			require.NoError(t, err)
 
-			// Verify that pricing persists
+			// Verify that pricing persists across all resource kinds.
 			np, err := pm2.getNodePricing(ctx)
-			if err != nil {
-				t.Fatalf("Failed to get node pricing: %v", err)
-			}
+			require.NoError(t, err, "getting node pricing")
+			require.NotNil(t, np, "expected node pricing to be persisted")
 
-			if np == nil {
-				t.Fatal("Expected node pricing to be persisted")
-			}
+			cp, err := pm2.getClusterPricing(ctx)
+			require.NoError(t, err, "getting cluster pricing")
+			require.NotNil(t, cp, "expected cluster pricing to be persisted")
+
+			netp, err := pm2.getNetworkPricing(ctx)
+			require.NoError(t, err, "getting network pricing")
+			require.NotNil(t, netp, "expected network pricing to be persisted")
+
+			vp, err := pm2.getPersistentVolumePricing(ctx)
+			require.NoError(t, err, "getting volume pricing")
+			require.NotNil(t, vp, "expected volume pricing to be persisted")
+
+			sp, err := pm2.getServicePricing(ctx)
+			require.NoError(t, err, "getting service pricing")
+			require.NotNil(t, sp, "expected service pricing to be persisted")
 		})
 	}
 }
@@ -152,6 +204,50 @@ func testDefaultPricing(t *testing.T, ctx context.Context, pm *PricingModule) {
 	if volumePrice.Price != DefaultPersistentVolumePricePerGiBHour {
 		t.Errorf("Expected volume price to be %f, got %f", DefaultPersistentVolumePricePerGiBHour, volumePrice.Price)
 	}
+
+	// Test default cluster pricing
+	cp, err := pm.getClusterPricing(ctx)
+	require.NoError(t, err, "getting cluster pricing")
+	require.NotNil(t, cp, "expected cluster pricing to exist")
+
+	clusterPrice, ok := cp.Prices[pricing.ResourceCluster]
+	require.True(t, ok, "expected to find cluster pricing")
+	require.Equal(t, DefaultClusterPricePerHour, clusterPrice.Price)
+	require.Equal(t, unit.Hour, clusterPrice.Unit)
+
+	// Test default network pricing. RAM, egress types, etc. all share the GiB
+	// unit, so the Resource key distinguishes them.
+	netp, err := pm.getNetworkPricing(ctx)
+	require.NoError(t, err, "getting network pricing")
+	require.NotNil(t, netp, "expected network pricing to exist")
+
+	networkChecks := []struct {
+		resource pricing.Resource
+		want     float64
+	}{
+		{pricing.ResourceLocalEgress, DefaultNetworkLocalEgressPricePerGiB},
+		{pricing.ResourceCrossZoneEgress, DefaultNetworkCrossZoneEgressPricePerGiB},
+		{pricing.ResourceCrossRegionEgress, DefaultNetworkCrossRegionEgressPricePerGiB},
+		{pricing.ResourceInternetEgress, DefaultNetworkInternetEgressPricePerGiB},
+		{pricing.ResourceNATGatewayEgress, DefaultNetworkNATGatewayEgressPricePerGiB},
+		{pricing.ResourceNATGatewayIngress, DefaultNetworkNATGatewayIngressPricePerGiB},
+	}
+	for _, c := range networkChecks {
+		price, ok := netp.Prices[c.resource]
+		require.Truef(t, ok, "expected to find %s pricing", c.resource)
+		require.Equalf(t, c.want, price.Price, "price for %s", c.resource)
+		require.Equalf(t, unit.GiB, price.Unit, "unit for %s", c.resource)
+	}
+
+	// Test default service pricing
+	sp, err := pm.getServicePricing(ctx)
+	require.NoError(t, err, "getting service pricing")
+	require.NotNil(t, sp, "expected service pricing to exist")
+
+	servicePrice, ok := sp.Prices[pricing.ResourceService]
+	require.True(t, ok, "expected to find service pricing")
+	require.Equal(t, DefaultServicePricePerHour, servicePrice.Price)
+	require.Equal(t, unit.Hour, servicePrice.Unit)
 }
 
 // testSetNodePricePerCPUCoreHour tests the SetNodePricePerCPUCoreHour function
@@ -254,7 +350,7 @@ func testSetNodePricePerLocalDiskGiBHour(t *testing.T, ctx context.Context, pm *
 func testSetVolumePricePerStorageGiBHour(t *testing.T, ctx context.Context, pm *PricingModule) {
 	newPrice := 0.0003
 
-	err := pm.SetVolumePricePerStorageGiBHour(ctx, newPrice)
+	err := pm.SetPersistentVolumePricePerStorageGiBHour(ctx, newPrice)
 	if err != nil {
 		t.Fatalf("Failed to set volume storage price: %v", err)
 	}
@@ -364,6 +460,222 @@ func testNewVolumePricingReader(t *testing.T, ctx context.Context, pm *PricingMo
 	}
 }
 
+// testMetadata verifies the source identity accessors.
+func testMetadata(t *testing.T, pm *PricingModule) {
+	require.Equal(t, SourceKind, pm.SourceKind())
+	require.Equal(t, SourceName, pm.SourceName())
+}
+
+// testGetPricingSet verifies GetPricingSet returns a populated set.
+func testGetPricingSet(t *testing.T, ctx context.Context, pm *PricingModule) {
+	ps, err := pm.GetPricingSet(ctx)
+	require.NoError(t, err)
+	require.NotNil(t, ps)
+	require.False(t, ps.IsEmpty())
+}
+
+// testPublicGetters exercises the public Get*Pricing wrappers, which apply a
+// nil-check around the private getters.
+func testPublicGetters(t *testing.T, ctx context.Context, pm *PricingModule) {
+	cp, err := pm.GetClusterPricing(ctx, pricing.ClusterPricingProperties{})
+	require.NoError(t, err)
+	require.NotNil(t, cp)
+
+	netp, err := pm.GetNetworkPricing(ctx, pricing.NetworkPricingProperties{})
+	require.NoError(t, err)
+	require.NotNil(t, netp)
+
+	np, err := pm.GetNodePricing(ctx, pricing.NodePricingProperties{})
+	require.NoError(t, err)
+	require.NotNil(t, np)
+
+	vp, err := pm.GetPersistentVolumePricing(ctx, pricing.PersistentVolumePricingProperties{})
+	require.NoError(t, err)
+	require.NotNil(t, vp)
+
+	sp, err := pm.GetServicePricing(ctx, pricing.ServicePricingProperties{})
+	require.NoError(t, err)
+	require.NotNil(t, sp)
+}
+
+// testSetClusterPricePerHour tests the SetClusterPricePerHour function.
+func testSetClusterPricePerHour(t *testing.T, ctx context.Context, pm *PricingModule) {
+	newPrice := 1.25
+
+	err := pm.SetClusterPricePerHour(ctx, newPrice)
+	require.NoError(t, err)
+
+	cp, err := pm.getClusterPricing(ctx)
+	require.NoError(t, err)
+
+	price, ok := cp.Prices[pricing.ResourceCluster]
+	require.True(t, ok, "expected to find cluster pricing")
+	require.Equal(t, newPrice, price.Price)
+	require.Equal(t, unit.Hour, price.Unit)
+}
+
+// testSetNetworkPrices tests each network setter. Each case verifies that only
+// the targeted Resource changes and that the other network prices are left
+// intact, guarding against a setter writing the wrong Resource key.
+func testSetNetworkPrices(t *testing.T, ctx context.Context, pm *PricingModule) {
+	cases := []struct {
+		name     string
+		resource pricing.Resource
+		newPrice float64
+		set      func(context.Context, float64) error
+	}{
+		{"LocalEgress", pricing.ResourceLocalEgress, 0.002, pm.SetNetworkLocalEgressPricePerGiB},
+		{"CrossZoneEgress", pricing.ResourceCrossZoneEgress, 0.012, pm.SetNetworkCrossZoneEgressPricePerGiB},
+		{"CrossRegionEgress", pricing.ResourceCrossRegionEgress, 0.022, pm.SetNetworkCrossRegionEgressPricePerGiB},
+		{"InternetEgress", pricing.ResourceInternetEgress, 0.111, pm.SetNetworkInternetEgressPricePerGiB},
+		{"NATGatewayEgress", pricing.ResourceNATGatewayEgress, 0.055, pm.SetNetworkNATGatewayEgressPricePerGiB},
+		{"NATGatewayIngress", pricing.ResourceNATGatewayIngress, 0.066, pm.SetNetworkNATGatewayIngressPricePerGiB},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			// Snapshot existing prices so we can verify non-targeted
+			// resources are untouched.
+			before, err := pm.getNetworkPricing(ctx)
+			require.NoError(t, err)
+			prev := make(map[pricing.Resource]float64, len(before.Prices))
+			for r, p := range before.Prices {
+				prev[r] = p.Price
+			}
+
+			require.NoError(t, c.set(ctx, c.newPrice))
+
+			after, err := pm.getNetworkPricing(ctx)
+			require.NoError(t, err)
+
+			price, ok := after.Prices[c.resource]
+			require.Truef(t, ok, "expected to find %s pricing", c.resource)
+			require.Equal(t, c.newPrice, price.Price)
+			require.Equal(t, unit.GiB, price.Unit)
+
+			// All other network resources must be unchanged.
+			for r, want := range prev {
+				if r == c.resource {
+					continue
+				}
+				require.Equalf(t, want, after.Prices[r].Price, "unexpected change to %s", r)
+			}
+		})
+	}
+}
+
+// testSetServicePricePerHour tests the SetServicePricePerHour function.
+func testSetServicePricePerHour(t *testing.T, ctx context.Context, pm *PricingModule) {
+	newPrice := 0.05
+
+	err := pm.SetServicePricePerHour(ctx, newPrice)
+	require.NoError(t, err)
+
+	sp, err := pm.getServicePricing(ctx)
+	require.NoError(t, err)
+
+	price, ok := sp.Prices[pricing.ResourceService]
+	require.True(t, ok, "expected to find service pricing")
+	require.Equal(t, newPrice, price.Price)
+	require.Equal(t, unit.Hour, price.Unit)
+}
+
+// testChecksum verifies the checksum is non-empty, deterministic, and sensitive
+// to pricing mutations.
+func testChecksum(t *testing.T, ctx context.Context, pm *PricingModule) {
+	sum1, err := pm.Checksum(ctx)
+	require.NoError(t, err)
+	require.NotEmpty(t, sum1)
+
+	// Deterministic: recomputing without mutation yields the same checksum.
+	sum2, err := pm.Checksum(ctx)
+	require.NoError(t, err)
+	require.Equal(t, sum1, sum2)
+
+	// Sensitive: mutating pricing changes the checksum.
+	require.NoError(t, pm.SetServicePricePerHour(ctx, sum1Sentinel))
+
+	sum3, err := pm.Checksum(ctx)
+	require.NoError(t, err)
+	require.NotEqual(t, sum1, sum3, "expected checksum to change after a price mutation")
+}
+
+// sum1Sentinel is an arbitrary price unlikely to match the existing service
+// price, used to force a checksum change.
+const sum1Sentinel = 0.0419
+
+// testNewClusterPricingReader verifies the cluster pricing reader yields exactly
+// one non-nil element.
+func testNewClusterPricingReader(t *testing.T, ctx context.Context, pm *PricingModule) {
+	rdr, err := pm.NewClusterPricingReader(ctx)
+	require.NoError(t, err)
+	require.NotNil(t, rdr)
+
+	dst := make([]*pricing.ClusterPricing, 10)
+	count := 0
+	for {
+		n, err := rdr.Read(ctx, dst)
+		count += n
+		for i := 0; i < n; i++ {
+			require.NotNil(t, dst[i], "expected non-nil ClusterPricing")
+		}
+		if err == reader.Done {
+			break
+		}
+		require.NoError(t, err)
+	}
+	require.Equal(t, 1, count, "expected exactly 1 ClusterPricing")
+	require.NoError(t, rdr.Close())
+}
+
+// testNewNetworkPricingReader verifies the network pricing reader yields exactly
+// one non-nil element.
+func testNewNetworkPricingReader(t *testing.T, ctx context.Context, pm *PricingModule) {
+	rdr, err := pm.NewNetworkPricingReader(ctx)
+	require.NoError(t, err)
+	require.NotNil(t, rdr)
+
+	dst := make([]*pricing.NetworkPricing, 10)
+	count := 0
+	for {
+		n, err := rdr.Read(ctx, dst)
+		count += n
+		for i := 0; i < n; i++ {
+			require.NotNil(t, dst[i], "expected non-nil NetworkPricing")
+		}
+		if err == reader.Done {
+			break
+		}
+		require.NoError(t, err)
+	}
+	require.Equal(t, 1, count, "expected exactly 1 NetworkPricing")
+	require.NoError(t, rdr.Close())
+}
+
+// testNewServicePricingReader verifies the service pricing reader yields exactly
+// one non-nil element.
+func testNewServicePricingReader(t *testing.T, ctx context.Context, pm *PricingModule) {
+	rdr, err := pm.NewServicePricingReader(ctx)
+	require.NoError(t, err)
+	require.NotNil(t, rdr)
+
+	dst := make([]*pricing.ServicePricing, 10)
+	count := 0
+	for {
+		n, err := rdr.Read(ctx, dst)
+		count += n
+		for i := 0; i < n; i++ {
+			require.NotNil(t, dst[i], "expected non-nil ServicePricing")
+		}
+		if err == reader.Done {
+			break
+		}
+		require.NoError(t, err)
+	}
+	require.Equal(t, 1, count, "expected exactly 1 ServicePricing")
+	require.NoError(t, rdr.Close())
+}
+
 func newFileStorage(t *testing.T) storage.Storage {
 	tempDir, err := os.MkdirTemp("", "pricing-test-*")
 	if err != nil {

+ 2 - 6
modules/pricing/public/module.go

@@ -261,16 +261,12 @@ func (pm *PricingModule) GetNetworkPricing(ctx context.Context, props pricing.Ne
 	}
 
 	for _, np := range pm.pricingSet.NetworkPricing {
-		if np.Properties.Provider == props.Provider &&
-			np.Properties.TrafficDirection == props.TrafficDirection &&
-			np.Properties.TrafficType == props.TrafficType &&
-			np.Properties.IsNatGateway == props.IsNatGateway {
+		if np.Properties.Provider == props.Provider {
 			return np, nil
 		}
 	}
 
-	return nil, fmt.Errorf("network pricing not found for provider=%s, trafficDirection=%s, trafficType=%s, isNatGateway=%t",
-		props.Provider, props.TrafficDirection, props.TrafficType, props.IsNatGateway)
+	return nil, fmt.Errorf("network pricing not found for provider=%s", props.Provider)
 }
 
 func (pm *PricingModule) NewNetworkPricingReader(ctx context.Context) (reader.Reader[*pricing.NetworkPricing], error) {