Prompt before automatically migrating legacy kvdb state to SQL when
litd starts with a SQL backend and active bbolt data is still present.
Detect prior migrations by checking for the SQL tombstone marker so
already-migrated stores can start without prompting. Add unit coverage
for the prompt flow and wire stdin through the itest harness so the
migration restart path can acknowledge the prompt automatically.
Migrate KV firewall actions by traversing the global actions-index
instead of iterating per-session buckets, so SQL action IDs follow the
same global creation order as the legacy KV store.
Also stabilize SQL action listing by ordering on created_at and id,
using id as a deterministic tie-breaker when multiple actions share the
same timestamp.
This is needed because KV actions have a real global sequence in
actions-index, while the old migration assigned SQL IDs based on bucket
traversal order. That could reorder legacy actions during migration.
Separately, ordering by created_at alone was not stable for equal
timestamps, so action queries could return different orders for the
same data.
NOTE: This commit explicitly edits the previous migration instead of
adding a new one. This is ok as SQL dbs are not yet supported in
production, so there are no live deployments to worry about.
Normalize action timestamp handling to UTC during KV and SQL
marshalling so migration validation behaves consistently on
Postgres backends outside UTC.
This also allows the action time comparison helper to use the
same timezone normalization approach as the other migration
paths.
Add clearer progress logging across the KVDB to SQL migration paths
for accounts, sessions, and firewalldb.
Log the total number of items to migrate where it is cheap to
determine up front, and emit progress updates every 100 migrated
items.
Mark the legacy kvdb stores as deprecated once the kvdb -> SQL
migration commits successfully. This prevents normal bbolt startup
from reopening accounts.db, session.db, or rules.db after their data
has already been migrated.
Add explicit deprecation checks to the three kvdb store open paths and
provide migration-only constructors that can still reopen deprecated
files when the SQL database is deleted or downgraded and the migration
must be rerun.
Use store-specific tombstones for the deprecation markers and add
tests that verify deprecated stores are rejected while migration
reruns continue to work.
In upcoming commits, we will introduce a new migration stream package
that will need to reference the db package, as well as the accounts,
session and firewalldb package in future commits. To avoid circular
dependencies, we therefore introduce a new migration stream that unit
tests can use, in order to avoid having to import the new migration
stream package.
Since the `assertMigrationResults` now expects a `*sqlcmig6.Queries`
instance to be passed directly into the function, there is no need to
create a separate *sqlcmig6.Queries instance than the one created when
the `*sqlcmig6.TxExecutor` transaction is created. We therefore
refactor the execution of the `assertMigrationResults` function to be
called in the scope of that transaction.
Due to this change, there's also no longer a need to return the `*SQLDB`
instance from the `makeSQLDB` helper function.
As the firewalldb package kvdb to sql migration tests creates `sqlc`
models to assert the migration results, we will need to update those
call sites to instead use the `sqlcmig6` models instead, in order to
be compatible with the `sqlcmig6.Queries` queries.
However, since we can't update the `SQLDB` methods to use `sqlcmig6`
models as params, we need to update the test code assertion to instead
use the `sqlc.Queries` object directly instead of the `SQLDB` object.
This makes it easy to swap that `sqlc.Queries` object to a
`sqlcmig6.Queries` object in the commit that updates the firewalldb
package to use the `sqlcmig6` package for the kvdb to sql migration.
As the legacy `NewTestPostgresDB` function is no longer used and has
been removed, it no longer makes sense to have a `V2` suffix on the
`NewTestPostgresV2DB` function. This commit renames it to
`NewTestPostgresDB`, to indicate that this function now replaces the
legacy function.
rename `sqlStore` to `store` in the firewalldb sql migration test file,
to make the name shorted. This is done in preparation for future commits
which will lengthen the lines where `sqlStore` is used, which otherwise
would make the lines exceed the 80 character limit.
Previous commits had forgotten to add the `ListAllKVStoresRecords` query
to the `firewalldb.SQLKVStoreQueries` interface. As that is required to
make the query useable when defining the `sqldb/v2`
`TransactionExecutor` for the `firewalldb` package, this commit
adds it to the interface.
- Replace occurrences of `// nolint:lll` with `// nolint:ll` across
files for consistency.
- Reformat multiline strings, comments, and function parameters to
improve clarity and adhere to style guidelines.
- Add `// nolint:ll` comments where necessary to prevent linter
warnings.
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.
If the user has deleted their session.db file, but kept their
rules.db file, there can exist privacy mapper pairs that point to a now
deleted session ID. Such pairs should be ignored during the migration,
as they are cannot be used anymore.
This commit updates the migration to handle this case.
If the user has deleted their session.db file, but kept their
rules.db file, there can exist kv entry values that point to a now
deleted session ID. Such kv entries should be ignored during the
migration, as they are cannot be used anymore.
This commit updates the migration to handle this case.
In the upcoming commits, we will update the kv stores and the privacy
mapper migration to not migrate entries if their linked session has been
deleted. As those checks will need to query the SQL db to see if the
session still exists, we move the session alias to session map to not
only be used in the actions migration, but throughout the migration
when ever we need to query a session by its alias. This is done to
avoid multiple queries to the SQL db for the same session alias, to
improve the performance of the migration.
This commit introduces the migration logic for transitioning the
actions store from kvdb to SQL.
Note that as of this commit, the migration is not yet triggered by any
production code, i.e. only tests execute the migration logic.
This commit adds an `accountStore` and a `rootKeyStore` arg the database
population functions of the kvdb to SQL migration tests of the
firewalldb.
As an action can be linked to an account, we need to enable simulation
of that in the migration tests of the actions store. In order to create
the accounts to link the actions to, we need to create the accounts in
the account store, which therefore requires passing the `accountStore`
to database population functions of the migration tests.
As the kvdb to SQL migration also will update the migrated actions to
not only store the 4 byte short ID of the action's corresponding
macaroon, but to it's full 8 byte root key ID. This requires the
migration function has access to all of lnd's 8 byte root key IDs, and
the migration function will therefore be change to accept a [][]byte
arg containing all of lnd's root key IDs.
As we can't access a full lnd instance in the migration unit tests, we
need to create a mock instance that simulates the root key store, and
this commit therefore adds mock `rootKeyStore` struct which is also
passed to the database population functions of the migration tests.
This `rootKeyStore` struct can be used to generate dummy root key IDs
when creating simulated actions in the migration tests.
In preparation for the kvdb to SQL migration of the actions store, this
commit adds an `actions` field to the expected result of the migration
tests. Once the migration is implemented, this field will be used to
validate that the migrated actions match the expected results.
Add helper method to `AddActionReq` returns the 4 byte macaroon ID that
is derived from the MacaroonRootKeyID. Using the helper removes some
code repetition at call sites, and makes the intended usage clearer.
As the `MacaroonRootKeyID` field of the `AddActionReq` struct also
contains the 4 bytes of the `MacaroonIdentifier`, we change all
call sites to instead use the last 4 bytes of the `MacaroonRootKeyID`
field. As the `MacaroonIdentifier` field therefore becomes redundant,
we also remove it.
When migrating the actions store from kvdb to sql, we will update the
existing actions to include the full mac root key, instead of just the
last 4 bytes (currently called `MacaroonIdentifier`). In order to do so,
we change the sql implementation of the `actions` store to persist the
full mac root key, instead of just the last 4 bytes. As no production
data in the sql actions store exists for users yet, it's fine for us to
change this without having to address old sql actions which only stored
the last 4 bytes.
Note though that since old actions stored in the kvdb implementation
only have the last 4 bytes of the mac root key persisted, we will only
ever persist the last 4 byte of the mac root key ID for kvdb actions.
When the actions are later read back from the kvdb store, the first 4
bytes of the mac root key ID will be padded with zeroes to make up the
full 8 bytes.
As no call site currently utilizes the full 8 bytes of the mac root key
ID, this is okay for now. When we later deprecate and remove the kvdb
implementation, we can then update the rest of `litd` to also use the
full mac root key ID.
The upcoming commit will update the `AddActionReq` struct to include an
extra field which the `kvdb` actions store will ignore. Therefore the
`assertEqualActions` for the `kvdb` version will need to be update to
ignore this field. In preparation for that change, we also do another
optimization of the `assertEqualActions` function under kvdb builds, to
not mutate the passed action references.
This commit introduces the migration logic for transitioning the
privacy mapper store from kvdb to SQL.
Note that as of this commit, the migration is not yet triggered by any
production code, i.e. only tests execute the migration logic.
This commit removes speculative comments from the firewalldb migration
docs that forecast future implementations. Such comments can create
confusion for developers looking at the current code base without
knowing our plans how the migration will be further developed.
Currently, the migration tests for firewalldb only migrates the kv
stores. In future commits, we will also migrate the privacy mapper
and the actions in the firewalldb,. Before this commit, the expected
results of the migrations tests could only be kv records, which will not
be the case when we also migrate the privacy mapper and the actions.
Therefore, we prepare the migration tests to expect more than just kv
records. This commit introduces a new type of `expectedResult` type
which the prep of the migration tests will use, which can specify more
than just one type of expected result.
As the firewalldb migration will include more than just the migration
of the kvstore data, we rename the migration tests that only migrate
the kvstore data to make it clearer which tests only focus on migrating
kv entries.
This commit introduces the migration logic for transitioning the
kvstores store from kvdb to SQL.
Note that as of this commit, the migration is not yet triggered by any
production code, i.e. only tests execute the migration logic.
During the migration of the kvstores to SQL, we'll iterate over the
buckets in the bbolt database, which holds all kvstores records.
In order to understand why the migration iterates over the buckets in
the specific order, we need to clarify the bbolt kvstores illustration
docs, so that it correctly reflects how the records are actually stored
in the bbolt database.
Rename the session_id to group_id in kvstores table in the SQL store, to
better represent how the field is actually used.
Note that this is a breaking change, and would normally require a new
migration. But as the SQL store is not used in production, and only
enabled under the dev build flag, we can rename it without a new
migration, as there's no users of the SQL store in production.
In the upcoming migration of the firewall database to SQL, the helper
functions that creates the test databases of different types, need to
return a unified interface in order to not have to control the
migration tests file by build tags. Therefore, we update the
`NewTestDB` functions to return the `FirewallDBs` interface instead of
the specific store implementation type.
In the upcoming migration of the firewall database to SQL, the helper
functions that creates the test databases of different types, need to
return a unified interface in order to not have to control the
migration tests file by build tags. Therefore, we export the unified
interface FirewallDBs, so that it can be returned public test DB
creation functions
We add a helper function to the functions that creates the test SQL
stores, in order to ensure that the store is properly closed when the
test is cleaned up.
Add a test covering a call to ListActions with group ID set and Reversed
set and assert that the actions are returned in the correct order. This
reveals a bug in the BoltDB implementation which was not making use of
the query.Reversed parameter when group ID is set. NOTE: it still wont
do pagination correctly (ie the pagination params still dont get used)
but we at least here can easily let it use the Reversed param.
For our kvdb firewalldb, we use an empty 4 byte array as the macaroon
identifier even if no macaroon was used to create the action. This is so
that we have some sort of "session ID" bucket to store these set of
actions under. For our SQL impl, however, this is not needed and we will
likely just use a nullable field for the macaroon ID. So in preparation
for this, we move the kvdb specific logic to the kvdb impl.
In this commit, we do our best to ensure that at least at the time of
action creation, if the account ID is set, then our bbolt actions store
impl will at least first check that the account really does exist. This
also forces us to update our tests in preparation for the SQL store
which will tightly couple the actions and accounts.
In this commit we add an optional AccountID to the RequestInfo type.
Then, we populate it if the caveat of the macaroon being used contains
an accounts caveat.
We also add an unused AccountID type to the AddActionReq and pass in the
value from the RequestLogger.
