Merge branch 'develop' into bolt/allocation-property

.github/workflows/pr.yaml → .github/workflows/build-test.yaml

@@ -1,6 +1,10 @@
-name: Develop PR - build test
+name: Build/Test
 
 on:
+  push:
+    branches:
+      - develop
+
   pull_request:
     branches:
       - develop
@@ -41,6 +45,11 @@ jobs:
         name: Build
         run: |
           just build-local
+      - name: Upload code coverage
+        uses: actions/upload-artifact@v3
+        with:
+          name: oc-code-coverage
+          path: coverage.out
 
   frontend:
     runs-on: ubuntu-latest

.github/workflows/sonar.yaml

@@ -0,0 +1,51 @@
+name: Sonar Code Coverage Upload
+on:
+  workflow_run:
+    workflows: ["Build/Test"]
+    types: [completed]
+jobs:
+  sonar:
+    name: Sonar
+    runs-on: ubuntu-latest
+    if: github.event.workflow_run.conclusion == 'success'
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          repository: ${{ github.event.workflow_run.head_repository.full_name }}
+          ref: ${{ github.event.workflow_run.head_branch }}
+          fetch-depth: 0
+      - name: 'Download code coverage'
+        uses: actions/github-script@v6
+        with:
+          script: |
+            let allArtifacts = await github.rest.actions.listWorkflowRunArtifacts({
+               owner: context.repo.owner,
+               repo: context.repo.repo,
+               run_id: context.payload.workflow_run.id,
+            });
+            let matchArtifact = allArtifacts.data.artifacts.filter((artifact) => {
+              return artifact.name == "oc-code-coverage"
+            })[0];
+            let download = await github.rest.actions.downloadArtifact({
+               owner: context.repo.owner,
+               repo: context.repo.repo,
+               artifact_id: matchArtifact.id,
+               archive_format: 'zip',
+            });
+            let fs = require('fs');
+            fs.writeFileSync(`${process.env.GITHUB_WORKSPACE}/oc-code-coverage.zip`, Buffer.from(download.data));
+      - name: 'Unzip code coverage'
+        run: unzip oc-code-coverage.zip -d coverage
+      - name: SonarCloud Scan
+        uses: sonarsource/sonarcloud-github-action@master
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
+        with:
+          args: >
+            -Dsonar.scm.revision=${{ github.event.workflow_run.head_sha }}
+            -Dsonar.pullrequest.key=${{ github.event.workflow_run.pull_requests[0].number }}
+            -Dsonar.pullrequest.branch=${{ github.event.workflow_run.pull_requests[0].head.ref }}
+            -Dsonar.pullrequest.base=${{ github.event.workflow_run.pull_requests[0].base.ref }}
+            -Dsonar.projectKey=opencost_opencost
+            -Dsonar.organization=opencost

justfile

@@ -8,7 +8,7 @@ default:
 
 # Run unit tests
 test:
-    {{commonenv}} go test ./...
+    {{commonenv}} go test ./... -coverprofile=coverage.out
 
 # Compile a local binary
 build-local:

pkg/cloud/aws/athenaintegration.go

@@ -147,7 +147,7 @@ func (ai *AthenaIntegration) GetCloudCost(start, end time.Time) (*kubecost.Cloud
 	groupByStr := strings.Join(groupByColumns, ", ")
 	queryStr := `
 		SELECT %s
-		FROM %s
+		FROM "%s"
 		WHERE %s
 		GROUP BY %s
 	`

pkg/cloud/azure/azurestorageintegration.go

@@ -67,11 +67,8 @@ func (asi *AzureStorageIntegration) GetCloudCost(start, end time.Time) (*kubecos
 			},
 		}
 
