From 04d6cfdcffa9889b65f4fbef74cf87be8b0ca31f Mon Sep 17 00:00:00 2001 From: Travis Patterson Date: Wed, 1 Jun 2022 14:20:54 -0600 Subject: [PATCH 1/2] Only delete data when WholeStreamDeletion or FilterAndDelete --- .../deletion/delete_requests_manager.go | 5 + .../deletion/delete_requests_manager_test.go | 95 +++++++++++++++++-- 2 files changed, 91 insertions(+), 9 deletions(-) diff --git a/pkg/storage/stores/shipper/compactor/deletion/delete_requests_manager.go b/pkg/storage/stores/shipper/compactor/deletion/delete_requests_manager.go index 057a020fdf73f..0a4b2c9a7e29f 100644 --- a/pkg/storage/stores/shipper/compactor/deletion/delete_requests_manager.go +++ b/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{ diff --git a/pkg/storage/stores/shipper/compactor/deletion/delete_requests_manager_test.go b/pkg/storage/stores/shipper/compactor/deletion/delete_requests_manager_test.go index 6f5f95caf6108..1d524e1c142cf 100644 --- a/pkg/storage/stores/shipper/compactor/deletion/delete_requests_manager_test.go +++ b/pkg/storage/stores/shipper/compactor/deletion/delete_requests_manager_test.go @@ -35,18 +35,21 @@ func TestDeleteRequestsManager_Expired(t *testing.T) { for _, tc := range []struct { name string + deletionMode Mode deleteRequestsFromStore []DeleteRequest expectedResp resp }{ { - name: "no delete requests", + name: "no delete requests", + deletionMode: WholeStreamDeletion, expectedResp: resp{ isExpired: false, nonDeletedIntervals: nil, }, }, { - name: "no relevant delete requests", + name: "no relevant delete requests", + deletionMode: WholeStreamDeletion, deleteRequestsFromStore: []DeleteRequest{ { UserID: "different-user", @@ -61,7 +64,8 @@ func TestDeleteRequestsManager_Expired(t *testing.T) { }, }, { - name: "whole chunk deleted by single request", + name: "whole chunk deleted by single request", + deletionMode: WholeStreamDeletion, deleteRequestsFromStore: []DeleteRequest{ { UserID: testUserID, @@ -76,7 +80,8 @@ func TestDeleteRequestsManager_Expired(t *testing.T) { }, }, { - name: "deleted interval out of range", + name: "deleted interval out of range", + deletionMode: WholeStreamDeletion, deleteRequestsFromStore: []DeleteRequest{ { UserID: testUserID, @@ -91,7 +96,8 @@ func TestDeleteRequestsManager_Expired(t *testing.T) { }, }, { - name: "multiple delete requests with one deleting the whole chunk", + name: "multiple delete requests with one deleting the whole chunk", + deletionMode: WholeStreamDeletion, deleteRequestsFromStore: []DeleteRequest{ { UserID: testUserID, @@ -112,7 +118,8 @@ func TestDeleteRequestsManager_Expired(t *testing.T) { }, }, { - name: "multiple delete requests causing multiple holes", + name: "multiple delete requests causing multiple holes", + deletionMode: WholeStreamDeletion, deleteRequestsFromStore: []DeleteRequest{ { UserID: testUserID, @@ -164,7 +171,8 @@ func TestDeleteRequestsManager_Expired(t *testing.T) { }, }, { - name: "multiple overlapping requests deleting the whole chunk", + name: "multiple overlapping requests deleting the whole chunk", + deletionMode: WholeStreamDeletion, deleteRequestsFromStore: []DeleteRequest{ { UserID: testUserID, @@ -185,7 +193,8 @@ func TestDeleteRequestsManager_Expired(t *testing.T) { }, }, { - name: "multiple non-overlapping requests deleting the whole chunk", + 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()) From 1a9ddea01004c296887ef27f2a5b8ef1f32c77aa Mon Sep 17 00:00:00 2001 From: Travis Patterson Date: Thu, 2 Jun 2022 11:33:32 -0600 Subject: [PATCH 2/2] remove broken snyn workflow --- .github/workflows/snyk.yml | 54 -------------------------------------- 1 file changed, 54 deletions(-) delete mode 100644 .github/workflows/snyk.yml diff --git a/.github/workflows/snyk.yml b/.github/workflows/snyk.yml deleted file mode 100644 index 824d888b1af3c..0000000000000 --- a/.github/workflows/snyk.yml +++ /dev/null @@ -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