carverauto/serviceradar

Fork 0

3002 bug docker builds failing #3021

Merged

mfreeman451 merged 2 commits from refs/pull/3021/head into staging

2026-03-07 20:03:54 +00:00

mfreeman451 commented

2026-03-07 20:02:33 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #3003
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/3003
Original created: 2026-03-07T20:02:33Z
Original updated: 2026-03-07T20:10:18Z
Original head: carverauto/serviceradar:3002-bug-docker-builds-failing
Original base: staging
Original merged: 2026-03-07T20:03:54Z by @mfreeman451

IMPORTANT: Please sign the Developer Certificate of Origin

Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include
a DCO sign-off statement indicating the DCO acceptance in one commit message. Here
is an example DCO Signed-off-by line in a commit message:

Signed-off-by: J. Doe <j.doe@domain.com>

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

I have signed the DCO?
The build completes without errors?
All tests are passing when running make test?

Imported from GitHub pull request. Original GitHub pull request: #3003 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/3003 Original created: 2026-03-07T20:02:33Z Original updated: 2026-03-07T20:10:18Z Original head: carverauto/serviceradar:3002-bug-docker-builds-failing Original base: staging Original merged: 2026-03-07T20:03:54Z by @mfreeman451 --- ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test?

qodo-code-review[bot] commented

2026-03-07 20:02:54 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/3003#issuecomment-4017242099
Original created: 2026-03-07T20:02:54Z

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Review Summary by Qodo

Add Docker Compose CNPG PG16-to-PG18 migration path with credential preservation

✨ Enhancement 🐞 Bug fix

Walkthroughs

Description

• Add Docker Compose CNPG PG16-to-PG18 migration workflow with automatic detection
• Implement version preflight check to prevent unsupported direct PG16-on-PG18 boots
• Preserve legacy and new credential models during migration, including superuser username support
• Update bootstrap scripts to handle file-based credentials and legacy recovery modes
• Add certificate generation improvements for dynamic superuser names and CN validation

Diagram

flowchart LR
  A["PG16 Data Volume"] -->|detect version mismatch| B["Version Preflight Check"]
  B -->|fail fast| C["Error: Migration Required"]
  B -->|version match| D["Normal Startup"]
  E["Migration Script"] -->|pg_dumpall| F["Dump PG16 Data"]
  F -->|restore to PG18| G["PG18 Target Volume"]
  G -->|promote| H["Updated cnpg-data Volume"]
  H -->|seed credentials| I["cnpg-credentials Volume"]
  I -->|bootstrap| J["Ready for PG18 Stack"]

File Changes

1. elixir/serviceradar_core/lib/serviceradar/cluster/startup_migrations.ex Refactoring +8/-6

Rename password file reader to generic text file reader

elixir/serviceradar_core/lib/serviceradar/cluster/startup_migrations.ex

2. docker/compose/bootstrap-db-credentials.sh ✨ Enhancement +77/-31

Add legacy credential recovery and superuser username support

docker/compose/bootstrap-db-credentials.sh

3. docker/compose/check-cnpg-major-version.sh ✨ Enhancement +23/-0

Add PostgreSQL version preflight check for CNPG

docker/compose/check-cnpg-major-version.sh

View more (11)

4. docker/compose/generate-certs.sh ✨ Enhancement +22/-3

Add dynamic superuser name support and certificate CN validation

docker/compose/generate-certs.sh

5. docker/compose/migrate-cnpg-pg16-to-pg18.sh ✨ Enhancement +308/-0

Add comprehensive PG16-to-PG18 migration workflow script

docker/compose/migrate-cnpg-pg16-to-pg18.sh

6. docker-compose.yml ⚙️ Configuration changes +41/-5

Add migration and version check services, update data paths

docker-compose.yml

7. DOCKER_QUICKSTART.md 📝 Documentation +21/-2

Document auto-migration and credential seeding for PG18 upgrade

DOCKER_QUICKSTART.md

8. README-Docker.md 📝 Documentation +34/-3

Document PG16-to-PG18 migration workflow and credential recovery

README-Docker.md

9. docs/docs/docker-setup.md 📝 Documentation +25/-2

Update Docker setup guide with migration and credential instructions

docs/docs/docker-setup.md

10. openspec/changes/add-compose-cnpg-pg18-migration-path/design.md 📝 Documentation +72/-0

Add design document for CNPG PG18 migration architecture

openspec/changes/add-compose-cnpg-pg18-migration-path/design.md

11. openspec/changes/add-compose-cnpg-pg18-migration-path/proposal.md 📝 Documentation +38/-0

Add proposal for Docker Compose CNPG migration feature

openspec/changes/add-compose-cnpg-pg18-migration-path/proposal.md

12. openspec/changes/add-compose-cnpg-pg18-migration-path/specs/cnpg/spec.md 📝 Documentation +22/-0

Add CNPG major upgrade requirements and scenarios

openspec/changes/add-compose-cnpg-pg18-migration-path/specs/cnpg/spec.md

13. openspec/changes/add-compose-cnpg-pg18-migration-path/specs/docker-compose-stack/spec.md 📝 Documentation +45/-0

Add Docker Compose CNPG migration requirements and test scenarios

openspec/changes/add-compose-cnpg-pg18-migration-path/specs/docker-compose-stack/spec.md

14. openspec/changes/add-compose-cnpg-pg18-migration-path/tasks.md 📝 Documentation +7/-0

Add implementation tasks checklist for CNPG migration

