Only delete data when WholeStreamDeletion or FilterAndDelete (#6286)

* Only delete data when WholeStreamDeletion or FilterAndDelete

* remove broken snyn workflow

.github/workflows/snyk.yml

@@ -1,54 +0,0 @@
-name: Snyk SBOM CI
-
-on:
-  push:
-    branches: [ main ]
-  pull_request:
-    branches: [ main ]
-
-jobs:
-  snyk_scans:
-
-    runs-on: ubuntu-latest
-
-    steps:
-
-      - uses: actions/checkout@v3
-      - name: Run Snyk to check for vulnerabilities - sarif output
-        continue-on-error: true
-        uses: snyk/actions/golang@master
-        env:
-          SNYK_TOKEN: ${{ secrets.SNYK_TOKEN }}
-        with:
-          command: test
-          args:  --all-projects --sarif-file-output=${{ github.event.repository.name }}.sarif --strict-out-of-sync=false
-
-      - name: Run Snyk to check for vulnerabilities - json output
-        continue-on-error: true
-        uses: snyk/actions/golang@master
-        env:
-          SNYK_TOKEN: ${{ secrets.SNYK_TOKEN }}
-        with:
-          command: test
-          args: --json --all-projects --json-file-output=${{ github.event.repository.name }}.json  --strict-out-of-sync=false
-
-      - name: install snyk-to-html
-        run: |
-          npm install snyk-to-html snyk2spdx snyk -g
-          snyk auth ${{ secrets.SNYK_TOKEN }}
-          snyk-to-html -i ${{ github.event.repository.name }}.json -o ${{ github.event.repository.name }}.html
-          snyk test --json --strict-out-of-sync=false | snyk2spdx --output ${{ github.event.repository.name }}.spdx
-
-      - name: Upload result to GitHub Code Scanning
-        uses: github/codeql-action/upload-sarif@v2
-        with:
-          sarif_file: ${{ github.event.repository.name }}.sarif
-
-      - name: Create results dir
-        run: mkdir -p snyk_scans && cp -v ${{ github.event.repository.name }}.{html,json,sarif,spdx} snyk_scans/
-
-      - name: Use the Upload Artifact GitHub Action
-        uses: actions/upload-artifact@v2
-        with:
-          name: snyk_scans
-          path: snyk_scans

pkg/storage/stores/shipper/compactor/deletion/delete_requests_manager.go

@@ -133,6 +133,11 @@ func (d *DeleteRequestsManager) Expired(ref retention.ChunkEntry, _ model.Time)
 		return false, nil
 	}
 
