Some comments on documentation #436

AndersAstrand · 2025-06-17T06:23:12Z

Some comments I had during a cursory read-through of the docs.

My comments are prefixed with AÅ:

Obviously this is never meant to be merged, it's just a way to give comments 🙂

Some comments I had during a cursory read-through of the docs. My comments are prefixed with AÅ:

codecov-commenter · 2025-06-17T06:27:12Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.65%. Comparing base (5610639) to head (f02c72d).

❌ Your project status has failed because the head coverage (84.65%) is below the target coverage (90.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@                  Coverage Diff                  @@
##           TDE_REL_17_STABLE     #436      +/-   ##
=====================================================
- Coverage              84.66%   84.65%   -0.01%     
=====================================================
  Files                     21       21              
  Lines                   2589     2588       -1     
  Branches                 402      401       -1     
=====================================================
- Hits                    2192     2191       -1     
  Misses                   316      316              
  Partials                  81       81

Components	Coverage Δ
access	`81.11% <ø> (ø)`
catalog	`88.22% <ø> (ø)`
common	`77.77% <ø> (ø)`
encryption	`73.45% <ø> (ø)`
keyring	`72.88% <ø> (ø)`
src	`91.44% <ø> (ø)`
smgr	`94.85% <ø> (-0.03%)`	⬇️
transam	`∅ <ø> (∅)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Andriciuc · 2025-06-17T07:18:42Z

contrib/pg_tde/documentation/docs/command-line-tools/pg-waldump.md

@@ -1,5 +1,7 @@
 # pg_waldump

+AÅ: We should probably include a notice about WAL encrypting being BETA and this information is subject to change
+
 [`pg_waldump` :octicons-link-external-16:](https://www.postgresql.org/docs/current/pgwaldump.html) is a tool to display a human-readable rendering of the Write-Ahead Log (WAL) of a PostgreSQL database cluster.



Added this note:

The WAL encryption feature is currently in beta and is not effective unless explicitly enabled. It is not yet production ready. Do not enable this feature in production environments.

Andriciuc · 2025-06-17T07:26:31Z

contrib/pg_tde/documentation/docs/faq.md

@@ -87,6 +87,7 @@ The initial decision on what file to encrypt is based on the table access method
 The principal key is used to encrypt the internal keys. The principal key is stored in the key management store. When you query the table, the principal key is retrieved from the key store to decrypt the table. Then the internal key for that table is used to decrypt the data.

 ### WAL encryption
+AÅ: We should mention this is a BETA feature and subject to change.



Added a note:

WAL encryption is currently in beta and is not effective unless explicitly enabled. It is not yet production ready. Do not enable this feature in production environments.

Andriciuc · 2025-06-17T07:28:52Z

contrib/pg_tde/documentation/docs/faq.md

@@ -134,6 +136,7 @@ You must restart the database in the following cases to apply the changes:

 * after you enabled the `pg_tde` extension
 * to turn on / off the WAL encryption
+AÅ: WAL encrytion is not yet production ready and should probably not be casually mentioned as something you can "turn on".



updated this to turn on/off line to:

when enabling WAL encryption, which is currently in beta. Do not enable this feature in production environments.

Andriciuc · 2025-06-17T07:53:04Z

contrib/pg_tde/documentation/docs/faq.md

@@ -150,6 +153,7 @@ In `pg_tde`, multi-tenancy is supported via a separate principal key per databas
 To control user access to the databases, you can use role-based access control (RBAC).

 WAL files are encrypted globally across the entire PostgreSQL cluster using the same encryption keys. Users don't interact with WAL files as these are used by the database management system to ensure data integrity and durability.
+AÅ: Again, WAL encryption hould probably not be mentioned as if it was a feature already.



commented the above paragraph out for now regarding WAL

Andriciuc · 2025-06-17T07:59:25Z

contrib/pg_tde/documentation/docs/faq.md

@@ -117,6 +118,7 @@ The support of other encryption mechanisms such as AES256 is planned for future
 ## Is post-quantum encryption supported?

 No, it's not yet supported. In our implementation we reply on OpenSSL libraries that don't yet support post-quantum encryption.
+AÅ: Afaik newer versions of openssl support post-quantum algorithms. Maybe we should use some better excuse or mention what version we use?



Updated the paragraph to:

No, pg_tde does not support post-quantum encryption.

Andriciuc · 2025-06-17T14:19:14Z

Just wanted to say, appreciate all the great suggestions Anders!

Andriciuc · 2025-06-17T14:36:25Z

contrib/pg_tde/documentation/docs/architecture/index.md

@@ -4,6 +4,8 @@

 Let's break down what it means.

+AÅ: We need to make it clear that this file are the ambitions for pg_tde, because a lot of this is not yet in there.
+


So, I wanted to mention that architecture is similar to design here, so I thought why not combine these two. Added the following:

This topic outlines the long-term architectural goals and intended design for pg_tde. While many of the described components are implemented, however some features are still under active development and may not yet be available in the current release.

Let's break down the key building blocks of this design.

Andriciuc · 2025-06-18T10:51:42Z

contrib/pg_tde/documentation/docs/features.md

 * Single-tenancy support via a global keyring provider
 * Multi-tenancy support
 * Table-level granularity for encryption and access control
 * Multiple Key management options
 * Logical replication of encrypted tables
+AÅ: We do not do anything for logical replication, do we?



Removed temporary tables feature to clear confusion, removed logical replication mention, removed WAL encryption as a feature.

Andriciuc · 2025-06-18T10:52:47Z

contrib/pg_tde/documentation/docs/global-key-provider-configuration/set-principal-key.md

@@ -36,6 +36,8 @@ The key material (actual cryptographic key) is auto-generated by `pg_tde` and st
 !!! note
    This process sets the **default principal key** for the server. Any database without its own key configuration will use this key.

+AÅ: This is confusing to me, but maybe makes sense in the rendered version. The Example below does _not_ set a default key, does it?
+
 ## Example



I have reworded the note and the below example to properly describe the correct param:

updated with correct code example using set_server_key_using_global parameter

updated note to reflect correct config

Andriciuc · 2025-06-18T10:57:07Z

contrib/pg_tde/documentation/docs/functions.md

@@ -10,6 +10,7 @@ However, database owners can run the “view keys” and “set principal key”

 * `GRANT EXECUTE`
 * `REVOKE EXECUTE`
+AÅ: This should probably say `GRANT EXECUTE ON FUNCTION` and similar for revoke



Fixed:

GRANT EXECUTE ON FUNCTION

REVOKE EXECUTE ON FUNCTION

Andriciuc · 2025-06-18T11:12:29Z

contrib/pg_tde/documentation/docs/functions.md

@@ -52,6 +53,8 @@ The `change` functions require the same parameters as the `add` functions. They

 Provider specific parameters differ for each implementation. Refer to the  respective subsection for details.

+AÅ: We should probably have a notice here about the modified provider settings will need to be able to provide the exact same principal keys as the original settings.
+


Added the following note for this:

!!! note
The updated provider must be able to retrieve the same principal keys as the original configuration.
If the new configuration cannot access existing keys, encrypted data and backups will become unreadable.

Andriciuc · 2025-06-18T11:13:28Z

contrib/pg_tde/documentation/docs/functions.md

@@ -231,6 +234,7 @@ These functions list the details of all key providers for the current database o
 ## Principal key management

 Use these functions to create a new principal key at a given keyprover, and to use those keys for a specific scope such as a current database, a global or default scope. You can also use them to start using a different existing key for a specific scope.
+AÅ: "keyprover"?



fixed, key provider.

Andriciuc · 2025-06-18T11:13:44Z

contrib/pg_tde/documentation/docs/functions.md

@@ -281,6 +285,7 @@ SELECT pg_tde_set_key_using_global_key_provider(
 ### pg_tde_set_server_key_using_global_key_provider

 Sets or rotates the server principal key using the specified global key provider. Use this function to set a principal key for WAL encryption.
+AÅ: Note that WAL encryption is not yet production ready



added the standard WAL warning here

Andriciuc · 2025-06-18T12:01:40Z

contrib/pg_tde/documentation/docs/how-to/multi-tenant-setup.md

@@ -82,6 +82,7 @@ You must do these steps for every database where you have created the extension.
    === "With HashiCorp Vault"

        The Vault server setup is out of scope of this document.
+        AÅ: Why doesn't KMIP have a similar comment? Is KMIP more in scope than vault? Why isn't OpenBao mentioned here?



Good point! Added the same paragraph for KMIP

DO NOT MERGE EVER

f02c72d

Some comments I had during a cursory read-through of the docs. My comments are prefixed with AÅ:

Andriciuc added the documentation Improvements or additions to documentation label Jun 17, 2025

Andriciuc reviewed Jun 17, 2025

View reviewed changes

Andriciuc reviewed Jun 18, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Some comments on documentation #436

Some comments on documentation #436

Uh oh!

AndersAstrand commented Jun 17, 2025

Uh oh!

codecov-commenter commented Jun 17, 2025

Uh oh!

Andriciuc Jun 17, 2025

Uh oh!

Andriciuc Jun 17, 2025

Uh oh!

Andriciuc Jun 17, 2025 •

edited

Loading

Uh oh!

Andriciuc Jun 17, 2025 •

edited

Loading

Uh oh!

Andriciuc Jun 17, 2025

Uh oh!

Andriciuc commented Jun 17, 2025

Uh oh!

Andriciuc Jun 17, 2025

Uh oh!

Andriciuc Jun 18, 2025

Uh oh!

Andriciuc Jun 18, 2025

Uh oh!

Andriciuc Jun 18, 2025

Uh oh!

Andriciuc Jun 18, 2025

Uh oh!

Andriciuc Jun 18, 2025

Uh oh!

Andriciuc Jun 18, 2025 •

edited

Loading

Uh oh!

Andriciuc Jun 18, 2025 •

edited

Loading

Uh oh!

Uh oh!

		@@ -4,6 +4,8 @@

		Let's break down what it means.

		AÅ: We need to make it clear that this file are the ambitions for pg_tde, because a lot of this is not yet in there.

		@@ -52,6 +53,8 @@ The `change` functions require the same parameters as the `add` functions. They

		Provider specific parameters differ for each implementation. Refer to the respective subsection for details.

		AÅ: We should probably have a notice here about the modified provider settings will need to be able to provide the exact same principal keys as the original settings.

Some comments on documentation #436

Are you sure you want to change the base?

Some comments on documentation #436

Uh oh!

Conversation

AndersAstrand commented Jun 17, 2025

Uh oh!

codecov-commenter commented Jun 17, 2025

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andriciuc Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andriciuc Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andriciuc commented Jun 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andriciuc Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andriciuc Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Andriciuc Jun 17, 2025 •

edited

Loading

Andriciuc Jun 17, 2025 •

edited

Loading

Andriciuc Jun 18, 2025 •

edited

Loading

Andriciuc Jun 18, 2025 •

edited

Loading