openspec/changes/add-compose-cnpg-pg18-migration-path/tasks.md

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/3003#issuecomment-4017242099 Original created: 2026-03-07T20:02:54Z --- <pre>ⓘ You are approaching your monthly quota for Qodo. <a href="https://www.qodo.ai/pricing">Upgrade your plan</a></pre> <h3>Review Summary by Qodo</h3> Add Docker Compose CNPG PG16-to-PG18 migration path with credential preservation <code>✨ Enhancement</code> <code>🐞 Bug fix</code> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <h3>Walkthroughs</h3> <details open> <summary>Description</summary> <pre> • Add Docker Compose CNPG PG16-to-PG18 migration workflow with automatic detection • Implement version preflight check to prevent unsupported direct PG16-on-PG18 boots • Preserve legacy and new credential models during migration, including superuser username support • Update bootstrap scripts to handle file-based credentials and legacy recovery modes • Add certificate generation improvements for dynamic superuser names and CN validation </pre> </details> <details> <summary>Diagram</summary> > ```mermaid flowchart LR A["PG16 Data Volume"] -->|detect version mismatch| B["Version Preflight Check"] B -->|fail fast| C["Error: Migration Required"] B -->|version match| D["Normal Startup"] E["Migration Script"] -->|pg_dumpall| F["Dump PG16 Data"] F -->|restore to PG18| G["PG18 Target Volume"] G -->|promote| H["Updated cnpg-data Volume"] H -->|seed credentials| I["cnpg-credentials Volume"] I -->|bootstrap| J["Ready for PG18 Stack"] ``` </details> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <h3>File Changes</h3> <details> <summary>1. elixir/serviceradar_core/lib/serviceradar/cluster/startup_migrations.ex <code> Refactoring </code> <code> +8/-6 </code> </summary> >Rename password file reader to generic text file reader > ><a href='https://github.com/carverauto/serviceradar/pull/3003/files#diff-576a6d0bf0bf904dc22e581cac58d4da08016f26ba51837b964090cf2f661ddf'> elixir/serviceradar_core/lib/serviceradar/cluster/startup_migrations.ex </a> <hr/> </details> <details> <summary>2. docker/compose/bootstrap-db-credentials.sh <code>✨ Enhancement</code> <code> +77/-31 </code> </summary> >Add legacy credential recovery and superuser username support > ><a href='https://github.com/carverauto/serviceradar/pull/3003/files#diff-c719beb8480debac95e9f0306aa1ae3c13d119d6bd3af1ec2cc4cbf66ae06d55'> docker/compose/bootstrap-db-credentials.sh </a> <hr/> </details> <details> <summary>3. docker/compose/check-cnpg-major-version.sh <code>✨ Enhancement</code> <code> +23/-0 </code> </summary> >Add PostgreSQL version preflight check for CNPG > ><a href='https://github.com/carverauto/serviceradar/pull/3003/files#diff-ab00972836afc11c697e6a80ec7a821da060c5a802fad5da35f92e79d923cea7'> docker/compose/check-cnpg-major-version.sh </a> <hr/> </details> <details><summary><ins>View more (11)</ins></summary> <details> <summary>4. docker/compose/generate-certs.sh <code>✨ Enhancement</code> <code> +22/-3 </code> </summary> >Add dynamic superuser name support and certificate CN validation > ><a href='https://github.com/carverauto/serviceradar/pull/3003/files#diff-8298241543b4744a6ac7780c760ac5b5a0a87ba62de19c8612ebe1aba0996ebd'> docker/compose/generate-certs.sh </a> <hr/> </details> <details> <summary>5. docker/compose/migrate-cnpg-pg16-to-pg18.sh <code>✨ Enhancement</code> <code> +308/-0 </code> </summary> >Add comprehensive PG16-to-PG18 migration workflow script > ><a href='https://github.com/carverauto/serviceradar/pull/3003/files#diff-15b9aa3f74894d188bc3823423c699c10db386174fc503c00e9e08e76e94de46'> docker/compose/migrate-cnpg-pg16-to-pg18.sh </a> <hr/> </details> <details> <summary>6. docker-compose.yml <code>⚙️ Configuration changes</code> <code> +41/-5 </code> </summary> >Add migration and version check services, update data paths > ><a href='https://github.com/carverauto/serviceradar/pull/3003/files#diff-e45e45baeda1c1e73482975a664062aa56f20c03dd9d64a827aba57775bed0d3'> docker-compose.yml </a> <hr/> </details> <details> <summary>7. DOCKER_QUICKSTART.md <code>📝 Documentation</code> <code> +21/-2 </code> </summary> >Document auto-migration and credential seeding for PG18 upgrade > ><a href='https://github.com/carverauto/serviceradar/pull/3003/files#diff-d39bb63b06e41b96069fe4f289e4d28a4f205f8e953c141e6f4d9c715bbb75b3'> DOCKER_QUICKSTART.md </a> <hr/> </details> <details> <summary>8. README-Docker.md <code>📝 Documentation</code> <code> +34/-3 </code> </summary> >Document PG16-to-PG18 migration workflow and credential recovery > ><a href='https://github.com/carverauto/serviceradar/pull/3003/files#diff-9fd61d24482efe68c22d8d41e2a1dcc440f39195aa56e7a050f2abe598179efd'> README-Docker.md </a> <hr/> </details> <details> <summary>9. docs/docs/docker-setup.md <code>📝 Documentation</code> <code> +25/-2 </code> </summary> >Update Docker setup guide with migration and credential instructions > ><a href='https://github.com/carverauto/serviceradar/pull/3003/files#diff-8604269dffb3ce4133e48cab374ca8e97745d0efbdef67cad792aeb5945fe5ec'> docs/docs/docker-setup.md </a> <hr/> </details> <details> <summary>10. openspec/changes/add-compose-cnpg-pg18-migration-path/design.md <code>📝 Documentation</code> <code> +72/-0 </code> </summary> >Add design document for CNPG PG18 migration architecture > ><a href='https://github.com/carverauto/serviceradar/pull/3003/files#diff-4ffb562da10017a9e9896dc6a40dbefcbe44ef5cb63d6773521dd74393e77a48'> openspec/changes/add-compose-cnpg-pg18-migration-path/design.md </a> <hr/> </details> <details> <summary>11. openspec/changes/add-compose-cnpg-pg18-migration-path/proposal.md <code>📝 Documentation</code> <code> +38/-0 </code> </summary> >Add proposal for Docker Compose CNPG migration feature > ><a href='https://github.com/carverauto/serviceradar/pull/3003/files#diff-3ab473af1c95807ed1c9126cf7edec8be8de632aa6d4033d509753895129a527'> openspec/changes/add-compose-cnpg-pg18-migration-path/proposal.md </a> <hr/> </details> <details> <summary>12. openspec/changes/add-compose-cnpg-pg18-migration-path/specs/cnpg/spec.md <code>📝 Documentation</code> <code> +22/-0 </code> </summary> >Add CNPG major upgrade requirements and scenarios > ><a href='https://github.com/carverauto/serviceradar/pull/3003/files#diff-d8eda3d23e4f5f924a9ad7756516f5695d479aaa6024b972eb74dd67038e84f3'> openspec/changes/add-compose-cnpg-pg18-migration-path/specs/cnpg/spec.md </a> <hr/> </details> <details> <summary>13. openspec/changes/add-compose-cnpg-pg18-migration-path/specs/docker-compose-stack/spec.md <code>📝 Documentation</code> <code> +45/-0 </code> </summary> >Add Docker Compose CNPG migration requirements and test scenarios > ><a href='https://github.com/carverauto/serviceradar/pull/3003/files#diff-c163a1a06bb9a68c2bb48062e63b64dfc835b7a232967e63a25b3ec1a6602c31'> openspec/changes/add-compose-cnpg-pg18-migration-path/specs/docker-compose-stack/spec.md </a> <hr/> </details> <details> <summary>14. openspec/changes/add-compose-cnpg-pg18-migration-path/tasks.md <code>📝 Documentation</code> <code> +7/-0 </code> </summary> >Add implementation tasks checklist for CNPG migration > ><a href='https://github.com/carverauto/serviceradar/pull/3003/files#diff-f9a9c26a334310e044e722adaf9ff09cd1e61ce08b3d14e95d4449641267ae34'> openspec/changes/add-compose-cnpg-pg18-migration-path/tasks.md </a> <hr/> </details> </details> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <a href="https://www.qodo.ai"><img src="https://www.qodo.ai/wp-content/uploads/2025/03/qodo-logo.svg" width="80" alt="Qodo Logo"></a>

