Merge branch 'develop' into sean/s3-memory

.github/PULL_REQUEST_TEMPLATE.md

@@ -16,5 +16,5 @@
 ## Does this PR require changes to documentation?
 * 
 
-## Have you labeled this PR and its corresponding Issue as "next release" if it should be part of the next Opencost release? If not, why not?
+## Have you labeled this PR and its corresponding Issue as "next release" if it should be part of the next OpenCost release? If not, why not?
 * 

pkg/cloud/aws/provider.go

@@ -534,6 +534,12 @@ func (aws *AWS) UpdateConfig(r io.Reader, updateType string) (*models.CustomPric
 				return err
 			}
 
+			// If the sample nil service key name is set, zero it out so that it is not
+			// misinterpreted as a real service key.
+			if asfi.ServiceKeyName == "AKIXXX" {
+				asfi.ServiceKeyName = ""
+			}
+
 			c.ServiceKeyName = asfi.ServiceKeyName
 			if asfi.ServiceKeySecret != "" {
 				c.ServiceKeySecret = asfi.ServiceKeySecret
@@ -551,6 +557,13 @@ func (aws *AWS) UpdateConfig(r io.Reader, updateType string) (*models.CustomPric
 			if err != nil {
 				return err
 			}
+
+			// If the sample nil service key name is set, zero it out so that it is not
+			// misinterpreted as a real service key.
+			if aai.ServiceKeyName == "AKIXXX" {
+				aai.ServiceKeyName = ""
+			}
+
 			c.AthenaBucketName = aai.AthenaBucketName
 			c.AthenaRegion = aai.AthenaRegion
 			c.AthenaDatabase = aai.AthenaDatabase
@@ -1401,7 +1414,6 @@ func (aws *AWS) ConfigureAuthWith(config *models.CustomPricing) error {
 
 // Gets the aws key id and secret
 func (aws *AWS) getAWSAuth(forceReload bool, cp *models.CustomPricing) (string, string) {
-
 	// 1. Check config values first (set from frontend UI)
 	if cp.ServiceKeyName != "" && cp.ServiceKeySecret != "" {
 		aws.ServiceAccountChecks.Set("hasKey", &models.ServiceAccountCheck{
@@ -1461,6 +1473,12 @@ func (aws *AWS) loadAWSAuthSecret(force bool) (*AWSAccessKey, error) {
 		return nil, err
 	}
 
+	// If the sample nil service key name is set, zero it out so that it is not
+	// misinterpreted as a real service key.
+	if ak.AccessKeyID == "AKIXXX" {
+		ak.AccessKeyID = ""
+	}
+
 	awsSecret = &ak
 	return awsSecret, nil
 }

pkg/cloud/provider/provider.go

@@ -162,6 +162,13 @@ func NewProvider(cache clustercache.ClusterCache, apiKey string, config *config.
 		cp.accountID = providerConfig.customPricing.ClusterAccountID
 	}
 
+	providerConfig.Update(func(cp *models.CustomPricing) error {
+		if cp.ServiceKeyName == "AKIXXX" {
+			cp.ServiceKeyName = ""
+		}
+		return nil
+	})
+
 	switch cp.provider {
 	case kubecost.CSVProvider:
 		log.Infof("Using CSV Provider with CSV at %s", env.GetCSVPath())

pkg/cloud/provider/providerconfig.go

@@ -143,6 +143,12 @@ func (pc *ProviderConfig) loadConfig(writeIfNotExists bool) (*models.CustomPrici
 		pc.customPricing.ShareTenancyCosts = models.DefaultShareTenancyCost
 	}
 
+	// If the sample nil service key name is set, zero it out so that it is not
+	// misinterpreted as a real service key.
+	if pc.customPricing.ServiceKeyName == "AKIXXX" {
+		pc.customPricing.ServiceKeyName = ""
+	}
+
 	return pc.customPricing, nil
 }
 

pkg/env/costmodelenv.go

@@ -215,7 +215,13 @@ func IsEmitKsmV1MetricsOnly() bool {
 // GetAWSAccessKeyID returns the environment variable value for AWSAccessKeyIDEnvVar which represents
 // the AWS access key for authentication
 func GetAWSAccessKeyID() string {
-	return Get(AWSAccessKeyIDEnvVar, "")
+	awsAccessKeyID := Get(AWSAccessKeyIDEnvVar, "")
+	// If the sample nil service key name is set, zero it out so that it is not
+	// misinterpreted as a real service key.
+	if awsAccessKeyID == "AKIXXX" {
+		awsAccessKeyID = ""
+	}
+	return awsAccessKeyID
 }
 
 // GetAWSAccessKeySecret returns the environment variable value for AWSAccessKeySecretEnvVar which represents

pkg/kubecost/allocation.go

@@ -9,6 +9,7 @@ import (
 	"github.com/opencost/opencost/pkg/log"
 	"github.com/opencost/opencost/pkg/util"
 	"github.com/opencost/opencost/pkg/util/timeutil"
+	"golang.org/x/exp/slices"
 )
 
 // TODO Clean-up use of IsEmpty; nil checks should be separated for safety.
@@ -1045,6 +1046,8 @@ type AllocationAggregationOptions struct {
 	Reconcile                             bool
 	ReconcileNetwork                      bool
 	ShareFuncs                            []AllocationMatchFunc
+	SharedNamespaces                      []string
+	SharedLabels                          map[string][]string
 	ShareIdle                             string
 	ShareSplit                            string
 	SharedHourlyCosts                     map[string]float64
@@ -1539,7 +1542,12 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 					if alloc.SharedCostBreakdown == nil {
 						alloc.SharedCostBreakdown = map[string]SharedCostBreakdown{}
 					}
-					sharedCostName := sharedAlloc.generateKey(aggregateBy, options.LabelConfig)
+
+					sharedCostName, err := sharedAlloc.determineSharingName(options)
+					if err != nil {
+						return fmt.Errorf("failed to group shared costs: %s", err)
+					}
+
 					// check if current allocation is a shared flat overhead cost
 					if strings.Contains(sharedAlloc.Name, SharedSuffix) {
 						sharedCostName = "overheadCost"
@@ -1979,6 +1987,69 @@ func deriveProportionalAssetResourceCosts(options *AllocationAggregationOptions,
 	return nil
 }
 
+func (a *Allocation) determineSharingName(options *AllocationAggregationOptions) (string, error) {
+	if a == nil {
+		return "", fmt.Errorf("determineSharingName called on nil Allocation")
+	} else if options == nil {
+		return "unknown", nil
+	}
+
+	// grab SharedLabels keys and sort them, to keep this function deterministic
+	var labelKeys []string
+	for labelKey, _ := range options.SharedLabels {
+		labelKeys = append(labelKeys, labelKey)
+	}
+	slices.Sort(labelKeys)
+
+	var sharedAggregateBy []string
+	var sharedLabels [][]string
+	for _, labelKey := range labelKeys {
+		sharedAgg := fmt.Sprintf("label:%s", labelKey)
+		if !slices.Contains(sharedAggregateBy, sharedAgg) {
+			sharedAggregateBy = append(sharedAggregateBy, sharedAgg)
+		}
+		sharedLabels = append(sharedLabels, options.SharedLabels[labelKey])
+	}
+	if len(options.SharedNamespaces) > 0 {
+		sharedAggregateBy = append(sharedAggregateBy, "namespace")
+	}
+	sharedCostName := a.generateKey(sharedAggregateBy, options.LabelConfig)
+
+	// get each value in the generated key, then reset the name
+	sharedCostNameValues := strings.Split(sharedCostName, "/")
+	sharedCostName = ""
+
+	// if we don't have as many values as aggregateBys, something went wrong in generateKey
+	if len(sharedCostNameValues) != len(sharedAggregateBy) {
+		log.Warnf("Unable to determine share cost group for allocation \"%s\"", a.Name)
+	} else {
+		// try to match to the first label
+		for i, sharedLabelValues := range sharedLabels {
+			allocLabel := sharedCostNameValues[i]
+			if slices.Contains(sharedLabelValues, allocLabel) {
+				return allocLabel, nil
+			}
+		}
+
+		// if we didn't match to a label, try to match to a namespace
+		if len(options.SharedNamespaces) > 0 {
+			// namespace will always be the last value, if SharedNamespaces is set
+			allocNamespace := sharedCostNameValues[len(sharedCostNameValues)-1]
+			if slices.Contains(options.SharedNamespaces, allocNamespace) {
+				return allocNamespace, nil
+			}
+		}
+
+		// if neither the labels nor the namespaces matched, we log a warning and mark this allocation
+		// as unknown
+		if len(sharedCostName) == 0 {
+			log.Warnf("Failed to determine shared cost grouping for allocation \"%s\"", a.Name)
+		}
+	}
+
+	return "unknown", nil
+}
+
 // getIdleId returns the providerId or cluster of an Allocation depending on the IdleByNode
 // option in the AllocationAggregationOptions and an error if the respective field is missing
 func (a *Allocation) getIdleId(options *AllocationAggregationOptions) (string, error) {

pkg/kubecost/allocation_test.go

@@ -3214,3 +3214,202 @@ func Test_AggregateByService_UnmountedLBs(t *testing.T) {
 	spew.Config.DisableMethods = true
 	t.Logf("%s", spew.Sdump(set.Allocations))
 }
+
+func Test_DetermineSharingName(t *testing.T) {
+	var alloc *Allocation
+	var name string
+	var err error
+
+	// test nil allocation with nil options
+	name, err = alloc.determineSharingName(nil)
+	if err == nil {
+		t.Fatalf("determineSharingName: expected error; actual nil")
+	}
+
+	// test nil with non-nil options
+	name, err = alloc.determineSharingName(&AllocationAggregationOptions{})
+	if err == nil {
+		t.Fatalf("determineSharingName: expected error; actual nil")
+	}
+
+	alloc = &Allocation{}
+	alloc.Properties = &AllocationProperties{
+		Cluster: "cluster1",
+		Labels: map[string]string{
+			"app": "app1",
+			"env": "env1",
+		},
+		Namespace: "namespace1",
+	}
+
+	// test non-nil allocation with nil options
+	name, err = alloc.determineSharingName(nil)
+	if err != nil {
+		t.Fatalf("determineSharingName: expected no error; actual \"%s\"", err)
+	} else if err != nil || name != "unknown" {
+		t.Fatalf("determineSharingName: expected \"unknown\"; actual \"%s\"", name)
+	}
+
+	// test non-nil allocation with empty options
+	options := &AllocationAggregationOptions{}
+	name, err = alloc.determineSharingName(options)
+	if err != nil {
+		t.Fatalf("determineSharingName: expected no error; actual \"%s\"", err)
+	} else if err != nil || name != "unknown" {
+		t.Fatalf("determineSharingName: expected \"unknown\"; actual \"%s\"", name)
+	}
+
+	// test non-nil allocation with matching namespace options
+	options.SharedNamespaces = []string{"namespace1"}
+	name, err = alloc.determineSharingName(options)
+	if err != nil {
+		t.Fatalf("determineSharingName: expected no error; actual \"%s\"", err)
+	} else if err != nil || name != "namespace1" {
+		t.Fatalf("determineSharingName: expected \"namespace1\"; actual \"%s\"", name)
+	}
+
+	// test non-nil allocation with non-matching namespace options
+	options.SharedNamespaces = []string{"namespace2"}
+	name, err = alloc.determineSharingName(options)
+	if err != nil {
+		t.Fatalf("determineSharingName: expected no error; actual \"%s\"", err)
+	} else if err != nil || name != "unknown" {
+		t.Fatalf("determineSharingName: expected \"unknown\"; actual \"%s\"", name)
+	}
+
+	// test non-nil allocation with matching label options
+	options.SharedNamespaces = nil
+	options.SharedLabels = map[string][]string{
+		"app": {"app1"},
+	}
+	name, err = alloc.determineSharingName(options)
+	if err != nil {
+		t.Fatalf("determineSharingName: expected no error; actual \"%s\"", err)
+	} else if err != nil || name != "app1" {
+		t.Fatalf("determineSharingName: expected \"app1\"; actual \"%s\"", name)
+	}
+
+	// test non-nil allocation with partial-matching label options
+	options.SharedLabels = map[string][]string{
+		"app": {"app1", "app2"},
+	}
+	name, err = alloc.determineSharingName(options)
+	if err != nil {
+		t.Fatalf("determineSharingName: expected no error; actual \"%s\"", err)
+	} else if err != nil || name != "app1" {
+		t.Fatalf("determineSharingName: expected \"app1\"; actual \"%s\"", name)
+	}
+
+	// test non-nil allocation with non-matching label options
+	options.SharedLabels = map[string][]string{
+		"app": {"app2"},
+	}
+	name, err = alloc.determineSharingName(options)
+	if err != nil {
+		t.Fatalf("determineSharingName: expected no error; actual \"%s\"", err)
+	} else if err != nil || name != "unknown" {
+		t.Fatalf("determineSharingName: expected \"unknown\"; actual \"%s\"", name)
+	}
+
+	// test non-nil allocation with matching namespace and label options
+	options.SharedNamespaces = []string{"namespace1"}
+	options.SharedLabels = map[string][]string{
+		"app": {"app1"},
+	}
+	name, err = alloc.determineSharingName(options)
+	if err != nil {
+		t.Fatalf("determineSharingName: expected no error; actual \"%s\"", err)
+	} else if err != nil || name != "app1" {
+		t.Fatalf("determineSharingName: expected \"app1\"; actual \"%s\"", name)
+	}
+
+	// test non-nil allocation with non-matching namespace and matching label options
+	options.SharedNamespaces = []string{"namespace2"}
+	options.SharedLabels = map[string][]string{
+		"app": {"app1"},
+	}
+	name, err = alloc.determineSharingName(options)
+	if err != nil {
+		t.Fatalf("determineSharingName: expected no error; actual \"%s\"", err)
+	} else if err != nil || name != "app1" {
+		t.Fatalf("determineSharingName: expected \"app1\"; actual \"%s\"", name)
+	}
+
+	// test non-nil allocation with non-matching namespace and non-matching label options
+	options.SharedNamespaces = []string{"namespace2"}
+	options.SharedLabels = map[string][]string{
+		"app": {"app2"},
+	}
+	name, err = alloc.determineSharingName(options)
+	if err != nil {
+		t.Fatalf("determineSharingName: expected no error; actual \"%s\"", err)
+	} else if err != nil || name != "unknown" {
+		t.Fatalf("determineSharingName: expected \"unknown\"; actual \"%s\"", name)
+	}
+
+	// test non-nil allocation with multiple matching label options
+	alloc.Properties.Labels = map[string]string{
+		"app": "app1",
+		"env": "env1",
+	}
+	options.SharedNamespaces = nil
+	options.SharedLabels = map[string][]string{
+		"app": {"app1"},
+		"env": {"env1"},
+	}
+	name, err = alloc.determineSharingName(options)
+	if err != nil {
+		t.Fatalf("determineSharingName: expected no error; actual \"%s\"", err)
+	} else if err != nil || name != "app1" {
+		t.Fatalf("determineSharingName: expected \"app1\"; actual \"%s\"", name)
+	}
+
+	// test non-nil allocation with one matching label option
+	alloc.Properties.Labels = map[string]string{
+		"app": "app2",
+		"env": "env1",
+	}
+	options.SharedNamespaces = nil
+	options.SharedLabels = map[string][]string{
+		"app": {"app1"},
+		"env": {"env1"},
+	}
+	name, err = alloc.determineSharingName(options)
+	if err != nil {
+		t.Fatalf("determineSharingName: expected no error; actual \"%s\"", err)
+	} else if err != nil || name != "env1" {
+		t.Fatalf("determineSharingName: expected \"env1\"; actual \"%s\"", name)
+	}
+
+	// test non-nil allocation with one matching namespace option
+	alloc.Properties.Namespace = "namespace1"
+	options.SharedNamespaces = []string{"namespace1", "namespace2"}
+	options.SharedLabels = nil
+	name, err = alloc.determineSharingName(options)
+	if err != nil {
+		t.Fatalf("determineSharingName: expected no error; actual \"%s\"", err)
+	} else if err != nil || name != "namespace1" {
+		t.Fatalf("determineSharingName: expected \"namespace1\"; actual \"%s\"", name)
+	}
+
+	// test non-nil allocation with another one matching namespace option
+	alloc.Properties.Namespace = "namespace2"
+	options.SharedNamespaces = []string{"namespace1", "namespace2"}
+	options.SharedLabels = nil
+	name, err = alloc.determineSharingName(options)
+	if err != nil {
+		t.Fatalf("determineSharingName: expected no error; actual \"%s\"", err)
+	} else if err != nil || name != "namespace2" {
+		t.Fatalf("determineSharingName: expected \"namespace2\"; actual \"%s\"", name)
+	}
+
+	// test non-nil allocation with non-matching namespace options
+	alloc.Properties.Namespace = "namespace3"
+	options.SharedNamespaces = []string{"namespace1", "namespace2"}
+	name, err = alloc.determineSharingName(options)
+	if err != nil {
+		t.Fatalf("determineSharingName: expected no error; actual \"%s\"", err)
+	} else if err != nil || name != "unknown" {
+		t.Fatalf("determineSharingName: expected \"unknown\"; actual \"%s\"", name)
+	}
+}