Skip to content

Commit 36752a8

Browse files
pascal-fischerpascal-fischer
authored
[proxy] add access log cleanup (#5376)
1 parent f117fc7 commit 36752a8

File tree

10 files changed

+422
-0
lines changed

10 files changed

+422
-0
lines changed

‎management/internals/modules/reverseproxy/accesslogs/interface.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,7 @@ import (
77
type Manager interface {
88
SaveAccessLog(ctx context.Context, proxyLog *AccessLogEntry) error
99
GetAllAccessLogs(ctx context.Context, accountID, userID string, filter *AccessLogFilter) ([]*AccessLogEntry, int64, error)
10+
CleanupOldAccessLogs(ctx context.Context, retentionDays int) (int64, error)
11+
StartPeriodicCleanup(ctx context.Context, retentionDays, cleanupIntervalHours int)
12+
StopPeriodicCleanup()
1013
}

‎management/internals/modules/reverseproxy/accesslogs/manager/manager.go‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package manager
33
import (
44
"context"
55
"strings"
6+
"time"
67

78
log "github.com/sirupsen/logrus"
89

@@ -19,6 +20,7 @@ type managerImpl struct {
1920
store store.Store
2021
permissionsManager permissions.Manager
2122
geo geolocation.Geolocation
23+
cleanupCancel context.CancelFunc
2224
}
2325

2426
func NewManager(store store.Store, permissionsManager permissions.Manager, geo geolocation.Geolocation) accesslogs.Manager {
@@ -78,6 +80,74 @@ func (m *managerImpl) GetAllAccessLogs(ctx context.Context, accountID, userID st
7880
return logs, totalCount, nil
7981
}
8082

83+
// CleanupOldAccessLogs deletes access logs older than the specified retention period
84+
func (m *managerImpl) CleanupOldAccessLogs(ctx context.Context, retentionDays int) (int64, error) {
85+
if retentionDays <= 0 {
86+
log.WithContext(ctx).Debug("access log cleanup skipped: retention days is 0 or negative")
87+
return 0, nil
88+
}
89+
90+
cutoffTime := time.Now().AddDate(0, 0, -retentionDays)
91+
deletedCount, err := m.store.DeleteOldAccessLogs(ctx, cutoffTime)
92+
if err != nil {
93+
log.WithContext(ctx).Errorf("failed to cleanup old access logs: %v", err)
94+
return 0, err
95+
}
96+
97+
if deletedCount > 0 {
98+
log.WithContext(ctx).Infof("cleaned up %d access logs older than %d days", deletedCount, retentionDays)
99+
}
100+
101+
return deletedCount, nil
102+
}
103+
104+
// StartPeriodicCleanup starts a background goroutine that periodically cleans up old access logs
105+
func (m *managerImpl) StartPeriodicCleanup(ctx context.Context, retentionDays, cleanupIntervalHours int) {
106+
if retentionDays <= 0 {
107+
log.WithContext(ctx).Debug("periodic access log cleanup disabled: retention days is 0 or negative")
108+
return
109+
}
110+
111+
if cleanupIntervalHours <= 0 {
112+
cleanupIntervalHours = 24
113+
}
114+
115+
cleanupCtx, cancel := context.WithCancel(ctx)
116+
m.cleanupCancel = cancel
117+
118+
cleanupInterval := time.Duration(cleanupIntervalHours) * time.Hour
119+
ticker := time.NewTicker(cleanupInterval)
120+
121+
go func() {
122+
defer ticker.Stop()
123+
124+
// Run cleanup immediately on startup
125+
log.WithContext(cleanupCtx).Infof("starting access log cleanup routine (retention: %d days, interval: %d hours)", retentionDays, cleanupIntervalHours)
126+
if _, err := m.CleanupOldAccessLogs(cleanupCtx, retentionDays); err != nil {
127+
log.WithContext(cleanupCtx).Errorf("initial access log cleanup failed: %v", err)
128+
}
129+
130+
for {
131+
select {
132+
case <-cleanupCtx.Done():
133+
log.WithContext(cleanupCtx).Info("stopping access log cleanup routine")
134+
return
135+
case <-ticker.C:
136+
if _, err := m.CleanupOldAccessLogs(cleanupCtx, retentionDays); err != nil {
137+
log.WithContext(cleanupCtx).Errorf("periodic access log cleanup failed: %v", err)
138+
}
139+
}
140+
}
141+
}()
142+
}
143+
144+
// StopPeriodicCleanup stops the periodic cleanup routine
145+
func (m *managerImpl) StopPeriodicCleanup() {
146+
if m.cleanupCancel != nil {
147+
m.cleanupCancel()
148+
}
149+
}
150+
81151
// resolveUserFilters converts user email/name filters to user ID filter
82152
func (m *managerImpl) resolveUserFilters(ctx context.Context, accountID string, filter *accesslogs.AccessLogFilter) error {
83153
if filter.UserEmail == nil && filter.UserName == nil {

‎management/internals/modules/reverseproxy/accesslogs/manager/manager_test.go‎

Lines changed: 281 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,281 @@
1+
package manager
2+
3+
import (
4+
"context"
5+
"testing"
6+
"time"
7+
8+
"github.com/golang/mock/gomock"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
12+
"github.com/netbirdio/netbird/management/server/store"
13+
)
14+
15+
func TestCleanupOldAccessLogs(t *testing.T) {
16+
tests := []struct {
17+
name string
18+
retentionDays int
19+
setupMock func(*store.MockStore)
20+
expectedCount int64
21+
expectedError bool
22+
}{
23+
{
24+
name: "cleanup logs older than retention period",
25+
retentionDays: 30,
26+
setupMock: func(mockStore *store.MockStore) {
27+
mockStore.EXPECT().
28+
DeleteOldAccessLogs(gomock.Any(), gomock.Any()).
29+
DoAndReturn(func(ctx context.Context, olderThan time.Time) (int64, error) {
30+
expectedCutoff := time.Now().AddDate(0, 0, -30)
31+
timeDiff := olderThan.Sub(expectedCutoff)
32+
if timeDiff.Abs() > time.Second {
33+
t.Errorf("cutoff time not as expected: got %v, want ~%v", olderThan, expectedCutoff)
34+
}
35+
return 5, nil
36+
})
37+
},
38+
expectedCount: 5,
39+
expectedError: false,
40+
},
41+
{
42+
name: "no logs to cleanup",
43+
retentionDays: 30,
44+
setupMock: func(mockStore *store.MockStore) {
45+
mockStore.EXPECT().
46+
DeleteOldAccessLogs(gomock.Any(), gomock.Any()).
47+
Return(int64(0), nil)
48+
},
49+
expectedCount: 0,
50+
expectedError: false,
51+
},
52+
{
53+
name: "zero retention days skips cleanup",
54+
retentionDays: 0,
55+
setupMock: func(mockStore *store.MockStore) {
56+
// No expectations - DeleteOldAccessLogs should not be called
57+
},
58+
expectedCount: 0,
59+
expectedError: false,
60+
},
61+
{
62+
name: "negative retention days skips cleanup",
63+
retentionDays: -10,
64+
setupMock: func(mockStore *store.MockStore) {
65+
// No expectations - DeleteOldAccessLogs should not be called
66+
},
67+
expectedCount: 0,
68+
expectedError: false,
69+
},
70+
}
71+
72+
for _, tt := range tests {
73+
t.Run(tt.name, func(t *testing.T) {
74+
ctrl := gomock.NewController(t)
75+
defer ctrl.Finish()
76+
77+
mockStore := store.NewMockStore(ctrl)
78+
tt.setupMock(mockStore)
79+
80+
manager := &managerImpl{
81+
store: mockStore,
82+
}
83+
84+
ctx := context.Background()
85+
deletedCount, err := manager.CleanupOldAccessLogs(ctx, tt.retentionDays)
86+
87+
if tt.expectedError {
88+
require.Error(t, err)
89+
} else {
90+
require.NoError(t, err)
91+
}
92+
assert.Equal(t, tt.expectedCount, deletedCount, "unexpected number of deleted logs")
93+
})
94+
}
95+
}
96+
97+
func TestCleanupWithExactBoundary(t *testing.T) {
98+
ctrl := gomock.NewController(t)
99+
defer ctrl.Finish()
100+
101+
mockStore := store.NewMockStore(ctrl)
102+
103+
mockStore.EXPECT().
104+
DeleteOldAccessLogs(gomock.Any(), gomock.Any()).
105+
DoAndReturn(func(ctx context.Context, olderThan time.Time) (int64, error) {
106+
expectedCutoff := time.Now().AddDate(0, 0, -30)
107+
timeDiff := olderThan.Sub(expectedCutoff)
108+
assert.Less(t, timeDiff.Abs(), time.Second, "cutoff time should be close to expected value")
109+
return 1, nil
110+
})
111+
112+
manager := &managerImpl{
113+
store: mockStore,
114+
}
115+
116+
ctx := context.Background()
117+
deletedCount, err := manager.CleanupOldAccessLogs(ctx, 30)
118+
119+
require.NoError(t, err)
120+
assert.Equal(t, int64(1), deletedCount)
121+
}
122+
123+
func TestStartPeriodicCleanup(t *testing.T) {
124+
t.Run("periodic cleanup disabled with zero retention", func(t *testing.T) {
125+
ctrl := gomock.NewController(t)
126+
defer ctrl.Finish()
127+
128+
mockStore := store.NewMockStore(ctrl)
129+
// No expectations - cleanup should not run
130+
131+
manager := &managerImpl{
132+
store: mockStore,
133+
}
134+
135+
ctx, cancel := context.WithCancel(context.Background())
136+
defer cancel()
137+
138+
manager.StartPeriodicCleanup(ctx, 0, 1)
139+
140+
time.Sleep(100 * time.Millisecond)
141+
142+
// If DeleteOldAccessLogs was called, the test will fail due to unexpected call
143+
})
144+
145+
t.Run("periodic cleanup runs immediately on start", func(t *testing.T) {
146+
ctrl := gomock.NewController(t)
147+
defer ctrl.Finish()
148+
149+
mockStore := store.NewMockStore(ctrl)
150+
151+
mockStore.EXPECT().
152+
DeleteOldAccessLogs(gomock.Any(), gomock.Any()).
153+
Return(int64(2), nil).
154+
Times(1)
155+
156+
manager := &managerImpl{
157+
store: mockStore,
158+
}
159+
160+
ctx, cancel := context.WithCancel(context.Background())
161+
defer cancel()
162+
163+
manager.StartPeriodicCleanup(ctx, 30, 24)
164+
165+
time.Sleep(200 * time.Millisecond)
166+
167+
// Expectations verified by gomock on defer ctrl.Finish()
168+
})
169+
170+
t.Run("periodic cleanup stops on context cancel", func(t *testing.T) {
171+
ctrl := gomock.NewController(t)
172+
defer ctrl.Finish()
173+
174+
mockStore := store.NewMockStore(ctrl)
175+
176+
mockStore.EXPECT().
177+
DeleteOldAccessLogs(gomock.Any(), gomock.Any()).
178+
Return(int64(1), nil).
179+
Times(1)
180+
181+
manager := &managerImpl{
182+
store: mockStore,
183+
}
184+
185+
ctx, cancel := context.WithCancel(context.Background())
186+
187+
manager.StartPeriodicCleanup(ctx, 30, 24)
188+
189+
time.Sleep(100 * time.Millisecond)
190+
191+
cancel()
192+
193+
time.Sleep(200 * time.Millisecond)
194+
195+
})
196+
197+
t.Run("cleanup interval defaults to 24 hours when invalid", func(t *testing.T) {
198+
ctrl := gomock.NewController(t)
199+
defer ctrl.Finish()
200+
201+
mockStore := store.NewMockStore(ctrl)
202+
203+
mockStore.EXPECT().
204+
DeleteOldAccessLogs(gomock.Any(), gomock.Any()).
205+
Return(int64(0), nil).
206+
Times(1)
207+
208+
manager := &managerImpl{
209+
store: mockStore,
210+
}
211+
212+
ctx, cancel := context.WithCancel(context.Background())
213+
defer cancel()
214+
215+
manager.StartPeriodicCleanup(ctx, 30, 0)
216+
217+
time.Sleep(100 * time.Millisecond)
218+
219+
manager.StopPeriodicCleanup()
220+
})
221+
222+
t.Run("cleanup interval uses configured hours", func(t *testing.T) {
223+
ctrl := gomock.NewController(t)
224+
defer ctrl.Finish()
225+
226+
mockStore := store.NewMockStore(ctrl)
227+
228+
mockStore.EXPECT().
229+
DeleteOldAccessLogs(gomock.Any(), gomock.Any()).
230+
Return(int64(3), nil).
231+
Times(1)
232+
233+
manager := &managerImpl{
234+
store: mockStore,
235+
}
236+
237+
ctx, cancel := context.WithCancel(context.Background())
238+
defer cancel()
239+
240+
manager.StartPeriodicCleanup(ctx, 30, 12)
241+
242+
time.Sleep(100 * time.Millisecond)
243+
244+
manager.StopPeriodicCleanup()
245+
})
246+
}
247+
248+
func TestStopPeriodicCleanup(t *testing.T) {
249+
ctrl := gomock.NewController(t)
250+
defer ctrl.Finish()
251+
252+
mockStore := store.NewMockStore(ctrl)
253+
254+
mockStore.EXPECT().
255+
DeleteOldAccessLogs(gomock.Any(), gomock.Any()).
256+
Return(int64(1), nil).
257+
Times(1)
258+
259+
manager := &managerImpl{
260+
store: mockStore,
261+
}
262+
263+
ctx := context.Background()
264+
265+
manager.StartPeriodicCleanup(ctx, 30, 24)
266+
267+
time.Sleep(100 * time.Millisecond)
268+
269+
manager.StopPeriodicCleanup()
270+
271+
time.Sleep(200 * time.Millisecond)
272+
273+
// Expectations verified by gomock - would fail if more than 1 call happened
274+
}
275+
276+
func TestStopPeriodicCleanup_NotStarted(t *testing.T) {
277+
manager := &managerImpl{}
278+
279+
// Should not panic if cleanup was never started
280+
manager.StopPeriodicCleanup()
281+
}

‎management/internals/server/boot.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,11 @@ func (s *BaseServer) ProxyTokenStore() *nbgrpc.OneTimeTokenStore {
197197
func (s *BaseServer) AccessLogsManager() accesslogs.Manager {
198198
return Create(s, func() accesslogs.Manager {
199199
accessLogManager := accesslogsmanager.NewManager(s.Store(), s.PermissionsManager(), s.GeoLocationManager())
200+
accessLogManager.StartPeriodicCleanup(
201+
context.Background(),
202+
s.Config.ReverseProxy.AccessLogRetentionDays,
203+
s.Config.ReverseProxy.AccessLogCleanupIntervalHours,
204+
)
200205
return accessLogManager
201206
})
202207
}

0 commit comments

Comments
 (0)