+	if d.deletionMode == Disabled || d.deletionMode == FilterOnly {
+		// Don't process deletes
+		return false, nil
+	}
+
 	d.chunkIntervalsToRetain = d.chunkIntervalsToRetain[:0]
 	d.chunkIntervalsToRetain = append(d.chunkIntervalsToRetain, retention.IntervalFilter{
 		Interval: model.Interval{

pkg/storage/stores/shipper/compactor/deletion/delete_requests_manager_test.go

@@ -35,11 +35,13 @@ func TestDeleteRequestsManager_Expired(t *testing.T) {
 
 	for _, tc := range []struct {
 		name                    string
+		deletionMode            Mode
 		deleteRequestsFromStore []DeleteRequest
 		expectedResp            resp
 	}{
 		{
 			name:         "no delete requests",
+			deletionMode: WholeStreamDeletion,
 			expectedResp: resp{
 				isExpired:           false,
 				nonDeletedIntervals: nil,
@@ -47,6 +49,7 @@ func TestDeleteRequestsManager_Expired(t *testing.T) {
 		},
 		{
 			name:         "no relevant delete requests",
+			deletionMode: WholeStreamDeletion,
 			deleteRequestsFromStore: []DeleteRequest{
 				{
 					UserID:    "different-user",
@@ -62,6 +65,7 @@ func TestDeleteRequestsManager_Expired(t *testing.T) {
 		},
 		{
 			name:         "whole chunk deleted by single request",
+			deletionMode: WholeStreamDeletion,
 			deleteRequestsFromStore: []DeleteRequest{
 				{
 					UserID:    testUserID,
@@ -77,6 +81,7 @@ func TestDeleteRequestsManager_Expired(t *testing.T) {
 		},
 		{
 			name:         "deleted interval out of range",
+			deletionMode: WholeStreamDeletion,
 			deleteRequestsFromStore: []DeleteRequest{
 				{
 					UserID:    testUserID,
@@ -92,6 +97,7 @@ func TestDeleteRequestsManager_Expired(t *testing.T) {
 		},
 		{
 			name:         "multiple delete requests with one deleting the whole chunk",
+			deletionMode: WholeStreamDeletion,
 			deleteRequestsFromStore: []DeleteRequest{
 				{
 					UserID:    testUserID,
@@ -113,6 +119,7 @@ func TestDeleteRequestsManager_Expired(t *testing.T) {
 		},
 		{
 			name:         "multiple delete requests causing multiple holes",
+			deletionMode: WholeStreamDeletion,
 			deleteRequestsFromStore: []DeleteRequest{
 				{
 					UserID:    testUserID,
@@ -165,6 +172,7 @@ func TestDeleteRequestsManager_Expired(t *testing.T) {
 		},
 		{
 			name:         "multiple overlapping requests deleting the whole chunk",
+			deletionMode: WholeStreamDeletion,
 			deleteRequestsFromStore: []DeleteRequest{
 				{
 					UserID:    testUserID,
@@ -186,6 +194,7 @@ func TestDeleteRequestsManager_Expired(t *testing.T) {
 		},
 		{
 			name:         "multiple non-overlapping requests deleting the whole chunk",
+			deletionMode: WholeStreamDeletion,
 			deleteRequestsFromStore: []DeleteRequest{
 				{
 					UserID:    testUserID,
@@ -211,9 +220,77 @@ func TestDeleteRequestsManager_Expired(t *testing.T) {
 				nonDeletedIntervals: nil,
 			},
 		},
+		{
+			name:         "deletes are disabled",
+			deletionMode: Disabled,
+			deleteRequestsFromStore: []DeleteRequest{
+				{
+					UserID:    testUserID,
+					Query:     lblFoo.String(),
+					StartTime: now.Add(-13 * time.Hour),
+					EndTime:   now.Add(-11 * time.Hour),
+				},
+				{
+					UserID:    testUserID,
+					Query:     lblFoo.String(),
+					StartTime: now.Add(-10 * time.Hour),
+					EndTime:   now.Add(-8 * time.Hour),
+				},
+				{
+					UserID:    testUserID,
+					Query:     lblFoo.String(),
+					StartTime: now.Add(-6 * time.Hour),
+					EndTime:   now.Add(-5 * time.Hour),
+				},
+				{
+					UserID:    testUserID,
+					Query:     lblFoo.String(),
+					StartTime: now.Add(-2 * time.Hour),
+					EndTime:   now,
+				},
+			},
+			expectedResp: resp{
+				isExpired:           false,
+				nonDeletedIntervals: nil,
+			},
+		},
+		{
+			name:         "deletes are `filter-only`",
+			deletionMode: FilterOnly,
+			deleteRequestsFromStore: []DeleteRequest{
+				{
+					UserID:    testUserID,
+					Query:     lblFoo.String(),
+					StartTime: now.Add(-13 * time.Hour),
+					EndTime:   now.Add(-11 * time.Hour),
+				},
+				{
+					UserID:    testUserID,
+					Query:     lblFoo.String(),
+					StartTime: now.Add(-10 * time.Hour),
+					EndTime:   now.Add(-8 * time.Hour),
+				},
+				{
+					UserID:    testUserID,
+					Query:     lblFoo.String(),
+					StartTime: now.Add(-6 * time.Hour),
+					EndTime:   now.Add(-5 * time.Hour),
+				},
+				{
+					UserID:    testUserID,
+					Query:     lblFoo.String(),
+					StartTime: now.Add(-2 * time.Hour),
+					EndTime:   now,
+				},
+			},
+			expectedResp: resp{
+				isExpired:           false,
+				nonDeletedIntervals: nil,
+			},
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			mgr := NewDeleteRequestsManager(mockDeleteRequestsStore{deleteRequests: tc.deleteRequestsFromStore}, time.Hour, nil, WholeStreamDeletion)
+			mgr := NewDeleteRequestsManager(mockDeleteRequestsStore{deleteRequests: tc.deleteRequestsFromStore}, time.Hour, nil, tc.deletionMode)
 			require.NoError(t, mgr.loadDeleteRequestsToProcess())
 
 			isExpired, nonDeletedIntervals := mgr.Expired(chunkEntry, model.Now())