qodo-code-review[bot] commented

2026-03-07 20:02:56 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/3003#issuecomment-4017242156
Original created: 2026-03-07T20:02:56Z

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (1)

1. Legacy creds bypass spec 🐞 Bug ⛨ Security

Description

The credential bootstrap now auto-assumes legacy default credentials for any existing CNPG data
volume that lacks persisted credentials and explicit overrides, instead of failing fast. This
contradicts the repo’s docker-compose-stack spec and risks persisting incorrect/insecure defaults
when the existing DB was not actually using those legacy values.

Code

docker/compose/bootstrap-db-credentials.sh[R63-107]

+has_any_persisted_credentials=false
+for target in "$SUPERUSER_FILE" "$SUPERUSER_USER_FILE" "$APP_FILE"; do
+  if [ -s "$target" ]; then
+    has_any_persisted_credentials=true
+    break
+  fi
+done
+
+has_any_explicit_overrides=false
+for value in "${CNPG_SUPERUSER:-}" "${CNPG_SUPERUSER_PASSWORD:-}" "${CNPG_PASSWORD:-}"; do
+  if [ -n "$value" ]; then
+    has_any_explicit_overrides=true
+    break
+  fi
+done
+
+use_legacy_defaults=false
+if [ "$has_existing_db" = true ] && [ "$has_any_persisted_credentials" = false ] && [ "$has_any_explicit_overrides" = false ]; then
+  case "$LEGACY_RECOVERY_MODE" in
+    auto|"")
+      use_legacy_defaults=true
+      ;;
+    true|TRUE|1|yes|YES|on|ON)
+      use_legacy_defaults=true
+      ;;
+  esac
+fi
+
+if [ "$use_legacy_defaults" = true ]; then
+  echo "Existing CNPG data detected without persisted credentials; assuming legacy Docker Compose defaults."
+  echo "To override recovery values, set CNPG_SUPERUSER, CNPG_SUPERUSER_PASSWORD, and CNPG_PASSWORD."
+fi
+
+if [ -n "${CNPG_SUPERUSER:-}" ]; then
+  superuser_name="$CNPG_SUPERUSER"
+else
+  superuser_name="$(read_value "$SUPERUSER_USER_FILE")"
+  if [ -z "$superuser_name" ]; then
+    if [ "$use_legacy_defaults" = true ]; then
+      superuser_name="$LEGACY_SUPERUSER"
+    else
+      superuser_name="postgres"
+    fi
+  fi
+fi

