Skip to content

Commit 1e257d1

Browse files
ellemoutonellemouton
authored
Merge pull request #1029 from ellemouton/sql28
[sql-28] firewall+sessions: clean up in preparation for SQL backend
2 parents 846dc68 + 771bc31 commit 1e257d1

File tree

6 files changed

+187
-66
lines changed

6 files changed

+187
-66
lines changed

‎firewalldb/kvstores_kvdb.go

+54-45
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ func (db *BoltDB) GetKVStores(rule string, groupID session.ID,
6060
db: db.DB,
6161
wrapTx: func(tx *bbolt.Tx) KVStoreTx {
6262
return &kvStoreTx{
63-
boltTx: tx,
63+
boltTx: tx,
64+
sessions: db.sessionIDIndex,
6465
kvStores: &kvStores{
6566
ruleName: rule,
6667
groupID: groupID,
@@ -109,18 +110,20 @@ type getBucketFunc func(tx *bbolt.Tx, create bool) (*bbolt.Bucket, error)
109110
type kvStoreTx struct {
110111
boltTx *bbolt.Tx
111112
getBucket getBucketFunc
113+
sessions session.IDToGroupIndex
112114

113115
*kvStores
114116
}
115117

116118
// Global gives the caller access to the global kv store of the rule.
117119
//
118120
// NOTE: this is part of the rules.KVStoreTx interface.
119-
func (tx *kvStoreTx) Global() KVStore {
121+
func (s *kvStoreTx) Global() KVStore {
120122
return &kvStoreTx{
121-
kvStores: tx.kvStores,
122-
boltTx: tx.boltTx,
123-
getBucket: getGlobalRuleBucket(true, tx.ruleName),
123+
kvStores: s.kvStores,
124+
boltTx: s.boltTx,
125+
sessions: s.sessions,
126+
getBucket: getGlobalRuleBucket(true, s.ruleName),
124127
}
125128
}
126129

@@ -129,17 +132,16 @@ func (tx *kvStoreTx) Global() KVStore {
129132
// how the kv store was initialised.
130133
//
131134
// NOTE: this is part of the KVStoreTx interface.
132-
func (tx *kvStoreTx) Local() KVStore {
133-
fn := getSessionRuleBucket(true, tx.ruleName, tx.groupID)
134-
if tx.featureName != "" {
135-
fn = getSessionFeatureRuleBucket(
136-
true, tx.ruleName, tx.groupID, tx.featureName,
137-
)
135+
func (s *kvStoreTx) Local() KVStore {
136+
fn := s.getSessionRuleBucket(true)
137+
if s.featureName != "" {
138+
fn = s.getSessionFeatureRuleBucket(true)
138139
}
139140

140141
return &kvStoreTx{
141-
kvStores: tx.kvStores,
142-
boltTx: tx.boltTx,
142+
kvStores: s.kvStores,
143+
boltTx: s.boltTx,
144+
sessions: s.sessions,
143145
getBucket: fn,
144146
}
145147
}
@@ -148,29 +150,29 @@ func (tx *kvStoreTx) Local() KVStore {
148150
// rule.
149151
//
150152
// NOTE: this is part of the KVStoreTx interface.
151-
func (tx *kvStoreTx) GlobalTemp() KVStore {
153+
func (s *kvStoreTx) GlobalTemp() KVStore {
152154
return &kvStoreTx{
153-
kvStores: tx.kvStores,
154-
boltTx: tx.boltTx,
155-
getBucket: getGlobalRuleBucket(false, tx.ruleName),
155+
kvStores: s.kvStores,
156+
boltTx: s.boltTx,
157+
sessions: s.sessions,
158+
getBucket: getGlobalRuleBucket(false, s.ruleName),
156159
}
157160
}
158161

159162
// LocalTemp gives the caller access to the temporary local kv store of the
160163
// rule.
161164
//
162165
// NOTE: this is part of the KVStoreTx interface.
163-
func (tx *kvStoreTx) LocalTemp() KVStore {
164-
fn := getSessionRuleBucket(false, tx.ruleName, tx.groupID)
165-
if tx.featureName != "" {
166-
fn = getSessionFeatureRuleBucket(
167-
false, tx.ruleName, tx.groupID, tx.featureName,
168-
)
166+
func (s *kvStoreTx) LocalTemp() KVStore {
167+
fn := s.getSessionRuleBucket(false)
168+
if s.featureName != "" {
169+
fn = s.getSessionFeatureRuleBucket(false)
169170
}
170171

171172
return &kvStoreTx{
172-
kvStores: tx.kvStores,
173-
boltTx: tx.boltTx,
173+
kvStores: s.kvStores,
174+
boltTx: s.boltTx,
175+
sessions: s.sessions,
174176
getBucket: fn,
175177
}
176178
}
@@ -179,8 +181,8 @@ func (tx *kvStoreTx) LocalTemp() KVStore {
179181
// If no value is found, nil is returned.
180182
//
181183
// NOTE: this is part of the KVStore interface.
182-
func (tx *kvStoreTx) Get(_ context.Context, key string) ([]byte, error) {
183-
bucket, err := tx.getBucket(tx.boltTx, false)
184+
func (s *kvStoreTx) Get(_ context.Context, key string) ([]byte, error) {
185+
bucket, err := s.getBucket(s.boltTx, false)
184186
if err != nil {
185187
return nil, err
186188
}
@@ -194,8 +196,8 @@ func (tx *kvStoreTx) Get(_ context.Context, key string) ([]byte, error) {
194196
// Set sets the given key-value pair in the underlying kv store.
195197
//
196198
// NOTE: this is part of the KVStore interface.
197-
func (tx *kvStoreTx) Set(_ context.Context, key string, value []byte) error {
198-
bucket, err := tx.getBucket(tx.boltTx, true)
199+
func (s *kvStoreTx) Set(_ context.Context, key string, value []byte) error {
200+
bucket, err := s.getBucket(s.boltTx, true)
199201
if err != nil {
200202
return err
201203
}
@@ -206,8 +208,8 @@ func (tx *kvStoreTx) Set(_ context.Context, key string, value []byte) error {
206208
// Del deletes the value under the given key in the underlying kv store.
207209
//
208210
// NOTE: this is part of the .KVStore interface.
209-
func (tx *kvStoreTx) Del(_ context.Context, key string) error {
210-
bucket, err := tx.getBucket(tx.boltTx, false)
211+
func (s *kvStoreTx) Del(_ context.Context, key string) error {
212+
bucket, err := s.getBucket(s.boltTx, false)
211213
if err != nil {
212214
return err
213215
}
@@ -286,11 +288,9 @@ func getGlobalRuleBucket(perm bool, ruleName string) getBucketFunc {
286288
// bucket under which a kv store for a specific rule-name and group ID is
287289
// stored. The `perm` param determines if the temporary or permanent store is
288290
// used.
289-
func getSessionRuleBucket(perm bool, ruleName string,
290-
groupID session.ID) getBucketFunc {
291-
291+
func (s *kvStoreTx) getSessionRuleBucket(perm bool) getBucketFunc {
292292
return func(tx *bbolt.Tx, create bool) (*bbolt.Bucket, error) {
293-
ruleBucket, err := getRuleBucket(perm, ruleName)(tx, create)
293+
ruleBucket, err := getRuleBucket(perm, s.ruleName)(tx, create)
294294
if err != nil {
295295
return nil, err
296296
}
@@ -300,35 +300,44 @@ func getSessionRuleBucket(perm bool, ruleName string,
300300
}
301301

302302
if create {
303+
// NOTE: for a bbolt backend, the context is in any case
304+
// dropped behind the GetSessionIDs call. So passing in
305+
// a new context here is not a problem.
306+
ctx := context.Background()
307+
308+
// If create is true, we expect this to be an existing
309+
// session. So we check that now and return an error
310+
// accordingly if the session does not exist.
311+
_, err := s.sessions.GetSessionIDs(ctx, s.groupID)
312+
if err != nil {
313+
return nil, err
314+
}
315+
303316
sessBucket, err := ruleBucket.CreateBucketIfNotExists(
304317
sessKVStoreBucketKey,
305318
)
306319
if err != nil {
307320
return nil, err
308321
}
309322

310-
return sessBucket.CreateBucketIfNotExists(groupID[:])
323+
return sessBucket.CreateBucketIfNotExists(s.groupID[:])
311324
}
312325

313326
sessBucket := ruleBucket.Bucket(sessKVStoreBucketKey)
314327
if sessBucket == nil {
315328
return nil, nil
316329
}
317-
return sessBucket.Bucket(groupID[:]), nil
330+
return sessBucket.Bucket(s.groupID[:]), nil
318331
}
319332
}
320333

321334
// getSessionFeatureRuleBucket returns a function that can be used to fetch the
322335
// bucket under which a kv store for a specific rule-name, group ID and
323336
// feature name is stored. The `perm` param determines if the temporary or
324337
// permanent store is used.
325-
func getSessionFeatureRuleBucket(perm bool, ruleName string,
326-
groupID session.ID, featureName string) getBucketFunc {
327-
338+
func (s *kvStoreTx) getSessionFeatureRuleBucket(perm bool) getBucketFunc {
328339
return func(tx *bbolt.Tx, create bool) (*bbolt.Bucket, error) {
329-
sessBucket, err := getSessionRuleBucket(
330-
perm, ruleName, groupID,
331-
)(tx, create)
340+
sessBucket, err := s.getSessionRuleBucket(perm)(tx, create)
332341
if err != nil {
333342
return nil, err
334343
}
@@ -346,14 +355,14 @@ func getSessionFeatureRuleBucket(perm bool, ruleName string,
346355
}
347356

348357
return featureBucket.CreateBucketIfNotExists(
349-
[]byte(featureName),
358+
[]byte(s.featureName),
350359
)
351360
}
352361

353362
featureBucket := sessBucket.Bucket(featureKVStoreBucketKey)
354363
if featureBucket == nil {
355364
return nil, nil
356365
}
357-
return featureBucket.Bucket([]byte(featureName)), nil
366+
return featureBucket.Bucket([]byte(s.featureName)), nil
358367
}
359368
}

‎firewalldb/kvstores_test.go

+107-16
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ import (
55
"context"
66
"fmt"
77
"testing"
8+
"time"
89

910
"github.com/lightninglabs/lightning-terminal/session"
11+
"github.com/lightningnetwork/lnd/clock"
1012
"github.com/stretchr/testify/require"
1113
)
1214

@@ -83,15 +85,21 @@ func testTempAndPermStores(t *testing.T, featureSpecificStore bool) {
8385
featureName = "auto-fees"
8486
}
8587

86-
store := NewTestDB(t)
88+
sessions := session.NewTestDB(t, clock.NewDefaultClock())
89+
store := NewTestDBWithSessions(t, sessions)
8790
db := NewDB(store)
8891
require.NoError(t, db.Start(ctx))
8992

90-
kvstores := db.GetKVStores(
91-
"test-rule", [4]byte{1, 1, 1, 1}, featureName,
93+
// Create a session that we can reference.
94+
sess, err := sessions.NewSession(
95+
ctx, "test", session.TypeAutopilot, time.Unix(1000, 0),
96+
"something",
9297
)
98+
require.NoError(t, err)
99+
100+
kvstores := db.GetKVStores("test-rule", sess.GroupID, featureName)
93101

94-
err := kvstores.Update(ctx, func(ctx context.Context,
102+
err = kvstores.Update(ctx, func(ctx context.Context,
95103
tx KVStoreTx) error {
96104

97105
// Set an item in the temp store.
@@ -137,7 +145,7 @@ func testTempAndPermStores(t *testing.T, featureSpecificStore bool) {
137145
require.NoError(t, db.Stop())
138146
})
139147

140-
kvstores = db.GetKVStores("test-rule", [4]byte{1, 1, 1, 1}, featureName)
148+
kvstores = db.GetKVStores("test-rule", sess.GroupID, featureName)
141149

142150
// The temp store should no longer have the stored value but the perm
143151
// store should .
@@ -164,23 +172,31 @@ func testTempAndPermStores(t *testing.T, featureSpecificStore bool) {
164172
func TestKVStoreNameSpaces(t *testing.T) {
165173
t.Parallel()
166174
ctx := context.Background()
167-
db := NewTestDB(t)
168175

169-
var (
170-
groupID1 = intToSessionID(1)
171-
groupID2 = intToSessionID(2)
176+
sessions := session.NewTestDB(t, clock.NewDefaultClock())
177+
db := NewTestDBWithSessions(t, sessions)
178+
179+
// Create 2 sessions that we can reference.
180+
sess1, err := sessions.NewSession(
181+
ctx, "test", session.TypeAutopilot, time.Unix(1000, 0), "",
172182
)
183+
require.NoError(t, err)
184+
185+
sess2, err := sessions.NewSession(
186+
ctx, "test1", session.TypeAutopilot, time.Unix(1000, 0), "",
187+
)
188+
require.NoError(t, err)
173189

174190
// Two DBs for same group but different features.
175-
rulesDB1 := db.GetKVStores("test-rule", groupID1, "auto-fees")
176-
rulesDB2 := db.GetKVStores("test-rule", groupID1, "re-balance")
191+
rulesDB1 := db.GetKVStores("test-rule", sess1.GroupID, "auto-fees")
192+
rulesDB2 := db.GetKVStores("test-rule", sess1.GroupID, "re-balance")
177193

178194
// The third DB is for the same rule but a different group. It is
179195
// for the same feature as db 2.
180-
rulesDB3 := db.GetKVStores("test-rule", groupID2, "re-balance")
196+
rulesDB3 := db.GetKVStores("test-rule", sess2.GroupID, "re-balance")
181197

182198
// Test that the three ruleDBs share the same global space.
183-
err := rulesDB1.Update(ctx, func(ctx context.Context,
199+
err = rulesDB1.Update(ctx, func(ctx context.Context,
184200
tx KVStoreTx) error {
185201

186202
return tx.Global().Set(
@@ -311,9 +327,9 @@ func TestKVStoreNameSpaces(t *testing.T) {
311327
// Test that the group space is shared by the first two dbs but not
312328
// the third. To do this, we re-init the DB's but leave the feature
313329
// names out. This way, we will access the group storage.
314-
rulesDB1 = db.GetKVStores("test-rule", groupID1, "")
315-
rulesDB2 = db.GetKVStores("test-rule", groupID1, "")
316-
rulesDB3 = db.GetKVStores("test-rule", groupID2, "")
330+
rulesDB1 = db.GetKVStores("test-rule", sess1.GroupID, "")
331+
rulesDB2 = db.GetKVStores("test-rule", sess1.GroupID, "")
332+
rulesDB3 = db.GetKVStores("test-rule", sess2.GroupID, "")
317333

318334
err = rulesDB1.Update(ctx, func(ctx context.Context,
319335
tx KVStoreTx) error {
@@ -376,6 +392,81 @@ func TestKVStoreNameSpaces(t *testing.T) {
376392
require.True(t, bytes.Equal(v, []byte("thing 3")))
377393
}
378394

395+
// TestKVStoreSessionCoupling tests if we attempt to write to a kvstore that
396+
// is namespaced by a session that does not exist, then we should get an error.
397+
func TestKVStoreSessionCoupling(t *testing.T) {
398+
t.Parallel()
399+
ctx := context.Background()
400+
401+
sessions := session.NewTestDB(t, clock.NewDefaultClock())
402+
db := NewTestDBWithSessions(t, sessions)
403+
404+
// Get a kvstore namespaced by a session ID for a session that does
405+
// not exist.
406+
store := db.GetKVStores("AutoFees", [4]byte{1, 1, 1, 1}, "auto-fees")
407+
408+
err := store.Update(ctx, func(ctx context.Context,
409+
tx KVStoreTx) error {
410+
411+
// First, show that any call to the global namespace will not
412+
// error since it is not namespaced by a session.
413+
res, err := tx.Global().Get(ctx, "foo")
414+
require.NoError(t, err)
415+
require.Nil(t, res)
416+
417+
err = tx.Global().Set(ctx, "foo", []byte("bar"))
418+
require.NoError(t, err)
419+
420+
res, err = tx.Global().Get(ctx, "foo")
421+
require.NoError(t, err)
422+
require.Equal(t, []byte("bar"), res)
423+
424+
// Now we switch to the local store. We don't expect the Get
425+
// call to error since it should just return a nil value for
426+
// key that has not been set.
427+
_, err = tx.Local().Get(ctx, "foo")
428+
require.NoError(t, err)
429+
430+
// For Set, we expect an error since the session does not exist.
431+
err = tx.Local().Set(ctx, "foo", []byte("bar"))
432+
require.ErrorIs(t, err, session.ErrUnknownGroup)
433+
434+
// We again don't expect the error for delete since we just
435+
// expect it to return nil if the key is not found.
436+
err = tx.Local().Del(ctx, "foo")
437+
require.NoError(t, err)
438+
439+
return nil
440+
})
441+
require.NoError(t, err)
442+
443+
// Now, go and create a sessions in the session DB.
444+
sess, err := sessions.NewSession(
445+
ctx, "test", session.TypeAutopilot, time.Unix(1000, 0),
446+
"something",
447+
)
448+
require.NoError(t, err)
449+
450+
// Get a kvstore namespaced by a session ID for a session that now
451+
// does exist.
452+
store = db.GetKVStores("AutoFees", sess.GroupID, "auto-fees")
453+
454+
// Now, repeat the "Set" call for this session's kvstore to
455+
// show that it no longer errors.
456+
err = store.Update(ctx, func(ctx context.Context, tx KVStoreTx) error {
457+
// For Set, we expect an error since the session does not exist.
458+
err = tx.Local().Set(ctx, "foo", []byte("bar"))
459+
require.NoError(t, err)
460+
461+
res, err := tx.Local().Get(ctx, "foo")
462+
require.NoError(t, err)
463+
require.Equal(t, []byte("bar"), res)
464+
465+
return nil
466+
})
467+
require.NoError(t, err)
468+
}
469+
379470
func intToSessionID(i uint32) session.ID {
380471
var id session.ID
381472
byteOrder.PutUint32(id[:], i)

0 commit comments

Comments
 (0)