The new name, KeyringProviderRecordInFile, describes what it is rather
than what it's used for. But the real reason is that I want to use it
for other things than the WAL in future commits.
This regression file didn't really test anything. The tests it was
supposed to do was removed here e270322f72
So now it doesn't test anything that key_provider.sql doesn't already do
for us.
Be consistent about always passing curr_pos when reading or writing the
map file. The code is easier to understand if only one variable is used
for positioning in the file.
The fucntions for lsiting keys do not look at inherit_global_providers
but even if they did it does not seem like something which would belong
in the architecture documentation.
Before this commit, we Xlogged the binary result of the _map file
content during key rotation. This led to issues:
1. Replicas would rewrite their own WAL keys with the primary's ones.
And WAL keys are different on replicas. The same would have happened
with SMGR keys since we're also planning to have them different across
replicas.
2. The crash recovery would rewrite the latest WAL key as it's being
created before redo.
This commit switches to rather Xlogging the event of rotation (to which
key should rotate) and lets redo/replicas perform the actual rotation.
Fixes PG-1468, PG-1541
There is a restart function so there is not and need to call first stop
and then start. And since by default a start, stop or restart call does
not return on error it is totally pointless to assert anything about the
return value. And since PostgreSQL's own tests also are fine with just
bailing out on error we do the same.
While at it we also always call these three functions without
parentheses to be consistent.
Since we tried to check if flags & MAP_ENTRY_EMPTY was true when
searching for empty entries the code was broken since x & 0 always is
false. We fix this by refactoring pg_tde_read_one_map_entry() so the
filtering of the entries is done outside the function. This make
implementing search for empty entries much easier.
Replaces the old way we deleted keys which was built for tde_heap_basic
with deleting the the relation key when smgr_unlink() is called on the
main fork. This function is always called after commit/abort when a
relation deletion has been registered, even if no main fork would exist.
This approach means we do not need to WAL log any event for deleting
relation keys, the normal SMGR unlink also handles that which fits well
into the current approach of doing most of the encryption at the SMGR
layer.
We also remove the subtransaction test which is no longer useful since
it tested things very specific to the old key deleteion.
Just as we use pgindent to validate that our c files conform to postgres
coding standards, we also run pgperltidy to do the same for perl files.
We only run it on our own code in contrib/pg_tde/
This doesn't actually run pgperltidy as we need to inject some options
in a way that didn't seem possible in that script. Instead it does the
same thing with some slight changes.
We also bump the ubuntu version for this google actions job to the
newest LTS as the older ubuntu version seems to have a version of
perltidy that doesn't support the options used by pgperltidy.
Just as we use pgindent we should probably use pgperltidy. This is an
initial run of it using the following command:
src/tools/pgindent/pgperltidy contrib/pg_tde
This helper mostly added confusion by making it seem like it did more
work than is actually did. And especially since we will want to call
init in the future with different parameters for some tests or
initialize from a backup.
Since until we actually have CI for older versions the code is likely
broken anyway we might as well not try to support versions we do not
actually support. It is easy to re-add this once we want to add support
for PostgreSQL 14.
The code wrongly assumed that the databaseId set in the keyInfo returned
from GetPrincipalKeyNoDefault() would be the Oid of the key provider
owner, while in reality it is the Oid of the database using it as a
principal key.
The reason to do this is that the old approach created an unnecessary
diff against upstream where they had forgot SinglePartitionSpec in
typedefs.list.
Additionally add two new structs from our SMGR patch to the list.
While there is a quite big variation already among PostgreSQL's own
record types and decriptions at least try not to invent something
totally different.
This simplifies working with tests a lot since now we will also always
get the errors from failed queries directly in the test output instead
of it being truncated when the TAP tests aborts due to the query
failing.
There is still a good case for why we should instead write idiomatic TAP
tests but this at least does a lot to improve the expereince of people
who have to work with these tests without changing the way the tests
work. Plus that the code is cleaner now so it should be easier to move
away from this way of testing in the future.