Evidence

The compose spec requires the bootstrap job to exit with an error when an existing DB is detected
but credentials are missing and no explicit env overrides are provided. The updated bootstrap script
instead enables use_legacy_defaults by default (CNPG_LEGACY_RECOVERY=auto) and proceeds to
write/assume legacy credentials in that exact scenario; docs describe this behavior as intentional,
but the spec was not updated accordingly and the behavior can still be wrong for non-default legacy
installs.

openspec/specs/docker-compose-stack/spec.md[74-81]
docker/compose/bootstrap-db-credentials.sh[63-108]
DOCKER_QUICKSTART.md[104-109]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The credential bootstrap now auto-recovers legacy defaults on existing DB volumes without credentials/overrides, which contradicts the current docker-compose-stack spec (fail-fast requirement) and can persist incorrect/insecure defaults.

## Issue Context
Docs state legacy recovery is intentional, but the canonical spec still mandates fail-fast. Also, if an operator had a legacy install with non-default credentials, auto-assuming defaults will write misleading values into the credentials volume and may cause confusing authentication failures.

## Fix Focus Areas
- docker/compose/bootstrap-db-credentials.sh[63-127]
- openspec/specs/docker-compose-stack/spec.md[74-81]
- DOCKER_QUICKSTART.md[104-109]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Migrator fails when volume missing 📎 Requirement gap ⛯ Reliability

Description

cnpg-migrator will exit non-zero if the cnpg-data Docker volume does not exist yet, which can
prevent db-credentials (and therefore cert-generator) from completing successfully. This can
cause docker compose up -d to fail in scenarios like fresh installs or unexpected volume removal.

Code

docker/compose/migrate-cnpg-pg16-to-pg18.sh[R216-217]
+ensure_volume_exists "$DATA_VOLUME"
+ensure_compose_stack_stopped

Evidence
Compliance requires docker compose up -d to complete without db-credentials/cert-generator
startup failures (PR Compliance ID 1). The new Compose flow gates db-credentials on
cnpg-migrator success, but the migrator script currently hard-fails if the target data volume is
absent, which would cascade into Compose startup failure for the credential/cert bootstrap chain.
Docker Compose services start successfully without cert-generator or db-credentials exiting with errors
docker-compose.yml[23-41]
docker-compose.yml[55-58]
docker/compose/migrate-cnpg-pg16-to-pg18.sh[68-73]
docker/compose/migrate-cnpg-pg16-to-pg18.sh[216-223]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`cnpg-migrator` hard-fails when the CNPG data volume does not exist, but `db-credentials` is gated on `cnpg-migrator` completing successfully. This can block Compose startup.

## Issue Context
The migration should be a no-op for fresh installs and already-migrated volumes. A missing `cnpg-data` volume should be treated equivalently to an empty/fresh install.

## Fix Focus Areas
- docker/compose/migrate-cnpg-pg16-to-pg18.sh[68-73]
- docker/compose/migrate-cnpg-pg16-to-pg18.sh[216-223]
- docker-compose.yml[23-41]
- docker-compose.yml[55-58]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Migrator hardcodes chmod 🐞 Bug ⛨ Security

Description

The migration script seeds the credentials volume with hardcoded 0644 permissions, ignoring
CNPG_CRED_MODE used by the normal bootstrap path. This creates inconsistent behavior and can
unintentionally widen credential readability when operators configure stricter permissions.

Code

docker/compose/migrate-cnpg-pg16-to-pg18.sh[R138-158]

