fix: default handling in custom provider (#3906)

Signed-off-by: York Chen <york.chen@nutanix.com>
Co-authored-by: Alex Meijer <ameijer@users.noreply.github.com>

configs/default.json

@@ -12,4 +12,4 @@
     "internetNetworkEgress": "0.12",
     "natGatewayEgress": "0.045",
     "natGatewayIngress": "0.045"
-}
+}

pkg/cloud/provider/customprovider.go

@@ -137,14 +137,25 @@ func (cp *CustomProvider) ClusterInfo() (map[string]string, error) {
 	if err != nil {
 		return nil, err
 	}
-	m := make(map[string]string)
-	if conf.ClusterName != "" {
-		m["name"] = conf.ClusterName
+
+	const defaultClusterName = "Custom Cluster"
+	clusterID := coreenv.GetClusterID()
+	if clusterID == "" {
+		clusterID = "default-cluster"
+	}
+	clusterName := conf.ClusterName
+	if clusterName == "" {
+		if clusterName = coreenv.GetClusterID(); clusterName == "" {
+			clusterName = defaultClusterName
+		}
 	}
+
+	m := make(map[string]string)
+	m["name"] = clusterName
 	m["provider"] = opencost.CustomProvider
 	m["region"] = cp.ClusterRegion
 	m["account"] = cp.ClusterAccountID
-	m["id"] = coreenv.GetClusterID()
+	m["id"] = clusterID
 	return m, nil
 }
 
@@ -325,20 +336,39 @@ func (cp *CustomProvider) NetworkPricing() (*models.Network, error) {
 	}, nil
 }
 
+func parsePriceOrZero(field, value string) (float64, error) {
+	if value == "" {
+		return 0, nil
+	}
+	price, err := strconv.ParseFloat(value, 64)
+	if err != nil {
+		return 0, fmt.Errorf("invalid custom pricing value %q for %s: %w", value, field, err)
+	}
+	return price, nil
+}
+
 func (cp *CustomProvider) LoadBalancerPricing() (*models.LoadBalancer, error) {
 	cpricing, err := cp.Config.GetCustomPricingData()
 	if err != nil {
 		return nil, err
 	}
-	fffrc, err := strconv.ParseFloat(cpricing.FirstFiveForwardingRulesCost, 64)
+
+	firstFiveForwardingRulesCostField := "firstFiveForwardingRulesCost"
+	firstFiveForwardingRulesCost := cpricing.FirstFiveForwardingRulesCost
+	if firstFiveForwardingRulesCost == "" && cpricing.DefaultLBPrice != "" {
+		firstFiveForwardingRulesCostField = "defaultLBPrice"
+		firstFiveForwardingRulesCost = cpricing.DefaultLBPrice
+	}
+
+	fffrc, err := parsePriceOrZero(firstFiveForwardingRulesCostField, firstFiveForwardingRulesCost)
 	if err != nil {
 		return nil, err
 	}
-	afrc, err := strconv.ParseFloat(cpricing.AdditionalForwardingRuleCost, 64)
+	afrc, err := parsePriceOrZero("additionalForwardingRuleCost", cpricing.AdditionalForwardingRuleCost)
 	if err != nil {
 		return nil, err
 	}
-	lbidc, err := strconv.ParseFloat(cpricing.LBIngressDataCost, 64)
+	lbidc, err := parsePriceOrZero("LBIngressDataCost", cpricing.LBIngressDataCost)
 	if err != nil {
 		return nil, err
 	}

pkg/cloud/provider/provider_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/opencost/opencost/core/pkg/clustercache"
+	coreenv "github.com/opencost/opencost/core/pkg/env"
 	"github.com/opencost/opencost/core/pkg/storage"
 	"github.com/opencost/opencost/pkg/config"
 	v1 "k8s.io/api/core/v1"
@@ -205,3 +206,89 @@ func TestCustomProviderNodePricingUsesDetectedGPUCount(t *testing.T) {
 		t.Errorf("GPU = %q, want %q", node.GPU, "2")
 	}
 }
+
+func TestCustomProviderClusterInfoUsesStaticDefaultName(t *testing.T) {
+	t.Setenv(coreenv.ClusterIDEnvVar, "")
+
+	customProvider := newTestCustomProvider(t, nil)
+
+	info, err := customProvider.ClusterInfo()
+	if err != nil {
+		t.Fatalf("ClusterInfo returned error: %v", err)
+	}
+
+	if info["name"] != "Custom Cluster" {
+		t.Errorf("name = %q, want %q", info["name"], "Custom Cluster")
+	}
+	if info["id"] != "default-cluster" {
+		t.Errorf("id = %q, want %q", info["id"], "default-cluster")
+	}
+}
+
+func TestCustomProviderLoadBalancerPricingEmptyConfig(t *testing.T) {
+	customProvider := newTestCustomProvider(t, nil)
+
+	lb, err := customProvider.LoadBalancerPricing()
+	if err != nil {
+		t.Fatalf("LoadBalancerPricing returned error: %v", err)
+	}
+	if lb.Cost != 0 {
+		t.Errorf("Cost = %f, want 0", lb.Cost)
+	}
+}
+
+func TestCustomProviderLoadBalancerPricingUsesDefaultLBPriceFallback(t *testing.T) {
+	customProvider := newTestCustomProvider(t, map[string]string{
+		"defaultLBPrice": "0.025",
+	})
+
+	lb, err := customProvider.LoadBalancerPricing()
+	if err != nil {
+		t.Fatalf("LoadBalancerPricing returned error: %v", err)
+	}
+	if lb.Cost != 0.025 {
+		t.Errorf("Cost = %f, want 0.025", lb.Cost)
+	}
+}
+
+func TestCustomProviderLoadBalancerPricingUsesForwardingRulePrice(t *testing.T) {
+	customProvider := newTestCustomProvider(t, map[string]string{
+		"firstFiveForwardingRulesCost": "0.02",
+		"defaultLBPrice":               "0.025",
+	})
+
+	lb, err := customProvider.LoadBalancerPricing()
+	if err != nil {
+		t.Fatalf("LoadBalancerPricing returned error: %v", err)
+	}
+	if lb.Cost != 0.02 {
+		t.Errorf("Cost = %f, want 0.02", lb.Cost)
+	}
+}
+
+func TestCustomProviderLoadBalancerPricingInvalidValue(t *testing.T) {
+	customProvider := newTestCustomProvider(t, map[string]string{
+		"firstFiveForwardingRulesCost": "not-a-price",
+	})
+
+	_, err := customProvider.LoadBalancerPricing()
+	if err == nil {
+		t.Fatal("LoadBalancerPricing returned nil error, want invalid pricing error")
+	}
+}
+
+func newTestCustomProvider(t *testing.T, pricing map[string]string) *CustomProvider {
+	t.Helper()
+
+	confMan := config.NewConfigFileManager(storage.NewMemoryStorage())
+	providerConfig := NewProviderConfig(confMan, "default.json")
+	if pricing != nil {
+		if _, err := providerConfig.UpdateFromMap(pricing); err != nil {
+			t.Fatalf("UpdateFromMap returned error: %v", err)
+		}
+	}
+
+	return &CustomProvider{
+		Config: providerConfig,
+	}
+}