From 1d184b8a42a58f78c9e7332da3a1ade31dcc94a6 Mon Sep 17 00:00:00 2001 From: Viktor Torstensson Date: Thu, 16 Oct 2025 16:16:06 +0200 Subject: [PATCH] firewalldb: actions mig handle empty RPCParamsJson If there are no RPCParamsJson set for an action, the value is represented differently in KVDB vs SQL. In the SQL DB, empty RPCParamsJson are represented as nil, while they are represented as an empty array in the KVDB version. Therefore, we need to override the RPCParamsJson in that scenario, so that they are set to the same representation when a KVDB and an SQL action is compared. --- firewalldb/sql_migration.go | 16 ++++++++++++ firewalldb/sql_migration_test.go | 43 ++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/firewalldb/sql_migration.go b/firewalldb/sql_migration.go index 1664ea073..90b70142e 100644 --- a/firewalldb/sql_migration.go +++ b/firewalldb/sql_migration.go @@ -1131,6 +1131,11 @@ func validateMigratedAction(ctx context.Context, sqlTx SQLQueries, // the SQL action has the full 8 bytes set. overrideMacRootKeyID(migAction) + // If there are no RPCParamsJson set for the actions, that's represented + // differently in KVDB vs SQL. We therefore override the RPCParamsJson + // so that both actions represent them in the same way. + overrideRPCParamsJson(kvAction, migAction) + // Now that we have overridden the fields that are expected to differ // between the original KVDB action and the migrated SQL action, we can // compare the two actions to ensure that they match. @@ -1606,3 +1611,14 @@ func overrideMacRootKeyID(action *Action) { action.MacaroonRootKeyID = fn.Some(last32) }) } + +// overrideRPCParamsJson overrides the SQL action's RPCParamsJson in case they +// are empty for the kvAction. In the SQL DB, empty RPCParamsJson are +// represented as nil, while they are represented as an empty array in the +// KVDB version. Therefore, this function overrides the SQL action's +// RPCParamsJson to an empty array if they are nil. +func overrideRPCParamsJson(kvAction *Action, sqlAction *Action) { + if len(kvAction.RPCParamsJson) == 0 && sqlAction.RPCParamsJson == nil { + sqlAction.RPCParamsJson = []byte{} + } +} diff --git a/firewalldb/sql_migration_test.go b/firewalldb/sql_migration_test.go index 314dcc55a..689720d2f 100644 --- a/firewalldb/sql_migration_test.go +++ b/firewalldb/sql_migration_test.go @@ -474,6 +474,10 @@ func TestFirewallDBMigration(t *testing.T) { name: "action with no session or account", populateDB: actionNoSessionOrAccount, }, + { + name: "action with empty RPCParamsJson", + populateDB: actionEmptyRPCParamsJson, + }, { name: "action with session but no account", populateDB: actionWithSessionNoAccount, @@ -1268,6 +1272,32 @@ func actionNoSessionOrAccount(t *testing.T, ctx context.Context, } } +// actionEmptyRPCParamsJson adds an action which has no RPCParamsJson set. +func actionEmptyRPCParamsJson(t *testing.T, ctx context.Context, + boltDB *BoltDB, _ session.Store, _ accounts.Store, + rStore *rootKeyMockStore) *expectedResult { + + // As the action is not linked to any session, we add a random root + // key which we use as the macaroon identifier for the action. + // This simulates how similar actions would have been created in + // production. + rootKey := rStore.addRandomRootKey() + + actionReq := testActionReq + actionReq.MacaroonRootKeyID = fn.Some(rootKey) + actionReq.SessionID = fn.None[session.ID]() + actionReq.AccountID = fn.None[accounts.AccountID]() + actionReq.RPCParamsJson = []byte{} + + action := addAction(t, ctx, boltDB, &actionReq) + + return &expectedResult{ + kvEntries: []*kvEntry{}, + privPairs: make(privacyPairs), + actions: []*Action{action}, + } +} + // actionWithSessionNoAccount adds an action which is linked a session but no // account. func actionWithSessionNoAccount(t *testing.T, ctx context.Context, @@ -1967,6 +1997,14 @@ func addAction(t *testing.T, ctx context.Context, boltDB *BoltDB, action.AccountID = actionReq.AccountID action.MacaroonRootKeyID = actionReq.MacaroonRootKeyID + // In case the actionReq's RPCParamsJson wasn't set, we need to set the + // expected action's RPCParamsJson to nil as that's how such + // RPCParamsJson are represented in the SQL database, when the returned + // expected action should reflect. + if len(actionReq.RPCParamsJson) == 0 { + action.RPCParamsJson = nil + } + return action } @@ -2155,6 +2193,11 @@ func randomBytes(n int) []byte { // Keys are random strings like "key1", "key2"... // Values are random ints, floats, or strings. func randomJSON(n int) (string, error) { + // When 0 pairs are requested, we can return immediately. + if n <= 0 { + return "", nil + } + obj := make(map[string]any, n) for i := 0; i < n; i++ { key := fmt.Sprintf("key%d", i+1)