+seed_credentials_volume() {
+  if ! docker volume inspect "$CREDENTIALS_VOLUME" >/dev/null 2>&1; then
+    docker volume create "$CREDENTIALS_VOLUME" >/dev/null
+  fi
+
+  docker run --rm \
+    -e CNPG_SUPERUSER="${SOURCE_SUPERUSER}" \
+    -e CNPG_SUPERUSER_PASSWORD="${SOURCE_SUPERUSER_PASSWORD}" \
+    -e CNPG_PASSWORD="${APP_PASSWORD}" \
+    -v "${CREDENTIALS_VOLUME}:/etc/serviceradar/cnpg" \
+    alpine:3.20 \
+    sh -ceu '
+      umask 077
+      mkdir -p /etc/serviceradar/cnpg
+      printf "%s" "$CNPG_SUPERUSER" > /etc/serviceradar/cnpg/superuser-username
+      printf "%s" "$CNPG_SUPERUSER_PASSWORD" > /etc/serviceradar/cnpg/superuser-password
+      printf "%s" "$CNPG_PASSWORD" > /etc/serviceradar/cnpg/serviceradar-password
+      chmod 0644 /etc/serviceradar/cnpg/superuser-username \
+        /etc/serviceradar/cnpg/superuser-password \
+        /etc/serviceradar/cnpg/serviceradar-password
+    '

Evidence
The normal credentials bootstrap honors CNPG_CRED_MODE for permission control, but the migrator
always writes credential files as 0644, which may violate operator expectations and makes the two
seeding paths inconsistent.
docker/compose/migrate-cnpg-pg16-to-pg18.sh[138-158]
docker/compose/bootstrap-db-credentials.sh[29-55]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PG16→PG18 migrator seeds credential files with fixed `chmod 0644`, ignoring the existing `CNPG_CRED_MODE` convention used by `bootstrap-db-credentials.sh`.

## Issue Context
Operators who set `CNPG_CRED_MODE` expecting tighter permissions will get widened permissions after migration, and bootstrap vs migrator behavior becomes inconsistent.

## Fix Focus Areas
- docker/compose/migrate-cnpg-pg16-to-pg18.sh[138-159]
- docker/compose/bootstrap-db-credentials.sh[29-55]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. Migration guard too coarse 🐞 Bug ⛯ Reliability

Description

In Compose auto-migration, the migrator bypasses its “compose project running” safety check, and the
remaining check is coarse (project-label based) rather than verifying the cnpg-data volume is not
currently mounted by a running Postgres container. Adding a direct “volume in use” check would
reduce risk in upgrade/partial-start scenarios.

Code

docker-compose.yml[R23-36]

+  cnpg-migrator:
+    image: docker:28.0.1-cli
+    container_name: serviceradar-cnpg-migrator
+    environment:
+      - FORCE=true
+      - CNPG_ALLOW_RUNNING_COMPOSE_PROJECT=true
+      - SERVICERADAR_VOLUME_PREFIX=${SERVICERADAR_VOLUME_PREFIX:-serviceradar}
+      - CNPG_DATA_VOLUME=${SERVICERADAR_VOLUME_PREFIX:-serviceradar}_cnpg-data
+      - CNPG_CREDENTIALS_VOLUME=${SERVICERADAR_VOLUME_PREFIX:-serviceradar}_cnpg-credentials
+      - CNPG_DATABASE=${CNPG_DATABASE:-serviceradar}
+      - CNPG_SUPERUSER=${CNPG_SUPERUSER:-}
+      - CNPG_SUPERUSER_PASSWORD=${CNPG_SUPERUSER_PASSWORD:-}
+      - CNPG_PASSWORD=${CNPG_PASSWORD:-}
+      - CNPG_EXPECTED_PG_MAJOR=18

Evidence

Compose explicitly sets CNPG_ALLOW_RUNNING_COMPOSE_PROJECT=true, which makes
ensure_compose_stack_stopped() return early; the only built-in guard is a project-level label
check (not a volume-in-use check). This makes the migrator more dependent on external operational
behavior (e.g., ensuring old cnpg is stopped) than it needs to be.

docker-compose.yml[23-36]
docker/compose/migrate-cnpg-pg16-to-pg18.sh[195-203]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The migrator’s safety check is project-label based and is bypassed in Compose. It would be safer to also (or instead) verify that the CNPG data volume is not currently mounted by a running container before starting temporary Postgres containers.

## Issue Context
Compose sets `CNPG_ALLOW_RUNNING_COMPOSE_PROJECT=true` so the current guard is effectively disabled. A direct “volume in use” check would protect against edge cases (partial upgrades, leftover running containers, manual operator actions) without blocking the intended Compose workflow.

## Fix Focus Areas
- docker/compose/migrate-cnpg-pg16-to-pg18.sh[195-217]
- docker-compose.yml[23-36]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ The new review experience is currently in Beta. Learn more

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/3003#issuecomment-4017242156 Original created: 2026-03-07T20:02:56Z --- <h3>Code Review by Qodo</h3> <code>🐞 Bugs (3)</code> <code>📘 Rule violations (0)</code> <code>📎 Requirement gaps (1)</code> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <img src="https://www.qodo.ai/wp-content/uploads/2026/01/action-required.png" height="20" alt="Action required"> <details> <summary> 1. Legacy creds bypass spec <code>🐞 Bug</code> <code>⛨ Security</code></summary> > <details open> ><summary>Description</summary> > > ><pre> >The credential bootstrap now auto-assumes legacy default credentials for any existing CNPG data >volume that lacks persisted credentials and explicit overrides, instead of failing fast. This >contradicts the repo’s docker-compose-stack spec and risks persisting incorrect/insecure defaults >when the existing DB was not actually using those legacy values. ></pre> ></details> > <details open> ><summary>Code</summary> > > ><code>[docker/compose/bootstrap-db-credentials.sh[R63-107]](https://github.com/carverauto/serviceradar/pull/3003/files#diff-c719beb8480debac95e9f0306aa1ae3c13d119d6bd3af1ec2cc4cbf66ae06d55R63-R107)</code> > >```diff >+has_any_persisted_credentials=false >+for target in "$SUPERUSER_FILE" "$SUPERUSER_USER_FILE" "$APP_FILE"; do >+ if [ -s "$target" ]; then >+ has_any_persisted_credentials=true >+ break >+ fi >+done >+ >+has_any_explicit_overrides=false >+for value in "${CNPG_SUPERUSER:-}" "${CNPG_SUPERUSER_PASSWORD:-}" "${CNPG_PASSWORD:-}"; do >+ if [ -n "$value" ]; then >+ has_any_explicit_overrides=true >+ break >+ fi >+done >+ >+use_legacy_defaults=false >+if [ "$has_existing_db" = true ] && [ "$has_any_persisted_credentials" = false ] && [ "$has_any_explicit_overrides" = false ]; then >+ case "$LEGACY_RECOVERY_MODE" in >+ auto|"") >+ use_legacy_defaults=true >+ ;; >+ true|TRUE|1|yes|YES|on|ON) >+ use_legacy_defaults=true >+ ;; >+ esac >+fi >+ >+if [ "$use_legacy_defaults" = true ]; then >+ echo "Existing CNPG data detected without persisted credentials; assuming legacy Docker Compose defaults." >+ echo "To override recovery values, set CNPG_SUPERUSER, CNPG_SUPERUSER_PASSWORD, and CNPG_PASSWORD." >+fi >+ >+if [ -n "${CNPG_SUPERUSER:-}" ]; then >+ superuser_name="$CNPG_SUPERUSER" >+else >+ superuser_name="$(read_value "$SUPERUSER_USER_FILE")" >+ if [ -z "$superuser_name" ]; then >+ if [ "$use_legacy_defaults" = true ]; then >+ superuser_name="$LEGACY_SUPERUSER" >+ else >+ superuser_name="postgres" >+ fi >+ fi >+fi >``` ></details> > <details > ><summary>Evidence</summary> > > ><pre> >The compose spec requires the bootstrap job to exit with an error when an existing DB is detected >but credentials are missing and no explicit env overrides are provided. The updated bootstrap script >instead enables use_legacy_defaults by default (CNPG_LEGACY_RECOVERY=auto) and proceeds to >write/assume legacy credentials in that exact scenario; docs describe this behavior as intentional, >but the spec was not updated accordingly and the behavior can still be wrong for non-default legacy >installs. ></pre> > > <code>[openspec/specs/docker-compose-stack/spec.md[74-81]](https://github.com/carverauto/serviceradar/blob/2e4264d1e68303fafb4f9dcab87e826d86a3f3a7/openspec/specs/docker-compose-stack/spec.md/#L74-L81)</code> > <code>[docker/compose/bootstrap-db-credentials.sh[63-108]](https://github.com/carverauto/serviceradar/blob/2e4264d1e68303fafb4f9dcab87e826d86a3f3a7/docker/compose/bootstrap-db-credentials.sh/#L63-L108)</code> > <code>[DOCKER_QUICKSTART.md[104-109]](https://github.com/carverauto/serviceradar/blob/2e4264d1e68303fafb4f9dcab87e826d86a3f3a7/DOCKER_QUICKSTART.md/#L104-L109)</code> ></details> > <details> ><summary>Agent prompt</summary> > > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >The credential bootstrap now auto-recovers legacy defaults on existing DB volumes without credentials/overrides, which contradicts the current docker-compose-stack spec (fail-fast requirement) and can persist incorrect/insecure defaults. > >## Issue Context >Docs state legacy recovery is intentional, but the canonical spec still mandates fail-fast. Also, if an operator had a legacy install with non-default credentials, auto-assuming defaults will write misleading values into the credentials volume and may cause confusing authentication failures. > >## Fix Focus Areas >- docker/compose/bootstrap-db-credentials.sh[63-127] >- openspec/specs/docker-compose-stack/spec.md[74-81] >- DOCKER_QUICKSTART.md[104-109] >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <img src="https://www.qodo.ai/wp-content/uploads/2026/01/review-recommended.png" height="20" alt="Remediation recommended"> <details> <summary> 2. Migrator fails when volume missing <code>📎 Requirement gap</code> <code>⛯ Reliability</code></summary> > <details open> ><summary>Description</summary> > > ><pre> >cnpg-migrator will exit non-zero if the cnpg-data Docker volume does not exist yet, which can >prevent db-credentials (and therefore cert-generator) from completing successfully. This can >cause docker compose up -d to fail in scenarios like fresh installs or unexpected volume removal. ></pre> ></details> > <details open> ><summary>Code</summary> > > ><code>[docker/compose/migrate-cnpg-pg16-to-pg18.sh[R216-217]](https://github.com/carverauto/serviceradar/pull/3003/files#diff-15b9aa3f74894d188bc3823423c699c10db386174fc503c00e9e08e76e94de46R216-R217)</code> > >```diff >+ensure_volume_exists "$DATA_VOLUME" >+ensure_compose_stack_stopped >``` ></details> > <details > ><summary>Evidence</summary> > > ><pre> >Compliance requires docker compose up -d to complete without db-credentials/cert-generator >startup failures (PR Compliance ID 1). The new Compose flow gates db-credentials on >cnpg-migrator success, but the migrator script currently hard-fails if the target data volume is >absent, which would cascade into Compose startup failure for the credential/cert bootstrap chain. ></pre> > > <code>[Docker Compose services start successfully without cert-generator or db-credentials exiting with errors](https://github.com/carverauto/serviceradar/issues/3002)</code> > <code>[docker-compose.yml[23-41]](https://github.com/carverauto/serviceradar/blob/2e4264d1e68303fafb4f9dcab87e826d86a3f3a7/docker-compose.yml/#L23-L41)</code> > <code>[docker-compose.yml[55-58]](https://github.com/carverauto/serviceradar/blob/2e4264d1e68303fafb4f9dcab87e826d86a3f3a7/docker-compose.yml/#L55-L58)</code> > <code>[docker/compose/migrate-cnpg-pg16-to-pg18.sh[68-73]](https://github.com/carverauto/serviceradar/blob/2e4264d1e68303fafb4f9dcab87e826d86a3f3a7/docker/compose/migrate-cnpg-pg16-to-pg18.sh/#L68-L73)</code> > <code>[docker/compose/migrate-cnpg-pg16-to-pg18.sh[216-223]](https://github.com/carverauto/serviceradar/blob/2e4264d1e68303fafb4f9dcab87e826d86a3f3a7/docker/compose/migrate-cnpg-pg16-to-pg18.sh/#L216-L223)</code> ></details> > <details> ><summary>Agent prompt</summary> > > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >`cnpg-migrator` hard-fails when the CNPG data volume does not exist, but `db-credentials` is gated on `cnpg-migrator` completing successfully. This can block Compose startup. > >## Issue Context >The migration should be a no-op for fresh installs and already-migrated volumes. A missing `cnpg-data` volume should be treated equivalently to an empty/fresh install. > >## Fix Focus Areas >- docker/compose/migrate-cnpg-pg16-to-pg18.sh[68-73] >- docker/compose/migrate-cnpg-pg16-to-pg18.sh[216-223] >- docker-compose.yml[23-41] >- docker-compose.yml[55-58] >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <details> <summary> 3. Migrator hardcodes chmod <code>🐞 Bug</code> <code>⛨ Security</code></summary> > <details open> ><summary>Description</summary> > > ><pre> >The migration script seeds the credentials volume with hardcoded 0644 permissions, ignoring >CNPG_CRED_MODE used by the normal bootstrap path. This creates inconsistent behavior and can >unintentionally widen credential readability when operators configure stricter permissions. ></pre> ></details> > <details open> ><summary>Code</summary> > > ><code>[docker/compose/migrate-cnpg-pg16-to-pg18.sh[R138-158]](https://github.com/carverauto/serviceradar/pull/3003/files#diff-15b9aa3f74894d188bc3823423c699c10db386174fc503c00e9e08e76e94de46R138-R158)</code> > >```diff >+seed_credentials_volume() { >+ if ! docker volume inspect "$CREDENTIALS_VOLUME" >/dev/null 2>&1; then >+ docker volume create "$CREDENTIALS_VOLUME" >/dev/null >+ fi >+ >+ docker run --rm \ >+ -e CNPG_SUPERUSER="${SOURCE_SUPERUSER}" \ >+ -e CNPG_SUPERUSER_PASSWORD="${SOURCE_SUPERUSER_PASSWORD}" \ >+ -e CNPG_PASSWORD="${APP_PASSWORD}" \ >+ -v "${CREDENTIALS_VOLUME}:/etc/serviceradar/cnpg" \ >+ alpine:3.20 \ >+ sh -ceu ' >+ umask 077 >+ mkdir -p /etc/serviceradar/cnpg >+ printf "%s" "$CNPG_SUPERUSER" > /etc/serviceradar/cnpg/superuser-username >+ printf "%s" "$CNPG_SUPERUSER_PASSWORD" > /etc/serviceradar/cnpg/superuser-password >+ printf "%s" "$CNPG_PASSWORD" > /etc/serviceradar/cnpg/serviceradar-password >+ chmod 0644 /etc/serviceradar/cnpg/superuser-username \ >+ /etc/serviceradar/cnpg/superuser-password \ >+ /etc/serviceradar/cnpg/serviceradar-password >+ ' >``` ></details> > <details > ><summary>Evidence</summary> > > ><pre> >The normal credentials bootstrap honors CNPG_CRED_MODE for permission control, but the migrator >always writes credential files as 0644, which may violate operator expectations and makes the two >seeding paths inconsistent. ></pre> > > <code>[docker/compose/migrate-cnpg-pg16-to-pg18.sh[138-158]](https://github.com/carverauto/serviceradar/blob/2e4264d1e68303fafb4f9dcab87e826d86a3f3a7/docker/compose/migrate-cnpg-pg16-to-pg18.sh/#L138-L158)</code> > <code>[docker/compose/bootstrap-db-credentials.sh[29-55]](https://github.com/carverauto/serviceradar/blob/2e4264d1e68303fafb4f9dcab87e826d86a3f3a7/docker/compose/bootstrap-db-credentials.sh/#L29-L55)</code> ></details> > <details> ><summary>Agent prompt</summary> > > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >The PG16→PG18 migrator seeds credential files with fixed `chmod 0644`, ignoring the existing `CNPG_CRED_MODE` convention used by `bootstrap-db-credentials.sh`. > >## Issue Context >Operators who set `CNPG_CRED_MODE` expecting tighter permissions will get widened permissions after migration, and bootstrap vs migrator behavior becomes inconsistent. > >## Fix Focus Areas >- docker/compose/migrate-cnpg-pg16-to-pg18.sh[138-159] >- docker/compose/bootstrap-db-credentials.sh[29-55] >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <details> <summary> 4. Migration guard too coarse <code>🐞 Bug</code> <code>⛯ Reliability</code></summary> > <details open> ><summary>Description</summary> > > ><pre> >In Compose auto-migration, the migrator bypasses its “compose project running” safety check, and the >remaining check is coarse (project-label based) rather than verifying the cnpg-data volume is not >currently mounted by a running Postgres container. Adding a direct “volume in use” check would >reduce risk in upgrade/partial-start scenarios. ></pre> ></details> > <details open> ><summary>Code</summary> > > ><code>[docker-compose.yml[R23-36]](https://github.com/carverauto/serviceradar/pull/3003/files#diff-e45e45baeda1c1e73482975a664062aa56f20c03dd9d64a827aba57775bed0d3R23-R36)</code> > >```diff >+ cnpg-migrator: >+ image: docker:28.0.1-cli >+ container_name: serviceradar-cnpg-migrator >+ environment: >+ - FORCE=true >+ - CNPG_ALLOW_RUNNING_COMPOSE_PROJECT=true >+ - SERVICERADAR_VOLUME_PREFIX=${SERVICERADAR_VOLUME_PREFIX:-serviceradar} >+ - CNPG_DATA_VOLUME=${SERVICERADAR_VOLUME_PREFIX:-serviceradar}_cnpg-data >+ - CNPG_CREDENTIALS_VOLUME=${SERVICERADAR_VOLUME_PREFIX:-serviceradar}_cnpg-credentials >+ - CNPG_DATABASE=${CNPG_DATABASE:-serviceradar} >+ - CNPG_SUPERUSER=${CNPG_SUPERUSER:-} >+ - CNPG_SUPERUSER_PASSWORD=${CNPG_SUPERUSER_PASSWORD:-} >+ - CNPG_PASSWORD=${CNPG_PASSWORD:-} >+ - CNPG_EXPECTED_PG_MAJOR=18 >``` ></details> > <details > ><summary>Evidence</summary> > > ><pre> >Compose explicitly sets CNPG_ALLOW_RUNNING_COMPOSE_PROJECT=true, which makes >ensure_compose_stack_stopped() return early; the only built-in guard is a project-level label >check (not a volume-in-use check). This makes the migrator more dependent on external operational >behavior (e.g., ensuring old cnpg is stopped) than it needs to be. ></pre> > > <code>[docker-compose.yml[23-36]](https://github.com/carverauto/serviceradar/blob/2e4264d1e68303fafb4f9dcab87e826d86a3f3a7/docker-compose.yml/#L23-L36)</code> > <code>[docker/compose/migrate-cnpg-pg16-to-pg18.sh[195-203]](https://github.com/carverauto/serviceradar/blob/2e4264d1e68303fafb4f9dcab87e826d86a3f3a7/docker/compose/migrate-cnpg-pg16-to-pg18.sh/#L195-L203)</code> ></details> > <details> ><summary>Agent prompt</summary> > > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >The migrator’s safety check is project-label based and is bypassed in Compose. It would be safer to also (or instead) verify that the CNPG data volume is not currently mounted by a running container before starting temporary Postgres containers. > >## Issue Context >Compose sets `CNPG_ALLOW_RUNNING_COMPOSE_PROJECT=true` so the current guard is effectively disabled. A direct “volume in use” check would protect against edge cases (partial upgrades, leftover running containers, manual operator actions) without blocking the intended Compose workflow. > >## Fix Focus Areas >- docker/compose/migrate-cnpg-pg16-to-pg18.sh[195-217] >- docker-compose.yml[23-36] >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <pre>ⓘ The new review experience is currently in Beta. <a href="https://docs.qodo.ai/qodo-documentation/code-review">Learn more</a></pre> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider">  <a href="https://www.qodo.ai"><img src="https://www.qodo.ai/wp-content/uploads/2025/03/qodo-logo.svg" width="80" alt="Qodo Logo"></a>

qodo-code-review[bot] commented

2026-03-07 20:10:18 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/3003#discussion_r2900432489
Original created: 2026-03-07T20:10:18Z
Original path: docker/compose/bootstrap-db-credentials.sh
Original line: 107

1. Legacy creds bypass spec 🐞 Bug ⛨ Security

The credential bootstrap now auto-assumes legacy default credentials for any existing CNPG data
volume that lacks persisted credentials and explicit overrides, instead of failing fast. This
contradicts the repo’s docker-compose-stack spec and risks persisting incorrect/insecure defaults
when the existing DB was not actually using those legacy values.

Agent Prompt

## Issue description
The credential bootstrap now auto-recovers legacy defaults on existing DB volumes without credentials/overrides, which contradicts the current docker-compose-stack spec (fail-fast requirement) and can persist incorrect/insecure defaults.

## Issue Context
Docs state legacy recovery is intentional, but the canonical spec still mandates fail-fast. Also, if an operator had a legacy install with non-default credentials, auto-assuming defaults will write misleading values into the credentials volume and may cause confusing authentication failures.

## Fix Focus Areas
- docker/compose/bootstrap-db-credentials.sh[63-127]
- openspec/specs/docker-compose-stack/spec.md[74-81]
- DOCKER_QUICKSTART.md[104-109]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Imported GitHub PR review comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/3003#discussion_r2900432489 Original created: 2026-03-07T20:10:18Z Original path: docker/compose/bootstrap-db-credentials.sh Original line: 107 --- <img src="https://www.qodo.ai/wp-content/uploads/2025/12/v2-action-required.svg" height="20" alt="Action required"> 1\. Legacy creds bypass spec <code>🐞 Bug</code> <code>⛨ Security</code> <pre> The credential bootstrap now auto-assumes legacy default credentials for any existing CNPG data volume that lacks persisted credentials and explicit overrides, instead of failing fast. This contradicts the repo’s docker-compose-stack spec and risks persisting incorrect/insecure defaults when the existing DB was not actually using those legacy values. </pre> <details> <summary>Agent Prompt</summary> ``` ## Issue description The credential bootstrap now auto-recovers legacy defaults on existing DB volumes without credentials/overrides, which contradicts the current docker-compose-stack spec (fail-fast requirement) and can persist incorrect/insecure defaults. ## Issue Context Docs state legacy recovery is intentional, but the canonical spec still mandates fail-fast. Also, if an operator had a legacy install with non-default credentials, auto-assuming defaults will write misleading values into the credentials volume and may cause confusing authentication failures. ## Fix Focus Areas - docker/compose/bootstrap-db-credentials.sh[63-127] - openspec/specs/docker-compose-stack/spec.md[74-81] - DOCKER_QUICKSTART.md[104-109] ``` <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> </details>