feat(deploy): containerize autopg + reusable Helm chart - #141
Conversation
Add a self-contained, offline-boot container image and a Helm chart that runs autopg (embedded PostgreSQL 18) as a single-replica StatefulSet on Kubernetes (verified on OrbStack arm64). deploy/Dockerfile - glibc base (debian:12-slim); bundled PG18 links libssl.so.1.1 + libicu.so.60 - pre-seeds the ~/.autopg/bin/<platformKey>/ binary cache from the release tarball's postgres/ tree + a .version marker so the FIRST boot never fetches @embedded-postgres from npm (proven with `docker run --network none`) - non-root user (uid 1000); postmaster is PID 1 (graceful SIGTERM) - ships postgresql-client for pg_isready (probes) + psql (provisioning) deploy/helm/autopg - StatefulSet (PVC at /var/lib/autopg/data; PGDATA is the pgdata/ subdir so a non-root process owns it and initdb's chmod succeeds), headless + ClusterIP Services, settings.json ConfigMap, password Secret (stable across upgrades), and a post-install/upgrade provisioning Job - listen_addresses='*' via postgres._extra; operator GUCs as curated top-level keys (autopg's curated defaults win over _extra) - postStart hook appends a pod-network pg_hba rule + reloads (autopg's initdb writes localhost-only pg_hba and exposes no knob for it) - provisioning Job idempotently creates a scoped LOGIN role that OWNS its db + schema public (so it can run migrations), and rotates the superuser password to the managed Secret Default provisions db `omni` owned by role `omni`.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds a Docker-based container image build (multi-stage Dockerfile with build context restrictions) for autopg, plus a full Helm chart for Kubernetes deployment including StatefulSet, Services, ConfigMap, Secret, provisioning Job, template helpers, values defaults, and deployment documentation. ChangesContainer Image
Estimated code review effort: 3 (Moderate) | ~25 minutes Helm Chart
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Helm as Helm Install/Upgrade
participant Secret as Secret Template
participant StatefulSet as StatefulSet Pod
participant Job as Provision Job
participant PG as PostgreSQL
Helm->>Secret: Render superuser and app passwords
Helm->>StatefulSet: Deploy pod with settings.json ConfigMap
StatefulSet->>PG: Start autopg postmaster
StatefulSet->>PG: postStart hook appends pg_hba.conf rule, reloads config
Helm->>Job: Trigger post-install/post-upgrade hook
Job->>PG: Wait for readiness via pg_isready
Job->>PG: Authenticate as postgres (default or managed password)
Job->>PG: Rotate superuser password to Secret value
loop for each provisioned app
Job->>PG: Create/update role, database, ownership, grants
end
Estimated code review effort: 3 (Moderate) | ~30 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces containerization and a Helm chart for autopg (embedded PostgreSQL 18) to support offline-boot deployments in Kubernetes. The review feedback highlights several critical improvements: enabling multi-architecture support in the Dockerfile by dynamically handling TARGETARCH for the pre-seeded binary cache, securing the provisioning Job against SQL injection by passing variables safely to psql, making the postStart hook more robust against database initialization timeouts, and using idiomatic Helm functions like mustMerge and deepCopy to simplify configuration rendering.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| FROM debian:12-slim AS runtime | ||
| ARG PG_VERSION_MARKER | ||
|
|
There was a problem hiding this comment.
To support multi-architecture builds (e.g., amd64), we need to make the TARGETARCH build argument available in the runtime stage. This allows us to dynamically determine the correct platform key (linux-arm64 or linux-x64) for the pre-seeded binary cache.
FROM debian:12-slim AS runtime
ARG PG_VERSION_MARKER
ARG TARGETARCH
| # 2) THE GOTCHA: pre-seed the binary cache at | ||
| # $AUTOPG_CONFIG_DIR/bin/linux-arm64/{bin,lib,share} and drop the `.version` | ||
| # marker so isCachedValid() short-circuits before any npm fetch. | ||
| COPY --from=fetch /stage/autopg/postgres/ /var/lib/autopg/bin/linux-arm64/ | ||
|
|
||
| RUN set -eux; \ | ||
| printf '%s\n' "${PG_VERSION_MARKER}" > /var/lib/autopg/bin/linux-arm64/.version; \ | ||
| chmod +x /usr/local/bin/autopg; \ | ||
| mkdir -p /var/lib/autopg/data /var/run/autopg; \ | ||
| chown -R autopg:autopg /var/lib/autopg /var/run/autopg; \ | ||
| # sanity: binaries present + marker correct | ||
| test -x /var/lib/autopg/bin/linux-arm64/bin/postgres; \ | ||
| test -x /var/lib/autopg/bin/linux-arm64/bin/initdb; \ | ||
| grep -qx "${PG_VERSION_MARKER}" /var/lib/autopg/bin/linux-arm64/.version |
There was a problem hiding this comment.
The pre-seeded binary cache path is currently hardcoded to linux-arm64. When building an amd64 image, the files will still be copied to linux-arm64, but autopg (running on linux-x64) will look for the cache in linux-x64. This causes the offline boot to fail on amd64 hosts as it attempts to download the binaries from npm.
We should dynamically copy the binaries to the correct platform-specific directory based on the TARGETARCH build argument.
# 2) THE GOTCHA: pre-seed the binary cache at
# $AUTOPG_CONFIG_DIR/bin/<platformKey>/{bin,lib,share} and drop the `.version`
# marker so isCachedValid() short-circuits before any npm fetch.
COPY --from=fetch /stage/autopg/postgres/ /var/lib/autopg/bin/temp/
RUN set -eux; \
case "${TARGETARCH:-arm64}" in \
arm64) PLATFORM_KEY="linux-arm64" ;; \
amd64) PLATFORM_KEY="linux-x64" ;; \
*) echo "unsupported TARGETARCH: ${TARGETARCH}" >&2; exit 1 ;; \
esac; \
mkdir -p "/var/lib/autopg/bin/${PLATFORM_KEY}"; \
mv /var/lib/autopg/bin/temp/* "/var/lib/autopg/bin/${PLATFORM_KEY}/"; \
rm -rf /var/lib/autopg/bin/temp; \
printf '%s\n' "${PG_VERSION_MARKER}" > "/var/lib/autopg/bin/${PLATFORM_KEY}/.version"; \
chmod +x /usr/local/bin/autopg; \
mkdir -p /var/lib/autopg/data /var/run/autopg; \
chown -R autopg:autopg /var/lib/autopg /var/run/autopg; \
# sanity: binaries present + marker correct
test -x "/var/lib/autopg/bin/${PLATFORM_KEY}/bin/postgres"; \
test -x "/var/lib/autopg/bin/${PLATFORM_KEY}/bin/initdb"; \
grep -qx "${PG_VERSION_MARKER}" "/var/lib/autopg/bin/${PLATFORM_KEY}/.version"
| # psql 15 (debian) does NOT interpolate :'var' in -c (that is | ||
| # psql 16+), so passwords are single-quoted directly. Managed | ||
| # passwords are alphanumeric (randAlphaNum) => injection-safe; | ||
| # operator-supplied passwords must not contain a single quote. | ||
| psqlx() { psql -v ON_ERROR_STOP=1 -h "$PGHOST" -p "$PGPORT" -U postgres "$@"; } | ||
|
|
||
| # ---- rotate the superuser password to the managed value ------- | ||
| psqlx -d postgres -c "ALTER USER postgres PASSWORD '$SUPERUSER_PASSWORD';" | ||
| export PGPASSWORD="$SUPERUSER_PASSWORD" | ||
| echo "superuser password synced to managed secret" | ||
|
|
||
| # ---- provision a scoped app: role + db + schema ownership ------ | ||
| provision_app() { | ||
| db="$1"; role="$2"; pw="$3" | ||
| echo "provisioning db=$db role=$role" | ||
| if [ "$(psqlx -d postgres -tAc "SELECT 1 FROM pg_roles WHERE rolname = '$role'")" = "1" ]; then | ||
| psqlx -d postgres -c "ALTER ROLE \"$role\" LOGIN PASSWORD '$pw';" | ||
| else | ||
| psqlx -d postgres -c "CREATE ROLE \"$role\" LOGIN PASSWORD '$pw';" | ||
| fi | ||
| if [ "$(psqlx -d postgres -tAc "SELECT 1 FROM pg_database WHERE datname = '$db'")" != "1" ]; then | ||
| psqlx -d postgres -c "CREATE DATABASE \"$db\" OWNER \"$role\";" | ||
| fi | ||
| # Ownership so the role can run migrations (CREATE/ALTER/DROP) | ||
| # against its own schema. | ||
| psqlx -d "$db" -c "ALTER DATABASE \"$db\" OWNER TO \"$role\";" | ||
| psqlx -d "$db" -c "ALTER SCHEMA public OWNER TO \"$role\";" | ||
| psqlx -d "$db" -c "GRANT ALL ON SCHEMA public TO \"$role\";" | ||
| psqlx -d "$db" -c "GRANT ALL ON DATABASE \"$db\" TO \"$role\";" | ||
| echo "provisioned db=$db role=$role (role owns schema public)" | ||
| } |
There was a problem hiding this comment.
The provisioning script currently interpolates passwords and identifiers directly into the SQL command strings passed to psql -c. If any password, database name, or role name contains special characters (such as single quotes, double quotes, or backslashes), this will result in syntax errors or potential SQL injection vulnerabilities.
Since psql does not support variable interpolation with the -c option, we can pass the variables using the -v flag and pipe the SQL commands via standard input (stdin). This allows psql to safely quote and escape the values as SQL literals (:'var') or identifiers (:"var").
# Passwords and identifiers are passed safely via psql variables
# and stdin to prevent SQL injection and syntax errors.
psql_run() {
psql -v ON_ERROR_STOP=1 -h "$PGHOST" -p "$PGPORT" -U postgres "$@"
}
# ---- rotate the superuser password to the managed value -------
psql_run -v pw="$SUPERUSER_PASSWORD" -d postgres <<EOF
ALTER USER postgres PASSWORD :'pw';
EOF
export PGPASSWORD="$SUPERUSER_PASSWORD"
echo "superuser password synced to managed secret"
# ---- provision a scoped app: role + db + schema ownership ------
provision_app() {
db="$1"; role="$2"; pw="$3"
echo "provisioning db=$db role=$role"
role_exists=$(psql_run -v r="$role" -d postgres -tAc "SELECT 1 FROM pg_roles WHERE rolname = :'r'")
if [ "$role_exists" = "1" ]; then
psql_run -v r="$role" -v pw="$pw" -d postgres <<EOF
ALTER ROLE :"r" LOGIN PASSWORD :'pw';
EOF
else
psql_run -v r="$role" -v pw="$pw" -d postgres <<EOF
CREATE ROLE :"r" LOGIN PASSWORD :'pw';
EOF
fi
db_exists=$(psql_run -v d="$db" -d postgres -tAc "SELECT 1 FROM pg_database WHERE datname = :'d'")
if [ "$db_exists" != "1" ]; then
psql_run -v d="$db" -v r="$role" -d postgres <<EOF
CREATE DATABASE :"d" OWNER :"r";
EOF
fi
psql_run -v d="$db" -v r="$role" -d "$db" <<EOF
ALTER DATABASE :"d" OWNER TO :"r";
ALTER SCHEMA public OWNER TO :"r";
GRANT ALL ON SCHEMA public TO :"r";
GRANT ALL ON DATABASE :"d" TO :"r";
EOF
echo "provisioned db=$db role=$role (role owns schema public)"
}| for i in $(seq 1 90); do | ||
| pg_isready -h /var/run/autopg -p {{ .Values.port }} -q && break | ||
| sleep 2 | ||
| done | ||
| if ! grep -qxF "$RULE" "$HBA" 2>/dev/null; then | ||
| printf '# added by autopg chart (in-cluster peer access)\n%s\n' "$RULE" >> "$HBA" | ||
| psql -h /var/run/autopg -p {{ .Values.port }} -U postgres -d postgres \ | ||
| -c 'SELECT pg_reload_conf();' | ||
| fi |
There was a problem hiding this comment.
If the PostgreSQL server takes longer than 180 seconds to start (e.g., during a slow first-boot initdb on a cold PVC), the postStart hook loop will finish, and the subsequent psql command will fail. Because the shell runs with -e, this failure will cause the postStart hook to exit with a non-zero code, prompting Kubernetes to kill and restart the container. This can interrupt the ongoing initdb and potentially corrupt the database.
We should check if pg_isready succeeded before attempting to run psql, and log a warning instead of failing the hook if it times out.
for i in $(seq 1 90); do
if pg_isready -h /var/run/autopg -p {{ .Values.port }} -q; then
READY=1
break
fi
sleep 2
done
if [ "${READY:-0}" -eq 1 ]; then
if ! grep -qxF "$RULE" "$HBA" 2>/dev/null; then
printf '# added by autopg chart (in-cluster peer access)\n%s\n' "$RULE" >> "$HBA"
psql -h /var/run/autopg -p {{ .Values.port }} -U postgres -d postgres \
-c 'SELECT pg_reload_conf();'
fi
else
echo "PostgreSQL was not ready after 180s, skipping pg_hba.conf update" >&2
fi| {{- define "autopg.settingsJson" -}} | ||
| {{- $postgres := dict -}} | ||
| {{- range $k, $v := .Values.settings.gucs -}} | ||
| {{- $_ := set $postgres $k $v -}} | ||
| {{- end -}} | ||
| {{- $extra := dict "listen_addresses" .Values.settings.listenAddresses -}} | ||
| {{- range $k, $v := .Values.settings.extraGucs -}} | ||
| {{- $_ := set $extra $k $v -}} | ||
| {{- end -}} | ||
| {{- $_ := set $postgres "_extra" $extra -}} | ||
| {{- dict "postgres" $postgres | toPrettyJson -}} | ||
| {{- end -}} |
There was a problem hiding this comment.
Instead of using imperative range and set loops to construct the settings dictionary, we can use Helm's built-in mustMerge and deepCopy functions. This is more idiomatic, concise, and less error-prone.
{{- define "autopg.settingsJson" -}}
{{- $gucs := deepCopy .Values.settings.gucs | default dict -}}
{{- $extraGucs := deepCopy .Values.settings.extraGucs | default dict -}}
{{- $extra := mustMerge (dict "listen_addresses" .Values.settings.listenAddresses) $extraGucs -}}
{{- $postgres := mustMerge (dict "_extra" $extra) $gucs -}}
{{- dict "postgres" $postgres | toPrettyJson -}}
{{- end -}}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91834bd5a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # 2) THE GOTCHA: pre-seed the binary cache at | ||
| # $AUTOPG_CONFIG_DIR/bin/linux-arm64/{bin,lib,share} and drop the `.version` | ||
| # marker so isCachedValid() short-circuits before any npm fetch. | ||
| COPY --from=fetch /stage/autopg/postgres/ /var/lib/autopg/bin/linux-arm64/ |
There was a problem hiding this comment.
When this Dockerfile is built for linux/amd64, the fetch stage selects the linux-x64-glibc tarball, but the runtime image still copies it into /var/lib/autopg/bin/linux-arm64. On x64, src/postgres.js resolves the cache as bin/linux-x64, so the image misses the pre-seeded binaries and falls back to the npm download path; that breaks the advertised offline first boot for amd64 images even though the Dockerfile explicitly supports TARGETARCH=amd64.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| {{- range .Values.provisionedApps }} | ||
| provision_app {{ .db | quote }} {{ .role | quote }} "$APP_PW_{{ .role }}" |
There was a problem hiding this comment.
For any provisioned role containing shell metacharacters such as a hyphen, this renders a reference like "$APP_PW_my-app"; POSIX sh parses that as $APP_PW_my plus the literal -app, so the Job provisions a password different from the Secret (or is rejected by clusters that still validate env var names strictly). Since the SQL below quotes role names and the Secret key format supports names like my-app-password, these role names appear supported but produce unusable credentials.
Useful? React with 👍 / 👎.
| if ! grep -qxF "$RULE" "$HBA" 2>/dev/null; then | ||
| printf '# added by autopg chart (in-cluster peer access)\n%s\n' "$RULE" >> "$HBA" |
There was a problem hiding this comment.
If the chart is first installed with the default hostAuth.cidr: all and later upgraded to a narrower CIDR or different method, this append-only logic leaves the old chart-managed host all all all ... row in the persisted PVC before the new row. PostgreSQL uses the first matching pg_hba entry, so the old broad rule keeps allowing the previous access and the operator's tightening has no effect.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
deploy/README.md (1)
9-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a language hint to the fenced code block.
markdownlint flags this block (MD040) for missing a language identifier.
📝 Proposed fix
-``` +```text deploy/ Dockerfile arm64, glibc (debian:12-slim), pre-seeded binary cache .dockerignore helm/autopg/ the chart (Chart.yaml, values.yaml, templates/)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/README.md` around lines 9 - 14, Add a language identifier to the fenced block in the README so markdownlint MD040 passes; update the code fence containing the deploy directory listing to use a plain text hint, and keep the content unchanged. Use the existing README fenced block near the deploy tree listing as the target for this fix.Source: Linters/SAST tools
deploy/helm/autopg/values.yaml (1)
95-102: 🔒 Security & Privacy | 🔵 TrivialDefault
hostAuth.cidr: "all"opens PostgreSQL to any pod in the cluster.Password auth (scram-sha-256) is required, but the default trusts the whole cluster network rather than a scoped pod/service CIDR. Given pod CIDRs are cluster-specific and unknown at chart-author time, this is a reasonable pragmatic default, but consider documenting the recommendation to pair this with a
NetworkPolicyrestricting ingress to known namespaces/pods, since the chart itself doesn't create one.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/autopg/values.yaml` around lines 95 - 102, The default hostAuth.cidr setting is too broad for the chart’s PostgreSQL access model and should be documented more clearly in the values file. Update the hostAuth section in values.yaml to explicitly recommend pairing hostAuth.cidr: "all" with a NetworkPolicy or a tighter pod/service CIDR, and reference the hostAuth.enabled, hostAuth.cidr, and hostAuth.method settings so users understand the intended secure deployment pattern. Keep the default unchanged, but add concise guidance that the chart does not create ingress restrictions itself.deploy/helm/autopg/templates/secret.yaml (1)
31-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse the
autopg.appPasswordKeyhelper instead of reimplementing the key format inline.
_helpers.tplalready definesautopg.appPasswordKeyfor exactly this<role>-passwordformat, but this block recomputes it manually viaprintf. Duplicating the format risks drift if the helper's convention ever changes.♻️ Reuse the existing helper
{{- range .Values.provisionedApps }} - {{- $key := printf "%s-password" .role -}} + {{- $key := include "autopg.appPasswordKey" . -}} {{- $pw := .password -}}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/autopg/templates/secret.yaml` around lines 31 - 40, The secret template block is duplicating the app password key format inline instead of using the existing helper. Update the logic in the range over .Values.provisionedApps to use autopg.appPasswordKey for the role-derived password key, replacing the manual printf-based construction so the key format stays centralized and consistent with _helpers.tpl.deploy/helm/autopg/templates/statefulset.yaml (1)
71-79: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valuepostStart hook proceeds to modify pg_hba/reload even if readiness wait times out.
After the 90×2s wait loop exhausts without
pg_isreadysucceeding, the script falls through togrep/psqlanyway. Since the shebang uses-ec, a failingpsqlwill cause the postStart hook (and thus container start) to fail — acceptable as a fail-safe, but there's no explicit log message distinguishing "timed out waiting" from "rule already applied," which would help debugging first-boot failures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/autopg/templates/statefulset.yaml` around lines 71 - 79, The postStart hook in the statefulset template falls through after the readiness loop without clearly reporting whether it timed out or completed successfully. Update the hook logic around the pg_isready wait, the HBA check, and the psql reload so it emits an explicit log message when the 90x2s wait expires before continuing or failing, and keep the existing idempotent grep/pg_reload_conf flow in the same script block. Use the hook script in the autopg StatefulSet template to distinguish “timed out waiting for Postgres” from “pg_hba rule already present” for easier first-boot debugging.deploy/helm/autopg/templates/NOTES.txt (1)
11-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
$PASSWORDplaceholder isn't wired to the preceding command.The password-retrieval command (Line 14) only prints to stdout; Line 15's
URL:then references$PASSWORDas if it were already exported. First-time users copy-pasting these commands will get a literal empty$PASSWORDin the URL. Consider making the retrieval command exportable, e.g.export PASSWORD=$(kubectl ... | base64 -d).✏️ Proposed tweak
- db: {{ .db }} role: {{ .role }} - password: kubectl -n {{ $.Release.Namespace }} get secret {{ include "autopg.secretName" $ }} -o jsonpath='{.data.{{ .role }}-password}' | base64 -d + password: export PASSWORD=$(kubectl -n {{ $.Release.Namespace }} get secret {{ include "autopg.secretName" $ }} -o jsonpath='{.data.{{ .role }}-password}' | base64 -d) URL: postgresql://{{ .role }}:$PASSWORD@{{ include "autopg.fullname" $ }}.{{ $.Release.Namespace }}.svc.cluster.local:{{ $.Values.port }}/{{ .db }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/autopg/templates/NOTES.txt` around lines 11 - 16, The NOTES.txt output for provisionedApps shows a password command followed by a URL that assumes PASSWORD is already set, but it is never exported or assigned. Update the template around the provisionedApps loop so the password-retrieval line actually sets PASSWORD for later use, and keep the URL line consistent with that variable in the rendered instructions. Use the existing symbols include "autopg.secretName" and include "autopg.fullname" to locate the block.deploy/helm/autopg/templates/provision-job.yaml (1)
79-107: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winInjection-safety note covers passwords only, not role/db identifiers.
The comment on Lines 79-83 documents that operator-supplied passwords must not contain a single quote, but
$role/$dbare also interpolated directly into SQL text (e.g. Lines 94, 97, 100, 104-107) without an equivalent caveat. Consider extending the same documented constraint (or a lightweightcaseguard rejecting characters outside[A-Za-z0-9_]) torole/dbfor defense-in-depth, since values.yaml doesn't currently constrain these fields.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/autopg/templates/provision-job.yaml` around lines 79 - 107, The provisioning script in provision-job.yaml only documents injection-safety for passwords, but provision_app still interpolates role and db directly into SQL. Add a guard in provision_app (or near psqlx) that validates $role and $db against a safe allowlist such as [A-Za-z0-9_], and reject invalid values before any psqlx calls. Also update the existing comment near the psqlx/password handling to reflect that role/db identifiers are constrained too, so the assumptions match the SQL used in CREATE/ALTER/GRANT statements.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/helm/autopg/templates/provision-job.yaml`:
- Around line 45-51: The provision job uses APP_PW_{{ .role }} as an env var
name, which can produce invalid Kubernetes env keys for roles with dashes, dots,
or leading digits. Update the env var naming in the provisionedApps loop in
provision-job.yaml to use a stable, admission-safe scheme such as the loop
index, and make the matching "$APP_PW_..." reference use the same identifier so
the Job still reads the correct password.
In `@deploy/helm/autopg/templates/statefulset.yaml`:
- Around line 29-30: The StatefulSet in the autopg chart is missing
container-level hardening, leaving the root filesystem writable. Update the
container security settings in the StatefulSet template to set
readOnlyRootFilesystem: true alongside the existing podSecurityContext, and add
the small writable mounts needed by the process for /var/run/autopg and /tmp
using emptyDir volumes. Make sure the volumeMounts and matching volumes are
wired through the same StatefulSet/container spec so PGDATA stays on its
dedicated volume while only the expected runtime paths remain writable.
---
Nitpick comments:
In `@deploy/helm/autopg/templates/NOTES.txt`:
- Around line 11-16: The NOTES.txt output for provisionedApps shows a password
command followed by a URL that assumes PASSWORD is already set, but it is never
exported or assigned. Update the template around the provisionedApps loop so the
password-retrieval line actually sets PASSWORD for later use, and keep the URL
line consistent with that variable in the rendered instructions. Use the
existing symbols include "autopg.secretName" and include "autopg.fullname" to
locate the block.
In `@deploy/helm/autopg/templates/provision-job.yaml`:
- Around line 79-107: The provisioning script in provision-job.yaml only
documents injection-safety for passwords, but provision_app still interpolates
role and db directly into SQL. Add a guard in provision_app (or near psqlx) that
validates $role and $db against a safe allowlist such as [A-Za-z0-9_], and
reject invalid values before any psqlx calls. Also update the existing comment
near the psqlx/password handling to reflect that role/db identifiers are
constrained too, so the assumptions match the SQL used in CREATE/ALTER/GRANT
statements.
In `@deploy/helm/autopg/templates/secret.yaml`:
- Around line 31-40: The secret template block is duplicating the app password
key format inline instead of using the existing helper. Update the logic in the
range over .Values.provisionedApps to use autopg.appPasswordKey for the
role-derived password key, replacing the manual printf-based construction so the
key format stays centralized and consistent with _helpers.tpl.
In `@deploy/helm/autopg/templates/statefulset.yaml`:
- Around line 71-79: The postStart hook in the statefulset template falls
through after the readiness loop without clearly reporting whether it timed out
or completed successfully. Update the hook logic around the pg_isready wait, the
HBA check, and the psql reload so it emits an explicit log message when the
90x2s wait expires before continuing or failing, and keep the existing
idempotent grep/pg_reload_conf flow in the same script block. Use the hook
script in the autopg StatefulSet template to distinguish “timed out waiting for
Postgres” from “pg_hba rule already present” for easier first-boot debugging.
In `@deploy/helm/autopg/values.yaml`:
- Around line 95-102: The default hostAuth.cidr setting is too broad for the
chart’s PostgreSQL access model and should be documented more clearly in the
values file. Update the hostAuth section in values.yaml to explicitly recommend
pairing hostAuth.cidr: "all" with a NetworkPolicy or a tighter pod/service CIDR,
and reference the hostAuth.enabled, hostAuth.cidr, and hostAuth.method settings
so users understand the intended secure deployment pattern. Keep the default
unchanged, but add concise guidance that the chart does not create ingress
restrictions itself.
In `@deploy/README.md`:
- Around line 9-14: Add a language identifier to the fenced block in the README
so markdownlint MD040 passes; update the code fence containing the deploy
directory listing to use a plain text hint, and keep the content unchanged. Use
the existing README fenced block near the deploy tree listing as the target for
this fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2ff83d91-9f59-45a8-8364-4179fc8b0b3f
📒 Files selected for processing (14)
deploy/.dockerignoredeploy/Dockerfiledeploy/README.mddeploy/helm/autopg/.helmignoredeploy/helm/autopg/Chart.yamldeploy/helm/autopg/templates/NOTES.txtdeploy/helm/autopg/templates/_helpers.tpldeploy/helm/autopg/templates/configmap.yamldeploy/helm/autopg/templates/provision-job.yamldeploy/helm/autopg/templates/secret.yamldeploy/helm/autopg/templates/service-headless.yamldeploy/helm/autopg/templates/service.yamldeploy/helm/autopg/templates/statefulset.yamldeploy/helm/autopg/values.yaml
| {{- range .Values.provisionedApps }} | ||
| - name: APP_PW_{{ .role }} | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ include "autopg.secretName" $ }} | ||
| key: {{ printf "%s-password" .role }} | ||
| {{- end }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## File map\n'
git ls-files deploy/helm/autopg/templates deploy/helm/autopg | sed -n '1,200p'
printf '\n## Relevant template outline\n'
ast-grep outline deploy/helm/autopg/templates/provision-job.yaml --view expanded || true
printf '\n## Relevant values / helpers search\n'
rg -n --no-heading 'provisionedApps|secretName|APP_PW_|role:|db:' deploy/helm/autopg -S
printf '\n## Read target file slice\n'
cat -n deploy/helm/autopg/templates/provision-job.yaml | sed -n '35,130p'
printf '\n## Read values file slice\n'
cat -n deploy/helm/autopg/values.yaml | sed -n '1,220p'
printf '\n## Read helpers file slice\n'
cat -n deploy/helm/autopg/templates/_helpers.tpl | sed -n '1,220p'Repository: automagik-dev/autopg
Length of output: 18581
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## Search for role validation / constraints\n'
rg -n --no-heading 'role.*regex|regex.*role|validate.*role|C_IDENTIFIER|A-Za-z_][A-Za-z0-9_]*|allowed characters|sanitize|sanitize.*role|provisionedApps' deploy/helm/autopg -S
printf '\n## Search for appPasswordKey helper usage\n'
rg -n --no-heading 'autopg\.appPasswordKey|printf "%s-password"|APP_PW_' deploy/helm/autopg/templates -S
printf '\n## Read provision-job and NOTES slices around password usage\n'
cat -n deploy/helm/autopg/templates/provision-job.yaml | sed -n '40,120p'
printf '\n---\n'
cat -n deploy/helm/autopg/templates/NOTES.txt | sed -n '1,80p'Repository: automagik-dev/autopg
Length of output: 7162
Use a stable env var name for provisioned app passwords.
APP_PW_{{ .role }} will be rejected for roles containing -, ., or a leading digit, so this Job can fail at admission for valid-looking app names. Use a fixed name scheme (for example, the loop index) here and in the "$APP_PW_..." reference below.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deploy/helm/autopg/templates/provision-job.yaml` around lines 45 - 51, The
provision job uses APP_PW_{{ .role }} as an env var name, which can produce
invalid Kubernetes env keys for roles with dashes, dots, or leading digits.
Update the env var naming in the provisionedApps loop in provision-job.yaml to
use a stable, admission-safe scheme such as the loop index, and make the
matching "$APP_PW_..." reference use the same identifier so the Job still reads
the correct password.
| securityContext: | ||
| {{- toYaml .Values.podSecurityContext | nindent 8 }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Container root filesystem is writable (Trivy KSV-0014).
No readOnlyRootFilesystem: true is set at the container level. Given PGDATA is on a dedicated volume mount and the socket dir//tmp are the only other expected write targets, this is achievable by adding a container securityContext plus small emptyDir mounts for /var/run/autopg and /tmp.
🔒 Suggested hardening
containers:
- name: autopg
image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"
imagePullPolicy: {{ .Values.image.pullPolicy }}
+ securityContext:
+ readOnlyRootFilesystem: true
+ allowPrivilegeEscalation: false
command: ["autopg", "postmaster"]Add matching emptyDir volumes/mounts for /var/run/autopg and /tmp if the process writes there.
Also applies to: 98-106
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deploy/helm/autopg/templates/statefulset.yaml` around lines 29 - 30, The
StatefulSet in the autopg chart is missing container-level hardening, leaving
the root filesystem writable. Update the container security settings in the
StatefulSet template to set readOnlyRootFilesystem: true alongside the existing
podSecurityContext, and add the small writable mounts needed by the process for
/var/run/autopg and /tmp using emptyDir volumes. Make sure the volumeMounts and
matching volumes are wired through the same StatefulSet/container spec so PGDATA
stays on its dedicated volume while only the expected runtime paths remain
writable.
Source: Linters/SAST tools
…tion Verified in the wild on a node restart (2026-07-03): kubelet fsGroup re-perms pgdata on every mount (postgres refuses setgid/g+w) and an unclean shutdown leaves a stale postmaster.pid that fatals under PID reuse — a fix-pgdata initContainer repairs both each pod start. Also: the postmaster hardcodes the default superuser password for its admin pool (AUTOPG_PG_PASSWORD ignored on that path), so the Job's rotation crash-looped recreated pods; the Job now pins the default.
1 participant
What
Adds NEW files under
deploy/(no changes to existing autopg source) to run autopg (embedded PostgreSQL 18) on Kubernetes:deploy/Dockerfile— arm64, glibc, offline-boot self-contained imagedeploy/helm/autopg/— a reusable Helm chartdeploy/README.md— chart interface + rationaleBuilt and verified end-to-end on OrbStack arm64 k8s.
The critical gotcha (offline boot)
The compiled
autopgbinary resolves postgres/initdb from a per-platform cache under$AUTOPG_CONFIG_DIR/bin/<platformKey>/, and downloads@embedded-postgresfrom npm on first boot if absent (src/postgres.jsgetBinaryPaths/isCachedValid). The Dockerfile pre-seeds that cache from the release tarball'spostgres/tree plus a.versionmarker (18.3.0-beta.17, matchingPINNED_PG_VERSION). Proven offline withdocker run --network none→PostgreSQL 18.3 accepting connections, zero npm fetch.Chart highlights
/var/lib/autopg/data; PGDATA is thepgdata/subdir so the non-root user (uid 1000) owns it and initdb'schmod 0700succeeds under a k8s PVC (whose root isroot:fsGroup).listen_addresses='*'viapostgres._extra; operator GUC tuning as curated top-levelpostgres.*keys (autopg's curated defaults win over_extra).pg_hbarule + reloads — autopg's initdb writes a localhost-onlypg_hba.confand exposes no knob, so in-cluster peers would otherwise be rejected despitelisten_addresses='*'.lookup).LOGINrole that OWNS its database +schema public(so it can run migrations), and rotates the superuser password to the managed Secret. Default: dbomni, roleomni.Verification (OrbStack arm64)
docker build --platform linux/arm64✓--network none) → PG 18.3 accepting connections, no npm ✓helm install→ pod Ready; no npm fetch in pod logs ✓omnidb +omnirole created ✓omniover the Service and runs CREATE/ALTER/INSERT/DROP;tableowner = omni✓ (proves schema ownership + in-cluster peer auth)postgresrejected) ✓helm upgradeidempotent; passwords stable ✓Summary by CodeRabbit
New Features
Documentation