Skip to content

Corrected pg-port environment variable name - #56

Merged
guanzhousongmicrosoft merged 1 commit into
documentdb:mainfrom
Asamsig:main
Aug 3, 2026
Merged

Corrected pg-port environment variable name#56
guanzhousongmicrosoft merged 1 commit into
documentdb:mainfrom
Asamsig:main

Conversation

@Asamsig

@AsamsigAsamsig commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Corrected the PostgreSQL port environment variable name from PG_PORT to the actually used name POSTGRESQL_PORT in the docker-local entrypoint: emulator_entrypoint.sh#L247.

Signed-off-by: Alexander Samsig <alexander.samsig@gmail.com>

@GuanzhouSongGuanzhouSong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified against upstream: POSTGRESQL_PORT is correct. emulator_entrypoint.sh parses --pg-port into export POSTGRESQL_PORT=$1, defaults it to 9712, and Dockerfile_documentdb_local sets it. Upstream's own test_emulator_entrypoint.py pins this exact string. Thanks for catching it.

One note for the record: PG_PORT wasn't a stray typo — it's a real variable in a different tool, documentdb-local/scripts/documentdb-gateway-admin.sh, which has its own --pg-port. Easy to conflate the two.

Apologies for the wall of review comments — an automated pass validated the whole page against upstream, not just your diff. None of those are requests on this PR. Everything except your one line is pre-existing and I'm fixing it in a follow-up. Approving as-is.

@documentdbdocumentdb deleted a comment from GuanzhouSongAug 3, 2026
@documentdbdocumentdb deleted a comment from GuanzhouSongAug 3, 2026
@documentdbdocumentdb deleted a comment from GuanzhouSongAug 3, 2026
@documentdbdocumentdb deleted a comment from GuanzhouSongAug 3, 2026
@documentdbdocumentdb deleted a comment from GuanzhouSongAug 3, 2026
@documentdbdocumentdb deleted a comment from GuanzhouSongAug 3, 2026
@guanzhousongmicrosoft
guanzhousongmicrosoft merged commit 6fe15e1 into documentdb:mainAug 3, 2026
guanzhousongmicrosoft added a commit that referenced this pull request Aug 3, 2026
* Correct documentdb-local option reference against upstream source
Follow-up to #56, which fixed the PG_PORT -> POSTGRESQL_PORT name. The
same class of error was present throughout the page. Validated every row
of the option table against documentdb-local/scripts/emulator_entrypoint.sh
and packaging/gateway/docker/Dockerfile_documentdb_local.
Corrections:
- --documentdb-port documented PORT; the entrypoint reads DOCUMENTDB_PORT.
Identical bug to the one #56 fixed, five rows above it.
- --password was documented as required with default NA. The real default
is Admin100, applied before the required-check, so the check never fires.
Documented the actual default and why it must be overridden.
- --start-pg and --create-user claimed no env var. Both read
START_POSTGRESQL / CREATE_USER, and both are set in the Dockerfile.
- --start-pg, --create-user, --allow-external-connections and
--enable-telemetry all consume a following value but were documented as
bare flags. The documented form swallows the next argument.
- --cert-path described .pfx certificates and a CERT_SECRET variable.
Neither exists; the gateway is always configured with CertType PemFile.
- The mandatory --cert-path/--key-file pairing was never stated. Supplying
one without the other exits 1 at startup.
- Dropped the Azure Application Insights attribution from --enable-telemetry;
it appears nowhere in the product.
- Added the five supported options that were missing entirely: --tlsMode,
--init-data, --init-data-path, --skip-init-data, --disable-extended-rum.
--tlsMode governs whether the tls=true examples on this page work.
Beyond the table:
- Added a readiness step. docker ps reports Up long before the gateway
accepts connections; the gateway poll alone allows 60s. Following the
page literally produced MongoServerSelectionError.
- Certificates are generated on first start and reused, not regenerated
every start. The gateway logs "reusing existing certificate" and the
reuse is deliberate so client trust pinning survives restart. Same stale
sentence corrected in three getting-started pages.
- The certificate copy path no longer exists. Auto-generated TLS material
now lives under a resolved state directory; the example pins it with
DOCUMENTDB_TLS_STATE_DIR rather than hardcoding a path that depends on
which directories happen to be writable.
- Noted that data is discarded on container removal unless a volume is
mounted, and corrected the --data-path example, which was only a bind
mount and never changed the data path.
- Sample output showed container name optimistic_blackwell despite
--name docdb in the command above it, and leaked a real home directory
into the mongosh connection string.
* Address review findings on documentdb-local option reference
Follow-up corrections after validating the page against the upstream
entrypoint, Dockerfile, and gateway source:
- Certificate section no longer starts a second container. The name and
published port collided with the container started under Running, so
the page could not be followed top to bottom. It now documents the
path the gateway actually resolves to in this image
(/home/documentdb/.local/state/documentdb-gateway/tls) and keeps the
log-grep as the authoritative check.
- Data persistence: the image declares VOLUME ["/data"], so data is
orphaned in an anonymous volume, not discarded.
- Readiness wait is now a bounded command. "docker logs -f" never
returns, because the entrypoint streams logs for the container's life.
- --log-level, --enable-telemetry, and --disable-extended-rum are marked
as known issues: all three are validated at startup and then have no
effect.
- Valueless flags (--skip-init-data, --disable-extended-rum) warn that
passing a value hangs the argument parser.
- --username documents the reserved-name and blocked-prefix constraints.
- --owner documents that any non-default value aborts startup.
- --allow-external-connections documents the pg_hba rule it adds, the
need to publish 9712, and that it is ignored with --start-pg false.
- --data-path clarifies that the mount is a docker option and the flag a
container argument; adds a complete example.
- Certificate stability is scoped to restarts of the same container, and
the 365-day validity is stated.
Getting-started pages: align the credential and TLS wording with the
corrected option table (credentials default rather than being required,
default tlsMode accepts plain connections) and carry the readiness step.
configuration.md notes the TLS state dir fallback.
* Refine readiness command portability and certificate persistence guidance
- The readiness wait used `timeout`, which is GNU coreutils and not
present on macOS. Replaced with a plain `until` loop plus explicit
guidance on what a hang means.
- The persistence example pinned DOCUMENTDB_TLS_STATE_DIR at a separate
named volume. Docker creates such a mount point root-owned and the
gateway runs as the unprivileged `documentdb` user, so it could not
write the key there. Pinned inside the data volume instead, which the
entrypoint chowns on every start, and stated the chmod trade-off.
- --owner wording now describes the initialized cluster rather than the
image.
* Drop unproven in-page anchor; document --start-pg / --create-user effects
No other page in this repo uses in-page anchor links, so the reference to
the Running section is now plain text.
--start-pg and --create-user had tautological descriptions. Both have
real consequences: false means supplying your own PostgreSQL, and false
means no admin user is created, so --username / --password do nothing and
nothing can authenticate.
* Tighten --create-user description
--username / --password are still validated and still used by data
initialization when --create-user is false; only authentication is
impossible. Say that rather than "have no effect".
* Note the teardown step before the TLS-state-dir example
Third docker run on the page reusing --name docdb and -p 10260:10260;
say to remove the earlier container first so the page still reads
top to bottom.
---------
Co-authored-by: Guanzhou Song <guanzhousong@microsoft.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Asamsig@GuanzhouSong@guanzhousongmicrosoft