K8SPG-911 Documented TDE - #366
Conversation
There was a problem hiding this comment.
Pull request overview
Adds end-user documentation for Transparent Data Encryption (TDE) / data-at-rest encryption support and exposes it in the MkDocs navigation and Operator configuration reference.
Changes:
- Adds a new “Data-at-rest encryption” documentation section (overview + Vault setup guide).
- Updates
docs/operator.mdto document newextensions.*options (includingextensions.pg_tde.*) and mark olderextensions.builtin.*entries as deprecated. - Wires the new docs into the site navigation via
mkdocs-base.yml.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| mkdocs-base.yml | Adds nav entries for the new data-at-rest encryption docs. |
| docs/operator.md | Adds/updates extension option documentation, deprecation notes, and new pg_tde/Vault settings. |
| docs/encryption.md | New overview page for data-at-rest encryption (TDE) and how it works. |
| docs/encryption-setup.md | New step-by-step Vault setup and CR configuration guide for pg_tde. |
| docs/encryption-disable.md | New guide describing how to disable encryption safely. |
| docs/assets/fragments/vault-install-tls.txt | New Vault-on-Kubernetes (Helm + TLS) procedure fragment used by the setup guide. |
| docs/assets/fragments/vault-generate-tls-certs.txt | New TLS certificate generation procedure fragment used by the setup guide. |
| docs/assets/fragments/vault-enable-kv.txt | New fragment describing how to enable Vault KV v2 at a mount path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ??? example "Expected output" | ||
|
|
||
| ```{.text .no-value} | ||
| Key Value | ||
| --- ----- | ||
| Seal Type shamir | ||
| Initialized true | ||
| Sealed true | ||
| Total Shares 1 | ||
| Threshold 1 | ||
| Unseal Progress 0/1 | ||
| Unseal Nonce n/a | ||
| Version 1.19.0 | ||
| Build Date 2025-03-04T12:36:40Z | ||
| Storage Type raft | ||
| Removed From Cluster false | ||
| HA Enabled true | ||
| ``` |
There was a problem hiding this comment.
The "Expected output" for unsealing the remaining pods shows Sealed true and Unseal Progress 0/1, which indicates the pod is still sealed. After a successful unseal (threshold=1), the output should show Sealed false. Also, the code fence uses .no-value, which doesn’t appear elsewhere in the docs (likely meant .no-copy).
|
|
||
| ```bash | ||
| openssl req -new -key $WORKDIR/vault.key \ | ||
| -subj "/CN=system:node:${SERVICE}.${NAMESPACE}.svc;/O=system:nodes" \ |
There was a problem hiding this comment.
The OpenSSL -subj value uses ; between DN components (...svc;/O=...). OpenSSL expects DN components separated by / (e.g. /CN=.../O=...); with the current command the CSR subject may be malformed and the procedure may fail.
| -subj "/CN=system:node:${SERVICE}.${NAMESPACE}.svc;/O=system:nodes" \ | |
| -subj "/CN=system:node:${SERVICE}.${NAMESPACE}.svc/O=system:nodes" \ |
| 3. Apply the changes: | ||
|
|
||
| ```bash | ||
| kubectl apply -f deploy/cr.yaml -n $NAMESPACE | ||
| ``` |
There was a problem hiding this comment.
This guide uses $NAMESPACE in the kubectl apply commands, but the variable is never defined here (and the setup guide uses $CLUSTER_NAMESPACE for the database/CR namespace). Use the correct namespace variable consistently so the commands are runnable as-is.
egegunes
left a comment
egegunes
left a comment
There was a problem hiding this comment.
LGTM. Please just check Copilot comments.
|
@janwieremjewicz if you have some time, please take a look at this pg_tde documentation for the operator |
Render
Destroyed
Render
Destroyed
Render
Destroyed
77e5302 to
34675a0
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Split text into sections for scannability
e3b45cd to
1421b5e
Compare
| @@ -0,0 +1,197 @@ | |||
| ## Install Vault with TLS | |||
There was a problem hiding this comment.
Taken into account that this is not our software and the vault conf can change - can we add some note about this and add link to the official vault docs?
There was a problem hiding this comment.
We do have it as it 1 in Assumptions
|
|
||
| ## Known limitations | ||
|
|
||
| 1. WAL encryption is not yet supported. The Operator forces `pg_tde.wal_encrypt=off`. WAL encryption will be added in future releases. |
There was a problem hiding this comment.
JFYI we're also adding WAL encryption in this release. it's okay to merge with this since we'll need to document WAL encryption in detail as well
|
|
||
| !!! important | ||
|
|
||
| Enable WAL encryption before the cluster has application writes. |
There was a problem hiding this comment.
this sounds a bit confusing. we should say that it's our recommendation for the least amount of problems
| ### Considerations for WAL encryption | ||
|
|
||
| 1. To enable WAL encryption, you must first enable `pg_tde` in the cluster. Wait for the cluster to become ready with `pg_tde` enabled, and only then enable WAL encryption as a separate step. Creating a new cluster with both `enabled` and `walEncryption` set to `true` causes Patroni bootstrap to fail. | ||
| 2. The safest time to enable WAL encryption is before the cluster has application writes. Enabling WAL encryption on clusters that already have data needs further validation. |
There was a problem hiding this comment.
what does further validation mean?
|
|
||
| With `pg_tde` enabled you can make backups and restores as usual. For restore, the Operator must have access to the encryption key that was used to encrypt the backup data. | ||
|
|
||
| ## Key rotation |
There was a problem hiding this comment.
let's put a warning here for doing the rotation by creating a new secret and changing secret name in cr.yaml. changing the existing secret contents is prone to failures
No description provided.