-		// Check if Item
-		if abv.IsCompute(cc.Properties.Category) {
-			// TODO: Will need to split VMSS for other features
-			ccsr.LoadCloudCost(cc)
-		}
+		ccsr.LoadCloudCost(cc)
+
 		return nil
 	})
 	if err != nil {

pkg/cloudcost/memoryrepository_test.go

@@ -0,0 +1,358 @@
+package cloudcost
+
+import (
+	"reflect"
+	"testing"
+	"time"
+
+	"github.com/opencost/opencost/pkg/kubecost"
+	"github.com/opencost/opencost/pkg/util/timeutil"
+)
+
+func TestMemoryRepository_Get(t *testing.T) {
+	defaultStart := time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC)
+	defaultEnd := defaultStart.Add(timeutil.Day)
+	defaultData := map[string]map[time.Time]*kubecost.CloudCostSet{
+		"key-1": {
+			defaultStart: DefaultMockCloudCostSet(defaultStart, defaultEnd, "aws", "key-1"),
+		},
+	}
+	tests := map[string]struct {
+		data      map[string]map[time.Time]*kubecost.CloudCostSet
+		startTime time.Time
+		key       string
+		want      *kubecost.CloudCostSet
+		wantErr   bool
+	}{
+		"No Data": {
+			data:      map[string]map[time.Time]*kubecost.CloudCostSet{},
+			startTime: defaultStart,
+			key:       "key-1",
+			want:      nil,
+			wantErr:   false,
+		},
+		"has data": {
+			data:      defaultData,
+			startTime: defaultStart,
+			key:       "key-1",
+			want:      DefaultMockCloudCostSet(defaultStart, defaultEnd, "aws", "key-1"),
+			wantErr:   false,
+		},
+		"wrong key": {
+			data:      defaultData,
+			startTime: defaultStart,
+			key:       "key-2",
+			want:      nil,
+			wantErr:   false,
+		},
+		"wrong time": {
+			data:      defaultData,
+			startTime: defaultEnd,
+			key:       "key-1",
+			want:      nil,
+			wantErr:   false,
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			m := &MemoryRepository{
+				data: tt.data,
+			}
+			got, err := m.Get(tt.startTime, tt.key)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Get() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("Get() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestMemoryRepository_Has(t *testing.T) {
+	defaultStart := time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC)
+	defaultEnd := defaultStart.Add(timeutil.Day)
+	defaultData := map[string]map[time.Time]*kubecost.CloudCostSet{
+		"key-1": {
+			defaultStart: DefaultMockCloudCostSet(defaultStart, defaultEnd, "aws", "key-1"),
+		},
+	}
+	tests := map[string]struct {
+		data      map[string]map[time.Time]*kubecost.CloudCostSet
+		startTime time.Time
+		key       string
+		want      bool
+		wantErr   bool
+	}{
+		"No Data": {
+			data:      map[string]map[time.Time]*kubecost.CloudCostSet{},
+			startTime: defaultStart,
+			key:       "key-1",
+			want:      false,
+			wantErr:   false,
+		},
+		"has data": {
+			data:      defaultData,
+			startTime: defaultStart,
+			key:       "key-1",
+			want:      true,
+			wantErr:   false,
+		},
+		"wrong key": {
+			data:      defaultData,
+			startTime: defaultStart,
+			key:       "key-2",
+			want:      false,
+			wantErr:   false,
+		},
+		"wrong time": {
+			data:      defaultData,
+			startTime: defaultEnd,
+			key:       "key-1",
+			want:      false,
+			wantErr:   false,
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			m := &MemoryRepository{
+				data: tt.data,
+			}
+			got, err := m.Has(tt.startTime, tt.key)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Has() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if got != tt.want {
+				t.Errorf("Has() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestMemoryRepository_Keys(t *testing.T) {
+
+	tests := map[string]struct {
+		data    map[string]map[time.Time]*kubecost.CloudCostSet
+		want    []string
+		wantErr bool
+	}{
+		"empty": {
+			data:    map[string]map[time.Time]*kubecost.CloudCostSet{},
+			want:    []string{},
+			wantErr: false,
+		},
+		"one-key": {
+			data: map[string]map[time.Time]*kubecost.CloudCostSet{
+				"key-1": nil,
+			},
+			want:    []string{"key-1"},
+			wantErr: false,
+		},
+		"two-key": {
+			data: map[string]map[time.Time]*kubecost.CloudCostSet{
+				"key-1": nil,
+				"key-2": {
+					time.Now():        nil,
+					time.Now().Add(1): nil,
+				},
+			},
+			want:    []string{"key-1", "key-2"},
+			wantErr: false,
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			m := &MemoryRepository{
+				data: tt.data,
+			}
+			got, err := m.Keys()
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Keys() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("Keys() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestMemoryRepository_Put(t *testing.T) {
+	defaultStart := time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC)
+	defaultEnd := defaultStart.Add(timeutil.Day)
+
+	tests := map[string]struct {
+		data    map[string]map[time.Time]*kubecost.CloudCostSet
+		input   *kubecost.CloudCostSet
+		want    map[string]map[time.Time]*kubecost.CloudCostSet
+		wantErr bool
+	}{
+
+		"nil set": {
+			data:    map[string]map[time.Time]*kubecost.CloudCostSet{},
+			input:   nil,
+			want:    map[string]map[time.Time]*kubecost.CloudCostSet{},
+			wantErr: true,
+		},
+		"invalid window": {
+			data: map[string]map[time.Time]*kubecost.CloudCostSet{},
+			input: &kubecost.CloudCostSet{
+				CloudCosts:  nil,
+				Window:      kubecost.Window{},
+				Integration: "key-1",
+			},
+			want:    map[string]map[time.Time]*kubecost.CloudCostSet{},
+			wantErr: true,
+		},
+		"invalid key": {
+			data: map[string]map[time.Time]*kubecost.CloudCostSet{},
+			input: &kubecost.CloudCostSet{
+				CloudCosts:  nil,
+				Window:      kubecost.NewClosedWindow(defaultStart, defaultEnd),
+				Integration: "",
+			},
+			want:    map[string]map[time.Time]*kubecost.CloudCostSet{},
+			wantErr: true,
+		},
+		"valid input": {
+			data:  map[string]map[time.Time]*kubecost.CloudCostSet{},
+			input: DefaultMockCloudCostSet(defaultStart, defaultEnd, "aws", "key-1"),
+			want: map[string]map[time.Time]*kubecost.CloudCostSet{
+				"key-1": {
+					defaultStart: DefaultMockCloudCostSet(defaultStart, defaultEnd, "aws", "key-1"),
+				},
+			},
+			wantErr: false,
+		},
+		"overwrite": {
+			data: map[string]map[time.Time]*kubecost.CloudCostSet{
+				"key-1": {
+					defaultStart: DefaultMockCloudCostSet(defaultStart, defaultEnd, "gcp", "key-1"),
+				},
+			},
+			input: DefaultMockCloudCostSet(defaultStart, defaultEnd, "aws", "key-1"),
+			want: map[string]map[time.Time]*kubecost.CloudCostSet{
+				"key-1": {
+					defaultStart: DefaultMockCloudCostSet(defaultStart, defaultEnd, "aws", "key-1"),
+				},
+			},
+			wantErr: false,
+		},
+		"invalid overwrite": {
+			data: map[string]map[time.Time]*kubecost.CloudCostSet{
+				"key-1": {
+					defaultStart: DefaultMockCloudCostSet(defaultStart, defaultEnd, "gcp", "key-1"),
+				},
+			},
+			input: &kubecost.CloudCostSet{
+				Window:      kubecost.NewWindow(&defaultStart, nil),
+				Integration: "key-1",
+			},
+			want: map[string]map[time.Time]*kubecost.CloudCostSet{
+				"key-1": {
+					defaultStart: DefaultMockCloudCostSet(defaultStart, defaultEnd, "gcp", "key-1"),
+				},
+			},
+			wantErr: true,
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			m := &MemoryRepository{data: tt.data}
+
+			if err := m.Put(tt.input); (err != nil) != tt.wantErr {
+				t.Errorf("Put() error = %v, wantErr %v", err, tt.wantErr)
+			}
+
+			if !reflect.DeepEqual(m.data, tt.want) {
+				t.Errorf("Put() got = %v, want %v", m.data, tt.want)
+			}
+		})
+	}
+}
+
+func TestMemoryRepository_Expire(t *testing.T) {
+	dayOne := time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC)
+	dayTwo := time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC)
+	dayThree := time.Date(2023, 1, 3, 0, 0, 0, 0, time.UTC)
+	tests := map[string]struct {
+		data    map[string]map[time.Time]*kubecost.CloudCostSet
+		limit   time.Time
+		want    map[string]map[time.Time]*kubecost.CloudCostSet
+		wantErr bool
+	}{
+		"no expire": {
+			data: map[string]map[time.Time]*kubecost.CloudCostSet{
+				"key-1": {
+					dayTwo: nil,
+				},
+			},
+			limit: dayOne,
+			want: map[string]map[time.Time]*kubecost.CloudCostSet{
+				"key-1": {
+					dayTwo: nil,
+				},
+			},
+			wantErr: false,
+		},
+		"limit match": {
+			data: map[string]map[time.Time]*kubecost.CloudCostSet{
+				"key-1": {
+					dayTwo: nil,
+				},
+			},
+			limit: dayTwo,
+			want: map[string]map[time.Time]*kubecost.CloudCostSet{
+				"key-1": {
+					dayTwo: nil,
+				},
+			},
+			wantErr: false,
+		},
+		"single expire": {
+			data: map[string]map[time.Time]*kubecost.CloudCostSet{
+				"key-1": {
+					dayTwo: nil,
+				},
+			},
+			limit:   dayThree,
+			want:    map[string]map[time.Time]*kubecost.CloudCostSet{},
+			wantErr: false,
+		},
+		"one key expire": {
+			data: map[string]map[time.Time]*kubecost.CloudCostSet{
+				"key-1": {
+					dayOne: nil,
+					dayTwo: nil,
+				},
+				"key-2": {
+					dayOne: nil,
+				},
+			},
+			limit: dayTwo,
+			want: map[string]map[time.Time]*kubecost.CloudCostSet{
+				"key-1": {
+					dayTwo: nil,
+				},
+			},
+			wantErr: false,
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			m := &MemoryRepository{
+				data: tt.data,
+			}
+			if err := m.Expire(tt.limit); (err != nil) != tt.wantErr {
+				t.Errorf("Expire() error = %v, wantErr %v", err, tt.wantErr)
+			}
+
+			if !reflect.DeepEqual(m.data, tt.want) {
+				t.Errorf("Expire() got = %v, want %v", m.data, tt.want)
+			}
+
+		})
+	}
+}

pkg/cloudcost/mock.go

@@ -0,0 +1,90 @@
+package cloudcost
+
+import (
+	"time"
+
+	"github.com/opencost/opencost/pkg/kubecost"
+)
+
+func DefaultMockCloudCostSet(start, end time.Time, provider, integration string) *kubecost.CloudCostSet {
+	ccs := kubecost.NewCloudCostSet(start, end)
+
+	ccs.Integration = integration
+
+	ccs.Insert(&kubecost.CloudCost{
+		Window: ccs.Window,
+		Properties: &kubecost.CloudCostProperties{
+			Provider:        provider,
+			AccountID:       "account1",
+			InvoiceEntityID: "invoiceEntity1",
+			Service:         provider + "-storage",
+			Category:        kubecost.StorageCategory,
+			Labels: kubecost.CloudCostLabels{
+				"label1": "value1",
+				"label2": "value2",
+				"label3": "value3",
+			},
+			ProviderID: "id1",
+		},
+		ListCost: kubecost.CostMetric{
+			Cost:              100,
+			KubernetesPercent: 0,
+		},
+		NetCost: kubecost.CostMetric{
+			Cost:              100,
+			KubernetesPercent: 0,
+		},
+	})
+
+	ccs.Insert(&kubecost.CloudCost{
+		Window: ccs.Window,
+		Properties: &kubecost.CloudCostProperties{
+			Provider:        provider,
+			AccountID:       "account1",
+			InvoiceEntityID: "invoiceEntity1",
+			Service:         provider + "-compute",
+			Category:        kubecost.ComputeCategory,
+			Labels: kubecost.CloudCostLabels{
+				"label1": "value1",
+				"label2": "value2",
+				"label3": "value3",
+			},
+			ProviderID: "id2",
+		},
+		ListCost: kubecost.CostMetric{
+			Cost:              2000,
+			KubernetesPercent: 1,
+		},
+		NetCost: kubecost.CostMetric{
+			Cost:              1800,
+			KubernetesPercent: 1,
+		},
+	})
+
+	ccs.Insert(&kubecost.CloudCost{
+		Window: ccs.Window,
+		Properties: &kubecost.CloudCostProperties{
+			Provider:        provider,
+			AccountID:       "account2",
+			InvoiceEntityID: "invoiceEntity2",
+			Service:         provider + "-compute",
+			Category:        kubecost.ComputeCategory,
+			Labels: kubecost.CloudCostLabels{
+				"label1": "value1",
+				"label2": "value2",
+				"label3": "value3",
+			},
+			ProviderID: "id3",
+		},
+		ListCost: kubecost.CostMetric{
+			Cost:              8000,
+			KubernetesPercent: 1,
+		},
+		NetCost: kubecost.CostMetric{
+			Cost:              8000,
+			KubernetesPercent: 1,
+		},
+	})
+
+	return ccs
+}

pkg/cloudcost/querier_test.go

@@ -0,0 +1,118 @@
+package cloudcost
+
+import (
+	"testing"
+)
+
+func TestParseSortDirection(t *testing.T) {
+	tests := map[string]struct {
+		input   string
+		want    SortDirection
+		wantErr bool
+	}{
+		"Empty String": {
+			input:   "",
+			want:    SortDirectionNone,
+			wantErr: true,
+		},
+		"invalid input": {
+			input:   "invalid",
+			want:    SortDirectionNone,
+			wantErr: true,
+		},
+		"upper case ascending": {
+			input:   "ASC",
+			want:    SortDirectionAscending,
+			wantErr: false,
+		},
+		"lower case ascending": {
+			input:   "asc",
+			want:    SortDirectionAscending,
+			wantErr: false,
+		},
+		"upper case descending": {
+			input:   "DESC",
+			want:    SortDirectionDescending,
+			wantErr: false,
+		},
+		"lower case descending": {
+			input:   "desc",
+			want:    SortDirectionDescending,
+			wantErr: false,
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			got, err := ParseSortDirection(tt.input)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("ParseSortDirection() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if got != tt.want {
+				t.Errorf("ParseSortDirection() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestParseSortField(t *testing.T) {
+
+	tests := map[string]struct {
+		input   string
+		want    SortField
+		wantErr bool
+	}{
+		"Empty String": {
+			input:   "",
+			want:    SortFieldNone,
+			wantErr: true,
+		},
+		"invalid input": {
+			input:   "invalid",
+			want:    SortFieldNone,
+			wantErr: true,
+		},
+		"upper case cost": {
+			input:   "Cost",
+			want:    SortFieldCost,
+			wantErr: false,
+		},
+		"lower case cost": {
+			input:   "cost",
+			want:    SortFieldCost,
+			wantErr: false,
+		},
+		"upper case k8s %": {
+			input:   "KubernetesPercent",
+			want:    SortFieldKubernetesPercent,
+			wantErr: false,
+		},
+		"lower case k8s %": {
+			input:   "kubernetesPercent",
+			want:    SortFieldKubernetesPercent,
+			wantErr: false,
+		},
+		"upper case name": {
+			input:   "Name",
+			want:    SortFieldName,
+			wantErr: false,
+		},
+		"lower case Name": {
+			input:   "name",
+			want:    SortFieldName,
+			wantErr: false,
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			got, err := ParseSortField(tt.input)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("ParseSortField() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if got != tt.want {
+				t.Errorf("ParseSortField() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/cloudcost/queryservice.go

@@ -1,16 +1,12 @@
 package cloudcost
 
 import (
-	"encoding/csv"
 	"fmt"
 	"net/http"
 	"strings"
 
 	"github.com/julienschmidt/httprouter"
-	filter21 "github.com/opencost/opencost/pkg/filter21"
-	"github.com/opencost/opencost/pkg/filter21/cloudcost"
 	"github.com/opencost/opencost/pkg/kubecost"
-	"github.com/opencost/opencost/pkg/prom"
 	"github.com/opencost/opencost/pkg/util/httputil"
 	"go.opentelemetry.io/otel"
 )
@@ -52,7 +48,8 @@ func (s *QueryService) GetCloudCostHandler() func(w http.ResponseWriter, r *http
 			return
 		}
 
-		request, err := parseCloudCostRequest(r)
+		qp := httputil.NewQueryParams(r.URL.Query())
+		request, err := ParseCloudCostRequest(qp)
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
@@ -89,7 +86,8 @@ func (s *QueryService) GetCloudCostViewGraphHandler() func(w http.ResponseWriter
 			return
 		}
 
-		request, err := parseCloudCostViewRequest(r)
+		qp := httputil.NewQueryParams(r.URL.Query())
+		request, err := parseCloudCostViewRequest(qp)
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
@@ -131,7 +129,8 @@ func (s *QueryService) GetCloudCostViewTotalsHandler() func(w http.ResponseWrite
 			return
 		}
 
-		request, err := parseCloudCostViewRequest(r)
+		qp := httputil.NewQueryParams(r.URL.Query())
+		request, err := parseCloudCostViewRequest(qp)
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
@@ -173,13 +172,13 @@ func (s *QueryService) GetCloudCostViewTableHandler() func(w http.ResponseWriter
 			return
 		}
 
-		request, err := parseCloudCostViewRequest(r)
+		qp := httputil.NewQueryParams(r.URL.Query())
+		request, err := parseCloudCostViewRequest(qp)
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
 		}
 
-		qp := httputil.NewQueryParams(r.URL.Query())
 		format := qp.Get("format", "json")
 		if strings.HasPrefix(format, csvFormat) {
 			w.Header().Set("Content-Type", "text/csv")
@@ -206,165 +205,3 @@ func (s *QueryService) GetCloudCostViewTableHandler() func(w http.ResponseWriter
 		protocol.WriteData(w, resp)
 	}
 }
-
-func parseCloudCostRequest(r *http.Request) (*QueryRequest, error) {
-	qp := httputil.NewQueryParams(r.URL.Query())
-
-	windowStr := qp.Get("window", "")
-	if windowStr == "" {
-		return nil, fmt.Errorf("missing require window param")
-	}
-
-	window, err := kubecost.ParseWindowUTC(windowStr)
-	if err != nil {
-		return nil, fmt.Errorf("invalid window parameter: %w", err)
-	}
-	if window.IsOpen() {
-		return nil, fmt.Errorf("invalid window parameter: %s", window.String())
-	}
-
-	aggregateByRaw := qp.GetList("aggregate", ",")
-	aggregateBy := []string{}
-	for _, aggBy := range aggregateByRaw {
-		prop, err := ParseCloudCostProperty(aggBy)
-		if err != nil {
-			return nil, fmt.Errorf("error parsing aggregate by %v", err)
-		}
-		aggregateBy = append(aggregateBy, prop)
-	}
-	if len(aggregateBy) == 0 {
-		aggregateBy = []string{
-			kubecost.CloudCostInvoiceEntityIDProp,
-			kubecost.CloudCostAccountIDProp,
-			kubecost.CloudCostProviderProp,
-			kubecost.CloudCostProviderIDProp,
-			kubecost.CloudCostCategoryProp,
-			kubecost.CloudCostServiceProp,
-		}
-	}
-
-	accumulate := kubecost.ParseAccumulate(qp.Get("accumulate", ""))
-
-	var filter filter21.Filter
-	filterString := qp.Get("filter", "")
-	if filterString != "" {
-		parser := cloudcost.NewCloudCostFilterParser()
-		filter, err = parser.Parse(filterString)
-		if err != nil {
-			return nil, fmt.Errorf("Parsing 'filter' parameter: %s", err)
-		}
-	}
-
-	opts := &QueryRequest{
-		Start:       *window.Start(),
-		End:         *window.End(),
-		AggregateBy: aggregateBy,
-		Accumulate:  accumulate,
-		Filter:      filter,
-	}
-
-	return opts, nil
-}
-
-func ParseCloudCostProperty(text string) (string, error) {
-	switch strings.TrimSpace(strings.ToLower(text)) {
-	case strings.ToLower(kubecost.CloudCostInvoiceEntityIDProp):
-		return kubecost.CloudCostInvoiceEntityIDProp, nil
-	case strings.ToLower(kubecost.CloudCostAccountIDProp):
-		return kubecost.CloudCostAccountIDProp, nil
-	case strings.ToLower(kubecost.CloudCostProviderProp):
-		return kubecost.CloudCostProviderProp, nil
-	case strings.ToLower(kubecost.CloudCostProviderIDProp):
-		return kubecost.CloudCostProviderIDProp, nil
-	case strings.ToLower(kubecost.CloudCostCategoryProp):
-		return kubecost.CloudCostCategoryProp, nil
-	case strings.ToLower(kubecost.CloudCostServiceProp):
-		return kubecost.CloudCostServiceProp, nil
-	}
-
-	if strings.HasPrefix(text, "label:") {
-		label := prom.SanitizeLabelName(strings.TrimSpace(strings.TrimPrefix(text, "label:")))
-		return fmt.Sprintf("label:%s", label), nil
-	}
-
-	return "", fmt.Errorf("invalid cloud cost property: %s", text)
-}
-
-func parseCloudCostViewRequest(r *http.Request) (*ViewQueryRequest, error) {
-	qr, err := parseCloudCostRequest(r)
-	if err != nil {
-		return nil, err
-	}
-	qp := httputil.NewQueryParams(r.URL.Query())
-
-	// parse cost metric
-	costMetricName, err := kubecost.ParseCostMetricName(qp.Get("costMetric", string(kubecost.CostMetricAmortizedNetCost)))
-	if err != nil {
-		return nil, fmt.Errorf("error parsing 'costMetric': %w", err)
-	}
-
-	limit := qp.GetInt("limit", 0)
-	offset := qp.GetInt("offset", 0)
-
-	// parse order
-	order, err := ParseSortDirection(qp.Get("sortByOrder", "desc"))
-	if err != nil {
-		return nil, fmt.Errorf("error parsing 'sortByOrder: %w", err)
-	}
-
-	sortColumn, err := ParseSortField(qp.Get("sortBy", "cost"))
-	if err != nil {
-		return nil, fmt.Errorf("error parsing 'sortBy': %w", err)
-	}
-
-	return &ViewQueryRequest{
-		QueryRequest:     *qr,
-		CostMetricName:   costMetricName,
-		ChartItemsLength: DefaultChartItemsLength,
-		Limit:            limit,
-		Offset:           offset,
-		SortDirection:    order,
-		SortColumn:       sortColumn,
-	}, nil
-}
-
-// CloudCostViewTableRowsToCSV takes the csv writer and writes the ViewTableRows into the writer.
-func CloudCostViewTableRowsToCSV(writer *csv.Writer, ctr ViewTableRows, window string) error {
-	defer writer.Flush()
-	// Write the column headers
-	headers := []string{
-		"Name",
-		"K8s Utilization",
-		"Total",
-		"Window",
-	}
-	err := writer.Write(headers)
-	if err != nil {
-		return fmt.Errorf("CloudCostViewTableRowsToCSV: failed to convert ViewTableRows to csv with error: %w", err)
-	}
-
-	// Write one row per entry in the ViewTableRows
-	for _, row := range ctr {
-		err = writer.Write([]string{
-			row.Name,
-			fmt.Sprintf("%.3f", row.KubernetesPercent),
-			fmt.Sprintf("%.3f", row.Cost),
-			window,
-		})
-		if err != nil {
-			return fmt.Errorf("CloudCostViewTableRowsToCSV: failed to convert ViewTableRows to csv with error: %w", err)
-		}
-	}
-
-	return nil
-}
-
-func writeCloudCostViewTableRowsAsCSV(w http.ResponseWriter, ctr ViewTableRows, window string) {
-	writer := csv.NewWriter(w)
-
-	err := CloudCostViewTableRowsToCSV(writer, ctr, window)
-	if err != nil {
-		protocol.WriteError(w, protocol.InternalServerError(err.Error()))
-		return
-	}
-}

pkg/cloudcost/queryservice_helper.go

@@ -0,0 +1,170 @@
+package cloudcost
+
+import (
+	"encoding/csv"
+	"fmt"
+	"net/http"
+	"strings"
+
+	filter21 "github.com/opencost/opencost/pkg/filter21"
+	"github.com/opencost/opencost/pkg/filter21/cloudcost"
+	"github.com/opencost/opencost/pkg/kubecost"
+	"github.com/opencost/opencost/pkg/prom"
+	"github.com/opencost/opencost/pkg/util/httputil"
+)
+
+func ParseCloudCostRequest(qp httputil.QueryParams) (*QueryRequest, error) {
+
+	windowStr := qp.Get("window", "")
+	if windowStr == "" {
+		return nil, fmt.Errorf("missing require window param")
+	}
+
+	window, err := kubecost.ParseWindowUTC(windowStr)
+	if err != nil {
+		return nil, fmt.Errorf("invalid window parameter: %w", err)
+	}
+	if window.IsOpen() {
+		return nil, fmt.Errorf("invalid window parameter: %s", window.String())
+	}
+
+	aggregateByRaw := qp.GetList("aggregate", ",")
+	var aggregateBy []string
+	for _, aggBy := range aggregateByRaw {
+		prop, err := ParseCloudCostProperty(aggBy)
+		if err != nil {
+			return nil, fmt.Errorf("error parsing aggregate by %v", err)
+		}
+		aggregateBy = append(aggregateBy, prop)
+	}
+
+	accumulate := kubecost.ParseAccumulate(qp.Get("accumulate", ""))
+
+	var filter filter21.Filter
+	filterString := qp.Get("filter", "")
+	if filterString != "" {
+		parser := cloudcost.NewCloudCostFilterParser()
+		filter, err = parser.Parse(filterString)
+		if err != nil {
+			return nil, fmt.Errorf("Parsing 'filter' parameter: %s", err)
+		}
+	}
+
+	opts := &QueryRequest{
+		Start:       *window.Start(),
+		End:         *window.End(),
+		AggregateBy: aggregateBy,
+		Accumulate:  accumulate,
+		Filter:      filter,
+	}
+
+	return opts, nil
+}
+
+func ParseCloudCostProperty(text string) (string, error) {
+	switch strings.TrimSpace(strings.ToLower(text)) {
+	case strings.ToLower(kubecost.CloudCostInvoiceEntityIDProp):
+		return kubecost.CloudCostInvoiceEntityIDProp, nil
+	case strings.ToLower(kubecost.CloudCostAccountIDProp):
+		return kubecost.CloudCostAccountIDProp, nil
+	case strings.ToLower(kubecost.CloudCostProviderProp):
+		return kubecost.CloudCostProviderProp, nil
+	case strings.ToLower(kubecost.CloudCostProviderIDProp):
+		return kubecost.CloudCostProviderIDProp, nil
+	case strings.ToLower(kubecost.CloudCostCategoryProp):
+		return kubecost.CloudCostCategoryProp, nil
+	case strings.ToLower(kubecost.CloudCostServiceProp):
+		return kubecost.CloudCostServiceProp, nil
+	}
+
+	if strings.HasPrefix(text, "label:") {
+		label := prom.SanitizeLabelName(strings.TrimSpace(strings.TrimPrefix(text, "label:")))
+		return fmt.Sprintf("label:%s", label), nil
+	}
+
+	return "", fmt.Errorf("invalid cloud cost property: %s", text)
+}
+
+func parseCloudCostViewRequest(qp httputil.QueryParams) (*ViewQueryRequest, error) {
+	qr, err := ParseCloudCostRequest(qp)
+	if err != nil {
+		return nil, err
+	}
+
+	// parse cost metric
+	costMetricName, err := kubecost.ParseCostMetricName(qp.Get("costMetric", string(kubecost.CostMetricAmortizedNetCost)))
+	if err != nil {
+		return nil, fmt.Errorf("error parsing 'costMetric': %w", err)
+	}
+
+	limit := qp.GetInt("limit", 0)
+	if limit < 0 {
+		return nil, fmt.Errorf("invalid value for limit %d", limit)
+	}
+	offset := qp.GetInt("offset", 0)
+	if offset < 0 {
+		return nil, fmt.Errorf("invalid value for offset %d", offset)
+	}
+
+	// parse order
+	order, err := ParseSortDirection(qp.Get("sortByOrder", "desc"))
+	if err != nil {
+		return nil, fmt.Errorf("error parsing 'sortByOrder: %w", err)
+	}
+
+	sortColumn, err := ParseSortField(qp.Get("sortBy", "cost"))
+	if err != nil {
+		return nil, fmt.Errorf("error parsing 'sortBy': %w", err)
+	}
+
+	return &ViewQueryRequest{
+		QueryRequest:     *qr,
+		CostMetricName:   costMetricName,
+		ChartItemsLength: DefaultChartItemsLength,
+		Limit:            limit,
+		Offset:           offset,
+		SortDirection:    order,
+		SortColumn:       sortColumn,
+	}, nil
+}
+
+// CloudCostViewTableRowsToCSV takes the csv writer and writes the ViewTableRows into the writer.
+func CloudCostViewTableRowsToCSV(writer *csv.Writer, ctr ViewTableRows, window string) error {
+	defer writer.Flush()
+	// Write the column headers
+	headers := []string{
+		"Name",
+		"K8s Utilization",
+		"Total",
+		"Window",
+	}
+	err := writer.Write(headers)
+	if err != nil {
+		return fmt.Errorf("CloudCostViewTableRowsToCSV: failed to convert ViewTableRows to csv with error: %w", err)
+	}
+
+	// Write one row per entry in the ViewTableRows
+	for _, row := range ctr {
+		err = writer.Write([]string{
+			row.Name,
+			fmt.Sprintf("%.3f", row.KubernetesPercent),
+			fmt.Sprintf("%.3f", row.Cost),
+			window,
+		})
+		if err != nil {
+			return fmt.Errorf("CloudCostViewTableRowsToCSV: failed to convert ViewTableRows to csv with error: %w", err)
+		}
+	}
+
+	return nil
+}
+
+func writeCloudCostViewTableRowsAsCSV(w http.ResponseWriter, ctr ViewTableRows, window string) {
+	writer := csv.NewWriter(w)
+
+	err := CloudCostViewTableRowsToCSV(writer, ctr, window)
+	if err != nil {
+		protocol.WriteError(w, protocol.InternalServerError(err.Error()))
+		return
+	}
+}

pkg/cloudcost/queryservice_helper_test.go

@@ -0,0 +1,136 @@
+package cloudcost
+
+import (
+	"reflect"
+	"testing"
+	"time"
+
+	"github.com/opencost/opencost/pkg/filter21/cloudcost"
+	"github.com/opencost/opencost/pkg/kubecost"
+	"github.com/opencost/opencost/pkg/util/httputil"
+)
+
+func TestParseCloudCostRequest(t *testing.T) {
+	windowStr := "2023-01-01T00:00:00Z,2023-01-02T00:00:00Z"
+	start := time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC)
+	end := time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC)
+	validFilterStr := `service:"AmazonEC2"`
+	parser := cloudcost.NewCloudCostFilterParser()
+	validFilter, _ := parser.Parse(validFilterStr)
+	tests := map[string]struct {
+		values  map[string][]string
+		want    *QueryRequest
+		wantErr bool
+	}{
+		"missing window": {
+			values:  map[string][]string{},
+			want:    nil,
+			wantErr: true,
+		},
+		"invalid window": {
+			values: map[string][]string{
+				"window": {"invalid"},
+			},
+			want:    nil,
+			wantErr: true,
+		},
+		"valid window": {
+			values: map[string][]string{
+				"window": {windowStr},
+			},
+			want: &QueryRequest{
+				Start:       start,
+				End:         end,
+				AggregateBy: nil,
+				Accumulate:  "",
+				Filter:      nil,
+			},
+			wantErr: false,
+		},
+		"valid aggregate": {
+			values: map[string][]string{
+				"window":    {windowStr},
+				"aggregate": {"invoiceEntityID,accountID,label:app"},
+			},
+			want: &QueryRequest{
+				Start:       start,
+				End:         end,
+				AggregateBy: []string{kubecost.CloudCostInvoiceEntityIDProp, kubecost.CloudCostAccountIDProp, "label:app"},
+				Accumulate:  "",
+				Filter:      nil,
+			},
+			wantErr: false,
+		},
+		"invalid aggregate": {
+			values: map[string][]string{
+				"window":    {windowStr},
+				"aggregate": {"invalid"},
+			},
+			want:    nil,
+			wantErr: true,
+		},
+		"valid accumulate": {
+			values: map[string][]string{
+				"window":     {windowStr},
+				"accumulate": {"week"},
+			},
+			want: &QueryRequest{
+				Start:       start,
+				End:         end,
+				AggregateBy: nil,
+				Accumulate:  kubecost.AccumulateOptionWeek,
+				Filter:      nil,
+			},
+			wantErr: false,
+		},
+		"invalid accumulate": {
+			values: map[string][]string{
+				"window":     {windowStr},
+				"accumulate": {"invalid"},
+			},
+			want: &QueryRequest{
+				Start:       start,
+				End:         end,
+				AggregateBy: nil,
+				Accumulate:  kubecost.AccumulateOptionNone,
+				Filter:      nil,
+			},
+			wantErr: false,
+		},
+		"valid filter": {
+			values: map[string][]string{
+				"window": {windowStr},
+				"filter": {validFilterStr},
+			},
+			want: &QueryRequest{
+				Start:       start,
+				End:         end,
+				AggregateBy: nil,
+				Accumulate:  kubecost.AccumulateOptionNone,
+				Filter:      validFilter,
+			},
+			wantErr: false,
+		},
+		"invalid filter": {
+			values: map[string][]string{
+				"window": {windowStr},
+				"filter": {"invalid"},
+			},
+			want:    nil,
+			wantErr: true,
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			qp := httputil.NewQueryParams(tt.values)
+			got, err := ParseCloudCostRequest(qp)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("ParseCloudCostRequest() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("ParseCloudCostRequest() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/cloudcost/repositoryquerier.go

@@ -220,10 +220,17 @@ func (rq *RepositoryQuerier) QueryViewTable(request ViewQueryRequest, ctx contex
 		return make([]*ViewTableRow, 0), nil
 	}
 
-	limit := request.Offset + request.Limit
-	if limit > len(rows) {
+	if request.Limit > 0 {
+		limit := request.Offset + request.Limit
+		if limit > len(rows) {
+			return rows[request.Offset:], nil
+		}
+		return rows[request.Offset:limit], nil
+	}
+
+	if request.Offset > 0 {
 		return rows[request.Offset:], nil
 	}
 
-	return rows[request.Offset:limit], nil
+	return rows, nil
 }