PMM-15238: Expose the built-in PostgreSQL to SEP - #5700
Open
ademidoff wants to merge 1 commit into
Open
Conversation
ademidoff
requested review from
4nte and
JiriCtvrtka
and removed request for
a team
July 26, 2026 23:24
1 task
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5700 +/- ##
==========================================
+ Coverage 43.59% 45.52% +1.92%
==========================================
Files 415 418 +3
Lines 43134 43409 +275
==========================================
+ Hits 18804 19761 +957
+ Misses 22454 21694 -760
- Partials 1876 1954 +78
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
JiriCtvrtka
approved these changes
Jul 27, 2026
4nte
approved these changes
Jul 27, 2026
1 task
1 task
yyyyyyyan
approved these changes
Aug 5, 2026
15 tasks
Add an opt-in integration that lets SEP, running in a side container on a shared Docker bridge network, use PMM's embedded PostgreSQL for its persistence layer. When PMM_ENABLE_SEP is set, the entrypoint appends marker-delimited blocks to postgresql.conf (listen_addresses) and pg_hba.conf (one scram-sha-256 rule per attached Docker subnet, scoped to the sep database and role), then provisions a non-superuser sep role owning a dedicated sep database. Nothing is published on the host, and the postgres, pmm-managed and grafana accounts remain unreachable over the network. Unsetting the variable reverts the configuration on the next start and leaves the role and database intact. The postgres data directory, password file and binary directory are now declared once in the entrypoint and passed to the helper scripts via a subshell-scoped export, replacing the /usr/pgsql-14 literals that were duplicated across them.
yyyyyyyan
force-pushed
the
PMM-15238-expose-pg-to-sep
branch
from
August 10, 2026 16:16
42cf667 to
45f2a37
Compare
WalkthroughThe change adds optional SEP access to the built-in PostgreSQL server. It introduces conditional role and database provisioning, subnet-based authentication rules, configurable PostgreSQL binary paths, and entrypoint handling for SEP environment variables. ChangesSEP PostgreSQL access
Sequence Diagram(s)sequenceDiagram
participant Entrypoint
participant postgres-sep
participant PostgreSQL
Entrypoint->>postgres-sep: Export SEP and PostgreSQL settings
postgres-sep->>PostgreSQL: Apply network and authentication configuration
postgres-sep->>PostgreSQL: Create or update sep role and database
PostgreSQL-->>postgres-sep: Return command results
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@managed/utils/envvars/parser.go`:
- Around line 123-125: Update the raw environment-entry trace logging in the
parser, before the switch handling “PMM_ENABLE_SEP” and
“PMM_SEP_POSTGRES_PASSWORD”, to avoid emitting the password value; log only the
environment key or redact its value while preserving existing filtering
behavior.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5f3afa25-7950-41c5-8756-d7229db5279d
📒 Files selected for processing (7)
.env.examplebuild/ansible/roles/postgres/files/postgres-migrationbuild/ansible/roles/postgres/files/postgres-sepbuild/docker/server/entrypoint.shdocker-compose.ymlmanaged/utils/envvars/parser.gomanaged/utils/envvars/parser_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
percona/pmm-qa(manual)percona/pmm(manual)
Comment on lines
+123
to
+125
| case "PMM_ENABLE_SEP", "PMM_SEP_POSTGRES_PASSWORD": | ||
| // skip env variables consumed by the entrypoint to expose postgres to SEP | ||
| continue |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Prevent PMM_SEP_POSTGRES_PASSWORD from reaching trace logs.
Line 85 logs every raw environment entry before this case executes. When trace logging is enabled, it records the SEP database password. Log only the key, or redact the value before logging. Make it so.
Proposed fix
- logrus.Tracef("ParseEnvVars: %#q: k=%#q v=%#q", env, k, v)
+ logrus.Tracef("ParseEnvVars: k=%#q", k)🤖 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 `@managed/utils/envvars/parser.go` around lines 123 - 125, Update the raw
environment-entry trace logging in the parser, before the switch handling
“PMM_ENABLE_SEP” and “PMM_SEP_POSTGRES_PASSWORD”, to avoid emitting the password
value; log only the environment key or redact its value while preserving
existing filtering behavior.
Source: Coding guidelines
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket number: PMM-15238
Feature build: Percona-Lab/pmm-submodules#4495
Lets SEP, running in a side container on a shared Docker bridge network, use PMM's embedded PostgreSQL for its persistence layer. Entirely opt-in — with
PMM_ENABLE_SEPunset, nothing about PMM changes.What it does
When
PMM_ENABLE_SEP=1andPMM_SEP_POSTGRES_PASSWORDare set, the entrypoint runs a new helper (build/ansible/roles/postgres/files/postgres-sep) before supervisord starts. It:postgresql.confsettinglisten_addresses = '*'— this only covers the container's own network namespace, nothing is published on the host;pg_hba.confwith onescram-sha-256rule per attached Docker subnet (derived fromip -o -4 route show scope link), scoped to thesepdatabase andsepuser only;seprole (NOSUPERUSER NOCREATEROLE NOCREATEDB) owning a dedicatedsepdatabase.The
postgres,pmm-managedandgrafanaaccounts stay unreachable over the network — Postgres rejects them withno pg_hba.conf entry.Both config blocks are delimited by
# BEGIN PMM SEP/# END PMM SEPand rewritten on every start, so the operation is idempotent. UnsettingPMM_ENABLE_SEPstrips them on the next start and returns Postgres to loopback-only; theseprole and database are deliberately left in place, since a config flag should not destroy data.Drive-by: PG version literals
/srv/postgres14,/srv/.postgres_passwordand/usr/pgsql-14/binwere duplicated across the entrypoint andpostgres-migration. They are now declared once inbuild/docker/server/entrypoint.shand passed to both helper scripts via a subshell-scoped export, so they do not leak into the environment supervisord and its children inherit (verified across every/proc/*/environ). Each script aborts with a named message if one is missing.psqlnow resolves through$POSTGRES_BIN_DIRrather than/usr/bin/psql, which is an alternatives symlink — fine today, but it resolves to whichever version alternatives prefers, exactly the wrong property during a 14 → 18 migration with both installed.Still pinned to 14 and out of scope here:
build/ansible/roles/supervisord/files/pmm.ini,managed/services/supervisord/pmm_config.go, and thebuild/ansible/roles/postgres/tasks/*.ymlplaybooks.Testing
Verified against
percona/pmm-server:3.8.1with the scripts bind-mounted, using a psql client container on a shared bridge network.sep@sepconnects, DDL/DML works,pg_stat_statementscreatedsep→pmm-managed/postgressuperuser /grafanaover the networkno pg_hba.conf entryrolsuper,rolcreatedb,rolcreateroleall falsePMM_ENABLE_SEPunsetseprole and database survivePMM_SEP_POSTGRES_PASSWORDPMM_DISABLE_BUILTIN_POSTGRES=1/PMM_HA_ENABLE=1hostrule per subnet, connects from bothreadyzOK,health=healthyPOSTGRES_*in supervisord and all childrenshellcheckclean on all three scripts;go test ./managed/utils/envvars/...passes.Notes
pmm-managedandgrafana, or thesepdatabase will be silently dropped — nothing in PMM's code references it, so the omission would not be caught.