feat(postgres): add pgsodium, gzip, and pgzstd extensions - #61
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the PostgreSQL environment by integrating three powerful extensions: pgsodium for advanced cryptographic capabilities, pgsql-gzip for efficient SQL-native GZIP compression, and pgzstd for Zstandard compression, offering better ratios and faster decompression. These additions provide users with robust tools for data security and storage optimization directly within the database, improving overall data management and performance. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request adds support for the pgsodium, gzip, and pgzstd extensions to the PostgreSQL image. The changes include updating the Dockerfile to build these extensions from source, modifying the configuration to enable them, and adding corresponding smoke tests. The implementation is solid, but I have a few suggestions to improve build reproducibility and script robustness. My main concerns are with unpinned dependencies in the Dockerfile, which could lead to unpredictable builds. I've also suggested an optimization for the Dockerfile and a fix for a minor bug in the new shell script function.
|
|
||
| # Build pgsql-gzip from source | ||
| RUN set -eux; \ | ||
| git clone --depth 1 https://github.com/pramsey/pgsql-gzip.git /tmp/pgsql-gzip; \ |
|
|
||
| # Build pgzstd from source | ||
| RUN set -eux; \ | ||
| git clone --depth 1 https://github.com/grahamedgecombe/pgzstd.git /tmp/pgzstd; \ |
| # Build pgsodium from source | ||
| ARG PGSODIUM_VERSION=3.1.9 | ||
| RUN set -eux; \ | ||
| git clone --depth 1 --branch "v${PGSODIUM_VERSION}" --single-branch https://github.com/michelp/pgsodium.git /tmp/pgsodium; \ | ||
| cd /tmp/pgsodium; \ | ||
| make PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \ | ||
| make install PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \ | ||
| rm -rf /tmp/pgsodium | ||
|
|
||
| # Build pgsql-gzip from source | ||
| RUN set -eux; \ | ||
| git clone --depth 1 https://github.com/pramsey/pgsql-gzip.git /tmp/pgsql-gzip; \ | ||
| cd /tmp/pgsql-gzip; \ | ||
| make PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \ | ||
| make install PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \ | ||
| rm -rf /tmp/pgsql-gzip | ||
|
|
||
| # Build pgzstd from source | ||
| RUN set -eux; \ | ||
| git clone --depth 1 https://github.com/grahamedgecombe/pgzstd.git /tmp/pgzstd; \ | ||
| cd /tmp/pgzstd; \ | ||
| make PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \ | ||
| make install PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \ | ||
| rm -rf /tmp/pgzstd |
There was a problem hiding this comment.
To optimize the Docker image, you can combine these consecutive RUN commands into a single one. This reduces the number of image layers and can improve build times. This change also makes the cleanup step more robust by removing all temporary directories at the end.
# Build extensions from source
ARG PGSODIUM_VERSION=3.1.9
RUN set -eux; \
git clone --depth 1 --branch "v${PGSODIUM_VERSION}" --single-branch https://github.com/michelp/pgsodium.git /tmp/pgsodium; \
cd /tmp/pgsodium; \
make PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \
make install PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \
\
git clone --depth 1 https://github.com/pramsey/pgsql-gzip.git /tmp/pgsql-gzip; \
cd /tmp/pgsql-gzip; \
make PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \
make install PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \
\
git clone --depth 1 https://github.com/grahamedgecombe/pgzstd.git /tmp/pgzstd; \
cd /tmp/pgzstd; \
make PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \
make install PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \
\
rm -rf /tmp/pgsodium /tmp/pgsql-gzip /tmp/pgzstd
| for lib in "${missing[@]}"; do | ||
| new_value="${new_value},${lib}" | ||
| done |
There was a problem hiding this comment.
The current logic for appending missing libraries can create an invalid shared_preload_libraries string with a leading comma if the original list was empty. It's safer to handle this case explicitly to make the script more robust.
| for lib in "${missing[@]}"; do | |
| new_value="${new_value},${lib}" | |
| done | |
| for lib in "${missing[@]}"; do | |
| if [[ -z "${new_value}" ]]; then | |
| new_value="${lib}" | |
| else | |
| new_value="${new_value},${lib}" | |
| fi | |
| done |
There was a problem hiding this comment.
Pull request overview
This PR adds three new PostgreSQL extensions to the core_data platform: pgsodium (libsodium-based cryptography with Transparent Column Encryption support), pgsql-gzip (SQL-native gzip compression/decompression), and pgzstd (Zstandard compression/decompression). All three are built from source in the Docker image since they aren't available as apt packages for PG17. A new enforce_shared_preload_libraries() mechanism ensures required preload libraries aren't accidentally removed from the config.
Changes:
- Added build blocks for pgsodium, pgsql-gzip, and pgzstd in the Dockerfile with their required dev dependencies, plus a
pgsodium_getkey.shscript for TCE key management - Added all three extensions to
CORE_EXTENSION_LISTand pgsodium to a newREQUIRED_PRELOAD_LIBRARIESarray, with a startup enforcement function in00-render-config.sh - Added smoke tests and pgTap assertions (plan count 39 → 42) for the three new extensions
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| postgres/Dockerfile | Adds libsodium-dev, zlib1g-dev, libzstd-dev build deps and source-build blocks for pgsodium, pgsql-gzip, pgzstd |
| scripts/lib/extensions_list.sh | Adds 3 extensions to CORE_EXTENSION_LIST and introduces REQUIRED_PRELOAD_LIBRARIES array |
| scripts/lib/extensions.sh | Adds smoke test queries and pgTap has_extension assertions for the 3 new extensions |
| postgres/tools/pgsodium_getkey.sh | New script to retrieve pgsodium root encryption key from a file |
| postgres/initdb/00-render-config.sh | Adds enforce_shared_preload_libraries() function and integrates it into startup flow |
| postgres/conf/postgresql.conf.tpl | Adds pgsodium to shared_preload_libraries and sets pgsodium.getkey_script |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| local new_value="${current}" | ||
| for lib in "${missing[@]}"; do | ||
| new_value="${new_value},${lib}" | ||
| done |
There was a problem hiding this comment.
If current is empty (e.g., after applying the || true fix for the grep, or if the config has shared_preload_libraries = ''), the loop on lines 42-44 produces a leading comma: ,age,pgaudit,.... This creates an invalid shared_preload_libraries value. The function should handle the empty/unset current case, for example by initializing new_value conditionally or stripping a leading comma before writing.
| done | |
| done | |
| new_value="${new_value#,}" |
|
|
||
| # pgsodium server key retrieval script. | ||
| # Called by pgsodium to obtain the root encryption key for Transparent Column Encryption. | ||
| # The key must be exactly 32 bytes (256 bits) of raw key material. |
There was a problem hiding this comment.
The comment on line 7 says "The key must be exactly 32 bytes (256 bits) of raw key material" but the example command on line 15 generates a 64-character hex-encoded string (not raw bytes). pgsodium's getkey_script actually expects a hex-encoded key. The comment should clarify that the key file should contain 64 hex characters representing a 256-bit key, not raw binary data.
| if [[ "${FORCE_RENDER_CONFIG}" != "1" ]]; then | ||
| echo "[core_data] Configuration already rendered; refreshing network allow entries." >&2 | ||
| apply_network_allow_entries | ||
| local needs_reload=0 |
There was a problem hiding this comment.
local is used outside of a function (top-level script body). In bash, local is only valid inside a function. While bash may not always error out, it will emit a warning like local: can only be used in a function and the variable won't actually be local. This should be changed to a regular variable assignment (just needs_reload=0).
| local needs_reload=0 | |
| needs_reload=0 |
| # Build pgsodium from source | ||
| ARG PGSODIUM_VERSION=3.1.9 | ||
| RUN set -eux; \ | ||
| git clone --depth 1 --branch "v${PGSODIUM_VERSION}" --single-branch https://github.com/michelp/pgsodium.git /tmp/pgsodium; \ | ||
| cd /tmp/pgsodium; \ | ||
| make PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \ | ||
| make install PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \ | ||
| rm -rf /tmp/pgsodium |
There was a problem hiding this comment.
After building pgsodium, the pgsodium_getkey.sh script will be copied to /opt/core_data/tools/ (via COPY postgres/tools later in the Dockerfile) but is never made executable. The existing chmod +x at line 131 only targets /docker-entrypoint-initdb.d/*.sh. Since pgsodium.getkey_script in postgresql.conf.tpl points to this file, pgsodium will fail to invoke it at runtime. A chmod +x /opt/core_data/tools/*.sh should be added after the COPY.
| # Build pgzstd from source | ||
| RUN set -eux; \ | ||
| git clone --depth 1 https://github.com/grahamedgecombe/pgzstd.git /tmp/pgzstd; \ | ||
| cd /tmp/pgzstd; \ | ||
| make PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \ | ||
| make install PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \ | ||
| rm -rf /tmp/pgzstd |
There was a problem hiding this comment.
Same as pgsql-gzip above: pgzstd is cloned from the default branch without a version tag. Consider pinning to a specific release tag (using an ARG like PGSODIUM_VERSION) to ensure reproducible builds.
| [[ -f "${conf_file}" ]] || return 0 | ||
|
|
||
| local current | ||
| current=$(grep -E "^shared_preload_libraries" "${conf_file}" | sed "s/shared_preload_libraries *= *'\\(.*\\)'/\\1/") |
There was a problem hiding this comment.
Under set -euo pipefail (set at the top of the script), if shared_preload_libraries is not found in postgresql.conf (e.g., someone removed the entire line), grep returns exit code 1, making the pipeline fail and causing the script to abort immediately instead of gracefully adding the missing directive. Consider adding || true to the grep pipeline (e.g., current=$(grep ... | sed ... || true)) or using an explicit if grep ... ; then pattern to handle the no-match case.
| current=$(grep -E "^shared_preload_libraries" "${conf_file}" | sed "s/shared_preload_libraries *= *'\\(.*\\)'/\\1/") | |
| current=$(grep -E "^shared_preload_libraries" "${conf_file}" | sed "s/shared_preload_libraries *= *'\\(.*\\)'/\\1/" || true) |
| # Build pgsodium from source | ||
| ARG PGSODIUM_VERSION=3.1.9 | ||
| RUN set -eux; \ | ||
| git clone --depth 1 --branch "v${PGSODIUM_VERSION}" --single-branch https://github.com/michelp/pgsodium.git /tmp/pgsodium; \ | ||
| cd /tmp/pgsodium; \ | ||
| make PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \ | ||
| make install PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \ | ||
| rm -rf /tmp/pgsodium | ||
|
|
||
| # Build pgsql-gzip from source | ||
| RUN set -eux; \ | ||
| git clone --depth 1 https://github.com/pramsey/pgsql-gzip.git /tmp/pgsql-gzip; \ | ||
| cd /tmp/pgsql-gzip; \ | ||
| make PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \ | ||
| make install PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \ | ||
| rm -rf /tmp/pgsql-gzip | ||
|
|
||
| # Build pgzstd from source | ||
| RUN set -eux; \ | ||
| git clone --depth 1 https://github.com/grahamedgecombe/pgzstd.git /tmp/pgzstd; \ | ||
| cd /tmp/pgzstd; \ | ||
| make PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \ | ||
| make install PG_CONFIG="/usr/lib/postgresql/${PG_MAJOR}/bin/pg_config"; \ | ||
| rm -rf /tmp/pgzstd |
There was a problem hiding this comment.
These git clone + make install steps pull and build third-party PostgreSQL extensions directly from remote GitHub repositories without any integrity pinning (commit SHA, checksum, or signature). If an attacker compromises one of these repos or modifies the referenced tag/branch, malicious code could be compiled into the image and executed inside PostgreSQL with full access to database data and credentials. Pin these dependencies to immutable commit SHAs and/or vendor them locally, and add integrity verification (e.g., checksums or signed releases) before building.
Summary
Closes #60
shared_preload_librariesgzip/gunzipfor compressing/decompressingbyteadataAll three are built from source in the Docker image (not available as apt packages for PG17).
Changes
libsodium-dev,zlib1g-dev,libzstd-devbuild dependencies and source build blocks in DockerfileCORE_EXTENSION_LISTfor automatic creation in all databases at startuppgsodiumtoshared_preload_librarieswithgetkey_scriptconfig for TCEREQUIRED_PRELOAD_LIBRARIEScanonical array andenforce_shared_preload_libraries()function in00-render-config.sh— validates and correctsshared_preload_librarieson every startup, even when config re-rendering is skippedTest Plan
shared_preload_librariesenforce_shared_preload_libraries()corrects manually removed entries on restart